feat(openstack-sync-operator): manage ironic runbooks using openstack-sync-operator

[haseebsyed12]

Introduces declarative Ironic runbook management through OpenStack Sync.

Sites can now define runbooks as Kubernetes CRs, keep them in the deployment repo, and let the operator converge Ironic state from Git. That makes runbook ownership, updates, pruning, and status reporting part of the same GitOps workflow used for other OpenStack sync resources.

The design keeps shared framework code limited to generic sync mechanics: binding-context handling, credential grouping, status updates, pruning orchestration, and reusable OpenStack pagination. Ironic-specific behavior stays in the runbook plugin, including API microversion requirements, trait matching, owner/public handling, and runbook deletion rules.

What does this change do?

Upgrade impact

• This change requires operator action to upgrade. If checked, add the
  upgrade-impact label and a release note: run scriv create from the
  repository root and describe the required action in the generated
  changelog.d/ file. See RELEASING.md.

Operator action means anything a deployment has to do beyond a normal resync:
deploy repo or values changes, new or removed secrets, enabling or disabling a
component, or a manual one-time step.

[cardoe]

Overall this is a good clean up and improvement. Six inline comments. The two I'd like resolved before this merges are the CRD ownership handoff (data loss on upgrade) and the /owner patch (403 loop that parks every CR in Failed); the rest are smaller.

One thing that didn't fit inline because the files aren't in this diff — stale docs:

• docs/operator-guide/baremetal-ironic-cleanup-runbook.md (around line 502) still points at runbook-crd and runbook-crd/samples as where the CRD and samples live. Those paths still exist on disk after this PR but are no longer rendered by any kustomization, so anyone following the doc edits files that are never applied — the worst kind of stale, since it looks like it worked.
• docs/operator-guide/server-firmware-update.md still describes the legacy shell-operator hooks.

Both should move to components/openstack-sync-plugins/ironic-runbooks/examples/.

For the record, things I specifically checked that are fine:

• The 1.112 microversion floor is right — that's where runbook description and the /runbooks/{id}/traits sub-resource land, and traits are correctly kept out of the create/patch bodies.
• Step payload shape matches RUNBOOK_STEP_SCHEMA.
• The public-transition path is fine; Ironic nulls owner itself when /public is patched.
• Proxy.request defaults to raise_exc=False, so the explicit raise_from_response and NotFoundException handling is doing real work, not dead code.
• Narrowing prune_credentials from _credentials(desired) to _credentials(changed) in framework.py reads as a deliberate tightening and is safe.

All 263 existing tests pass on the branch.

---

Generated by Claude Code

[cardoe]

We're dropping ./runbook-crd but leaving ./runbook-operator (shell-operator-ironic) deployed, and its RBAC in runbook-operator/role.yaml still grants it ironicrunbooks. So with plugins.ironicRunbooks: true we have two operators reconciling the same IronicRunbook objects against the same Ironic API.

That's not just redundant, the two fight each other:

• The legacy hook's status patch doesn't set observedGeneration, so the new operator's _status_is_current never sees a current status and re-syncs. Its own patch then trips the legacy hook. That's a permanent 30s reconcile ping-pong hitting Ironic, with no CR change driving it.
• The legacy hook rewrites extra without preserving the _understack_runbook_* markers, so it strips exactly the ownership markers is_managed_runbook needs. Once they're gone, prune stops recognizing its own runbooks and silently keeps orphans forever.

Can we remove ./runbook-operator in this PR too, or gate it so only one of the two is ever active?

---

Generated by Claude Code

[cardoe]

This is the one that worries me most, because it's data loss on upgrade rather than a bug we can patch later.

components/ironic/runbook-crd/bases/baremetal.ironicproject.org_runbooks.yaml defines ironicrunbooks.baremetal.ironicproject.org — the same CRD name and kind: IronicRunbook that components/openstack-sync-operator/crds/ now ships. Dropping this line doesn't delete the directory, it just stops rendering it, so the CRD moves from the ironic Application to the openstack-sync-operator Application. Both run with prune: true.

ArgoCD gives us no ordering guarantee between two Applications, so the ironic app is free to prune the CRD before (or after) the sync-operator app adopts it. A pruned CRD cascade-deletes every IronicRunbook in the cluster, including bmc-maintenance, which this PR moves to openstack-sync-plugins/ironic-runbooks/examples/ — i.e. somewhere that is never applied. So the CR we ship today just disappears with nothing recreating it.

Two more things on top of that:

• The new CRD makes spec.cloudCredentialsRef required. Any site manifest written against the old schema gets rejected on apply, not migrated.
• No upgrade-impact label and no changelog.d/ fragment, so operators get no warning that they need to re-apply their runbooks.

Options I'd be happy with: keep the CRD rendered from components/ironic for one release and have the sync-operator app not own it yet, or add explicit sync-wave ordering plus an upgrade-impact note documenting that runbook CRs must be re-applied. Either way I think we need bmc-maintenance to stay in an applied path.

---

Generated by Claude Code

[cardoe]

What if we moved the new CRDs to the understack.rackspace.net namespace that we've discussed about all of these?

[cardoe]

This elif will 403 on the second reconcile of every non-public runbook we create.

On a project-scoped create Ironic assigns owner = project_id itself. So for a CR that leaves spec.owner unset — which the CRD explicitly recommends ("leave unset to let Ironic assign the credentials' own project") — the first sync creates the runbook, Ironic sets owner, and the next reconcile sees spec.owner falsy and existing["owner"] populated and emits a remove of /owner.

Patching /owner is gated on baremetal:runbook:update:owner, which defaults to ROLE:admin and SYSTEM_SCOPED:True upstream. Our project-scoped credentials don't have that, so we get a 403 → CR parks in Failed → the exception propagates and prune never runs for that credential.

I think the fix is to only clear owner when the CR previously set it and now doesn't, rather than whenever it's unset. Tracking that needs the marker in extra (or just treating unset as "don't manage this field"). The simpler read of the CRD docs is the latter — if unset means "let Ironic decide", we should never patch it.

Worth a test that reconciles twice against a stub that returns owner after create; that'd have caught this.

---

Generated by Claude Code

[cardoe]

Two problems in these three lines.

page[-1][marker_key] isn't guarded. list_runbooks filters the page down to dicts, but that happens after we've already indexed the raw last element here. Any non-dict item, or a dict Ironic returns without uuid, raises TypeError/KeyError out of pagination — which aborts the caller's prune rather than degrading. Since we bothered to filter downstream, we should be equally defensive here (use get_value, and bail with a warning if the marker is missing).

len(page) < page_limit is the wrong terminator when the server clamps. page_limit is 100, but Ironic caps every collection response at [api] max_limit (default 1000, though operators do lower it). If a deployment sets max_limit below our page_limit, every page comes back short, we return on the first one, and prune only ever sees the first max_limit runbooks. Orphans past that point are never deleted and nothing logs a word about it.

Terminating on an empty page instead of a short one handles both the clamp and the exact-multiple case, at the cost of one extra request. Given this feeds prune, I'd rather pay the request than silently under-prune.

---

Generated by Claude Code

[cardoe]

ironicUUID is declared here but nothing ever writes it — patch_resource_status has no parameter for it, and sync_runbook only logs the UUID.

This is the field an operator would reach for to correlate a CR with openstack baremetal runbook show, so it's worth wiring up rather than leaving as a permanently-empty field. Either thread it through patch_resource_status or drop it from the schema until we do.

---

Generated by Claude Code

[haseebsyed12]

implemented dropping of ironicUUID