Merge ~slyon/snappy-hwe-snaps/+git/network-manager:slyon/drop-vendorized-netplan into ~snappy-hwe-team/snappy-hwe-snaps/+git/network-manager:snap-20

Proposed by Lukas Märdian
Status: Merged
Approved by: Alfonso Sanchez-Beato
Approved revision: b30293022f595124fe8cebd6e6ac3f17f3ea84d8
Merged at revision: ecc97bad2af908d74162d16b0efc2e67b4b1d01a
Proposed branch: ~slyon/snappy-hwe-snaps/+git/network-manager:slyon/drop-vendorized-netplan
Merge into: ~snappy-hwe-team/snappy-hwe-snaps/+git/network-manager:snap-20
Diff against target: 537 lines (+70/-211)
3 files modified
dev/null (+0/-83)
snap-patch/networkmanager/0002-nm-netplan-keyfile.patch (+63/-93)
snap/snapcraft.yaml (+7/-35)
Reviewer Review Type Date Requested Status
System Enablement Bot continuous-integration Approve
Alfonso Sanchez-Beato Approve
Review via email: mp+412634@code.launchpad.net

Commit message

Drop vendorized netplan

All required changes landed:
- https://github.com/snapcore/snapd/pull/10212
- https://github.com/canonical/netplan/pull/208
- https://bugs.launchpad.net/snappy/+bug/1926442

Refresh 0002-nm-netplan-keyfile.patch

netplan 0.103 compat changes are now integrated with the original patch, as
that version landed in UC20, changes to NM's unit-tests are not needed anymore.

Also, avoid calling into the netplan_generate() libnetplan API, this would just
execute the 'netplan generate' CLI, which would then call the
io.netplan.Netplan.Generate() DBus API in snap environments.
We can avoid this additional redirection and be independent from deprecation
or changes that are incompatible with the snap environment in the
netplan_generate() API, by calling the 'netplan generate' CLI directly.
Ideally, in the future we would avoid this redirection as well and call into
the io.netplan.Netplan.Generate() DBus method directly. But we need to pass a
--root-dir argument during the execution of NM's unit-tests, that is currently
not supported by the DBus method, so let's stick with the CLI for now.

This depends on the snapd snap to be >= 2.53.2+git864.g5823952 (currently in snapd/edge).

Description of the change

Drop vendorized netplan

All required changes landed:
- https://github.com/snapcore/snapd/pull/10212
- https://github.com/canonical/netplan/pull/208
- https://bugs.launchpad.net/snappy/+bug/1926442

Refresh 0002-nm-netplan-keyfile.patch

netplan 0.103 compat changes are now integrated with the original patch, as
that version landed in UC20, changes to NM's unit-tests are not needed anymore.

Also, avoid calling into the netplan_generate() libnetplan API, this would just
execute the 'netplan generate' CLI, which would then call the
io.netplan.Netplan.Generate() DBus API in snap environments.
We can avoid this additional redirection and be independent from deprecation
or changes that are incompatible with the snap environment in the
netplan_generate() API, by calling the 'netplan generate' CLI directly.
Ideally, in the future we would avoid this redirection as well and call into
the io.netplan.Netplan.Generate() DBus method directly. But we need to pass a
--root-dir argument during the execution of NM's unit-tests, that is currently
not supported by the DBus method, so let's stick with the CLI for now.

This depends on the snapd snap to be >= 2.53.2+git864.g5823952 (currently in snapd/edge).

To post a comment you must log in.
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

Awesome, thanks for this! However, before merging we need to add a dependency so newer versions of the NM snap can be installed only for snapd with the interface change, like:

assumes:
  - snapd<version>

But we cannot really do this until snapd with the change has landed in the stable channel. Meanwhile, I'm marking this as WIP to avoid accidental merges.

Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Lukas Märdian (slyon) wrote :

snapd 2.54.2 landed in the stable channel and the test is passing now!

May I ask for another review/merge, Alfonso?

I'm not adding the "assumes:" stanza as that will be part of this other MP and I'd like to avoid a merge conflict:
https://code.launchpad.net/~alfonsosanchezbeato/snappy-hwe-snaps/+git/network-manager/+merge/414094

Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

Thanks for this!

Thinking about netplan/libnetplan, is there any dependency on a newer core20 version added due to the changes in the NM netplan patch? If that is the case we would need an assumes also for core20.

For the removed "software-properties-common" build dep, there was a comment related to "package-repositories:". Does that mean that the comments at the beginning of snapcraft.yaml need to be removed too? I mean:

# Some packages might land in the ubuntu-image PPA for making their way
# into a Ubuntu Core base image and only later land in a LTS release.
# Enable ubuntu-image PPA once the --enable-experimental-package-repositories
# flag is not needed anymore
# package-repositories:
# - type: apt
# ppa: canonical-foundations/ubuntu-image

Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

I was also thinking, is the NM netplan patch stored in some git repo also? It would be nice to know where to get history for it.

Revision history for this message
Lukas Märdian (slyon) wrote :

Thank you, those are some good comments!

I've now added the "assumes" dependencies on snapd and core20 as both are needed to access the "io.netplan.Netplan.Generate()" DBus API.

I was thinking about leaving that comment about the extra ubuntu-image PPA as is, in case it might be needed in the future. But no strong opinion about that; it's not needed currently, so I removed it now.

The link to the NM netplan patch git repository is listed in snapcraft.yaml, but I've now also added it to the patch header itself, for better visibility. There is not a lot of history on that repository, though, as all changes are squashed into a single commit. That is because carrying all the "overhead" (patch headers and description of individual commits) produces a very big patch, which was criticized in the past.

Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

Ah, I had not seen the link to the repo in the snapcraft.yaml. Thanks for your changes!

review: Approve
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

@slyon, I've been asking in snappy channel, and "assumes" is actually not valid for base snaps. The suggestion is to ship the library inside NM if there is such a dependency. Does that make sense?

Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

Also, I've tried the snap and fails on installation, most probably that is causing the CI issues.

Revision history for this message
Lukas Märdian (slyon) wrote :

Alright!
So let me remove the 'assumes:' stanza again, and let's pull in the snapd dependency via your MR#414094, while renouncing the core20 dependency.

Shipping libnetplan inside the NM snap isn't really an option for two reasons:
1/ 'netplan generate' accesses systemd files that are outside of the confinement and snapd didn't want to allow-list them. (See history on https://github.com/snapcore/snapd/pull/10212 – we've been working on this since April'21) So the only option is the hack that we currently have, vendorizing a patched version of netplan, that is reduced in functionality. But that needs to be updated (and patches refreshed) with each netplan SRU, while still not providing the full netplan functionality.

2/ The netplan configuration read from /etc/netplan is also used by the system (e.g. on bootup), so if we ship a different version of netplan inside the snap than we have in the outside system, the outside system might be happily creating and using new configuration settings, while the (potentially outdated) version inside the snap would fail to process those very same config files. As already happening in https://bugs.launchpad.net/rochester/+bug/1957828

So I guess we just have to expect the core snap to be up-to-date, containing a version of netplan that provides the io.netplan.Netplan.Generate() DBus API (that has been backported down to Bionic), without defining any dependency. This is the same way that we expected it to provide the io.netplan.Netplan.Apply() DBus API ever since.

Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

@slyon ok, what you say makes sense. Maybe what we need to do is just that NM does not fail catastrophically is an older core20 is present. What would happen if we install this NM with an old core20?

Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Lukas Märdian (slyon) wrote :

From my understanding of the code in `nms-keyfile-utils.c -> generate_netplan()` if the .Generate() DBus API would not be available on an older version of core20, NM would still be able to successfully produce new YAML (or modify existing YAML) in /etc/netplan but would then fail to generate the corresponding keyfile data out of that. So the NM state would basically not be updated, but the old configuration still in place. After a reboot (or 'netplan apply' call from the outside system) the current state would be visible to NM.

I don't have access to older versions of core20 (that would not provide the .Generate() API), so I could not test it in the real environment, but tested it inside a normal ("classic") Ubuntu Focal installation, where the modified NetworkManager is installed; that confirmed my assumption from above:

# nmcli con add type ethernet
[...]
Error: Failed to add 'ethernet' connection: failure adding connection: settings plugin does not support adding connections

# nmcli con mod ethernet con-name abc
[...]
Error: Failed to modify connection 'abc': failed to update connection: keyfile writer produces an invalid connection: cannot access file: No such file or directory

# nmcli con reload
Call failed: Unknown method Generate or interface io.netplan.Netplan.
Traceback (most recent call last):
  File "/usr/sbin/netplan", line 23, in <module>
    netplan.main()
  File "/usr/share/netplan/netplan/cli/core.py", line 50, in main
    self.run_command()
  File "/usr/share/netplan/netplan/cli/utils.py", line 310, in run_command
    self.func()
  File "/usr/share/netplan/netplan/cli/commands/generate.py", line 46, in run
    self.run_command()
  File "/usr/share/netplan/netplan/cli/utils.py", line 310, in run_command
    self.func()
  File "/usr/share/netplan/netplan/cli/commands/generate.py", line 73, in command_generate
    raise RuntimeError(
RuntimeError: failed to communicate with dbus service: error 1

All operations display a similar error message as the "nmcli con reload" command: "Call failed: Unknown method Generate or interface io.netplan.Netplan" / "RuntimeError: failed to communicate with dbus service: error 1"

Deleting of connection profiles via NM even works (but also prints the error message about the missing DBus API):
# nmcli con del abc
[...]
Connection 'abc' (6d8d8b90-5874-458f-b6c9-e1857f3e7b06) successfully deleted.

NetworkManager keeps running at all times (does not crash) and handles the latest configuration after reboot or 'netplan apply' was executed from the outside system. I'd say this qualifies for "NM does not fail catastrophically" and we should be good to go.

Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/snap-patch/netplan.patch b/snap-patch/netplan.patch
2deleted file mode 100644
3index 96d35e2..0000000
4--- a/snap-patch/netplan.patch
5+++ /dev/null
6@@ -1,103 +0,0 @@
7-From a989fcd2e08c6aa207f2bde2fab1bb7ea273ccf6 Mon Sep 17 00:00:00 2001
8-From: =?UTF-8?q?Lukas=20M=C3=A4rdian?= <lukas.maerdian@canonical.com>
9-Date: Wed, 5 May 2021 17:07:57 +0200
10-Subject: [PATCH] generate: disable generation of non-NM config
11-
12----
13- src/generate.c | 28 +++++++++++++++-------------
14- 1 file changed, 15 insertions(+), 13 deletions(-)
15-
16-diff --git a/src/generate.c b/src/generate.c
17-index d820f35..5424316 100644
18---- a/src/generate.c
19-+++ b/src/generate.c
20-@@ -36,7 +36,7 @@
21- static gchar* rootdir;
22- static gchar** files;
23- static gboolean any_networkd;
24--static gboolean any_sriov;
25-+//static gboolean any_sriov;
26- static gchar* mapping_iface;
27-
28- static GOptionEntry options[] = {
29-@@ -46,12 +46,14 @@ static GOptionEntry options[] = {
30- {NULL}
31- };
32-
33-+/*
34- static void
35- reload_udevd(void)
36- {
37- const gchar *argv[] = { "/sbin/udevadm", "control", "--reload", NULL };
38- g_spawn_sync(NULL, (gchar**)argv, NULL, G_SPAWN_STDERR_TO_DEV_NULL, NULL, NULL, NULL, NULL, NULL, NULL);
39- };
40-+*/
41-
42- // LCOV_EXCL_START
43- /* covered via 'cloud-init' integration test */
44-@@ -85,13 +87,13 @@ static void
45- nd_iterator_list(gpointer value, gpointer user_data)
46- {
47- NetplanNetDefinition* def = (NetplanNetDefinition*) value;
48-- if (write_networkd_conf(def, (const char*) user_data))
49-- any_networkd = TRUE;
50-+ //if (write_networkd_conf(def, (const char*) user_data))
51-+ // any_networkd = TRUE;
52-
53-- write_ovs_conf(def, (const char*) user_data);
54-+ //write_ovs_conf(def, (const char*) user_data);
55- write_nm_conf(def, (const char*) user_data);
56-- if (def->sriov_explicit_vf_count < G_MAXUINT || def->sriov_link)
57-- any_sriov = TRUE;
58-+ //if (def->sriov_explicit_vf_count < G_MAXUINT || def->sriov_link)
59-+ // any_sriov = TRUE;
60- }
61-
62-
63-@@ -223,28 +225,28 @@ int main(int argc, char** argv)
64- }
65-
66- /* Clean up generated config from previous runs */
67-- cleanup_networkd_conf(rootdir);
68-+ //cleanup_networkd_conf(rootdir);
69- cleanup_nm_conf(rootdir);
70-- cleanup_ovs_conf(rootdir);
71-- cleanup_sriov_conf(rootdir);
72-+ //cleanup_ovs_conf(rootdir);
73-+ //cleanup_sriov_conf(rootdir);
74-
75- if (mapping_iface && netdefs)
76- return find_interface(mapping_iface);
77-
78- /* Generate backend specific configuration files from merged data. */
79-- write_ovs_conf_finish(rootdir); // OVS cleanup unit is always written
80-+ //write_ovs_conf_finish(rootdir); // OVS cleanup unit is always written
81- if (netdefs) {
82- g_debug("Generating output files..");
83- g_list_foreach (netdefs_ordered, nd_iterator_list, rootdir);
84- write_nm_conf_finish(rootdir);
85-- if (any_sriov) write_sriov_conf_finish(rootdir);
86-+ //if (any_sriov) write_sriov_conf_finish(rootdir);
87- /* We may have written .rules & .link files, thus we must
88- * invalidate udevd cache of its config as by default it only
89- * invalidates cache at most every 3 seconds. Not sure if this
90- * should live in `generate' or `apply', but it is confusing
91- * when udevd ignores just-in-time created rules files.
92- */
93-- reload_udevd();
94-+ //reload_udevd();
95- }
96-
97- /* Disable /usr/lib/NetworkManager/conf.d/10-globally-managed-devices.conf
98-@@ -262,7 +264,7 @@ int main(int argc, char** argv)
99- FILE* f = fopen(generator_run_stamp, "w");
100- g_assert(f != NULL);
101- fclose(f);
102-- } else if (check_called_just_in_time()) {
103-+ } else if (FALSE && check_called_just_in_time()) {
104- /* netplan-feature: generate-just-in-time */
105- /* When booting with cloud-init, network configuration
106- * might be provided just-in-time. Specifically after
107---
108-2.30.2
109-
110diff --git a/snap-patch/networkmanager/0002-nm-netplan-keyfile.patch b/snap-patch/networkmanager/0002-nm-netplan-keyfile.patch
111index a9fa763..6dbaf2a 100644
112--- a/snap-patch/networkmanager/0002-nm-netplan-keyfile.patch
113+++ b/snap-patch/networkmanager/0002-nm-netplan-keyfile.patch
114@@ -1,7 +1,7 @@
115-From e5d3bc03eb28ee09351f947670b3b6608786491e Mon Sep 17 00:00:00 2001
116+From 58fd964ae79bfb635928b839f5788e1face8ddfb Mon Sep 17 00:00:00 2001
117 From: Lukas Märdian <lukas.maerdian@canonical.com>
118 Date: Tue, 2 Feb 2021 15:52:05 +0100
119-Subject: [PATCH 1/2] netplan: make use of libnetplan for YAML backend
120+Subject: [PATCH] netplan: make use of libnetplan for YAML backend
121
122 This patch modifies NetworkManager's nms-keyfile-plugin in a way to
123 write YAML connections (according to the netplan spec) to
124@@ -27,16 +27,19 @@ Each time the YAML data was modified, NetworkManager calls
125 netplan generator can be used as intended (no need for duplicated
126 keyfile export functionality) and the nms-keyfile-writer can be re-used
127 without any patching needed.
128+
129+Current upstream repository/branch:
130+https://github.com/slyon/NetworkManager/tree/slyon/backend-1.22.10
131 ---
132- Makefile.am | 5 +-
133- meson.build | 3 +
134- src/meson.build | 2 +
135- .../plugins/keyfile/nms-keyfile-plugin.c | 19 ++++
136- .../plugins/keyfile/nms-keyfile-utils.c | 41 ++++++++
137- .../plugins/keyfile/nms-keyfile-utils.h | 2 +
138- .../plugins/keyfile/nms-keyfile-writer.c | 97 +++++++++++++++++++
139- .../keyfile/tests/test-keyfile-settings.c | 45 +++++++--
140- 8 files changed, 205 insertions(+), 9 deletions(-)
141+ Makefile.am | 5 +-
142+ meson.build | 3 +
143+ src/meson.build | 2 +
144+ .../plugins/keyfile/nms-keyfile-plugin.c | 19 +++
145+ .../plugins/keyfile/nms-keyfile-utils.c | 57 +++++++++
146+ .../plugins/keyfile/nms-keyfile-utils.h | 4 +
147+ .../plugins/keyfile/nms-keyfile-writer.c | 108 ++++++++++++++++++
148+ .../keyfile/tests/test-keyfile-settings.c | 45 ++++++--
149+ 8 files changed, 234 insertions(+), 9 deletions(-)
150
151 diff --git a/Makefile.am b/Makefile.am
152 index b37aa4e712311cb23a675af70ec1287fcd3d63f5..2e4e146412de6102068bb953c38cb8091e7f010a 100644
153@@ -89,7 +92,7 @@ index 748fa519bc56305bfe485b9ea1207942fe5b5874..398efb3c69c37bd1d6214602d194dfad
154
155 if enable_wext
156 diff --git a/src/settings/plugins/keyfile/nms-keyfile-plugin.c b/src/settings/plugins/keyfile/nms-keyfile-plugin.c
157-index fdb88d2a6d6218460fe191d7bc5be08b492886f5..548eda536354c9aa3a305540038bf4a03be8fcb6 100644
158+index fdb88d2a6d6218460fe191d7bc5be08b492886f5..60b8490fdaa7cbf1ffae3dc89f19289bc842acad 100644
159 --- a/src/settings/plugins/keyfile/nms-keyfile-plugin.c
160 +++ b/src/settings/plugins/keyfile/nms-keyfile-plugin.c
161 @@ -12,6 +12,7 @@
162@@ -115,7 +118,7 @@ index fdb88d2a6d6218460fe191d7bc5be08b492886f5..548eda536354c9aa3a305540038bf4a0
163 nm_auto_clear_sett_util_storages NMSettUtilStorages storages_new = NM_SETT_UTIL_STORAGES_INIT (storages_new, nms_keyfile_storage_destroy);
164 int i;
165
166-+ netplan_generate(NULL);
167++ generate_netplan(NULL);
168 + _fix_netplan_interface_name(NULL);
169 _load_dir (self, NMS_KEYFILE_STORAGE_TYPE_RUN, priv->dirname_run, &storages_new);
170 if (priv->dirname_etc)
171@@ -140,14 +143,14 @@ index fdb88d2a6d6218460fe191d7bc5be08b492886f5..548eda536354c9aa3a305540038bf4a0
172
173 + g_autofree gchar* netplan_id = netplan_get_id_from_nm_filename(previous_filename, ssid);
174 + netplan_delete_connection(netplan_id, NULL);
175-+ netplan_generate(NULL);
176++ generate_netplan(NULL);
177 + _fix_netplan_interface_name(NULL);
178 +
179 _LOGT ("commit: deleted \"%s\", %s %s (%s%s%s%s)",
180 previous_filename,
181 storage->is_meta_data ? "meta-data" : "profile",
182 diff --git a/src/settings/plugins/keyfile/nms-keyfile-utils.c b/src/settings/plugins/keyfile/nms-keyfile-utils.c
183-index f03c601a56a6548c947f1b7cc6d4cd6b441e5b1d..44edabd37599187ee00da2dfea4c3a09fcb2cb9d 100644
184+index f03c601a56a6548c947f1b7cc6d4cd6b441e5b1d..20c3d949b284ba64de0d01794795a1bb6367ba4c 100644
185 --- a/src/settings/plugins/keyfile/nms-keyfile-utils.c
186 +++ b/src/settings/plugins/keyfile/nms-keyfile-utils.c
187 @@ -9,6 +9,7 @@
188@@ -205,8 +208,28 @@ index f03c601a56a6548c947f1b7cc6d4cd6b441e5b1d..44edabd37599187ee00da2dfea4c3a09
189 const char *
190 nms_keyfile_nmmeta_check_filename (const char *filename,
191 guint *out_uuid_len)
192+@@ -366,3 +407,19 @@ nms_keyfile_utils_check_file_permissions (NMSKeyfileFiletype filetype,
193+ NM_SET_OUT (out_st, st);
194+ return TRUE;
195+ }
196++
197++gboolean
198++generate_netplan(const char* rootdir)
199++{
200++ /* TODO: call the io.netplan.Netplan.Generate() DBus method directly, after
201++ * finding a way to pass the --root-dir parameter via DBus, to make it work
202++ * inside NM's unit-tests where netplan needs to read & generate outside of
203++ * /etc/netplan and /run/{systemd,NetworkManager} */
204++ const gchar *argv[] = { "netplan", "generate", NULL , NULL, NULL };
205++ if (rootdir) {
206++ argv[2] = "--root-dir";
207++ argv[3] = rootdir;
208++ }
209++ return g_spawn_sync(NULL, (gchar**)argv, NULL, G_SPAWN_SEARCH_PATH,
210++ NULL, NULL, NULL, NULL, NULL, NULL);
211++}
212 diff --git a/src/settings/plugins/keyfile/nms-keyfile-utils.h b/src/settings/plugins/keyfile/nms-keyfile-utils.h
213-index f943d65ca6834efd2bc243397d122243d906712e..f9c29899990ffd70ca4aba50f42284fa2277440f 100644
214+index f943d65ca6834efd2bc243397d122243d906712e..4803135d3f60cdfae4a23640cefcbe6d4f4f1de4 100644
215 --- a/src/settings/plugins/keyfile/nms-keyfile-utils.h
216 +++ b/src/settings/plugins/keyfile/nms-keyfile-utils.h
217 @@ -30,6 +30,8 @@ NMS_KEYFILE_STORAGE_TYPE_LIB (guint run_idx)
218@@ -218,8 +241,15 @@ index f943d65ca6834efd2bc243397d122243d906712e..f9c29899990ffd70ca4aba50f42284fa
219 const char *nms_keyfile_nmmeta_check_filename (const char *filename,
220 guint *out_uuid_len);
221
222+@@ -71,4 +73,6 @@ gboolean nms_keyfile_utils_check_file_permissions (NMSKeyfileFiletype filetype,
223+ struct stat *out_st,
224+ GError **error);
225+
226++gboolean generate_netplan(const char* rootdir);
227++
228+ #endif /* __NMS_KEYFILE_UTILS_H__ */
229 diff --git a/src/settings/plugins/keyfile/nms-keyfile-writer.c b/src/settings/plugins/keyfile/nms-keyfile-writer.c
230-index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..fb1c0437a45ac0eae9a9fbb6d140abf7ad49bb00 100644
231+index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..53d8f929921181adbe2b0349ee80d1b93939e829 100644
232 --- a/src/settings/plugins/keyfile/nms-keyfile-writer.c
233 +++ b/src/settings/plugins/keyfile/nms-keyfile-writer.c
234 @@ -11,6 +11,10 @@
235@@ -241,7 +271,7 @@ index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..fb1c0437a45ac0eae9a9fbb6d140abf7
236 GError **error)
237 {
238 gs_unref_keyfile GKeyFile *kf_file = NULL;
239-@@ -364,6 +369,92 @@ _internal_write_connection (NMConnection *connection,
240+@@ -364,6 +369,103 @@ _internal_write_connection (NMConnection *connection,
241 && !nm_streq (path, existing_path))
242 unlink (existing_path);
243
244@@ -278,7 +308,7 @@ index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..fb1c0437a45ac0eae9a9fbb6d140abf7
245 + * we've written the /etc/netplan/*.yaml file instead. */
246 + unlink(path);
247 + g_free(path);
248-+ if (!netplan_generate(rootdir)) {
249++ if (!generate_netplan(rootdir)) {
250 + g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
251 + "netplan generate failed");
252 + return FALSE;
253@@ -296,6 +326,17 @@ index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..fb1c0437a45ac0eae9a9fbb6d140abf7
254 + else
255 + path = g_strdup_printf("%s/run/NetworkManager/system-connections/netplan-NM-%s.nmconnection",
256 + rootdir ?: "", nm_connection_get_uuid (connection));
257++
258++ // Since netplan v0.103 logical interfaces (bridge/bond/vlan/...) use the interface name as ID
259++ if (!g_file_test(path, G_FILE_TEST_EXISTS)) {
260++ g_free(path);
261++ if (escaped_ssid)
262++ path = g_strdup_printf("%s/run/NetworkManager/system-connections/netplan-%s-%s.nmconnection",
263++ rootdir ?: "", (char *)netdef_id, escaped_ssid);
264++ else
265++ path = g_strdup_printf("%s/run/NetworkManager/system-connections/netplan-%s.nmconnection",
266++ rootdir ?: "", (char *)netdef_id);
267++ }
268 + }
269 +
270 + /* re-read again: this time the connection profile newly generated by netplan in /run/... */
271@@ -334,7 +375,7 @@ index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..fb1c0437a45ac0eae9a9fbb6d140abf7
272 NM_SET_OUT (out_reread, g_steal_pointer (&reread));
273 NM_SET_OUT (out_reread_same, reread_same);
274 NM_SET_OUT (out_path, g_steal_pointer (&path));
275-@@ -407,6 +498,7 @@ nms_keyfile_writer_connection (NMConnection *connection,
276+@@ -407,6 +509,7 @@ nms_keyfile_writer_connection (NMConnection *connection,
277 out_path,
278 out_reread,
279 out_reread_same,
280@@ -342,7 +383,7 @@ index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..fb1c0437a45ac0eae9a9fbb6d140abf7
281 error);
282 }
283
284-@@ -420,6 +512,10 @@ nms_keyfile_writer_test_connection (NMConnection *connection,
285+@@ -420,6 +523,10 @@ nms_keyfile_writer_test_connection (NMConnection *connection,
286 gboolean *out_reread_same,
287 GError **error)
288 {
289@@ -353,7 +394,7 @@ index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..fb1c0437a45ac0eae9a9fbb6d140abf7
290 return _internal_write_connection (connection,
291 FALSE,
292 FALSE,
293-@@ -438,5 +534,6 @@ nms_keyfile_writer_test_connection (NMConnection *connection,
294+@@ -438,5 +545,6 @@ nms_keyfile_writer_test_connection (NMConnection *connection,
295 out_path,
296 out_reread,
297 out_reread_same,
298@@ -470,74 +511,3 @@ index d2da09da71c45226a8fb5e9f46e376deb5a48be8..fd2aa1424a218ea6a61369da56b9044d
299 --
300 2.32.0
301
302-
303-From 99bd17f98438acb4e9263ef7c1b43b78d955cbcb Mon Sep 17 00:00:00 2001
304-From: Lukas Märdian <slyon@ubuntu.com>
305-Date: Tue, 5 Oct 2021 11:42:19 +0200
306-Subject: [PATCH 2/2] netplan: v0.103 compatibility
307-
308----
309- src/settings/plugins/keyfile/nms-keyfile-writer.c | 11 +++++++++++
310- .../plugins/keyfile/tests/test-keyfile-settings.c | 11 ++++++++---
311- 2 files changed, 19 insertions(+), 3 deletions(-)
312-
313-diff --git a/src/settings/plugins/keyfile/nms-keyfile-writer.c b/src/settings/plugins/keyfile/nms-keyfile-writer.c
314-index fb1c0437a45ac0eae9a9fbb6d140abf7ad49bb00..1bc761c4c92ba8df341705630b35a429749b4853 100644
315---- a/src/settings/plugins/keyfile/nms-keyfile-writer.c
316-+++ b/src/settings/plugins/keyfile/nms-keyfile-writer.c
317-@@ -420,6 +420,17 @@ _internal_write_connection (NMConnection *connection,
318- else
319- path = g_strdup_printf("%s/run/NetworkManager/system-connections/netplan-NM-%s.nmconnection",
320- rootdir ?: "", nm_connection_get_uuid (connection));
321-+
322-+ // Since netplan v0.103 logical interfaces (bridge/bond/vlan/...) use the interface name as ID
323-+ if (!g_file_test(path, G_FILE_TEST_EXISTS)) {
324-+ g_free(path);
325-+ if (escaped_ssid)
326-+ path = g_strdup_printf("%s/run/NetworkManager/system-connections/netplan-%s-%s.nmconnection",
327-+ rootdir ?: "", (char *)netdef_id, escaped_ssid);
328-+ else
329-+ path = g_strdup_printf("%s/run/NetworkManager/system-connections/netplan-%s.nmconnection",
330-+ rootdir ?: "", (char *)netdef_id);
331-+ }
332- }
333-
334- /* re-read again: this time the connection profile newly generated by netplan in /run/... */
335-diff --git a/src/settings/plugins/keyfile/tests/test-keyfile-settings.c b/src/settings/plugins/keyfile/tests/test-keyfile-settings.c
336-index fd2aa1424a218ea6a61369da56b9044d75232cac..84c7081e941cab54c2027231e1d55065f18633ef 100644
337---- a/src/settings/plugins/keyfile/tests/test-keyfile-settings.c
338-+++ b/src/settings/plugins/keyfile/tests/test-keyfile-settings.c
339-@@ -461,7 +461,9 @@ test_write_wired_connection (void)
340- /* Routes */
341- add_one_ip_route (s_ip4, route1, route1_nh, 24, 3);
342- add_one_ip_route (s_ip4, route2, route2_nh, 8, 1);
343-- add_one_ip_route (s_ip4, route3, route3_nh, 7, -1);
344-+ /* XXX: netplan 0.103 requires "to" and "via" for unicast routes,
345-+ * but NM drops the "via"/nh field if metric is not defined. */
346-+ add_one_ip_route (s_ip4, route3, route3_nh, 7, 0);
347-
348- rt = nm_ip_route_new (AF_INET, route4, 6, route4_nh, 4, &error);
349- g_assert_no_error (error);
350-@@ -492,14 +494,17 @@ test_write_wired_connection (void)
351- add_one_ip_route (s_ip6, route6_1, route6_1_nh, 64, 3);
352- add_one_ip_route (s_ip6, route6_2, route6_2_nh, 56, 1);
353- add_one_ip_route (s_ip6, route6_3, route6_3_nh, 63, 5);
354-- add_one_ip_route (s_ip6, route6_4, route6_4_nh, 62, -1);
355-+ /* XXX: netplan 0.103 requires "to" and "via" for unicast routes,
356-+ * but NM drops the "via"/nh field if metric is not defined. */
357-+ add_one_ip_route (s_ip6, route6_4, route6_4_nh, 62, 0);
358-
359- /* DNS servers */
360- nm_setting_ip_config_add_dns (s_ip6, dns6_1);
361- nm_setting_ip_config_add_dns (s_ip6, dns6_2);
362-
363- /* DNS searches */
364-- nm_setting_ip_config_add_dns_search (s_ip6, "wallaceandgromit.com");
365-+ //XXX: netplan cannot differentiate between IPv4/6 search domains
366-+ //nm_setting_ip_config_add_dns_search (s_ip6, "wallaceandgromit.com");
367-
368- write_test_connection_and_reread (connection, FALSE);
369- }
370---
371-2.32.0
372-
373diff --git a/snap-patch/networkmanager/0005-netplan-use-vendorized-generate-binary.patch b/snap-patch/networkmanager/0005-netplan-use-vendorized-generate-binary.patch
374deleted file mode 100644
375index 7359624..0000000
376--- a/snap-patch/networkmanager/0005-netplan-use-vendorized-generate-binary.patch
377+++ /dev/null
378@@ -1,83 +0,0 @@
379-From ba8d280149cccfd24c7cbb78e8f301edaf4a0cfb Mon Sep 17 00:00:00 2001
380-From: Lukas Märdian <slyon@ubuntu.com>
381-Date: Thu, 6 May 2021 15:05:33 +0200
382-Subject: [PATCH] netplan: use vendorized generate binary
383-
384----
385- src/settings/plugins/keyfile/nms-keyfile-plugin.c | 4 ++--
386- src/settings/plugins/keyfile/nms-keyfile-utils.c | 14 ++++++++++++++
387- src/settings/plugins/keyfile/nms-keyfile-utils.h | 2 ++
388- src/settings/plugins/keyfile/nms-keyfile-writer.c | 2 +-
389- 4 files changed, 19 insertions(+), 3 deletions(-)
390-
391-diff --git a/src/settings/plugins/keyfile/nms-keyfile-plugin.c b/src/settings/plugins/keyfile/nms-keyfile-plugin.c
392-index 548eda536..83bcdb857 100644
393---- a/src/settings/plugins/keyfile/nms-keyfile-plugin.c
394-+++ b/src/settings/plugins/keyfile/nms-keyfile-plugin.c
395-@@ -561,7 +561,7 @@ reload_connections (NMSettingsPlugin *plugin,
396- nm_auto_clear_sett_util_storages NMSettUtilStorages storages_new = NM_SETT_UTIL_STORAGES_INIT (storages_new, nms_keyfile_storage_destroy);
397- int i;
398-
399-- netplan_generate(NULL);
400-+ _netplan_generate(NULL);
401- _fix_netplan_interface_name(NULL);
402- _load_dir (self, NMS_KEYFILE_STORAGE_TYPE_RUN, priv->dirname_run, &storages_new);
403- if (priv->dirname_etc)
404-@@ -1027,7 +1027,7 @@ delete_connection (NMSettingsPlugin *plugin,
405-
406- g_autofree gchar* netplan_id = netplan_get_id_from_nm_filename(previous_filename, ssid);
407- netplan_delete_connection(netplan_id, NULL);
408-- netplan_generate(NULL);
409-+ _netplan_generate(NULL);
410- _fix_netplan_interface_name(NULL);
411-
412- _LOGT ("commit: deleted \"%s\", %s %s (%s%s%s%s)",
413-diff --git a/src/settings/plugins/keyfile/nms-keyfile-utils.c b/src/settings/plugins/keyfile/nms-keyfile-utils.c
414-index 44edabd37..e2030ec5c 100644
415---- a/src/settings/plugins/keyfile/nms-keyfile-utils.c
416-+++ b/src/settings/plugins/keyfile/nms-keyfile-utils.c
417-@@ -407,3 +407,17 @@ nms_keyfile_utils_check_file_permissions (NMSKeyfileFiletype filetype,
418- NM_SET_OUT (out_st, st);
419- return TRUE;
420- }
421-+
422-+gboolean
423-+_netplan_generate(const char* rootdir)
424-+{
425-+ /* Use 'netplan_generate()' from libnetplan once that is fixed to not be
426-+ * DENIED by apparmor via snapd's 'network-setup-control' interface.
427-+ * This needs https://github.com/snapcore/snapd/pull/10212 to be landed. */
428-+ const gchar *argv[] = { "/snap/network-manager/current/lib/netplan/generate", NULL , NULL, NULL };
429-+ if (rootdir) {
430-+ argv[1] = "--root-dir";
431-+ argv[2] = rootdir;
432-+ }
433-+ return g_spawn_sync(NULL, (gchar**)argv, NULL, 0, NULL, NULL, NULL, NULL, NULL, NULL);
434-+}
435-diff --git a/src/settings/plugins/keyfile/nms-keyfile-utils.h b/src/settings/plugins/keyfile/nms-keyfile-utils.h
436-index f9c298999..16b965ffd 100644
437---- a/src/settings/plugins/keyfile/nms-keyfile-utils.h
438-+++ b/src/settings/plugins/keyfile/nms-keyfile-utils.h
439-@@ -73,4 +73,6 @@ gboolean nms_keyfile_utils_check_file_permissions (NMSKeyfileFiletype filetype,
440- struct stat *out_st,
441- GError **error);
442-
443-+gboolean _netplan_generate(const char* rootdir);
444-+
445- #endif /* __NMS_KEYFILE_UTILS_H__ */
446-diff --git a/src/settings/plugins/keyfile/nms-keyfile-writer.c b/src/settings/plugins/keyfile/nms-keyfile-writer.c
447-index 1bc761c4c..022eebce9 100644
448---- a/src/settings/plugins/keyfile/nms-keyfile-writer.c
449-+++ b/src/settings/plugins/keyfile/nms-keyfile-writer.c
450-@@ -402,7 +402,7 @@ _internal_write_connection (NMConnection *connection,
451- * we've written the /etc/netplan/*.yaml file instead. */
452- unlink(path);
453- g_free(path);
454-- if (!netplan_generate(rootdir)) {
455-+ if (!_netplan_generate(rootdir)) {
456- g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
457- "netplan generate failed");
458- return FALSE;
459---
460-2.32.0
461-
462diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml
463index b214e02..f5701f3 100644
464--- a/snap/snapcraft.yaml
465+++ b/snap/snapcraft.yaml
466@@ -23,14 +23,6 @@ grade: stable
467 # will be adjusted accordingly.
468 epoch: 1
469
470-# Some packages might land in the ubuntu-image PPA for making their way
471-# into a Ubuntu Core base image and only later land in a LTS release.
472-# Enable ubuntu-image PPA once the --enable-experimental-package-repositories
473-# flag is not needed anymore
474-# package-repositories:
475-# - type: apt
476-# ppa: canonical-foundations/ubuntu-image
477-
478 slots:
479 service: network-manager
480
481@@ -152,29 +144,6 @@ parts:
482 prime:
483 - $binaries
484
485- netplan:
486- after: [ stack-snaps-tools ]
487- plugin: make
488- source: https://git.launchpad.net/ubuntu/+source/netplan.io
489- source-type: git
490- source-tag: applied/0.102-0ubuntu1_20.04.2
491- build-packages:
492- - build-essential
493- - libyaml-dev
494- - libglib2.0-dev
495- - uuid-dev
496- - libsystemd-dev
497- - pandoc
498- override-build: |
499- set -ex
500- git apply "$SNAPCRAFT_PROJECT_DIR"/snap-patch/netplan.patch
501- snapcraftctl build
502- filesets:
503- binaries:
504- - lib/netplan/generate
505- prime:
506- - $binaries
507-
508 networkmanager:
509 after: [ stack-snaps-tools ]
510 plugin: autotools
511@@ -220,7 +189,6 @@ parts:
512 - libnetplan-dev
513 - netplan.io
514 - libyaml-dev
515- - software-properties-common # Can be removed once "package-repositories:" is working
516 autotools-configure-parameters:
517 - --prefix=/usr
518 - --sysconfdir=/etc
519@@ -280,11 +248,15 @@ parts:
520 ./autogen.sh
521 snapcraftctl build
522
523- # Simulate vendorized netplan generator binary to pass tests
524- mkdir -p /snap/network-manager/current/lib/netplan
525- ln -sf /lib/netplan/generate /snap/network-manager/current/lib/netplan/generate
526 # Run all tests NetworkManager ships by default
527+ # Hide the $SNAP env variable during testing, to prevent the netplan CLI
528+ # from using the snap fallback io.netplan.Netplan.Generate() DBus API as
529+ # that does not allow to pass a --root-dir argument, needed for testing
530+ _SNAP=$SNAP
531+ unset SNAP
532 make check
533+ SNAP=$_SNAP
534+ unset _SNAP
535 stage-packages:
536 - iputils-arping
537 - libasn1-8-heimdal

Subscribers

People subscribed via source and target branches