netplan.c: Don't drop files with just global values on 'set' (LP: #2027584)#382
Conversation
|
The test failure needs to be investigated. It's related to |
slyon
force-pushed
the
set-renderer-lp2027584
branch
2 times, most recently
from
ed3660a to
04e7d72
Compare
| # 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? |
There was a problem hiding this comment.
Yeah, well... we're not handling them as of now. But it's supposed to be fixed via LP#2003727
|
I fixed the autopkgtest, added another unit-test and rebased. |
daniloegea
left a comment
daniloegea
left a comment
There was a problem hiding this comment.
The fix looks correct to me.
One little left over after executing your reproducer is an, almost, empty 70-netplan-set.yaml file that is created from nowhere. I was wondering if we could avoid creating it as it's useless and maybe unexpected.
Maybe we could skip the default file creation in the loop below the code you added by checking if it's the fallback file, if there are no netdefs and if the default file wasn't loaded from disk. Something like this:
g_hash_table_iter_init(&hash_iter, perfile_netdefs);
while (g_hash_table_iter_next (&hash_iter, &key, &value)) {
const char *filename = key;
gboolean is_fallback = (g_strcmp0(filename, default_path) == 0);
GList* netdefs = value;
/* new code */
if (is_fallback && !netdefs && np_state->sources && !g_hash_table_contains(np_state->sources, default_path))
continue;
out_fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, 0600);
if (out_fd < 0)
goto file_error;
if (!netplan_netdef_list_write_yaml(np_state, netdefs, out_fd, filename, is_fallback, error))
goto cleanup; // LCOV_EXCL_LINE
close(out_fd);
}Do you think it would work?
…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: canonical#254 canonical#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
slyon
force-pushed
the
set-renderer-lp2027584
branch
from
04e7d72 to
f2c8f64
Compare
Oh very good catch! But instead of skipping the creation of that |
2 participants
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:
FR-4793
Description
Checklist
make checksuccessfully.make check-coverage).