Migrate CriticalConnection (deprecated) to KeepConfiguration

See: https://bugs.launchpad.net/netplan/+bug/1896799

Author: chr4

abi-compat/jammy_0.107.xml

@@ -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'/>
@@ -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'/>
@@ -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'/>

doc/netplan-yaml.md

@@ -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", drop 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)
 

netplan_cli/cli/commands/apply.py

@@ -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:
@@ -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 = {}
@@ -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
 

python-cffi/netplan/_build_cffi.py

@@ -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);

python-cffi/netplan/netdef.py

@@ -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:

src/abi.h

@@ -211,7 +211,7 @@ struct netplan_net_definition {
     /* status options */
     gboolean optional;
     NetplanOptionalAddressFlag optional_addresses;
-    gboolean critical;
+    char* keep_configuration;
 
     /* addresses */
     gboolean dhcp4;

src/netplan.c

@@ -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");

src/networkd.c

@@ -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))
@@ -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)
@@ -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)

src/parse.c

@@ -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)}, \

src/types.c

@@ -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;
@@ -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

src/util-internal.h

@@ -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);

tests/generator/test_common.py

@@ -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]
@@ -482,7 +482,7 @@ def test_dhcp_critical_true(self):
 LinkLocalAddressing=ipv6
 
 [DHCP]
-CriticalConnection=true
+KeepConfiguration=true
 '''})
 
     def test_dhcp_identifier_mac(self):

tests/test_libnetplan.py

@@ -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: