diff --git a/components/openstack-sync-operator/crds/neutron.understack.rackspace.net_neutronrouterflavors.yaml b/components/openstack-sync-operator/crds/neutron.understack.rackspace.net_neutronrouterflavors.yaml index b41887e83..8778b4d5b 100644 --- a/components/openstack-sync-operator/crds/neutron.understack.rackspace.net_neutronrouterflavors.yaml +++ b/components/openstack-sync-operator/crds/neutron.understack.rackspace.net_neutronrouterflavors.yaml @@ -24,9 +24,9 @@ spec: - name: ServiceType type: string jsonPath: .spec.service_type - - name: Driver - type: string - jsonPath: .spec.driver + - name: Enabled + type: boolean + jsonPath: .spec.is_enabled - name: SyncStatus type: string jsonPath: .status.syncStatus @@ -35,7 +35,13 @@ spec: jsonPath: .metadata.creationTimestamp schema: openAPIV3Schema: - description: NeutronRouterFlavor defines one Neutron router flavor and its service profile. + description: >- + NeutronRouterFlavor defines one Neutron router flavor and the + service profiles bound to it. Neutron supports many-to-one from + flavor to service_profile, so ``spec.service_profiles`` is a + list of profile specs. The operator find-or-creates each profile + by ``(driver, meta_info)`` and reconciles the flavor's set of + bound profiles to match. type: object required: - spec @@ -50,14 +56,14 @@ spec: type: object required: - name - - driver + - service_profiles - cloudCredentialsRef properties: cloudCredentialsRef: description: >- cloudCredentialsRef points to a Kubernetes Secret containing an OpenStack clouds.yaml file. The operator reads this secret - directly at reconcile time — no volume mount is required. + directly at reconcile time; no volume mount is required. type: object required: - secretName @@ -85,54 +91,73 @@ spec: maxLength: 255 pattern: ^[A-Za-z0-9._-]+$ service_type: - description: Neutron service type for the flavor. + description: >- + Neutron service type for the flavor. For router flavors this + is always L3_ROUTER_NAT (plugin_constants.L3 in neutron-lib). type: string enum: - L3_ROUTER_NAT minLength: 1 maxLength: 255 default: L3_ROUTER_NAT - service_provider: - description: Optional Neutron service provider name used when generating Neutron configuration. - type: string - minLength: 1 - maxLength: 255 - pattern: ^[A-Za-z0-9._-]+$ + is_enabled: + description: >- + Whether the Neutron router flavor is enabled. The operator + reconciles drift toward this value, so setting it to false + disables an operator-managed flavor without deleting it. + type: boolean + default: true description: description: Description stored on the Neutron router flavor. type: string maxLength: 1024 - driver: - description: Service profile driver class. - type: string - minLength: 1 - maxLength: 1024 - pattern: ^[A-Za-z_][A-Za-z0-9_]*(\.[A-Za-z_][A-Za-z0-9_]*)+$ - profile_description: - description: Description stored on the Neutron service profile. - type: string - maxLength: 1024 - profile_id: - description: Existing Neutron service profile ID to attach instead of creating or discovering one. - type: string - format: uuid - meta_info: - description: Service profile metainfo payload. - type: object - properties: - resource_class: - description: Resource class consumed by a physical router provider. - type: string - minLength: 1 - maxLength: 255 - pattern: ^[A-Za-z0-9._:-]+$ - vni_alloc: - description: VNI allocation mode for VRF router providers. - type: string - enum: - - "off" - - "on" - - auto + service_profiles: + description: >- + Service profiles to associate with this flavor. Neutron + supports multiple profiles per flavor and the operator + reconciles the full set: profiles listed here are + find-or-created and associated, and any operator-managed + profile currently attached to the flavor but absent from + this list is disassociated. Unmanaged profiles attached + out-of-band are left untouched. + type: array + minItems: 1 + items: + type: object + required: + - driver + properties: + driver: + description: Service profile driver class. + type: string + minLength: 1 + maxLength: 1024 + pattern: ^[A-Za-z_][A-Za-z0-9_]*(\.[A-Za-z_][A-Za-z0-9_]*)+$ + description: + description: Description stored on the Neutron service profile. + type: string + maxLength: 1024 + is_enabled: + description: Whether the service profile is enabled in Neutron. + type: boolean + default: true + meta_info: + description: Service profile metadata payload. + type: object + properties: + resource_class: + description: Resource class consumed by a physical router provider. + type: string + minLength: 1 + maxLength: 255 + pattern: ^[A-Za-z0-9._:-]+$ + vni_alloc: + description: VNI allocation mode for VRF router providers. + type: string + enum: + - "off" + - "on" + - auto status: description: NeutronRouterFlavorStatus defines the observed sync state. type: object diff --git a/components/openstack-sync-operator/examples/extra-rbac-rules-values.yaml b/components/openstack-sync-operator/examples/extra-rbac-rules-values.yaml new file mode 100644 index 000000000..4af0387ea --- /dev/null +++ b/components/openstack-sync-operator/examples/extra-rbac-rules-values.yaml @@ -0,0 +1,39 @@ +# Example values override for a future plugin that needs Kubernetes resources +# outside the chart-generated defaults. +# +# Use with: +# helm template openstack-sync-operator ../ -f extra-rbac-rules-values.yaml +# +# Default RBAC +# ------------ +# Without rbac.rules, the chart generates the permissions it can infer: +# +# 1. Secret read access, always: +# apiGroups: [""] +# resources: ["secrets"] +# verbs: ["get"] +# +# 2. For each enabled plugin, CRD read/watch access from pluginData..hook.crd: +# verbs: ["get", "list", "watch"] +# +# 3. For each enabled plugin whose CRD defines a status subresource: +# resources: ["/status"] +# verbs: ["get", "patch", "update"] +# +# For example, enabling plugins.neutronRouterFlavors adds access to: +# - neutronrouterflavors +# - neutronrouterflavors/status +# +# Extra RBAC +# ---------- +# rbac.rules is only for resources the chart cannot infer from plugin CRDs. +# Each item is appended verbatim to the generated Role or ClusterRole. +# +# When rbac.clusterWide is false, these rules go into a namespaced Role. +# When rbac.clusterWide is true, these rules go into a ClusterRole. + +rbac: + rules: + - apiGroups: [""] + resources: ["configmaps"] + verbs: ["get", "list", "watch"] diff --git a/components/openstack-sync-operator/templates/_crd.tpl b/components/openstack-sync-operator/templates/_crd.tpl index 41c313d2f..3524b221f 100644 --- a/components/openstack-sync-operator/templates/_crd.tpl +++ b/components/openstack-sync-operator/templates/_crd.tpl @@ -7,19 +7,19 @@ Read hook CRD metadata used by RBAC and shell-operator environment wiring. {{- $hook := index . 2 -}} {{- $crdPath := get $hook "crd" -}} {{- if not $crdPath -}} -{{- fail (printf "hooks.%s.crd is required for CRD metadata" $hookName) -}} +{{- fail (printf "pluginData.%s.hook.crd is required for CRD metadata" $hookName) -}} {{- end -}} -{{- $crdYaml := required (printf "hooks.%s.crd file %s is empty or missing" $hookName $crdPath) ($root.Files.Get $crdPath) -}} +{{- $crdYaml := required (printf "pluginData.%s.hook.crd file %s is empty or missing" $hookName $crdPath) ($root.Files.Get $crdPath) -}} {{- $crd := fromYaml $crdYaml -}} {{- if ne $crd.kind "CustomResourceDefinition" -}} -{{- fail (printf "hooks.%s.crd must point to a CustomResourceDefinition" $hookName) -}} +{{- fail (printf "pluginData.%s.hook.crd must point to a CustomResourceDefinition" $hookName) -}} {{- end -}} -{{- $group := required (printf "hooks.%s.crd spec.group is required" $hookName) $crd.spec.group -}} -{{- $kind := required (printf "hooks.%s.crd spec.names.kind is required" $hookName) $crd.spec.names.kind -}} -{{- $plural := required (printf "hooks.%s.crd spec.names.plural is required" $hookName) $crd.spec.names.plural -}} +{{- $group := required (printf "pluginData.%s.hook.crd spec.group is required" $hookName) $crd.spec.group -}} +{{- $kind := required (printf "pluginData.%s.hook.crd spec.names.kind is required" $hookName) $crd.spec.names.kind -}} +{{- $plural := required (printf "pluginData.%s.hook.crd spec.names.plural is required" $hookName) $crd.spec.names.plural -}} {{- $storageVersion := "" -}} {{- $hasStatus := false -}} -{{- range $version := required (printf "hooks.%s.crd spec.versions is required" $hookName) $crd.spec.versions }} +{{- range $version := required (printf "pluginData.%s.hook.crd spec.versions is required" $hookName) $crd.spec.versions }} {{- if $version.storage -}} {{- $storageVersion = $version.name -}} {{- end -}} @@ -28,7 +28,7 @@ Read hook CRD metadata used by RBAC and shell-operator environment wiring. {{- end -}} {{- end -}} {{- if not $storageVersion -}} -{{- fail (printf "hooks.%s.crd must define a storage version" $hookName) -}} +{{- fail (printf "pluginData.%s.hook.crd must define a storage version" $hookName) -}} {{- end -}} {{- dict "apiVersion" (printf "%s/%s" $group $storageVersion) diff --git a/components/openstack-sync-operator/templates/_helpers.tpl b/components/openstack-sync-operator/templates/_helpers.tpl index e5c2ad443..f3ffe0933 100644 --- a/components/openstack-sync-operator/templates/_helpers.tpl +++ b/components/openstack-sync-operator/templates/_helpers.tpl @@ -69,7 +69,7 @@ required because shell-operator reads hook watches only when the pod starts. {{- end }} {{/* -Normalize built-in plugin hooks and direct hook definitions. +Normalize built-in plugin hooks. */}} {{- define "openstack-sync-operator.configuredHooks" -}} {{- $hooks := dict -}} @@ -91,8 +91,5 @@ Normalize built-in plugin hooks and direct hook definitions. {{- $_2 := set $hooks $pluginName $hookValues -}} {{- end -}} {{- end -}} -{{- range $hookName, $hook := default dict .Values.hooks -}} -{{- $_ := set $hooks $hookName $hook -}} -{{- end -}} {{- $hooks | toYaml -}} {{- end }} diff --git a/components/openstack-sync-operator/templates/deployment.yaml.tpl b/components/openstack-sync-operator/templates/deployment.yaml.tpl index e2d017ec8..b61c738dc 100644 --- a/components/openstack-sync-operator/templates/deployment.yaml.tpl +++ b/components/openstack-sync-operator/templates/deployment.yaml.tpl @@ -27,6 +27,13 @@ {{- end }} {{- end }} {{- end -}} +{{- $operatorEnv := dict "LOG_LEVEL" "info" -}} +{{- range $envName, $envValue := default dict .Values.env }} +{{- if hasKey $hookEnv $envName }} +{{- fail (printf "duplicate operator environment variable %s" $envName) }} +{{- end }} +{{- $_ = set $operatorEnv $envName $envValue -}} +{{- end }} apiVersion: apps/v1 kind: Deployment metadata: @@ -65,7 +72,7 @@ spec: - | missing=0 {{- range $hookName, $hook := $enabledHooks }} - {{- $hookPath := required (printf "hooks.%s.path is required when hook is enabled" $hookName) $hook.path }} + {{- $hookPath := required (printf "pluginData.%s.hook.path is required when hook is enabled" $hookName) $hook.path }} if [ ! -x {{ $hookPath | quote }} ]; then echo {{ printf "enabled hook %s missing or not executable: %s" $hookName $hookPath | quote }} >&2 missing=1 @@ -104,6 +111,10 @@ spec: - name: {{ $envName }} value: {{ get $hookEnv $envName | quote }} {{- end }} + {{- range $envName := keys $operatorEnv | sortAlpha }} + - name: {{ $envName }} + value: {{ get $operatorEnv $envName | quote }} + {{- end }} {{- with .Values.resources }} resources: {{- toYaml . | nindent 12 }} diff --git a/components/openstack-sync-operator/values.schema.json b/components/openstack-sync-operator/values.schema.json index 3b31e71ad..4d8d47a35 100644 --- a/components/openstack-sync-operator/values.schema.json +++ b/components/openstack-sync-operator/values.schema.json @@ -3,12 +3,9 @@ "type": "object", "additionalProperties": true, "properties": { - "hooks": { - "type": "object", - "description": "Additional hook definitions keyed by hook name.", - "additionalProperties": { - "$ref": "#/definitions/hook" - } + "env": { + "description": "Operator-level environment variables injected directly into the container.", + "$ref": "#/definitions/operatorEnv" }, "plugins": { "type": "object", @@ -21,24 +18,44 @@ "type": "object", "description": "Built-in plugin hook data keyed by plugin name.", "additionalProperties": { - "type": "object", - "additionalProperties": true, - "properties": { - "hook": { - "$ref": "#/definitions/hook" - } - } + "$ref": "#/definitions/pluginData" } } }, "definitions": { - "hook": { + "operatorEnv": { + "type": "object", + "propertyNames": { + "pattern": "^[A-Z][A-Z0-9_]*$" + }, + "properties": { + "LOG_LEVEL": { + "type": "string", + "default": "info", + "enum": [ + "debug", + "info", + "error" + ] + } + }, + "additionalProperties": { + "$ref": "#/definitions/envValue" + } + }, + "pluginData": { + "type": "object", + "additionalProperties": false, + "properties": { + "hook": { + "$ref": "#/definitions/pluginHook" + } + } + }, + "pluginHook": { "type": "object", "additionalProperties": false, "properties": { - "enabled": { - "type": "boolean" - }, "path": { "type": "string", "minLength": 1 @@ -53,17 +70,34 @@ "pattern": "^[A-Z][A-Z0-9_]*$" }, "env": { - "type": "object", - "additionalProperties": { - "type": [ - "string", - "number", - "integer", - "boolean" - ] - } + "$ref": "#/definitions/hookEnv" } } + }, + "hookEnv": { + "type": "object", + "propertyNames": { + "pattern": "^[A-Z][A-Z0-9_]*$" + }, + "additionalProperties": { + "$ref": "#/definitions/envValue" + } + }, + "envValue": { + "anyOf": [ + { + "type": "boolean" + }, + { + "type": "number" + }, + { + "type": "string", + "not": { + "pattern": "^\\s*([Tt][Rr][Uu][Ee]|[Ff][Aa][Ll][Ss][Ee]|1|0|[Yy][Ee][Ss]|[Nn][Oo]|[Oo][Nn]|[Oo][Ff][Ff])\\s*$" + } + } + ] } } } diff --git a/components/openstack-sync-operator/values.yaml b/components/openstack-sync-operator/values.yaml index ab89eed9b..b985e59ff 100644 --- a/components/openstack-sync-operator/values.yaml +++ b/components/openstack-sync-operator/values.yaml @@ -1,5 +1,10 @@ replicaCount: 1 +# Operator-level environment variables injected directly into the container. +# LOG_LEVEL controls both shell-operator and Python hook logging. +env: + LOG_LEVEL: info + image: repository: ghcr.io/rackerlabs/understack/openstack-sync-operator pullPolicy: IfNotPresent @@ -13,12 +18,7 @@ serviceAccount: rbac: create: true clusterWide: false - # The base placeholder hook has no Kubernetes bindings. Add hook permissions - # here with the hook that needs them. - rules: [] -# Built-in plugin enablement. Site values normally only override these booleans -# and selected pluginData..hook.env values. # Built-in hook configuration. Site values normally override only: # - plugins.: enable or disable a hook # - pluginData..hook.env: override selected hook env values @@ -48,17 +48,10 @@ pluginData: envPrefix: NEUTRON_ROUTER_FLAVOR env: SYNC_CRONTAB: "0 * * * *" - PRUNE: "false" - DEFAULT_SECRET: infrasetup - DEFAULT_CLOUD: understack - -hooks: {} - -podAnnotations: {} -podLabels: {} - -resources: {} - -nodeSelector: {} -tolerations: [] -affinity: {} + # Neutron readiness wait before a router flavor reconcile fails. + # Total wait is READY_RETRIES * READY_DELAY seconds. + READY_RETRIES: 30 + READY_DELAY: 10 + # When true, removing a NeutronRouterFlavor CR also deletes its unused + # operator-managed OpenStack flavor. Enable this before removing the CR. + PRUNE: false diff --git a/components/openstack-sync-plugins/neutron-router-flavors/dynamic-vrf.yaml b/components/openstack-sync-plugins/neutron-router-flavors/dynamic-vrf.yaml index ac5bff691..ff14c3dc1 100644 --- a/components/openstack-sync-plugins/neutron-router-flavors/dynamic-vrf.yaml +++ b/components/openstack-sync-plugins/neutron-router-flavors/dynamic-vrf.yaml @@ -15,7 +15,8 @@ spec: name: dynamic_vrf service_type: L3_ROUTER_NAT description: Dynamic Fabric VRF (auto VNI) - driver: neutron_understack.l3_router.vrf.Vrf - profile_description: Dynamic Fabric VRF (auto VNI) - meta_info: - vni_alloc: auto + service_profiles: + - driver: neutron_understack.l3_router.vrf.Vrf + description: Dynamic Fabric VRF (auto VNI) + meta_info: + vni_alloc: auto diff --git a/components/openstack-sync-plugins/neutron-router-flavors/pa1410.yaml b/components/openstack-sync-plugins/neutron-router-flavors/pa1410.yaml index e71c9391c..62b36ab41 100644 --- a/components/openstack-sync-plugins/neutron-router-flavors/pa1410.yaml +++ b/components/openstack-sync-plugins/neutron-router-flavors/pa1410.yaml @@ -15,7 +15,8 @@ spec: name: pa1410 service_type: L3_ROUTER_NAT description: Physical PA 1410 - driver: neutron_understack.l3_router.palo_alto.PaloAlto - profile_description: Physical PA 1410 - meta_info: - resource_class: pa1410 + service_profiles: + - driver: neutron_understack.l3_router.palo_alto.PaloAlto + description: Physical PA 1410 + meta_info: + resource_class: pa1410 diff --git a/components/openstack-sync-plugins/neutron-router-flavors/static-vrf.yaml b/components/openstack-sync-plugins/neutron-router-flavors/static-vrf.yaml index 16ea464db..af1f5a563 100644 --- a/components/openstack-sync-plugins/neutron-router-flavors/static-vrf.yaml +++ b/components/openstack-sync-plugins/neutron-router-flavors/static-vrf.yaml @@ -15,7 +15,8 @@ spec: name: static_vrf service_type: L3_ROUTER_NAT description: Static Fabric VRF (admin supplied VNI) - driver: neutron_understack.l3_router.vrf.Vrf - profile_description: Static Fabric VRF (admin supplied VNI) - meta_info: - vni_alloc: "on" + service_profiles: + - driver: neutron_understack.l3_router.vrf.Vrf + description: Static Fabric VRF (admin supplied VNI) + meta_info: + vni_alloc: "on" diff --git a/components/openstack-sync-plugins/neutron-router-flavors/svi.yaml b/components/openstack-sync-plugins/neutron-router-flavors/svi.yaml index 6451b4ba2..dff04bcb3 100644 --- a/components/openstack-sync-plugins/neutron-router-flavors/svi.yaml +++ b/components/openstack-sync-plugins/neutron-router-flavors/svi.yaml @@ -15,6 +15,6 @@ spec: name: svi service_type: L3_ROUTER_NAT description: On-Fabric SVI Gateways - driver: neutron_understack.l3_router.svi.Svi - profile_description: On-Fabric SVI Gateways - meta_info: {} + service_profiles: + - driver: neutron_understack.l3_router.svi.Svi + description: On-Fabric SVI Gateways diff --git a/python/openstack-sync/README.md b/python/openstack-sync/README.md index 1c87b91e3..d7e6c528b 100644 --- a/python/openstack-sync/README.md +++ b/python/openstack-sync/README.md @@ -2,5 +2,121 @@ Shell-operator package for OpenStack reconciliation hooks. -The base image ships with a no-op placeholder hook. Resource-specific sync hooks -are added as plugins. +Each hook reconciles one Kubernetes CRD into one kind of OpenStack resource. The +generic machinery lives in `openstack_sync/hooks/framework.py`; a plugin supplies +only the parts that are specific to its resource. + +## Layout + +``` +openstack_sync/ + utils.py Kubernetes Secret access + memoised connections + hooks/ + common.py binding-context I/O, CR status patching + framework.py HookConfig, SyncPlugin, run_sync(), run_hook() + placeholder.py connectivity probe (no CRs) + router_flavors.py NeutronRouterFlavor hook + plugins/ + common.py OpenStack helpers shared by all plugins + neutron/router_flavors/ + config.py plugin constants + markers.py ownership markers + reconcile.py converge one CR + prune.py delete resources whose CR was removed +``` + +## What the framework does for you + +`run_sync` groups CRs by the credentials in `spec.cloudCredentialsRef`, opens one +connection per credential group, waits for the OpenStack service, reconciles each +CR, patches `Synced`/`Failed` onto the CR status, and then prunes. If any +reconcile fails it **skips the prune entirely** — a failed reconcile means the +desired state is unknown, so deleting anything would be unsafe. + +`run_hook` handles the shell-operator calling convention: `--config`, logging, +reading the binding context, and the exit code. + +## Adding a plugin + +1. **Write the CRD** in `components/openstack-sync-operator/crds/`. Include a + `status` subresource and a required `spec.cloudCredentialsRef` with + `secretName` and `cloudName` — the framework relies on both. Put validation + (`required`, `enum`, `minLength`, `default`) in the schema so the API server + rejects bad CRs and the Python side does not have to re-check them. + +2. **Register it** in `components/openstack-sync-operator/values.yaml`: + + ```yaml + plugins: + myResource: false # opt in per site + pluginData: + myResource: + hook: + path: /hooks/my_resource.py + crd: crds/_.yaml + envPrefix: MY_RESOURCE + env: + SYNC_CRONTAB: "0 * * * *" + ``` + + The chart derives `MY_RESOURCE_ENABLED`, `_CRD_API_VERSION`, `_CRD_KIND`, + `_CRD_RESOURCE` and `_STATUS_ENABLED` from the CRD file, and turns each `env` + key into `MY_RESOURCE_`. `HookConfig.from_env` reads exactly that set, so + the chart and Python cannot drift apart. + +3. **Write the plugin package** under `plugins///` with the + same four modules as `router_flavors`: `config.py` (constants), `markers.py` + (how you record that the operator owns a resource), `reconcile.py`, `prune.py`. + +4. **Write the hook** — subclass `SyncPlugin` and wire it up: + + ```python + class MyResourcePlugin(SyncPlugin): + noun = "my resource" + + def wait_for_api(self, conn) -> None: ... + + def reconcile(self, conn, spec, cache) -> list[str]: + return reconcile_module.sync(conn, spec, cache) + + def prune(self, conn, desired_specs, *, authoritative_empty) -> None: + if self.config.prune: + prune_module.prune(conn, desired_specs, + authoritative_empty=authoritative_empty) + + def main() -> int: + def run(contexts): + if not hook_enabled(ENV_PREFIX): + return 0 + config = HookConfig.from_env(ENV_PREFIX, binding_name=BINDING_NAME) + return run_sync(MyResourcePlugin(config), hook_inputs(contexts, config)) + + return run_hook(lambda: build_crd_hook_config(ENV_PREFIX, BINDING_NAME), run) + ``` + + `wait_for_api` and `reconcile` are required; `new_cache` and `prune` have + working defaults. + +## Two rules worth knowing + +**Only touch what you own.** Every plugin records ownership on the resources it +creates, and only ever updates or deletes resources carrying that marker. This is +what makes the operator safe to run against a cloud that also has hand-made +resources. Never adopt an existing resource by stamping the marker onto it — +that enrols somebody else's resource for eventual deletion. + +**Report what you cannot fix.** `reconcile` returns a list of notes. Use it for +state that diverges from the spec but that OpenStack will not let the operator +correct — for example Neutron rejects `update_service_profile` with a 409 while +the profile is bound to any flavor. The resource is still `Synced`, but the notes +appear on the CR status and in the logs so an operator can act. Raise an +exception only for an actual failure. + +## Tests + +```sh +.venv/bin/python -m pytest tests/ -q +``` + +`tests/test_framework.py` exercises the driver with a stub plugin and no +OpenStack at all — read it first to understand the contract a plugin gets. diff --git a/python/openstack-sync/openstack_sync/hooks/common.py b/python/openstack-sync/openstack_sync/hooks/common.py new file mode 100644 index 000000000..3191845f7 --- /dev/null +++ b/python/openstack-sync/openstack_sync/hooks/common.py @@ -0,0 +1,262 @@ +"""Generic shell-operator hook utilities shared across all hooks. + +Provides binding context I/O and status patching via kubectl. +""" + +from __future__ import annotations + +import datetime as dt +import json +import logging +import os +import subprocess +import sys +from typing import Any + +LOG = logging.getLogger(__name__) + + +def configure_logging() -> None: + """Configure runtime hook logging without affecting --config output.""" + logging.basicConfig( + level=os.environ.get("LOG_LEVEL", "info").upper(), + format="%(levelname)s:%(name)s:%(message)s", + stream=sys.stderr, + ) + + +# --------------------------------------------------------------------------- +# Type coercions +# --------------------------------------------------------------------------- + + +def string_or_none(value: Any) -> str | None: + return None if value is None else str(value) + + +def int_or_none(value: Any) -> int | None: + if value is None: + return None + try: + return int(value) + except (TypeError, ValueError): + return None + + +# --------------------------------------------------------------------------- +# Binding context I/O +# --------------------------------------------------------------------------- + + +def read_binding_context() -> list[dict[str, Any]]: + """Read and parse the shell-operator binding context. + + An absent ``BINDING_CONTEXT_PATH`` or an empty file yields no contexts; + shell-operator does invoke hooks with nothing to do. Malformed JSON raises + :exc:`json.JSONDecodeError`, a :exc:`ValueError`. + """ + path = os.environ.get("BINDING_CONTEXT_PATH") + if not path: + return [] + with open(path, encoding="utf-8") as f: + raw = f.read() + if not raw.strip(): + return [] + contexts = json.loads(raw) + if not isinstance(contexts, list): + raise ValueError("Shell-operator binding context must be a list") + return contexts + + +def snapshot_items( + contexts: list[dict[str, Any]], + binding_name: str, +) -> list[Any] | None: + """Return snapshot items for *binding_name* from *contexts*, or None.""" + for context in contexts: + snapshots = context.get("snapshots") + if not isinstance(snapshots, dict): + continue + items = snapshots.get(binding_name) + if items is not None: + if not isinstance(items, list): + raise ValueError(f"Snapshot {binding_name} must be a list") + return items + return None + + +def synchronization_items( + contexts: list[dict[str, Any]], + binding_name: str, +) -> list[Any] | None: + """Return Synchronization objects for *binding_name* from *contexts*, or None.""" + for context in contexts: + if ( + context.get("binding") == binding_name + and context.get("type") == "Synchronization" + ): + items = context.get("objects", []) + if not isinstance(items, list): + raise ValueError( + f"Synchronization {binding_name} objects must be a list" + ) + return items + return None + + +# --------------------------------------------------------------------------- +# Status patching +# --------------------------------------------------------------------------- + + +def utc_timestamp() -> str: + """Return the current UTC time as an ISO-8601 string with Z suffix.""" + timestamp = dt.datetime.now(dt.UTC).replace(microsecond=0) + return timestamp.isoformat().replace("+00:00", "Z") + + +def truncate_message(message: Any, max_length: int = 2048) -> str: + """Truncate *message* to *max_length* characters, appending '...' if cut.""" + text = str(message) + if len(text) <= max_length: + return text + return f"{text[: max_length - 3]}..." + + +def _condition_status(sync_status: str) -> str: + return "True" if sync_status == "Synced" else "False" + + +def _condition_reason(sync_status: str) -> str: + return "ReconcileSucceeded" if sync_status == "Synced" else "ReconcileFailed" + + +def _desired_condition(sync_status: str, message: str) -> dict[str, str]: + return { + "type": "Synced", + "status": _condition_status(sync_status), + "reason": _condition_reason(sync_status), + "message": truncate_message(message), + } + + +def _synced_condition(current: dict[str, Any]) -> dict[str, Any] | None: + conditions = current.get("conditions") + if not isinstance(conditions, list): + return None + for condition in conditions: + if isinstance(condition, dict) and condition.get("type") == "Synced": + return condition + return None + + +def _status_is_current( + current: dict[str, Any] | None, + sync_status: str, + message: str, + generation: int | 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. + """ + if not current: + return False + + truncated_message = truncate_message(message) + if current.get("syncStatus") != sync_status: + return False + if current.get("message") != truncated_message: + return False + if generation is not None and current.get("observedGeneration") != generation: + return False + + current_condition = _synced_condition(current) + if current_condition is None: + return False + for key, value in _desired_condition(sync_status, truncated_message).items(): + if current_condition.get(key) != value: + return False + return True + + +def patch_resource_status( + *, + name: str, + namespace: str | None, + generation: int | None, + sync_status: str, + message: str, + crd_resource: str, + crd_kind: str, + status_enabled: bool, + current_status: dict[str, Any] | None = None, +) -> None: + """Patch the status subresource of a CR via kubectl. + + Args: + name: CR metadata.name. + namespace: CR metadata.namespace (optional). + generation: CR metadata.generation for observedGeneration (optional). + sync_status: One of ``"Synced"`` or ``"Failed"``. + message: Human-readable detail for the status message. + crd_resource: Fully-qualified CRD resource name for kubectl (e.g. + ``neutronrouterflavors.neutron.understack.rackspace.net``). + crd_kind: CRD kind used in log messages (e.g. ``NeutronRouterFlavor``). + 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. + """ + if not status_enabled: + return + + if _status_is_current(current_status, sync_status, message, generation): + LOG.debug( + "skipping %s status patch for %s; status is already current", + crd_kind, + name, + ) + return + + timestamp = utc_timestamp() + condition = _desired_condition(sync_status, message) + condition["lastTransitionTime"] = timestamp + status: dict[str, Any] = { + "syncStatus": sync_status, + "lastSyncTime": timestamp, + "message": truncate_message(message), + "conditions": [condition], + } + if generation is not None: + status["observedGeneration"] = generation + + command = [ + "kubectl", + "patch", + crd_resource, + name, + "--type", + "merge", + "--subresource", + "status", + "-p", + json.dumps({"status": status}, sort_keys=True), + ] + if namespace: + command.extend(["-n", namespace]) + + try: + result = subprocess.run( # noqa: S603,S607 + command, + capture_output=True, + check=False, + text=True, + ) + except FileNotFoundError: + LOG.warning("kubectl not found; unable to patch %s status", crd_kind) + return + + if result.returncode != 0: + error = (result.stderr or result.stdout or "unknown error").strip() + LOG.warning("failed to patch %s status for %s: %s", crd_kind, name, error) diff --git a/python/openstack-sync/openstack_sync/hooks/framework.py b/python/openstack-sync/openstack_sync/hooks/framework.py new file mode 100644 index 000000000..6f394c82a --- /dev/null +++ b/python/openstack-sync/openstack_sync/hooks/framework.py @@ -0,0 +1,619 @@ +"""Framework for CR-driven OpenStack resource sync plugins. + +A plugin supplies four things: how to wait for its OpenStack service, how to +converge one CR spec, an optional per-credential-group cache, and an optional +prune. This module supplies everything else -- shell-operator hook config, +credential grouping, connection setup, per-resource status patching, the +reconcile-then-prune ordering, and the exit code contract. + +See ``README.md`` for the steps to add a plugin. +""" + +from __future__ import annotations + +import json +import logging +import os +import sys +from abc import ABC +from abc import abstractmethod +from collections.abc import Callable +from dataclasses import dataclass +from typing import Any + +from openstack_sync.hooks.common import configure_logging +from openstack_sync.hooks.common import patch_resource_status +from openstack_sync.hooks.common import read_binding_context +from openstack_sync.hooks.common import snapshot_items +from openstack_sync.hooks.common import synchronization_items +from openstack_sync.plugins.common import ConfigError +from openstack_sync.plugins.common import env_bool +from openstack_sync.plugins.common import env_float +from openstack_sync.plugins.common import env_int +from openstack_sync.plugins.common import env_required +from openstack_sync.utils import get_openstack_connection + +LOG = logging.getLogger(__name__) + +#: A plugin's OpenStack credentials: ``(secret_name, cloud_name)``. +CredentialKey = tuple[str, str] + + +# --------------------------------------------------------------------------- +# Configuration +# --------------------------------------------------------------------------- + + +@dataclass(frozen=True) +class HookConfig: + """Runtime configuration for one hook, built from its chart env prefix. + + The Helm chart injects ``_ENABLED``, ``_CRD_API_VERSION``, + ``_CRD_KIND``, ``_CRD_RESOURCE`` and + ``_STATUS_ENABLED`` for every plugin that declares a CRD, plus one + variable per ``pluginData..hook.env`` key. This dataclass is the + Python half of that contract. + + Nothing here is read at import time. Shell-operator invokes ``--config`` + before the full environment is guaranteed to be present, so ``from_env`` is + called from ``main`` and only once the hook is known to be enabled. + """ + + prefix: str + crd_api_version: str + crd_kind: str + crd_resource: str + binding_name: str + namespace: str | None + status_enabled: bool + prune: bool + sync_crontab: str + ready_retries: int + ready_delay: float + + @classmethod + def from_env(cls, prefix: str, *, binding_name: str) -> HookConfig: + """Build config from the environment the Helm chart injected.""" + return cls( + prefix=prefix, + crd_api_version=env_required(f"{prefix}_CRD_API_VERSION"), + crd_kind=env_required(f"{prefix}_CRD_KIND"), + crd_resource=env_required(f"{prefix}_CRD_RESOURCE"), + binding_name=binding_name, + namespace=os.environ.get("POD_NAMESPACE"), + status_enabled=env_bool(f"{prefix}_STATUS_ENABLED", False), + prune=env_bool(f"{prefix}_PRUNE", False), + sync_crontab=os.environ.get(f"{prefix}_SYNC_CRONTAB", "").strip(), + ready_retries=env_int(f"{prefix}_READY_RETRIES", 30), + ready_delay=env_float(f"{prefix}_READY_DELAY", 10), + ) + + +def hook_enabled(prefix: str) -> bool: + """Return whether the chart enabled the plugin behind *prefix*.""" + return env_bool(f"{prefix}_ENABLED", False) + + +def build_crd_hook_config(prefix: str, binding_name: str) -> dict[str, Any]: + """Return the shell-operator hook config for a CRD-watching plugin. + + When the plugin is disabled the config carries only an ``onStartup`` + binding, because shell-operator requires every hook to declare at least + one binding but the hook must not register Kubernetes watches it will not + service. + """ + hook_config: dict[str, Any] = { + "configVersion": "v1", + "settings": {"executionMinInterval": "30s", "executionBurst": 1}, + } + + if not hook_enabled(prefix): + hook_config["onStartup"] = 10 + return hook_config + + config = HookConfig.from_env(prefix, binding_name=binding_name) + binding: dict[str, Any] = { + "name": config.binding_name, + "apiVersion": config.crd_api_version, + "kind": config.crd_kind, + "executeHookOnEvent": ["Added", "Modified", "Deleted"], + "jqFilter": ".", + "includeSnapshotsFrom": [config.binding_name], + # Dedicated queue so a slow readiness wait or reconcile only delays + # this hook's own tasks, not other hooks sharing the default queue. + "queue": config.binding_name, + } + if config.namespace: + binding["namespace"] = {"nameSelector": {"matchNames": [config.namespace]}} + + hook_config["kubernetes"] = [binding] + if config.sync_crontab: + hook_config["schedule"] = [ + { + "name": "periodic sync", + "crontab": config.sync_crontab, + "includeSnapshotsFrom": [config.binding_name], + "queue": config.binding_name, + } + ] + return hook_config + + +# --------------------------------------------------------------------------- +# Resources +# --------------------------------------------------------------------------- + + +@dataclass(frozen=True) +class SyncResource: + """One CR with its resolved OpenStack credentials. + + ``spec`` is the CR spec with ``cloudCredentialsRef`` removed, so a plugin + sees only its own fields. + """ + + spec: dict[str, Any] + name: str | None + namespace: str | None + generation: int | None + secret_name: str + cloud_name: str + current_status: dict[str, Any] | None = None + + @property + def credentials(self) -> CredentialKey: + return (self.secret_name, self.cloud_name) + + @property + def display_name(self) -> str: + """Return the OpenStack resource name, falling back to the CR name.""" + return str(self.spec.get("name") or self.name or "") + + +@dataclass(frozen=True) +class HookInputs: + """Binding context split by reconciliation purpose. + + The four-way split matters: an event-driven run reconciles only the changed + CRs, but must prune against the *full* desired set from the snapshot, and + must know which credentials a deleted CR used in order to prune at all. + """ + + resources_to_reconcile: list[SyncResource] + desired_resources_for_prune: list[SyncResource] + deleted_resources: list[SyncResource] + prune_credentials: frozenset[CredentialKey] + + +def group_by_credentials( + resources: list[SyncResource], +) -> dict[CredentialKey, list[SyncResource]]: + """Group *resources* by the credentials they authenticate with.""" + grouped: dict[CredentialKey, list[SyncResource]] = {} + for resource in resources: + grouped.setdefault(resource.credentials, []).append(resource) + return grouped + + +def _credentials(resources: list[SyncResource]) -> frozenset[CredentialKey]: + return frozenset(resource.credentials for resource in resources) + + +# --------------------------------------------------------------------------- +# Binding context -> resources +# --------------------------------------------------------------------------- + + +def _resource_from_object(obj: dict[str, Any]) -> SyncResource: + """Build a :class:`SyncResource` from a Kubernetes object. + + The CRD marks ``cloudCredentialsRef.secretName`` and ``.cloudName`` + required with ``minLength: 1``, so the API server rejects a CR missing them + long before a hook sees it. They are read directly rather than re-validated. + """ + spec = dict(obj["spec"]) + creds = spec.pop("cloudCredentialsRef") + metadata = obj.get("metadata", {}) + + return SyncResource( + spec=spec, + name=metadata.get("name"), + namespace=metadata.get("namespace"), + generation=metadata.get("generation"), + secret_name=creds["secretName"], + cloud_name=creds["cloudName"], + current_status=obj.get("status"), + ) + + +def _resources_from_items(items: list[Any]) -> list[SyncResource]: + """Build resources from snapshot or Synchronization items. + + Snapshot items wrap the object as ``{"object": {...}}``; Synchronization + items are the object itself. + """ + resources = [_resource_from_object(item.get("object", item)) for item in items] + return sorted(resources, key=lambda r: str(r.spec.get("name", ""))) + + +def _status_is_current(resource: SyncResource) -> bool: + """Return True when the CR status already records this generation as Synced. + + The hook's own status patch surfaces as a Modified event carrying the same + ``metadata.generation``. Without this check the hook would reconcile itself + in a loop. + """ + status = resource.current_status + return ( + resource.generation is not None + and status is not None + and status.get("syncStatus") == "Synced" + and status.get("observedGeneration") == resource.generation + ) + + +def _split_events( + contexts: list[dict[str, Any]], config: HookConfig +) -> tuple[list[SyncResource], list[SyncResource], frozenset[str]]: + """Split this binding's Event contexts into changed and deleted resources.""" + changed: list[SyncResource] = [] + deleted: list[SyncResource] = [] + watch_events: set[str] = set() + + for context in contexts: + if context.get("binding") != config.binding_name: + continue + if context.get("type") != "Event": + continue + + watch_event = context["watchEvent"] + watch_events.add(watch_event) + + obj = context.get("object") + if not obj: + LOG.warning( + "%s %s event carries no object; ignoring it", + config.crd_kind, + watch_event, + ) + continue + + resource = _resource_from_object(obj) + if watch_event == "Deleted": + deleted.append(resource) + elif watch_event == "Modified" and _status_is_current(resource): + LOG.info( + "Skipping %s Modified event; generation %s is already Synced", + resource.display_name, + resource.generation, + ) + else: + changed.append(resource) + + changed.sort(key=lambda r: str(r.spec.get("name", ""))) + return changed, deleted, frozenset(watch_events) + + +def hook_inputs(contexts: list[dict[str, Any]], config: HookConfig) -> HookInputs: + """Split a shell-operator binding context by reconciliation purpose. + + Event-driven runs reconcile only the changed CRs but prune against the full + desired set from the accompanying snapshot. Schedule and Synchronization + runs reconcile everything they are given. + """ + changed, deleted, watch_events = _split_events(contexts, config) + items = snapshot_items(contexts, config.binding_name) + + if watch_events: + if items is None: + raise ConfigError( + f"Shell-operator {config.binding_name} event context does not " + f"contain {config.binding_name} snapshot objects" + ) + desired = _resources_from_items(items) + # Only prune when something actually changed. A bare Added/Modified for + # an unrelated CR must not trigger a prune sweep. + if changed or deleted or "Deleted" in watch_events: + prune_credentials = _credentials(desired) | _credentials(deleted) + else: + prune_credentials = frozenset() + return HookInputs(changed, desired, deleted, prune_credentials) + + if items is None: + items = synchronization_items(contexts, config.binding_name) + if items is None: + raise ConfigError( + f"Shell-operator binding context does not contain " + f"{config.binding_name} event, snapshot, or synchronization objects" + ) + + resources = _resources_from_items(items) + return HookInputs(resources, resources, [], _credentials(resources)) + + +# --------------------------------------------------------------------------- +# Plugin contract +# --------------------------------------------------------------------------- + + +class SyncPlugin(ABC): + """One CR-driven OpenStack resource sync. + + Subclasses implement ``wait_for_api`` and ``reconcile``; ``new_cache`` and + ``prune`` have usable defaults. ``run_sync`` drives the rest. + """ + + #: Human-readable singular noun used in logs and CR status messages. + noun: str = "resource" + + def __init__(self, config: HookConfig) -> None: + self.config = config + + @abstractmethod + 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]: + """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. + """ + + def new_cache(self) -> Any: + """Return a scratch cache shared by every CR in one credential group.""" + return {} + + def prune( + self, + conn: Any, + desired_specs: list[dict[str, Any]], + *, + authoritative_empty: bool, + ) -> None: + """Delete resources whose CR was removed. + + Optional: the default does nothing, which is correct for a plugin whose + resources outlive their CR or that has nothing safe to delete. + """ + LOG.debug("%s defines no prune step", type(self).__name__) + + +# --------------------------------------------------------------------------- +# Status +# --------------------------------------------------------------------------- + + +def _patch_status( + plugin: SyncPlugin, resource: SyncResource, sync_status: str, message: str +) -> None: + config = plugin.config + if not resource.name: + LOG.warning( + "Unable to patch %s status; Kubernetes metadata.name is missing", + config.crd_kind, + ) + return + patch_resource_status( + name=resource.name, + namespace=resource.namespace or config.namespace, + generation=resource.generation, + sync_status=sync_status, + message=message, + crd_resource=config.crd_resource, + crd_kind=config.crd_kind, + status_enabled=config.status_enabled, + current_status=resource.current_status, + ) + + +def synced_message(noun: str, notes: list[str]) -> str: + """Return the Synced message, qualified by anything needing manual action. + + The resource really is converged, so the status stays Synced. Reporting a + bare success while state diverges from the spec is how a broken resource + stays invisible until it is used. + """ + message = f"Successfully reconciled {noun}" + if not notes: + return message + return f"{message}; needs manual action: {'; '.join(notes)}" + + +# --------------------------------------------------------------------------- +# Driver +# --------------------------------------------------------------------------- + + +def run_sync(plugin: SyncPlugin, inputs: HookInputs) -> int: + """Reconcile every CR, then prune. Returns a process exit code.""" + noun = plugin.noun + resources = inputs.resources_to_reconcile + LOG.info("Found %s %s(s) to reconcile", len(resources), noun) + + grouped = group_by_credentials(resources) + grouped_desired = group_by_credentials(inputs.desired_resources_for_prune) + grouped_deleted = group_by_credentials(inputs.deleted_resources) + connections: dict[CredentialKey, Any] = {} + failed = 0 + + for credentials in sorted(grouped): + secret_name, cloud_name = credentials + group = grouped[credentials] + + try: + conn = get_openstack_connection(secret_name, cloud_name) + except Exception as exc: # noqa: BLE001 + failed += len(group) + _fail_group(plugin, group, f"OpenStack connection failed: {exc}") + LOG.error( + "Failed to connect to OpenStack cloud=%r secret=%r: %s", + cloud_name, + secret_name, + exc, + ) + continue + + connections[credentials] = conn + try: + plugin.wait_for_api(conn) + except Exception as exc: # noqa: BLE001 + failed += len(group) + _fail_group(plugin, group, f"OpenStack API unavailable: {exc}") + LOG.error( + "OpenStack API unavailable for cloud=%r secret=%r: %s", + cloud_name, + secret_name, + exc, + ) + continue + + # Shared across every CR in this credential group so lookups made for + # one CR are reused by the next. + cache = plugin.new_cache() + + for resource in group: + try: + notes = plugin.reconcile(conn, resource.spec, cache) + except Exception as exc: # noqa: BLE001 + failed += 1 + _patch_status(plugin, resource, "Failed", str(exc)) + LOG.error( + "Failed to reconcile %s %s: %s", noun, resource.display_name, exc + ) + continue + + if notes: + LOG.warning( + "%s %s converged but needs manual action: %s", + noun.capitalize(), + resource.display_name, + "; ".join(notes), + ) + _patch_status(plugin, resource, "Synced", synced_message(noun, notes)) + + if failed: + # Pruning deletes resources absent from the desired set. A failed + # reconcile means the desired set could not be established, so deleting + # anything now risks removing a resource that should exist. + LOG.error( + "Skipping %s prune because %s resource(s) failed to reconcile", + noun, + failed, + ) + return 1 + + return _run_prune(plugin, inputs, grouped_desired, grouped_deleted, connections) + + +def _fail_group(plugin: SyncPlugin, group: list[SyncResource], message: str) -> None: + for resource in group: + _patch_status(plugin, resource, "Failed", message) + + +def _run_prune( + plugin: SyncPlugin, + inputs: HookInputs, + grouped_desired: dict[CredentialKey, list[SyncResource]], + grouped_deleted: dict[CredentialKey, list[SyncResource]], + connections: dict[CredentialKey, Any], +) -> int: + noun = plugin.noun + prune_failed = False + + for credentials in sorted(inputs.prune_credentials): + secret_name, cloud_name = credentials + desired = grouped_desired.get(credentials, []) + # An empty desired set is only authoritative when we know a CR was + # deleted; otherwise it may just be a snapshot we could not read, and + # pruning against it would delete everything. + authoritative_empty = credentials in grouped_deleted and not desired + if not desired and not authoritative_empty: + LOG.info( + "Skipping %s prune for cloud=%r secret=%r; no desired resources", + noun, + cloud_name, + secret_name, + ) + continue + + conn = connections.get(credentials) + if conn is None: + if not plugin.config.prune: + continue + try: + conn = get_openstack_connection(secret_name, cloud_name) + plugin.wait_for_api(conn) + except Exception as exc: # noqa: BLE001 + prune_failed = True + LOG.error( + "Cannot reach OpenStack for %s prune cloud=%r secret=%r: %s", + noun, + cloud_name, + secret_name, + exc, + ) + continue + connections[credentials] = conn + + try: + plugin.prune( + conn, + [resource.spec for resource in desired], + authoritative_empty=authoritative_empty, + ) + except Exception as exc: # noqa: BLE001 + prune_failed = True + LOG.error( + "Failed to prune %s cloud=%r secret=%r: %s", + noun, + cloud_name, + secret_name, + exc, + ) + + if prune_failed: + return 1 + + LOG.info("Finished reconciling %s(s)", noun) + return 0 + + +# --------------------------------------------------------------------------- +# Entrypoint +# --------------------------------------------------------------------------- + + +def run_hook( + build_config: Callable[[], dict[str, Any]], + run: Callable[[list[dict[str, Any]]], int], +) -> int: + """Handle the shell-operator calling convention shared by every hook. + + ``--config`` prints the hook config and exits; otherwise the binding + context is read and handed to *run*. An empty or absent binding context is + not an error -- shell-operator invokes hooks with no work to do. + """ + if len(sys.argv) > 1 and sys.argv[1] == "--config": + print(json.dumps(build_config(), indent=2)) + return 0 + + configure_logging() + + try: + contexts = read_binding_context() + except ValueError as exc: + LOG.error("failed to parse binding context: %s", exc) + return 1 + + if not contexts: + return 0 + + try: + return run(contexts) + except Exception as exc: # noqa: BLE001 + LOG.error("%s", exc) + return 1 diff --git a/python/openstack-sync/openstack_sync/hooks/placeholder.py b/python/openstack-sync/openstack_sync/hooks/placeholder.py index 04923e3fb..664226e9a 100644 --- a/python/openstack-sync/openstack_sync/hooks/placeholder.py +++ b/python/openstack-sync/openstack_sync/hooks/placeholder.py @@ -1,112 +1,82 @@ #!/usr/bin/env python3 """Shell-operator hook for OpenStack connectivity verification. -When ``OPENSTACK_PLACEHOLDER_ENABLED`` is ``true`` this hook runs on startup -to verify that the operator can authenticate against OpenStack. When the flag -is ``false`` (the default) the hook registers only an ``onStartup`` binding so -the base image satisfies shell-operator's requirement for at least one binding -without needing any Kubernetes watches or extra RBAC. +When ``OPENSTACK_PLACEHOLDER_ENABLED`` is ``true`` this hook runs on startup to +verify that the operator can authenticate against OpenStack. When it is ``false`` +(the default) the hook still registers an ``onStartup`` binding, because +shell-operator requires every hook to declare at least one binding -- but it +does no work, so the base image needs no Kubernetes watches or extra RBAC. + +This is a connectivity probe rather than a CR reconciler, so it uses only +``run_hook`` and not the :class:`~openstack_sync.hooks.framework.SyncPlugin` +machinery. """ from __future__ import annotations -import json +import logging import os import sys from typing import Any +from openstack_sync.hooks.framework import hook_enabled +from openstack_sync.hooks.framework import run_hook from openstack_sync.utils import get_openstack_connection -TRUTHY_VALUES = {"1", "true", "yes", "on"} +LOG = logging.getLogger(__name__) - -def env_is_truthy(name: str, default: str = "false") -> bool: - return os.environ.get(name, default).lower() in TRUTHY_VALUES +ENV_PREFIX = "OPENSTACK_PLACEHOLDER" def build_hook_config() -> dict[str, Any]: - hook_config: dict[str, Any] = { + return { "configVersion": "v1", - "settings": { - "executionMinInterval": "30s", - "executionBurst": 1, - }, + "settings": {"executionMinInterval": "30s", "executionBurst": 1}, "onStartup": 10, } - return hook_config - - -HOOK_CONFIG = build_hook_config() def check_openstack_connectivity() -> None: - """Attempt to authenticate against OpenStack and log the result. - - Reads credentials from the Kubernetes Secret named by - ``OPENSTACK_PLACEHOLDER_DEFAULT_SECRET`` using - the cloud entry ``OPENSTACK_PLACEHOLDER_DEFAULT_CLOUD``. + """Authenticate against OpenStack and log the result. - Raises: - Exception: Re-raises any connection failure after logging it. + Credentials come from the Secret named by + ``OPENSTACK_PLACEHOLDER_DEFAULT_SECRET`` using the cloud entry + ``OPENSTACK_PLACEHOLDER_DEFAULT_CLOUD``. """ - secret_name = os.environ.get("OPENSTACK_PLACEHOLDER_DEFAULT_SECRET") - cloud_name = os.environ.get("OPENSTACK_PLACEHOLDER_DEFAULT_CLOUD") + secret_name = os.environ.get(f"{ENV_PREFIX}_DEFAULT_SECRET") + cloud_name = os.environ.get(f"{ENV_PREFIX}_DEFAULT_CLOUD") - print( - f"connectivity check: authenticating against cloud={cloud_name!r} " - f"secret={secret_name!r}", - flush=True, + LOG.info( + "connectivity check: authenticating against cloud=%r secret=%r", + cloud_name, + secret_name, ) conn = get_openstack_connection(secret_name, cloud_name) - # Lightweight probe: check_token(str) -> bool confirms the token is valid - # and Keystone is reachable without any side effects. + # check_token(str) -> bool confirms the token is valid and Keystone is + # reachable, with no side effects. conn.identity.check_token(conn.auth_token) - print( - f"connectivity check: OK cloud={cloud_name!r} secret={secret_name!r}", - flush=True, - ) + LOG.info("connectivity check: OK cloud=%r secret=%r", cloud_name, secret_name) def main() -> int: - if len(sys.argv) > 1 and sys.argv[1] == "--config": - print(json.dumps(build_hook_config(), indent=2)) - return 0 - - context_path = os.environ.get("BINDING_CONTEXT_PATH") - if not context_path: - return 0 - with open(context_path) as f: - raw = f.read() - if not raw.strip(): - return 0 - - try: - binding_contexts = json.loads(raw) - except json.JSONDecodeError as exc: - print(f"failed to parse binding context: {exc}", file=sys.stderr) - return 1 - - for context in binding_contexts: - # Shell-operator passes [{"binding": "onStartup"}] for startup runs. - if context.get("binding") == "onStartup": - if not env_is_truthy("OPENSTACK_PLACEHOLDER_ENABLED"): - print( - "connectivity check: skipped" - " (OPENSTACK_PLACEHOLDER_ENABLED is not set)", - flush=True, + def run(contexts: list[dict[str, Any]]) -> int: + for context in contexts: + # Shell-operator passes [{"binding": "onStartup"}] for startup runs. + if context.get("binding") != "onStartup": + continue + if not hook_enabled(ENV_PREFIX): + LOG.info( + "connectivity check: skipped (%s_ENABLED is not set)", ENV_PREFIX ) continue try: check_openstack_connectivity() except Exception as exc: # noqa: BLE001 - print( - f"connectivity check FAILED: {exc}", - file=sys.stderr, - flush=True, - ) + LOG.error("connectivity check FAILED: %s", exc) return 1 + return 0 - return 0 + return run_hook(build_hook_config, run) if __name__ == "__main__": diff --git a/python/openstack-sync/openstack_sync/hooks/router_flavors.py b/python/openstack-sync/openstack_sync/hooks/router_flavors.py index b8ce1c0bc..1de539ec5 100644 --- a/python/openstack-sync/openstack_sync/hooks/router_flavors.py +++ b/python/openstack-sync/openstack_sync/hooks/router_flavors.py @@ -3,143 +3,68 @@ from __future__ import annotations -import json -import os import sys from typing import Any -from openstack_sync.utils import get_openstack_connection -from openstack_sync.utils import pod_namespace # noqa: F401 — re-exported for tests - -TRUTHY_VALUES = {"1", "true", "yes", "on"} - - -def env_is_truthy(name: str, default: str = "false") -> bool: - return os.environ.get(name, default).lower() in TRUTHY_VALUES - - -def router_flavor_namespace() -> str | None: - return ( - os.environ.get("NEUTRON_ROUTER_FLAVOR_NAMESPACE") - or os.environ.get("POD_NAMESPACE") - or None - ) - - -# --------------------------------------------------------------------------- -# Reconciliation -# --------------------------------------------------------------------------- - - -def reconcile_router_flavor(event: dict[str, Any]) -> None: - """Reconcile a single NeutronRouterFlavor resource against OpenStack. - - Reads ``spec.cloudCredentialsRef`` from the event to determine which - Kubernetes Secret and which cloud entry to use. No operator-level - cloud configuration is required — each resource is self-describing. - """ - obj = event["object"] - spec = obj.get("spec", {}) - - creds_ref = spec.get("cloudCredentialsRef", {}) - secret_name = creds_ref.get("secretName") - cloud_name = creds_ref.get("cloudName") - - if not secret_name or not cloud_name: - raise ValueError( - f"NeutronRouterFlavor {obj.get('metadata', {}).get('name')!r} " - "is missing spec.cloudCredentialsRef.secretName or .cloudName" +from openstack_sync.hooks.framework import HookConfig +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 +from openstack_sync.hooks.framework import hook_inputs +from openstack_sync.hooks.framework import run_hook +from openstack_sync.hooks.framework import run_sync +from openstack_sync.plugins.common import wait_for_openstack_network +from openstack_sync.plugins.neutron.router_flavors import prune as prune_module +from openstack_sync.plugins.neutron.router_flavors import reconcile as reconcile_module +from openstack_sync.plugins.neutron.router_flavors.config import BINDING_NAME +from openstack_sync.plugins.neutron.router_flavors.config import ENV_PREFIX + + +class RouterFlavorPlugin(SyncPlugin): + """Sync NeutronRouterFlavor CRs into Neutron flavors and service profiles.""" + + noun = "router flavor" + + def wait_for_api(self, conn: Any) -> None: + wait_for_openstack_network( + conn, + retries=self.config.ready_retries, + delay=self.config.ready_delay, ) - conn = get_openstack_connection(secret_name, cloud_name) # noqa: F841 - - # Full reconciliation logic (create/update/delete router flavor) will be - # wired in here once the connection-per-resource pattern is established. - # The connection object is available as `conn` for subsequent API calls. - - -# --------------------------------------------------------------------------- -# Hook configuration -# --------------------------------------------------------------------------- - - -def build_hook_config() -> dict[str, object]: - hook_config: dict[str, object] = { - "configVersion": "v1", - "settings": { - "executionMinInterval": "30s", - "executionBurst": 1, - }, - } - - if not env_is_truthy("NEUTRON_ROUTER_FLAVOR_ENABLED"): - # Shell-operator requires at least one binding. - hook_config["onStartup"] = 10 - return hook_config - - kubernetes_binding: dict[str, object] = { - "name": "neutron-router-flavors", - "apiVersion": "neutron.understack.rackspace.net/v1alpha1", - "kind": "NeutronRouterFlavor", - "executeHookOnEvent": ["Added", "Modified", "Deleted"], - "jqFilter": ".", - "includeSnapshotsFrom": ["neutron-router-flavors"], - } - namespace = router_flavor_namespace() - if namespace: - kubernetes_binding["namespace"] = { - "nameSelector": { - "matchNames": [namespace], - }, - } - - hook_config["kubernetes"] = [kubernetes_binding] - hook_config["schedule"] = [ - { - "name": "hourly sync", - "crontab": os.environ.get( - "NEUTRON_ROUTER_FLAVOR_SYNC_CRONTAB", "0 * * * *" - ), - "includeSnapshotsFrom": ["neutron-router-flavors"], - } - ] - return hook_config - - -HOOK_CONFIG = build_hook_config() - - -# --------------------------------------------------------------------------- -# Entry point -# --------------------------------------------------------------------------- + def new_cache(self) -> reconcile_module.ProfileCache: + # Keyed by driver and shared across every flavor in one credential + # group, so two flavors wanting the same profile share one lookup and + # end up sharing one profile. + return {} + + def reconcile( + self, conn: Any, spec: dict[str, Any], cache: reconcile_module.ProfileCache + ) -> list[str]: + return reconcile_module.sync_flavor(conn, spec, cache) + + def prune( + self, + conn: Any, + desired_specs: list[dict[str, Any]], + *, + authoritative_empty: bool, + ) -> None: + if not self.config.prune: + return + prune_module.prune_removed_flavors( + conn, desired_specs, authoritative_empty=authoritative_empty + ) def main() -> int: - if len(sys.argv) > 1 and sys.argv[1] == "--config": - print(json.dumps(build_hook_config(), indent=2)) - return 0 - - context_path = os.environ.get("BINDING_CONTEXT_PATH") - if not context_path: - return 0 - with open(context_path) as f: - raw = f.read() - if not raw.strip(): - return 0 - - try: - binding_contexts = json.loads(raw) - except json.JSONDecodeError as exc: - print(f"failed to parse binding context: {exc}", file=sys.stderr) - return 1 - - for context in binding_contexts: - binding = context.get("binding", "") - if binding == "neutron-router-flavors": - for item in context.get("objects", []): - reconcile_router_flavor(item) + def run(contexts: list[dict[str, Any]]) -> int: + if not hook_enabled(ENV_PREFIX): + return 0 + config = HookConfig.from_env(ENV_PREFIX, binding_name=BINDING_NAME) + return run_sync(RouterFlavorPlugin(config), hook_inputs(contexts, config)) - return 0 + return run_hook(lambda: build_crd_hook_config(ENV_PREFIX, BINDING_NAME), run) if __name__ == "__main__": diff --git a/python/openstack-sync/openstack_sync/plugins/__init__.py b/python/openstack-sync/openstack_sync/plugins/__init__.py new file mode 100644 index 000000000..57add78ab --- /dev/null +++ b/python/openstack-sync/openstack_sync/plugins/__init__.py @@ -0,0 +1 @@ +"""OpenStack sync plugin implementations.""" diff --git a/python/openstack-sync/openstack_sync/plugins/common.py b/python/openstack-sync/openstack_sync/plugins/common.py new file mode 100644 index 000000000..887486798 --- /dev/null +++ b/python/openstack-sync/openstack_sync/plugins/common.py @@ -0,0 +1,202 @@ +"""Generic utilities shared across all openstack-sync plugins. + +Provides environment helpers, OpenStack SDK resource accessors, +meta_info normalisation, exception classifiers, and common API helpers +that are reusable by any plugin regardless of which OpenStack service it +targets. +""" + +from __future__ import annotations + +import json +import logging +import os +import time +from typing import Any + +from openstack import exceptions as openstack_exceptions + +LOG = logging.getLogger(__name__) + +# --------------------------------------------------------------------------- +# Environment helpers +# --------------------------------------------------------------------------- + + +def env_bool(name: str, default: bool) -> bool: + """Return a boolean from an environment variable. + + Accepts only the exact values ``true`` and ``false``. Returns *default* + when the variable is unset. + """ + value = os.environ.get(name) + if value is None: + return default + if value == "true": + return True + if value == "false": + return False + raise ConfigError(f"{name} must be true or false") + + +def env_int(name: str, default: int) -> int: + """Return an integer from an environment variable.""" + value = os.environ.get(name) + if value is None: + return default + try: + return int(value) + except ValueError as exc: + raise ConfigError(f"{name} must be an integer") from exc + + +def env_float(name: str, default: float) -> float: + """Return a float from an environment variable.""" + value = os.environ.get(name) + if value is None: + return default + try: + return float(value) + except ValueError as exc: + raise ConfigError(f"{name} must be a number") from exc + + +def env_required(name: str) -> str: + """Return the value of a required environment variable. + + Raises :exc:`ConfigError` when the variable is absent or empty. Use + this for values that must be present at runtime but must not be read at + import time (e.g. CRD identity vars injected by the Helm chart). + """ + value = os.environ.get(name) + if not value: + raise ConfigError( + f"{name} is required but not set; " + "ensure the Helm chart has injected it before the hook runs" + ) + return value + + +# --------------------------------------------------------------------------- +# Error type +# --------------------------------------------------------------------------- + + +class ConfigError(Exception): + """Raised when a plugin receives an invalid or incomplete configuration.""" + + +# --------------------------------------------------------------------------- +# OpenStack SDK resource accessors +# --------------------------------------------------------------------------- + + +def get_value(resource: Any, name: str, default: Any = None) -> Any: + """Return a field from a CR spec dict or an openstacksdk resource. + + Specs are plain dicts read by exact key; OpenStack resources are read by + their openstacksdk attribute name (``meta_info``, ``service_profile_ids``), + which the SDK has already mapped from the Neutron wire name. + """ + value = ( + resource.get(name) + if isinstance(resource, dict) + else getattr(resource, name, None) + ) + return default if value is None else value + + +def resource_id(resource: Any) -> str: + """Return the string ID of an OpenStack resource.""" + return str(get_value(resource, "id")) + + +# --------------------------------------------------------------------------- +# meta_info helpers +# --------------------------------------------------------------------------- + + +def normalize_meta_info(value: Any) -> Any: + """Normalise a meta_info value into a Python dict (or passthrough). + + The operator uses the openstacksdk field name ``meta_info``. Neutron + stores that value as JSON text, so existing service profiles may return a + string while desired specs provide a dict. Non-JSON strings pass through + unchanged so drift reports can show the raw value. + """ + if value is None or value == "": + return {} + + if isinstance(value, str): + text = value.strip() + if not text: + return {} + try: + return json.loads(text) + except json.JSONDecodeError: + return text + + return value + + +def meta_info_payload(value: Any) -> str: + """Return a canonical compact JSON string representation of *value*.""" + normalized = normalize_meta_info(value) + return json.dumps(normalized, sort_keys=True, separators=(",", ":")) + + +# --------------------------------------------------------------------------- +# Neutron network readiness probe +# --------------------------------------------------------------------------- + + +def wait_for_openstack_network( + conn: Any, + retries: int = 30, + delay: float = 10.0, +) -> None: + """Poll until the Neutron network API is reachable. + + Args: + conn: An authenticated OpenStack connection. + retries: Maximum number of attempts before raising. + delay: Seconds to wait between attempts. + + Raises: + RuntimeError: When the API does not become ready within *retries*. + """ + for attempt in range(1, retries + 1): + try: + next(iter(conn.network.flavors()), None) + return + except Exception as exc: + if attempt >= retries: + raise RuntimeError( + f"Neutron API did not become ready after {retries} attempt(s)" + ) from exc + LOG.info("Waiting for Neutron API (%s/%s): %s", attempt, retries, exc) + time.sleep(delay) + + +# --------------------------------------------------------------------------- +# Service profile helpers +# --------------------------------------------------------------------------- + + +def get_service_profile(conn: Any, profile_id: str) -> Any | None: + """Fetch a service profile by ID, returning None if it no longer exists.""" + try: + return conn.network.get_service_profile(profile_id) + except openstack_exceptions.NotFoundException: + return None + + +def service_profile_ids(flavor: Any) -> list[str]: + """Return the service profile IDs attached to *flavor*. + + The openstacksdk ``Flavor.service_profile_ids`` attribute maps Neutron's + ``service_profiles`` wire field. + """ + return [ + str(profile) for profile in get_value(flavor, "service_profile_ids", default=[]) + ] diff --git a/python/openstack-sync/openstack_sync/plugins/neutron/__init__.py b/python/openstack-sync/openstack_sync/plugins/neutron/__init__.py new file mode 100644 index 000000000..0c5b0ff67 --- /dev/null +++ b/python/openstack-sync/openstack_sync/plugins/neutron/__init__.py @@ -0,0 +1 @@ +"""Neutron sync plugin implementations.""" diff --git a/python/openstack-sync/openstack_sync/plugins/neutron/router_flavors/__init__.py b/python/openstack-sync/openstack_sync/plugins/neutron/router_flavors/__init__.py new file mode 100644 index 000000000..cd8db3a64 --- /dev/null +++ b/python/openstack-sync/openstack_sync/plugins/neutron/router_flavors/__init__.py @@ -0,0 +1 @@ +"""Neutron router flavor sync package.""" diff --git a/python/openstack-sync/openstack_sync/plugins/neutron/router_flavors/config.py b/python/openstack-sync/openstack_sync/plugins/neutron/router_flavors/config.py new file mode 100644 index 000000000..0944f9530 --- /dev/null +++ b/python/openstack-sync/openstack_sync/plugins/neutron/router_flavors/config.py @@ -0,0 +1,19 @@ +"""Router-flavor plugin constants. + +Runtime configuration comes from :class:`openstack_sync.hooks.framework.HookConfig`, +built from the ``NEUTRON_ROUTER_FLAVOR`` env prefix the Helm chart injects. The +values here are not configurable at runtime: the chart never set them, so +carrying env plumbing for them only obscured what they are. +""" + +from __future__ import annotations + +#: Env prefix the Helm chart uses for this plugin's variables. +ENV_PREFIX = "NEUTRON_ROUTER_FLAVOR" + +#: shell-operator binding label for the CRD watch. +BINDING_NAME = "neutron-router-flavors" + +#: The only service type Neutron accepts for router flavors +#: (``plugin_constants.L3`` in neutron-lib). The CRD pins it with an enum. +SERVICE_TYPE = "L3_ROUTER_NAT" diff --git a/python/openstack-sync/openstack_sync/plugins/neutron/router_flavors/markers.py b/python/openstack-sync/openstack_sync/plugins/neutron/router_flavors/markers.py new file mode 100644 index 000000000..3f372b2b5 --- /dev/null +++ b/python/openstack-sync/openstack_sync/plugins/neutron/router_flavors/markers.py @@ -0,0 +1,106 @@ +"""Ownership markers for operator-managed router flavors and service profiles. + +The operator only ever creates, updates or deletes resources carrying one of +these markers. That is what makes it safe to run alongside flavors and profiles +a human created by hand. + +Two mechanisms, because Neutron gives the two resources different places to +write to: service profiles carry marker keys inside ``meta_info``, flavors carry +a marker string appended to ``description``. +""" + +from __future__ import annotations + +from typing import Any + +from openstack_sync.plugins.common import get_value +from openstack_sync.plugins.common import meta_info_payload +from openstack_sync.plugins.common import normalize_meta_info + +MANAGED_META_INFO_KEY = "_understack_router_flavor_operator" +MANAGED_META_INFO_VALUE = "managed" +MARKER_VERSION_META_INFO_KEY = "_understack_router_flavor_marker_version" +MARKER_VERSION_META_INFO_VALUE = "v1" +MARKER_SOURCE_META_INFO_KEY = "_understack_router_flavor_source" +MARKER_SOURCE_META_INFO_VALUE = "NeutronRouterFlavor" + +FLAVOR_DESCRIPTION_MARKER = "[understack-router-flavor-operator]" + +#: Marker keys stamped into a managed service profile's ``meta_info``. +OPERATOR_META_INFO_MARKERS = { + MANAGED_META_INFO_KEY: MANAGED_META_INFO_VALUE, + MARKER_VERSION_META_INFO_KEY: MARKER_VERSION_META_INFO_VALUE, + MARKER_SOURCE_META_INFO_KEY: MARKER_SOURCE_META_INFO_VALUE, +} + +_MARKER_KEYS = frozenset(OPERATOR_META_INFO_MARKERS) + + +# --------------------------------------------------------------------------- +# Service profiles: markers live in meta_info +# --------------------------------------------------------------------------- + + +def service_profile_meta_info(profile: Any) -> Any: + """Return the ``meta_info`` of *profile*.""" + return get_value(profile, "meta_info", default={}) + + +def _comparable(value: Any) -> Any: + """Strip operator marker keys so specs and Neutron state compare equal.""" + normalized = normalize_meta_info(value) + if isinstance(normalized, dict): + return {k: v for k, v in normalized.items() if k not in _MARKER_KEYS} + return normalized + + +def meta_info_matches(current: Any, desired: Any) -> bool: + """Return True when *current* and *desired* meta_info are logically equal.""" + return meta_info_payload(_comparable(current)) == meta_info_payload( + _comparable(desired) + ) + + +def managed_meta_info(value: Any) -> Any: + """Return *value* with the operator ownership markers merged in.""" + normalized = normalize_meta_info(value) + if not isinstance(normalized, dict): + return normalized + return {**normalized, **OPERATOR_META_INFO_MARKERS} + + +def is_managed_service_profile(profile: Any) -> bool: + """Return True when *profile* carries the operator ownership marker.""" + meta_info = normalize_meta_info(service_profile_meta_info(profile)) + return ( + isinstance(meta_info, dict) + and meta_info.get(MANAGED_META_INFO_KEY) == MANAGED_META_INFO_VALUE + ) + + +# --------------------------------------------------------------------------- +# Flavors: the marker lives in description +# --------------------------------------------------------------------------- + + +def clean_flavor_description(value: Any) -> str: + """Return *value* with the operator description marker stripped.""" + return str(value or "").replace(FLAVOR_DESCRIPTION_MARKER, "").strip() + + +def managed_flavor_description(value: Any) -> str: + """Return *value* with the operator description marker appended.""" + description = clean_flavor_description(value) + if not description: + return FLAVOR_DESCRIPTION_MARKER + return f"{description} {FLAVOR_DESCRIPTION_MARKER}" + + +def flavor_description_has_marker(value: Any) -> bool: + """Return True when *value* contains the operator description marker.""" + return FLAVOR_DESCRIPTION_MARKER in str(value or "") + + +def is_managed_flavor(flavor: Any) -> bool: + """Return True when the flavor's description carries the operator marker.""" + return flavor_description_has_marker(get_value(flavor, "description", default="")) diff --git a/python/openstack-sync/openstack_sync/plugins/neutron/router_flavors/prune.py b/python/openstack-sync/openstack_sync/plugins/neutron/router_flavors/prune.py new file mode 100644 index 000000000..49810ef51 --- /dev/null +++ b/python/openstack-sync/openstack_sync/plugins/neutron/router_flavors/prune.py @@ -0,0 +1,169 @@ +"""Delete router flavors and service profiles whose CR was removed. + +Everything here is gated on the operator's ownership markers, so a flavor or +profile created by hand is never touched. Ownership is the only gate needed: a +resource carrying the marker was created by this operator, which makes any +further filtering redundant. +""" + +from __future__ import annotations + +import logging +from collections import Counter +from typing import Any + +from openstack import exceptions as openstack_exceptions + +from openstack_sync.plugins.common import get_service_profile +from openstack_sync.plugins.common import get_value +from openstack_sync.plugins.common import resource_id +from openstack_sync.plugins.common import service_profile_ids +from openstack_sync.plugins.neutron.router_flavors.config import SERVICE_TYPE +from openstack_sync.plugins.neutron.router_flavors.markers import is_managed_flavor +from openstack_sync.plugins.neutron.router_flavors.markers import ( + is_managed_service_profile, +) + +LOG = logging.getLogger(__name__) + +#: Service profiles fetched during a prune, keyed by ID. None means "gone". +ProfileCache = dict[str, Any | None] + + +def _cached_profile(conn: Any, profile_id: str, cache: ProfileCache) -> Any | None: + if profile_id not in cache: + cache[profile_id] = get_service_profile(conn, profile_id) + return cache[profile_id] + + +def _attachment_counts(flavors: list[Any]) -> Counter[str]: + """Count how many flavors each service profile is bound to.""" + counts: Counter[str] = Counter() + for flavor in flavors: + counts.update(set(service_profile_ids(flavor))) + return counts + + +def _flavor_has_routers(conn: Any, flavor: Any, flavor_name: str) -> bool: + """Return True when routers still use *flavor*, or when we cannot tell.""" + try: + routers = list(conn.network.routers(flavor_id=resource_id(flavor))) + except Exception as exc: # noqa: BLE001 + LOG.warning( + "Unable to check routers for router flavor %s; skipping deletion: %s", + flavor_name, + exc, + ) + return True + + if routers: + LOG.info( + "Router flavor %s is still used by %s router(s); skipping deletion", + flavor_name, + len(routers), + ) + return True + return False + + +def maybe_delete_profile( + conn: Any, profile_id: str, cache: ProfileCache, counts: Counter[str] +) -> None: + """Delete *profile_id* when the operator owns it and nothing is bound to it.""" + profile = _cached_profile(conn, profile_id, cache) + if not profile: + return + if not is_managed_service_profile(profile): + LOG.info("Keeping service profile %s; it is not operator-owned", profile_id) + return + if counts[profile_id] > 0: + LOG.info("Keeping service profile %s; it is still attached", profile_id) + return + + LOG.info("Deleting unused service profile %s", profile_id) + try: + conn.network.delete_service_profile(profile, ignore_missing=True) + cache[profile_id] = None + except openstack_exceptions.NotFoundException: + cache[profile_id] = None + except openstack_exceptions.ConflictException: + LOG.info("Service profile %s is still in use; skipping delete", profile_id) + + +def _delete_flavor( + conn: Any, flavor: Any, cache: ProfileCache, counts: Counter[str] +) -> None: + flavor_id = resource_id(flavor) + flavor_name = get_value(flavor, "name", default=flavor_id) + profile_ids = service_profile_ids(flavor) + + if _flavor_has_routers(conn, flavor, flavor_name): + return + + LOG.info("Deleting removed router flavor %s (%s)", flavor_name, flavor_id) + try: + conn.network.delete_flavor(flavor, ignore_missing=True) + except openstack_exceptions.NotFoundException: + LOG.info("Router flavor %s (%s) is already absent", flavor_name, flavor_id) + except openstack_exceptions.ConflictException: + LOG.info("Router flavor %s is still in use; skipping delete", flavor_name) + return + + # The flavor is gone, so its profiles lost one attachment each. + for profile_id in profile_ids: + counts[profile_id] -= 1 + if counts[profile_id] <= 0: + del counts[profile_id] + + for profile_id in profile_ids: + maybe_delete_profile(conn, profile_id, cache, counts) + + +def _prune_orphaned_profiles( + conn: Any, cache: ProfileCache, counts: Counter[str] +) -> None: + """Delete owned, unattached profiles left behind by an earlier partial failure. + + Safe to run every cycle: it only ever touches operator-owned profiles that + no flavor is bound to. + """ + LOG.info("Scanning for orphaned operator-owned service profiles") + for profile in list(conn.network.service_profiles()): + if is_managed_service_profile(profile): + maybe_delete_profile(conn, resource_id(profile), cache, counts) + + +def prune_removed_flavors( + conn: Any, + desired_specs: list[dict[str, Any]], + *, + authoritative_empty: bool = False, +) -> None: + """Delete operator-owned router flavors absent from *desired_specs*. + + An empty *desired_specs* is only acted on when *authoritative_empty* says a + CR really was deleted; otherwise it may be a snapshot we could not read, and + pruning against it would delete every managed flavor. + """ + if not desired_specs and not authoritative_empty: + LOG.warning( + "No desired router flavors found; skipping prune to avoid deleting " + "all managed router flavors" + ) + return + + desired_names = {str(spec["name"]) for spec in desired_specs if spec.get("name")} + cache: ProfileCache = {} + + LOG.info("Pruning removed router flavors") + current = list(conn.network.flavors(service_type=SERVICE_TYPE)) + counts = _attachment_counts(current) + for flavor in current: + name = get_value(flavor, "name") + if not name or name in desired_names: + continue + if not is_managed_flavor(flavor): + continue + _delete_flavor(conn, flavor, cache, counts) + + _prune_orphaned_profiles(conn, cache, counts) diff --git a/python/openstack-sync/openstack_sync/plugins/neutron/router_flavors/reconcile.py b/python/openstack-sync/openstack_sync/plugins/neutron/router_flavors/reconcile.py new file mode 100644 index 000000000..9ebe599fd --- /dev/null +++ b/python/openstack-sync/openstack_sync/plugins/neutron/router_flavors/reconcile.py @@ -0,0 +1,420 @@ +"""Reconcile a NeutronRouterFlavor CR onto Neutron. + +Ordered as the reconcile reads: resolve the service profiles the spec asks for, +converge the flavor itself, then converge the set of profiles bound to it. +""" + +from __future__ import annotations + +import json +import logging +from dataclasses import dataclass +from typing import Any + +from openstack import exceptions as openstack_exceptions + +from openstack_sync.plugins.common import ConfigError +from openstack_sync.plugins.common import get_service_profile +from openstack_sync.plugins.common import get_value +from openstack_sync.plugins.common import meta_info_payload +from openstack_sync.plugins.common import resource_id +from openstack_sync.plugins.common import service_profile_ids +from openstack_sync.plugins.neutron.router_flavors.markers import ( + clean_flavor_description, +) +from openstack_sync.plugins.neutron.router_flavors.markers import ( + flavor_description_has_marker, +) +from openstack_sync.plugins.neutron.router_flavors.markers import ( + is_managed_service_profile, +) +from openstack_sync.plugins.neutron.router_flavors.markers import ( + managed_flavor_description, +) +from openstack_sync.plugins.neutron.router_flavors.markers import managed_meta_info +from openstack_sync.plugins.neutron.router_flavors.markers import meta_info_matches +from openstack_sync.plugins.neutron.router_flavors.markers import ( + service_profile_meta_info, +) + +LOG = logging.getLogger(__name__) + +#: Service profiles already fetched this run, keyed by driver. +ProfileCache = dict[str, list[Any]] + + +@dataclass(frozen=True) +class ProfileDrift: + """One field of a reused service profile that diverged from the CR spec. + + Profile drift is reported, never auto-corrected. Neutron's + ``update_service_profile`` calls ``_ensure_service_profile_not_in_use`` and + raises ``ServiceProfileInUse`` (HTTP 409) while *any* flavor binding exists + -- not merely while a router is using it -- and this operator binds every + profile it manages. An update attempt would fail every cycle. Correcting + drift means unbinding the profile from every flavor first, which is an + operator decision. + """ + + profile_id: str + field: str + have: Any + want: Any + + def describe(self) -> str: + return ( + f"service profile {self.profile_id} {self.field}: " + f"have={self.have!r} want={self.want!r}" + ) + + +# --------------------------------------------------------------------------- +# Service profiles +# --------------------------------------------------------------------------- + + +def profiles_for_driver(conn: Any, driver: str, cache: ProfileCache) -> list[Any]: + """Return every service profile for *driver*, fetched once per run.""" + if driver not in cache: + cache[driver] = list(conn.network.service_profiles(driver=driver)) + return cache[driver] + + +def find_matching_profile(profiles: list[Any], meta_info: Any) -> Any | None: + """Return the operator-owned profile matching *meta_info*, if any. + + Only operator-owned profiles are reuse candidates. Reusing a profile the + operator does not own would bind it to the flavor, and + ``reconcile_flavor_profiles`` unbinds only profiles carrying the ownership + marker -- so the operator would have created a binding it can never remove. + That binding outlives the spec that created it, and Neutron's + ``get_flavor_next_provider`` picks an arbitrary binding (``objs[0]``), so a + stale one can end up serving routers with the wrong ``meta_info``. + + An unowned profile that happens to match is left completely alone -- not + adopted by stamping the marker onto it, which would enrol somebody else's + profile into ``prune`` for eventual deletion -- and ``ensure_profile`` + creates a dedicated managed profile alongside it. + """ + unowned: list[str] = [] + for profile in profiles: + if not meta_info_matches(service_profile_meta_info(profile), meta_info): + continue + if is_managed_service_profile(profile): + return profile + unowned.append(str(get_value(profile, "id", default=""))) + + if unowned: + LOG.info( + "Not reusing service profile(s) %s: they match the desired meta_info " + "but are not operator-owned, and the operator only binds profiles it " + "can unbind again; creating a dedicated managed profile instead", + sorted(unowned), + ) + return None + + +def _profile_drift( + profile: Any, profile_id: str, flavor_name: str, spec: dict[str, Any] +) -> list[ProfileDrift]: + """Return the spec fields a reused *profile* disagrees with. + + ``meta_info`` is excluded by construction -- the profile was selected by + matching it -- and ``driver`` is excluded because profiles are queried per + driver. That leaves ``is_enabled`` and ``description``. + + ``is_enabled`` is the consequential one: Neutron's + ``get_flavor_next_provider`` raises ``ServiceProfileDisabled`` (HTTP 503) + when the profile it selects is disabled, so every router create against the + flavor fails while the flavor still looks healthy. + """ + checks = ( + ( + "is_enabled", + bool(get_value(profile, "is_enabled", default=True)), + bool(spec["is_enabled"]), + ), + ( + "description", + str(get_value(profile, "description", default="")), + str(spec.get("description", "")), + ), + ) + drift = [ + ProfileDrift(profile_id=profile_id, field=field, have=have, want=want) + for field, have, want in checks + if have != want + ] + + for item in drift: + LOG.warning( + "Service profile %s reused by router flavor %s has drifted from the " + "CR spec (%s: have=%r want=%r). Neutron rejects updates to a profile " + "bound to any flavor, so the operator cannot correct this; unbind it " + "from every flavor to update it, or delete it and let the operator " + "recreate it", + profile_id, + flavor_name, + item.field, + item.have, + item.want, + ) + return drift + + +def ensure_profile( + conn: Any, + flavor_name: str, + spec: dict[str, Any], + cache: ProfileCache, + drift: list[ProfileDrift], +) -> Any: + """Find or create the service profile *spec* describes. + + The CRD guarantees ``driver`` and ``is_enabled`` are present; ``description`` + and ``meta_info`` are optional and fall back to empty. Drift on a reused + profile is appended to *drift* -- this is the only place holding both the + desired spec value and the Neutron state, so it is the only place drift can + be detected. + """ + driver = spec["driver"] + meta_info = spec.get("meta_info", {}) + + profiles = profiles_for_driver(conn, driver, cache) + profile = find_matching_profile(profiles, meta_info) + if profile: + profile_id = resource_id(profile) + LOG.info( + "Reusing service profile %s for %s driver=%s", + profile_id, + flavor_name, + driver, + ) + drift.extend(_profile_drift(profile, profile_id, flavor_name, spec)) + return profile + + LOG.info( + "Creating service profile for %s driver=%s is_enabled=%s", + flavor_name, + driver, + spec["is_enabled"], + ) + created = conn.network.create_service_profile( + description=spec.get("description", ""), + driver=driver, + meta_info=meta_info_payload(managed_meta_info(meta_info)), + is_enabled=spec["is_enabled"], + ) + # Visible to any later flavor this run with an identical (driver, meta_info) + # spec, so it reuses this profile instead of creating a duplicate. + profiles.append(created) + return created + + +# --------------------------------------------------------------------------- +# The flavor +# --------------------------------------------------------------------------- + + +def find_flavor(conn: Any, name: str) -> Any | None: + """Return the flavor named *name*, or None. + + The SDK passes ``name=`` as a server-side query parameter which Neutron + filters in SQL, so at most one record comes back; the equality check guards + against a future change to substring semantics. + """ + for flavor in conn.network.flavors(name=name): + if get_value(flavor, "name") == name: + return flavor + return None + + +def ensure_flavor(conn: Any, spec: dict[str, Any]) -> Any: + """Find or create the router flavor *spec* describes, reconciling drift.""" + name = spec["name"] + service_type = spec["service_type"] + description = spec.get("description", "") + is_enabled = spec["is_enabled"] + + flavor = find_flavor(conn, name) + if not flavor: + LOG.info( + "Creating router flavor %s service_type=%s is_enabled=%s", + name, + service_type, + is_enabled, + ) + return conn.network.create_flavor( + name=name, + service_type=service_type, + is_enabled=is_enabled, + description=managed_flavor_description(description), + ) + + LOG.info("Router flavor %s already exists", name) + current_service_type = get_value(flavor, "service_type", default="") + if current_service_type != service_type: + raise ConfigError( + f"Router flavor {name!r} already exists in Neutron with " + f"service_type={current_service_type!r}; expected {service_type!r}. " + f"Neutron does not allow updating service_type on an existing " + f"flavor. Rename the CR or remove the existing Neutron flavor to " + f"let the operator recreate it." + ) + + current_description = get_value(flavor, "description", default="") + current_is_enabled = bool(get_value(flavor, "is_enabled", default=True)) + description_changed = clean_flavor_description( + current_description + ) != clean_flavor_description(description) + marker_missing = not flavor_description_has_marker(current_description) + is_enabled_changed = current_is_enabled != is_enabled + + if is_enabled_changed: + LOG.info( + "Router flavor %s is_enabled drift: have=%s want=%s; reconciling", + name, + current_is_enabled, + is_enabled, + ) + + if description_changed or marker_missing or is_enabled_changed: + return conn.network.update_flavor( + flavor, + description=managed_flavor_description(description), + is_enabled=is_enabled, + ) + return flavor + + +# --------------------------------------------------------------------------- +# Flavor <-> profile bindings +# --------------------------------------------------------------------------- + + +def _associate(conn: Any, flavor: Any, profile: Any) -> None: + flavor_id = resource_id(flavor) + profile_id = resource_id(profile) + LOG.info("Binding service profile %s to router flavor %s", profile_id, flavor_id) + try: + conn.network.associate_flavor_with_service_profile(flavor, profile) + except openstack_exceptions.ConflictException: + # Another reconcile bound it first. + LOG.info( + "Router flavor %s already has service profile %s", flavor_id, profile_id + ) + + +def _disassociate(conn: Any, flavor: Any, profile: Any) -> None: + flavor_id = resource_id(flavor) + profile_id = resource_id(profile) + LOG.info( + "Unbinding operator-managed service profile %s from router flavor %s", + profile_id, + flavor_id, + ) + try: + conn.network.disassociate_flavor_from_service_profile(flavor, profile) + except openstack_exceptions.NotFoundException: + LOG.info( + "Service profile %s already absent from router flavor %s", + profile_id, + flavor_id, + ) + except openstack_exceptions.ConflictException: + LOG.warning( + "Cannot unbind service profile %s from router flavor %s (Neutron " + "reports conflict, likely in use); leaving it attached", + profile_id, + flavor_id, + ) + + +def reconcile_flavor_profiles( + conn: Any, flavor: Any, desired_profiles: list[Any] +) -> Any: + """Converge the set of service profiles bound to *flavor*. + + Profiles missing from the flavor are bound; operator-owned profiles bound to + it but absent from the desired set are unbound. Profiles attached + out-of-band are left alone -- the operator only unbinds what it owns. + """ + flavor = conn.network.get_flavor(flavor) + flavor_name = get_value(flavor, "name", default=resource_id(flavor)) + + desired_by_id = {resource_id(p): p for p in desired_profiles} + current_ids = set(service_profile_ids(flavor)) + to_bind = set(desired_by_id) - current_ids + to_unbind = current_ids - set(desired_by_id) + + if not to_bind and not to_unbind: + LOG.info( + "Router flavor %s already has the desired service profiles %s", + flavor_name, + sorted(current_ids), + ) + return flavor + + for profile_id in sorted(to_bind): + _associate(conn, flavor, desired_by_id[profile_id]) + + for profile_id in sorted(to_unbind): + profile = get_service_profile(conn, profile_id) + if profile is None: + LOG.info( + "Service profile %s already absent from Neutron; nothing to unbind", + profile_id, + ) + continue + if not is_managed_service_profile(profile): + LOG.info( + "Keeping unowned service profile %s on router flavor %s; the " + "operator only unbinds profiles it owns", + profile_id, + flavor_name, + ) + continue + _disassociate(conn, flavor, profile) + + return conn.network.get_flavor(flavor) + + +# --------------------------------------------------------------------------- +# Entry point +# --------------------------------------------------------------------------- + + +def render_flavor(flavor: Any) -> dict[str, Any]: + """Return the reconciled flavor as a loggable dict.""" + return { + "id": get_value(flavor, "id"), + "name": get_value(flavor, "name"), + "service_type": get_value(flavor, "service_type"), + "description": get_value(flavor, "description"), + "is_enabled": get_value(flavor, "is_enabled"), + "service_profile_ids": service_profile_ids(flavor), + } + + +def sync_flavor(conn: Any, spec: dict[str, Any], cache: ProfileCache) -> list[str]: + """Converge one NeutronRouterFlavor spec, returning drift notes.""" + name = spec["name"] + profile_specs = spec["service_profiles"] + + LOG.info( + "Reconciling router flavor %s with %s service profile(s)", + name, + len(profile_specs), + ) + drift: list[ProfileDrift] = [] + desired_profiles = [ + ensure_profile(conn, name, profile_spec, cache, drift) + for profile_spec in profile_specs + ] + flavor = ensure_flavor(conn, spec) + flavor = reconcile_flavor_profiles(conn, flavor, desired_profiles) + LOG.info( + "Reconciled router flavor: %s", + json.dumps(render_flavor(flavor), sort_keys=True), + ) + return [item.describe() for item in drift] diff --git a/python/openstack-sync/pyproject.toml b/python/openstack-sync/pyproject.toml index ef728ddd2..c43f68b26 100644 --- a/python/openstack-sync/pyproject.toml +++ b/python/openstack-sync/pyproject.toml @@ -81,4 +81,10 @@ force-single-line = true convention = "google" [tool.ruff.lint.per-file-ignores] -"tests/*" = ["S101"] # assert is the point in tests +"tests/*" = [ + "S101", # assert is the point in tests + # Fixtures name Kubernetes Secrets, which flake8-bandit reads as passwords. + "S105", + "S106", + "S107", +] diff --git a/python/openstack-sync/tests/conftest.py b/python/openstack-sync/tests/conftest.py new file mode 100644 index 000000000..ca92df680 --- /dev/null +++ b/python/openstack-sync/tests/conftest.py @@ -0,0 +1,61 @@ +"""Pytest configuration and shared fixtures for openstack-sync tests. + +Two levels of configuration, matching where the code reads it: + +* Hook-level tests drive ``main()``, which reads the environment the Helm chart + injects. The autouse fixture below provides the CRD identity variables the + chart always sets, so those tests exercise the real boundary. +* Everything below the hook takes a :class:`HookConfig` argument, so unit tests + use the ``hook_config`` fixture and never touch the environment. +""" + +from __future__ import annotations + +import pytest + +from openstack_sync.hooks.framework import HookConfig +from openstack_sync.plugins.neutron.router_flavors.config import BINDING_NAME +from openstack_sync.plugins.neutron.router_flavors.config import ENV_PREFIX + +CRD_API_VERSION = "neutron.understack.rackspace.net/v1alpha1" +CRD_KIND = "NeutronRouterFlavor" +CRD_RESOURCE = "neutronrouterflavors.neutron.understack.rackspace.net" + +_CRD_IDENTITY_ENV = { + f"{ENV_PREFIX}_CRD_API_VERSION": CRD_API_VERSION, + f"{ENV_PREFIX}_CRD_KIND": CRD_KIND, + f"{ENV_PREFIX}_CRD_RESOURCE": CRD_RESOURCE, +} + + +@pytest.fixture(autouse=True) +def _crd_identity_env(monkeypatch: pytest.MonkeyPatch) -> None: + """Provide the CRD identity variables the Helm chart always injects. + + Individual tests may override these with their own monkeypatch calls. + """ + for key, value in _CRD_IDENTITY_ENV.items(): + monkeypatch.setenv(key, value) + + +def make_hook_config(**overrides) -> HookConfig: + """Build a HookConfig without touching the environment.""" + defaults = { + "prefix": ENV_PREFIX, + "crd_api_version": CRD_API_VERSION, + "crd_kind": CRD_KIND, + "crd_resource": CRD_RESOURCE, + "binding_name": BINDING_NAME, + "namespace": "openstack", + "status_enabled": False, + "prune": False, + "sync_crontab": "", + "ready_retries": 30, + "ready_delay": 10.0, + } + return HookConfig(**{**defaults, **overrides}) + + +@pytest.fixture +def hook_config() -> HookConfig: + return make_hook_config() diff --git a/python/openstack-sync/tests/test_framework.py b/python/openstack-sync/tests/test_framework.py new file mode 100644 index 000000000..71b5595d8 --- /dev/null +++ b/python/openstack-sync/tests/test_framework.py @@ -0,0 +1,767 @@ +"""Tests for the generic sync framework. + +Deliberately free of Neutron: the driver is exercised through a stub plugin, so +these tests describe the contract any future plugin can rely on. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any +from unittest import mock + +import pytest + +from openstack_sync.hooks import framework +from openstack_sync.hooks.framework import HookConfig +from openstack_sync.hooks.framework import HookInputs +from openstack_sync.hooks.framework import SyncPlugin +from openstack_sync.hooks.framework import SyncResource +from openstack_sync.hooks.framework import build_crd_hook_config +from openstack_sync.hooks.framework import hook_inputs +from openstack_sync.hooks.framework import run_hook +from openstack_sync.hooks.framework import run_sync +from openstack_sync.hooks.framework import synced_message +from openstack_sync.plugins.common import ConfigError +from tests.conftest import CRD_API_VERSION +from tests.conftest import CRD_KIND +from tests.conftest import CRD_RESOURCE +from tests.conftest import make_hook_config + +PREFIX = "NEUTRON_ROUTER_FLAVOR" +BINDING = "neutron-router-flavors" + +ENV_NAMES = ( + "BINDING_CONTEXT_PATH", + f"{PREFIX}_ENABLED", + f"{PREFIX}_SYNC_CRONTAB", + f"{PREFIX}_PRUNE", + f"{PREFIX}_STATUS_ENABLED", + f"{PREFIX}_READY_RETRIES", + f"{PREFIX}_READY_DELAY", + "POD_NAMESPACE", +) + + +def clear_env(monkeypatch: pytest.MonkeyPatch) -> None: + for name in ENV_NAMES: + monkeypatch.delenv(name, raising=False) + + +# --------------------------------------------------------------------------- +# Stub plugin +# --------------------------------------------------------------------------- + + +class StubPlugin(SyncPlugin): + """Records what the driver asked it to do.""" + + noun = "widget" + + def __init__( + self, + config: HookConfig, + *, + fail_for: tuple[str, ...] = (), + notes_for: dict[str, list[str]] | None = None, + prune_raises: bool = False, + ) -> None: + super().__init__(config) + self.fail_for = set(fail_for) + self.notes_for = notes_for or {} + self.prune_raises = prune_raises + self.reconciled: list[str] = [] + self.pruned: list[tuple[list[str], bool]] = [] + self.waits = 0 + self.caches: list[Any] = [] + + def wait_for_api(self, conn: Any) -> None: + self.waits += 1 + + def new_cache(self) -> Any: + cache: dict[str, Any] = {} + self.caches.append(cache) + return cache + + def reconcile(self, conn: Any, spec: dict[str, Any], cache: Any) -> list[str]: + 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, [])) + + def prune( + self, + conn: Any, + desired_specs: list[dict[str, Any]], + *, + authoritative_empty: bool, + ) -> None: + if self.prune_raises: + raise RuntimeError("prune exploded") + self.pruned.append( + ([spec["name"] for spec in desired_specs], authoritative_empty) + ) + + +def _resource( + name: str, secret: str = "infrasetup", cloud: str = "understack" +) -> SyncResource: + return SyncResource( + spec={"name": name}, + name=name, + namespace="openstack", + generation=1, + secret_name=secret, + cloud_name=cloud, + ) + + +def _inputs( + reconcile: list[SyncResource], + desired: list[SyncResource] | None = None, + deleted: list[SyncResource] | None = None, + prune_credentials: frozenset[tuple[str, str]] | None = None, +) -> HookInputs: + desired = reconcile if desired is None else desired + deleted = deleted or [] + if prune_credentials is None: + prune_credentials = frozenset(r.credentials for r in desired + deleted) + return HookInputs(reconcile, desired, deleted, prune_credentials) + + +def _drive(plugin: StubPlugin, inputs: HookInputs): + """Run the driver with connections and status patching stubbed out.""" + with ( + mock.patch.object(framework, "get_openstack_connection") as connect, + mock.patch.object(framework, "patch_resource_status") as patch_status, + ): + code = run_sync(plugin, inputs) + return code, patch_status, connect + + +# --------------------------------------------------------------------------- +# HookConfig: the chart contract +# --------------------------------------------------------------------------- + + +def test_hook_config_reads_the_chart_contract(monkeypatch): + clear_env(monkeypatch) + monkeypatch.setenv(f"{PREFIX}_STATUS_ENABLED", "true") + monkeypatch.setenv(f"{PREFIX}_PRUNE", "true") + monkeypatch.setenv(f"{PREFIX}_SYNC_CRONTAB", "0 * * * *") + monkeypatch.setenv(f"{PREFIX}_READY_RETRIES", "7") + monkeypatch.setenv(f"{PREFIX}_READY_DELAY", "2.5") + monkeypatch.setenv("POD_NAMESPACE", "openstack") + + config = HookConfig.from_env(PREFIX, binding_name=BINDING) + + assert config.crd_api_version == CRD_API_VERSION + assert config.crd_kind == CRD_KIND + assert config.crd_resource == CRD_RESOURCE + assert config.binding_name == BINDING + assert config.namespace == "openstack" + assert config.status_enabled is True + assert config.prune is True + assert config.sync_crontab == "0 * * * *" + assert config.ready_retries == 7 + assert config.ready_delay == 2.5 + + +def test_hook_config_defaults_are_off(monkeypatch): + clear_env(monkeypatch) + + config = HookConfig.from_env(PREFIX, binding_name=BINDING) + + assert config.status_enabled is False + assert config.prune is False + assert config.sync_crontab == "" + assert config.ready_retries == 30 + assert config.ready_delay == 10 + + +def test_hook_config_requires_crd_identity(monkeypatch): + clear_env(monkeypatch) + monkeypatch.delenv(f"{PREFIX}_CRD_KIND", raising=False) + + with pytest.raises(ConfigError, match=f"{PREFIX}_CRD_KIND"): + HookConfig.from_env(PREFIX, binding_name=BINDING) + + +# --------------------------------------------------------------------------- +# Hook config JSON +# --------------------------------------------------------------------------- + + +def test_disabled_hook_config_is_a_valid_noop(monkeypatch): + clear_env(monkeypatch) + + config = build_crd_hook_config(PREFIX, BINDING) + + # shell-operator requires at least one binding, but a disabled hook must not + # register Kubernetes watches it will never service. + assert config["onStartup"] == 10 + assert "kubernetes" not in config + assert "schedule" not in config + + +def test_disabled_hook_config_does_not_read_runtime_env(monkeypatch): + """--config runs before the environment is guaranteed to be complete.""" + clear_env(monkeypatch) + for name in ( + f"{PREFIX}_CRD_API_VERSION", + f"{PREFIX}_CRD_KIND", + f"{PREFIX}_CRD_RESOURCE", + ): + monkeypatch.delenv(name, raising=False) + + config = build_crd_hook_config(PREFIX, BINDING) + + assert config["onStartup"] == 10 + + +def test_crontab_does_not_enable_a_disabled_hook(monkeypatch): + clear_env(monkeypatch) + monkeypatch.setenv(f"{PREFIX}_SYNC_CRONTAB", "0 * * * *") + + config = build_crd_hook_config(PREFIX, BINDING) + + assert "schedule" not in config + assert config["onStartup"] == 10 + + +def test_enabled_hook_config_watches_the_crd(monkeypatch): + clear_env(monkeypatch) + monkeypatch.setenv(f"{PREFIX}_ENABLED", "true") + monkeypatch.setenv("POD_NAMESPACE", "openstack") + + config = build_crd_hook_config(PREFIX, BINDING) + + (binding,) = config["kubernetes"] + assert binding["name"] == BINDING + assert binding["apiVersion"] == CRD_API_VERSION + assert binding["kind"] == CRD_KIND + assert binding["executeHookOnEvent"] == ["Added", "Modified", "Deleted"] + # The full object is needed: the reconcile reads spec and status. + assert binding["jqFilter"] == "." + assert binding["includeSnapshotsFrom"] == [BINDING] + # A dedicated queue keeps a slow reconcile from blocking other hooks. + assert binding["queue"] == BINDING + assert binding["namespace"] == {"nameSelector": {"matchNames": ["openstack"]}} + + +def test_enabled_hook_config_omits_schedule_without_crontab(monkeypatch): + clear_env(monkeypatch) + monkeypatch.setenv(f"{PREFIX}_ENABLED", "true") + + assert "schedule" not in build_crd_hook_config(PREFIX, BINDING) + + +def test_enabled_hook_config_adds_schedule_with_crontab(monkeypatch): + clear_env(monkeypatch) + monkeypatch.setenv(f"{PREFIX}_ENABLED", "true") + monkeypatch.setenv(f"{PREFIX}_SYNC_CRONTAB", "*/5 * * * *") + + (schedule,) = build_crd_hook_config(PREFIX, BINDING)["schedule"] + + assert schedule["crontab"] == "*/5 * * * *" + assert schedule["includeSnapshotsFrom"] == [BINDING] + assert schedule["queue"] == BINDING + + +def test_enabled_hook_config_omits_namespace_without_pod_namespace(monkeypatch): + clear_env(monkeypatch) + monkeypatch.setenv(f"{PREFIX}_ENABLED", "true") + + (binding,) = build_crd_hook_config(PREFIX, BINDING)["kubernetes"] + + assert "namespace" not in binding + + +# --------------------------------------------------------------------------- +# Binding context -> HookInputs +# --------------------------------------------------------------------------- + + +def _cr(name: str, generation: int = 3, status: dict | None = None) -> dict: + obj = { + "apiVersion": CRD_API_VERSION, + "kind": CRD_KIND, + "metadata": {"name": name, "namespace": "openstack", "generation": generation}, + "spec": { + "name": name, + "cloudCredentialsRef": { + "secretName": "infrasetup", + "cloudName": "understack", + }, + }, + } + if status is not None: + obj["status"] = status + return obj + + +def test_hook_inputs_from_snapshot_reconciles_everything(): + config = make_hook_config() + contexts = [ + { + "binding": BINDING, + "type": "Schedule", + "snapshots": {BINDING: [{"object": _cr("b")}, {"object": _cr("a")}]}, + } + ] + + inputs = hook_inputs(contexts, config) + + # Sorted so a run is deterministic. + assert [r.spec["name"] for r in inputs.resources_to_reconcile] == ["a", "b"] + assert inputs.desired_resources_for_prune == inputs.resources_to_reconcile + assert inputs.deleted_resources == [] + assert inputs.prune_credentials == frozenset({("infrasetup", "understack")}) + + +def test_hook_inputs_from_synchronization(): + config = make_hook_config() + contexts = [ + { + "binding": BINDING, + "type": "Synchronization", + "objects": [{"object": _cr("a")}], + } + ] + + inputs = hook_inputs(contexts, config) + + assert [r.spec["name"] for r in inputs.resources_to_reconcile] == ["a"] + + +def test_hook_inputs_strips_cloud_credentials_from_spec(): + """A plugin must see only its own fields.""" + config = make_hook_config() + contexts = [ + { + "binding": BINDING, + "type": "Schedule", + "snapshots": {BINDING: [{"object": _cr("a")}]}, + } + ] + + (resource,) = hook_inputs(contexts, config).resources_to_reconcile + + assert "cloudCredentialsRef" not in resource.spec + assert resource.secret_name == "infrasetup" + assert resource.cloud_name == "understack" + + +def test_hook_inputs_keeps_current_status(): + """The status is needed to break the status-patch feedback loop.""" + config = make_hook_config() + status = {"syncStatus": "Synced", "observedGeneration": 3} + contexts = [ + { + "binding": BINDING, + "type": "Schedule", + "snapshots": {BINDING: [{"object": _cr("a", status=status)}]}, + } + ] + + (resource,) = hook_inputs(contexts, config).resources_to_reconcile + + assert resource.current_status == status + + +def test_added_event_reconciles_only_the_changed_resource(): + config = make_hook_config() + contexts = [ + { + "binding": BINDING, + "type": "Event", + "watchEvent": "Added", + "object": _cr("new"), + "snapshots": {BINDING: [{"object": _cr("new")}, {"object": _cr("old")}]}, + } + ] + + inputs = hook_inputs(contexts, config) + + assert [r.spec["name"] for r in inputs.resources_to_reconcile] == ["new"] + # Prune still needs the full desired set, or it would delete "old". + assert [r.spec["name"] for r in inputs.desired_resources_for_prune] == [ + "new", + "old", + ] + + +def test_deleted_event_reconciles_nothing_but_prunes(): + config = make_hook_config() + contexts = [ + { + "binding": BINDING, + "type": "Event", + "watchEvent": "Deleted", + "object": _cr("gone"), + "snapshots": {BINDING: [{"object": _cr("kept")}]}, + } + ] + + inputs = hook_inputs(contexts, config) + + assert inputs.resources_to_reconcile == [] + assert [r.spec["name"] for r in inputs.deleted_resources] == ["gone"] + assert inputs.prune_credentials == frozenset({("infrasetup", "understack")}) + + +def test_modified_event_skipped_when_status_already_current(): + """The hook's own status patch must not trigger another reconcile.""" + config = make_hook_config() + current = {"syncStatus": "Synced", "observedGeneration": 3} + obj = _cr("a", generation=3, status=current) + contexts = [ + { + "binding": BINDING, + "type": "Event", + "watchEvent": "Modified", + "object": obj, + "snapshots": {BINDING: [{"object": obj}]}, + } + ] + + inputs = hook_inputs(contexts, config) + + assert inputs.resources_to_reconcile == [] + + +def test_modified_event_reconciles_when_generation_bumped(): + config = make_hook_config() + stale = {"syncStatus": "Synced", "observedGeneration": 2} + obj = _cr("a", generation=3, status=stale) + contexts = [ + { + "binding": BINDING, + "type": "Event", + "watchEvent": "Modified", + "object": obj, + "snapshots": {BINDING: [{"object": obj}]}, + } + ] + + inputs = hook_inputs(contexts, config) + + assert [r.spec["name"] for r in inputs.resources_to_reconcile] == ["a"] + + +def test_event_context_without_snapshot_is_an_error(): + config = make_hook_config() + contexts = [ + {"binding": BINDING, "type": "Event", "watchEvent": "Added", "object": _cr("a")} + ] + + with pytest.raises(ConfigError, match="snapshot"): + hook_inputs(contexts, config) + + +def test_unrecognised_context_is_an_error(): + config = make_hook_config() + + with pytest.raises(ConfigError, match="does not contain"): + hook_inputs([{"binding": "something-else", "type": "Event"}], config) + + +# --------------------------------------------------------------------------- +# run_sync +# --------------------------------------------------------------------------- + + +def test_run_sync_reconciles_and_reports_synced(): + plugin = StubPlugin(make_hook_config()) + + code, patch_status, _ = _drive(plugin, _inputs([_resource("a"), _resource("b")])) + + assert code == 0 + assert plugin.reconciled == ["a", "b"] + statuses = [call.kwargs["sync_status"] for call in patch_status.call_args_list] + assert statuses == ["Synced", "Synced"] + assert patch_status.call_args_list[0].kwargs["message"] == ( + "Successfully reconciled widget" + ) + + +def test_run_sync_waits_for_api_once_per_credential_group(): + plugin = StubPlugin(make_hook_config()) + resources = [ + _resource("a"), + _resource("b"), + _resource("c", secret="other", cloud="other-cloud"), + ] + + _drive(plugin, _inputs(resources)) + + assert plugin.waits == 2 + # One cache per group, so lookups are shared within a group but not across. + assert len(plugin.caches) == 2 + + +def test_run_sync_connects_with_each_resources_own_credentials(): + plugin = StubPlugin(make_hook_config()) + resources = [ + _resource("a", secret="secret-a", cloud="cloud-a"), + _resource("b", secret="secret-b", cloud="cloud-b"), + ] + + _, _, connect = _drive(plugin, _inputs(resources)) + + assert sorted(call.args for call in connect.call_args_list) == [ + ("secret-a", "cloud-a"), + ("secret-b", "cloud-b"), + ] + + +def test_run_sync_forwards_crd_identity_and_current_status_to_the_patch(): + """Forward everything patch_resource_status needs. + + The CRD identity targets kubectl, and the current status decides whether the + patch can be skipped. + """ + config = make_hook_config(status_enabled=True) + plugin = StubPlugin(config) + status = {"syncStatus": "Synced", "observedGeneration": 1} + resource = SyncResource( + spec={"name": "a"}, + name="a", + namespace="openstack", + generation=1, + secret_name="infrasetup", + cloud_name="understack", + current_status=status, + ) + + _, patch_status, _ = _drive(plugin, _inputs([resource])) + + kwargs = patch_status.call_args.kwargs + assert kwargs["crd_resource"] == CRD_RESOURCE + assert kwargs["crd_kind"] == CRD_KIND + assert kwargs["status_enabled"] is True + assert kwargs["current_status"] == status + assert kwargs["generation"] == 1 + assert kwargs["namespace"] == "openstack" + + +def test_run_sync_reports_notes_without_failing(): + plugin = StubPlugin(make_hook_config(), notes_for={"a": ["thing drifted"]}) + + code, patch_status, _ = _drive(plugin, _inputs([_resource("a")])) + + assert code == 0 + assert patch_status.call_args.kwargs["sync_status"] == "Synced" + message = patch_status.call_args.kwargs["message"] + assert message.startswith("Successfully reconciled widget") + assert "thing drifted" in message + + +def test_run_sync_marks_failure_and_skips_prune(): + """A failed reconcile means the desired set is unknown, so prune must not run.""" + plugin = StubPlugin(make_hook_config(prune=True), fail_for=("b",)) + + code, patch_status, _ = _drive(plugin, _inputs([_resource("a"), _resource("b")])) + + assert code == 1 + assert plugin.pruned == [] + by_name = { + call.kwargs["name"]: call.kwargs["sync_status"] + for call in patch_status.call_args_list + } + assert by_name == {"a": "Synced", "b": "Failed"} + + +def test_run_sync_continues_after_one_failure(): + plugin = StubPlugin(make_hook_config(), fail_for=("a",)) + + _drive(plugin, _inputs([_resource("a"), _resource("b")])) + + assert plugin.reconciled == ["a", "b"] + + +def test_run_sync_marks_whole_group_failed_when_connection_fails(): + plugin = StubPlugin(make_hook_config()) + inputs = _inputs([_resource("a"), _resource("b")]) + + with ( + mock.patch.object( + framework, + "get_openstack_connection", + side_effect=RuntimeError("no route to keystone"), + ), + mock.patch.object(framework, "patch_resource_status") as patch_status, + ): + code = run_sync(plugin, inputs) + + assert code == 1 + assert plugin.reconciled == [] + statuses = {call.kwargs["sync_status"] for call in patch_status.call_args_list} + assert statuses == {"Failed"} + assert "no route to keystone" in patch_status.call_args.kwargs["message"] + + +def test_run_sync_marks_group_failed_when_api_never_becomes_ready(): + class NeverReady(StubPlugin): + def wait_for_api(self, conn): + raise RuntimeError("api not ready") + + plugin = NeverReady(make_hook_config()) + + code, patch_status, _ = _drive(plugin, _inputs([_resource("a")])) + + assert code == 1 + assert plugin.reconciled == [] + assert patch_status.call_args.kwargs["sync_status"] == "Failed" + assert "api not ready" in patch_status.call_args.kwargs["message"] + + +def test_run_sync_prunes_after_successful_reconcile(): + plugin = StubPlugin(make_hook_config(prune=True)) + + code, _, _ = _drive(plugin, _inputs([_resource("a")])) + + assert code == 0 + assert plugin.pruned == [(["a"], False)] + + +def test_run_sync_prune_is_authoritative_for_deleted_credentials(): + """A confirmed deletion lets prune act on an empty desired set.""" + plugin = StubPlugin(make_hook_config(prune=True)) + deleted = _resource("gone") + inputs = _inputs([], desired=[], deleted=[deleted]) + + code, _, _ = _drive(plugin, inputs) + + assert code == 0 + assert plugin.pruned == [([], True)] + + +def test_run_sync_skips_prune_for_credentials_with_no_desired_resources(): + """An empty desired set with no deletion may be an unreadable snapshot.""" + plugin = StubPlugin(make_hook_config(prune=True)) + inputs = HookInputs([], [], [], frozenset({("infrasetup", "understack")})) + + code, _, _ = _drive(plugin, inputs) + + assert code == 0 + assert plugin.pruned == [] + + +def test_run_sync_does_not_connect_for_prune_when_prune_disabled(): + """A deleted-only run must not open a connection just to do nothing.""" + plugin = StubPlugin(make_hook_config(prune=False)) + inputs = _inputs([], desired=[], deleted=[_resource("gone")]) + + code, _, connect = _drive(plugin, inputs) + + assert code == 0 + assert connect.call_count == 0 + assert plugin.pruned == [] + + +def test_run_sync_returns_error_when_prune_fails(): + plugin = StubPlugin(make_hook_config(prune=True), prune_raises=True) + + code, _, _ = _drive(plugin, _inputs([_resource("a")])) + + assert code == 1 + + +def test_run_sync_skips_status_patch_without_metadata_name(): + plugin = StubPlugin(make_hook_config()) + nameless = SyncResource( + spec={"name": "a"}, + name=None, + namespace="openstack", + generation=1, + secret_name="infrasetup", + cloud_name="understack", + ) + + code, patch_status, _ = _drive(plugin, _inputs([nameless])) + + assert code == 0 + patch_status.assert_not_called() + + +def test_synced_message_is_unqualified_without_notes(): + assert synced_message("widget", []) == "Successfully reconciled widget" + + +def test_synced_message_lists_every_note(): + message = synced_message("widget", ["first", "second"]) + + assert "first" in message + assert "second" in message + + +# --------------------------------------------------------------------------- +# run_hook +# --------------------------------------------------------------------------- + + +def _write_context(path: Path, payload: str) -> str: + context_path = path / "binding-context.json" + context_path.write_text(payload, encoding="utf-8") + return str(context_path) + + +def test_run_hook_prints_config_and_exits(monkeypatch, capsys): + monkeypatch.setattr(framework.sys, "argv", ["hook.py", "--config"]) + + code = run_hook(lambda: {"configVersion": "v1"}, lambda contexts: 99) + + assert code == 0 + assert json.loads(capsys.readouterr().out) == {"configVersion": "v1"} + + +def test_run_hook_returns_zero_without_context_path(monkeypatch): + monkeypatch.setattr(framework.sys, "argv", ["hook.py"]) + monkeypatch.delenv("BINDING_CONTEXT_PATH", raising=False) + called = [] + + code = run_hook(dict, lambda contexts: called.append(contexts) or 0) + + assert code == 0 + assert called == [] + + +def test_run_hook_returns_zero_on_empty_context(monkeypatch, tmp_path): + monkeypatch.setattr(framework.sys, "argv", ["hook.py"]) + monkeypatch.setenv("BINDING_CONTEXT_PATH", _write_context(tmp_path, " ")) + called = [] + + code = run_hook(dict, lambda contexts: called.append(contexts) or 0) + + assert code == 0 + assert called == [] + + +def test_run_hook_returns_error_on_invalid_json(monkeypatch, tmp_path, caplog): + monkeypatch.setattr(framework.sys, "argv", ["hook.py"]) + monkeypatch.setenv("BINDING_CONTEXT_PATH", _write_context(tmp_path, "{not json")) + + code = run_hook(dict, lambda contexts: 0) + + assert code == 1 + assert "binding context" in caplog.text + + +def test_run_hook_returns_error_when_context_is_not_a_list(monkeypatch, tmp_path): + monkeypatch.setattr(framework.sys, "argv", ["hook.py"]) + monkeypatch.setenv("BINDING_CONTEXT_PATH", _write_context(tmp_path, '{"a": 1}')) + + assert run_hook(dict, lambda contexts: 0) == 1 + + +def test_run_hook_converts_an_unexpected_error_into_exit_one(monkeypatch, tmp_path): + monkeypatch.setattr(framework.sys, "argv", ["hook.py"]) + monkeypatch.setenv("BINDING_CONTEXT_PATH", _write_context(tmp_path, "[{}]")) + + def boom(contexts): + raise ConfigError("bad spec") + + assert run_hook(dict, boom) == 1 diff --git a/python/openstack-sync/tests/test_hook_common.py b/python/openstack-sync/tests/test_hook_common.py new file mode 100644 index 000000000..2a8d1bf21 --- /dev/null +++ b/python/openstack-sync/tests/test_hook_common.py @@ -0,0 +1,350 @@ +"""Tests for openstack_sync.hooks.common — generic shell-operator utilities.""" + +from __future__ import annotations + +import json +import logging +from unittest import mock + +import pytest + +from openstack_sync.hooks import common as hc + +# --------------------------------------------------------------------------- +# configure_logging +# --------------------------------------------------------------------------- + + +def test_configure_logging_defaults_to_info(monkeypatch): + monkeypatch.delenv("LOG_LEVEL", raising=False) + + with mock.patch.object(logging, "basicConfig") as basic_config: + hc.configure_logging() + + assert basic_config.call_args.kwargs["level"] == "INFO" + + +def test_configure_logging_reads_log_level(monkeypatch): + monkeypatch.setenv("LOG_LEVEL", "debug") + + with mock.patch.object(logging, "basicConfig") as basic_config: + hc.configure_logging() + + assert basic_config.call_args.kwargs["level"] == "DEBUG" + + +# --------------------------------------------------------------------------- +# Type coercions +# --------------------------------------------------------------------------- + + +def test_string_or_none_returns_none_for_none(): + assert hc.string_or_none(None) is None + + +def test_string_or_none_converts_value(): + assert hc.string_or_none(42) == "42" + assert hc.string_or_none("hello") == "hello" + + +def test_int_or_none_returns_none_for_none(): + assert hc.int_or_none(None) is None + + +def test_int_or_none_converts_int_string(): + assert hc.int_or_none("7") == 7 + assert hc.int_or_none(3) == 3 + + +def test_int_or_none_returns_none_for_invalid(): + assert hc.int_or_none("not-a-number") is None + assert hc.int_or_none([]) is None + + +# --------------------------------------------------------------------------- +# read_binding_context +# --------------------------------------------------------------------------- + + +def test_read_binding_context_returns_empty_when_no_env(monkeypatch): + monkeypatch.delenv("BINDING_CONTEXT_PATH", raising=False) + assert hc.read_binding_context() == [] + + +def test_read_binding_context_parses_json(monkeypatch, tmp_path): + ctx = [{"binding": "test", "type": "Event"}] + ctx_file = tmp_path / "ctx.json" + ctx_file.write_text(json.dumps(ctx), encoding="utf-8") + monkeypatch.setenv("BINDING_CONTEXT_PATH", str(ctx_file)) + + assert hc.read_binding_context() == ctx + + +def test_read_binding_context_raises_on_non_list(monkeypatch, tmp_path): + ctx_file = tmp_path / "ctx.json" + ctx_file.write_text(json.dumps({"not": "a list"}), encoding="utf-8") + monkeypatch.setenv("BINDING_CONTEXT_PATH", str(ctx_file)) + + with pytest.raises(ValueError, match="must be a list"): + hc.read_binding_context() + + +# --------------------------------------------------------------------------- +# snapshot_items +# --------------------------------------------------------------------------- + + +def test_snapshot_items_returns_items(): + contexts = [ + { + "binding": "schedule", + "snapshots": {"my-binding": [{"object": {"id": "1"}}]}, + } + ] + items = hc.snapshot_items(contexts, "my-binding") + assert items == [{"object": {"id": "1"}}] + + +def test_snapshot_items_returns_none_when_absent(): + contexts = [{"binding": "schedule", "snapshots": {"other": []}}] + assert hc.snapshot_items(contexts, "my-binding") is None + + +def test_snapshot_items_raises_on_non_list(): + contexts = [{"snapshots": {"my-binding": "not-a-list"}}] + with pytest.raises(ValueError, match="must be a list"): + hc.snapshot_items(contexts, "my-binding") + + +# --------------------------------------------------------------------------- +# synchronization_items +# --------------------------------------------------------------------------- + + +def test_synchronization_items_returns_objects(): + contexts = [ + { + "binding": "my-binding", + "type": "Synchronization", + "objects": [{"object": {"id": "1"}}], + } + ] + items = hc.synchronization_items(contexts, "my-binding") + assert items == [{"object": {"id": "1"}}] + + +def test_synchronization_items_returns_none_when_absent(): + contexts = [{"binding": "other", "type": "Synchronization", "objects": []}] + assert hc.synchronization_items(contexts, "my-binding") is None + + +def test_synchronization_items_raises_on_non_list(): + contexts = [{"binding": "my-binding", "type": "Synchronization", "objects": "bad"}] + with pytest.raises(ValueError, match="must be a list"): + hc.synchronization_items(contexts, "my-binding") + + +# --------------------------------------------------------------------------- +# utc_timestamp / truncate_message +# --------------------------------------------------------------------------- + + +def test_utc_timestamp_format(): + ts = hc.utc_timestamp() + assert ts.endswith("Z") + assert "T" in ts + + +def test_truncate_message_short(): + assert hc.truncate_message("hello") == "hello" + + +def test_truncate_message_exact_limit(): + msg = "x" * 2048 + assert hc.truncate_message(msg) == msg + + +def test_truncate_message_truncates(): + msg = "x" * 3000 + result = hc.truncate_message(msg) + assert len(result) == 2048 + assert result.endswith("...") + + +def test_truncate_message_custom_limit(): + result = hc.truncate_message("abcdefgh", max_length=5) + assert result == "ab..." + + +def _matching_status( + *, + sync_status: str = "Synced", + message: str = "ok", + generation: int | None = 1, +) -> dict: + condition_status = "True" if sync_status == "Synced" else "False" + reason = "ReconcileSucceeded" if sync_status == "Synced" else "ReconcileFailed" + status = { + "syncStatus": sync_status, + "lastSyncTime": "2026-08-19T06:20:21Z", + "message": message, + "conditions": [ + { + "type": "Synced", + "status": condition_status, + "reason": reason, + "message": message, + "lastTransitionTime": "2026-08-19T06:20:21Z", + } + ], + } + if generation is not None: + status["observedGeneration"] = generation + return status + + +def test_status_is_current_ignores_timestamps(): + current = _matching_status( + message="Successfully reconciled router flavor", + generation=3, + ) + + assert hc._status_is_current( + current, + "Synced", + "Successfully reconciled router flavor", + 3, + ) + + +@pytest.mark.parametrize( + ("current", "sync_status", "message", "generation"), + [ + (None, "Synced", "ok", 1), + ({}, "Synced", "ok", 1), + (_matching_status(sync_status="Failed"), "Synced", "ok", 1), + (_matching_status(message="old"), "Synced", "new", 1), + (_matching_status(generation=1), "Synced", "ok", 2), + ({**_matching_status(), "conditions": []}, "Synced", "ok", 1), + ], +) +def test_status_is_current_detects_real_status_differences( + current, + sync_status, + message, + generation, +): + assert not hc._status_is_current(current, sync_status, message, generation) + + +# --------------------------------------------------------------------------- +# patch_resource_status +# --------------------------------------------------------------------------- + + +def test_patch_resource_status_skips_when_disabled(): + with mock.patch("subprocess.run") as mock_run: + hc.patch_resource_status( + name="test-flavor", + namespace="openstack", + generation=1, + sync_status="Synced", + message="ok", + crd_resource="neutronrouterflavors.neutron.understack.rackspace.net", + crd_kind="NeutronRouterFlavor", + status_enabled=False, + ) + + mock_run.assert_not_called() + + +def test_patch_resource_status_calls_kubectl(): + with mock.patch("subprocess.run") as mock_run: + mock_run.return_value = mock.MagicMock(returncode=0) + hc.patch_resource_status( + name="test-flavor", + namespace="openstack", + generation=2, + sync_status="Synced", + message="all good", + crd_resource="neutronrouterflavors.neutron.understack.rackspace.net", + crd_kind="NeutronRouterFlavor", + status_enabled=True, + ) + + mock_run.assert_called_once() + cmd = mock_run.call_args[0][0] + assert "kubectl" in cmd + assert "test-flavor" in cmd + assert "-n" in cmd + assert "openstack" in cmd + + +def test_patch_resource_status_skips_when_current_status_matches(): + with mock.patch("subprocess.run") as mock_run: + hc.patch_resource_status( + name="test-flavor", + namespace="openstack", + generation=1, + sync_status="Synced", + message="ok", + crd_resource="neutronrouterflavors.neutron.understack.rackspace.net", + crd_kind="NeutronRouterFlavor", + status_enabled=True, + current_status=_matching_status(), + ) + + mock_run.assert_not_called() + + +def test_patch_resource_status_no_namespace(): + with mock.patch("subprocess.run") as mock_run: + mock_run.return_value = mock.MagicMock(returncode=0) + hc.patch_resource_status( + name="test-flavor", + namespace=None, + generation=None, + sync_status="Failed", + message="error", + crd_resource="neutronrouterflavors.neutron.understack.rackspace.net", + crd_kind="NeutronRouterFlavor", + status_enabled=True, + ) + + cmd = mock_run.call_args[0][0] + assert "-n" not in cmd + + +def test_patch_resource_status_logs_on_kubectl_not_found(caplog): + with mock.patch("subprocess.run", side_effect=FileNotFoundError): + with caplog.at_level(logging.WARNING, logger="openstack_sync.hooks.common"): + hc.patch_resource_status( + name="test-flavor", + namespace=None, + generation=None, + sync_status="Synced", + message="ok", + crd_resource="neutronrouterflavors.neutron.understack.rackspace.net", + crd_kind="NeutronRouterFlavor", + status_enabled=True, + ) + assert "kubectl not found" in caplog.text + + +def test_patch_resource_status_logs_on_kubectl_failure(caplog): + with mock.patch("subprocess.run") as mock_run: + mock_run.return_value = mock.MagicMock( + returncode=1, stderr="not found", stdout="" + ) + with caplog.at_level(logging.WARNING, logger="openstack_sync.hooks.common"): + hc.patch_resource_status( + name="test-flavor", + namespace="openstack", + generation=None, + sync_status="Synced", + message="ok", + crd_resource="neutronrouterflavors.neutron.understack.rackspace.net", + crd_kind="NeutronRouterFlavor", + status_enabled=True, + ) + assert "failed to patch" in caplog.text diff --git a/python/openstack-sync/tests/test_placeholder.py b/python/openstack-sync/tests/test_placeholder.py index 4df7cc7fe..a4105ecb6 100644 --- a/python/openstack-sync/tests/test_placeholder.py +++ b/python/openstack-sync/tests/test_placeholder.py @@ -1,22 +1,144 @@ -"""Tests for the openstack-sync placeholder hook.""" +"""Tests for the openstack-sync placeholder hook and shared utils.""" from __future__ import annotations import json from unittest import mock +import pytest + +import openstack_sync.utils as utils from openstack_sync.hooks import placeholder +FAKE_CLOUDS_YAML = """ +clouds: + understack: + auth: + auth_url: https://keystone.example.com/v3 + username: infrasetup + password: secret + project_name: baremetal + region_name: iad3 +""" + + +# --------------------------------------------------------------------------- +# placeholder hook config +# --------------------------------------------------------------------------- + def test_placeholder_hook_config(capsys): with mock.patch.object(placeholder.sys, "argv", ["placeholder.py", "--config"]): assert placeholder.main() == 0 config = json.loads(capsys.readouterr().out) - assert config == placeholder.HOOK_CONFIG + assert config == placeholder.build_hook_config() assert config["onStartup"] == 10 def test_placeholder_hook_run_is_noop(): with mock.patch.object(placeholder.sys, "argv", ["placeholder.py"]): assert placeholder.main() == 0 + + +# --------------------------------------------------------------------------- +# utils.read_secret_key +# --------------------------------------------------------------------------- + + +def test_read_secret_key_raises_on_missing_key(): + """read_secret_key propagates KeyError when the key is absent.""" + with mock.patch.object( + utils, "read_secret_key", side_effect=KeyError("clouds.yaml") + ): + with pytest.raises(KeyError): + utils.read_secret_key("infrasetup", "clouds.yaml", "openstack") + + +# --------------------------------------------------------------------------- +# utils.get_openstack_connection +# --------------------------------------------------------------------------- + + +def test_get_openstack_connection_reads_secret(monkeypatch): + """Connection is built from the named K8s secret via read_secret_key.""" + monkeypatch.setattr(utils, "_connection_cache", {}) + monkeypatch.setenv("POD_NAMESPACE", "openstack") + + fake_conn = mock.MagicMock(name="fake_conn") + + with mock.patch( + "openstack_sync.utils.openstack.connection.Connection", + return_value=fake_conn, + ): + with mock.patch.object(utils, "read_secret_key", return_value=FAKE_CLOUDS_YAML): + conn = utils.get_openstack_connection("infrasetup", "understack") + + assert conn is fake_conn + + +def test_get_openstack_connection_memoized(monkeypatch): + """Same (secret_name, cloud_name) returns cached connection.""" + monkeypatch.setattr(utils, "_connection_cache", {}) + monkeypatch.setenv("POD_NAMESPACE", "openstack") + + fake_conn = mock.MagicMock(name="fake_conn") + + with mock.patch( + "openstack_sync.utils.openstack.connection.Connection", + return_value=fake_conn, + ) as mock_conn: + with mock.patch.object(utils, "read_secret_key", return_value=FAKE_CLOUDS_YAML): + conn1 = utils.get_openstack_connection("infrasetup", "understack") + conn2 = utils.get_openstack_connection("infrasetup", "understack") + + assert conn1 is conn2 + mock_conn.assert_called_once() + + +def test_get_openstack_connection_separate_per_secret(monkeypatch): + """Different secrets produce independent connections.""" + monkeypatch.setattr(utils, "_connection_cache", {}) + monkeypatch.setenv("POD_NAMESPACE", "openstack") + + conn_a = mock.MagicMock(name="conn_a") + conn_b = mock.MagicMock(name="conn_b") + bm_yaml = FAKE_CLOUDS_YAML.replace("infrasetup", "baremetal-manage") + + def fake_read(secret_name, secret_key, namespace): + return FAKE_CLOUDS_YAML if secret_name == "infrasetup" else bm_yaml # noqa: S105 + + with mock.patch( + "openstack_sync.utils.openstack.connection.Connection", + side_effect=[conn_a, conn_b], + ): + with mock.patch.object(utils, "read_secret_key", side_effect=fake_read): + result_a = utils.get_openstack_connection("infrasetup", "understack") + result_b = utils.get_openstack_connection("baremetal-manage", "understack") + + assert result_a is conn_a + assert result_b is conn_b + + +# --------------------------------------------------------------------------- +# cloudCredentialsRef resolution (shared behaviour used by all hooks) +# --------------------------------------------------------------------------- + + +def test_get_openstack_connection_uses_per_resource_credentials(monkeypatch): + """Per-resource secretName/cloudName passed through to get_openstack_connection.""" + monkeypatch.setattr(utils, "_connection_cache", {}) + monkeypatch.setenv("POD_NAMESPACE", "openstack") + + fake_conn = mock.MagicMock(name="fake_conn") + + with mock.patch( + "openstack_sync.utils.openstack.connection.Connection", + return_value=fake_conn, + ): + with mock.patch.object( + utils, "read_secret_key", return_value=FAKE_CLOUDS_YAML + ) as mock_read: + utils.get_openstack_connection("baremetal-manage", "understack") + + mock_read.assert_called_once_with("baremetal-manage", "clouds.yaml", "openstack") diff --git a/python/openstack-sync/tests/test_plugins_common.py b/python/openstack-sync/tests/test_plugins_common.py new file mode 100644 index 000000000..8e4b2368f --- /dev/null +++ b/python/openstack-sync/tests/test_plugins_common.py @@ -0,0 +1,113 @@ +"""Tests for shared openstack-sync plugin utilities.""" + +from __future__ import annotations + +import pytest +from openstack import exceptions as sdk_exceptions +from openstack.network.v2 import flavor as sdk_flavor +from openstack.network.v2 import service_profile as sdk_service_profile + +from openstack_sync.plugins import common + + +def test_env_bool_accepts_only_lowercase_true_false(monkeypatch): + assert common.env_bool("OPENSTACK_SYNC_TEST_MISSING_TRUE", True) is True + assert common.env_bool("OPENSTACK_SYNC_TEST_MISSING_FALSE", False) is False + + monkeypatch.setenv("OPENSTACK_SYNC_TEST_BOOL", "true") + assert common.env_bool("OPENSTACK_SYNC_TEST_BOOL", False) is True + + monkeypatch.setenv("OPENSTACK_SYNC_TEST_BOOL", "false") + assert common.env_bool("OPENSTACK_SYNC_TEST_BOOL", True) is False + + +@pytest.mark.parametrize( + "value", + ["1", "0", "yes", "no", "on", "off", "TRUE", "FALSE", " true "], +) +def test_env_bool_rejects_boolean_aliases(monkeypatch, value): + monkeypatch.setenv("OPENSTACK_SYNC_TEST_BOOL", value) + + with pytest.raises(common.ConfigError, match="must be true or false"): + common.env_bool("OPENSTACK_SYNC_TEST_BOOL", False) + + +def test_get_value_reads_openstacksdk_attribute_names(): + profile = sdk_service_profile.ServiceProfile( + id="profile-id", + driver="neutron_understack.l3_router.vrf.Vrf", + metainfo={"vni_alloc": "auto"}, + ) + + assert common.resource_id(profile) == "profile-id" + assert common.get_value(profile, "driver") == "neutron_understack.l3_router.vrf.Vrf" + assert common.get_value(profile, "meta_info") == {"vni_alloc": "auto"} + + +def test_get_value_reads_exact_dict_keys_only(): + assert common.get_value( + {"meta_info": {"vni_alloc": "auto"}}, + "meta_info", + ) == {"vni_alloc": "auto"} + assert ( + common.get_value( + {"metainfo": {"vni_alloc": "auto"}}, + "meta_info", + default="missing", + ) + == "missing" + ) + + +def test_openstacksdk_maps_wire_names_to_attribute_names(): + profile = sdk_service_profile.ServiceProfile( + id="profile-id", + metainfo={"vni_alloc": "auto"}, + ) + flavor = sdk_flavor.Flavor( + id="flavor-id", + service_profiles=["profile-id"], + ) + + assert common.get_value(profile, "meta_info") == {"vni_alloc": "auto"} + assert common.service_profile_ids(flavor) == ["profile-id"] + + +def test_service_profile_ids_reads_openstacksdk_flavor(): + flavor = sdk_flavor.Flavor( + id="flavor-id", + service_profiles=["profile-1", "profile-2"], + ) + + assert common.service_profile_ids(flavor) == ["profile-1", "profile-2"] + + +def test_get_value_returns_default_for_missing_or_none_values(): + assert common.get_value({"name": None}, "name", default="fallback") == "fallback" + assert ( + common.get_value({"name": "router-flavor"}, "missing", default="fallback") + == "fallback" + ) + + +def test_sdk_not_found_and_conflict_are_independent(): + """reconcile.py and prune.py catch these in separate except clauses. + + If either became a subclass of the other, the first clause would swallow + both and, for example, a 409 "still in use" would be logged as "already + absent" while the resource stayed attached. + """ + assert not issubclass( + sdk_exceptions.ConflictException, sdk_exceptions.NotFoundException + ) + assert not issubclass( + sdk_exceptions.NotFoundException, sdk_exceptions.ConflictException + ) + + +def test_meta_info_payload_canonicalizes_json_strings(): + assert common.meta_info_payload('{"b": 2, "a": 1}') == '{"a":1,"b":2}' + + +def test_normalize_meta_info_leaves_non_json_strings_unchanged(): + assert common.normalize_meta_info("{'b': 2, 'a': 1}") == "{'b': 2, 'a': 1}" diff --git a/python/openstack-sync/tests/test_prune.py b/python/openstack-sync/tests/test_prune.py new file mode 100644 index 000000000..255c28977 --- /dev/null +++ b/python/openstack-sync/tests/test_prune.py @@ -0,0 +1,228 @@ +"""Tests for router flavor prune behaviour. + +Pruning is gated entirely on the operator's ownership markers, so these tests +are mostly about what must *not* be deleted. Whether pruning runs at all is the +hook's decision (``config.prune``), tested in ``test_framework.py``. +""" + +from __future__ import annotations + +import types +from types import SimpleNamespace +from typing import Any + +from openstack import exceptions as openstack_exceptions + +from openstack_sync.plugins.neutron.router_flavors import markers +from openstack_sync.plugins.neutron.router_flavors import prune +from openstack_sync.plugins.neutron.router_flavors.config import SERVICE_TYPE + +_DRIVER = "neutron_understack.l3_router.vrf.Vrf" + + +class FakeNetwork: + """Minimal Neutron network API recording flavor and profile deletes.""" + + def __init__(self, flavors: list[dict[str, Any]], profiles: dict[str, Any]): + self._flavors = flavors + self._profiles = profiles + self.deleted_flavors: list[str] = [] + self.deleted_profiles: list[str] = [] + self.flavor_list_calls = 0 + + def flavors(self, service_type: str | None = None) -> list[dict[str, Any]]: + self.flavor_list_calls += 1 + return [ + flavor + for flavor in self._flavors + if service_type is None or flavor["service_type"] == service_type + ] + + def routers(self, flavor_id: str) -> list[dict[str, Any]]: + return [] + + def service_profiles(self) -> list[Any]: + return [p for p in self._profiles.values() if p is not None] + + def get_service_profile(self, profile_id: str) -> Any: + profile = self._profiles.get(profile_id) + if profile is None: + raise openstack_exceptions.NotFoundException(f"no profile {profile_id}") + return profile + + def delete_flavor( + self, flavor: dict[str, Any], ignore_missing: bool = True + ) -> None: + self.deleted_flavors.append(flavor["id"]) + self._flavors = [f for f in self._flavors if f["id"] != flavor["id"]] + + def delete_service_profile(self, profile: Any, ignore_missing: bool = True) -> None: + profile_id = profile.id if hasattr(profile, "id") else profile["id"] + self.deleted_profiles.append(profile_id) + self._profiles[profile_id] = None + + +def _owned_profile(profile_id: str, driver: str = _DRIVER) -> Any: + return types.SimpleNamespace( + id=profile_id, + driver=driver, + meta_info=markers.managed_meta_info({"vni_alloc": "auto"}), + ) + + +def _owned_flavor( + flavor_id: str, name: str, service_profile_ids: list[str] | None = None +) -> dict[str, Any]: + return { + "id": flavor_id, + "name": name, + "service_type": SERVICE_TYPE, + "description": markers.managed_flavor_description("created by operator"), + "service_profile_ids": list(service_profile_ids or []), + } + + +def _conn(flavors: list[dict[str, Any]], profiles: dict[str, Any] | None = None) -> Any: + return SimpleNamespace(network=FakeNetwork(flavors, profiles or {})) + + +# --------------------------------------------------------------------------- +# Ownership gates deletion +# --------------------------------------------------------------------------- + + +def test_prune_keeps_unowned_flavor_even_with_owned_profile(): + flavor = { + "id": "manual-flavor-id", + "name": "manual-flavor", + "service_type": SERVICE_TYPE, + "description": "created outside the operator", + "service_profile_ids": ["owned-profile-id"], + } + profile = _owned_profile("owned-profile-id") + conn = _conn([flavor], {profile.id: profile}) + + prune.prune_removed_flavors(conn, [{"name": "kept-flavor"}]) + + assert conn.network.deleted_flavors == [] + + +def test_prune_deletes_removed_owned_flavor(): + conn = _conn([_owned_flavor("managed-flavor-id", "removed-managed-flavor")]) + + prune.prune_removed_flavors(conn, [{"name": "kept-flavor"}]) + + assert conn.network.deleted_flavors == ["managed-flavor-id"] + + +def test_prune_deletes_removed_flavor_and_its_unused_profile(): + profile = _owned_profile("managed-profile-id") + flavor = _owned_flavor("managed-flavor-id", "removed-managed-flavor", [profile.id]) + conn = _conn([flavor], {profile.id: profile}) + + prune.prune_removed_flavors(conn, [{"name": "kept-flavor"}]) + + assert conn.network.deleted_flavors == ["managed-flavor-id"] + assert conn.network.deleted_profiles == ["managed-profile-id"] + + +# --------------------------------------------------------------------------- +# The empty-desired guard +# --------------------------------------------------------------------------- + + +def test_prune_keeps_owned_flavors_when_desired_list_is_empty(): + """An empty desired set may be an unreadable snapshot, not a deletion.""" + conn = _conn([_owned_flavor("managed-flavor-id", "removed-managed-flavor")]) + + prune.prune_removed_flavors(conn, []) + + assert conn.network.deleted_flavors == [] + + +def test_prune_deletes_when_empty_desired_is_authoritative(): + """A confirmed CR deletion makes the empty desired set actionable.""" + conn = _conn([_owned_flavor("managed-flavor-id", "removed-managed-flavor")]) + + prune.prune_removed_flavors(conn, [], authoritative_empty=True) + + assert conn.network.deleted_flavors == ["managed-flavor-id"] + + +# --------------------------------------------------------------------------- +# Orphaned profile sweep +# --------------------------------------------------------------------------- + + +def test_prune_deletes_orphaned_owned_profile(): + """A profile whose parent flavor is already gone is collected.""" + orphan = _owned_profile("orphan-profile-id") + conn = _conn([], {orphan.id: orphan}) + + prune.prune_removed_flavors(conn, [{"name": "kept-flavor"}]) + + assert conn.network.deleted_profiles == ["orphan-profile-id"] + + +def test_prune_keeps_unowned_profile(): + """A profile without the ownership marker is never touched.""" + unowned = types.SimpleNamespace( + id="unmanaged-profile-id", + driver=_DRIVER, + meta_info={"vni_alloc": "auto"}, # no ownership marker + ) + conn = _conn([], {unowned.id: unowned}) + + prune.prune_removed_flavors(conn, [{"name": "kept-flavor"}]) + + assert conn.network.deleted_profiles == [] + + +def test_prune_keeps_attached_profile(): + """A profile still bound to a surviving flavor is kept.""" + attached = _owned_profile("attached-profile-id") + kept = _owned_flavor("kept-flavor-id", "kept-flavor", [attached.id]) + conn = _conn([kept], {attached.id: attached}) + + prune.prune_removed_flavors(conn, [{"name": "kept-flavor"}]) + + assert conn.network.deleted_flavors == [] + assert conn.network.deleted_profiles == [] + + +def test_prune_lists_flavors_once_for_all_profile_checks(): + """Attachment counts come from a single flavor listing, not one per profile.""" + removed_profile = _owned_profile("removed-profile-id") + orphan_profile = _owned_profile("orphan-profile-id") + attached_profile = _owned_profile("attached-profile-id") + conn = _conn( + [ + _owned_flavor("removed-flavor-id", "removed-flavor", [removed_profile.id]), + _owned_flavor("kept-flavor-id", "kept-flavor", [attached_profile.id]), + ], + { + removed_profile.id: removed_profile, + orphan_profile.id: orphan_profile, + attached_profile.id: attached_profile, + }, + ) + + prune.prune_removed_flavors(conn, [{"name": "kept-flavor"}]) + + assert conn.network.flavor_list_calls == 1 + assert conn.network.deleted_flavors == ["removed-flavor-id"] + assert conn.network.deleted_profiles == [ + "removed-profile-id", + "orphan-profile-id", + ] + + +def test_prune_skips_flavor_still_used_by_routers(): + """A flavor with routers attached is never deleted.""" + flavor = _owned_flavor("in-use-flavor-id", "removed-flavor") + conn = _conn([flavor]) + conn.network.routers = lambda flavor_id: [{"id": "router-1"}] + + prune.prune_removed_flavors(conn, [{"name": "kept-flavor"}]) + + assert conn.network.deleted_flavors == [] diff --git a/python/openstack-sync/tests/test_reconcile.py b/python/openstack-sync/tests/test_reconcile.py new file mode 100644 index 000000000..357681b51 --- /dev/null +++ b/python/openstack-sync/tests/test_reconcile.py @@ -0,0 +1,758 @@ +"""Tests for router flavor reconciliation. + +Covers the profile cache, create-or-reuse by ``(driver, meta_info)``, the +owned-only reuse rule, drift reporting on reused profiles, the flavor +``service_type`` guard and ``is_enabled``/description reconcile, and the +flavor-to-profile binding set. +""" + +from __future__ import annotations + +import logging +import types +from typing import Any +from unittest import mock + +import pytest + +from openstack_sync.plugins import common as plugin_common +from openstack_sync.plugins.common import ConfigError +from openstack_sync.plugins.neutron.router_flavors import markers +from openstack_sync.plugins.neutron.router_flavors import reconcile +from openstack_sync.plugins.neutron.router_flavors.config import SERVICE_TYPE + +_DRIVER = "neutron_understack.l3_router.vrf.Vrf" +_NAME = "test-flavor" +_DESCRIPTION = "my flavor" + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_profile( + profile_id: str, + driver: str = _DRIVER, + meta_info: Any = None, + managed: bool = True, + is_enabled: bool = True, + description: str = "desc", +) -> Any: + """Build an openstacksdk-shaped service profile. + + ``description`` defaults to the ``_profile_spec`` default so a profile and a + spec built with defaults are drift-free; drift tests pass a mismatch. + """ + raw_meta = dict(meta_info or {}) + if managed: + raw_meta.update(markers.OPERATOR_META_INFO_MARKERS) + return types.SimpleNamespace( + id=profile_id, + driver=driver, + is_enabled=is_enabled, + description=description, + meta_info=plugin_common.meta_info_payload(raw_meta), + ) + + +def _profile_spec( + driver: str = _DRIVER, + description: str = "desc", + meta_info: dict[str, Any] | None = None, + is_enabled: bool = True, +) -> dict[str, Any]: + return { + "driver": driver, + "description": description, + "meta_info": meta_info if meta_info is not None else {}, + "is_enabled": is_enabled, + } + + +def _make_flavor( + flavor_id: str = "flavor-id", + name: str = _NAME, + service_profile_ids: list[str] | None = None, + service_type: str = SERVICE_TYPE, + description: str = f"{_DESCRIPTION} {markers.FLAVOR_DESCRIPTION_MARKER}", + is_enabled: bool = True, +) -> Any: + return types.SimpleNamespace( + id=flavor_id, + name=name, + service_type=service_type, + description=description, + is_enabled=is_enabled, + service_profile_ids=list(service_profile_ids or []), + ) + + +def _flavor_spec( + name: str = _NAME, + description: str = _DESCRIPTION, + service_type: str = SERVICE_TYPE, + is_enabled: bool = True, + service_profiles: list[dict[str, Any]] | None = None, +) -> dict[str, Any]: + """Build a CR spec as the API server materialises it (defaults applied).""" + return { + "name": name, + "description": description, + "service_type": service_type, + "is_enabled": is_enabled, + "service_profiles": ( + service_profiles if service_profiles is not None else [_profile_spec()] + ), + } + + +def _reuse_conn(profile: Any) -> Any: + """A connection whose only existing service profile is *profile*.""" + network = mock.MagicMock() + network.service_profiles.return_value = [profile] + return types.SimpleNamespace(network=network) + + +def _create_conn(created: Any, existing: list[Any] | None = None) -> Any: + network = mock.MagicMock() + network.service_profiles.return_value = list(existing or []) + network.create_service_profile.return_value = created + return types.SimpleNamespace(network=network) + + +def _bindings_conn(flavor: Any, lookup: dict[str, Any] | None = None) -> Any: + """A connection for binding tests. + + ``get_flavor`` returns *flavor*; ``get_service_profile`` resolves unbind + candidates from *lookup* so ownership can be evaluated. + """ + resolved = lookup or {} + network = mock.MagicMock() + network.get_flavor.return_value = flavor + network.get_service_profile.side_effect = lambda pid: resolved.get(pid) + return types.SimpleNamespace(network=network) + + +# --------------------------------------------------------------------------- +# Profile cache +# --------------------------------------------------------------------------- + + +def test_profiles_for_driver_queries_by_driver(): + network = mock.MagicMock() + network.service_profiles.return_value = [_make_profile("profile-id")] + conn = types.SimpleNamespace(network=network) + + result = reconcile.profiles_for_driver(conn, "some.Driver", {}) + + assert result == list(network.service_profiles.return_value) + network.service_profiles.assert_called_once_with(driver="some.Driver") + + +def test_profiles_for_driver_caches_per_driver(): + first = _make_profile("first-profile", driver="first.Driver") + second = _make_profile("second-profile", driver="second.Driver") + network = mock.MagicMock() + network.service_profiles.side_effect = [[first], [second]] + conn = types.SimpleNamespace(network=network) + cache: reconcile.ProfileCache = {} + + first_result = reconcile.profiles_for_driver(conn, "first.Driver", cache) + cached = reconcile.profiles_for_driver(conn, "first.Driver", cache) + second_result = reconcile.profiles_for_driver(conn, "second.Driver", cache) + + assert first_result == [first] + assert cached is first_result + assert second_result == [second] + assert network.service_profiles.call_args_list == [ + mock.call(driver="first.Driver"), + mock.call(driver="second.Driver"), + ] + + +# --------------------------------------------------------------------------- +# ensure_profile: create or reuse by (driver, meta_info) +# --------------------------------------------------------------------------- + + +def test_ensure_profile_creates_with_ownership_markers(): + conn = _create_conn(_make_profile("new-profile")) + + reconcile.ensure_profile( + conn, _NAME, _profile_spec(meta_info={"vni_alloc": "auto"}), {}, [] + ) + + kwargs = conn.network.create_service_profile.call_args.kwargs + assert kwargs["driver"] == _DRIVER + assert kwargs["is_enabled"] is True + meta_info = plugin_common.normalize_meta_info(kwargs["meta_info"]) + assert meta_info["vni_alloc"] == "auto" + for key, value in markers.OPERATOR_META_INFO_MARKERS.items(): + assert meta_info[key] == value + + +def test_ensure_profile_creates_disabled_when_spec_disables(): + conn = _create_conn(_make_profile("new-profile", is_enabled=False)) + + reconcile.ensure_profile(conn, _NAME, _profile_spec(is_enabled=False), {}, []) + + assert conn.network.create_service_profile.call_args.kwargs["is_enabled"] is False + + +def test_ensure_profile_reuses_existing_owned_profile(): + meta_info = {"vni_alloc": "auto"} + existing = _make_profile("existing-profile", meta_info=meta_info) + conn = _reuse_conn(existing) + + result = reconcile.ensure_profile( + conn, _NAME, _profile_spec(meta_info=meta_info), {}, [] + ) + + assert result is existing + conn.network.create_service_profile.assert_not_called() + + +def test_ensure_profile_appends_created_profile_to_driver_cache(): + """A profile created for one flavor must be visible to the next flavor. + + The cache is shared across all flavors in a credential group, so two + flavors with an identical ``(driver, meta_info)`` spec share one profile + rather than each creating a duplicate. + """ + meta_info = {"vni_alloc": "auto"} + created = _make_profile("new-profile", meta_info=meta_info) + conn = _create_conn(created) + cache: reconcile.ProfileCache = {} + + first = reconcile.ensure_profile( + conn, "flavor-a", _profile_spec(meta_info=meta_info), cache, [] + ) + second = reconcile.ensure_profile( + conn, "flavor-b", _profile_spec(meta_info=meta_info), cache, [] + ) + + assert first is created + assert second is first + conn.network.create_service_profile.assert_called_once() + conn.network.service_profiles.assert_called_once_with(driver=_DRIVER) + + +def test_ensure_profile_does_not_reuse_across_drivers(): + meta_info = {"vni_alloc": "auto"} + first_profile = _make_profile("first-profile", driver="first.Driver") + second_profile = _make_profile("second-profile", driver="second.Driver") + network = mock.MagicMock() + network.service_profiles.side_effect = [[], []] + network.create_service_profile.side_effect = [first_profile, second_profile] + conn = types.SimpleNamespace(network=network) + cache: reconcile.ProfileCache = {} + + first = reconcile.ensure_profile( + conn, + "flavor-a", + _profile_spec(driver="first.Driver", meta_info=meta_info), + cache, + [], + ) + second = reconcile.ensure_profile( + conn, + "flavor-b", + _profile_spec(driver="second.Driver", meta_info=meta_info), + cache, + [], + ) + + assert first is first_profile + assert second is second_profile + assert network.create_service_profile.call_count == 2 + + +# --------------------------------------------------------------------------- +# Only operator-owned profiles are reused +# --------------------------------------------------------------------------- + + +def test_find_matching_profile_ignores_unowned_match(): + meta_info = {"vni_alloc": "auto"} + unowned = _make_profile("adhoc-profile", meta_info=meta_info, managed=False) + + assert reconcile.find_matching_profile([unowned], meta_info) is None + + +def test_find_matching_profile_prefers_owned_over_unowned(): + meta_info = {"vni_alloc": "auto"} + unowned = _make_profile("adhoc-profile", meta_info=meta_info, managed=False) + owned = _make_profile("owned-profile", meta_info=meta_info) + + assert reconcile.find_matching_profile([unowned, owned], meta_info) is owned + + +def test_ensure_profile_creates_owned_profile_instead_of_reusing_unowned(): + """An unowned profile must never be bound, because it can never be unbound. + + ``reconcile_flavor_profiles`` only unbinds profiles carrying the ownership + marker, so reusing somebody else's profile would create a binding that + outlives the spec that created it and that nothing can ever remove. + """ + meta_info = {"vni_alloc": "auto"} + unowned = _make_profile("adhoc-profile", meta_info=meta_info, managed=False) + created = _make_profile("new-profile", meta_info=meta_info) + conn = _create_conn(created, existing=[unowned]) + + result = reconcile.ensure_profile( + conn, _NAME, _profile_spec(meta_info=meta_info), {}, [] + ) + + assert result is created + conn.network.create_service_profile.assert_called_once() + new_meta = plugin_common.normalize_meta_info( + conn.network.create_service_profile.call_args.kwargs["meta_info"] + ) + for key, value in markers.OPERATOR_META_INFO_MARKERS.items(): + assert new_meta[key] == value + + +def test_ensure_profile_never_adopts_an_unowned_profile(): + """The unowned profile is left alone, not stamped with the marker. + + Adopting it would enrol somebody else's profile into the prune sweep, which + deletes owned unattached profiles -- an irreversible side effect on a + resource the operator did not create. + """ + meta_info = {"vni_alloc": "auto"} + unowned = _make_profile("adhoc-profile", meta_info=meta_info, managed=False) + conn = _create_conn(_make_profile("new-profile"), existing=[unowned]) + + reconcile.ensure_profile(conn, _NAME, _profile_spec(meta_info=meta_info), {}, []) + + conn.network.update_service_profile.assert_not_called() + conn.network.delete_service_profile.assert_not_called() + + +def test_ensure_profile_reuses_owned_when_unowned_match_also_exists(): + meta_info = {"vni_alloc": "auto"} + unowned = _make_profile("adhoc-profile", meta_info=meta_info, managed=False) + owned = _make_profile("owned-profile", meta_info=meta_info) + network = mock.MagicMock() + network.service_profiles.return_value = [unowned, owned] + conn = types.SimpleNamespace(network=network) + + result = reconcile.ensure_profile( + conn, _NAME, _profile_spec(meta_info=meta_info), {}, [] + ) + + assert result is owned + network.create_service_profile.assert_not_called() + + +def test_profile_created_beside_unowned_match_can_later_be_unbound(): + """End-to-end guard for why unowned profiles are not reused. + + Resolve a profile while an unowned match exists, then reconcile the flavor + against a different spec. The profile bound earlier must be unbindable, + which holds only because the operator created and owns it. + """ + meta_info = {"vni_alloc": "auto"} + unowned = _make_profile("adhoc-profile", meta_info=meta_info, managed=False) + created = _make_profile("prof-a", meta_info=meta_info) + conn = _create_conn(created, existing=[unowned]) + + bound = reconcile.ensure_profile( + conn, _NAME, _profile_spec(meta_info=meta_info), {}, [] + ) + + flavor = _make_flavor(service_profile_ids=["prof-a"]) + bindings = _bindings_conn(flavor, {"prof-a": bound}) + + reconcile.reconcile_flavor_profiles(bindings, flavor, [_make_profile("prof-b")]) + + disassociate = bindings.network.disassociate_flavor_from_service_profile + disassociate.assert_called_once() + assert disassociate.call_args.args[1] is bound + + +# --------------------------------------------------------------------------- +# Drift reporting on reused profiles +# --------------------------------------------------------------------------- + + +def test_ensure_profile_reports_is_enabled_drift_on_reuse(caplog): + """A profile disabled out-of-band is reported, not silently accepted. + + Neutron's get_flavor_next_provider raises ServiceProfileDisabled for the + profile it selects, so every router create against the flavor fails while + the flavor itself still looks converged. + """ + existing = _make_profile("owned-profile", is_enabled=False) + conn = _reuse_conn(existing) + drift: list[reconcile.ProfileDrift] = [] + + with caplog.at_level(logging.WARNING): + result = reconcile.ensure_profile( + conn, _NAME, _profile_spec(is_enabled=True), {}, drift + ) + + assert result is existing + assert [(d.field, d.have, d.want) for d in drift] == [("is_enabled", False, True)] + assert drift[0].profile_id == "owned-profile" + assert "is_enabled" in caplog.text + # Neutron rejects updates to a profile bound to any flavor: never try one. + conn.network.update_service_profile.assert_not_called() + + +def test_ensure_profile_reports_description_drift_on_reuse(): + existing = _make_profile("owned-profile", description="stale") + conn = _reuse_conn(existing) + drift: list[reconcile.ProfileDrift] = [] + + reconcile.ensure_profile( + conn, _NAME, _profile_spec(description="wanted"), {}, drift + ) + + assert [(d.field, d.have, d.want) for d in drift] == [ + ("description", "stale", "wanted") + ] + + +def test_ensure_profile_reports_every_drifted_field(): + existing = _make_profile("owned-profile", is_enabled=False, description="stale") + conn = _reuse_conn(existing) + drift: list[reconcile.ProfileDrift] = [] + + reconcile.ensure_profile( + conn, _NAME, _profile_spec(description="wanted", is_enabled=True), {}, drift + ) + + assert sorted(d.field for d in drift) == ["description", "is_enabled"] + + +def test_ensure_profile_accumulates_drift_across_profiles(): + """sync_flavor passes one list across every profile in the spec.""" + existing = _make_profile("owned-profile", is_enabled=False) + conn = _reuse_conn(existing) + already = reconcile.ProfileDrift( + profile_id="other", field="is_enabled", have=False, want=True + ) + drift = [already] + + reconcile.ensure_profile(conn, _NAME, _profile_spec(is_enabled=True), {}, drift) + + assert len(drift) == 2 + assert drift[0] is already + + +def test_ensure_profile_reports_no_drift_when_profile_matches_spec(): + conn = _reuse_conn(_make_profile("owned-profile")) + drift: list[reconcile.ProfileDrift] = [] + + reconcile.ensure_profile(conn, _NAME, _profile_spec(), {}, drift) + + assert drift == [] + + +def test_ensure_profile_reports_no_drift_for_freshly_created_profile(): + """A profile the operator just created from the spec cannot have drifted.""" + conn = _create_conn(_make_profile("new-profile", is_enabled=False)) + drift: list[reconcile.ProfileDrift] = [] + + reconcile.ensure_profile(conn, _NAME, _profile_spec(is_enabled=False), {}, drift) + + assert drift == [] + + +def test_profile_drift_describe_names_profile_and_field(): + drift = reconcile.ProfileDrift( + profile_id="prof-a", field="is_enabled", have=False, want=True + ) + + described = drift.describe() + + assert "prof-a" in described + assert "is_enabled" in described + assert "False" in described and "True" in described + + +# --------------------------------------------------------------------------- +# ensure_flavor: service_type is immutable +# --------------------------------------------------------------------------- + + +def test_ensure_flavor_raises_on_service_type_mismatch(): + flavor = _make_flavor(service_type="DIFFERENT_TYPE") + conn = mock.MagicMock() + conn.network.flavors.return_value = [flavor] + + with pytest.raises(ConfigError, match="service_type"): + reconcile.ensure_flavor(conn, _flavor_spec()) + + +def test_ensure_flavor_error_message_contains_both_service_types(): + flavor = _make_flavor(service_type="WRONG") + conn = mock.MagicMock() + conn.network.flavors.return_value = [flavor] + + with pytest.raises(ConfigError) as exc_info: + reconcile.ensure_flavor(conn, _flavor_spec()) + + message = str(exc_info.value) + assert "WRONG" in message + assert SERVICE_TYPE in message + assert _NAME in message + + +# --------------------------------------------------------------------------- +# ensure_flavor: is_enabled and description reconcile +# --------------------------------------------------------------------------- + + +def _existing_flavor_conn(flavor: Any, updated: Any | None = None) -> Any: + conn = mock.MagicMock() + conn.network.flavors.return_value = [flavor] + conn.network.update_flavor.return_value = updated or _make_flavor() + return conn + + +def test_ensure_flavor_reenables_disabled_flavor(caplog): + conn = _existing_flavor_conn(_make_flavor(is_enabled=False)) + + with caplog.at_level(logging.INFO, logger="openstack_sync"): + reconcile.ensure_flavor(conn, _flavor_spec(is_enabled=True)) + + conn.network.update_flavor.assert_called_once() + assert conn.network.update_flavor.call_args.kwargs["is_enabled"] is True + assert "is_enabled drift" in caplog.text + assert "have=False" in caplog.text + assert "want=True" in caplog.text + + +def test_ensure_flavor_disables_when_spec_disables(caplog): + conn = _existing_flavor_conn(_make_flavor(is_enabled=True)) + + with caplog.at_level(logging.INFO, logger="openstack_sync"): + reconcile.ensure_flavor(conn, _flavor_spec(is_enabled=False)) + + conn.network.update_flavor.assert_called_once() + assert conn.network.update_flavor.call_args.kwargs["is_enabled"] is False + assert "have=True" in caplog.text + assert "want=False" in caplog.text + + +def test_ensure_flavor_no_update_when_both_disabled(): + flavor = _make_flavor(is_enabled=False) + conn = _existing_flavor_conn(flavor) + + result = reconcile.ensure_flavor(conn, _flavor_spec(is_enabled=False)) + + conn.network.update_flavor.assert_not_called() + assert result is flavor + + +def test_ensure_flavor_no_update_when_already_correct(): + flavor = _make_flavor(is_enabled=True) + conn = _existing_flavor_conn(flavor) + + result = reconcile.ensure_flavor(conn, _flavor_spec(is_enabled=True)) + + conn.network.update_flavor.assert_not_called() + assert result is flavor + + +def test_ensure_flavor_reenables_even_when_description_matches(): + """is_enabled drift must trigger an update even if the description is current.""" + conn = _existing_flavor_conn(_make_flavor(is_enabled=False)) + + reconcile.ensure_flavor(conn, _flavor_spec(is_enabled=True)) + + conn.network.update_flavor.assert_called_once() + + +def test_ensure_flavor_updates_changed_description(): + conn = _existing_flavor_conn(_make_flavor(description="old description")) + + reconcile.ensure_flavor(conn, _flavor_spec()) + + conn.network.update_flavor.assert_called_once() + + +def test_ensure_flavor_adds_missing_marker(): + conn = _existing_flavor_conn(_make_flavor(description="no marker here")) + + reconcile.ensure_flavor(conn, _flavor_spec()) + + conn.network.update_flavor.assert_called_once() + kwargs = conn.network.update_flavor.call_args.kwargs + assert markers.FLAVOR_DESCRIPTION_MARKER in kwargs["description"] + + +# --------------------------------------------------------------------------- +# ensure_flavor: creates when absent +# --------------------------------------------------------------------------- + + +def test_ensure_flavor_creates_when_not_found(): + conn = mock.MagicMock() + conn.network.flavors.return_value = [] + conn.network.create_flavor.return_value = _make_flavor() + + reconcile.ensure_flavor(conn, _flavor_spec()) + + kwargs = conn.network.create_flavor.call_args.kwargs + assert kwargs["name"] == _NAME + assert kwargs["service_type"] == SERVICE_TYPE + assert kwargs["is_enabled"] is True + assert markers.FLAVOR_DESCRIPTION_MARKER in kwargs["description"] + + +def test_ensure_flavor_creates_disabled_from_spec(): + """A CR that opts out of enabled must create the Neutron flavor disabled.""" + conn = mock.MagicMock() + conn.network.flavors.return_value = [] + conn.network.create_flavor.return_value = _make_flavor(is_enabled=False) + + reconcile.ensure_flavor(conn, _flavor_spec(is_enabled=False)) + + assert conn.network.create_flavor.call_args.kwargs["is_enabled"] is False + + +def test_find_flavor_ignores_partial_name_match(): + """Guards against a future change to substring query semantics.""" + conn = mock.MagicMock() + conn.network.flavors.return_value = [_make_flavor(name="test-flavor-other")] + + assert reconcile.find_flavor(conn, _NAME) is None + + +# --------------------------------------------------------------------------- +# reconcile_flavor_profiles: bind missing, unbind owned extras +# --------------------------------------------------------------------------- + + +def test_reconcile_flavor_profiles_no_op_when_matching(): + flavor = _make_flavor(service_profile_ids=["prof-a", "prof-b"]) + conn = _bindings_conn(flavor) + + result = reconcile.reconcile_flavor_profiles( + conn, flavor, [_make_profile("prof-a"), _make_profile("prof-b")] + ) + + conn.network.associate_flavor_with_service_profile.assert_not_called() + conn.network.disassociate_flavor_from_service_profile.assert_not_called() + assert result is flavor + + +def test_reconcile_flavor_profiles_binds_missing(): + flavor = _make_flavor(service_profile_ids=[]) + conn = _bindings_conn(flavor) + + reconcile.reconcile_flavor_profiles( + conn, flavor, [_make_profile("prof-a"), _make_profile("prof-b")] + ) + + calls = conn.network.associate_flavor_with_service_profile.call_args_list + assert sorted(call.args[1].id for call in calls) == ["prof-a", "prof-b"] + conn.network.disassociate_flavor_from_service_profile.assert_not_called() + + +def test_reconcile_flavor_profiles_unbinds_owned_extra(): + flavor = _make_flavor(service_profile_ids=["prof-a", "prof-extra"]) + extra = _make_profile("prof-extra") + conn = _bindings_conn(flavor, {"prof-extra": extra}) + + reconcile.reconcile_flavor_profiles(conn, flavor, [_make_profile("prof-a")]) + + conn.network.associate_flavor_with_service_profile.assert_not_called() + conn.network.disassociate_flavor_from_service_profile.assert_called_once() + assert ( + conn.network.disassociate_flavor_from_service_profile.call_args.args[1] is extra + ) + + +def test_reconcile_flavor_profiles_keeps_unowned_extra(): + """A profile attached out-of-band must not be unbound.""" + flavor = _make_flavor(service_profile_ids=["prof-a", "prof-adhoc"]) + unowned = _make_profile("prof-adhoc", managed=False) + conn = _bindings_conn(flavor, {"prof-adhoc": unowned}) + + reconcile.reconcile_flavor_profiles(conn, flavor, [_make_profile("prof-a")]) + + conn.network.disassociate_flavor_from_service_profile.assert_not_called() + + +def test_reconcile_flavor_profiles_binds_and_unbinds_together(): + flavor = _make_flavor(service_profile_ids=["prof-old"]) + old = _make_profile("prof-old") + conn = _bindings_conn(flavor, {"prof-old": old}) + + reconcile.reconcile_flavor_profiles(conn, flavor, [_make_profile("prof-new")]) + + conn.network.associate_flavor_with_service_profile.assert_called_once() + assert ( + conn.network.associate_flavor_with_service_profile.call_args.args[1].id + == "prof-new" + ) + conn.network.disassociate_flavor_from_service_profile.assert_called_once() + assert ( + conn.network.disassociate_flavor_from_service_profile.call_args.args[1] is old + ) + + +def test_reconcile_flavor_profiles_skips_deleted_extra(): + """An unbind candidate that no longer exists is a silent no-op.""" + flavor = _make_flavor(service_profile_ids=["prof-a", "prof-gone"]) + conn = _bindings_conn(flavor, {"prof-gone": None}) + + reconcile.reconcile_flavor_profiles(conn, flavor, [_make_profile("prof-a")]) + + conn.network.disassociate_flavor_from_service_profile.assert_not_called() + + +# --------------------------------------------------------------------------- +# sync_flavor +# --------------------------------------------------------------------------- + + +def _sync_conn(flavor: Any, existing_profiles: list[Any] | None = None) -> Any: + conn = mock.MagicMock() + conn.network.flavors.return_value = [flavor] + conn.network.get_flavor.return_value = flavor + conn.network.service_profiles.return_value = list(existing_profiles or []) + conn.network.create_service_profile.return_value = _make_profile("prof-a") + return conn + + +def test_sync_flavor_returns_no_notes_when_nothing_drifted(): + flavor = _make_flavor(service_profile_ids=["prof-a"]) + conn = _sync_conn(flavor) + + assert reconcile.sync_flavor(conn, _flavor_spec(), {}) == [] + + +def test_sync_flavor_reports_profile_drift(): + """Drift found while resolving profiles reaches the caller as notes. + + The flavor itself is converged, so this is not a failure -- but the caller + must be able to qualify the status it reports. + """ + flavor = _make_flavor(service_profile_ids=["owned-profile"]) + drifted_profile = _make_profile("owned-profile", is_enabled=False) + conn = _sync_conn(flavor, existing_profiles=[drifted_profile]) + + notes = reconcile.sync_flavor( + conn, _flavor_spec(service_profiles=[_profile_spec(is_enabled=True)]), {} + ) + + assert len(notes) == 1 + assert "owned-profile" in notes[0] + assert "is_enabled" in notes[0] + + +def test_sync_flavor_passes_is_enabled_from_spec(): + """The value the API server put on the CR reaches the Neutron flavor.""" + flavor = _make_flavor(is_enabled=True, service_profile_ids=["prof-a"]) + conn = _sync_conn(flavor) + conn.network.update_flavor.return_value = flavor + + reconcile.sync_flavor(conn, _flavor_spec(is_enabled=False), {}) + + conn.network.update_flavor.assert_called_once() + assert conn.network.update_flavor.call_args.kwargs["is_enabled"] is False diff --git a/python/openstack-sync/tests/test_router_flavors.py b/python/openstack-sync/tests/test_router_flavors.py deleted file mode 100644 index c500c90ab..000000000 --- a/python/openstack-sync/tests/test_router_flavors.py +++ /dev/null @@ -1,349 +0,0 @@ -"""Tests for the Neutron router flavors hook.""" - -from __future__ import annotations - -import json -from unittest import mock - -import pytest - -import openstack_sync.utils as k8s_module -from openstack_sync.hooks import router_flavors - -# --------------------------------------------------------------------------- -# Helpers -# --------------------------------------------------------------------------- - - -def clear_router_flavor_env(monkeypatch): - monkeypatch.delenv("NEUTRON_ROUTER_FLAVOR_ENABLED", raising=False) - monkeypatch.delenv("NEUTRON_ROUTER_FLAVOR_NAMESPACE", raising=False) - monkeypatch.delenv("NEUTRON_ROUTER_FLAVOR_SYNC_CRONTAB", raising=False) - monkeypatch.delenv("POD_NAMESPACE", raising=False) - - -FAKE_CLOUDS_YAML = """ -clouds: - understack: - auth: - auth_url: https://keystone.example.com/v3 - username: infrasetup - password: secret - project_name: baremetal - region_name: iad3 -""" - - -# --------------------------------------------------------------------------- -# build_hook_config -# --------------------------------------------------------------------------- - - -def test_router_flavor_hook_config_disabled(monkeypatch): - clear_router_flavor_env(monkeypatch) - - config = router_flavors.build_hook_config() - - assert config["onStartup"] == 10 - assert "kubernetes" not in config - assert "schedule" not in config - - -def test_router_flavor_hook_config_uses_pod_namespace(monkeypatch): - clear_router_flavor_env(monkeypatch) - monkeypatch.setenv("NEUTRON_ROUTER_FLAVOR_ENABLED", "true") - monkeypatch.setenv("POD_NAMESPACE", "openstack") - - config = router_flavors.build_hook_config() - - kubernetes_binding = config["kubernetes"][0] - assert kubernetes_binding["namespace"] == { - "nameSelector": { - "matchNames": ["openstack"], - }, - } - assert config["schedule"][0]["crontab"] == "0 * * * *" - assert "onStartup" not in config - - -def test_router_flavor_hook_config_namespace_override(monkeypatch): - clear_router_flavor_env(monkeypatch) - monkeypatch.setenv("NEUTRON_ROUTER_FLAVOR_ENABLED", "true") - monkeypatch.setenv("NEUTRON_ROUTER_FLAVOR_NAMESPACE", "custom") - monkeypatch.setenv("POD_NAMESPACE", "openstack") - - config = router_flavors.build_hook_config() - - kubernetes_binding = config["kubernetes"][0] - assert kubernetes_binding["namespace"]["nameSelector"]["matchNames"] == ["custom"] - - -def test_router_flavor_hook_config_output_uses_runtime_environment(monkeypatch, capsys): - clear_router_flavor_env(monkeypatch) - monkeypatch.setenv("NEUTRON_ROUTER_FLAVOR_ENABLED", "true") - monkeypatch.setenv("POD_NAMESPACE", "openstack") - monkeypatch.setenv("NEUTRON_ROUTER_FLAVOR_SYNC_CRONTAB", "*/15 * * * *") - - with mock.patch.object( - router_flavors.sys, "argv", ["router_flavors.py", "--config"] - ): - assert router_flavors.main() == 0 - - config = json.loads(capsys.readouterr().out) - assert config["kubernetes"][0]["namespace"]["nameSelector"]["matchNames"] == [ - "openstack" - ] - assert config["schedule"][0]["crontab"] == "*/15 * * * *" - - -def test_router_flavor_hook_config_uses_full_object_filter(monkeypatch): - """JqFilter must be '.' so cloudCredentialsRef is available in the event.""" - clear_router_flavor_env(monkeypatch) - monkeypatch.setenv("NEUTRON_ROUTER_FLAVOR_ENABLED", "true") - - config = router_flavors.build_hook_config() - - assert config["kubernetes"][0]["jqFilter"] == "." - - -# --------------------------------------------------------------------------- -# k8s.read_secret_key (common module) -# --------------------------------------------------------------------------- - - -def test_read_secret_key_raises_on_missing_key(): - """read_secret_key propagates KeyError when the key is absent.""" - with mock.patch.object( - k8s_module, "read_secret_key", side_effect=KeyError("clouds.yaml") - ): - with pytest.raises(KeyError): - k8s_module.read_secret_key("infrasetup", "clouds.yaml", "openstack") - - -# --------------------------------------------------------------------------- -# k8s.get_openstack_connection (common module, used by all hooks) -# --------------------------------------------------------------------------- - - -def test_get_openstack_connection_reads_secret(monkeypatch): - """Connection is built from the named K8s secret via read_secret_key.""" - monkeypatch.setattr(k8s_module, "_connection_cache", {}) - monkeypatch.setenv("POD_NAMESPACE", "openstack") - - fake_conn = mock.MagicMock(name="fake_conn") - - with mock.patch( - "openstack_sync.utils.openstack.connection.Connection", - return_value=fake_conn, - ): - with mock.patch.object( - k8s_module, "read_secret_key", return_value=FAKE_CLOUDS_YAML - ): - conn = k8s_module.get_openstack_connection("infrasetup", "understack") - - assert conn is fake_conn - - -def test_get_openstack_connection_memoized(monkeypatch): - """Same (secret_name, cloud_name) returns cached connection.""" - monkeypatch.setattr(k8s_module, "_connection_cache", {}) - monkeypatch.setenv("POD_NAMESPACE", "openstack") - - fake_conn = mock.MagicMock(name="fake_conn") - - with mock.patch( - "openstack_sync.utils.openstack.connection.Connection", - return_value=fake_conn, - ) as mock_conn: - with mock.patch.object( - k8s_module, "read_secret_key", return_value=FAKE_CLOUDS_YAML - ): - conn1 = k8s_module.get_openstack_connection("infrasetup", "understack") - conn2 = k8s_module.get_openstack_connection("infrasetup", "understack") - - assert conn1 is conn2 - mock_conn.assert_called_once() - - -def test_get_openstack_connection_separate_per_secret(monkeypatch): - """Different secrets produce independent connections.""" - monkeypatch.setattr(k8s_module, "_connection_cache", {}) - monkeypatch.setenv("POD_NAMESPACE", "openstack") - - conn_a = mock.MagicMock(name="conn_a") - conn_b = mock.MagicMock(name="conn_b") - - bm_yaml = FAKE_CLOUDS_YAML.replace("infrasetup", "baremetal-manage") - - def fake_read(secret_name, secret_key, namespace): - return FAKE_CLOUDS_YAML if secret_name == "infrasetup" else bm_yaml # noqa: S105 - - with mock.patch( - "openstack_sync.utils.openstack.connection.Connection", - side_effect=[conn_a, conn_b], - ): - with mock.patch.object(k8s_module, "read_secret_key", side_effect=fake_read): - result_a = k8s_module.get_openstack_connection("infrasetup", "understack") - result_b = k8s_module.get_openstack_connection( - "baremetal-manage", "understack" - ) - - assert result_a is conn_a - assert result_b is conn_b - - -# --------------------------------------------------------------------------- -# reconcile_router_flavor -# --------------------------------------------------------------------------- - - -def test_reconcile_router_flavor_reads_credentials_ref(monkeypatch): - """Hook reads secretName + cloudName from spec.cloudCredentialsRef.""" - monkeypatch.setattr(k8s_module, "_connection_cache", {}) - monkeypatch.setenv("POD_NAMESPACE", "openstack") - - fake_conn = mock.MagicMock() - - event = { - "object": { - "metadata": {"name": "test-flavor"}, - "spec": { - "name": "test-flavor", - "driver": "some.Driver", - "cloudCredentialsRef": { - "secretName": "baremetal-manage", - "cloudName": "understack", - }, - }, - } - } - - with mock.patch( - "openstack_sync.utils.openstack.connection.Connection", - return_value=fake_conn, - ): - with mock.patch.object( - k8s_module, "read_secret_key", return_value=FAKE_CLOUDS_YAML - ) as mock_read: - router_flavors.reconcile_router_flavor(event) - - mock_read.assert_called_once_with("baremetal-manage", "clouds.yaml", "openstack") - - -def test_reconcile_router_flavor_raises_when_creds_ref_missing(): - """Missing cloudCredentialsRef raises ValueError.""" - event = { - "object": { - "metadata": {"name": "bad-flavor"}, - "spec": {"name": "bad-flavor", "driver": "some.Driver"}, - } - } - - with pytest.raises(ValueError, match="cloudCredentialsRef"): - router_flavors.reconcile_router_flavor(event) - - -def test_reconcile_router_flavor_raises_when_secret_name_missing(): - event = { - "object": { - "metadata": {"name": "bad-flavor"}, - "spec": { - "name": "bad-flavor", - "driver": "some.Driver", - "cloudCredentialsRef": {"cloudName": "understack"}, - }, - } - } - - with pytest.raises(ValueError, match="cloudCredentialsRef"): - router_flavors.reconcile_router_flavor(event) - - -def test_reconcile_router_flavor_raises_when_cloud_name_missing(): - event = { - "object": { - "metadata": {"name": "bad-flavor"}, - "spec": { - "name": "bad-flavor", - "driver": "some.Driver", - "cloudCredentialsRef": {"secretName": "baremetal-manage"}, - }, - } - } - - with pytest.raises(ValueError, match="cloudCredentialsRef"): - router_flavors.reconcile_router_flavor(event) - - -# --------------------------------------------------------------------------- -# main() — binding context dispatch -# --------------------------------------------------------------------------- - - -def test_main_dispatches_binding_context(monkeypatch, capsys, tmp_path): - monkeypatch.setattr(k8s_module, "_connection_cache", {}) - monkeypatch.setenv("POD_NAMESPACE", "openstack") - - fake_conn = mock.MagicMock() - - binding_context = json.dumps( - [ - { - "binding": "neutron-router-flavors", - "objects": [ - { - "object": { - "metadata": {"name": "flavor-a"}, - "spec": { - "name": "flavor-a", - "driver": "some.Driver", - "cloudCredentialsRef": { - "secretName": "infrasetup", - "cloudName": "understack", - }, - }, - } - } - ], - } - ] - ) - - ctx_file = tmp_path / "binding_context.json" - ctx_file.write_text(binding_context) - monkeypatch.setenv("BINDING_CONTEXT_PATH", str(ctx_file)) - - with mock.patch( - "openstack_sync.utils.openstack.connection.Connection", - return_value=fake_conn, - ): - with mock.patch.object( - k8s_module, "read_secret_key", return_value=FAKE_CLOUDS_YAML - ): - with mock.patch.object(router_flavors.sys, "argv", ["router_flavors.py"]): - result = router_flavors.main() - - assert result == 0 - - -def test_main_returns_error_on_invalid_json(monkeypatch, capsys, tmp_path): - ctx_file = tmp_path / "binding_context.json" - ctx_file.write_text("not-json") - monkeypatch.setenv("BINDING_CONTEXT_PATH", str(ctx_file)) - - with mock.patch.object(router_flavors.sys, "argv", ["router_flavors.py"]): - result = router_flavors.main() - - assert result == 1 - assert "failed to parse binding context" in capsys.readouterr().err - - -def test_main_returns_zero_on_empty_stdin(monkeypatch, tmp_path): - ctx_file = tmp_path / "binding_context.json" - ctx_file.write_text("") - monkeypatch.setenv("BINDING_CONTEXT_PATH", str(ctx_file)) - - with mock.patch.object(router_flavors.sys, "argv", ["router_flavors.py"]): - result = router_flavors.main() - - assert result == 0 diff --git a/python/openstack-sync/tests/test_router_flavors_hook.py b/python/openstack-sync/tests/test_router_flavors_hook.py new file mode 100644 index 000000000..4705ce19b --- /dev/null +++ b/python/openstack-sync/tests/test_router_flavors_hook.py @@ -0,0 +1,377 @@ +"""Tests for the router flavor hook: how the plugin wires into the framework. + +The generic driver is covered in ``test_framework.py``; these tests cover only +what is specific to this plugin, plus one end-to-end run through ``main()``. +""" + +from __future__ import annotations + +import importlib +import json +import types +from pathlib import Path +from typing import Any +from unittest import mock + +import pytest + +import openstack_sync.utils as utils +from openstack_sync.hooks import router_flavors as hook +from openstack_sync.plugins.neutron.router_flavors import markers +from openstack_sync.plugins.neutron.router_flavors.config import BINDING_NAME +from openstack_sync.plugins.neutron.router_flavors.config import ENV_PREFIX +from openstack_sync.plugins.neutron.router_flavors.config import SERVICE_TYPE +from tests.conftest import CRD_API_VERSION +from tests.conftest import CRD_KIND +from tests.conftest import make_hook_config + +ENV_NAMES = ( + "BINDING_CONTEXT_PATH", + f"{ENV_PREFIX}_ENABLED", + f"{ENV_PREFIX}_SYNC_CRONTAB", + f"{ENV_PREFIX}_PRUNE", + f"{ENV_PREFIX}_STATUS_ENABLED", + f"{ENV_PREFIX}_READY_RETRIES", + f"{ENV_PREFIX}_READY_DELAY", + "POD_NAMESPACE", +) + +FAKE_CLOUDS_YAML = """ +clouds: + understack: + auth: + auth_url: https://keystone.example.com/v3 + username: infrasetup + password: secret + project_name: baremetal + region_name: iad3 +""" + + +def clear_env(monkeypatch: pytest.MonkeyPatch) -> None: + for name in ENV_NAMES: + monkeypatch.delenv(name, raising=False) + + +def router_flavor_object(name: str, spec: dict | None = None) -> dict: + flavor_spec: dict[str, Any] = { + "name": name, + "service_type": SERVICE_TYPE, + "description": f"{name} description", + "is_enabled": True, + "service_profiles": [ + { + "driver": "neutron_understack.l3_router.vrf.Vrf", + "description": f"{name} profile", + "meta_info": {"vni_alloc": "auto"}, + "is_enabled": True, + } + ], + "cloudCredentialsRef": { + "secretName": "infrasetup", + "cloudName": "understack", + }, + } + flavor_spec.update(spec or {}) + return { + "apiVersion": CRD_API_VERSION, + "kind": CRD_KIND, + "metadata": {"name": name, "namespace": "openstack", "generation": 3}, + "spec": flavor_spec, + } + + +def write_binding_context(path: Path, contexts: list[dict]) -> str: + context_path = path / "binding-context.json" + context_path.write_text(json.dumps(contexts), encoding="utf-8") + return str(context_path) + + +# --------------------------------------------------------------------------- +# Import safety +# --------------------------------------------------------------------------- + + +def test_module_import_is_safe_with_bad_runtime_env(monkeypatch): + """Importing must not read runtime config. + + Shell-operator imports the hook to ask for its config before the full + environment is guaranteed, so a malformed value must not break import. + """ + monkeypatch.setenv(f"{ENV_PREFIX}_READY_RETRIES", "not-a-number") + monkeypatch.setenv(f"{ENV_PREFIX}_ENABLED", "not-a-bool") + + importlib.reload(hook) + + +def test_config_flag_prints_json(monkeypatch, capsys): + clear_env(monkeypatch) + monkeypatch.setattr(hook.sys, "argv", ["router_flavors.py", "--config"]) + + assert hook.main() == 0 + assert json.loads(capsys.readouterr().out)["onStartup"] == 10 + + +def test_enabled_config_flag_watches_this_crd(monkeypatch, capsys): + clear_env(monkeypatch) + monkeypatch.setenv(f"{ENV_PREFIX}_ENABLED", "true") + monkeypatch.setattr(hook.sys, "argv", ["router_flavors.py", "--config"]) + + assert hook.main() == 0 + config = json.loads(capsys.readouterr().out) + (binding,) = config["kubernetes"] + assert binding["name"] == BINDING_NAME + assert binding["kind"] == CRD_KIND + + +# --------------------------------------------------------------------------- +# Plugin wiring +# --------------------------------------------------------------------------- + + +def test_plugin_reconcile_delegates_to_sync_flavor(): + plugin = hook.RouterFlavorPlugin(make_hook_config()) + conn = mock.MagicMock() + cache: dict[str, Any] = {} + spec = {"name": "flavor-a"} + + with mock.patch.object( + hook.reconcile_module, "sync_flavor", return_value=["a note"] + ) as sync_flavor: + notes = plugin.reconcile(conn, spec, cache) + + assert notes == ["a note"] + sync_flavor.assert_called_once_with(conn, spec, cache) + + +def test_plugin_wait_for_api_uses_configured_retry_budget(): + plugin = hook.RouterFlavorPlugin( + make_hook_config(ready_retries=5, ready_delay=0.25) + ) + conn = mock.MagicMock() + + with mock.patch.object(hook, "wait_for_openstack_network") as wait: + plugin.wait_for_api(conn) + + wait.assert_called_once_with(conn, retries=5, delay=0.25) + + +def test_plugin_prune_is_a_noop_when_disabled(): + plugin = hook.RouterFlavorPlugin(make_hook_config(prune=False)) + + with mock.patch.object(hook.prune_module, "prune_removed_flavors") as prune: + plugin.prune(mock.MagicMock(), [{"name": "a"}], authoritative_empty=False) + + prune.assert_not_called() + + +def test_plugin_prune_forwards_authoritative_empty_when_enabled(): + plugin = hook.RouterFlavorPlugin(make_hook_config(prune=True)) + conn = mock.MagicMock() + specs = [{"name": "a"}] + + with mock.patch.object(hook.prune_module, "prune_removed_flavors") as prune: + plugin.prune(conn, specs, authoritative_empty=True) + + prune.assert_called_once_with(conn, specs, authoritative_empty=True) + + +def test_plugin_cache_is_per_credential_group(): + plugin = hook.RouterFlavorPlugin(make_hook_config()) + + assert plugin.new_cache() == {} + assert plugin.new_cache() is not plugin.new_cache() + + +# --------------------------------------------------------------------------- +# End to end through main() +# --------------------------------------------------------------------------- + + +def _neutron_conn() -> Any: + """A Neutron connection that already holds the desired flavor and profile.""" + profile = types.SimpleNamespace( + id="profile-id", + driver="neutron_understack.l3_router.vrf.Vrf", + is_enabled=True, + description="pa1410 profile", + meta_info=markers.managed_meta_info({"vni_alloc": "auto"}), + ) + flavor = types.SimpleNamespace( + id="flavor-id", + name="pa1410", + service_type=SERVICE_TYPE, + description=markers.managed_flavor_description("pa1410 description"), + is_enabled=True, + service_profile_ids=["profile-id"], + ) + conn = mock.MagicMock() + conn.network.service_profiles.return_value = [profile] + conn.network.flavors.return_value = [flavor] + conn.network.get_flavor.return_value = flavor + return conn + + +def _run_main(monkeypatch, tmp_path, contexts: list[dict], conn: Any): + monkeypatch.setenv( + "BINDING_CONTEXT_PATH", write_binding_context(tmp_path, contexts) + ) + with ( + mock.patch.object(hook.sys, "argv", ["router_flavors.py"]), + mock.patch( + "openstack_sync.hooks.framework.get_openstack_connection", + return_value=conn, + ), + mock.patch( + "openstack_sync.hooks.framework.patch_resource_status" + ) as patch_status, + mock.patch.object(hook, "wait_for_openstack_network"), + ): + code = hook.main() + return code, patch_status + + +def _schedule_context(*names: str) -> list[dict]: + return [ + { + "binding": BINDING_NAME, + "type": "Schedule", + "snapshots": { + BINDING_NAME: [{"object": router_flavor_object(n)} for n in names] + }, + } + ] + + +def test_main_returns_zero_when_hook_disabled(monkeypatch, tmp_path): + clear_env(monkeypatch) + conn = _neutron_conn() + + code, patch_status = _run_main( + monkeypatch, tmp_path, _schedule_context("pa1410"), conn + ) + + assert code == 0 + patch_status.assert_not_called() + conn.network.flavors.assert_not_called() + + +def test_main_reconciles_an_already_converged_flavor(monkeypatch, tmp_path): + clear_env(monkeypatch) + monkeypatch.setenv(f"{ENV_PREFIX}_ENABLED", "true") + monkeypatch.setenv("POD_NAMESPACE", "openstack") + conn = _neutron_conn() + + code, patch_status = _run_main( + monkeypatch, tmp_path, _schedule_context("pa1410"), conn + ) + + assert code == 0 + assert patch_status.call_args.kwargs["sync_status"] == "Synced" + assert patch_status.call_args.kwargs["message"] == ( + "Successfully reconciled router flavor" + ) + # Already converged: no writes to Neutron. + conn.network.create_flavor.assert_not_called() + conn.network.create_service_profile.assert_not_called() + conn.network.associate_flavor_with_service_profile.assert_not_called() + + +def test_main_reports_profile_drift_on_the_cr_status(monkeypatch, tmp_path): + """A disabled profile keeps the flavor Synced but must show on the status.""" + clear_env(monkeypatch) + monkeypatch.setenv(f"{ENV_PREFIX}_ENABLED", "true") + conn = _neutron_conn() + conn.network.service_profiles.return_value[0].is_enabled = False + + code, patch_status = _run_main( + monkeypatch, tmp_path, _schedule_context("pa1410"), conn + ) + + assert code == 0 + assert patch_status.call_args.kwargs["sync_status"] == "Synced" + message = patch_status.call_args.kwargs["message"] + assert "is_enabled" in message + assert "profile-id" in message + + +def test_main_reports_failure_and_skips_prune(monkeypatch, tmp_path): + clear_env(monkeypatch) + monkeypatch.setenv(f"{ENV_PREFIX}_ENABLED", "true") + monkeypatch.setenv(f"{ENV_PREFIX}_PRUNE", "true") + conn = _neutron_conn() + # An existing flavor whose service_type cannot be changed is a hard failure. + conn.network.flavors.return_value[0].service_type = "WRONG_TYPE" + + with mock.patch.object(hook.prune_module, "prune_removed_flavors") as prune: + code, patch_status = _run_main( + monkeypatch, tmp_path, _schedule_context("pa1410"), conn + ) + + assert code == 1 + assert patch_status.call_args.kwargs["sync_status"] == "Failed" + assert "service_type" in patch_status.call_args.kwargs["message"] + prune.assert_not_called() + + +def test_main_prunes_after_a_successful_reconcile(monkeypatch, tmp_path): + clear_env(monkeypatch) + monkeypatch.setenv(f"{ENV_PREFIX}_ENABLED", "true") + monkeypatch.setenv(f"{ENV_PREFIX}_PRUNE", "true") + conn = _neutron_conn() + + with mock.patch.object(hook.prune_module, "prune_removed_flavors") as prune: + code, _ = _run_main(monkeypatch, tmp_path, _schedule_context("pa1410"), conn) + + assert code == 0 + prune.assert_called_once() + assert [spec["name"] for spec in prune.call_args.args[1]] == ["pa1410"] + + +def test_main_fails_loudly_on_a_cr_missing_cloud_credentials(monkeypatch, tmp_path): + """A CR without credentials must fail the run, not be skipped. + + The CRD marks cloudCredentialsRef required, so the API server should reject + it first; this guards the case where something bypasses that. + """ + clear_env(monkeypatch) + monkeypatch.setenv(f"{ENV_PREFIX}_ENABLED", "true") + obj = router_flavor_object("pa1410") + del obj["spec"]["cloudCredentialsRef"] + contexts = [ + { + "binding": BINDING_NAME, + "type": "Schedule", + "snapshots": {BINDING_NAME: [{"object": obj}]}, + } + ] + + code, _ = _run_main(monkeypatch, tmp_path, contexts, _neutron_conn()) + + assert code == 1 + + +def test_main_uses_the_credentials_named_by_each_cr(monkeypatch, tmp_path): + clear_env(monkeypatch) + monkeypatch.setenv(f"{ENV_PREFIX}_ENABLED", "true") + monkeypatch.setattr(utils, "_connection_cache", {}) + contexts = _schedule_context("pa1410") + contexts[0]["snapshots"][BINDING_NAME][0]["object"]["spec"][ + "cloudCredentialsRef" + ] = {"secretName": "other-secret", "cloudName": "other-cloud"} + monkeypatch.setenv( + "BINDING_CONTEXT_PATH", write_binding_context(tmp_path, contexts) + ) + + with ( + mock.patch.object(hook.sys, "argv", ["router_flavors.py"]), + mock.patch( + "openstack_sync.hooks.framework.get_openstack_connection", + return_value=_neutron_conn(), + ) as connect, + mock.patch("openstack_sync.hooks.framework.patch_resource_status"), + mock.patch.object(hook, "wait_for_openstack_network"), + ): + assert hook.main() == 0 + + connect.assert_called_once_with("other-secret", "other-cloud") diff --git a/schema/openstack-sync/neutron-router-flavor.schema.json b/schema/openstack-sync/neutron-router-flavor.schema.json index 6d3cbebdc..c24c90f58 100644 --- a/schema/openstack-sync/neutron-router-flavor.schema.json +++ b/schema/openstack-sync/neutron-router-flavor.schema.json @@ -26,6 +26,24 @@ "type": "object", "additionalProperties": false, "properties": { + "cloudCredentialsRef": { + "description": "Reference to a Kubernetes Secret containing the OpenStack clouds.yaml.", + "type": "object", + "additionalProperties": false, + "required": ["secretName", "cloudName"], + "properties": { + "secretName": { + "type": "string", + "minLength": 1, + "maxLength": 253 + }, + "cloudName": { + "type": "string", + "minLength": 1, + "maxLength": 256 + } + } + }, "name": { "description": "Neutron router flavor name.", "type": "string", @@ -52,6 +70,31 @@ "type": "string", "maxLength": 1024 }, + "is_enabled": { + "description": "Whether the Neutron router flavor is enabled.", + "type": "boolean", + "default": true + }, + "service_profiles": { + "description": "Service profiles to associate with this flavor. The operator find-or-creates each profile by (driver, meta_info) and reconciles the flavor's set of bound profiles to match.", + "type": "array", + "minItems": 1, + "items": { + "$ref": "#/definitions/serviceProfileSpec" + } + } + }, + "required": [ + "name", + "service_profiles" + ] + }, + "serviceProfileSpec": { + "description": "A single service profile entry.", + "type": "object", + "additionalProperties": false, + "required": ["driver"], + "properties": { "driver": { "description": "Service profile driver class.", "type": "string", @@ -59,24 +102,20 @@ "maxLength": 1024, "pattern": "^[A-Za-z_][A-Za-z0-9_]*(\\.[A-Za-z_][A-Za-z0-9_]*)+$" }, - "profile_description": { + "description": { "description": "Description stored on the Neutron service profile.", "type": "string", "maxLength": 1024 }, - "profile_id": { - "description": "Existing Neutron service profile ID to attach instead of creating or discovering one.", - "type": "string", - "format": "uuid" + "is_enabled": { + "description": "Whether the service profile is enabled in Neutron.", + "type": "boolean", + "default": true }, "meta_info": { "$ref": "#/definitions/metaInfo" } - }, - "required": [ - "name", - "driver" - ] + } }, "metaInfo": { "description": "Service profile metainfo payload.",