hooks: defer internal OVS startup until charm has configured identity

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:

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."""