Skip to content

Commit

Permalink
netplan.c: Don't drop files with just global values on 'set' (LP: #20…
Browse files Browse the repository at this point in the history
…27584)

The "unlink" part at the bottom of netplan.c:netplan_state_update_yaml_hierarchy
seems to ignore any global values (such as "renderer") and operates on files
containing netdefs only.

The issue is happens due to a combination of the following PRs:
#254
#299

Which got SRUed into Jammy via 0.105-0ubuntu2~22.04.2

Here's a minimal reproducer:
```
netplan set network.renderer=NetworkManager --origin-hint=00-no-netdefs-just-globals
netplan set network.ethernets.eth99.dhcp4=true --origin-hint=90-some-netdefs
ls -l /etc/netplan/
netplan set network.ethernets.eth99=NULL
cat /etc/netplan/00-no-netdefs-just-globals.yaml
```

FR-4793
  • Loading branch information
slyon committed Jul 20, 2023
1 parent 2a02fd0 commit ed3660a
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 0 deletions.
16 changes: 16 additions & 0 deletions src/netplan.c
Original file line number Diff line number Diff line change
Expand Up @@ -1214,6 +1214,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;
Expand Down
42 changes: 42 additions & 0 deletions tests/cli/test_get_set.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,48 @@ 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'])

def test_set_invalid(self):
with self.assertRaises(Exception) as context:
self._set(['xxx.yyy=abc'])
Expand Down

0 comments on commit ed3660a

Please sign in to comment.