Merge ~slyon/snappy-hwe-snaps/+git/network-manager:slyon/netplan-modify-connections 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: 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 :
review: Approve (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
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
1diff --git a/snap-patch/networkmanager/0002-nm-netplan-keyfile.patch b/snap-patch/networkmanager/0002-nm-netplan-keyfile.patch
2index ad89e95..ef7d899 100644
3--- a/snap-patch/networkmanager/0002-nm-netplan-keyfile.patch
4+++ b/snap-patch/networkmanager/0002-nm-netplan-keyfile.patch
5@@ -1,4 +1,4 @@
6-From 5ffe6c36ed2e9a5effc07d706f0b8dc34c7515ab Mon Sep 17 00:00:00 2001
7+From 4c06fede36936e58e75babcb9542a6b27a7cb19f Mon Sep 17 00:00:00 2001
8 From: Lukas Märdian <lukas.maerdian@canonical.com>
9 Date: Tue, 2 Feb 2021 15:52:05 +0100
10 Subject: [PATCH] netplan: make use of libnetplan for YAML backend
11@@ -34,12 +34,12 @@ https://github.com/slyon/NetworkManager/tree/slyon/backend-1.22.10
12 Makefile.am | 5 +-
13 meson.build | 3 +
14 src/meson.build | 2 +
15- .../plugins/keyfile/nms-keyfile-plugin.c | 19 +++
16+ .../plugins/keyfile/nms-keyfile-plugin.c | 22 ++++
17 .../plugins/keyfile/nms-keyfile-utils.c | 57 +++++++++
18 .../plugins/keyfile/nms-keyfile-utils.h | 4 +
19- .../plugins/keyfile/nms-keyfile-writer.c | 108 ++++++++++++++++++
20- .../keyfile/tests/test-keyfile-settings.c | 45 ++++++--
21- 8 files changed, 234 insertions(+), 9 deletions(-)
22+ .../plugins/keyfile/nms-keyfile-writer.c | 118 ++++++++++++++++++
23+ .../keyfile/tests/test-keyfile-settings.c | 45 +++++--
24+ 8 files changed, 247 insertions(+), 9 deletions(-)
25
26 diff --git a/Makefile.am b/Makefile.am
27 index b37aa4e712311cb23a675af70ec1287fcd3d63f5..2e4e146412de6102068bb953c38cb8091e7f010a 100644
28@@ -92,7 +92,7 @@ index 748fa519bc56305bfe485b9ea1207942fe5b5874..398efb3c69c37bd1d6214602d194dfad
29
30 if enable_wext
31 diff --git a/src/settings/plugins/keyfile/nms-keyfile-plugin.c b/src/settings/plugins/keyfile/nms-keyfile-plugin.c
32-index fdb88d2a6d6218460fe191d7bc5be08b492886f5..60b8490fdaa7cbf1ffae3dc89f19289bc842acad 100644
33+index fdb88d2a6d6218460fe191d7bc5be08b492886f5..038cf13a1d7ea287d8c491848b9284334d1b9d70 100644
34 --- a/src/settings/plugins/keyfile/nms-keyfile-plugin.c
35 +++ b/src/settings/plugins/keyfile/nms-keyfile-plugin.c
36 @@ -12,6 +12,7 @@
37@@ -137,14 +137,17 @@ index fdb88d2a6d6218460fe191d7bc5be08b492886f5..60b8490fdaa7cbf1ffae3dc89f19289b
38 if (!NM_IN_SET (storage->storage_type, NMS_KEYFILE_STORAGE_TYPE_ETC,
39 NMS_KEYFILE_STORAGE_TYPE_RUN)) {
40 nm_utils_error_set (error,
41-@@ -1011,6 +1025,11 @@ delete_connection (NMSettingsPlugin *plugin,
42+@@ -1011,6 +1025,14 @@ delete_connection (NMSettingsPlugin *plugin,
43 } else
44 operation_message = "deleted from disk";
45
46 + g_autofree gchar* netplan_id = netplan_get_id_from_nm_filename(previous_filename, ssid);
47-+ netplan_delete_connection(netplan_id, NULL);
48-+ generate_netplan(NULL);
49-+ _fix_netplan_interface_name(NULL);
50++ if (netplan_id) {
51++ _LOGI ("deleting netplan connection: %s", netplan_id);
52++ netplan_delete_connection(netplan_id, NULL);
53++ generate_netplan(NULL);
54++ _fix_netplan_interface_name(NULL);
55++ }
56 +
57 _LOGT ("commit: deleted \"%s\", %s %s (%s%s%s%s)",
58 previous_filename,
59@@ -249,7 +252,7 @@ index f943d65ca6834efd2bc243397d122243d906712e..4803135d3f60cdfae4a23640cefcbe6d
60 +
61 #endif /* __NMS_KEYFILE_UTILS_H__ */
62 diff --git a/src/settings/plugins/keyfile/nms-keyfile-writer.c b/src/settings/plugins/keyfile/nms-keyfile-writer.c
63-index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..627c72d7a0ff76e9b00e0a5d6c19111583ba3d66 100644
64+index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..1b8affdfe91935049edc9ba4323dc627109a59b5 100644
65 --- a/src/settings/plugins/keyfile/nms-keyfile-writer.c
66 +++ b/src/settings/plugins/keyfile/nms-keyfile-writer.c
67 @@ -11,6 +11,10 @@
68@@ -271,7 +274,7 @@ index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..627c72d7a0ff76e9b00e0a5d6c191115
69 GError **error)
70 {
71 gs_unref_keyfile GKeyFile *kf_file = NULL;
72-@@ -364,6 +369,103 @@ _internal_write_connection (NMConnection *connection,
73+@@ -364,6 +369,113 @@ _internal_write_connection (NMConnection *connection,
74 && !nm_streq (path, existing_path))
75 unlink (existing_path);
76
77@@ -283,7 +286,17 @@ index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..627c72d7a0ff76e9b00e0a5d6c191115
78 + g_autofree gchar* netplan_id = (existing_path && strstr(existing_path, "system-connections/netplan-")) ?
79 + netplan_get_id_from_nm_filename(existing_path, ssid) : NULL;
80 + netplan_clear_netdefs();
81-+ if (!netplan_parse_keyfile(path, &local_err)) { // push keyfile into libnetplan for parsing
82++
83++ const gchar* kf_path = path;
84++ if (netplan_id && existing_path) {
85++ GFile* from = g_file_new_for_path(path);
86++ GFile* to = g_file_new_for_path(existing_path);
87++ g_file_copy(from, to, G_FILE_COPY_OVERWRITE, NULL, NULL, NULL, NULL);
88++ kf_path = existing_path;
89++ }
90++ // push keyfile into libnetplan for parsing (using existing_path, if available,
91++ // to be able to extract the original netdef_id and override existing settings)
92++ if (!netplan_parse_keyfile(kf_path, &local_err)) {
93 + g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
94 + "netplan: YAML translation failed");
95 + return FALSE;
96@@ -375,7 +388,7 @@ index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..627c72d7a0ff76e9b00e0a5d6c191115
97 NM_SET_OUT (out_reread, g_steal_pointer (&reread));
98 NM_SET_OUT (out_reread_same, reread_same);
99 NM_SET_OUT (out_path, g_steal_pointer (&path));
100-@@ -407,6 +509,7 @@ nms_keyfile_writer_connection (NMConnection *connection,
101+@@ -407,6 +519,7 @@ nms_keyfile_writer_connection (NMConnection *connection,
102 out_path,
103 out_reread,
104 out_reread_same,
105@@ -383,7 +396,7 @@ index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..627c72d7a0ff76e9b00e0a5d6c191115
106 error);
107 }
108
109-@@ -420,6 +523,10 @@ nms_keyfile_writer_test_connection (NMConnection *connection,
110+@@ -420,6 +533,10 @@ nms_keyfile_writer_test_connection (NMConnection *connection,
111 gboolean *out_reread_same,
112 GError **error)
113 {
114@@ -394,7 +407,7 @@ index fa95198c0084ac6b907f7ffa7ba826a1cca6a025..627c72d7a0ff76e9b00e0a5d6c191115
115 return _internal_write_connection (connection,
116 FALSE,
117 FALSE,
118-@@ -438,5 +545,6 @@ nms_keyfile_writer_test_connection (NMConnection *connection,
119+@@ -438,5 +555,6 @@ nms_keyfile_writer_test_connection (NMConnection *connection,
120 out_path,
121 out_reread,
122 out_reread_same,
123diff --git a/tests/full/immutable-netplan-config/task.yaml b/tests/full/immutable-netplan-config/task.yaml
124index 4188515..4a87a8d 100644
125--- a/tests/full/immutable-netplan-config/task.yaml
126+++ b/tests/full/immutable-netplan-config/task.yaml
127@@ -7,6 +7,8 @@ execute: |
128 # Ensure we're using a configuration generated from netplan
129 test -e /etc/netplan/00-snapd-config.yaml
130 test -e /run/NetworkManager/system-connections/netplan-$eth_if.nmconnection
131+ # And that we're not connected to a volatile connection generated by NM
132+ /snap/bin/network-manager.nmcli c up netplan-$eth_if
133
134 /snap/bin/network-manager.nmcli c | MATCH "netplan-$eth_if.*$eth_if"
135 # Try to change the DNS server used on this connection

Subscribers

People subscribed via source and target branches