snappy-hwe-snaps

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

  1. Git
  2. lp:~slyon/snappy-hwe-snaps/+git/network-manager
  3. slyon/netplan-modify-connections
  4. Merge into snap-20
Proposed by Lukas Märdian
Status: Merged
Approved by: Alfonso Sanchez-Beato
Approved revision: 2144bc3d731412694e804679fe35a048d2e2f804
Merged at revision: dd5fde3b5a5e17f60612ed6f0fbb9438fcf42e85
Proposed branch: ~slyon/snappy-hwe-snaps/+git/network-manager:slyon/netplan-modify-connections
Merge into: ~snappy-hwe-team/snappy-hwe-snaps/+git/network-manager:snap-20
Diff against target: 135 lines (+31/-16)
2 files modified
snap-patch/networkmanager/0002-nm-netplan-keyfile.patch (+29/-16)
tests/full/immutable-netplan-config/task.yaml (+2/-0)
Reviewer Review Type Date Requested Status
Alfonso Sanchez-Beato Approve
System Enablement Bot continuous-integration Approve
Review via email: mp+415913@code.launchpad.net

Commit message

Update netplan integration patch

If existing connections are being modified via 'nmcli' netplan does not know
about the existing netdef_id, as the path to the keyfile is not in the format
'run/NetworkManager/system-connections/netplan-*.nmconnection'

If there is an previous existing netplan connection, make sure to pass the new
keyfile, but at the location/path of the old connection profile, so that
netplan is able to derive the original netdef_id.

Also, make the full:immutable-netplan-config a bit more robust, by avoiding the volatile connection profile generated by NM. And avoid deleting non-netplan (e.g. volatile) connections via libnetplan.

Description of the change

Update netplan integration patch

If existing connections are being modified via 'nmcli' netplan does not know
about the existing netdef_id, as the path to the keyfile is not in the format
'run/NetworkManager/system-connections/netplan-*.nmconnection'

If there is an previous existing netplan connection, make sure to pass the new
keyfile, but at the location/path of the old connection profile, so that
netplan is able to derive the original netdef_id.

Also, make the full:immutable-netplan-config a bit more robust, by avoiding the volatile connection profile generated by NM. And avoid deleting non-netplan (e.g. volatile) connections via libnetplan.

To post a comment you must log in.
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :

PASSED: Continuous integration, rev:22ec1c68a8b6e194a4bdafec8b7b325f3747363a
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/667/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=amd64/694
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=arm64/694
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=armhf/694
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=i386/694
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-docs/1475
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-cleanup/1217
    None: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-update-mp/1280/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/667/rebuild

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

PASSED: Continuous integration, rev:2144bc3d731412694e804679fe35a048d2e2f804
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/668/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=amd64/695
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=arm64/695
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=armhf/695
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=i386/695
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-docs/1476
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-cleanup/1218
    None: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-update-mp/1281/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/668/rebuild

review: Approve (continuous-integration)
Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

LGTM,thanks

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

Thank you for the approval! I think we should be releasing this soon.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/snap-patch/networkmanager/0002-nm-netplan-keyfile.patch b/snap-patch/networkmanager/0002-nm-netplan-keyfile.patch
index ad89e95..ef7d899 100644
--- a/snap-patch/networkmanager/0002-nm-netplan-keyfile.patch
+++ b/snap-patch/networkmanager/0002-nm-netplan-keyfile.patch
@@ -1,4 +1,4 @@
1From 5ffe6c36ed2e9a5effc07d706f0b8dc34c7515ab Mon Sep 17 00:00:00 20011From 4c06fede36936e58e75babcb9542a6b27a7cb19f Mon Sep 17 00:00:00 2001
2From: Lukas Märdian <lukas.maerdian@canonical.com>2From: Lukas Märdian <lukas.maerdian@canonical.com>
3Date: Tue, 2 Feb 2021 15:52:05 +01003Date: Tue, 2 Feb 2021 15:52:05 +0100
4Subject: [PATCH] netplan: make use of libnetplan for YAML backend4Subject: [PATCH] netplan: make use of libnetplan for YAML backend
@@ -34,12 +34,12 @@ https://github.com/slyon/NetworkManager/tree/slyon/backend-1.22.10
34 Makefile.am | 5 +-34 Makefile.am | 5 +-
35 meson.build | 3 +35 meson.build | 3 +
36 src/meson.build | 2 +36 src/meson.build | 2 +
37 .../plugins/keyfile/nms-keyfile-plugin.c | 19 +++37 .../plugins/keyfile/nms-keyfile-plugin.c | 22 ++++
38 .../plugins/keyfile/nms-keyfile-utils.c | 57 +++++++++38 .../plugins/keyfile/nms-keyfile-utils.c | 57 +++++++++
39 .../plugins/keyfile/nms-keyfile-utils.h | 4 +39 .../plugins/keyfile/nms-keyfile-utils.h | 4 +
40 .../plugins/keyfile/nms-keyfile-writer.c | 108 ++++++++++++++++++40 .../plugins/keyfile/nms-keyfile-writer.c | 118 ++++++++++++++++++
41 .../keyfile/tests/test-keyfile-settings.c | 45 ++++++--41 .../keyfile/tests/test-keyfile-settings.c | 45 +++++--
42 8 files changed, 234 insertions(+), 9 deletions(-)42 8 files changed, 247 insertions(+), 9 deletions(-)
4343
44diff --git a/Makefile.am b/Makefile.am44diff --git a/Makefile.am b/Makefile.am
45index b37aa4e712311cb23a675af70ec1287fcd3d63f5..2e4e146412de6102068bb953c38cb8091e7f010a 10064445index b37aa4e712311cb23a675af70ec1287fcd3d63f5..2e4e146412de6102068bb953c38cb8091e7f010a 100644
@@ -92,7 +92,7 @@ index 748fa519bc56305bfe485b9ea1207942fe5b5874..398efb3c69c37bd1d6214602d194dfad
92 92
93 if enable_wext93 if enable_wext
94diff --git a/src/settings/plugins/keyfile/nms-keyfile-plugin.c b/src/settings/plugins/keyfile/nms-keyfile-plugin.c94diff --git a/src/settings/plugins/keyfile/nms-keyfile-plugin.c b/src/settings/plugins/keyfile/nms-keyfile-plugin.c
95index fdb88d2a6d6218460fe191d7bc5be08b492886f5..60b8490fdaa7cbf1ffae3dc89f19289bc842acad 10064495index fdb88d2a6d6218460fe191d7bc5be08b492886f5..038cf13a1d7ea287d8c491848b9284334d1b9d70 100644
96--- a/src/settings/plugins/keyfile/nms-keyfile-plugin.c96--- a/src/settings/plugins/keyfile/nms-keyfile-plugin.c
97+++ b/src/settings/plugins/keyfile/nms-keyfile-plugin.c97+++ b/src/settings/plugins/keyfile/nms-keyfile-plugin.c
98@@ -12,6 +12,7 @@98@@ -12,6 +12,7 @@
@@ -137,14 +137,17 @@ index fdb88d2a6d6218460fe191d7bc5be08b492886f5..60b8490fdaa7cbf1ffae3dc89f19289b
137 if (!NM_IN_SET (storage->storage_type, NMS_KEYFILE_STORAGE_TYPE_ETC,137 if (!NM_IN_SET (storage->storage_type, NMS_KEYFILE_STORAGE_TYPE_ETC,
138 NMS_KEYFILE_STORAGE_TYPE_RUN)) {138 NMS_KEYFILE_STORAGE_TYPE_RUN)) {
139 nm_utils_error_set (error,139 nm_utils_error_set (error,
140@@ -1011,6 +1025,11 @@ delete_connection (NMSettingsPlugin *plugin,140@@ -1011,6 +1025,14 @@ delete_connection (NMSettingsPlugin *plugin,
141 } else141 } else
142 operation_message = "deleted from disk";142 operation_message = "deleted from disk";
143 143
144+ g_autofree gchar* netplan_id = netplan_get_id_from_nm_filename(previous_filename, ssid);144+ g_autofree gchar* netplan_id = netplan_get_id_from_nm_filename(previous_filename, ssid);
145+ netplan_delete_connection(netplan_id, NULL);145+ if (netplan_id) {
146+ generate_netplan(NULL);146+ _LOGI ("deleting netplan connection: %s", netplan_id);
147+ _fix_netplan_interface_name(NULL);147+ netplan_delete_connection(netplan_id, NULL);
148+ generate_netplan(NULL);
149+ _fix_netplan_interface_name(NULL);
150+ }
148+151+
149 _LOGT ("commit: deleted \"%s\", %s %s (%s%s%s%s)",152 _LOGT ("commit: deleted \"%s\", %s %s (%s%s%s%s)",
150 previous_filename,153 previous_filename,
@@ -249,7 +252,7 @@ index f943d65ca6834efd2bc243397d122243d906712e..4803135d3f60cdfae4a23640cefcbe6d
249+252+
250 #endif /* __NMS_KEYFILE_UTILS_H__ */253 #endif /* __NMS_KEYFILE_UTILS_H__ */
251diff --git a/src/settings/plugins/keyfile/nms-keyfile-writer.c b/src/settings/plugins/keyfile/nms-keyfile-writer.c254diff --git a/src/settings/plugins/keyfile/nms-keyfile-writer.c b/src/settings/plugins/keyfile/nms-keyfile-writer.c
252index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..627c72d7a0ff76e9b00e0a5d6c19111583ba3d66 100644255index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..1b8affdfe91935049edc9ba4323dc627109a59b5 100644
253--- a/src/settings/plugins/keyfile/nms-keyfile-writer.c256--- a/src/settings/plugins/keyfile/nms-keyfile-writer.c
254+++ b/src/settings/plugins/keyfile/nms-keyfile-writer.c257+++ b/src/settings/plugins/keyfile/nms-keyfile-writer.c
255@@ -11,6 +11,10 @@258@@ -11,6 +11,10 @@
@@ -271,7 +274,7 @@ index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..627c72d7a0ff76e9b00e0a5d6c191115
271 GError **error)274 GError **error)
272 {275 {
273 gs_unref_keyfile GKeyFile *kf_file = NULL;276 gs_unref_keyfile GKeyFile *kf_file = NULL;
274@@ -364,6 +369,103 @@ _internal_write_connection (NMConnection *connection,277@@ -364,6 +369,113 @@ _internal_write_connection (NMConnection *connection,
275 && !nm_streq (path, existing_path))278 && !nm_streq (path, existing_path))
276 unlink (existing_path);279 unlink (existing_path);
277 280
@@ -283,7 +286,17 @@ index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..627c72d7a0ff76e9b00e0a5d6c191115
283+ g_autofree gchar* netplan_id = (existing_path && strstr(existing_path, "system-connections/netplan-")) ?286+ g_autofree gchar* netplan_id = (existing_path && strstr(existing_path, "system-connections/netplan-")) ?
284+ netplan_get_id_from_nm_filename(existing_path, ssid) : NULL;287+ netplan_get_id_from_nm_filename(existing_path, ssid) : NULL;
285+ netplan_clear_netdefs();288+ netplan_clear_netdefs();
286+ if (!netplan_parse_keyfile(path, &local_err)) { // push keyfile into libnetplan for parsing289+
290+ const gchar* kf_path = path;
291+ if (netplan_id && existing_path) {
292+ GFile* from = g_file_new_for_path(path);
293+ GFile* to = g_file_new_for_path(existing_path);
294+ g_file_copy(from, to, G_FILE_COPY_OVERWRITE, NULL, NULL, NULL, NULL);
295+ kf_path = existing_path;
296+ }
297+ // push keyfile into libnetplan for parsing (using existing_path, if available,
298+ // to be able to extract the original netdef_id and override existing settings)
299+ if (!netplan_parse_keyfile(kf_path, &local_err)) {
287+ g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,300+ g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
288+ "netplan: YAML translation failed");301+ "netplan: YAML translation failed");
289+ return FALSE;302+ return FALSE;
@@ -375,7 +388,7 @@ index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..627c72d7a0ff76e9b00e0a5d6c191115
375 NM_SET_OUT (out_reread, g_steal_pointer (&reread));388 NM_SET_OUT (out_reread, g_steal_pointer (&reread));
376 NM_SET_OUT (out_reread_same, reread_same);389 NM_SET_OUT (out_reread_same, reread_same);
377 NM_SET_OUT (out_path, g_steal_pointer (&path));390 NM_SET_OUT (out_path, g_steal_pointer (&path));
378@@ -407,6 +509,7 @@ nms_keyfile_writer_connection (NMConnection *connection,391@@ -407,6 +519,7 @@ nms_keyfile_writer_connection (NMConnection *connection,
379 out_path,392 out_path,
380 out_reread,393 out_reread,
381 out_reread_same,394 out_reread_same,
@@ -383,7 +396,7 @@ index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..627c72d7a0ff76e9b00e0a5d6c191115
383 error);396 error);
384 }397 }
385 398
386@@ -420,6 +523,10 @@ nms_keyfile_writer_test_connection (NMConnection *connection,399@@ -420,6 +533,10 @@ nms_keyfile_writer_test_connection (NMConnection *connection,
387 gboolean *out_reread_same,400 gboolean *out_reread_same,
388 GError **error)401 GError **error)
389 {402 {
@@ -394,7 +407,7 @@ index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..627c72d7a0ff76e9b00e0a5d6c191115
394 return _internal_write_connection (connection,407 return _internal_write_connection (connection,
395 FALSE,408 FALSE,
396 FALSE,409 FALSE,
397@@ -438,5 +545,6 @@ nms_keyfile_writer_test_connection (NMConnection *connection,410@@ -438,5 +555,6 @@ nms_keyfile_writer_test_connection (NMConnection *connection,
398 out_path,411 out_path,
399 out_reread,412 out_reread,
400 out_reread_same,413 out_reread_same,
diff --git a/tests/full/immutable-netplan-config/task.yaml b/tests/full/immutable-netplan-config/task.yaml
index 4188515..4a87a8d 100644
--- a/tests/full/immutable-netplan-config/task.yaml
+++ b/tests/full/immutable-netplan-config/task.yaml
@@ -7,6 +7,8 @@ execute: |
7 # Ensure we're using a configuration generated from netplan7 # Ensure we're using a configuration generated from netplan
8 test -e /etc/netplan/00-snapd-config.yaml8 test -e /etc/netplan/00-snapd-config.yaml
9 test -e /run/NetworkManager/system-connections/netplan-$eth_if.nmconnection9 test -e /run/NetworkManager/system-connections/netplan-$eth_if.nmconnection
10 # And that we're not connected to a volatile connection generated by NM
11 /snap/bin/network-manager.nmcli c up netplan-$eth_if
1012
11 /snap/bin/network-manager.nmcli c | MATCH "netplan-$eth_if.*$eth_if"13 /snap/bin/network-manager.nmcli c | MATCH "netplan-$eth_if.*$eth_if"
12 # Try to change the DNS server used on this connection14 # Try to change the DNS server used on this connection

Subscribers

People subscribed via source and target branches