feat(openstack-sync-operator): Operator for syncing openstack data as kubernetes custom resources

[haseebsyed12]

openstack-sync-operator is a shell-operator-based reconciliation system that syncs Kubernetes-defined OpenStack configuration into live OpenStack clouds. It uses a declarative, Helm-driven plugin model where new resource types are defined via:

1. A CRD that defines the Kubernetes schema
2. A Python hook script in the container image
3. Helm values entries that wire everything together at deployment time

┌─────────────────────────────────────────────────────────────────┐
│ Kubernetes Layer (Operator Deployment)                          │
│  - shell-operator base pod                                      │
│  - Helm-wired hook discovery                                    │
│  - RBAC auto-generated from CRD metadata                        │
└──────────────────────┬──────────────────────────────────────────┘
                       │ (watches K8s resources)
┌──────────────────────▼──────────────────────────────────────────┐
│ Hook/Plugin Layer (Python Scripts in /hooks)                    │
│  - Each hook: a standalone Python script                        │
│  - Contract: exports HOOK_CONFIG, handles --config flag         │
│  - Lifecycle: onStartup, onChange, periodicCron, etc            │
└──────────────────────┬──────────────────────────────────────────┘
                       │ (makes API calls)
┌──────────────────────▼──────────────────────────────────────────┐
│ OpenStack API Layer                                             │
│  - Uses OpenStack SDK via Python environment                    │
│  - Authenticated via /etc/openstack/clouds.yaml                 │
│  - Reconciles: create, update, delete, prune operations         │
└─────────────────────────────────────────────────────────────────┘

Full CRUD implementation of neutron router flavors are implemented in feat(openstack-sync-plugins): neutron router flavor - #2217

[cardoe]

Code review

Nice chunk of work here. My goal for the initial commit is a generic shell-operator container with the OpenStack CLI, where watched resources are enrolled through separate charts and adding a hook is easy to follow. Most of the scaffolding is right — the Application shape, the site.<component>.enabled toggles, chart placement, and the docs wiring all match existing conventions. The notes below are about the plugin layer on top of that, plus two destructive bugs in the prune path.

Tracked separately, not this PR: the pyproject.toml ruff extend deviation is pre-existing and repo-wide (#2207); migrating shell-operator-ironic onto this operator is a follow-on (#2208); multi-cloud support with per-resource cloud selection and system vs project scope is a follow-on (#2210).

---

Framework shape

1. Building hooks into the image is right — the docs just contradict it

Baking hooks in is what we want: hooks and their Python dependencies version together, CI tests the artifact that ships, and the image tag fully determines behavior. It also matches containers/shell-operator-ironic. A new container version per new resource type is expected and fine.

The problem is that the component doc claims the opposite:

understack/docs/deploy-guide/components/openstack-sync-operator.md

Lines 112 to 115 in 582e234

	This design keeps the operator image generic while allowing services to inject
	their configuration through ConfigMaps. No operator image changes are needed
	when adding new service plugins—plugins control their own hooks via environment
	configuration.

while the README correctly documents adding a COPY line per plugin:

understack/containers/openstack-sync-operator/README.md

Lines 60 to 67 in 582e234

	3. Package the hook in this image.
	Add a `COPY --chmod=755` line to `Dockerfile` so shell-operator can discover
	the hook in `/hooks`:
	```dockerfile
	COPY --chmod=755 python/openstack-sync/openstack_sync/plugins/<service>/<target>/hook.py /hooks/<target>.py
	```

Please make the component doc match the Dockerfile and state the versioning contract explicitly: adding a watched resource requires a new image, and the operator chart is bumped to match.

2. No OpenStack CLI in the image

The only dependency is openstacksdk, so there's no openstack binary. Compare containers/shell-operator-ironic/requirements.txt, which installs python-openstackclient and python-ironicclient.

understack/containers/openstack-sync-operator/Dockerfile

Lines 17 to 21 in 582e234

	RUN --mount=type=cache,target=/root/.cache/uv \
	uv pip install \
	--upgrade \
	--constraint /upper-constraints.txt \
	/src/openstack-sync

3. CRDs belong with the operator; only the CRs live separately

A CRD is the API contract of the hook that reads it, so schema and code must version together. Right now the CRD is in the plugin chart at components/openstack-sync-neutron-router-flavors/crds/, which lets a site load a schema newer or older than the code consuming it. Move it to components/openstack-sync-operator/crds/ using Helm's crds/ convention, as go/dexop/helm/crds/clients.yaml and go/nautobotop/helm/crds/clients.yaml do, and as components/ironic/kustomization.yaml does by pulling runbook-crd and runbook-operator together.

The custom resources then live in their own chart with their own Application, following charts/nautobot-job-queues + application-nautobot-job-queues.yaml. Note that pattern also adds a $deploy/<site>/<name> source so a site can add resources without changing understack.

Side benefit: with CRDs in the operator chart, helm template emits them and ArgoCD applies them with the operator, so the sync-wave -1 plus SkipDryRunOnMissingResource=true juggling inside one Application is no longer needed.

4. Drop the convention-based plugin discovery

Nothing else in charts/argocd-understack/templates/ discovers components by name prefix; every other Application is an explicit template.

understack/charts/argocd-understack/templates/_helpers.tpl

Lines 236 to 257 in 582e234

	*/}}
	{{- define "understack.openstackSyncPlugins" -}}
	{{- $root := . -}}
	{{- $plugins := list -}}
	{{- range $pluginSiteKey := keys $root.Values.site | sortAlpha }}
	{{- $pluginValues := get $root.Values.site $pluginSiteKey }}
	{{- if kindIs "map" $pluginValues }}
	{{- if and (hasPrefix "openstack_sync_" $pluginSiteKey) (ne $pluginSiteKey "openstack_sync_operator") }}
	{{- if eq (include "understack.isEnabled" (list $root.Values.site $pluginSiteKey)) "true" }}
	{{- $componentName := replace "_" "-" $pluginSiteKey }}
	{{- $plugins = append $plugins (dict
	"componentName" $componentName
	"envConfigMapName" (default (printf "%s-env" $componentName) (get $pluginValues "envConfigMapName"))
	"operatorServiceAccountName" (default "openstack-sync-operator" (get $pluginValues "operatorServiceAccountName"))
	"siteKey" $pluginSiteKey
	) }}
	{{- end }}
	{{- end }}
	{{- end }}
	{{- end }}
	{{- dict "plugins" $plugins | toYaml -}}
	{{- end }}

understack/charts/argocd-understack/templates/application-openstack-sync-operator.yaml

Lines 31 to 40 in 582e234

	{{- if $enabledPlugins }}
	valuesObject:
	extraEnvFrom:
	{{- range $plugin := $enabledPlugins }}
	- configMapRef:
	name: {{ get $plugin "envConfigMapName" | quote }}
	optional: false
	{{- end }}
	{{- end }}
	valueFiles:

5. Hook enablement and crontabs belong in the operator chart's values

HOOK_CONFIG is built at module import, and shell-operator only invokes --config at startup:

understack/python/openstack-sync/openstack_sync/plugins/neutron/router_flavors/hook.py

Lines 58 to 67 in 582e234

	if common.SYNC_ENABLED:
	HOOK_CONFIG["kubernetes"] = [_router_flavor_binding()]
	HOOK_CONFIG["schedule"] = [
	{
	"name": "hourly sync",
	"crontab": common.SYNC_CRONTAB,
	"includeSnapshotsFrom": [common.CRD_BINDING_NAME],
	}
	]

understack/python/openstack-sync/openstack_sync/plugins/neutron/router_flavors/router_flavors_common.py

Lines 27 to 30 in 582e234

	SYNC_CRONTAB = os.environ.get("NEUTRON_ROUTER_FLAVOR_SYNC_CRONTAB", "0 * * * *")
	SYNC_ENABLED = env_bool("NEUTRON_ROUTER_FLAVOR_ENABLED", False)
	CRD_API_VERSION = os.environ.get(

Those values arrive via envFrom: configMapRef, which Kubernetes does not propagate to a running container, and there's no checksum annotation to force a rollout — nor can there be, since the ConfigMap is rendered by a different Helm release:

understack/components/openstack-sync-operator/templates/deployment.yaml

Lines 40 to 44 in 582e234

	{{- end }}
	{{- with .Values.extraEnvFrom }}
	envFrom:
	{{- toYaml . | nindent 8 }}
	{{- end }}

Failure case: set routerFlavors.crontab: "*/15 * * * *", ArgoCD syncs green, the pod keeps syncing hourly, nothing reports a problem. Same for prune and enabled.

Since the image knows its own hooks, put "which hooks are enabled and on what crontab" in the operator chart's own values rather than a ConfigMap authored by a separate chart. That removes the cross-release coupling, makes a checksum/config pod annotation possible so a schedule change actually rolls out, and gives one uniform way to register a schedule instead of a per-plugin <SERVICE>_<TARGET>_SYNC_CRONTAB namespace that the operator has to know about.

6. Fail loudly when a hook is enabled but not present in /hooks

Prod pins the image to a hash via the deployment repo's values, so the pinned image and understack_ref move independently by design. A site pins an image, someone bumps understack_ref to pick up a newly added resource chart, the corresponding hook isn't in the pinned image — ArgoCD is green, the CRs are applied, nothing reconciles them, no error surfaces. The operator should verify at startup that every hook it's told to enable actually exists.

---

Correctness

7. Prune deletes router flavors the operator never created

is_prunable_flavor() treats a flavor as prunable if its own description carries the marker or if any attached service profile is operator-managed:

understack/python/openstack-sync/openstack_sync/plugins/neutron/router_flavors/delete_router_flavors.py

Lines 53 to 70 in 582e234

	common.get_value(flavor, "service_type", "Service Type")
	!= common.DEFAULT_SERVICE_TYPE
	):
	return False
	if common.is_managed_flavor(flavor):
	return True
	for profile_id in common.service_profile_ids(flavor):
	profile = get_cached_service_profile(conn, profile_id, profile_cache)
	if (
	profile
	and common.is_managed_service_profile(profile)
	and is_prunable_service_profile(profile)
	):
	return True
	return False

The second branch infers ownership from a shared object. The PR's own test documents it — fl-remove has no description marker at all, yet is asserted deleted purely because it shares sp-shared with a desired flavor:

understack/python/openstack-sync/tests/test_router_flavors.py

Lines 510 to 540 in 582e234

	def test_prune_keeps_profile_still_attached_to_another_flavor(self):
	conn = FakeConnection(
	FakeNetwork(
	flavors=[
	{
	"id": "fl-keep",
	"name": "keep",
	"service_type": "L3_ROUTER_NAT",
	"service_profile_ids": ["sp-shared"],
	},
	{
	"id": "fl-remove",
	"name": "remove",
	"service_type": "L3_ROUTER_NAT",
	"service_profile_ids": ["sp-shared"],
	},
	],
	profiles=[
	{
	"id": "sp-shared",
	"driver": "neutron_understack.l3_router.vrf.Vrf",
	"meta_info": common.OPERATOR_META_INFO_MARKERS,
	},
	],
	)
	)
	delete_router_flavors.prune_removed_flavors(conn, [{"name": "keep"}])
	self.assertEqual(["fl-remove"], conn.network.deleted_flavors)
	self.assertEqual([], conn.network.deleted_profiles)

Failure case: someone hand-creates a router flavor against the shared neutron_understack.l3_router.vrf.Vrf service profile, and it isn't attached to a live router yet. The next hourly sync deletes it. The description marker is the only real ownership signal; the profile branch should go.

8. Disabling the plugin deletes every managed flavor

understack/components/openstack-sync-neutron-router-flavors/templates/hook-env-configmap.yaml

Lines 10 to 12 in 582e234

	data:
	NEUTRON_ROUTER_FLAVOR_ENABLED: "true"
	NEUTRON_ROUTER_FLAVOR_SYNC_CRONTAB: {{ .Values.routerFlavors.crontab | quote }}

NEUTRON_ROUTER_FLAVOR_ENABLED is a literal "true" gated only on .Values.hook.enabled (default true), not on .Values.routerFlavors.enabled. Set routerFlavors.enabled: false and templates/router-flavors.yaml stops rendering CRs while the hook stays enabled. The binding context yields an empty snapshot, so run() calls prune_removed_flavors(conn, []) with an empty desired set — and with prune: true by default, every operator-managed L3_ROUTER_NAT flavor in Neutron is deleted. Combined with finding 7 above that reaches flavors the operator never created. "Disable this plugin" must not be destructive.

(templates/rbac.yaml is also gated on routerFlavors.enabled, so the same config drops RBAC out from under an enabled hook.)

9. The operator chart ships no RBAC for its own ServiceAccount

components/openstack-sync-operator/templates/ contains only serviceaccount.yaml and deployment.yaml; every permission comes from whichever plugin chart happens to be enabled. Since site.openstack_sync_operator.enabled toggles independently, enabling the operator with no plugins yields a Pod whose SA can do nothing, with ArgoCD reporting Synced/Healthy and the only symptom in container logs. Compare components/ironic/runbook-operator/role.yaml, which ships its ClusterRole alongside the Deployment.

10. A single bad flavor aborts the whole reconcile

In hook.py, run() re-raises on the first sync_flavor failure, so every later flavor is left unsynced with no status written.

Minor: no livenessProbe/readinessProbe, though shell-operator serves /healthz. wait_for_openstack_network() retries 30×10s before raising, so a hung hook sits Running with no kubelet restart.

---

Housekeeping

11. Chart templates should be .yaml.tpl

Every existing Helm chart under components/ uses that extension (etcdbackup, components/openstack/templates/, envoy-configs, understack-cdn). Helm renders them the same, and the extension is why they don't trip the YAML linters. Using plain .yaml here required broadening a global exclude, which disables check-yaml for all of components/*/templates/ repo-wide and for any file added there in future:

understack/.pre-commit-config.yaml

Lines 14 to 16 in 582e234

	- --allow-multiple-documents
	exclude: "properdocs.yml|^charts/.*/templates/|^components/.*/templates/"
	- id: check-yaml

Renaming lets both this and the .yamllint.yaml change be reverted.

12. Drop the pr-2205 image tag

understack/components/openstack-sync-operator/values.yaml

Lines 3 to 6 in 582e234

	image:
	repository: ghcr.io/rackerlabs/understack/openstack-sync-operator
	tag: "pr-2205"
	pullPolicy: Always

Leave tag unset so it falls back to appVersion, with the deployment repo pinning the hash — the convention in charts/undersync/values.yaml. No other values.yaml under components/ or charts/ pins a pr- tag.

---

Suggested split

Land the framework alone — container, generic chart with the CRDs, plain Application — and move the Neutron router flavors work (CRs, router_flavors.json, hook, Python package) to a follow-up as its first consumer. That proves the "add a hook without touching the operator chart" property instead of asserting it, and keeps findings 7 and 8 off the critical path for the framework commit.

The reconciliation logic is otherwise in reasonable shape, and the service-profile marker scheme in router_flavors_common.py is a sound idea — it just needs to stop being used to infer flavor ownership.

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[cardoe]

Code review — pass 2 (536301ad, "openstack-sync-plugins iter-1")

Re-reviewed after your push. The chart layer got a solid pass. python/openstack-sync/ and containers/openstack-sync-operator/Dockerfile are byte-identical to the previous revision, so everything I raised about the hooks, the image, and the prune logic is still open — spelled out in full below so you don't have to cross-reference.

Resolved

Convention-based plugin discovery is gone. understack.openstackSyncPlugins deleted from _helpers.tpl, and the Application now keys off an explicit site.openstack_sync_plugins toggle. Collapsing the per-service charts into one components/openstack-sync-plugins chart works for me.

Disabling a plugin is no longer destructive. <PREFIX>_ENABLED now derives from $plugin.enabled rather than being a hardcoded "true", and hook-env-configmap.yaml, rbac.yaml, and resources.yaml all gate on that same flag, so the RBAC-vs-hook skew went away with it.

templates/_crd.tpl is more than I asked for, and good. Deriving apiVersion, kind, resource, group, plural, and hasStatus from the CRD file means the env vars and RBAC rules can't drift from the schema. The values.schema.json is a welcome addition too.

New in this push

config: [] silently falls back to the bundled data, and enabled-with-no-config still prunes everything

understack/components/openstack-sync-plugins/templates/resources.yaml

Lines 6 to 13 in 536301a

	{{- $config := get $plugin "config" }}
	{{- if not $config }}
	{{- $configPath := get $plugin "configPath" }}
	{{- if $configPath }}
	{{- $config = required (printf "plugins.%s.configPath file %s is empty or missing" $pluginKey $configPath) ($root.Files.Get $configPath) }}
	{{- end }}
	{{- end }}
	{{- if $config }}

{{- if not $config }} treats an empty list as falsy, so plugins.neutronRouterFlavors.config: [] doesn't mean "no resources" — it silently reverts to the chart's bundled data/router_flavors.json. There is no way to express an empty set.

Worse, a plugin enabled with neither config nor configPath renders zero CRs while <PREFIX>_ENABLED stays "true". The hook then runs against an empty snapshot and calls prune_removed_flavors(conn, []) with an empty desired set, deleting every operator-managed L3_ROUTER_NAT flavor. Same destination as the bug this push fixed, reached by a different route. The durable fix belongs in Python: treat an empty desired set as "nothing to do" rather than "delete everything", and require an explicit opt-in for a full purge.

Stale yamllint entry

understack/.yamllint.yaml

Lines 6 to 8 in 536301a

	- components/openstack-sync-plugins/templates/
	- components/openstack-sync-neutron-router-flavors/templates/
	- charts/argocd-understack/templates/

components/openstack-sync-neutron-router-flavors/templates/ no longer exists.

Squash before merge

The branch is two commits now, the second being openstack-sync-plugins iter-1. Please squash to a single standalone commit — the history shouldn't narrate the design iterations.

Still open

Prune infers flavor ownership from a shared service profile. This is the one I would gate a real cloud on.

understack/python/openstack-sync/openstack_sync/plugins/neutron/router_flavors/delete_router_flavors.py

Lines 46 to 71 in 536301a

	def is_prunable_flavor(
	conn: Any,
	flavor: Any,
	profile_cache: dict[str, Any | None],
	) -> bool:
	if (
	common.get_value(flavor, "service_type", "Service Type")
	!= common.DEFAULT_SERVICE_TYPE
	):
	return False
	if common.is_managed_flavor(flavor):
	return True
	for profile_id in common.service_profile_ids(flavor):
	profile = get_cached_service_profile(conn, profile_id, profile_cache)
	if (
	profile
	and common.is_managed_service_profile(profile)
	and is_prunable_service_profile(profile)
	):
	return True
	return False

is_prunable_flavor() treats a flavor as prunable if its own description carries the marker or if any attached service profile is operator-managed. The second branch infers ownership from a shared object. The test at test_router_flavors.py#L510-L540 documents it: fl-remove has no description marker at all, yet is asserted deleted purely because it shares sp-shared with a desired flavor. So a hand-created flavor using the shared neutron_understack.l3_router.vrf.Vrf profile, not yet attached to a live router, gets deleted on the next hourly sync. The description marker is the only real ownership signal; the profile branch should go.

The docs still contradict the Dockerfile.

understack/docs/deploy-guide/components/openstack-sync-operator.md

Lines 116 to 120 in 536301a

	This design keeps the operator image generic while allowing services to inject
	their configuration through ConfigMaps. No operator image changes are needed
	when adding new service plugins—plugins control their own hooks via environment
	configuration.

This says no operator image changes are needed to add a plugin, while README.md#L60-L67 correctly documents adding a COPY line. Building hooks into the image is the right call — hooks and their Python dependencies version together, CI tests the artifact that ships, and it matches containers/shell-operator-ironic. So fix the doc, and state the contract: adding a watched resource requires a new image, and the operator chart is bumped to match.

No OpenStack CLI in the image.

understack/containers/openstack-sync-operator/Dockerfile

Lines 17 to 21 in 536301a

	RUN --mount=type=cache,target=/root/.cache/uv \
	uv pip install \
	--upgrade \
	--constraint /upper-constraints.txt \
	/src/openstack-sync

Only openstacksdk is installed, so there is no openstack binary. Compare containers/shell-operator-ironic/requirements.txt, which installs python-openstackclient and python-ironicclient.

CRDs belong with the operator; only the CRs belong in the plugins chart. The CRD is the API contract of the hook that reads it, so schema and code must version together. It currently sits in components/openstack-sync-plugins/crds/, which lets a site load a schema newer or older than the code consuming it. Move it to components/openstack-sync-operator/crds/, as go/dexop/helm/crds/clients.yaml and go/nautobotop/helm/crds/clients.yaml do, and as components/ironic/kustomization.yaml does by pulling runbook-crd and runbook-operator in together. That also drops the need for the sync-wave -1 plus SkipDryRunOnMissingResource=true juggling.

The plugins chart should be its own Application, not an extra source on the operator's. Follow charts/nautobot-job-queues plus application-nautobot-job-queues.yaml. That pattern also adds a $deploy/<site>/<name> source so a site can add resources without changing understack.

Changing a crontab never reaches a running pod.

understack/python/openstack-sync/openstack_sync/plugins/neutron/router_flavors/hook.py

Lines 58 to 67 in 536301a

	if common.SYNC_ENABLED:
	HOOK_CONFIG["kubernetes"] = [_router_flavor_binding()]
	HOOK_CONFIG["schedule"] = [
	{
	"name": "hourly sync",
	"crontab": common.SYNC_CRONTAB,
	"includeSnapshotsFrom": [common.CRD_BINDING_NAME],
	}
	]

HOOK_CONFIG is built at module import and shell-operator only invokes --config at startup. The values arrive via extraEnvFrom referencing a ConfigMap rendered by a different Helm release, and Kubernetes does not propagate ConfigMap changes into a running container — so there is no checksum annotation that could force a rollout. Set SYNC_CRONTAB: "*/15 * * * *", ArgoCD syncs green, the pod keeps syncing hourly, nothing reports a problem. Since the image knows its own hooks, put "which hooks are enabled and on what crontab" in the operator chart's own values so a checksum/config pod annotation works and there is one uniform way to register a schedule.

A hook that is enabled but absent from /hooks should fail loudly. Prod pins the image via the deployment repo's values, so the pinned image and understack_ref move independently by design. Bump understack_ref to pick up a new plugin against an older pinned image and the hook simply isn't there: ArgoCD green, CRs applied, nothing reconciles them, no error.

The operator chart ships no RBAC for its own ServiceAccount. components/openstack-sync-operator/templates/ contains only _helpers.tpl, deployment.yaml, and serviceaccount.yaml. Every permission comes from the plugins chart. Since site.openstack_sync_operator.enabled toggles independently, enabling the operator alone yields a Pod whose SA can do nothing, with ArgoCD reporting Synced/Healthy and the only symptom in container logs. Compare components/ironic/runbook-operator/role.yaml, which ships its ClusterRole next to the Deployment.

One bad flavor aborts the whole reconcile. run() re-raises on the first sync_flavor failure, so every later flavor is left unsynced with no status written. Also no livenessProbe/readinessProbe, though shell-operator serves /healthz, and wait_for_openstack_network() retries 30×10s before raising — so a hung hook sits Running with no kubelet restart.

Chart templates should be .yaml.tpl. Every existing Helm chart under components/ uses that extension (etcdbackup, components/openstack/templates/, envoy-configs, understack-cdn). Helm renders them identically, and the extension is why they don't trip the YAML linters. Using plain .yaml required broadening a global exclude, which disables check-yaml for all of components/*/templates/ repo-wide and for anything added there in future:

understack/.pre-commit-config.yaml

Lines 14 to 16 in 536301a

	- --allow-multiple-documents
	exclude: "properdocs.yml|^charts/.*/templates/|^components/.*/templates/"
	- id: check-yaml

Renaming lets both that and the .yamllint.yaml additions be reverted.

Drop the pr-2205 image tag.

understack/components/openstack-sync-operator/values.yaml

Lines 3 to 6 in 536301a

	image:
	repository: ghcr.io/rackerlabs/understack/openstack-sync-operator
	tag: "pr-2205"
	pullPolicy: Always

Leave tag unset so it falls back to appVersion, with the deployment repo pinning the hash — the convention in charts/undersync/values.yaml.

---

Still worth splitting the Neutron router flavors work into a follow-up so the framework lands on its own. That moves the prune-ownership bug, the abort-on-first-failure, and the empty-config purge off this PR's critical path.

Tracked separately, not this PR: the pyproject.toml ruff extend deviation (#2207), migrating shell-operator-ironic onto this operator (#2208), and multi-cloud support with per-resource cloud selection and system vs project scope (#2210).

🤖 Generated with Claude Code

[cardoe]

Code review — pass 3 (893a4c30)

Big improvement. The framework now looks the way I was asking for: CRDs ship with the operator and are read back through _crd.tpl so RBAC and hook env can't drift from the schema, hook enablement and crontabs live in the operator chart's own values behind a checksum/ annotation that actually rolls the pod, a verify-hooks initContainer fails loudly on a missing hook, RBAC ships with the Deployment, probes are wired to 9115, templates are back to .yaml.tpl so the .pre-commit-config.yaml/.yamllint.yaml broadening is reverted, the plugins CRs are their own Application with a $deploy/<site>/ source, and the pr-2205 tag is gone. The prune-ownership and abort-on-first-failure bugs went away with the split.

Found 4 issues:

1. extraEnv and extraEnvFrom are declared in values but never rendered, so setting them silently does nothing

understack/components/openstack-sync-operator/values.yaml

Lines 24 to 27 in 893a4c3

	extraEnv: []
	extraEnvFrom: []

git grep extraEnv components/openstack-sync-operator/ matches only those two lines — no template reads them. The container's env: is built solely from OS_CLOUD, POD_NAMESPACE, and the derived hook vars, and there is no envFrom: at all:

understack/components/openstack-sync-operator/templates/deployment.yaml.tpl

Lines 96 to 108 in 893a4c3

	failureThreshold: 3
	env:
	- name: OS_CLOUD
	value: {{ .Values.openstack.cloud | quote }}
	- name: POD_NAMESPACE
	valueFrom:
	fieldRef:
	fieldPath: metadata.namespace
	{{- range $envName := keys $hookEnv | sortAlpha }}
	- name: {{ $envName }}
	value: {{ get $hookEnv $envName | quote }}
	{{- end }}
	volumeMounts:

Every other pod-level value in this chart is consumed (podAnnotations, podLabels, resources, nodeSelector, tolerations, affinity, rbac.rules), so these two are the outliers. values.schema.json is additionalProperties: true at the top level, so it won't flag them either. Either render them or drop them — a site that sets extraEnv to inject a proxy or an extra credential gets no error and no effect.

2. The image needs openstacksdk, python-openstackclient and python-ironicclient

understack/python/openstack-sync/pyproject.toml

Lines 4 to 13 in 893a4c3

	[project]
	name = "openstack-sync"
	description = "Shell-operator hooks for OpenStack reconciliation"
	authors = [{ name = "Understack Developers" }]
	requires-python = ">=3.12"
	readme = "README.md"
	license = { text = "Apache-2.0" }
	dynamic = ["version"]

[project] has no dependencies key at all. The previous revision at least had openstacksdk, so this went backwards. The Dockerfile installs python3 plus the package itself and nothing else:

understack/containers/openstack-sync-operator/Dockerfile

Lines 12 to 19 in 893a4c3

	COPY python/openstack-sync /src/openstack-sync
	RUN --mount=type=cache,target=/root/.cache/uv \
	uv pip install \
	--upgrade \
	/src/openstack-sync
	COPY --chmod=755 python/openstack-sync/openstack_sync/hooks/placeholder.py /hooks/placeholder.py

Meanwhile the chart already mounts clouds.yaml and sets OS_CLOUD, which only means something if a client is present:

understack/components/openstack-sync-operator/templates/deployment.yaml.tpl

Lines 97 to 112 in 893a4c3

	env:
	- name: OS_CLOUD
	value: {{ .Values.openstack.cloud | quote }}
	- name: POD_NAMESPACE
	valueFrom:
	fieldRef:
	fieldPath: metadata.namespace
	{{- range $envName := keys $hookEnv | sortAlpha }}
	- name: {{ $envName }}
	value: {{ get $hookEnv $envName | quote }}
	{{- end }}
	volumeMounts:
	- name: openstack-clouds
	mountPath: /etc/openstack/clouds.yaml
	subPath: clouds.yaml
	readOnly: true

Please add all three — openstacksdk for hooks that talk to the APIs directly, and python-openstackclient plus python-ironicclient so a hook can shell out to openstack / baremetal. That matches containers/shell-operator-ironic/requirements.txt, and it is what #2208 will need when the Ironic runbook operator moves onto this image. The point of this container is to be the shared plugin host, so its dependency set should be stable across hooks rather than growing with each one.

3. neutronRouterFlavors is wired up but router_flavors.py doesn't exist

understack/components/openstack-sync-operator/values.yaml

Lines 32 to 41 in 893a4c3

	pluginData:
	neutronRouterFlavors:
	hook:
	path: /hooks/router_flavors.py
	crd: crds/neutron.understack.rackspace.net_neutronrouterflavors.yaml
	envPrefix: NEUTRON_ROUTER_FLAVOR
	env:
	SYNC_CRONTAB: "0 * * * *"
	PRUNE: "false"

pluginData.neutronRouterFlavors.hook.path points at /hooks/router_flavors.py, which is nowhere in the branch — the Dockerfile only copies placeholder.py:

understack/containers/openstack-sync-operator/Dockerfile

Lines 18 to 19 in 893a4c3

	COPY --chmod=755 python/openstack-sync/openstack_sync/hooks/placeholder.py /hooks/placeholder.py

So the commit titled "Adding first consumer Neutron router flavors to sync operator" adds the CRD, four CRs and the values scaffolding but no consumer, and flipping the one boolean the docs call the enablement step puts the Deployment into Init:Error via verify-hooks. Both toggles default to false and the docs do warn to enable it "only after the image contains the hook", so nothing breaks as shipped — but a plugin entry that can only ever fail is a trap for the next person. Either drop the neutronRouterFlavors values and CRs so the framework lands genuinely on its own, or file a tracking issue for the hook the way #2208 and #2210 are tracked and reference it from the commit message.

4. The two new component docs skip the doc-macro convention

understack/docs/deploy-guide/components/openstack-sync-operator.md

Lines 1 to 4 in 893a4c3

	# openstack-sync-operator
	Deploys the OpenStack sync shell-operator in the OpenStack namespace.

All 54 existing pages under docs/deploy-guide/components/ open with frontmatter (charts:/kustomize_paths: plus deploy_overrides:) and call {{ component_argocd_builds() }} and {{ secrets_disclaimer }} from docs/macros.py. Both new pages have no frontmatter and hand-write that prose instead — see the "How ArgoCD Builds It" section listing the value files literally. Commit f4bd7159 converted all 52 pages at the time specifically to stop hand-copying this, and scripts/check-component-docs.py only checks that the file exists, so CI won't catch the regression. Compare undersync.md.

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[cardoe]

Code review — pass 4 (0b0ef7aa)

Pass 3's four issues are all addressed: extraEnv/extraEnvFrom are gone, openstacksdk + python-openstackclient + python-ironicclient are declared in pyproject.toml, router_flavors.py exists and is COPY'd into the image, and both docs pages now carry the frontmatter and {{ component_argocd_builds() }}.

Found 2 issues:

1. The operator doc states reconciliation behavior that router_flavors.py does not implement. main() only handles --config; on a CR event or a scheduled run shell-operator invokes the hook with no arguments and it returns 0 without reading $BINDING_CONTEXT_PATH, importing openstacksdk, or patching status — nothing in python/openstack-sync/ does. The commit subject says "(interface only)" and the PR body points at feat(openstack-sync-plugins): neutron router flavor #2217, but neither reaches the published page, which asserts the behavior as present fact and only caveats the disabled state. Follow the enablement step at L113-L119 and you get a pod that passes verify-hooks, generated RBAC, green probes, applied CRs, and no Neutron changes, with no error logged.

understack/docs/deploy-guide/components/openstack-sync-operator.md

Lines 161 to 166 in 0b0ef7a

	For Neutron router flavors, the enabled hook watches `NeutronRouterFlavor` CRs,
	runs on the configured schedule, reads a full snapshot of current CRs, reconciles
	Neutron flavors and service profiles through `openstacksdk`, and patches CR
	status when the CRD exposes the status subresource.

understack/python/openstack-sync/openstack_sync/hooks/router_flavors.py

Lines 70 to 75 in 0b0ef7a

	def main() -> int:
	if len(sys.argv) > 1 and sys.argv[1] == "--config":
	print(json.dumps(build_hook_config(), indent=2))
	return 0

2. A plugins key with no matching pluginData entry is silently ignored. configuredHooks ranges over .Values.pluginData and looks each name up in .Values.plugins, never the reverse, and both deployment.yaml.tpl and rbac.yaml.tpl consume only its output. So a misspelled or renamed key in $deploy/<site>/openstack-sync-operator/values.yaml — e.g. plugins.neutronRouterFlavor: true — yields no env var, no RBAC rule, no verify-hooks entry and no error; helm template succeeds and ArgoCD reports Synced/Healthy. values.schema.json accepts any boolean-valued key, so it can't catch it either. This is the residual case of the gap verify-hooks was added to close, and deployment.yaml.tpl already uses fail for the duplicate-env-var case, so the same treatment fits.

understack/components/openstack-sync-operator/templates/_helpers.tpl

Lines 76 to 85 in 0b0ef7a

	{{- $enabledPlugins := default dict .Values.plugins -}}
	{{- range $pluginName, $plugin := default dict .Values.pluginData -}}
	{{- $hook := default dict $plugin.hook -}}
	{{- if gt (len $hook) 0 -}}
	{{- $hookValues := dict -}}
	{{- range $key, $value := $hook -}}
	{{- $_ := set $hookValues $key $value -}}
	{{- end -}}
	{{- $_ := set $hookValues "enabled" (eq (get $enabledPlugins $pluginName) true) -}}
	{{- $_ := set $hooks $pluginName $hookValues -}}

understack/components/openstack-sync-operator/values.schema.json

Lines 12 to 19 in 0b0ef7a

	},
	"plugins": {
	"type": "object",
	"description": "Built-in plugin enablement keyed by plugin name.",
	"additionalProperties": {
	"type": "boolean"
	}
	},

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[cardoe]

Pass 4 disposition — merging, follow-ups to come

Four items to carry into a follow-on. None block the merge: the framework shape is what I was after, and both site.openstack_sync_operator.enabled and site.openstack_sync_plugins.enabled default to false, so none of this is live on merge.

1. The operator doc describes reconciliation that router_flavors.py doesn't implement. main() only handles --config; on a CR event or a scheduled run shell-operator invokes the hook with no arguments and it returns 0 without reading $BINDING_CONTEXT_PATH, importing openstacksdk, or patching status. The commit subject says "(interface only)" and the PR body points at #2217, but neither reaches the published page, which states the behavior as present fact and only caveats the disabled state. Following the enablement step at L113-L119 gives a pod that passes verify-hooks, generated RBAC, green probes, applied CRs, and no Neutron changes, with no error logged. Either tone the doc down to match "interface only" or land it alongside #2217.

understack/docs/deploy-guide/components/openstack-sync-operator.md

Lines 161 to 166 in 0b0ef7a

	For Neutron router flavors, the enabled hook watches `NeutronRouterFlavor` CRs,
	runs on the configured schedule, reads a full snapshot of current CRs, reconciles
	Neutron flavors and service profiles through `openstacksdk`, and patches CR
	status when the CRD exposes the status subresource.

understack/python/openstack-sync/openstack_sync/hooks/router_flavors.py

Lines 70 to 75 in 0b0ef7a

	def main() -> int:
	if len(sys.argv) > 1 and sys.argv[1] == "--config":
	print(json.dumps(build_hook_config(), indent=2))
	return 0

2. A plugins key with no matching pluginData entry is silently ignored. configuredHooks ranges over .Values.pluginData and looks each name up in .Values.plugins, never the reverse, and both deployment.yaml.tpl and rbac.yaml.tpl consume only its output. A misspelled or renamed key in $deploy/<site>/openstack-sync-operator/values.yaml yields no env var, no RBAC rule, no verify-hooks entry and no error — helm template succeeds and ArgoCD reports Synced/Healthy. values.schema.json takes any boolean-valued key, so it won't catch it either. This is the residual case of the gap verify-hooks was added to close, and deployment.yaml.tpl already uses fail for the duplicate-env-var case, so the same treatment fits.

understack/components/openstack-sync-operator/templates/_helpers.tpl

Lines 76 to 85 in 0b0ef7a

	{{- $enabledPlugins := default dict .Values.plugins -}}
	{{- range $pluginName, $plugin := default dict .Values.pluginData -}}
	{{- $hook := default dict $plugin.hook -}}
	{{- if gt (len $hook) 0 -}}
	{{- $hookValues := dict -}}
	{{- range $key, $value := $hook -}}
	{{- $_ := set $hookValues $key $value -}}
	{{- end -}}
	{{- $_ := set $hookValues "enabled" (eq (get $enabledPlugins $pluginName) true) -}}
	{{- $_ := set $hooks $pluginName $hookValues -}}

3. Both sources of application-openstack-sync-plugins.yaml carry a ref: that is never dereferenced. There's no helm.valueFiles and no $understack//$deploy/ usage anywhere in the Application, so neither ref does anything. To be clear this isn't introduced here — application-global-workflows.yaml and application-argo-events-workflows.yaml have the identical two-source pattern and survived the cleanups in 92ee95e and 8f9e2d7, which only touched single-source apps. Since those refs are what tripped argoproj/argo-cd#25460, I'd rather clean up all three together than let a fourth copy accumulate.

understack/charts/argocd-understack/templates/application-openstack-sync-plugins.yaml

Lines 22 to 28 in 0b0ef7a

	- path: components/openstack-sync-plugins
	ref: understack
	repoURL: {{ include "understack.understack_url" $ }}
	targetRevision: {{ include "understack.understack_ref" $ }}
	- path: {{ include "understack.deploy_path" $ }}/openstack-sync-plugins
	ref: deploy
	repoURL: {{ include "understack.deploy_url" $ }}

4. rbac.yaml.tpl calls hookCrd unguarded, so an enabled hook without a CRD can't render. _crd.tpl does required "...crd is required when hook is enabled", so helm template --set hooks.myhook.enabled=true --set hooks.myhook.path=/hooks/x.py --set hooks.myhook.envPrefix=MYHOOK fails outright at rbac.yaml.tpl:10. The chart contradicts itself here: deployment.yaml.tpl guards the same call with if $envPrefix / if or $hookEnabled $crdPath, and values.schema.json declares crd optional. It fails loudly rather than silently and every hook we have planned has a CRD, so it's low priority — but a cron-only hook is a shape the framework should support.

understack/components/openstack-sync-operator/templates/rbac.yaml.tpl

Lines 8 to 12 in 0b0ef7a

	{{- range $hookName, $hook := default dict $configuredHooks }}
	{{- if eq $hook.enabled true }}
	{{- $crd := include "openstack-sync-operator.hookCrd" (list $ $hookName $hook) | fromYaml }}
	{{- $rules = append $rules (dict "apiGroups" (list $crd.group) "resources" (list $crd.plural) "verbs" (list "get" "list" "watch")) }}
	{{- if $crd.hasStatus }}

understack/components/openstack-sync-operator/templates/_crd.tpl

Lines 7 to 10 in 0b0ef7a

	{{- $hook := index . 2 -}}
	{{- $crdPath := required (printf "hooks.%s.crd is required when hook is enabled" $hookName) $hook.crd -}}
	{{- $crdYaml := required (printf "hooks.%s.crd file %s is empty or missing" $hookName $crdPath) ($root.Files.Get $crdPath) -}}
	{{- $crd := fromYaml $crdYaml -}}

Merging as-is and opening a follow-on for these.