Skip to content

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

Merged
cardoe merged 7 commits into
mainfrom
openstack-sync-plugin-ironic-runbooks
Sep 8, 2026
Merged

feat(openstack-sync-operator): manage ironic runbooks using openstack-sync-operator#2314
cardoe merged 7 commits into
mainfrom
openstack-sync-plugin-ironic-runbooks

Conversation

@haseebsyed12

@haseebsyed12 haseebsyed12 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

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 cardoe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread components/ironic/kustomization.yaml Outdated
Comment thread components/ironic/kustomization.yaml
Comment thread python/openstack-sync/openstack_sync/plugins/ironic/runbooks/reconcile.py Outdated
Comment thread python/openstack-sync/openstack_sync/plugins/common.py Outdated
Comment thread python/openstack-sync/openstack_sync/plugins/ironic/runbooks/prune.py Outdated
@haseebsyed12
haseebsyed12 force-pushed the openstack-sync-plugin-ironic-runbooks branch from 227ac8e to ed30c35 Compare September 7, 2026 10:34
@haseebsyed12
haseebsyed12 force-pushed the openstack-sync-plugin-ironic-runbooks branch from ed30c35 to b78eed6 Compare September 7, 2026 18:57
A project-scoped create is assigned the caller's own project, so a CR that
names no owner reads one back on every reconcile after the first. Patching
/owner is gated on baremetal:runbook:update:owner (SYSTEM_MEMBER), so the
operator was emitting a 403 on the second pass of every runbook created with
a project-scoped credential, parking the CR in Failed and skipping prune.

An unset spec.owner now means "do not manage this field", which is the
reading the CRD already documented. Clearing an owner is still possible by
setting public, which Ironic does itself.

Also corrects what public promises. It governs visibility, not use:
runbook:get falls back to the runbook being public and a project-scoped list
filters on owner == project OR public, but runbook:use resolves the owner
alone, so a public runbook is usable only by a system-scoped or role:service
token. The CRD, schema, examples and the status note said it made the runbook
usable by every project.

Adds coverage for the credential-scope matrix: two reconciles against a fake
that refuses /owner and /public the way the policy does, the public-to-owned
transition Ironic only accepts atomically, and the two operations a
project-scoped credential cannot perform at all.
@haseebsyed12 haseebsyed12 added the upgrade-impact Requires operator action to upgrade; needs a changelog.d/ release note fragment label Sep 8, 2026
@haseebsyed12
haseebsyed12 requested a review from cardoe September 8, 2026 18:04
@haseebsyed12
haseebsyed12 force-pushed the openstack-sync-plugin-ironic-runbooks branch from 9ba20a6 to 42618e7 Compare September 8, 2026 18:48
@cardoe
cardoe added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit b6d54e2 Sep 8, 2026
69 checks passed
@cardoe
cardoe deleted the openstack-sync-plugin-ironic-runbooks branch September 8, 2026 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

upgrade-impact Requires operator action to upgrade; needs a changelog.d/ release note fragment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants