Skip to content

Commit

Permalink
generate: migrate to the new libnetplan APIs
Browse files Browse the repository at this point in the history 
This allows us to do proper cleanup on error cases.

Note that since generate was the only consumer of some of the legacy
APIs, those endpoints are not hit by the testsuite anymore and are thus
marked as ignored by the coverage calculations.
  • Loading branch information
@schopin-pro
schopin-pro committed Nov 12, 2021
1 parent fb92447 commit adde219
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 49 deletions.
8 changes: 6 additions & 2 deletions src/abi_compat.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,13 @@ char _global_backend_off[8+offsetof(struct netplan_state, backend)] = {};
NETPLAN_ABI
NetplanState global_state = {};

// LCOV_EXCL_START
NetplanBackend
netplan_get_global_backend()
{
return netplan_state_get_backend(&global_state);
}
// LCOV_EXCL_STOP

/**
* Clear NetplanNetDefinition hashtable
Expand All @@ -79,6 +81,7 @@ netplan_clear_netdefs()
return n;
}

// LCOV_EXCL_START
NETPLAN_INTERNAL void
write_network_file(const NetplanNetDefinition* def, const char* rootdir, const char* path)
{
Expand Down Expand Up @@ -118,7 +121,6 @@ cleanup_networkd_conf(const char* rootdir)

// There only for compatibility purposes, the proper implementation is now directly
// in the `generate` binary.
// LCOV_EXCL_START
NETPLAN_ABI void
enable_networkd(const char* generator_dir)
{
Expand All @@ -137,7 +139,6 @@ enable_networkd(const char* generator_dir)
exit(1);
}
}
// LCOV_EXCL_STOP

NETPLAN_INTERNAL void
write_nm_conf(NetplanNetDefinition* def, const char* rootdir)
Expand Down Expand Up @@ -184,6 +185,7 @@ cleanup_ovs_conf(const char* rootdir)
{
netplan_ovs_cleanup(rootdir);
}
// LCOV_EXCL_STOP

gboolean
netplan_parse_yaml(const char* filename, GError** error)
Expand Down Expand Up @@ -236,6 +238,7 @@ netplan_parse_keyfile(const char* filename, GError** error)
return netplan_parser_load_keyfile(&global_parser, filename, error);
}

// LCOV_EXCL_START
void process_input_file(const char *f)
{
GError* error = NULL;
Expand All @@ -257,6 +260,7 @@ process_yaml_hierarchy(const char* rootdir)
}
return TRUE;
}
// LCOV_EXCL_STOP

/**
* Helper function for testing only
Expand Down
93 changes: 54 additions & 39 deletions src/generate.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include "util.h"
#include "util-internal.h"
#include "parse.h"
#include "parse-globals.h"
#include "names.h"
#include "networkd.h"
#include "nm.h"
Expand All @@ -38,7 +37,7 @@

static gchar* rootdir;
static gchar** files;
static gboolean any_networkd;
static gboolean any_networkd = FALSE;
static gboolean any_sriov;
static gchar* mapping_iface;

Expand Down Expand Up @@ -110,22 +109,8 @@ start_unit_jit(gchar *unit)
};
// LCOV_EXCL_STOP

static void
nd_iterator_list(gpointer value, gpointer user_data)
{
NetplanNetDefinition* def = (NetplanNetDefinition*) value;
if (write_networkd_conf(def, (const char*) user_data))
any_networkd = TRUE;

write_ovs_conf(def, (const char*) user_data);
write_nm_conf(def, (const char*) user_data);
if (def->sriov_explicit_vf_count < G_MAXUINT || def->sriov_link)
any_sriov = TRUE;
}


static int
find_interface(gchar* interface)
find_interface(gchar* interface, GHashTable* netdefs)
{
GPtrArray *found;
GFileInfo *info;
Expand Down Expand Up @@ -200,6 +185,14 @@ find_interface(gchar* interface)
return ret;
}

#define CHECK_CALL(call) {\
if (!call) {\
error_code = 1; \
fprintf(stderr, "%s\n", error->message); \
goto cleanup;\
}\
}

int main(int argc, char** argv)
{
GError* error = NULL;
Expand All @@ -208,6 +201,9 @@ int main(int argc, char** argv)
gboolean called_as_generator = (strstr(argv[0], "systemd/system-generators/") != NULL);
g_autofree char* generator_run_stamp = NULL;
glob_t gl;
int error_code = 0;
NetplanParser* npp = NULL;
NetplanState* np_state = NULL;

/* Parse CLI options */
opt_context = g_option_context_new(NULL);
Expand All @@ -222,7 +218,7 @@ int main(int argc, char** argv)
g_option_context_add_main_entries(opt_context, options, NULL);

if (!g_option_context_parse(opt_context, &argc, &argv, &error)) {
g_fprintf(stderr, "failed to parse options: %s\n", error->message);
fprintf(stderr, "failed to parse options: %s\n", error->message);
return 1;
}

Expand All @@ -238,34 +234,48 @@ int main(int argc, char** argv)
}
}

npp = netplan_parser_new();
/* Read all input files */
if (files && !called_as_generator) {
for (gchar** f = files; f && *f; ++f)
process_input_file(*f);
} else if (!process_yaml_hierarchy(rootdir))
return 1; // LCOV_EXCL_LINE

netdefs = netplan_finish_parse(&error);
if (error) {
g_fprintf(stderr, "%s\n", error->message);
exit(1);
}
for (gchar** f = files; f && *f; ++f) {
CHECK_CALL(netplan_parser_load_yaml(npp, *f, &error));
}
} else
CHECK_CALL(netplan_parser_load_yaml_hierarchy(npp, rootdir, &error));

np_state = netplan_state_new();
CHECK_CALL(netplan_state_import_parser_results(np_state, npp, &error));

/* Clean up generated config from previous runs */
cleanup_networkd_conf(rootdir);
cleanup_nm_conf(rootdir);
cleanup_ovs_conf(rootdir);
netplan_networkd_cleanup(rootdir);
netplan_nm_cleanup(rootdir);
netplan_ovs_cleanup(rootdir);

cleanup_sriov_conf(rootdir);

if (mapping_iface && netdefs)
return find_interface(mapping_iface);
if (mapping_iface && np_state->netdefs) {
error_code = find_interface(mapping_iface, np_state->netdefs);
goto cleanup;
}

/* Generate backend specific configuration files from merged data. */
write_ovs_conf_finish(rootdir); // OVS cleanup unit is always written
if (netdefs) {
CHECK_CALL(netplan_state_finish_ovs_write(np_state, rootdir, &error)); // OVS cleanup unit is always written
if (np_state->netdefs) {
g_debug("Generating output files..");
g_list_foreach (netdefs_ordered, nd_iterator_list, rootdir);
write_nm_conf_finish(rootdir);
for (GList* iterator = np_state->netdefs_ordered; iterator; iterator = iterator->next) {
NetplanNetDefinition* def = (NetplanNetDefinition*) iterator->data;
gboolean has_been_written = FALSE;
CHECK_CALL(netplan_netdef_write_networkd(np_state, def, rootdir, &has_been_written, &error));
any_networkd = any_networkd || has_been_written;

CHECK_CALL(netplan_netdef_write_ovs(np_state, def, rootdir, &has_been_written, &error));
CHECK_CALL(netplan_netdef_write_nm(np_state, def, rootdir, &has_been_written, &error));

if (def->sriov_explicit_vf_count < G_MAXUINT || def->sriov_link)
any_sriov = TRUE;
}

CHECK_CALL(netplan_state_finish_nm_write(np_state, rootdir, &error));
if (any_sriov) write_sriov_conf_finish(rootdir);
/* We may have written .rules & .link files, thus we must
* invalidate udevd cache of its config as by default it only
Expand All @@ -278,7 +288,7 @@ int main(int argc, char** argv)

/* Disable /usr/lib/NetworkManager/conf.d/10-globally-managed-devices.conf
* (which restricts NM to wifi and wwan) if global renderer is NM */
if (netplan_get_global_backend() == NETPLAN_BACKEND_NM)
if (netplan_state_get_backend(np_state) == NETPLAN_BACKEND_NM)
g_string_free_to_file(g_string_new(NULL), rootdir, "/run/NetworkManager/conf.d/10-globally-managed-devices.conf", NULL);

if (called_as_generator) {
Expand Down Expand Up @@ -320,5 +330,10 @@ int main(int argc, char** argv)
// LCOV_EXCL_STOP
}

return 0;
cleanup:
if (npp)
netplan_parser_clear(&npp);
if (np_state)
netplan_state_clear(&np_state);
return error_code;
}
16 changes: 8 additions & 8 deletions tests/generator/test_dhcp_overrides.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ def assert_dhcp_overrides_bool(self, override_name, networkd_name):
dhcp6-overrides:
%s: no
''' % (override_name, override_name), expect_fail=True)
self.assertEqual(err, 'ERROR: engreen: networkd requires that '
'%s has the same value in both dhcp4_overrides and dhcp6_overrides\n' % override_name)
self.assertEqual(err.strip(), 'ERROR: engreen: networkd requires that '
'%s has the same value in both dhcp4_overrides and dhcp6_overrides' % override_name)

# Common tests for dhcp override strings
def assert_dhcp_overrides_string(self, override_name, networkd_name):
Expand Down Expand Up @@ -164,8 +164,8 @@ def assert_dhcp_overrides_string(self, override_name, networkd_name):
dhcp6-overrides:
%s: bar
''' % (override_name, override_name), expect_fail=True)
self.assertEqual(err, 'ERROR: engreen: networkd requires that '
'%s has the same value in both dhcp4_overrides and dhcp6_overrides\n' % override_name)
self.assertEqual(err.strip(), 'ERROR: engreen: networkd requires that '
'%s has the same value in both dhcp4_overrides and dhcp6_overrides' % override_name)

# Common tests for dhcp override booleans
def assert_dhcp_mtu_overrides_bool(self, override_name, networkd_name):
Expand Down Expand Up @@ -255,8 +255,8 @@ def assert_dhcp_mtu_overrides_bool(self, override_name, networkd_name):
dhcp6-overrides:
%s: no
''' % (override_name, override_name), expect_fail=True)
self.assertEqual(err, 'ERROR: engreen: networkd requires that '
'%s has the same value in both dhcp4_overrides and dhcp6_overrides\n' % override_name)
self.assertEqual(err.strip(), 'ERROR: engreen: networkd requires that '
'%s has the same value in both dhcp4_overrides and dhcp6_overrides' % override_name)

def assert_dhcp_overrides_guint(self, override_name, networkd_name):
# dhcp4 only
Expand Down Expand Up @@ -337,8 +337,8 @@ def assert_dhcp_overrides_guint(self, override_name, networkd_name):
dhcp6-overrides:
%s: 5555
''' % (override_name, override_name), expect_fail=True)
self.assertEqual(err, 'ERROR: engreen: networkd requires that '
'%s has the same value in both dhcp4_overrides and dhcp6_overrides\n' % override_name)
self.assertEqual(err.strip(), 'ERROR: engreen: networkd requires that '
'%s has the same value in both dhcp4_overrides and dhcp6_overrides' % override_name)

def test_dhcp_overrides_use_dns(self):
self.assert_dhcp_overrides_bool('use-dns', 'UseDNS')
Expand Down

0 comments on commit adde219

Please sign in to comment.