netplan.c: Don't drop files with just global values on 'set' (LP: #2027584)

src/netplan.c

@@ -1198,8 +1198,10 @@ netplan_state_update_yaml_hierarchy(const NetplanState* np_state, const char* de
 
     /* Dump global conf to the default path */
     if (!np_state->netdefs || g_hash_table_size(np_state->netdefs) == 0) {
-        if ((np_state->backend != NETPLAN_BACKEND_NONE)
-                || has_openvswitch(&np_state->ovs_settings, NETPLAN_BACKEND_NONE, NULL)) {
+        if (   has_openvswitch(&np_state->ovs_settings, NETPLAN_BACKEND_NONE, NULL)
+            || (np_state->backend != NETPLAN_BACKEND_NONE && np_state->global_renderer &&
+                (   g_hash_table_contains(np_state->global_renderer, default_path) // 70-netplan-set.yaml already exsits and defines a global renderer
+                 || g_hash_table_contains(np_state->global_renderer, "")))) { // 70-netplan-set.yaml doesn't exist, but we need to create it to define a global renderer
             g_hash_table_insert(perfile_netdefs, default_path, NULL);
         }
     } else {
@@ -1214,6 +1216,22 @@ netplan_state_update_yaml_hierarchy(const NetplanState* np_state, const char* de
         }
     }
 
+    /* Add files containing a global renderer value to "perfile_netdefs", so
+     * they are updated on disk. */
+    if (np_state->global_renderer && g_hash_table_size(np_state->global_renderer) > 0) {
+        g_hash_table_iter_init(&hash_iter, np_state->global_renderer);
+        while (g_hash_table_iter_next (&hash_iter, &key, &value)) {
+            char *filename = key;
+            /* Anonymous globals will go to the default YAML (see above) */
+            if (g_strcmp0(filename, "") == 0)
+                continue;
+            /* Ignore the update of this file if it's already going to be
+             * written, caused by updated netdefs. */
+            if (!g_hash_table_contains(perfile_netdefs, filename))
+                g_hash_table_insert(perfile_netdefs, filename, NULL);
+        }
+    }
+
     g_hash_table_iter_init(&hash_iter, perfile_netdefs);
     while (g_hash_table_iter_next (&hash_iter, &key, &value)) {
         const char *filename = key;

tests/cli/test_get_set.py

@@ -25,6 +25,7 @@
 
 import yaml
 
+from netplan.cli.commands.set import FALLBACK_FILENAME
 from netplan.libnetplan import NetplanException
 from tests.test_utils import call_cli
 
@@ -248,6 +249,78 @@ def test_set_network_null_global(self):
         self.assertFalse(os.path.isfile(self.path))
         self.assertFalse(os.path.isfile(some_hint))
 
+    def test_set_no_netdefs_just_globals(self):  # LP: #2027584
+        keepme1 = os.path.join(self.workdir.name, 'etc', 'netplan',
+                               '00-no-netdefs-just-renderer.yaml')
+        with open(keepme1, 'w') as f:
+            f.write('''network: {renderer: NetworkManager}''')
+        keepme2 = os.path.join(self.workdir.name, 'etc', 'netplan',
+                               '00-no-netdefs-just-version.yaml')
+        with open(keepme2, 'w') as f:
+            f.write('''network: {version: 2}''')
+        deleteme = os.path.join(self.workdir.name, 'etc', 'netplan',
+                                '90-some-netdefs.yaml')
+        with open(deleteme, 'w') as f:
+            f.write('''network: {ethernets: {eth99: {dhcp4: true}}}''')
+
+        self._set(['ethernets.eth99=NULL'])
+        self.assertFalse(os.path.isfile(deleteme))
+        self.assertTrue(os.path.isfile(keepme1))
+        with open(keepme1, 'r') as f:
+            yml = yaml.safe_load(f)
+            self.assertEqual('NetworkManager', yml['network']['renderer'])
+        # XXX: It's probably fine to delete a file that just contains "version: 2"
+        #      Or is it? And what about other globals, such as OVS ports?
+        self.assertFalse(os.path.isfile(keepme2))
+
+    def test_set_clear_netdefs_keep_globals(self):  # LP: #2027584
+        keep = os.path.join(self.workdir.name, 'etc', 'netplan', '00-keep.yaml')
+        with open(keep, 'w') as f:
+            f.write('''network:
+  version: 2
+  renderer: NetworkManager
+  bridges:
+    br54:
+      addresses: [1.2.3.4/24]
+''')
+        self._set(['network.bridges.br54=NULL'])
+        self.assertTrue(os.path.isfile(keep))
+        with open(keep, 'r') as f:
+            yml = yaml.safe_load(f)
+            self.assertEqual(2, yml['network']['version'])
+            self.assertEqual('NetworkManager', yml['network']['renderer'])
+            self.assertNotIn('bridges', yml['network'])
+        default = os.path.join(self.workdir.name, 'etc', 'netplan', FALLBACK_FILENAME)
+        self.assertFalse(os.path.isfile(default))
+
+    def test_set_clear_netdefs_keep_globals_default_renderer(self):
+        keep = os.path.join(self.workdir.name, 'etc', 'netplan', '00-keep.yaml')
+        with open(keep, 'w') as f:
+            f.write('''network:
+  version: 2
+  renderer: NetworkManager
+  bridges:
+    br54:
+      addresses: [1.2.3.4/24]
+''')
+        default = os.path.join(self.workdir.name, 'etc', 'netplan', FALLBACK_FILENAME)
+        with open(default, 'w') as f:
+            f.write('''network:
+  renderer: networkd
+''')
+        self._set(['network.bridges.br54=NULL'])
+        self.assertTrue(os.path.isfile(keep))
+        with open(keep, 'r') as f:
+            yml = yaml.safe_load(f)
+            self.assertEqual(2, yml['network']['version'])
+            self.assertEqual('NetworkManager', yml['network']['renderer'])
+            self.assertNotIn('bridges', yml['network'])
+        self.assertTrue(os.path.isfile(default))
+        with open(default, 'r') as f:
+            yml = yaml.safe_load(f)
+            self.assertEqual(2, yml['network']['version'])
+            self.assertEqual('networkd', yml['network']['renderer'])
+
     def test_set_invalid(self):
         with self.assertRaises(Exception) as context:
             self._set(['xxx.yyy=abc'])