From 9c8535a2b17c36fc5a3d6b4608b2c0f09fb90c02 Mon Sep 17 00:00:00 2001 From: haseeb Date: Mon, 21 Sep 2026 14:54:23 +0530 Subject: [PATCH] report each Nautobot prefix on NeutronSubnetPool status --- ...tack.rackspace.net_neutronsubnetpools.yaml | 17 ++ .../hooks/framework/__init__.py | 11 +- .../openstack_sync/hooks/framework/common.py | 50 +++++- .../hooks/framework/contracts.py | 29 ++- .../openstack_sync/hooks/framework/runner.py | 48 ++++- .../openstack_sync/hooks/framework/status.py | 13 +- .../openstack_sync/hooks/ironic_runbooks.py | 6 +- .../openstack_sync/hooks/router_flavors.py | 6 +- .../openstack_sync/hooks/subnet_pools.py | 3 +- .../openstack_sync/plugins/common.py | 10 +- .../plugins/neutron/subnet_pools/nautobot.py | 45 ++++- .../plugins/neutron/subnet_pools/reconcile.py | 11 +- python/openstack-sync/tests/test_framework.py | 69 +++++++- .../openstack-sync/tests/test_hook_common.py | 167 +++++++++++++++++- .../tests/test_ironic_runbooks_hook.py | 4 +- .../tests/test_plugins_common.py | 15 ++ .../tests/test_router_flavors_hook.py | 4 +- .../tests/test_subnet_pools_hook.py | 51 +++++- .../tests/test_subnet_pools_nautobot.py | 19 +- .../tests/test_subnet_pools_reconcile.py | 48 ++++- 20 files changed, 572 insertions(+), 54 deletions(-) diff --git a/components/openstack-sync-operator/crds/neutron.understack.rackspace.net_neutronsubnetpools.yaml b/components/openstack-sync-operator/crds/neutron.understack.rackspace.net_neutronsubnetpools.yaml index 6bf6fd078..3c1a5c407 100644 --- a/components/openstack-sync-operator/crds/neutron.understack.rackspace.net_neutronsubnetpools.yaml +++ b/components/openstack-sync-operator/crds/neutron.understack.rackspace.net_neutronsubnetpools.yaml @@ -350,6 +350,23 @@ spec: description: Message provides details about the last sync attempt. type: string maxLength: 2048 + prefixes: + description: >- + The Nautobot prefixes resolved for this pool, one entry per + spec.nautobot.prefix_refs item. + type: array + items: + type: object + properties: + id: + description: Nautobot prefix UUID (spec.nautobot.prefix_refs[].id). + type: string + cidr: + description: CIDR Nautobot resolved for this prefix. + type: string + url: + description: Nautobot UI URL for this prefix record. + type: string conditions: description: Conditions describe current observed state. type: array diff --git a/python/openstack-sync/openstack_sync/hooks/framework/__init__.py b/python/openstack-sync/openstack_sync/hooks/framework/__init__.py index 3493047ae..aab8eed4e 100644 --- a/python/openstack-sync/openstack_sync/hooks/framework/__init__.py +++ b/python/openstack-sync/openstack_sync/hooks/framework/__init__.py @@ -30,6 +30,7 @@ from openstack_sync.hooks.framework.contracts import CredentialKey from openstack_sync.hooks.framework.contracts import HookConfig from openstack_sync.hooks.framework.contracts import PruneRequest +from openstack_sync.hooks.framework.contracts import ReconcileResult from openstack_sync.hooks.framework.contracts import SyncPlan from openstack_sync.hooks.framework.contracts import SyncPlugin from openstack_sync.hooks.framework.contracts import SyncResource @@ -51,6 +52,7 @@ "FINALIZER", "HookConfig", "PruneRequest", + "ReconcileResult", "SyncPlan", "SyncPlugin", "SyncResource", @@ -75,7 +77,12 @@ def _patch_status( - plugin: SyncPlugin, resource: SyncResource, sync_status: str, message: str + plugin: SyncPlugin, + resource: SyncResource, + sync_status: str, + message: str, + extra_status: dict[str, Any] | None = None, + reason: str | None = None, ) -> None: patch_status( plugin.config, @@ -83,6 +90,8 @@ def _patch_status( sync_status, message, patch_resource_status=patch_resource_status, + extra_status=extra_status, + reason=reason, ) diff --git a/python/openstack-sync/openstack_sync/hooks/framework/common.py b/python/openstack-sync/openstack_sync/hooks/framework/common.py index 801c43ff8..8824f6370 100644 --- a/python/openstack-sync/openstack_sync/hooks/framework/common.py +++ b/python/openstack-sync/openstack_sync/hooks/framework/common.py @@ -129,19 +129,25 @@ def truncate_message(message: Any, max_length: int = 2048) -> str: def _desired_condition( - sync_status: str, message: str, generation: int | None = None + sync_status: str, + message: str, + generation: int | None = None, + reason: str | None = None, ) -> dict[str, Any]: """Return the Ready condition to write, without its timestamp. A CR has one notion of success, so it reports one condition. ``Ready`` is the name Kubernetes tooling expects: ``kubectl wait --for=condition=Ready`` and kubernetes-entrypoint's ``custom_resources`` dependency both work off it. + + ``reason`` overrides "ReconcileError" on failure; a synced CR always + reports "Reconciled". """ synced = sync_status == "Synced" condition: dict[str, Any] = { "type": "Ready", "status": "True" if synced else "False", - "reason": "Reconciled" if synced else "ReconcileError", + "reason": "Reconciled" if synced else (reason or "ReconcileError"), "message": truncate_message(message), } if generation is not None: @@ -183,11 +189,23 @@ def _status_is_current( sync_status: str, message: str, generation: int | None, + extra_status: dict[str, Any] | None = None, + reason: str | None = None, ) -> bool: """Return True when the existing CR status already matches desired state. Timestamp fields are intentionally ignored. Rewriting them on every no-op reconcile creates a Kubernetes Modified event and can requeue the hook. + + ``extra_status`` is opaque plugin-supplied status (see + :class:`~openstack_sync.hooks.framework.contracts.ReconcileResult`): every + key it carries must already be present on *current* with an equal value, + or the status is not current. A key missing from *current* -- for example + right after a CRD adds a new status field -- counts as a mismatch, so the + one-time backfill patch always goes out rather than being silently skipped + forever. + + ``reason`` must match too, so a changed reason is not skipped as a no-op. """ if not current: return False @@ -203,8 +221,15 @@ def _status_is_current( existing = _ready_condition(current) if existing is None: return False - desired = _desired_condition(sync_status, truncated_message, generation) - return all(existing.get(key) == value for key, value in desired.items()) + desired = _desired_condition(sync_status, truncated_message, generation, reason) + if not all(existing.get(key) == value for key, value in desired.items()): + return False + + if extra_status: + if any(current.get(key) != value for key, value in extra_status.items()): + return False + + return True #: Memoised CustomObjectsApi, so one config load serves every patch in a run. @@ -528,6 +553,8 @@ def patch_resource_status( crd_kind: str, status_enabled: bool, current_status: dict[str, Any] | None = None, + extra_status: dict[str, Any] | None = None, + reason: str | None = None, ) -> None: """Patch the status subresource of a CR. @@ -550,11 +577,20 @@ def patch_resource_status( status_enabled: When False the function returns immediately. current_status: Current CR status from the binding context. When it already matches the desired stable fields, the patch is skipped. + extra_status: Additional top-level status fields to merge in verbatim, + beyond syncStatus/lastSyncTime/message/conditions/ + observedGeneration. Opaque to this function: only the caller's CRD + schema gives these fields meaning. ``None`` or empty writes + nothing extra. + reason: Condition reason to report instead of "ReconcileError" on + failure. Ignored when ``sync_status`` is "Synced". """ if not status_enabled: return - if _status_is_current(current_status, sync_status, message, generation): + if _status_is_current( + current_status, sync_status, message, generation, extra_status, reason + ): LOG.debug( "skipping %s status patch for %s; status is already current", crd_kind, @@ -577,7 +613,7 @@ def patch_resource_status( return timestamp = utc_timestamp() - condition = _desired_condition(sync_status, message, generation) + condition = _desired_condition(sync_status, message, generation, reason) condition["lastTransitionTime"] = _transition_time( current_status, str(condition["status"]), timestamp ) @@ -589,6 +625,8 @@ def patch_resource_status( } if generation is not None: status["observedGeneration"] = generation + if extra_status: + status.update(extra_status) try: _customobjects_api().patch_namespaced_custom_object_status( diff --git a/python/openstack-sync/openstack_sync/hooks/framework/contracts.py b/python/openstack-sync/openstack_sync/hooks/framework/contracts.py index bbffc5213..a87145972 100644 --- a/python/openstack-sync/openstack_sync/hooks/framework/contracts.py +++ b/python/openstack-sync/openstack_sync/hooks/framework/contracts.py @@ -7,6 +7,7 @@ from abc import ABC from abc import abstractmethod from dataclasses import dataclass +from dataclasses import field from enum import Enum from typing import Any @@ -149,6 +150,21 @@ class PruneRequest: authoritative_empty: bool +@dataclass(frozen=True) +class ReconcileResult: + """Outcome of one CR's reconcile. + + ``notes`` are folded into the status message. ``extra_status`` is merged + into the CR's status subresource verbatim, alongside the framework's own + ``syncStatus``/``message``/``conditions`` fields -- the framework never + inspects or validates its contents, so only the plugin and its CRD's + status schema give it meaning. + """ + + notes: list[str] = field(default_factory=list) + extra_status: dict[str, Any] | None = None + + class CleanupPolicy(Enum): """How a plugin wants the framework to handle prune and finalizers.""" @@ -179,13 +195,16 @@ def wait_for_api(self, conn: Any) -> None: """Block until the OpenStack service this plugin targets is reachable.""" @abstractmethod - def reconcile(self, conn: Any, spec: dict[str, Any], cache: Any) -> list[str]: + def reconcile(self, conn: Any, spec: dict[str, Any], cache: Any) -> ReconcileResult: """Converge one CR spec onto OpenStack. - Returns human-readable notes about state that diverges from the spec but - that the operator cannot correct on its own -- usually empty. Notes do - not make the reconcile a failure; they qualify the success reported on - the CR status. Raise to signal an actual failure. + Returns a :class:`ReconcileResult` carrying human-readable notes about + state that diverges from the spec but that the operator cannot correct + on its own -- usually empty -- plus optional ``extra_status`` fields to + merge into the CR's status subresource, for a plugin whose CRD reports + structured status beyond syncStatus/message/conditions. Notes do not + make the reconcile a failure; they qualify the success reported on the + CR status. Raise to signal an actual failure. """ def new_cache(self) -> Any: diff --git a/python/openstack-sync/openstack_sync/hooks/framework/runner.py b/python/openstack-sync/openstack_sync/hooks/framework/runner.py index aa983dbcb..15498ed25 100644 --- a/python/openstack-sync/openstack_sync/hooks/framework/runner.py +++ b/python/openstack-sync/openstack_sync/hooks/framework/runner.py @@ -5,6 +5,7 @@ import logging from collections.abc import Callable from typing import Any +from typing import Protocol from openstack_sync.hooks.framework.contracts import CleanupPolicy from openstack_sync.hooks.framework.contracts import CredentialKey @@ -15,10 +16,29 @@ from openstack_sync.hooks.framework.resources import _resource_key from openstack_sync.hooks.framework.resources import group_by_credentials from openstack_sync.hooks.framework.status import synced_message +from openstack_sync.plugins.common import ConfigError LOG = logging.getLogger(__name__) -PatchStatus = Callable[[SyncPlugin, SyncResource, str, str], None] + +class PatchStatus(Protocol): + """Patch one CR's status. + + ``extra_status`` and ``reason`` default to None: a failed reconcile has + nothing extra to report, and reports the generic failure reason. + """ + + def __call__( + self, + plugin: SyncPlugin, + resource: SyncResource, + sync_status: str, + message: str, + extra_status: dict[str, Any] | None = None, + reason: str | None = None, + ) -> None: ... + + SyncLiveFinalizers = Callable[ [SyncPlugin, list[SyncResource], CleanupPolicy], set[tuple[str | None, str | None]], @@ -109,23 +129,31 @@ def run_sync( for resource in group: try: - notes = plugin.reconcile(conn, resource.spec, cache) + result = plugin.reconcile(conn, resource.spec, cache) except Exception as exc: # noqa: BLE001 failed += 1 - patch_status(plugin, resource, "Failed", str(exc)) + patch_status( + plugin, resource, "Failed", str(exc), reason=_failure_reason(exc) + ) LOG.error( "Failed to reconcile %s %s: %s", noun, resource.display_name, exc ) continue - if notes: + if result.notes: LOG.warning( "%s %s converged but needs manual action: %s", noun.capitalize(), resource.display_name, - "; ".join(notes), + "; ".join(result.notes), ) - patch_status(plugin, resource, "Synced", synced_message(noun, notes)) + patch_status( + plugin, + resource, + "Synced", + synced_message(noun, result.notes), + extra_status=result.extra_status, + ) if failed or unreadable: # Pruning deletes resources absent from the desired set. A CR that @@ -173,3 +201,11 @@ def _fail_group( ) -> None: for resource in group: patch_status(plugin, resource, "Failed", message) + + +def _failure_reason(exc: Exception) -> str | None: + """Return the reason a ConfigError names, or None for any other error.""" + if not isinstance(exc, ConfigError): + return None + reason = exc.reason + return reason if isinstance(reason, str) and reason else None diff --git a/python/openstack-sync/openstack_sync/hooks/framework/status.py b/python/openstack-sync/openstack_sync/hooks/framework/status.py index ad9c69971..e7bf3ece4 100644 --- a/python/openstack-sync/openstack_sync/hooks/framework/status.py +++ b/python/openstack-sync/openstack_sync/hooks/framework/status.py @@ -18,8 +18,17 @@ def patch_status( message: str, *, patch_resource_status: Callable[..., None], + extra_status: dict[str, object] | None = None, + reason: str | None = None, ) -> None: - """Patch one CR status with the sync outcome.""" + """Patch one CR status with the sync outcome. + + ``extra_status`` and ``reason`` are passed straight through to + *patch_resource_status*; see + :class:`~openstack_sync.hooks.framework.contracts.ReconcileResult` for + what ``extra_status`` is for. A failed reconcile has none to report, so + that call site omits it. + """ if not resource.name: LOG.error( "Unable to patch %s status; Kubernetes metadata.name is missing", @@ -37,6 +46,8 @@ def patch_status( crd_kind=config.crd_kind, status_enabled=config.status_enabled, current_status=resource.current_status, + extra_status=extra_status, + reason=reason, ) diff --git a/python/openstack-sync/openstack_sync/hooks/ironic_runbooks.py b/python/openstack-sync/openstack_sync/hooks/ironic_runbooks.py index 70f5bcdd3..1f9453092 100644 --- a/python/openstack-sync/openstack_sync/hooks/ironic_runbooks.py +++ b/python/openstack-sync/openstack_sync/hooks/ironic_runbooks.py @@ -8,6 +8,7 @@ from openstack_sync.hooks.framework import HookConfig from openstack_sync.hooks.framework import PruneRequest +from openstack_sync.hooks.framework import ReconcileResult from openstack_sync.hooks.framework import SyncPlugin from openstack_sync.hooks.framework import build_crd_hook_config from openstack_sync.hooks.framework import hook_enabled @@ -33,8 +34,9 @@ def wait_for_api(self, conn: Any) -> None: delay=self.config.ready_delay, ) - def reconcile(self, conn: Any, spec: dict[str, Any], cache: Any) -> list[str]: - return reconcile_module.sync_runbook(conn, spec, cache) + def reconcile(self, conn: Any, spec: dict[str, Any], cache: Any) -> ReconcileResult: + notes = reconcile_module.sync_runbook(conn, spec, cache) + return ReconcileResult(notes=notes) def prune_resources(self, conn: Any, request: PruneRequest) -> None: prune_module.prune_removed_runbooks( diff --git a/python/openstack-sync/openstack_sync/hooks/router_flavors.py b/python/openstack-sync/openstack_sync/hooks/router_flavors.py index 39650357c..8de8671f9 100644 --- a/python/openstack-sync/openstack_sync/hooks/router_flavors.py +++ b/python/openstack-sync/openstack_sync/hooks/router_flavors.py @@ -9,6 +9,7 @@ from openstack_sync.hooks.framework import CleanupPolicy from openstack_sync.hooks.framework import HookConfig from openstack_sync.hooks.framework import PruneRequest +from openstack_sync.hooks.framework import ReconcileResult from openstack_sync.hooks.framework import SyncPlugin from openstack_sync.hooks.framework import build_crd_hook_config from openstack_sync.hooks.framework import hook_enabled @@ -42,8 +43,9 @@ def new_cache(self) -> reconcile_module.ProfileCache: def reconcile( self, conn: Any, spec: dict[str, Any], cache: reconcile_module.ProfileCache - ) -> list[str]: - return reconcile_module.sync_flavor(conn, spec, cache) + ) -> ReconcileResult: + notes = reconcile_module.sync_flavor(conn, spec, cache) + return ReconcileResult(notes=notes) def prune_resources(self, conn: Any, request: PruneRequest) -> None: if self.cleanup_policy() is CleanupPolicy.BEST_EFFORT_PRUNE: diff --git a/python/openstack-sync/openstack_sync/hooks/subnet_pools.py b/python/openstack-sync/openstack_sync/hooks/subnet_pools.py index 3c5525b4e..7e437a6cb 100644 --- a/python/openstack-sync/openstack_sync/hooks/subnet_pools.py +++ b/python/openstack-sync/openstack_sync/hooks/subnet_pools.py @@ -9,6 +9,7 @@ from openstack_sync.hooks.framework import CleanupPolicy from openstack_sync.hooks.framework import HookConfig from openstack_sync.hooks.framework import PruneRequest +from openstack_sync.hooks.framework import ReconcileResult from openstack_sync.hooks.framework import SyncPlugin from openstack_sync.hooks.framework import build_crd_hook_config from openstack_sync.hooks.framework import hook_enabled @@ -43,7 +44,7 @@ def wait_for_api(self, conn: Any) -> None: def reconcile( self, conn: Any, spec: dict[str, Any], cache: dict[str, Any] - ) -> list[str]: + ) -> ReconcileResult: namespace = self.config.namespace or pod_namespace() return reconcile_module.sync_subnet_pool(conn, spec, namespace, cache) diff --git a/python/openstack-sync/openstack_sync/plugins/common.py b/python/openstack-sync/openstack_sync/plugins/common.py index 5dee4ae26..d6f8140b4 100644 --- a/python/openstack-sync/openstack_sync/plugins/common.py +++ b/python/openstack-sync/openstack_sync/plugins/common.py @@ -78,7 +78,15 @@ def env_required(name: str) -> str: class ConfigError(Exception): - """Raised when a plugin receives an invalid or incomplete configuration.""" + """Raised when a plugin receives an invalid or incomplete configuration. + + Optional ``reason``: a CamelCase condition reason to report on the CR + status instead of the generic "ReconcileError". Defaults to None. + """ + + def __init__(self, message: str, *, reason: str | None = None) -> None: + super().__init__(message) + self.reason = reason # --------------------------------------------------------------------------- diff --git a/python/openstack-sync/openstack_sync/plugins/neutron/subnet_pools/nautobot.py b/python/openstack-sync/openstack_sync/plugins/neutron/subnet_pools/nautobot.py index f1729b716..492264d90 100644 --- a/python/openstack-sync/openstack_sync/plugins/neutron/subnet_pools/nautobot.py +++ b/python/openstack-sync/openstack_sync/plugins/neutron/subnet_pools/nautobot.py @@ -34,6 +34,9 @@ #: the default require.location when a CR does not set one. SITE_ENV = "UNDERSTACK_SITE" +#: Condition reason reported when a referenced Nautobot prefix no longer exists. +PREFIX_NOT_FOUND_REASON = "NautobotPrefixMissing" + @dataclass(frozen=True) class NautobotPrefix: @@ -60,14 +63,16 @@ def resolve_spec( :exc:`ConfigError` when a referenced prefix is missing, fails the guardrails, or the group mixes IP versions. """ - _required_mapping(spec, "nautobot", "spec") + nautobot_spec = _required_mapping(spec, "nautobot", "spec") name = _required_string(spec, "name") + nautobot_url = _required_string(nautobot_spec, "url") client = _nautobot_client(spec, cache, namespace) prefix_refs = _nautobot_prefix_refs(spec) requirements = _nautobot_requirements(spec) prefixes = load_nautobot_prefixes( client, prefix_refs, + nautobot_url=nautobot_url, require_location=_require_location(requirements), ) _validate_prefixes(prefixes, requirements) @@ -75,6 +80,19 @@ def resolve_spec( resolved = dict(spec) resolved["prefixes"] = [prefix.prefix for prefix in prefixes] resolved["ip_version"] = _subnet_pool_ip_version(name, prefixes) + # Kept alongside the flat CIDR list above (which reconcile.py validates and + # sends to Neutron) so the plugin can report each prefix's Nautobot id and + # UI link on the CR status without reconcile.py needing to know about + # Nautobot at all. + resolved["nautobot_prefix_links"] = [ + { + "id": prefix.id, + "cidr": prefix.prefix, + "url": prefix_url(nautobot_url, prefix.id), + } + for prefix in prefixes + if prefix.id + ] LOG.info( "Resolved subnet pool %s from %s Nautobot prefix(es): %s", @@ -85,6 +103,17 @@ def resolve_spec( return resolved +def prefix_url(nautobot_url: str, prefix_id: str) -> str: + """Return the Nautobot UI URL for one prefix record. + + Mirrors the URL shape used by the ``link.argocd.argoproj.io/external-link`` + annotation hand-authored on today's CRs: ``/ipam/prefixes/ + /``. Centralised here so every consumer (CR status, future tooling) + builds the same link the same way. + """ + return f"{nautobot_url.rstrip('/')}/ipam/prefixes/{prefix_id}/" + + # --------------------------------------------------------------------------- # Nautobot client and prefix loading # --------------------------------------------------------------------------- @@ -123,12 +152,14 @@ def load_nautobot_prefixes( client: Any, prefix_refs: list[dict[str, str]], *, + nautobot_url: str, require_location: str | None = None, ) -> list[NautobotPrefix]: """Load prefixes from Nautobot by id, filtered by *require_location*.""" prefixes: list[NautobotPrefix] = [] for ref in prefix_refs: prefix_id = ref["id"] + link = prefix_url(nautobot_url, prefix_id) query: dict[str, str] = {"id": prefix_id} if require_location: query["location"] = require_location @@ -142,17 +173,21 @@ def load_nautobot_prefixes( "(for example 'iad3-dev')" ) from exc raise ConfigError( - f"Nautobot prefix lookup failed for id {prefix_id}: {exc}" + f"Nautobot prefix lookup failed for id {prefix_id} ({link}): {exc}" ) from exc if record is None: if require_location: raise ConfigError( - f"Nautobot prefix {prefix_id} was not found under " + f"Nautobot prefix {prefix_id} ({link}) was not found under " f"location {require_location!r}: it does not exist or is not " - "associated with that location (spec.nautobot.require.location)" + "associated with that location (spec.nautobot.require.location)", + reason=PREFIX_NOT_FOUND_REASON, ) - raise ConfigError(f"Nautobot prefix {prefix_id} was not found") + raise ConfigError( + f"Nautobot prefix {prefix_id} ({link}) was not found", + reason=PREFIX_NOT_FOUND_REASON, + ) prefixes.append(_prefix_from_record(record)) return prefixes diff --git a/python/openstack-sync/openstack_sync/plugins/neutron/subnet_pools/reconcile.py b/python/openstack-sync/openstack_sync/plugins/neutron/subnet_pools/reconcile.py index 661ef6a2c..a92112125 100644 --- a/python/openstack-sync/openstack_sync/plugins/neutron/subnet_pools/reconcile.py +++ b/python/openstack-sync/openstack_sync/plugins/neutron/subnet_pools/reconcile.py @@ -16,6 +16,7 @@ from openstack import exceptions as openstack_exceptions +from openstack_sync.hooks.framework import ReconcileResult from openstack_sync.plugins.common import ConfigError from openstack_sync.plugins.common import get_value from openstack_sync.plugins.common import resource_id @@ -438,11 +439,15 @@ def resolve_desired_names(specs: list[dict[str, Any]]) -> list[str]: def sync_subnet_pool( conn: Any, spec: dict[str, Any], namespace: str, cache: dict[str, Any] -) -> list[str]: +) -> ReconcileResult: """Converge one NeutronSubnetPool spec. Resolves the CR's Nautobot prefix references first, so *spec* need only carry the CRD fields; ``prefixes`` and ``ip_version`` are derived here. + + Returns a :class:`ReconcileResult` whose ``extra_status`` carries the + resolved prefixes (id, CIDR, and Nautobot URL) under the ``prefixes`` key, + for the CRD's ``status.prefixes``. """ resolved = nautobot_module.resolve_spec(spec, cache, namespace) name = resolved["name"] @@ -490,4 +495,6 @@ def sync_subnet_pool( "Reconciled subnet pool: %s", json.dumps(render_subnet_pool(pool), sort_keys=True), ) - return notes + return ReconcileResult( + notes=notes, extra_status={"prefixes": resolved["nautobot_prefix_links"]} + ) diff --git a/python/openstack-sync/tests/test_framework.py b/python/openstack-sync/tests/test_framework.py index 7cc0f74c4..202176d5f 100644 --- a/python/openstack-sync/tests/test_framework.py +++ b/python/openstack-sync/tests/test_framework.py @@ -20,6 +20,7 @@ from openstack_sync.hooks.framework import CredentialKey from openstack_sync.hooks.framework import HookConfig from openstack_sync.hooks.framework import PruneRequest +from openstack_sync.hooks.framework import ReconcileResult from openstack_sync.hooks.framework import SyncPlan from openstack_sync.hooks.framework import SyncPlugin from openstack_sync.hooks.framework import SyncResource @@ -45,6 +46,7 @@ "FINALIZER", "HookConfig", "PruneRequest", + "ReconcileResult", "SyncPlan", "SyncPlugin", "SyncResource", @@ -126,11 +128,13 @@ def __init__( config: HookConfig, *, fail_for: tuple[str, ...] = (), + fail_with_reason: dict[str, str] | None = None, notes_for: dict[str, list[str]] | None = None, prune_raises: bool = False, ) -> None: super().__init__(config) self.fail_for = set(fail_for) + self.fail_with_reason = fail_with_reason or {} self.notes_for = notes_for or {} self.prune_raises = prune_raises self.reconciled: list[str] = [] @@ -146,12 +150,17 @@ def new_cache(self) -> Any: self.caches.append(cache) return cache - def reconcile(self, conn: Any, spec: dict[str, Any], cache: Any) -> list[str]: + def reconcile(self, conn: Any, spec: dict[str, Any], cache: Any) -> ReconcileResult: name = spec["name"] self.reconciled.append(name) if name in self.fail_for: raise RuntimeError(f"reconcile failed for {name}") - return list(self.notes_for.get(name, [])) + if name in self.fail_with_reason: + raise ConfigError( + f"reconcile failed for {name}", + reason=self.fail_with_reason[name], + ) + return ReconcileResult(notes=list(self.notes_for.get(name, []))) def prune( self, @@ -180,9 +189,9 @@ def __init__(self, config: HookConfig) -> None: def wait_for_api(self, conn: Any) -> None: self.waits += 1 - def reconcile(self, conn: Any, spec: dict[str, Any], cache: Any) -> list[str]: + def reconcile(self, conn: Any, spec: dict[str, Any], cache: Any) -> ReconcileResult: self.reconciled.append(spec["name"]) - return [] + return ReconcileResult() class AlwaysPrunePlugin(StubPlugin): @@ -1461,6 +1470,58 @@ def test_run_sync_continues_after_one_failure(): assert plugin.reconciled == ["a", "b"] +def test_run_sync_reports_a_configerror_reason_on_the_failed_status(): + """A ConfigError naming a reason has it passed to patch_status, not lost.""" + plugin = StubPlugin( + make_hook_config(), fail_with_reason={"a": "NautobotPrefixMissing"} + ) + + _, patch_status, _ = _drive(plugin, _inputs([_resource("a")])) + + kwargs = patch_status.call_args.kwargs + assert kwargs["sync_status"] == "Failed" + assert kwargs["reason"] == "NautobotPrefixMissing" + + +def test_run_sync_reports_no_reason_for_a_plain_failure(): + """A plain failure reports no reason, so the generic default applies.""" + plugin = StubPlugin(make_hook_config(), fail_for=("a",)) + + _, patch_status, _ = _drive(plugin, _inputs([_resource("a")])) + + kwargs = patch_status.call_args.kwargs + assert kwargs["sync_status"] == "Failed" + assert kwargs.get("reason") is None + + +def test_run_sync_ignores_a_reason_attribute_on_a_non_configerror(): + """Only ConfigError's reason is trusted. + + other exceptions with a `reason` attribute + (e.g. urllib3/Kubernetes errors carrying an HTTP reason phrase + like "Bad Request") must not leak it into the condition. + """ + + class _LooksLikeConfigError(RuntimeError): + def __init__(self, message: str) -> None: + super().__init__(message) + self.reason = "Bad Request" + + class ImpostorPlugin(StubPlugin): + def reconcile(self, conn, spec, cache): + if spec["name"] == "a": + raise _LooksLikeConfigError("boom") + return super().reconcile(conn, spec, cache) + + plugin = ImpostorPlugin(make_hook_config()) + + _, patch_status, _ = _drive(plugin, _inputs([_resource("a")])) + + kwargs = patch_status.call_args.kwargs + assert kwargs["sync_status"] == "Failed" + assert kwargs.get("reason") is None + + def test_run_sync_marks_whole_group_failed_when_connection_fails(): plugin = StubPlugin(make_hook_config()) inputs = _inputs([_resource("a"), _resource("b")]) diff --git a/python/openstack-sync/tests/test_hook_common.py b/python/openstack-sync/tests/test_hook_common.py index fce3e079d..eda5dca0a 100644 --- a/python/openstack-sync/tests/test_hook_common.py +++ b/python/openstack-sync/tests/test_hook_common.py @@ -183,11 +183,13 @@ def _matching_status( sync_status: str = "Synced", message: str = "ok", generation: int | None = 1, + reason: str | None = None, ) -> dict: + default_reason = "Reconciled" if sync_status == "Synced" else "ReconcileError" condition = { "type": "Ready", "status": "True" if sync_status == "Synced" else "False", - "reason": "Reconciled" if sync_status == "Synced" else "ReconcileError", + "reason": reason or default_reason, "message": message, "lastTransitionTime": "2026-08-19T06:20:21Z", } @@ -248,6 +250,30 @@ def test_status_is_current_detects_real_status_differences( assert not hc._status_is_current(current, sync_status, message, generation) +def test_status_is_current_matches_when_the_reason_matches(): + current = _matching_status(sync_status="Failed", reason="NautobotPrefixMissing") + + assert hc._status_is_current( + current, "Failed", "ok", 1, None, "NautobotPrefixMissing" + ) + + +def test_status_is_current_detects_a_changed_reason(): + """A new reason must not be skipped as a no-op because the message matches.""" + current = _matching_status(sync_status="Failed", reason="ReconcileError") + + assert not hc._status_is_current( + current, "Failed", "ok", 1, None, "NautobotPrefixMissing" + ) + + +def test_status_is_current_detects_reason_cleared(): + """Clearing a reason is also a real change, not a no-op.""" + current = _matching_status(sync_status="Failed", reason="NautobotPrefixMissing") + + assert not hc._status_is_current(current, "Failed", "ok", 1, None, None) + + def test_status_is_current_rejects_a_stale_condition_generation(): current = _matching_status(generation=2) current["conditions"][0]["observedGeneration"] = 1 @@ -255,6 +281,56 @@ def test_status_is_current_rejects_a_stale_condition_generation(): assert not hc._status_is_current(current, "Synced", "ok", 2) +# --------------------------------------------------------------------------- +# _status_is_current: extra_status (plugin-supplied structured status) +# --------------------------------------------------------------------------- + + +def test_status_is_current_ignores_extra_status_when_not_given(): + """A plugin with no extra_status compares only the framework's own fields.""" + current = _matching_status() + assert hc._status_is_current(current, "Synced", "ok", 1) + assert hc._status_is_current(current, "Synced", "ok", 1, None) + assert hc._status_is_current(current, "Synced", "ok", 1, {}) + + +def test_status_is_current_true_when_extra_status_already_matches(): + current = {**_matching_status(), "prefixes": [{"id": "a", "cidr": "10.0.0.0/8"}]} + + assert hc._status_is_current( + current, "Synced", "ok", 1, {"prefixes": [{"id": "a", "cidr": "10.0.0.0/8"}]} + ) + + +def test_status_is_current_false_when_extra_status_value_differs(): + current = {**_matching_status(), "prefixes": [{"id": "a", "cidr": "10.0.0.0/8"}]} + + assert not hc._status_is_current( + current, "Synced", "ok", 1, {"prefixes": [{"id": "b", "cidr": "10.0.0.0/8"}]} + ) + + +def test_status_is_current_false_when_extra_status_key_is_missing(): + """A CRD/plugin upgrade that adds a new status field must force a repatch. + + A CR whose status predates the field looks like it lacks 'prefixes' + entirely; that must count as a mismatch, not as "nothing to compare." + """ + current = _matching_status() + assert "prefixes" not in current + + assert not hc._status_is_current( + current, "Synced", "ok", 1, {"prefixes": [{"id": "a"}]} + ) + + +def test_status_is_current_true_when_extra_status_value_is_empty_list(): + """An explicit empty list is a real value and must compare, not short-circuit.""" + current = {**_matching_status(), "prefixes": []} + + assert hc._status_is_current(current, "Synced", "ok", 1, {"prefixes": []}) + + # --------------------------------------------------------------------------- # patch_resource_status # --------------------------------------------------------------------------- @@ -664,6 +740,26 @@ def test_patch_resource_status_condition_goes_false_on_failure(): assert condition["reason"] == "ReconcileError" +def test_patch_resource_status_uses_the_given_reason_on_failure(): + """A caller-supplied reason overrides the generic ReconcileError.""" + condition = _written_condition( + sync_status="Failed", message="boom", reason="NautobotPrefixMissing" + ) + + assert condition["status"] == "False" + assert condition["reason"] == "NautobotPrefixMissing" + + +def test_patch_resource_status_ignores_reason_when_synced(): + """A synced CR always reports Reconciled; there is nothing to disambiguate.""" + condition = _written_condition( + sync_status="Synced", message="all good", reason="NautobotPrefixMissing" + ) + + assert condition["status"] == "True" + assert condition["reason"] == "Reconciled" + + def test_patch_resource_status_omits_condition_generation_when_absent(): assert "observedGeneration" not in _written_condition(generation=None) @@ -711,6 +807,75 @@ def test_patch_resource_status_skips_when_current_status_matches(): assert not call.called +# --------------------------------------------------------------------------- +# patch_resource_status: extra_status (plugin-supplied structured status) +# --------------------------------------------------------------------------- + + +def test_patch_resource_status_omits_extra_fields_when_not_given(): + """A plugin with no extra_status patches only the framework's own fields.""" + status = _patch().call_args.kwargs["body"]["status"] + assert set(status.keys()) == { + "syncStatus", + "lastSyncTime", + "message", + "conditions", + "observedGeneration", + } + + +def test_patch_resource_status_merges_extra_status_into_the_body(): + status = _patch( + extra_status={"prefixes": [{"id": "a", "cidr": "10.0.0.0/8"}]} + ).call_args.kwargs["body"]["status"] + + assert status["prefixes"] == [{"id": "a", "cidr": "10.0.0.0/8"}] + # The framework's own fields are still written alongside it, unchanged. + assert status["syncStatus"] == "Synced" + assert [c["type"] for c in status["conditions"]] == ["Ready"] + + +def test_patch_resource_status_skips_when_extra_status_also_matches(): + current = {**_matching_status(), "prefixes": [{"id": "a"}]} + + call = _patch( + generation=1, + message="ok", + current_status=current, + extra_status={"prefixes": [{"id": "a"}]}, + ) + + assert not call.called + + +def test_patch_resource_status_patches_when_extra_status_differs(): + current = {**_matching_status(), "prefixes": [{"id": "a"}]} + + status = _patch( + generation=1, + message="ok", + current_status=current, + extra_status={"prefixes": [{"id": "b"}]}, + ).call_args.kwargs["body"]["status"] + + assert status["prefixes"] == [{"id": "b"}] + + +def test_patch_resource_status_patches_when_extra_status_key_is_new(): + """A CR whose stored status predates the extra field gets backfilled once.""" + current = _matching_status() + assert "prefixes" not in current + + status = _patch( + generation=1, + message="ok", + current_status=current, + extra_status={"prefixes": []}, + ).call_args.kwargs["body"]["status"] + + assert status["prefixes"] == [] + + def test_patch_resource_status_skips_without_a_namespace(caplog): with caplog.at_level(logging.ERROR, logger="openstack_sync.hooks.framework.common"): call = _patch(namespace=None) diff --git a/python/openstack-sync/tests/test_ironic_runbooks_hook.py b/python/openstack-sync/tests/test_ironic_runbooks_hook.py index bfc6d0e48..03750db96 100644 --- a/python/openstack-sync/tests/test_ironic_runbooks_hook.py +++ b/python/openstack-sync/tests/test_ironic_runbooks_hook.py @@ -185,9 +185,9 @@ def test_plugin_reconcile_delegates_to_sync_runbook(): with mock.patch.object( hook.reconcile_module, "sync_runbook", return_value=["a note"] ) as sync_runbook: - notes = plugin.reconcile(conn, spec, cache) + result = plugin.reconcile(conn, spec, cache) - assert notes == ["a note"] + assert result.notes == ["a note"] sync_runbook.assert_called_once_with(conn, spec, cache) diff --git a/python/openstack-sync/tests/test_plugins_common.py b/python/openstack-sync/tests/test_plugins_common.py index 2e1343801..70aceb03a 100644 --- a/python/openstack-sync/tests/test_plugins_common.py +++ b/python/openstack-sync/tests/test_plugins_common.py @@ -34,6 +34,21 @@ def test_env_bool_rejects_boolean_aliases(monkeypatch, value): common.env_bool("OPENSTACK_SYNC_TEST_BOOL", False) +def test_config_error_reason_defaults_to_none(): + """Existing call sites raise ConfigError(message) with no reason kwarg.""" + error = common.ConfigError("something went wrong") + + assert str(error) == "something went wrong" + assert error.reason is None + + +def test_config_error_carries_an_explicit_reason(): + error = common.ConfigError("prefix gone", reason="NautobotPrefixMissing") + + assert str(error) == "prefix gone" + assert error.reason == "NautobotPrefixMissing" + + def test_get_value_reads_openstacksdk_attribute_names(): profile = sdk_service_profile.ServiceProfile( id="profile-id", diff --git a/python/openstack-sync/tests/test_router_flavors_hook.py b/python/openstack-sync/tests/test_router_flavors_hook.py index 190beed4a..4e4c21ac7 100644 --- a/python/openstack-sync/tests/test_router_flavors_hook.py +++ b/python/openstack-sync/tests/test_router_flavors_hook.py @@ -142,9 +142,9 @@ def test_plugin_reconcile_delegates_to_sync_flavor(): with mock.patch.object( hook.reconcile_module, "sync_flavor", return_value=["a note"] ) as sync_flavor: - notes = plugin.reconcile(conn, spec, cache) + result = plugin.reconcile(conn, spec, cache) - assert notes == ["a note"] + assert result.notes == ["a note"] sync_flavor.assert_called_once_with(conn, spec, cache) diff --git a/python/openstack-sync/tests/test_subnet_pools_hook.py b/python/openstack-sync/tests/test_subnet_pools_hook.py index 5eec13160..bbd12587a 100644 --- a/python/openstack-sync/tests/test_subnet_pools_hook.py +++ b/python/openstack-sync/tests/test_subnet_pools_hook.py @@ -22,6 +22,7 @@ from openstack_sync.hooks.framework import CleanupPolicy from openstack_sync.hooks.framework import HookConfig from openstack_sync.hooks.framework import PruneRequest +from openstack_sync.hooks.framework import ReconcileResult from openstack_sync.plugins.neutron.subnet_pools.config import BINDING_NAME from openstack_sync.plugins.neutron.subnet_pools.config import ENV_PREFIX from openstack_sync.plugins.neutron.subnet_pools.config import OWNERSHIP_TAG @@ -164,23 +165,25 @@ def test_plugin_reconcile_delegates_to_sync_subnet_pool(): conn = mock.MagicMock() cache: dict[str, Any] = {} spec = {"name": "pool-a"} + stub_result = ReconcileResult(notes=[], extra_status={"prefixes": []}) with mock.patch.object( - hook.reconcile_module, "sync_subnet_pool", return_value=[] + hook.reconcile_module, "sync_subnet_pool", return_value=stub_result ) as sync_subnet_pool: - notes = plugin.reconcile(conn, spec, cache) + result = plugin.reconcile(conn, spec, cache) - assert notes == [] + assert result is stub_result sync_subnet_pool.assert_called_once_with(conn, spec, "openstack", cache) def test_plugin_reconcile_falls_back_to_pod_namespace_when_unset(): plugin = hook.SubnetPoolPlugin(_config(namespace=None)) conn = mock.MagicMock() + stub_result = ReconcileResult(notes=[], extra_status={"prefixes": []}) with ( mock.patch.object( - hook.reconcile_module, "sync_subnet_pool", return_value=[] + hook.reconcile_module, "sync_subnet_pool", return_value=stub_result ) as sync_subnet_pool, mock.patch.object(hook, "pod_namespace", return_value="fallback-ns"), ): @@ -401,6 +404,46 @@ def test_main_reconciles_an_already_converged_pool(monkeypatch, tmp_path): conn.network.set_tags.assert_not_called() +def test_main_reports_resolved_prefixes_on_the_cr_status(monkeypatch, tmp_path): + """End to end: status.prefixes carries each Nautobot prefix's id/cidr/url. + + Exercises the real path from SubnetPoolPlugin.reconcile through + sync_subnet_pool and nautobot.resolve_spec -- only pynautobot.api and the + leaf patch_resource_status call are mocked -- so this is a regression test + for the framework's extra_status plumbing, not just the plugin in + isolation. + """ + clear_env(monkeypatch) + set_crd_identity(monkeypatch) + monkeypatch.setenv(f"{ENV_PREFIX}_ENABLED", "true") + monkeypatch.setenv(f"{ENV_PREFIX}_STATUS_ENABLED", "true") + monkeypatch.setenv("POD_NAMESPACE", "openstack") + conn = _neutron_conn() + + code, patch_status = _run_main( + monkeypatch, tmp_path, _schedule_context("PUBLIC-IP-POOL"), conn + ) + + assert code == 0 + extra_status = patch_status.call_args.kwargs["extra_status"] + assert extra_status == { + "prefixes": [ + { + "id": "2bc3ecab-b6dc-46cd-9bd4-1c0ea8a07f87", + "cidr": "204.232.163.128/25", + "url": "https://nautobot.example.test/ipam/prefixes/" + "2bc3ecab-b6dc-46cd-9bd4-1c0ea8a07f87/", + }, + { + "id": "53c8ee3b-09b9-41ab-a413-b1a1f5ecec6a", + "cidr": "10.4.88.0/24", + "url": "https://nautobot.example.test/ipam/prefixes/" + "53c8ee3b-09b9-41ab-a413-b1a1f5ecec6a/", + }, + ] + } + + def test_main_creates_a_missing_pool(monkeypatch, tmp_path): clear_env(monkeypatch) set_crd_identity(monkeypatch) diff --git a/python/openstack-sync/tests/test_subnet_pools_nautobot.py b/python/openstack-sync/tests/test_subnet_pools_nautobot.py index 15f17797b..b5caf3e1c 100644 --- a/python/openstack-sync/tests/test_subnet_pools_nautobot.py +++ b/python/openstack-sync/tests/test_subnet_pools_nautobot.py @@ -170,8 +170,14 @@ def test_resolve_spec_rejects_prefix_not_under_required_location(monkeypatch): Nautobot returns nothing for ?id=&location=, which the resolver reports as the prefix not being under the required location. """ - with pytest.raises(ConfigError, match="was not found under location 'iad3-dev'"): + with pytest.raises( + ConfigError, match="was not found under location 'iad3-dev'" + ) as excinfo: _resolve(monkeypatch, _single_ref_spec(), None) + assert f"https://nautobot.example.test/ipam/prefixes/{PUBLIC_ID}/" in str( + excinfo.value + ) + assert excinfo.value.reason == nautobot.PREFIX_NOT_FOUND_REASON def _location_400_request_error() -> nautobot.pynautobot.RequestError: @@ -277,12 +283,18 @@ def test_resolve_spec_reuses_cached_client(monkeypatch): def test_resolve_spec_rejects_missing_prefix(monkeypatch): - with pytest.raises(ConfigError, match="was not found"): + """A deleted/missing prefix's message includes a link and a reason.""" + with pytest.raises(ConfigError, match="was not found") as excinfo: _resolve(monkeypatch, _spec(), None) + assert f"https://nautobot.example.test/ipam/prefixes/{PUBLIC_ID}/" in str( + excinfo.value + ) + assert excinfo.value.reason == nautobot.PREFIX_NOT_FOUND_REASON def test_resolve_spec_rejects_prefix_that_fails_requirements(monkeypatch): - with pytest.raises(ConfigError, match="status is 'Reserved'"): + """A prefix failing a guardrail must not carry the missing-prefix reason.""" + with pytest.raises(ConfigError, match="status is 'Reserved'") as excinfo: _resolve( monkeypatch, _spec(), @@ -293,6 +305,7 @@ def test_resolve_spec_rejects_prefix_that_fails_requirements(monkeypatch): status=SimpleNamespace(name="Reserved"), ), ) + assert excinfo.value.reason is None def test_resolve_spec_rejects_prefix_missing_required_tag(monkeypatch): diff --git a/python/openstack-sync/tests/test_subnet_pools_reconcile.py b/python/openstack-sync/tests/test_subnet_pools_reconcile.py index 44f254592..83d44a8e4 100644 --- a/python/openstack-sync/tests/test_subnet_pools_reconcile.py +++ b/python/openstack-sync/tests/test_subnet_pools_reconcile.py @@ -50,13 +50,26 @@ def _pool( def _spec(**overrides: Any) -> dict[str, Any]: - """A spec already carrying the Nautobot-resolved prefixes and IP version.""" + """A spec already carrying what nautobot.resolve_spec attaches. + + ``prefixes``/``ip_version``/``nautobot_prefix_links`` are exactly the + fields the real resolver adds; ``_stub_nautobot`` below bypasses the + resolver itself but keeps its output contract. + """ spec: dict[str, Any] = { "name": "pool-a", "project_id": "project-a", "address_scope": {"name": "scope-a"}, "prefixes": ["10.0.0.0/8"], "ip_version": 4, + "nautobot_prefix_links": [ + { + "id": "2bc3ecab-b6dc-46cd-9bd4-1c0ea8a07f87", + "cidr": "10.0.0.0/8", + "url": "https://nautobot.example.test/ipam/prefixes/" + "2bc3ecab-b6dc-46cd-9bd4-1c0ea8a07f87/", + } + ], "default_prefix_length": 24, "minimum_prefix_length": 24, "maximum_prefix_length": 28, @@ -157,9 +170,9 @@ def test_sync_adopts_ansible_created_pool_without_shrinking(): network.update_subnet_pool.return_value = updated conn = _conn(network) - notes = reconcile.sync_subnet_pool(conn, _spec(shared=True), "openstack", {}) + result = reconcile.sync_subnet_pool(conn, _spec(shared=True), "openstack", {}) - assert notes == [] + assert result.notes == [] update_kwargs = network.update_subnet_pool.call_args.kwargs assert update_kwargs["address_scope_id"] == "scope-id" assert "prefixes" not in update_kwargs # unchanged, so not pushed @@ -183,14 +196,37 @@ def test_sync_reports_note_and_keeps_prefixes_when_pool_would_shrink(): conn = _conn(network) # Spec now only wants 10.0.0.0/8, dropping 192.0.2.0/24. - notes = reconcile.sync_subnet_pool(conn, _spec(prefixes=["10.0.0.0/8"]), "ns", {}) + result = reconcile.sync_subnet_pool(conn, _spec(prefixes=["10.0.0.0/8"]), "ns", {}) - assert len(notes) == 1 - assert "192.0.2.0/24" in notes[0] + assert len(result.notes) == 1 + assert "192.0.2.0/24" in result.notes[0] if network.update_subnet_pool.called: assert "prefixes" not in network.update_subnet_pool.call_args.kwargs +def test_sync_subnet_pool_reports_resolved_prefix_links(): + """nautobot_prefix_links on the resolved spec is surfaced verbatim.""" + links = [ + { + "id": "2bc3ecab-b6dc-46cd-9bd4-1c0ea8a07f87", + "cidr": "10.0.0.0/8", + "url": "https://nautobot.example.test/ipam/prefixes/" + "2bc3ecab-b6dc-46cd-9bd4-1c0ea8a07f87/", + } + ] + network = mock.MagicMock() + network.address_scopes.return_value = [_scope()] + network.subnet_pools.return_value = [] + network.create_subnet_pool.return_value = _pool(tags=[]) + conn = _conn(network) + + result = reconcile.sync_subnet_pool( + conn, _spec(nautobot_prefix_links=links), "openstack", {} + ) + + assert result.extra_status == {"prefixes": links} + + def test_ensure_address_scope_by_id_uses_exact_lookup(): network = mock.MagicMock() network.get_address_scope.return_value = _scope(scope_id="scope-by-id")