diff --git a/openstack_hypervisor/hooks.py b/openstack_hypervisor/hooks.py index 7a76abf..25cf34f 100644 --- a/openstack_hypervisor/hooks.py +++ b/openstack_hypervisor/hooks.py @@ -44,6 +44,7 @@ socket_path, ) from openstack_hypervisor.log import setup_logging +from openstack_hypervisor.ovn_env import OVNEnvError, parse_ovn_env from openstack_hypervisor.ovs import ( OVSCli, OVSCommandError, @@ -342,7 +343,6 @@ def _get_local_ip_by_default_route() -> str: "network.ovs-lcore-mask": UNSET, "network.ovs-dpdk-ports": UNSET, "network.dpdk-driver": "vfio-pci", - "network.ovn-sb-connection": UNSET, "network.ovn-cert": UNSET, "network.ovn-key": UNSET, "network.ovn-cacert": UNSET, @@ -386,7 +386,14 @@ def _get_local_ip_by_default_route() -> str: "credentials.ovn_metadata_proxy_shared_secret", "network.nova_metadata_proxy_url", ], - "neutron-ovn-metadata-agent": ["credentials", "network", "node", "network.ovn_key"], + "neutron-ovn-agent": [ + "credentials", + "network", + "node", + "network.ovn_key", + "network.ovn_nb_connection", + "network.ovn_sb_connection", + ], "ceilometer-compute-agent": [ "identity.password", "identity.username", @@ -518,11 +525,11 @@ def _split_dedicated_cores_by_profile( }, Path("etc/neutron/neutron.conf"): { "template": "neutron.conf.j2", - "services": ["neutron-ovn-metadata-agent"], + "services": ["neutron-ovn-agent"], }, - Path("etc/neutron/neutron_ovn_metadata_agent.ini"): { - "template": "neutron_ovn_metadata_agent.ini.j2", - "services": ["neutron-ovn-metadata-agent"], + Path("etc/neutron/neutron_ovn_agent.ini"): { + "template": "neutron_ovn_agent.ini.j2", + "services": ["neutron-ovn-agent"], }, Path("etc/neutron/neutron_sriov_nic_agent.ini"): { "template": "neutron_sriov_nic_agent.ini.j2", @@ -583,6 +590,7 @@ def __init__(self, snap: Snap, files: dict, exclude_services: list = None): self.files = files self.file_hash = {} self.exclude_services = exclude_services or [] + self.restarted_services = set() def __enter__(self): """Record all file hashes on entry.""" @@ -607,11 +615,13 @@ def __exit__(self, exc_type, exc_value, exc_traceback): if new_hash != self.file_hash[file]: restart_services.extend(self.files[file].get("services", [])) - restart_services = set([s for s in restart_services if s not in self.exclude_services]) - if not restart_services: + self.restarted_services = set( + [service for service in restart_services if service not in self.exclude_services] + ) + if not self.restarted_services: return - _restart_services(self.snap, restart_services) + _restart_services(self.snap, self.restarted_services) def _service_subset(snap: Snap, names: typing.Iterable[str]) -> tuple[list[str], Dict[str, Any]]: @@ -2198,7 +2208,6 @@ def _check_config_present(key: str, context: dict) -> bool: def _services_not_ready(context: dict) -> List[str]: """Check if any services are missing keys they need to function.""" - logging.warning(f"Context {context}") not_ready = [] for svc in services(): for required in REQUIRED_CONFIG.get(svc, []): @@ -2483,11 +2492,17 @@ def configure(snap: Snap) -> None: ovs_deferred = not _microovn_ovs_ready(snap) _ensure_services_stopped(snap, exclude_services) - with RestartOnChange(snap, {**TEMPLATES, **TLS_TEMPLATES}, exclude_services): + with RestartOnChange( + snap, {**TEMPLATES, **TLS_TEMPLATES}, exclude_services + ) as restart_on_change: _render_templates(snap, context) _configure_tls(snap, configure_ovn_tls=not ovs_deferred) _configure_webdav_apache(snap, context) + ovn_agent = "neutron-ovn-agent" + if ovn_agent not in exclude_services and ovn_agent not in restart_on_change.restarted_services: + _ensure_services_started(snap, [ovn_agent]) + if ovs_deferred: logging.info( "MicroOVN OVSDB socket not present yet, deferring OVS/OVN configuration " @@ -2568,8 +2583,6 @@ def _get_configure_context(snap: Snap) -> dict: context["compute"]["allocated_cores"] = allocated_cores context["compute"]["cpu_shared_set"] = cpu_shared_set - logging.info(context) - if not context.get("identity"): context["identity"] = {} if not context["identity"].get("keystone-region-name"): @@ -2582,6 +2595,19 @@ def _get_configure_context(snap: Snap) -> dict: # Add OVS socket path to network context for template rendering context["network"]["ovs_socket_path"] = ovs_switch_socket(snap) + context["network"].pop("ovn_nb_connection", None) + context["network"].pop("ovn_sb_connection", None) + ovn_env_path = snap.paths.data / "microovn" / "ovn-env" / "env" / "ovn.env" + try: + ovn_connections = parse_ovn_env(ovn_env_path) + except OVNEnvError: + logging.warning( + "MicroOVN connection data is unavailable or invalid; " + "neutron-ovn-agent will remain stopped" + ) + else: + context["network"]["ovn_nb_connection"] = ovn_connections["OVN_NB_CONNECT"] + context["network"]["ovn_sb_connection"] = ovn_connections["OVN_SB_CONNECT"] _set_nova_metadata_proxy_context(context) return context diff --git a/openstack_hypervisor/ovn_env.py b/openstack_hypervisor/ovn_env.py new file mode 100644 index 0000000..6e7d109 --- /dev/null +++ b/openstack_hypervisor/ovn_env.py @@ -0,0 +1,102 @@ +# SPDX-FileCopyrightText: 2026 - Canonical Ltd +# SPDX-License-Identifier: Apache-2.0 + +import ipaddress +import re +from pathlib import Path + +REQUIRED_CONNECTIONS = ("OVN_NB_CONNECT", "OVN_SB_CONNECT") +SUPPORTED_PROTOCOLS = frozenset(("ssl", "tcp")) + +_ASSIGNMENT = re.compile( + r"(?P[A-Za-z_][A-Za-z0-9_]*)=(?P['\"])(?P.*)(?P=quote)" +) +_IPV4_ENDPOINT = re.compile(r"(?P[^:]+):(?P[0-9]+)") +_IPV6_ENDPOINT = re.compile(r"\[(?P[^]]+)\]:(?P[0-9]+)") + + +class OVNEnvError(ValueError): + """Raised when MicroOVN's generated environment is unavailable or invalid.""" + + +def _validate_endpoint(endpoint: str, connection_name: str) -> None: + """Validate one MicroOVN-generated OVSDB endpoint.""" + if not endpoint or any(character.isspace() for character in endpoint): + raise OVNEnvError(f"Invalid endpoint in {connection_name}") + + try: + protocol, address = endpoint.split(":", 1) + except ValueError as exc: + raise OVNEnvError(f"Invalid endpoint in {connection_name}") from exc + if protocol not in SUPPORTED_PROTOCOLS: + raise OVNEnvError(f"Unsupported endpoint protocol in {connection_name}") + + ipv6 = address.startswith("[") + match = (_IPV6_ENDPOINT if ipv6 else _IPV4_ENDPOINT).fullmatch(address) + if match is None: + raise OVNEnvError(f"Invalid endpoint address in {connection_name}") + + try: + parsed_address = ipaddress.ip_address(match.group("host")) + except ValueError as exc: + raise OVNEnvError(f"Invalid endpoint address in {connection_name}") from exc + if ipv6 != (parsed_address.version == 6): + raise OVNEnvError(f"Invalid endpoint address in {connection_name}") + + port = int(match.group("port")) + if not 1 <= port <= 65535: + raise OVNEnvError(f"Invalid endpoint port in {connection_name}") + + +def _validate_connection(connection: str, connection_name: str) -> None: + """Validate every endpoint in a comma-separated connection string.""" + endpoints = connection.split(",") + if not endpoints: + raise OVNEnvError(f"Empty required assignment: {connection_name}") + for endpoint in endpoints: + _validate_endpoint(endpoint, connection_name) + + +def _read_lines(path: Path) -> list[str]: + """Read the environment as UTF-8 without interpreting its contents.""" + try: + return path.read_text(encoding="utf-8").splitlines() + except (OSError, UnicodeError) as exc: + raise OVNEnvError("Unable to read OVN environment file") from exc + + +def _parse_required_assignments(lines: list[str]) -> dict[str, str]: + """Extract required quoted assignments and reject malformed input.""" + connections: dict[str, str] = {} + for line_number, line in enumerate(lines, start=1): + if not line.strip() or line.lstrip().startswith("#"): + continue + + match = _ASSIGNMENT.fullmatch(line) + if match is None: + raise OVNEnvError(f"Malformed assignment on line {line_number}") + + name = match.group("name") + if name not in REQUIRED_CONNECTIONS: + continue + if name in connections: + raise OVNEnvError(f"Duplicate required assignment: {name}") + + value = match.group("value") + if not value: + raise OVNEnvError(f"Empty required assignment: {name}") + connections[name] = value + + missing = [name for name in REQUIRED_CONNECTIONS if name not in connections] + if missing: + raise OVNEnvError(f"Missing required assignment: {', '.join(missing)}") + return connections + + +def parse_ovn_env(path: Path) -> dict[str, str]: + """Strictly parse required OVN connections without executing the file.""" + connections = _parse_required_assignments(_read_lines(path)) + + for name, connection in connections.items(): + _validate_connection(connection, name) + return connections diff --git a/openstack_hypervisor/services.py b/openstack_hypervisor/services.py index f20ccd3..fae4f9b 100644 --- a/openstack_hypervisor/services.py +++ b/openstack_hypervisor/services.py @@ -134,48 +134,53 @@ def run(self, snap: Snap) -> int: nova_api_metadata = partial(entry_point, NovaAPIMetadataService) -class NeutronOVNMetadataAgentService(OpenStackService): - """A python service object used to run the neutron-ovn-metadata-agent daemon.""" +class NeutronOVNAgentService(OpenStackService): + """A service object used to run the neutron-ovn-agent daemon.""" conf_files = [ Path("etc/neutron/neutron.conf"), - Path("etc/neutron/neutron_ovn_metadata_agent.ini"), + Path("etc/neutron/neutron_ovn_agent.ini"), ] conf_dirs = [ Path("etc/neutron/neutron.conf.d"), ] - executable = Path("usr/bin/neutron-ovn-metadata-agent") + executable = Path("usr/bin/neutron-ovn-agent") def run(self, snap: Snap) -> int: - """Run neutron-ovn-metadata-agent once MicroOVN's OVSDB is ready.""" + """Run neutron-ovn-agent once required connections and local OVS are ready.""" setup_logging(snap.paths.common / f"{self.executable.name}-{snap.name}.log") - ovsdb_connection = self._ovsdb_connection(snap) - if not ovsdb_connection: + ovsdb_connections = self._ovsdb_connections(snap) + if ovsdb_connections is None: return 1 + ovsdb_connection = ovsdb_connections["ovsdb_connection"] if not self._wait_for_ovsdb_schema(snap, ovsdb_connection): return 1 return super().run(snap) - def _ovsdb_connection(self, snap: Snap) -> str | None: - """Read the configured MicroOVN OVSDB connection string.""" - config_path = snap.paths.common / "etc/neutron/neutron_ovn_metadata_agent.ini" + def _ovsdb_connections(self, snap: Snap) -> dict[str, str] | None: + """Read all connections required by the OVN agent.""" + config_path = snap.paths.common / "etc/neutron/neutron_ovn_agent.ini" parser = configparser.ConfigParser() try: if not parser.read(config_path): - logging.error("Unable to read OVN metadata agent config: %s", config_path) + logging.error("Unable to read OVN agent config: %s", config_path) return None - ovsdb_connection = parser.get("ovs", "ovsdb_connection", fallback="").strip() - except configparser.Error as exc: - logging.error("Unable to parse OVN metadata agent config %s: %s", config_path, exc) + connections = { + "ovsdb_connection": parser.get("ovs", "ovsdb_connection", fallback="").strip(), + "ovn_nb_connection": parser.get("ovn", "ovn_nb_connection", fallback="").strip(), + "ovn_sb_connection": parser.get("ovn", "ovn_sb_connection", fallback="").strip(), + } + except (configparser.Error, OSError, UnicodeError): + logging.error("Unable to parse OVN agent config: %s", config_path) return None - if not ovsdb_connection: - logging.error("ovsdb_connection is not configured in %s", config_path) + if not all(connections.values()): + logging.error("Required OVSDB connections are not configured in %s", config_path) return None - return ovsdb_connection + return connections def _wait_for_ovsdb_schema(self, snap: Snap, ovsdb_connection: str) -> bool: """Wait until ovsdb-client can retrieve the Open_vSwitch schema.""" @@ -190,14 +195,12 @@ def _wait_for_ovsdb_schema(self, snap: Snap, ovsdb_connection: str) -> bool: while True: if socket_path and not socket_path.exists(): - logging.info("Waiting for MicroOVN OVSDB socket: %s", socket_path) + logging.info("Waiting for the local MicroOVN OVSDB socket") elif self._ovsdb_schema_available(command): return True if time.monotonic() >= deadline: - logging.error( - "Timed out waiting for Open_vSwitch schema from %s", ovsdb_connection - ) + logging.error("Timed out waiting for the local Open_vSwitch schema") return False time.sleep(OVSDB_SCHEMA_CHECK_INTERVAL) @@ -221,7 +224,7 @@ def _unix_socket_path(self, ovsdb_connection: str) -> Path | None: return Path(ovsdb_connection.removeprefix("unix:")) -neutron_ovn_metadata_agent = partial(entry_point, NeutronOVNMetadataAgentService) +neutron_ovn_agent = partial(entry_point, NeutronOVNAgentService) class NeutronSRIOVNicAgentService(OpenStackService): diff --git a/pyproject.toml b/pyproject.toml index 896d822..229315e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -71,7 +71,7 @@ tics= [ [project.scripts] nova-compute-service = "openstack_hypervisor.services:nova_compute" nova-api-metadata-service = "openstack_hypervisor.services:nova_api_metadata" -neutron-ovn-metadata-agent-service = "openstack_hypervisor.services:neutron_ovn_metadata_agent" +neutron-ovn-agent-service = "openstack_hypervisor.services:neutron_ovn_agent" neutron-sriov-nic-agent-service = "openstack_hypervisor.services:neutron_sriov_nic_agent" ceilometer-compute-agent-service = "openstack_hypervisor.services:ceilometer_compute_agent" masakari-instancemonitor-service = "openstack_hypervisor.services:masakari_instancemonitor" diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index 861452b..7921560 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -210,8 +210,8 @@ apps: - mount-observe - nvme-control - neutron-ovn-metadata-agent: - command: 'bin/neutron-ovn-metadata-agent-service' + neutron-ovn-agent: + command: 'bin/neutron-ovn-agent-service' daemon: simple restart-condition: on-failure restart-delay: 7s @@ -223,6 +223,7 @@ apps: - ovn-chassis - microstack-support - firewall-control + - shared-memory neutron-sriov-nic-agent: command: 'bin/neutron-sriov-nic-agent-service' @@ -253,6 +254,7 @@ apps: - network-bind - firewall-control - microstack-support + - shared-memory masakari-instancemonitor: command: 'bin/masakari-instancemonitor-service' @@ -859,6 +861,7 @@ hooks: - epa-info - etc-driverctl - ovn-chassis + - ovn-env - dm-multipath connect-slot-hypervisor-config: plugs: @@ -879,3 +882,8 @@ plugs: ovn-chassis: interface: content target: $SNAP_DATA/microovn/chassis + ovn-env: + interface: content + target: $SNAP_DATA/microovn/ovn-env + shared-memory: + private: true diff --git a/templates/neutron_ovn_metadata_agent.ini.j2 b/templates/neutron_ovn_agent.ini.j2 similarity index 71% rename from templates/neutron_ovn_metadata_agent.ini.j2 rename to templates/neutron_ovn_agent.ini.j2 index ac004b2..6c5e3d2 100644 --- a/templates/neutron_ovn_metadata_agent.ini.j2 +++ b/templates/neutron_ovn_agent.ini.j2 @@ -9,13 +9,18 @@ metadata_proxy_shared_secret = {{ credentials.ovn_metadata_proxy_shared_secret } {% endif -%} debug = {{ logging.debug }} +[agent] +extensions = metadata + [ovs] ovsdb_connection = {{ network.ovs_socket_path }} [ovn] -{% if network.ovn_sb_connection %} +ovn_nb_connection = {{ network.ovn_nb_connection }} +ovn_nb_private_key = {{ snap_common }}/etc/ssl/private/ovn-key.pem +ovn_nb_certificate = {{ snap_common }}/etc/ssl/certs/ovn-cert.pem +ovn_nb_ca_cert = {{ snap_common }}/etc/ssl/certs/ovn-cacert.pem ovn_sb_connection = {{ network.ovn_sb_connection }} ovn_sb_private_key = {{ snap_common }}/etc/ssl/private/ovn-key.pem ovn_sb_certificate = {{ snap_common }}/etc/ssl/certs/ovn-cert.pem ovn_sb_ca_cert = {{ snap_common }}/etc/ssl/certs/ovn-cacert.pem -{% endif %} diff --git a/tests/unit/test_hooks.py b/tests/unit/test_hooks.py index 54186e3..ba1f30d 100644 --- a/tests/unit/test_hooks.py +++ b/tests/unit/test_hooks.py @@ -285,7 +285,7 @@ def test_services(self): "file-transfer", "libvirtd", "masakari-instancemonitor", - "neutron-ovn-metadata-agent", + "neutron-ovn-agent", "neutron-sriov-nic-agent", "nova-api-metadata", "nova-compute", @@ -316,7 +316,7 @@ def test_services_not_ready(self, snap): "ceilometer-compute-agent", "file-transfer", "masakari-instancemonitor", - "neutron-ovn-metadata-agent", + "neutron-ovn-agent", "nova-api-metadata", "nova-compute", ] @@ -325,7 +325,7 @@ def test_services_not_ready(self, snap): "ceilometer-compute-agent", "file-transfer", "masakari-instancemonitor", - "neutron-ovn-metadata-agent", + "neutron-ovn-agent", "nova-api-metadata", "nova-compute", ] @@ -333,7 +333,7 @@ def test_services_not_ready(self, snap): config["node"] = {"fqdn": "myhost.maas"} assert hooks._services_not_ready(config) == [ "file-transfer", - "neutron-ovn-metadata-agent", + "neutron-ovn-agent", "nova-api-metadata", ] config["network"] = { @@ -344,16 +344,26 @@ def test_services_not_ready(self, snap): } assert hooks._services_not_ready(config) == [ "file-transfer", - "neutron-ovn-metadata-agent", + "neutron-ovn-agent", "nova-api-metadata", ] config["network"]["nova_metadata_proxy_url"] = "http://internal/nova-metadata" assert hooks._services_not_ready(config) == [ "file-transfer", - "neutron-ovn-metadata-agent", + "neutron-ovn-agent", "nova-api-metadata", ] config["credentials"] = {"ovn_metadata_proxy_shared_secret": "secret"} + assert hooks._services_not_ready(config) == [ + "file-transfer", + "neutron-ovn-agent", + ] + config["network"].update( + { + "ovn_nb_connection": "ssl:10.0.0.10:6641", + "ovn_sb_connection": "ssl:10.0.0.10:6642", + } + ) assert hooks._services_not_ready(config) == ["file-transfer"] config["compute"] = { "cacert": "cacert", @@ -799,6 +809,225 @@ def as_dict(self): assert context["compute"]["cpu_shared_set"] == split_shared_set +def _minimal_config_options(): + class ConfigOptionsDict(dict): + def as_dict(self): + return dict(self) + + return ConfigOptionsDict( + { + section: {} + for section in [ + "compute", + "network", + "identity", + "logging", + "node", + "rabbitmq", + "credentials", + "telemetry", + "monitoring", + "ca", + "masakari", + "sev", + "internal", + ] + } + ) + + +def _prepare_minimal_configure_context(mocker, snap): + mocker.patch.object(snap.config, "get_options", return_value=_minimal_config_options()) + mocker.patch.object(hooks, "_is_multipathd_available", return_value=False) + mocker.patch.object(hooks, "get_cpu_pinning_from_socket", return_value=("", "")) + mocker.patch.object(hooks, "_set_sriov_context") + mocker.patch.object(hooks, "_set_pci_context") + + +def _prepare_configure_reconciliation(mocker, snap): + config_options = _minimal_config_options() + config_options["network"] = { + "ovn_key": "key", + "ovn_cert": "cert", + "ovn_cacert": "cacert", + } + config_options["credentials"] = {"ovn_metadata_proxy_shared_secret": "secret"} + config_options["node"] = {"fqdn": "compute-0.internal"} + mocker.patch.object(snap.config, "get_options", return_value=config_options) + + mocker.patch.object(hooks, "setup_logging") + 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, "_is_multipathd_available", return_value=False) + mocker.patch.object(hooks, "get_cpu_pinning_from_socket", return_value=("", "")) + mocker.patch.object(hooks, "_set_sriov_context") + mocker.patch.object(hooks, "_set_pci_context") + mocker.patch.object(hooks, "OVSCli") + mocker.patch.object(hooks, "_microovn_ovs_ready", return_value=False) + mocker.patch.object(hooks, "ovs_switchd_ctl_socket", return_value=None) + + agent_config = Path("etc/neutron/neutron_ovn_agent.ini") + mocker.patch.object( + hooks, + "TEMPLATES", + { + agent_config: { + "template": "neutron_ovn_agent.ini.j2", + "services": ["neutron-ovn-agent"], + } + }, + ) + mocker.patch.object(hooks, "TLS_TEMPLATES", {}) + mocker.patch.object( + hooks, + "_get_template", + return_value=hooks.Template( + "{{ network.ovn_nb_connection | default('') }}|" + "{{ network.ovn_sb_connection | default('') }}\n" + ), + ) + + for name in [ + "_configure_tls", + "_configure_webdav_apache", + "_configure_kvm", + "_configure_monitoring_services", + "_configure_ceph", + "_configure_masakari_services", + "_configure_sriov_agent_service", + ]: + mocker.patch.object(hooks, name) + + ensure_stopped = mocker.patch.object(hooks, "_ensure_services_stopped") + restart_services = mocker.patch.object(hooks, "_restart_services") + + rendered_config = snap.paths.common / agent_config + rendered_config.parent.mkdir(parents=True, exist_ok=True) + rendered_config.write_text("previous configuration\n", encoding="utf-8") + return ensure_stopped, restart_services + + +def test_configure_restarts_eligible_ovn_agent_with_valid_environment(mocker, snap): + ovn_env = snap.paths.data / "microovn" / "ovn-env" / "env" / "ovn.env" + ovn_env.parent.mkdir(parents=True) + ovn_env.write_text( + 'OVN_NB_CONNECT="ssl:192.0.2.10:6641"\n' 'OVN_SB_CONNECT="ssl:192.0.2.10:6642"\n', + encoding="utf-8", + ) + ensure_stopped, restart_services = _prepare_configure_reconciliation(mocker, snap) + snap.services.list.return_value = { + "neutron-ovn-agent": service_mock(enabled=False, active=False) + } + snapctl = mocker.patch.object(hooks, "SnapCtl").return_value + + hooks.configure(snap) + + excluded_services = ensure_stopped.call_args.args[1] + assert "neutron-ovn-agent" not in excluded_services + restarted_services = restart_services.call_args.args[1] + assert set(restarted_services) == {"neutron-ovn-agent"} + snapctl.start.assert_not_called() + + +def test_configure_starts_eligible_inactive_ovn_agent_when_config_is_unchanged(mocker, snap): + nb_connection = "ssl:192.0.2.10:6641" + sb_connection = "ssl:192.0.2.10:6642" + ovn_env = snap.paths.data / "microovn" / "ovn-env" / "env" / "ovn.env" + ovn_env.parent.mkdir(parents=True) + ovn_env.write_text( + f'OVN_NB_CONNECT="{nb_connection}"\n' f'OVN_SB_CONNECT="{sb_connection}"\n', + encoding="utf-8", + ) + ensure_stopped, restart_services = _prepare_configure_reconciliation(mocker, snap) + rendered_config = snap.paths.common / "etc/neutron/neutron_ovn_agent.ini" + rendered_config.write_text(f"{nb_connection}|{sb_connection}", encoding="utf-8") + snap.services.list.return_value = { + "neutron-ovn-agent": service_mock(enabled=False, active=False) + } + snapctl = mocker.patch.object(hooks, "SnapCtl").return_value + + hooks.configure(snap) + + excluded_services = ensure_stopped.call_args.args[1] + assert "neutron-ovn-agent" not in excluded_services + restart_services.assert_not_called() + snapctl.start.assert_called_once_with("neutron-ovn-agent", enable=True) + + +@pytest.mark.parametrize( + "ovn_env_contents", + [ + None, + 'OVN_NB_CONNECT="ssl:192.0.2.10:6641"\n' 'OVN_SB_CONNECT="malformed"\n', + ], + ids=("missing", "invalid"), +) +def test_configure_stops_ovn_agent_without_usable_environment(mocker, snap, ovn_env_contents): + if ovn_env_contents is not None: + ovn_env = snap.paths.data / "microovn" / "ovn-env" / "env" / "ovn.env" + ovn_env.parent.mkdir(parents=True) + ovn_env.write_text(ovn_env_contents, encoding="utf-8") + ensure_stopped, restart_services = _prepare_configure_reconciliation(mocker, snap) + snap.services.list.return_value = { + "neutron-ovn-agent": service_mock(enabled=False, active=False) + } + snapctl = mocker.patch.object(hooks, "SnapCtl").return_value + + hooks.configure(snap) + + excluded_services = ensure_stopped.call_args.args[1] + assert "neutron-ovn-agent" in excluded_services + restart_services.assert_not_called() + snapctl.start.assert_not_called() + + +def test_get_configure_context_reads_ovn_connections_without_logging_them(mocker, snap, caplog): + nb_connection = "ssl:192.0.2.10:6641" + sb_connection = "ssl:192.0.2.10:6642" + ovn_env = snap.paths.data / "microovn" / "ovn-env" / "env" / "ovn.env" + ovn_env.parent.mkdir(parents=True) + ovn_env.write_text( + f'OVN_NB_CONNECT="{nb_connection}"\n' f'OVN_SB_CONNECT="{sb_connection}"\n', + encoding="utf-8", + ) + _prepare_minimal_configure_context(mocker, snap) + + context = hooks._get_configure_context(snap) + hooks._get_exclude_services(context) + + assert context["network"]["ovn_nb_connection"] == nb_connection + assert context["network"]["ovn_sb_connection"] == sb_connection + assert nb_connection not in caplog.text + assert sb_connection not in caplog.text + + +@pytest.mark.parametrize( + "ovn_env_contents", + [ + None, + 'OVN_NB_CONNECT="ssl:192.0.2.10:6641"\nOVN_SB_CONNECT="malformed-secret"\n', + ], +) +def test_get_configure_context_excludes_ovn_agent_without_valid_environment( + mocker, snap, caplog, ovn_env_contents +): + if ovn_env_contents is not None: + ovn_env = snap.paths.data / "microovn" / "ovn-env" / "env" / "ovn.env" + ovn_env.parent.mkdir(parents=True) + ovn_env.write_text(ovn_env_contents, encoding="utf-8") + _prepare_minimal_configure_context(mocker, snap) + + context = hooks._get_configure_context(snap) + + assert "ovn_nb_connection" not in context["network"] + assert "ovn_sb_connection" not in context["network"] + assert "neutron-ovn-agent" in hooks._services_not_ready(context) + assert "ssl:192.0.2.10:6641" not in caplog.text + assert "malformed-secret" not in caplog.text + + @mock.patch("openstack_hypervisor.netplan.get_netplan_config") def test_process_dpdk_netplan_config(mock_get_netplan_config, get_pci_address): mock_get_netplan_config.return_value = yaml.safe_load( @@ -1497,8 +1726,10 @@ def test_microovn_socket_controls_network_configuration( 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, "RestartOnChange", return_value=nullcontext()) + restart_on_change = mock.Mock(restarted_services=set()) + mocker.patch.object(hooks, "RestartOnChange", return_value=nullcontext(restart_on_change)) mocker.patch.object(hooks, "_render_templates") + mocker.patch.object(hooks, "_ensure_services_started") mocker.patch.object(hooks, "_configure_webdav_apache") mocker.patch.object(hooks, "_configure_kvm") mocker.patch.object(hooks, "_configure_monitoring_services") diff --git a/tests/unit/test_ovn_env.py b/tests/unit/test_ovn_env.py new file mode 100644 index 0000000..bef4c3d --- /dev/null +++ b/tests/unit/test_ovn_env.py @@ -0,0 +1,174 @@ +# SPDX-FileCopyrightText: 2026 - Canonical Ltd +# SPDX-License-Identifier: Apache-2.0 + +from pathlib import Path + +import pytest + +from openstack_hypervisor.ovn_env import OVNEnvError, parse_ovn_env + + +def _write_ovn_env(path: Path, contents: str) -> Path: + path.write_text(contents, encoding="utf-8") + return path + + +def _assert_invalid(path: Path, *sensitive_values: str) -> None: + with pytest.raises(OVNEnvError) as exc_info: + parse_ovn_env(path) + + for value in sensitive_values: + assert value not in str(exc_info.value) + + +def test_parse_generated_environment_returns_only_required_connections(tmp_path): + nb_connection = "ssl:10.0.0.10:6641,ssl:[2001:db8::10]:6641" + sb_connection = "ssl:10.0.0.10:6642,ssl:[2001:db8::10]:6642" + path = _write_ovn_env( + tmp_path / "ovn.env", + "\n".join( + [ + "# # Generated by MicroOVN, DO NOT EDIT.", + "", + 'OVN_INITIAL_NB="10.0.0.10"', + 'OVN_INITIAL_SB="10.0.0.10"', + f'OVN_NB_CONNECT="{nb_connection}"', + f"OVN_SB_CONNECT='{sb_connection}'", + 'OVN_LOCAL_IP="10.0.0.20"', + "", + ] + ), + ) + + assert parse_ovn_env(path) == { + "OVN_NB_CONNECT": nb_connection, + "OVN_SB_CONNECT": sb_connection, + } + + +def test_parse_accepts_microovn_tcp_connections(tmp_path): + path = _write_ovn_env( + tmp_path / "ovn.env", + 'OVN_NB_CONNECT="tcp:10.0.0.10:6641"\n' 'OVN_SB_CONNECT="tcp:[2001:db8::10]:6642"\n', + ) + + assert parse_ovn_env(path) == { + "OVN_NB_CONNECT": "tcp:10.0.0.10:6641", + "OVN_SB_CONNECT": "tcp:[2001:db8::10]:6642", + } + + +@pytest.mark.parametrize("missing_key", ["OVN_NB_CONNECT", "OVN_SB_CONNECT"]) +def test_parse_rejects_missing_required_assignment(tmp_path, missing_key): + values = { + "OVN_NB_CONNECT": "ssl:10.0.0.10:6641", + "OVN_SB_CONNECT": "ssl:10.0.0.10:6642", + } + del values[missing_key] + path = _write_ovn_env( + tmp_path / "ovn.env", + "".join(f'{key}="{value}"\n' for key, value in values.items()), + ) + + _assert_invalid(path, *values.values()) + + +@pytest.mark.parametrize("duplicate_key", ["OVN_NB_CONNECT", "OVN_SB_CONNECT"]) +def test_parse_rejects_duplicate_required_assignment(tmp_path, duplicate_key): + nb_connection = "ssl:10.0.0.10:6641" + sb_connection = "ssl:10.0.0.10:6642" + values = { + "OVN_NB_CONNECT": nb_connection, + "OVN_SB_CONNECT": sb_connection, + } + path = _write_ovn_env( + tmp_path / "ovn.env", + "".join( + [ + f'{duplicate_key}="{values[duplicate_key]}"\n', + f'OVN_NB_CONNECT="{nb_connection}"\n', + f'OVN_SB_CONNECT="{sb_connection}"\n', + ] + ), + ) + + _assert_invalid(path, nb_connection, sb_connection) + + +@pytest.mark.parametrize( + "malformed_line", + [ + "OVN_NB_CONNECT=ssl:10.0.0.10:6641", + 'export OVN_NB_CONNECT="ssl:10.0.0.10:6641"', + 'OVN_NB_CONNECT="ssl:10.0.0.10:6641" trailing', + "not an assignment", + ], +) +def test_parse_rejects_malformed_assignment_syntax(tmp_path, malformed_line): + sb_connection = "ssl:10.0.0.10:6642" + path = _write_ovn_env( + tmp_path / "ovn.env", + f'{malformed_line}\nOVN_SB_CONNECT="{sb_connection}"\n', + ) + + _assert_invalid(path, sb_connection) + + +@pytest.mark.parametrize("empty_value", ['""', "''"]) +def test_parse_rejects_empty_required_value(tmp_path, empty_value): + sb_connection = "ssl:10.0.0.10:6642" + path = _write_ovn_env( + tmp_path / "ovn.env", + f'OVN_NB_CONNECT={empty_value}\nOVN_SB_CONNECT="{sb_connection}"\n', + ) + + _assert_invalid(path, sb_connection) + + +@pytest.mark.parametrize( + "unsupported_connection", + [ + "unix:/run/ovn/ovnnb_db.sock", + "ssh:10.0.0.10:6641", + "ssl:ovn-central.internal:6641", + "ssl:2001:db8::10:6641", + "ssl:10.0.0.10:0", + "ssl:10.0.0.10:65536", + "ssl:10.0.0.10", + "ssl:10.0.0.10:6641,", + ], +) +def test_parse_rejects_unsupported_endpoint(tmp_path, unsupported_connection): + sb_connection = "ssl:10.0.0.10:6642" + path = _write_ovn_env( + tmp_path / "ovn.env", + f'OVN_NB_CONNECT="{unsupported_connection}"\n' f'OVN_SB_CONNECT="{sb_connection}"\n', + ) + + _assert_invalid(path, unsupported_connection, sb_connection) + + +def test_parse_does_not_execute_assignment_values(tmp_path): + marker = tmp_path / "executed" + malicious_value = f"$(touch {marker})" + path = _write_ovn_env( + tmp_path / "ovn.env", + f'OVN_NB_CONNECT="{malicious_value}"\n' 'OVN_SB_CONNECT="ssl:10.0.0.10:6642"\n', + ) + + _assert_invalid(path, malicious_value) + assert not marker.exists() + + +def test_parse_rejects_missing_file(tmp_path): + _assert_invalid(tmp_path / "missing-ovn.env") + + +def test_parse_rejects_unreadable_file(tmp_path): + path = _write_ovn_env( + tmp_path / "ovn.env", + 'OVN_NB_CONNECT="ssl:10.0.0.10:6641"\n' 'OVN_SB_CONNECT="ssl:10.0.0.10:6642"\n', + ) + path.chmod(0) + + _assert_invalid(path) diff --git a/tests/unit/test_services.py b/tests/unit/test_services.py index 512215e..2cb43a1 100644 --- a/tests/unit/test_services.py +++ b/tests/unit/test_services.py @@ -11,7 +11,7 @@ import openstack_hypervisor.services as services_module from openstack_hypervisor.services import ( FileTransferService, - NeutronOVNMetadataAgentService, + NeutronOVNAgentService, NovaAPIMetadataService, NovaComputeService, ) @@ -209,13 +209,25 @@ def test_returns_1_without_metadata_proxy_url( mock_run.assert_not_called() -class TestNeutronOVNMetadataAgentService: - """Tests for NeutronOVNMetadataAgentService.""" +class TestNeutronOVNAgentService: + """Tests for NeutronOVNAgentService.""" - def _write_metadata_config(self, snap, ovsdb_connection): - config = snap.paths.common / "etc" / "neutron" / "neutron_ovn_metadata_agent.ini" + def _write_agent_config( + self, + snap, + ovsdb_connection, + ovn_nb_connection="ssl:10.0.0.10:6641", + ovn_sb_connection="ssl:10.0.0.10:6642", + ): + config = snap.paths.common / "etc" / "neutron" / "neutron_ovn_agent.ini" config.parent.mkdir(parents=True, exist_ok=True) - config.write_text(f"[ovs]\novsdb_connection = {ovsdb_connection}\n") + config.write_text( + "[ovs]\n" + f"ovsdb_connection = {ovsdb_connection}\n" + "[ovn]\n" + f"ovn_nb_connection = {ovn_nb_connection}\n" + f"ovn_sb_connection = {ovn_sb_connection}\n" + ) return config @patch("openstack_hypervisor.services.subprocess.run") @@ -231,13 +243,13 @@ def test_waits_for_ovsdb_schema_before_starting_agent( monkeypatch.setattr(services_module, "OVSDB_SCHEMA_CHECK_INTERVAL", 0, raising=False) ovs_socket = tmp_path / "db.sock" ovs_socket.touch() - self._write_metadata_config(snap, f"unix:{ovs_socket}") + self._write_agent_config(snap, f"unix:{ovs_socket}") mock_run.side_effect = [ MagicMock(returncode=0), MagicMock(returncode=0), ] - result = NeutronOVNMetadataAgentService().run(snap) + result = NeutronOVNAgentService().run(snap) assert result == 0 assert mock_run.call_count == 2 @@ -253,24 +265,40 @@ def test_waits_for_ovsdb_schema_before_starting_agent( ) mock_run.assert_any_call( [ - str(snap.paths.snap / "usr" / "bin" / "neutron-ovn-metadata-agent"), + str(snap.paths.snap / "usr" / "bin" / "neutron-ovn-agent"), "--config-file", str(snap.paths.common / "etc" / "neutron" / "neutron.conf"), "--config-file", - str(snap.paths.common / "etc" / "neutron" / "neutron_ovn_metadata_agent.ini"), + str(snap.paths.common / "etc" / "neutron" / "neutron_ovn_agent.ini"), "--config-dir", str(snap.paths.common / "etc" / "neutron" / "neutron.conf.d"), ] ) @patch("openstack_hypervisor.services.subprocess.run") - def test_returns_1_when_ovsdb_connection_missing(self, mock_run, snap): - """Service should fail fast when ovsdb_connection is not configured.""" - config = snap.paths.common / "etc" / "neutron" / "neutron_ovn_metadata_agent.ini" + @pytest.mark.parametrize( + "missing_option", + ["ovsdb_connection", "ovn_nb_connection", "ovn_sb_connection"], + ) + def test_returns_1_when_required_connection_missing(self, mock_run, snap, missing_option): + """Service should fail fast when any required connection is absent.""" + config = snap.paths.common / "etc" / "neutron" / "neutron_ovn_agent.ini" config.parent.mkdir(parents=True, exist_ok=True) - config.write_text("[ovs]\n") + options = { + "ovsdb_connection": "unix:/run/openvswitch/db.sock", + "ovn_nb_connection": "ssl:10.0.0.10:6641", + "ovn_sb_connection": "ssl:10.0.0.10:6642", + } + del options[missing_option] + config.write_text( + "[ovs]\n" + f"ovsdb_connection = {options.get('ovsdb_connection', '')}\n" + "[ovn]\n" + f"ovn_nb_connection = {options.get('ovn_nb_connection', '')}\n" + f"ovn_sb_connection = {options.get('ovn_sb_connection', '')}\n" + ) - result = NeutronOVNMetadataAgentService().run(snap) + result = NeutronOVNAgentService().run(snap) assert result == 1 mock_run.assert_not_called() @@ -287,9 +315,9 @@ def test_returns_1_when_unix_socket_missing( monkeypatch.setattr(services_module, "OVSDB_SCHEMA_TIMEOUT", 0, raising=False) monkeypatch.setattr(services_module, "OVSDB_SCHEMA_CHECK_INTERVAL", 0, raising=False) ovs_socket = tmp_path / "db.sock" - self._write_metadata_config(snap, f"unix:{ovs_socket}") + self._write_agent_config(snap, f"unix:{ovs_socket}") - result = NeutronOVNMetadataAgentService().run(snap) + result = NeutronOVNAgentService().run(snap) assert result == 1 mock_run.assert_not_called() @@ -307,10 +335,10 @@ def test_returns_1_when_schema_probe_times_out( monkeypatch.setattr(services_module, "OVSDB_SCHEMA_CHECK_INTERVAL", 0, raising=False) ovs_socket = tmp_path / "db.sock" ovs_socket.touch() - self._write_metadata_config(snap, f"unix:{ovs_socket}") + self._write_agent_config(snap, f"unix:{ovs_socket}") mock_run.return_value = MagicMock(returncode=1) - result = NeutronOVNMetadataAgentService().run(snap) + result = NeutronOVNAgentService().run(snap) assert result == 1 mock_run.assert_called_once_with( diff --git a/tests/unit/test_templates.py b/tests/unit/test_templates.py index 1864d54..f79bf78 100644 --- a/tests/unit/test_templates.py +++ b/tests/unit/test_templates.py @@ -16,6 +16,8 @@ "node": {"fqdn": "compute-0.internal", "ip_address": "10.0.0.10"}, "network": { "ovs_socket_path": "unix:/var/run/openvswitch/db.sock", + "ovn_nb_connection": "ssl:10.0.0.10:6641", + "ovn_sb_connection": "ssl:10.0.0.10:6642", "dns_servers": "", "nova_metadata_proxy_url": "http://internal/nova-metadata", "nova_metadata_proxy_scheme": "http", @@ -107,12 +109,27 @@ def test_neutron_clients_use_internal_interface(): ) -def test_neutron_ovn_metadata_agent_uses_nova_metadata_endpoint(): - output = _render("neutron_ovn_metadata_agent.ini.j2") +def test_neutron_ovn_agent_enables_metadata_with_all_ovsdb_connections(): + output = _render("neutron_ovn_agent.ini.j2") assert "nova_metadata_host = 127.0.0.1" in output assert "nova_metadata_port = 8775" in output assert "metadata_proxy_shared_secret = secret" in output + assert "extensions = metadata" in output + assert "ovsdb_connection = unix:/var/run/openvswitch/db.sock" in output + assert "ovn_nb_connection = ssl:10.0.0.10:6641" in output + assert "ovn_sb_connection = ssl:10.0.0.10:6642" in output + for option, relative_path in { + "ovn_nb_private_key": "private/ovn-key.pem", + "ovn_nb_certificate": "certs/ovn-cert.pem", + "ovn_nb_ca_cert": "certs/ovn-cacert.pem", + "ovn_sb_private_key": "private/ovn-key.pem", + "ovn_sb_certificate": "certs/ovn-cert.pem", + "ovn_sb_ca_cert": "certs/ovn-cacert.pem", + }.items(): + assert ( + f"{option} = /var/snap/openstack-hypervisor/common/etc/ssl/{relative_path}" in output + ) def test_metadata_templates_render_before_credentials_are_configured(): @@ -122,7 +139,7 @@ def test_metadata_templates_render_before_credentials_are_configured(): assert "service_metadata_proxy = True" not in output assert "metadata_proxy_shared_secret" not in output - output = _render("neutron_ovn_metadata_agent.ini.j2", credentials={}) + output = _render("neutron_ovn_agent.ini.j2", credentials={}) assert "nova_metadata_host = 127.0.0.1" in output assert "nova_metadata_port = 8775" in output