From e72c732c5077b39811a9f73c718178a5fb2f6a25 Mon Sep 17 00:00:00 2001 From: Hemanth Nakkina Date: Tue, 24 Mar 2026 18:33:26 +0530 Subject: [PATCH] hooks: defer internal OVS startup until charm has configured identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When openstack-hypervisor and microovn are deployed simultaneously, snapd fires the configure hook immediately after snap install — before the charm has applied any snap config. At that point network.ovs-managed-by is still 'auto' and the ovn-chassis plug is not yet connected, so is_ovs_external() returns False. This caused _ensure_internal_ovs_services to start ovs-vswitchd, which creates the system@ovs-system kernel datapath. When microovn subsequently tries to initialise OVS it finds the datapath already in use and fails. The same issue affects the second configure hook, triggered when the charm's own install hook calls `snap set network.ovs-managed-by=auto`, since the identity URL is still the placeholder at that point too. Fix: before calling _ensure_internal_ovs_services, check whether the charm has applied its identity configuration. The guard defers OVS startup when BOTH identity.auth-url is still the placeholder default ("http://localhost:5000/v3") AND identity.username is unset (None). Checking both fields ensures the guard still clears correctly for operators that legitimately run Keystone at http://localhost:5000/v3, since those deployments will always set identity.username. Once either condition is no longer true the guard clears and _ensure_internal_ovs_services runs normally. The guard is only active when network.ovs-managed-by is 'auto'. When explicitly set to 'hypervisor' the operator's intent is unambiguous and ovs-vswitchd is started immediately regardless of identity state. --- openstack_hypervisor/hooks.py | 31 +++++++- tests/unit/test_hooks.py | 141 ++++++++++++++++++++++++++++++++++ 2 files changed, 171 insertions(+), 1 deletion(-) diff --git a/openstack_hypervisor/hooks.py b/openstack_hypervisor/hooks.py index 13a3407..ca7fa78 100644 --- a/openstack_hypervisor/hooks.py +++ b/openstack_hypervisor/hooks.py @@ -3017,7 +3017,36 @@ def configure(snap: Snap) -> None: snap, bool(context.get("network", {}).get("sriov_nic_physical_device_mappings")) ) if not ovs_external: - _ensure_internal_ovs_services(snap, exclude_services) + # When OVS mode is 'auto', the snap doesn't yet know whether microovn will be + # used. The reliable signal that the charm has finished its initial configuration + # is that identity credentials have been set: identity.auth-url is a real Keystone + # endpoint (not the placeholder default) AND identity.username has been provided. + # Until then, skip starting ovs-vswitchd: creating system@ovs-system here would + # block a concurrently-installing microovn snap. + # + # Checking both fields guards against the edge case where an operator legitimately + # runs Keystone at http://localhost:5000/v3 — in that scenario the charm will + # always have set identity.username so the guard still clears correctly. + # + # When OVS mode is explicitly 'hypervisor' (operator/charm intent is clear), + # bypass this check and start internal OVS immediately. + identity_opts = snap.config.get_options("identity") + identity_url = identity_opts.get("identity.auth-url") + identity_username = identity_opts.get("identity.username") + charm_not_configured = ( + _OVS_MANAGED_BY == OVS_MANAGED_BY_AUTO + and identity_url == DEFAULT_CONFIG["identity.auth-url"] + and identity_username is None + ) + if charm_not_configured: + logging.info( + "identity.auth-url is still the default placeholder and " + "identity.username is unset: the charm has not yet applied " + "configuration. Deferring internal OVS service startup to avoid " + "creating system@ovs-system before microovn can install." + ) + else: + _ensure_internal_ovs_services(snap, exclude_services) def _get_configure_context(snap: Snap) -> dict: diff --git a/tests/unit/test_hooks.py b/tests/unit/test_hooks.py index 3744f27..478d67f 100644 --- a/tests/unit/test_hooks.py +++ b/tests/unit/test_hooks.py @@ -1543,6 +1543,9 @@ def test_internal_ovs_not_ready_defers_ovs_configuration(self, mocker, snap): "_ensure_internal_ovs_services", side_effect=lambda *_: order.append("ensure"), ) + # Simulate charm already configured (real identity URL) so the OVS + # startup guard does not interfere with what this test is checking. + snap.config.get_options.return_value.get.return_value = "http://10.0.0.1:5000/v3" hooks.configure(snap) @@ -1581,6 +1584,9 @@ def test_internal_ovs_ready_runs_configuration(self, mocker, snap): "_ensure_internal_ovs_services", side_effect=lambda *_: order.append("ensure"), ) + # Simulate charm already configured (real identity URL) so the OVS + # startup guard does not interfere with what this test is checking. + snap.config.get_options.return_value.get.return_value = "http://10.0.0.1:5000/v3" hooks.configure(snap) @@ -1660,6 +1666,141 @@ def test_external_ovs_ready_check_uses_socket_path(self, mocker, snap, tmp_path) socket_file.touch() assert hooks._external_ovs_ready(snap) is True + def test_internal_ovs_not_started_on_unconfigured_first_run(self, mocker, snap): + """Internal OVS services must NOT be started while identity is unconfigured. + + Snapd fires a configure hook automatically right after 'snap install', and + again when the charm calls ``snap set network.ovs-managed-by=auto`` in its + own install hook — both times before any real Keystone URL has been provided. + The snap detects this by checking that ``identity.auth-url`` still equals the + placeholder default. In that state ``_ensure_internal_ovs_services`` must be + skipped to avoid creating ``system@ovs-system`` before microovn installs. + """ + snap.services.list.return_value = {} + mocker.patch.object(hooks, "_mkdirs") + mocker.patch.object(hooks, "_update_default_config") + mocker.patch.object(hooks, "_setup_secrets") + mocker.patch.object(hooks, "_detect_compute_flavors") + mocker.patch.object(hooks, "_get_configure_context", return_value={"network": {}}) + mocker.patch.object(hooks, "_get_exclude_services", return_value=[]) + mocker.patch.object(hooks, "OVSCli", return_value=mock.Mock()) + # OVS mode is 'auto' (default from conftest) and plug is not connected + mocker.patch.object(hooks, "is_ovs_external", return_value=False) + mocker.patch.object(hooks, "_internal_ovs_ready", return_value=True) + mocker.patch.object(hooks, "RestartOnChange", return_value=nullcontext()) + mocker.patch.object(hooks, "_render_templates") + mocker.patch.object(hooks, "_configure_webdav_apache") + mocker.patch.object(hooks, "_configure_tls") + mocker.patch.object(hooks, "_configure_networking") + mocker.patch.object(hooks, "_configure_kvm") + mocker.patch.object(hooks, "_configure_monitoring_services") + mocker.patch.object(hooks, "_configure_ceph") + mocker.patch.object(hooks, "_configure_masakari_services") + mocker.patch.object(hooks, "_configure_sriov_agent_service") + mock_ensure = mocker.patch.object(hooks, "_ensure_internal_ovs_services") + + # Simulate snap not yet configured by the charm: identity URL is the placeholder + # AND username has not been set yet (None). Both conditions must hold for the + # guard to defer OVS startup. + def unconfigured_identity_get(key, default=None): + if key == "identity.auth-url": + return hooks.DEFAULT_CONFIG["identity.auth-url"] + if key == "identity.username": + return None + return default + + snap.config.get_options.return_value.get.side_effect = unconfigured_identity_get + + hooks.configure(snap) + + # _ensure_internal_ovs_services must NOT have been called — starting + # ovs-vswitchd here would create system@ovs-system and block microovn. + mock_ensure.assert_not_called() + + def test_internal_ovs_started_when_managed_by_hypervisor_with_default_identity( + self, mocker, snap + ): + """Internal OVS must start when explicitly managed by 'hypervisor'. + + Even if ``identity.auth-url`` is still at the placeholder default and + ``identity.username`` has not been set (i.e. the charm hasn't configured + identity yet), setting ``network.ovs-managed-by`` to ``'hypervisor'`` must + bypass the guard and ensure internal OVS services are started. + """ + snap.services.list.return_value = {} + mocker.patch.object(hooks, "_mkdirs") + mocker.patch.object(hooks, "_update_default_config") + mocker.patch.object(hooks, "_setup_secrets") + # Force OVS mode to 'hypervisor' (simulates charm explicitly setting it). + mocker.patch.object( + hooks, + "_set_ovs_managed_by", + side_effect=lambda _: setattr( + hooks, "_OVS_MANAGED_BY", hooks.OVS_MANAGED_BY_HYPERVISOR + ), + ) + mocker.patch.object(hooks, "_detect_compute_flavors") + mocker.patch.object(hooks, "_get_configure_context", return_value={"network": {}}) + mocker.patch.object(hooks, "_get_exclude_services", return_value=[]) + mocker.patch.object(hooks, "OVSCli", return_value=mock.Mock()) + mocker.patch.object(hooks, "is_ovs_external", return_value=False) + mocker.patch.object(hooks, "_internal_ovs_ready", return_value=True) + mocker.patch.object(hooks, "RestartOnChange", return_value=nullcontext()) + mocker.patch.object(hooks, "_render_templates") + mocker.patch.object(hooks, "_configure_webdav_apache") + mocker.patch.object(hooks, "_configure_tls") + mocker.patch.object(hooks, "_configure_networking") + mocker.patch.object(hooks, "_configure_kvm") + mocker.patch.object(hooks, "_configure_monitoring_services") + mocker.patch.object(hooks, "_configure_ceph") + mocker.patch.object(hooks, "_configure_masakari_services") + mocker.patch.object(hooks, "_configure_sriov_agent_service") + mock_ensure = mocker.patch.object(hooks, "_ensure_internal_ovs_services") + # Identity is still unconfigured (placeholder URL, no username) — the guard + # must be bypassed because mode is explicitly 'hypervisor'. + snap.config.get_options.return_value.get.return_value = hooks.DEFAULT_CONFIG[ + "identity.auth-url" + ] + + hooks.configure(snap) + + mock_ensure.assert_called_once() + + def test_internal_ovs_started_when_identity_configured(self, mocker, snap): + """Internal OVS must start once the charm has provided a real identity URL. + + When ``identity.auth-url`` has been set to a real Keystone endpoint (anything + other than the placeholder default) and ``network.ovs-managed-by`` is 'auto' + with no plug connected, ``_ensure_internal_ovs_services`` must be called. + """ + snap.services.list.return_value = {} + mocker.patch.object(hooks, "_mkdirs") + mocker.patch.object(hooks, "_update_default_config") + mocker.patch.object(hooks, "_setup_secrets") + mocker.patch.object(hooks, "_detect_compute_flavors") + mocker.patch.object(hooks, "_get_configure_context", return_value={"network": {}}) + mocker.patch.object(hooks, "_get_exclude_services", return_value=[]) + mocker.patch.object(hooks, "OVSCli", return_value=mock.Mock()) + mocker.patch.object(hooks, "is_ovs_external", return_value=False) + mocker.patch.object(hooks, "_internal_ovs_ready", return_value=True) + mocker.patch.object(hooks, "RestartOnChange", return_value=nullcontext()) + mocker.patch.object(hooks, "_render_templates") + mocker.patch.object(hooks, "_configure_webdav_apache") + mocker.patch.object(hooks, "_configure_tls") + mocker.patch.object(hooks, "_configure_networking") + mocker.patch.object(hooks, "_configure_kvm") + mocker.patch.object(hooks, "_configure_monitoring_services") + mocker.patch.object(hooks, "_configure_ceph") + mocker.patch.object(hooks, "_configure_masakari_services") + mocker.patch.object(hooks, "_configure_sriov_agent_service") + mock_ensure = mocker.patch.object(hooks, "_ensure_internal_ovs_services") + # Charm has provided the real Keystone URL. + snap.config.get_options.return_value.get.return_value = "http://10.0.0.1:5000/v3" + + hooks.configure(snap) + + mock_ensure.assert_called_once() + class TestDPDKConfigReady: """Tests for _dpdk_config_is_ready function."""