[squashme] revamp keep-configuration option backwards compatible

Author: chr4

abi-compat/jammy_0.107.xml

@@ -15,6 +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_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'/>
@@ -58,7 +59,6 @@
     <elf-symbol name='netplan_get_global_backend' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='netplan_get_id_from_nm_filename' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='netplan_get_id_from_nm_filepath' 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_backend' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='netplan_netdef_get_bond_link' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='netplan_netdef_get_bridge_link' 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='keep_configuration' type-id='type-id-42' visibility='default' filepath='../src/abi.h' line='204' column='1'/>
+        <var-decl name='critical' 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,6 +4191,10 @@
       <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'>
+      <parameter type-id='type-id-181' name='netdef' filepath='../src/types.c' line='597' column='1'/>
+      <return type-id='type-id-42'/>
+    </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'/>
       <return type-id='type-id-42'/>
@@ -4369,10 +4373,6 @@
       <parameter type-id='type-id-106' name='rootdir' filepath='../src/util.c' line='590' column='1'/>
       <return type-id='type-id-42'/>
     </function-decl>
-    <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-43'/>
-    </function-decl>
     <function-decl name='netplan_netdef_get_output_filename' mangled-name='netplan_netdef_get_output_filename' filepath='../src/util.c' line='645' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='netplan_netdef_get_output_filename'>
       <parameter type-id='type-id-181' name='netdef' filepath='../src/util.c' line='645' column='1'/>
       <parameter type-id='type-id-106' name='ssid' filepath='../src/util.c' line='645' column='1'/>

doc/netplan-yaml.md

@@ -374,14 +374,19 @@ 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.
 
-- **keep-configuration** (scalar)
+- **critical** (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"
+  > This option sets the systemd KeepConfiguration option.
+  > (not recognized by NetworkManager)
+  >
+  > 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)
 

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 network interfaces without keep_configuration set
+            # rename non-critical network interfaces
             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 interfaces not having keep_configuration set, which can be safely updated.
+        Only applies to non-critical interfaces 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.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'
+            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'
                                 .format(netdef.id, current_iface_name, newname))
                 continue
 

src/abi.h

@@ -32,6 +32,14 @@ typedef enum {
     NETPLAN_OPTIONAL_STATIC  = 1<<4,
 } NetplanOptionalAddressFlag;
 
+typedef enum {
+    NETPLAN_CRITICAL_TRUE,
+    NETPLAN_CRITICAL_FALSE,
+    NETPLAN_CRITICAL_STATIC,
+    NETPLAN_CRITICAL_DHCP,
+    NETPLAN_CRITICAL_DHCP_ON_STOP,
+} NetplanCriticalOption;
+
 /* Fields below are valid for dhcp4 and dhcp6 unless otherwise noted. */
 typedef struct dhcp_overrides {
     gboolean use_dns;
@@ -211,7 +219,7 @@ struct netplan_net_definition {
     /* status options */
     gboolean optional;
     NetplanOptionalAddressFlag optional_addresses;
-    char* keep_configuration;
+    NetplanCriticalOption critical;
 
     /* 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->keep_configuration)
-        YAML_STRING_PLAIN(def, event, emitter, "keep-configuration", def->keep_configuration);
+    if (def->critical)
+        YAML_STRING_PLAIN(def, event, emitter, "critical", def->critical);
 
     if (def->ignore_carrier)
         YAML_NONNULL_STRING_PLAIN(event, emitter, "ignore-carrier", "true");

src/networkd.c

@@ -916,13 +916,23 @@ netplan_netdef_write_network_file(
         }
     }
 
-    if (def->dhcp4 || def->dhcp6 || def->keep_configuration) {
+    if (def->dhcp4 || def->dhcp6 || def->critical) {
         /* NetworkManager compatible route metrics */
         g_string_append(network, "\n[DHCP]\n");
     }
 
-    if (def->keep_configuration)
-        g_string_append_printf(network, "KeepConfiguration=%s\n", def->keep_configuration);
+    if (def->critical) {
+        const char *s;
+        switch def->critical {
+            case NETPLAN_CRITICAL_TRUE: s = "true";
+            case NETPLAN_CRITICAL_STATIC: s = "static";
+            case NETPLAN_CRITICAL_DHCP: s = "dhcp";
+            case NETPLAN_CRITICAL_DHCP_ON_STOP: "dhcp-on-stop";
+            default: s = "no"
+        }
+
+        g_string_append_printf(network, "KeepConfiguration=%s\n", s);
+    }
 
     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}, \
-    {"keep-configuration", YAML_SCALAR_NODE, {.generic=handle_netdef_bool}, netdef_offset(keep_configuration)}, \
+    {"critical", YAML_SCALAR_NODE, {.generic=handle_netdef_bool}, netdef_offset(critical)}, \
     {"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;
-    FREE_AND_NULLIFY(netdef->keep_configuration);
+    FREE_AND_NULLIFY(netdef->critical);
 
     netdef->dhcp4 = FALSE;
     netdef->dhcp6 = FALSE;
@@ -594,6 +594,13 @@ _netplan_netdef_get_vlan_id(const NetplanNetDefinition* netdef)
     return netdef->vlan_id;
 }
 
+NetplanCriticalOption
+_netplan_netdef_get_critical(const NetplanNetDefinition* netdef)
+{
+    g_assert(netdef);
+    return netdef->critical;
+}
+
 gboolean
 _netplan_netdef_get_optional(const NetplanNetDefinition* netdef)
 {

src/util-internal.h

@@ -102,6 +102,9 @@ _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 NetplanCriticalOption
+_netplan_netdef_get_critical(const NetplanNetDefinition* netdef);
+
 NETPLAN_INTERNAL gboolean
 _netplan_netdef_get_optional(const NetplanNetDefinition* netdef);
 

src/util.c

@@ -1018,13 +1018,6 @@ netplan_netdef_match_interface(const NetplanNetDefinition* netdef, const char* n
     return TRUE;
 }
 
-char *
-netplan_netdef_get_keep_configuration(const NetplanNetDefinition* netdef)
-{
-    g_assert(netdef);
-    return netdef->keep_configuration;
-}
-
 ssize_t
 netplan_netdef_get_set_name(const NetplanNetDefinition* netdef, char* out_buffer, size_t out_buf_size)
 {

tests/generator/test_common.py

@@ -467,12 +467,12 @@ def test_bond_arp_ip_targets_multi_pass(self):
 Bond=bond0
 '''})
 
-    def test_dhcp_keep_configuration_true(self):
+    def test_dhcp_critical_true(self):
         self.generate('''network:
   version: 2
   ethernets:
     engreen:
-      keep-configuration: yes
+      critical: yes
 ''')
 
         self.assert_networkd({'engreen.network': '''[Match]

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_keep_configuration(self):
+    def test_critical(self):
         state = state_from_yaml(self.confdir, '''network:
   ethernets:
     eth0:
-      keep-configuration: true
+      critical: true
     eth1: {}''')
 
-        self.assertTrue(state['eth0'].keep_configuration)
-        self.assertFalse(state['eth1'].keep_configuration)
+        self.assertTrue(state['eth0'].critical)
+        self.assertFalse(state['eth1'].critical)
 
     def test_eq(self):
         state = state_from_yaml(self.confdir, '''network: