Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Extend KeepConfiguration= functionality #409

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Extend KeepConfiguration= functionality #409

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e71e865
Migrate CriticalConnection (deprecated) to KeepConfiguration
chr4 Sep 19, 2023
7a1914e
Make netplan_netdef_get_keep_configuration() public
chr4 Sep 19, 2023
82442d8
[squashme] revamp keep-configuration option backwards compatible
chr4 Sep 22, 2023
8ae3b07
[squashme] howto pyhon enum plz?
chr4 Sep 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions abi-compat/jammy_0.107.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<elf-symbol name='_netplan_iter_defs_per_devtype_next' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
<elf-symbol name='_netplan_nameserver_iter_free' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
<elf-symbol name='_netplan_nameserver_iter_next' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
<elf-symbol name='_netplan_netdef_get_critical' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
<elf-symbol name='_netplan_netdef_get_keep_configuration' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
<elf-symbol name='_netplan_netdef_get_delay_vf_rebind' type='func-type' binding='global-binding' visibility='default-visibility' alias='netplan_netdef_get_delay_virtual_functions_rebind' is-defined='yes'/>
<elf-symbol name='_netplan_netdef_get_embedded_switch_mode' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
<elf-symbol name='_netplan_netdef_get_optional' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
Expand Down Expand Up @@ -421,7 +421,7 @@
<var-decl name='optional_addresses' type-id='type-id-39' visibility='default' filepath='../src/abi.h' line='203' column='1'/>
</data-member>
<data-member access='public' layout-offset-in-bits='320'>
<var-decl name='critical' type-id='type-id-42' visibility='default' filepath='../src/abi.h' line='204' column='1'/>
<var-decl name='keep_configuration' type-id='type-id-42' visibility='default' filepath='../src/abi.h' line='204' column='1'/>
</data-member>
<data-member access='public' layout-offset-in-bits='352'>
<var-decl name='dhcp4' type-id='type-id-42' visibility='default' filepath='../src/abi.h' line='207' column='1'/>
Expand Down Expand Up @@ -4191,9 +4191,9 @@
<parameter type-id='type-id-181' name='netdef' filepath='../src/types.c' line='590' column='1'/>
<return type-id='type-id-44'/>
</function-decl>
<function-decl name='_netplan_netdef_get_critical' mangled-name='_netplan_netdef_get_critical' filepath='../src/types.c' line='597' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='_netplan_netdef_get_critical'>
<function-decl name='_netplan_netdef_get_keep_configuration' mangled-name='_netplan_netdef_get_keep_configuration' filepath='../src/types.c' line='597' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='_netplan_netdef_get_keep_configuration'>
<parameter type-id='type-id-181' name='netdef' filepath='../src/types.c' line='597' column='1'/>
<return type-id='type-id-42'/>
<return type-id='type-id-43'/>
</function-decl>
<function-decl name='_netplan_netdef_get_optional' mangled-name='_netplan_netdef_get_optional' filepath='../src/types.c' line='604' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='_netplan_netdef_get_optional'>
<parameter type-id='type-id-181' name='netdef' filepath='../src/types.c' line='604' column='1'/>
Expand Down
13 changes: 8 additions & 5 deletions doc/netplan-yaml.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,11 +374,14 @@ Match devices by MAC when setting options like: `wakeonlan` or `*-offload`.
> (networkd backend only) Allow the specified interface to be configured even
> if it has no carrier.

- **critical** (bool)

> Designate the connection as "critical to the system", meaning that special
> care will be taken by to not release the assigned IP when the daemon is
> restarted. (not recognized by NetworkManager)
- **keep-configuration** (scalar)

> When set to "static", static addresses and routes won't be dropped on starting up process.
> When set to "dhcp-on-stop", addresses and routes won't be dropped when stopping the daemon.
> When set to "dhcp", addresses and routes provided by a DHCP server will never be dropped even if the DHCP lease expires, implies "dhcp-on-stop"
> When set to "yes", "dhcp" and "static" is implied.
> Defaults to "no".
> (not recognized by NetworkManager)

- **dhcp-identifier** (scalar)

Expand Down
10 changes: 5 additions & 5 deletions netplan_cli/cli/commands/apply.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ def command_apply(self, run_generate=True, sync=False, exit_on_error=True, state
devices_after_udev = netifaces.interfaces()
# apply some more changes manually
for iface, settings in changes.items():
# rename non-critical network interfaces
# rename network interfaces without keep_configuration set
new_name = settings.get('name')
if new_name:
if len(new_name) >= IF_NAMESIZE:
Expand Down Expand Up @@ -356,7 +356,7 @@ def clear_virtual_links(prev_links, curr_links, devices=[]):
def process_link_changes(interfaces, config_manager: ConfigManager): # pragma: nocover (covered in autopkgtest)
"""
Go through the pending changes and pick what needs special handling.
Only applies to non-critical interfaces which can be safely updated.
Only applies to interfaces not having keep_configuration set, which can be safely updated.
"""

changes = {}
Expand Down Expand Up @@ -384,9 +384,9 @@ def process_link_changes(interfaces, config_manager: ConfigManager): # pragma:
# Skip interface if it already has the correct name
logging.debug('Skipping correctly named interface: {}'.format(newname))
continue
if netdef.critical:
# Skip interfaces defined as critical, as we should not take them down in order to rename
logging.warning('Cannot rename {} ({} -> {}) at runtime (needs reboot), due to being critical'
if netdef.keep_configuration:
# Skip interfaces with keep_configuration set, as we should not take them down in order to rename
logging.warning('Cannot rename {} ({} -> {}) at runtime (needs reboot), due to keep_configuration being set'
.format(netdef.id, current_iface_name, newname))
continue

Expand Down
2 changes: 1 addition & 1 deletion python-cffi/netplan/_build_cffi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@
ssize_t _netplan_netdef_get_embedded_switch_mode(const NetplanNetDefinition* netdef, char* out_buffer, size_t out_buf_size);
gboolean _netplan_netdef_get_sriov_vlan_filter(const NetplanNetDefinition* netdef);
guint _netplan_netdef_get_vlan_id(const NetplanNetDefinition* netdef);
gboolean _netplan_netdef_get_critical(const NetplanNetDefinition* netdef);
char* _netplan_netdef_get_keep_configuration(const NetplanNetDefinition* netdef);
gboolean _netplan_netdef_is_trivial_compound_itf(const NetplanNetDefinition* netdef);
int _netplan_state_get_vf_count_for_def(
const NetplanState* np_state, const NetplanNetDefinition* netdef, NetplanError** error);
Expand Down
4 changes: 2 additions & 2 deletions python-cffi/netplan/netdef.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ def set_name(self) -> str:
return _string_realloc_call_no_error(lambda b: lib.netplan_netdef_get_set_name(self._ptr, b, len(b)))

@property
def critical(self) -> bool:
return bool(lib._netplan_netdef_get_critical(self._ptr))
def keep_configuration(self) -> str:
return _string_realloc_call_no_error(lambda b: lib.netplan_netdef_get_keep_configuration(self._ptr, b, len(b)))

@property
def links(self) -> dict:
Expand Down
2 changes: 1 addition & 1 deletion src/abi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ struct netplan_net_definition {
/* status options */
gboolean optional;
NetplanOptionalAddressFlag optional_addresses;
gboolean critical;
char* keep_configuration;
Copy link
Collaborator

@daniloegea daniloegea Sep 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make things simpler, we should use an enum for this option. It has a well defined set of possible values, so it would naturally fit well in an enum:

enum {
    NETPLAN_CRITICAL_TRUE,
    NETPLAN_CRITICAL_FALSE,
    NETPLAN_CRITICAL_STATIC,
    ...,
} CriticalOptions;

Please, check my comments in the summary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! Let me revamp

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been trying to rework this, but I'm not sure if I'm on the right track, as quite a few tests are failing or even segfaulting.


/* addresses */
gboolean dhcp4;
Expand Down
4 changes: 2 additions & 2 deletions src/netplan.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -742,8 +742,8 @@ _serialize_yaml(

if (def->optional)
YAML_NONNULL_STRING_PLAIN(event, emitter, "optional", "true");
if (def->critical)
YAML_NONNULL_STRING_PLAIN(event, emitter, "critical", "true");
if (def->keep_configuration)
YAML_STRING_PLAIN(def, event, emitter, "keep-configuration", def->keep_configuration);

if (def->ignore_carrier)
YAML_NONNULL_STRING_PLAIN(event, emitter, "ignore-carrier", "true");
Expand Down
10 changes: 5 additions & 5 deletions src/networkd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ write_bond_parameters(const NetplanNetDefinition* def, GString* s)
if (def->bond_params.selection_logic)
g_string_append_printf(params, "\nAdSelect=%s", def->bond_params.selection_logic);
if (def->bond_params.all_members_active)
g_string_append_printf(params, "\nAllSlavesActive=%d", def->bond_params.all_members_active); /* wokeignore:rule=slave */
g_string_append_printf(params, "\nAllSlavesActive=%d", def->bond_params.all_members_active); /* wokeignore:rule=slave */
if (def->bond_params.arp_interval) {
g_string_append(params, "\nARPIntervalSec=");
if (interval_has_suffix(def->bond_params.arp_interval))
Expand Down Expand Up @@ -393,7 +393,7 @@ write_bond_parameters(const NetplanNetDefinition* def, GString* s)
g_string_append_printf(params, "\nGratuitousARP=%d", def->bond_params.gratuitous_arp);
/* TODO: add unsolicited_na, not documented as supported by NM. */
if (def->bond_params.packets_per_member)
g_string_append_printf(params, "\nPacketsPerSlave=%d", def->bond_params.packets_per_member); /* wokeignore:rule=slave */
g_string_append_printf(params, "\nPacketsPerSlave=%d", def->bond_params.packets_per_member); /* wokeignore:rule=slave */
if (def->bond_params.primary_reselect_policy)
g_string_append_printf(params, "\nPrimaryReselectPolicy=%s", def->bond_params.primary_reselect_policy);
if (def->bond_params.resend_igmp)
Expand Down Expand Up @@ -916,13 +916,13 @@ netplan_netdef_write_network_file(
}
}

if (def->dhcp4 || def->dhcp6 || def->critical) {
if (def->dhcp4 || def->dhcp6 || def->keep_configuration) {
/* NetworkManager compatible route metrics */
g_string_append(network, "\n[DHCP]\n");
}

if (def->critical)
g_string_append_printf(network, "CriticalConnection=true\n");
if (def->keep_configuration)
g_string_append_printf(network, "KeepConfiguration=%s\n", def->keep_configuration);

if (def->dhcp4 || def->dhcp6) {
if (def->dhcp_identifier)
Expand Down
2 changes: 1 addition & 1 deletion src/parse.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2785,7 +2785,7 @@ static const mapping_entry_handler dhcp6_overrides_handlers[] = {
{"accept-ra", YAML_SCALAR_NODE, {.generic=handle_accept_ra}, netdef_offset(accept_ra)}, \
{"activation-mode", YAML_SCALAR_NODE, {.generic=handle_activation_mode}, netdef_offset(activation_mode)}, \
{"addresses", YAML_SEQUENCE_NODE, {.generic=handle_addresses}, NULL}, \
{"critical", YAML_SCALAR_NODE, {.generic=handle_netdef_bool}, netdef_offset(critical)}, \
{"keep-configuration", YAML_SCALAR_NODE, {.generic=handle_netdef_bool}, netdef_offset(keep_configuration)}, \
{"ignore-carrier", YAML_SCALAR_NODE, {.generic=handle_netdef_bool}, netdef_offset(ignore_carrier)}, \
{"dhcp4", YAML_SCALAR_NODE, {.generic=handle_netdef_bool}, netdef_offset(dhcp4)}, \
{"dhcp6", YAML_SCALAR_NODE, {.generic=handle_netdef_bool}, netdef_offset(dhcp6)}, \
Expand Down
8 changes: 4 additions & 4 deletions src/types.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ reset_netdef(NetplanNetDefinition* netdef, NetplanDefType new_type, NetplanBacke

netdef->optional = FALSE;
netdef->optional_addresses = 0;
netdef->critical = FALSE;
FREE_AND_NULLIFY(netdef->keep_configuration);

netdef->dhcp4 = FALSE;
netdef->dhcp6 = FALSE;
Expand Down Expand Up @@ -594,11 +594,11 @@ _netplan_netdef_get_vlan_id(const NetplanNetDefinition* netdef)
return netdef->vlan_id;
}

gboolean
_netplan_netdef_get_critical(const NetplanNetDefinition* netdef)
char *
_netplan_netdef_get_keep_configuration(const NetplanNetDefinition* netdef)
{
g_assert(netdef);
return netdef->critical;
return netdef->keep_configuration;
}

gboolean
Expand Down
4 changes: 2 additions & 2 deletions src/util-internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ _netplan_state_get_vf_count_for_def(const NetplanState* np_state, const NetplanN
NETPLAN_INTERNAL gboolean
_netplan_netdef_get_sriov_vlan_filter(const NetplanNetDefinition* netdef);

NETPLAN_INTERNAL gboolean
_netplan_netdef_get_critical(const NetplanNetDefinition* netdef);
NETPLAN_INTERNAL char *
_netplan_netdef_get_keep_configuration(const NetplanNetDefinition* netdef);

NETPLAN_INTERNAL gboolean
_netplan_netdef_get_optional(const NetplanNetDefinition* netdef);
Expand Down
6 changes: 3 additions & 3 deletions tests/generator/test_common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,12 +467,12 @@ def test_bond_arp_ip_targets_multi_pass(self):
Bond=bond0
'''})

def test_dhcp_critical_true(self):
def test_dhcp_keep_configuration_true(self):
self.generate('''network:
version: 2
ethernets:
engreen:
critical: yes
keep-configuration: yes
''')

self.assert_networkd({'engreen.network': '''[Match]
Expand All @@ -482,7 +482,7 @@ def test_dhcp_critical_true(self):
LinkLocalAddressing=ipv6

[DHCP]
CriticalConnection=true
KeepConfiguration=true
'''})

def test_dhcp_identifier_mac(self):
Expand Down
8 changes: 4 additions & 4 deletions tests/test_libnetplan.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,15 +581,15 @@ def test_backend(self):
self.assertEqual(state['eth0'].backend, 'networkd')
self.assertEqual(state['eth1'].backend, 'NetworkManager')

def test_critical(self):
def test_keep_configuration(self):
state = state_from_yaml(self.confdir, '''network:
ethernets:
eth0:
critical: true
keep-configuration: true
eth1: {}''')

self.assertTrue(state['eth0'].critical)
self.assertFalse(state['eth1'].critical)
self.assertTrue(state['eth0'].keep_configuration)
self.assertFalse(state['eth1'].keep_configuration)

def test_eq(self):
state = state_from_yaml(self.confdir, '''network:
Expand Down