From fb851f3294fbf8ab1243dc6fcceef5adf0a403bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8D=C3=B1igo=20Huguet?= Date: Thu, 27 Jul 2023 12:35:15 +0200 Subject: [PATCH] nmcli: warn if daemon version mismatch When updating NetworkManager to a new version, normally the service is not restarted by the installer to avoid interrupting networking. However, next nmcli invocation will use the updated version, but against the older version of the daemon that is still running. Although this is suposed to work, it is advisable that nmcli and daemon's versions are the same. Emit a warning recommending restarting the daemon. Add nmcli test to check the new feature. To avoid breaking the existing tests, test-networkmanager-service now reports the same version than the running nmcli except if it's instructed to report a different one. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1703 --- Makefile.am | 1 + src/nmcli/common.c | 1 + src/nmcli/utils.c | 16 ++++++++++ src/nmcli/utils.h | 2 ++ .../test_version_warn.expected | 30 +++++++++++++++++ src/tests/client/test-client.py | 32 ++++++++++++++++--- tools/test-networkmanager-service.py | 4 ++- 7 files changed, 81 insertions(+), 5 deletions(-) create mode 100644 src/tests/client/test-client.check-on-disk/test_version_warn.expected diff --git a/Makefile.am b/Makefile.am index d49b92c719..1e09199612 100644 --- a/Makefile.am +++ b/Makefile.am @@ -5522,6 +5522,7 @@ EXTRA_DIST += \ src/tests/client/test-client.check-on-disk/test_003.expected \ src/tests/client/test-client.check-on-disk/test_004.expected \ src/tests/client/test-client.check-on-disk/test_offline.expected \ + src/tests/client/test-client.check-on-disk/test_version_warn.expected \ \ src/tests/client/meson.build \ $(NULL) diff --git a/src/nmcli/common.c b/src/nmcli/common.c index 2e8ded7e44..518faa2f73 100644 --- a/src/nmcli/common.c +++ b/src/nmcli/common.c @@ -1250,6 +1250,7 @@ got_client(GObject *source_object, GAsyncResult *res, gpointer user_data) error->message); } else { nmc->client = NM_CLIENT(source_object); + nmc_warn_if_version_mismatch(nmc->client); call_cmd(nmc, g_steal_pointer(&task), call->cmd, diff --git a/src/nmcli/utils.c b/src/nmcli/utils.c index 5fb1ba794f..29e84cc25c 100644 --- a/src/nmcli/utils.c +++ b/src/nmcli/utils.c @@ -1822,3 +1822,19 @@ print_data(const NmcConfig *nmc_config, field_values); } } + +void +nmc_warn_if_version_mismatch(NMClient *client) +{ + const char *nm_ver; + + g_return_if_fail(client != NULL); + + nm_ver = nm_client_get_version(client); + if (!nm_streq0(nm_ver, VERSION)) { + g_printerr(_("Warning: nmcli (%s) and NetworkManager (%s) versions don't match. " + "Restarting NetworkManager is advised.\n"), + VERSION, + nm_ver ? nm_ver : _("Unknown")); + } +} diff --git a/src/nmcli/utils.h b/src/nmcli/utils.h index 6c1fb1db1a..0960c45c6f 100644 --- a/src/nmcli/utils.h +++ b/src/nmcli/utils.h @@ -72,6 +72,8 @@ void print_data(const NmcConfig *nmc_config, int indent, const NmcOutputData *out); +void nmc_warn_if_version_mismatch(NMClient *client); + /*****************************************************************************/ extern const NMMetaEnvironment *const nmc_meta_environment; diff --git a/src/tests/client/test-client.check-on-disk/test_version_warn.expected b/src/tests/client/test-client.check-on-disk/test_version_warn.expected new file mode 100644 index 0000000000..3a22d726d2 --- /dev/null +++ b/src/tests/client/test-client.check-on-disk/test_version_warn.expected @@ -0,0 +1,30 @@ +size: 263 +location: src/tests/client/test-client.py:test_version_warn()/1 +cmd: $NMCLI c +lang: C +returncode: 0 +stdout: 1 bytes +>>> + + +<<< +stderr: 110 bytes +>>> +Warning: nmcli (X.Y.Z) and NetworkManager (A.B.C) versions don't match. Restarting NetworkManager is advised. + +<<< +size: 273 +location: src/tests/client/test-client.py:test_version_warn()/2 +cmd: $NMCLI c +lang: pl_PL.UTF-8 +returncode: 0 +stdout: 1 bytes +>>> + + +<<< +stderr: 110 bytes +>>> +Warning: nmcli (X.Y.Z) and NetworkManager (A.B.C) versions don't match. Restarting NetworkManager is advised. + +<<< diff --git a/src/tests/client/test-client.py b/src/tests/client/test-client.py index b618fb9703..baa229cfc5 100755 --- a/src/tests/client/test-client.py +++ b/src/tests/client/test-client.py @@ -713,6 +713,14 @@ class Util: extra_env, ) + @staticmethod + def get_nmcli_version(): + ver = NM.utils_version() + micro = ver & 0xFF + minor = (ver >> 8) & 0xFF + major = ver >> 16 + return f"{major}.{minor}.{micro}" + ############################################################################### @@ -834,11 +842,15 @@ class NMStubServer: except: return None - def __init__(self, seed): + def __init__(self, seed, version=None): service_path = PathConfiguration.test_networkmanager_service_path() self._conn = dbus.SessionBus() env = os.environ.copy() env["NM_TEST_NETWORKMANAGER_SERVICE_SEED"] = seed + if version is not None: + env["NM_TEST_NETWORKMANAGER_SERVICE_VERSION"] = version + else: + env["NM_TEST_NETWORKMANAGER_SERVICE_VERSION"] = Util.get_nmcli_version() p = subprocess.Popen( [sys.executable, service_path], stdin=subprocess.PIPE, env=env ) @@ -1069,9 +1081,9 @@ class NMTestContext: self._calling_num[calling_fcn] = calling_num return calling_num - def srv_start(self): + def srv_start(self, srv_version=None): self.srv_shutdown() - self.srv = NMStubServer(self.testMethodName) + self.srv = NMStubServer(self.testMethodName, srv_version) def srv_shutdown(self): if self.srv is not None: @@ -2245,6 +2257,18 @@ class TestNmcli(unittest.TestCase): nmc.pexp.expect("NetworkManager is stopped") end_mon(self, nmc) + @nm_test_no_dbus # we need dbus, but we need to pass arguments to srv_start + def test_version_warn(self): + self.ctx.srv_start(srv_version="A.B.C") + self.call_nmcli_l( + ["c"], + replace_stderr=[ + Util.ReplaceTextRegex( + r"\(" + Util.get_nmcli_version() + r"\)", "(X.Y.Z)" + ) + ], + ) + ############################################################################### @@ -2675,7 +2699,7 @@ def main(): sys.executable, __file__, "--started-with-dbus-session", - *sys.argv[1:] + *sys.argv[1:], ) except OSError as e: if e.errno != errno.ENOENT: diff --git a/tools/test-networkmanager-service.py b/tools/test-networkmanager-service.py index d3db7a2643..00dd9e86c5 100755 --- a/tools/test-networkmanager-service.py +++ b/tools/test-networkmanager-service.py @@ -1589,7 +1589,9 @@ class NetworkManager(ExportedObj): PRP_NM_ACTIVATING_CONNECTION: ExportedObj.to_path(None), PRP_NM_STARTUP: False, PRP_NM_STATE: dbus.UInt32(NM.State.DISCONNECTED), - PRP_NM_VERSION: "0.9.9.0", + PRP_NM_VERSION: os.environ.get( + "NM_TEST_NETWORKMANAGER_SERVICE_VERSION", "0.9.9.0" + ), PRP_NM_CONNECTIVITY: dbus.UInt32(NM.ConnectivityState.NONE), }