diff --git a/.github/workflows/containers.yaml b/.github/workflows/containers.yaml index 2accadca6..954bece85 100644 --- a/.github/workflows/containers.yaml +++ b/.github/workflows/containers.yaml @@ -12,7 +12,6 @@ on: - "containers/dnsmasq/**" - "containers/ironic-nautobot-client/**" - "containers/ironic-vnc-container/**" - - "containers/shell-operator-ironic/**" - "containers/openstack-sync-operator/**" - "containers/understack-tests/**" - "python/**" @@ -44,8 +43,6 @@ jobs: context_path: "./containers/ironic-vnc-container/" prebuild_script: ./sync_from_upstream.sh prebuild_script_working_dir: containers/ironic-vnc-container/ - - name: shell-operator-ironic - target: prod - name: openstack-sync-operator target: prod - name: nautobot diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index eb8ac3001..daa28f6c5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -111,7 +111,7 @@ repos: - id: component-docs-check name: Component Docs Check description: Ensure every ArgoCD Application template has a component docs page. - entry: python scripts/check-component-docs.py + entry: python3 scripts/check-component-docs.py language: system files: '^charts/argocd-understack/templates/application-.*\.yaml$|^docs/deploy-guide/components/.*\.md$' - id: trufflehog diff --git a/components/ironic/kustomization.yaml b/components/ironic/kustomization.yaml index 897b75690..fe935660f 100644 --- a/components/ironic/kustomization.yaml +++ b/components/ironic/kustomization.yaml @@ -11,8 +11,6 @@ resources: # less than ideal addition but necessary so that we can have the ironic.conf.d loading # working due to the way the chart hardcodes the config-file parameter which then # takes precedence over the directory - - ./runbook-crd - - ./runbook-operator # Alerting - pr-clean-failed-servers.yaml - pr-resource-availability.yaml diff --git a/components/ironic/runbook-crd/README.md b/components/ironic/runbook-crd/README.md deleted file mode 100644 index da43dbb71..000000000 --- a/components/ironic/runbook-crd/README.md +++ /dev/null @@ -1,154 +0,0 @@ -# Ironic Runbook Kubernetes CRD - -Kubernetes Custom Resource Definition (CRD) for managing Ironic baremetal runbooks. Runbooks define automated sequences of operations (cleaning, configuration, firmware updates) to be executed on baremetal nodes. - -## What is a Runbook? - -A Runbook is a collection of ordered steps that define automated operations on baremetal nodes in Ironic. Runbooks enable: - -- **Automated Cleaning**: Prepare nodes for reuse (disk wiping, BIOS config, firmware updates) -- **Declarative Workflows**: Define repeatable, version-controlled sequences -- **Trait-Based Matching**: Runbooks match to nodes when the runbook name matches a node trait - -## Quick Start - -### Installation - -```bash -# Install the CRD -kubectl apply -f bases/baremetal.ironicproject.org_runbooks.yaml -``` - -### Create Your First Runbook - -```bash -# Apply a minimal example -kubectl apply -f samples/runbook_v1alpha1_minimal.yaml - -# Verify it was created -kubectl get runbooks -kubectl describe runbook minimal-runbook -``` - -### View Available Samples - -```bash -# List all sample runbooks -ls samples/ - -# Apply a specific sample -kubectl apply -f samples/runbook_bios_config.yaml -``` - -## Field Requirements - -### ✅ Required Fields - -| Field | Type | Description | -|-------|------|-------------| -| `spec.runbookName` | string | Runbook name matching CUSTOM_* pattern | -| `spec.steps` | array | Ordered list of steps (minimum 1) | -| `steps[].interface` | enum | Hardware interface (bios, raid, deploy, etc.) | -| `steps[].step` | string | Step name (non-empty) | -| `steps[].order` | integer | Execution order (>= 0, unique) | - -### ❌ Optional Fields - -| Field | Type | Default | Description | -|-------|------|---------|-------------| -| `spec.disableRamdisk` | boolean | `false` | Skip ramdisk booting | -| `spec.public` | boolean | `false` | Public accessibility | -| `spec.owner` | string | `null` | Project/tenant owner | -| `spec.extra` | object | `{}` | Additional metadata | -| `steps[].args` | object | `{}` | Step-specific arguments | - -## Minimal Example - -```yaml -apiVersion: baremetal.ironicproject.org/v1alpha1 -kind: IronicRunbook -metadata: - name: minimal-runbook - namespace: default -spec: - runbookName: CUSTOM_MINIMAL - steps: - - interface: deploy - step: erase_devices - order: 1 -``` - -## Sample Runbooks - -| Sample | Use Case | Description | -|--------|----------|-------------| -| `runbook_v1alpha1_minimal.yaml` | Learning | Minimal example with required fields only | -| `runbook_v1alpha1_complete.yaml` | Reference | Complete example with all fields | -| `runbook_bios_config.yaml` | Compute Nodes | BIOS configuration for virtualization | -| `runbook_raid_config.yaml` | Storage Nodes | RAID setup (OS + data volumes) | -| `runbook_firmware_update.yaml` | Maintenance | Firmware updates (BIOS, BMC, NIC) | -| `runbook_disk_cleaning.yaml` | Node Reuse | Secure disk erasure | -| `runbook_gpu_node_setup.yaml` | ML/AI | GPU node configuration | - -## Running a Runbook - -Once the operator syncs the CRD into Ironic, you can execute a runbook against -a node using one of two CLI commands depending on the node's current -provisioning state: - -- **`node clean --runbook`** — node must be in `manageable` state -- **`node service --runbook`** — node must be in `active` or `available` state - -### OpenStack CLI - -```bash -# For nodes in 'manageable' state -openstack baremetal node clean --runbook CUSTOM_BMC_MAINTENANCE - -# For nodes in 'active' or 'available' state -openstack baremetal node service --runbook CUSTOM_BMC_MAINTENANCE - -# Check node state while the runbook executes -openstack baremetal node show -f value -c provision_state -``` - -### Python SDK - -```python -from understack_workflows.ironic_node import transition - -# node must already be in manageable state -transition( - node, - "clean", - expected_state="manageable", - runbook=runbook_uuid, -) -``` - -The `transition` helper calls `set_node_provision_state` and waits for the -node to return to `manageable` once all steps complete. - -### Trait-Based Automatic Execution - -Runbooks can also be triggered automatically by matching node traits. Add the -runbook name as a trait on the node: - -```bash -openstack baremetal node add trait CUSTOM_BMC_MAINTENANCE -``` - -Workflow code (e.g. `apply_firmware_updates` in `ironic_node.py`) can then -discover matching traits and execute the corresponding runbooks in order. - -## Support - -- **Ironic Documentation**: https://docs.openstack.org/ironic/latest/ -- **Kubernetes CRDs**: https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/ - ---- - -**Version**: v1alpha1 -**API Group**: baremetal.ironicproject.org -**Kind**: IronicRunbook -**Short Name**: rb diff --git a/components/ironic/runbook-crd/bases/baremetal.ironicproject.org_runbooks.yaml b/components/ironic/runbook-crd/bases/baremetal.ironicproject.org_runbooks.yaml deleted file mode 100644 index 437ceafd2..000000000 --- a/components/ironic/runbook-crd/bases/baremetal.ironicproject.org_runbooks.yaml +++ /dev/null @@ -1,198 +0,0 @@ -apiVersion: apiextensions.k8s.io/v1 -kind: CustomResourceDefinition -metadata: - name: ironicrunbooks.baremetal.ironicproject.org - annotations: - controller-gen.kubebuilder.io/version: v0.13.0 -spec: - group: baremetal.ironicproject.org - names: - kind: IronicRunbook - listKind: IronicRunbookList - plural: ironicrunbooks - singular: ironicrunbook - shortNames: - - rb - scope: Namespaced - versions: - - name: v1alpha1 - served: true - storage: true - schema: - openAPIV3Schema: - description: IronicRunbook represents a collection of ordered steps that define automated operations on baremetal nodes - type: object - required: - - spec - properties: - apiVersion: - description: 'APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' - type: string - kind: - description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' - type: string - metadata: - type: object - spec: - description: IronicRunbookSpec defines the desired state of IronicRunbook - type: object - required: - - runbookName - - steps - properties: - runbookName: - description: 'RunbookName is the unique name of the runbook (REQUIRED). From API microversion 1.112+, this is a logical identifier and can be any string of 1-255 characters. Node eligibility is determined by the traits field instead.' - type: string - pattern: '^[a-zA-Z0-9._-]+$' - minLength: 1 - maxLength: 255 - description: - description: 'Description is a human-readable description of the runbook (OPTIONAL). Consistent with other Ironic objects. Available from API microversion 1.112 onwards.' - type: string - nullable: true - maxLength: 1000 - traits: - description: 'Traits is a list of traits that determine which nodes are permitted to use this runbook (OPTIONAL). Decouples runbook eligibility from the runbook name. Each trait must follow the CUSTOM_* naming convention. Available from API microversion 1.112 onwards. Default: []' - type: array - default: [] - items: - type: string - pattern: '^CUSTOM_[A-Z0-9_]+$' - minLength: 1 - maxLength: 255 - steps: - description: 'Steps is an ordered list of operations to execute (REQUIRED). Minimum 1 step required.' - type: array - minItems: 1 - items: - description: RunbookStep defines a single step in the runbook - type: object - required: - - interface - - step - - order - properties: - interface: - description: 'Interface specifies which hardware interface handles this step (REQUIRED). Must be one of the valid Ironic cleaning interfaces.' - type: string - enum: - - bios - - raid - - deploy - - management - - power - - storage - - vendor - - rescue - - console - - boot - - inspect - - network - - firmware - step: - description: 'Step is the name of the step to execute (REQUIRED). Must be a valid step name for the specified interface.' - type: string - minLength: 1 - maxLength: 255 - order: - description: 'Order defines the execution sequence (REQUIRED). Must be >= 0 and unique within the runbook. Lower numbers execute first.' - type: integer - minimum: 0 - args: - description: 'Args contains step-specific arguments (OPTIONAL). Structure depends on the interface and step. Default: {}' - type: object - x-kubernetes-preserve-unknown-fields: true - disableRamdisk: - description: 'DisableRamdisk skips booting the ramdisk for cleaning operations (OPTIONAL). Use when steps can run without IPA (Ironic Python Agent). Default: false' - type: boolean - default: false - public: - description: 'Public makes the runbook accessible to all projects/tenants (OPTIONAL). Cannot be true if owner is set. Default: false' - type: boolean - default: false - owner: - description: 'Owner identifies the project/tenant that owns this runbook (OPTIONAL). Cannot be set if public is true. Default: null' - type: string - nullable: true - maxLength: 255 - extra: - description: 'Extra contains additional metadata (OPTIONAL). Use for descriptions, versions, maintainer info, etc. Default: {}' - type: object - x-kubernetes-preserve-unknown-fields: true - status: - description: RunbookStatus defines the observed state of Runbook - type: object - properties: - ironicUUID: - description: IronicUUID is the UUID of this runbook in the Ironic API - type: string - syncStatus: - description: SyncStatus indicates the synchronization state with Ironic - type: string - enum: - - Synced - - Pending - - Failed - - Unknown - lastSyncTime: - description: LastSyncTime is the timestamp of the last successful sync with Ironic - type: string - format: date-time - observedGeneration: - description: ObservedGeneration reflects the generation of the most recently observed Runbook - type: integer - format: int64 - conditions: - description: Conditions represent the latest available observations of the runbook's state - type: array - items: - description: Condition contains details for one aspect of the current state of this API Resource - type: object - required: - - type - - status - - lastTransitionTime - properties: - type: - description: Type of condition (e.g., Ready, Validated, Synced) - type: string - status: - description: Status of the condition (True, False, Unknown) - type: string - enum: - - "True" - - "False" - - Unknown - lastTransitionTime: - description: LastTransitionTime is the last time the condition transitioned from one status to another - type: string - format: date-time - reason: - description: Reason contains a programmatic identifier indicating the reason for the condition's last transition - type: string - message: - description: Message is a human readable message indicating details about the transition - type: string - subresources: - status: {} - additionalPrinterColumns: - - name: Runbook Name - type: string - description: The runbook name - jsonPath: .spec.runbookName - - name: Description - type: string - description: Human-readable description of the runbook - jsonPath: .spec.description - priority: 1 - - name: Public - type: boolean - description: Whether the runbook is public - jsonPath: .spec.public - - name: Sync Status - type: string - description: Synchronization status with Ironic - jsonPath: .status.syncStatus - - name: Age - type: date - jsonPath: .metadata.creationTimestamp diff --git a/components/ironic/runbook-crd/kustomization.yaml b/components/ironic/runbook-crd/kustomization.yaml deleted file mode 100644 index 416ca9728..000000000 --- a/components/ironic/runbook-crd/kustomization.yaml +++ /dev/null @@ -1,10 +0,0 @@ -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization - -# Namespace for runbook resources -namespace: openstack - -# Create namespace if it doesn't exist -resources: - - bases/baremetal.ironicproject.org_runbooks.yaml - - runbooks/runbook_bmc_maintenance.yaml diff --git a/components/ironic/runbook-crd/runbooks/runbook_bmc_maintenance.yaml b/components/ironic/runbook-crd/runbooks/runbook_bmc_maintenance.yaml deleted file mode 100644 index 8b525c907..000000000 --- a/components/ironic/runbook-crd/runbooks/runbook_bmc_maintenance.yaml +++ /dev/null @@ -1,20 +0,0 @@ -apiVersion: baremetal.ironicproject.org/v1alpha1 -kind: IronicRunbook -metadata: - name: bmc-maintenance - namespace: openstack -spec: - runbookName: bmc-maintenance - description: "Performs BMC maintenance operations including clearing the job queue and synchronizing the BMC clock." - disableRamdisk: true - traits: - - CUSTOM_DELL_IDRAC - - steps: - - interface: management - step: clear_job_queue - order: 1 - - - interface: management - step: set_bmc_clock - order: 2 diff --git a/components/ironic/runbook-crd/samples/runbook_v1alpha1_minimal.yaml b/components/ironic/runbook-crd/samples/runbook_v1alpha1_minimal.yaml deleted file mode 100644 index a2c865a01..000000000 --- a/components/ironic/runbook-crd/samples/runbook_v1alpha1_minimal.yaml +++ /dev/null @@ -1,26 +0,0 @@ -# Minimal Runbook Example - Required Fields Only -# -# This example shows the absolute minimum required to create a valid runbook. -# It includes only the 5 required fields: -# 1. spec.runbookName -# 2. spec.steps (array with min 1 step) -# 3. steps[].interface -# 4. steps[].step -# 5. steps[].order -# -# Use this as a starting point and add optional fields as needed. - -apiVersion: baremetal.ironicproject.org/v1alpha1 -kind: IronicRunbook -metadata: - name: minimal-runbook - namespace: default -spec: - # ✅ REQUIRED: Runbook name matching trait convention - runbookName: CUSTOM_MINIMAL - - # ✅ REQUIRED: At least one step - steps: - - interface: deploy # ✅ REQUIRED: Hardware interface - step: erase_devices # ✅ REQUIRED: Step name - order: 1 # ✅ REQUIRED: Execution order (unique) diff --git a/components/ironic/runbook-operator/kustomization.yaml b/components/ironic/runbook-operator/kustomization.yaml deleted file mode 100644 index 5560cd357..000000000 --- a/components/ironic/runbook-operator/kustomization.yaml +++ /dev/null @@ -1,12 +0,0 @@ -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization - -# Namespace for runbook resources -namespace: openstack - -# Create namespace if it doesn't exist -resources: - - service_account.yaml - - role.yaml - - role_binding.yaml - - shell-operator-ironic.yaml diff --git a/components/ironic/runbook-operator/role.yaml b/components/ironic/runbook-operator/role.yaml deleted file mode 100644 index 284d911cd..000000000 --- a/components/ironic/runbook-operator/role.yaml +++ /dev/null @@ -1,27 +0,0 @@ -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: runbook-controller-role - labels: - app.kubernetes.io/name: ironicrunbook - app.kubernetes.io/component: rbac -rules: - # Runbook permissions - - apiGroups: - - baremetal.ironicproject.org - resources: - - ironicrunbooks - verbs: - - get - - list - - watch - - # Status update permissions - - apiGroups: - - baremetal.ironicproject.org - resources: - - ironicrunbooks/status - verbs: - - get - - patch - - update diff --git a/components/ironic/runbook-operator/role_binding.yaml b/components/ironic/runbook-operator/role_binding.yaml deleted file mode 100644 index ac4add3a2..000000000 --- a/components/ironic/runbook-operator/role_binding.yaml +++ /dev/null @@ -1,21 +0,0 @@ -# RoleBinding for Runbook Controller -# -# This binds the runbook-controller-role to a service account. -# Modify the subjects section to bind to your desired users or service accounts. - -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRoleBinding -metadata: - name: runbook-controller-role-rolebinding - labels: - app.kubernetes.io/name: ironicrunbook - app.kubernetes.io/component: rbac -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: runbook-controller-role -subjects: - # Example: Bind to a service account (for controller) - - kind: ServiceAccount - name: runbook-controller - namespace: baremetal-system diff --git a/components/ironic/runbook-operator/service_account.yaml b/components/ironic/runbook-operator/service_account.yaml deleted file mode 100644 index 1426c3dd7..000000000 --- a/components/ironic/runbook-operator/service_account.yaml +++ /dev/null @@ -1,13 +0,0 @@ -# ServiceAccount for Runbook Controller -# -# This service account is used by the runbook controller/operator -# to manage runbook resources and sync with Ironic API. - -apiVersion: v1 -kind: ServiceAccount -metadata: - name: runbook-controller - namespace: baremetal-system - labels: - app.kubernetes.io/name: ironicrunbook - app.kubernetes.io/component: controller diff --git a/components/ironic/runbook-operator/shell-operator-ironic.yaml b/components/ironic/runbook-operator/shell-operator-ironic.yaml deleted file mode 100644 index a14fb1598..000000000 --- a/components/ironic/runbook-operator/shell-operator-ironic.yaml +++ /dev/null @@ -1,34 +0,0 @@ -apiVersion: apps/v1 -kind: Deployment -metadata: - name: shell-operator-ironic - namespace: openstack -spec: - replicas: 1 - selector: - matchLabels: - app: shell-operator - template: - metadata: - labels: - app: shell-operator - spec: - serviceAccountName: runbook-controller - restartPolicy: Always - containers: - - name: shell-operator - image: ghcr.io/rackerlabs/understack/shell-operator-ironic:latest - imagePullPolicy: Always - env: - - name: OS_CLOUD - value: understack - volumeMounts: - - mountPath: /etc/openstack - name: infrasetup-system - volumes: - - name: infrasetup-system - secret: - secretName: infrasetup-system - items: - - key: clouds.yaml - path: clouds.yaml diff --git a/components/openstack-sync-operator/crds/baremetal.ironicproject.org_ironicrunbooks.yaml b/components/openstack-sync-operator/crds/baremetal.ironicproject.org_ironicrunbooks.yaml new file mode 100644 index 000000000..9aabf7f88 --- /dev/null +++ b/components/openstack-sync-operator/crds/baremetal.ironicproject.org_ironicrunbooks.yaml @@ -0,0 +1,218 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: ironicrunbooks.baremetal.ironicproject.org +spec: + group: baremetal.ironicproject.org + names: + kind: IronicRunbook + listKind: IronicRunbookList + plural: ironicrunbooks + shortNames: + - rb + singular: ironicrunbook + scope: Namespaced + versions: + - name: v1alpha1 + served: true + storage: true + additionalPrinterColumns: + - name: Runbook + type: string + jsonPath: .spec.runbookName + - name: Description + type: string + jsonPath: .spec.description + priority: 1 + - name: Public + type: boolean + jsonPath: .spec.public + - name: SyncStatus + type: string + jsonPath: .status.syncStatus + - name: Age + type: date + jsonPath: .metadata.creationTimestamp + schema: + openAPIV3Schema: + description: >- + IronicRunbook defines one Ironic runbook. The operator-owned API + contract keeps OpenStack credentials on every CR so reconciliation can + be grouped by cloud, matching the other openstack-sync plugins. + type: object + required: + - spec + properties: + apiVersion: + type: string + kind: + type: string + metadata: + type: object + spec: + description: IronicRunbookSpec defines the desired runbook data. + type: object + required: + - cloudCredentialsRef + - runbookName + - steps + 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. + type: object + required: + - secretName + - cloudName + properties: + secretName: + description: >- + Name of a Secret in the same namespace as this resource. + The Secret must contain a key named clouds.yaml holding + an OpenStack clouds.yaml file. + type: string + minLength: 1 + maxLength: 253 + cloudName: + description: >- + Name of the cloud entry within the clouds.yaml to + authenticate as. + type: string + minLength: 1 + maxLength: 256 + runbookName: + description: >- + Runbook name, and the identity the operator syncs by. Renaming + creates a new runbook rather than renaming the existing one. + type: string + minLength: 1 + maxLength: 255 + pattern: ^[A-Za-z0-9._~-]+$ + description: + description: Human-readable runbook description. + type: string + maxLength: 255 + traits: + description: >- + Traits deciding which nodes this runbook may act on. A node + must carry at least one; a runbook with no traits matches no nodes. + type: array + default: [] + items: + type: string + minLength: 1 + maxLength: 255 + pattern: ^CUSTOM_[A-Z0-9_]+$ + steps: + description: Ordered runbook steps. + type: array + minItems: 1 + items: + type: object + required: + - interface + - step + - order + properties: + interface: + description: Interface that owns this cleaning step. + type: string + enum: + - bios + - deploy + - firmware + - management + - power + - raid + - vendor + step: + description: Step name for the selected interface. + type: string + minLength: 1 + maxLength: 255 + args: + description: Step-specific arguments. + type: object + x-kubernetes-preserve-unknown-fields: true + order: + description: Execution order. Lower numbers run first. + type: integer + minimum: 0 + disableRamdisk: + description: Whether to run without booting the cleaning ramdisk. + type: boolean + default: false + public: + description: >- + Whether the runbook is available to all projects. A public + runbook cannot have an owner. + type: boolean + default: false + owner: + description: >- + Project that owns this runbook. Leave unset to let Ironic + assign the credentials' own project. + type: string + maxLength: 255 + extra: + description: >- + Additional runbook metadata. The operator also keeps its + ownership markers here, under _understack_runbook_ keys. + type: object + x-kubernetes-preserve-unknown-fields: true + status: + description: IronicRunbookStatus defines the observed sync state. + type: object + properties: + ironicUUID: + description: Ironic UUID of this runbook. + type: string + syncStatus: + description: SyncStatus indicates the synchronization state with Ironic. + type: string + enum: + - Synced + - Failed + - Unknown + lastSyncTime: + description: LastSyncTime is the last time the operator attempted to sync the runbook. + type: string + format: date-time + observedGeneration: + description: ObservedGeneration is the metadata generation last processed by the operator. + type: integer + format: int64 + message: + description: Message provides details about the last sync attempt. + type: string + maxLength: 2048 + conditions: + description: Conditions describe current observed state. + type: array + items: + type: object + required: + - type + - status + properties: + type: + type: string + status: + type: string + enum: + - "True" + - "False" + - Unknown + reason: + type: string + message: + type: string + maxLength: 2048 + lastTransitionTime: + type: string + format: date-time + subresources: + status: {} diff --git a/components/openstack-sync-operator/values.yaml b/components/openstack-sync-operator/values.yaml index b985e59ff..54ca513cf 100644 --- a/components/openstack-sync-operator/values.yaml +++ b/components/openstack-sync-operator/values.yaml @@ -30,6 +30,7 @@ rbac: plugins: openstackPlaceholder: false neutronRouterFlavors: false + ironicRunbooks: false pluginData: openstackPlaceholder: @@ -55,3 +56,18 @@ pluginData: # When true, removing a NeutronRouterFlavor CR also deletes its unused # operator-managed OpenStack flavor. Enable this before removing the CR. PRUNE: false + + ironicRunbooks: + hook: + path: /hooks/ironic_runbooks.py + crd: crds/baremetal.ironicproject.org_ironicrunbooks.yaml + envPrefix: IRONIC_RUNBOOK + env: + SYNC_CRONTAB: "0 * * * *" + # Ironic readiness wait before a runbook reconcile fails. + # Total wait is READY_RETRIES * READY_DELAY seconds. + READY_RETRIES: 30 + READY_DELAY: 10 + # When true, removing an IronicRunbook CR also deletes its + # operator-owned Ironic runbook. Enable this before removing the CR. + PRUNE: false diff --git a/components/openstack-sync-plugins/ironic-runbooks/bmc_maintenance.yaml b/components/openstack-sync-plugins/ironic-runbooks/bmc_maintenance.yaml new file mode 100644 index 000000000..7f01e700c --- /dev/null +++ b/components/openstack-sync-plugins/ironic-runbooks/bmc_maintenance.yaml @@ -0,0 +1,43 @@ +# yaml-language-server: $schema=https://rackerlabs.github.io/understack/schema/openstack-sync/ironic-runbook.schema.json +# BMC Maintenance Runbook +# +# Clears the BMC job queue and resynchronizes the BMC clock on Dell iDRAC nodes. +# Runs without booting the cleaning ramdisk, so it is safe for out-of-band only work. +# +# Only nodes carrying the traits below are eligible. + +apiVersion: baremetal.ironicproject.org/v1alpha1 +kind: IronicRunbook +metadata: + name: bmc-maintenance + namespace: openstack + labels: + app.kubernetes.io/name: openstack-sync-plugins + app.kubernetes.io/component: ironic-runbooks + app.kubernetes.io/part-of: openstack-sync + use-case: bmc-maintenance + hardware-type: general +spec: + cloudCredentialsRef: + # System-scoped credential: Ironic requires system_scope:all to publish a + # runbook, which the project-scoped infrasetup credential cannot satisfy. + secretName: infrasetup-system + cloudName: understack + runbookName: bmc-maintenance + description: "Performs BMC maintenance operations including clearing the job queue and synchronizing the BMC clock." + public: true + disableRamdisk: true + traits: + - CUSTOM_DELL_IDRAC + steps: + - interface: management + step: clear_job_queue + order: 1 + - interface: management + step: set_bmc_clock + order: 2 + extra: + version: "1.0.0" + use_case: "BMC housekeeping and clock synchronization" + warnings: + - "Clearing the job queue discards pending BMC jobs, including scheduled firmware updates" diff --git a/components/openstack-sync-plugins/ironic-runbooks/examples/README.md b/components/openstack-sync-plugins/ironic-runbooks/examples/README.md new file mode 100644 index 000000000..df3f25f46 --- /dev/null +++ b/components/openstack-sync-plugins/ironic-runbooks/examples/README.md @@ -0,0 +1,45 @@ +# IronicRunbook Examples + +Reference CRs for the `ironicRunbooks` openstack-sync hook. **Nothing here is +applied.** The parent `kustomization.yaml` lists only the shared runbooks, and +this directory is not one of its `resources`. + +## Using one + +Copy the file to where the CRs for your site live, usually +`//openstack-sync-plugins/`, add it to that directory's +`kustomization.yaml`, then adjust three things: + +1. `metadata.namespace`: must be the namespace the operator watches + (`POD_NAMESPACE`, commonly `openstack`). The samples use + `baremetal-system` and `default`, which the hook will not see. +2. `spec.cloudCredentialsRef`: the Secret holding `clouds.yaml` and the cloud + entry to authenticate with. +3. `spec.traits`: Ironic only runs a runbook on a node carrying at least one of + them, so a runbook with no traits matches no nodes. + +Step names and arguments in these files are illustrative. Check that the +`interface` and `step` you want exist on the target hardware before relying on +them, and replace the firmware URLs and checksums with real ones. + +## The examples + +| File | Purpose | +|------|---------| +| `runbook_v1alpha1_minimal.yaml` | Smallest valid CR: required fields only | +| `runbook_v1alpha1_complete.yaml` | Every field, with each one annotated | +| `runbook_bios_config.yaml` | BIOS settings for virtualization on compute nodes | +| `runbook_raid_config.yaml` | RAID setup, OS volume plus data volume | +| `runbook_firmware_update.yaml` | BIOS, BMC and NIC firmware updates | +| `runbook_disk_cleaning.yaml` | Disk erasure for node reuse | +| `runbook_gpu_node_setup.yaml` | BIOS and firmware for GPU nodes | + +## Validation + +Editors pick up the published spec schema from the `yaml-language-server` line at +the top of `../bmc_maintenance.yaml`; add the same line to a copied example to +get completion and checking. Kubernetes validates the full CR against the CRD in +`components/openstack-sync-operator/crds/` when ArgoCD applies it. + +Running a synced firmware runbook against a node is covered in +`docs/operator-guide/server-firmware-update.md`. diff --git a/components/ironic/runbook-crd/samples/runbook_bios_config.yaml b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_bios_config.yaml similarity index 82% rename from components/ironic/runbook-crd/samples/runbook_bios_config.yaml rename to components/openstack-sync-plugins/ironic-runbooks/examples/runbook_bios_config.yaml index 5e875bc42..17112a338 100644 --- a/components/ironic/runbook-crd/samples/runbook_bios_config.yaml +++ b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_bios_config.yaml @@ -3,7 +3,7 @@ # This runbook configures BIOS settings for compute nodes. # Common use case: Enabling virtualization features for hypervisor nodes. # -# Matches nodes with trait: CUSTOM_COMPUTE_BIOS +# Selects nodes carrying the trait: CUSTOM_COMPUTE_BIOS apiVersion: baremetal.ironicproject.org/v1alpha1 kind: IronicRunbook @@ -14,8 +14,17 @@ metadata: use-case: bios-configuration hardware-type: compute spec: + # REQUIRED: Secret holding the clouds.yaml, and the cloud entry to use. + cloudCredentialsRef: + secretName: infrasetup + cloudName: understack + runbookName: CUSTOM_COMPUTE_BIOS + # Nodes must carry at least one of these traits for this runbook to act on them. + traits: + - CUSTOM_COMPUTE_BIOS + steps: - interface: bios step: apply_configuration diff --git a/components/ironic/runbook-crd/samples/runbook_disk_cleaning.yaml b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_disk_cleaning.yaml similarity index 79% rename from components/ironic/runbook-crd/samples/runbook_disk_cleaning.yaml rename to components/openstack-sync-plugins/ironic-runbooks/examples/runbook_disk_cleaning.yaml index 2599b4698..80b540973 100644 --- a/components/ironic/runbook-crd/samples/runbook_disk_cleaning.yaml +++ b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_disk_cleaning.yaml @@ -3,7 +3,7 @@ # This runbook performs secure disk erasure for node reuse. # Common use case: Preparing nodes for redeployment or decommissioning. # -# Matches nodes with trait: CUSTOM_DISK_CLEAN +# Selects nodes carrying the trait: CUSTOM_DISK_CLEAN apiVersion: baremetal.ironicproject.org/v1alpha1 kind: IronicRunbook @@ -14,8 +14,17 @@ metadata: use-case: disk-cleaning security-level: standard spec: + # REQUIRED: Secret holding the clouds.yaml, and the cloud entry to use. + cloudCredentialsRef: + secretName: infrasetup + cloudName: understack + runbookName: CUSTOM_DISK_CLEAN + # Nodes must carry at least one of these traits for this runbook to act on them. + traits: + - CUSTOM_DISK_CLEAN + steps: # Step 1: Erase all devices - interface: deploy diff --git a/components/ironic/runbook-crd/samples/runbook_firmware_update.yaml b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_firmware_update.yaml similarity index 88% rename from components/ironic/runbook-crd/samples/runbook_firmware_update.yaml rename to components/openstack-sync-plugins/ironic-runbooks/examples/runbook_firmware_update.yaml index 943d89773..d6959fdd1 100644 --- a/components/ironic/runbook-crd/samples/runbook_firmware_update.yaml +++ b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_firmware_update.yaml @@ -3,7 +3,7 @@ # This runbook updates firmware components on baremetal nodes. # Common use case: Updating BIOS, BMC, and NIC firmware. # -# Matches nodes with trait: CUSTOM_FIRMWARE_UPDATE +# Selects nodes carrying the trait: CUSTOM_FIRMWARE_UPDATE apiVersion: baremetal.ironicproject.org/v1alpha1 kind: IronicRunbook @@ -14,8 +14,17 @@ metadata: use-case: firmware-update hardware-type: general spec: + # REQUIRED: Secret holding the clouds.yaml, and the cloud entry to use. + cloudCredentialsRef: + secretName: infrasetup + cloudName: understack + runbookName: CUSTOM_FIRMWARE_UPDATE + # Nodes must carry at least one of these traits for this runbook to act on them. + traits: + - CUSTOM_FIRMWARE_UPDATE + steps: # Step 1: Update BIOS firmware - interface: management diff --git a/components/ironic/runbook-crd/samples/runbook_gpu_node_setup.yaml b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_gpu_node_setup.yaml similarity index 89% rename from components/ironic/runbook-crd/samples/runbook_gpu_node_setup.yaml rename to components/openstack-sync-plugins/ironic-runbooks/examples/runbook_gpu_node_setup.yaml index 0862763e2..30df8d6ef 100644 --- a/components/ironic/runbook-crd/samples/runbook_gpu_node_setup.yaml +++ b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_gpu_node_setup.yaml @@ -3,7 +3,7 @@ # This runbook configures nodes for GPU workloads. # Common use case: Preparing nodes for ML/AI or GPU compute workloads. # -# Matches nodes with trait: CUSTOM_GPU_SETUP +# Selects nodes carrying the trait: CUSTOM_GPU_SETUP apiVersion: baremetal.ironicproject.org/v1alpha1 kind: IronicRunbook @@ -14,8 +14,17 @@ metadata: use-case: gpu-configuration hardware-type: gpu-compute spec: + # REQUIRED: Secret holding the clouds.yaml, and the cloud entry to use. + cloudCredentialsRef: + secretName: infrasetup + cloudName: understack + runbookName: CUSTOM_GPU_SETUP + # Nodes must carry at least one of these traits for this runbook to act on them. + traits: + - CUSTOM_GPU_SETUP + steps: # Step 1: Configure BIOS for GPU support - interface: bios diff --git a/components/ironic/runbook-crd/samples/runbook_raid_config.yaml b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_raid_config.yaml similarity index 85% rename from components/ironic/runbook-crd/samples/runbook_raid_config.yaml rename to components/openstack-sync-plugins/ironic-runbooks/examples/runbook_raid_config.yaml index 54c43e73f..81e22fa52 100644 --- a/components/ironic/runbook-crd/samples/runbook_raid_config.yaml +++ b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_raid_config.yaml @@ -3,7 +3,7 @@ # This runbook configures RAID arrays for storage nodes. # Common use case: Setting up RAID 1 for OS and RAID 6 for data. # -# Matches nodes with trait: CUSTOM_STORAGE_RAID +# Selects nodes carrying the trait: CUSTOM_STORAGE_RAID apiVersion: baremetal.ironicproject.org/v1alpha1 kind: IronicRunbook @@ -14,8 +14,17 @@ metadata: use-case: raid-configuration hardware-type: storage spec: + # REQUIRED: Secret holding the clouds.yaml, and the cloud entry to use. + cloudCredentialsRef: + secretName: infrasetup + cloudName: understack + runbookName: CUSTOM_STORAGE_RAID + # Nodes must carry at least one of these traits for this runbook to act on them. + traits: + - CUSTOM_STORAGE_RAID + steps: # Step 1: Delete existing RAID configuration - interface: raid diff --git a/components/ironic/runbook-crd/samples/runbook_v1alpha1_complete.yaml b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_v1alpha1_complete.yaml similarity index 51% rename from components/ironic/runbook-crd/samples/runbook_v1alpha1_complete.yaml rename to components/openstack-sync-plugins/ironic-runbooks/examples/runbook_v1alpha1_complete.yaml index 5fcacc31b..0c8d01fcb 100644 --- a/components/ironic/runbook-crd/samples/runbook_v1alpha1_complete.yaml +++ b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_v1alpha1_complete.yaml @@ -3,8 +3,8 @@ # This example demonstrates all available fields in a runbook, # including both required and optional fields. # -# Required fields (✅): runbookName, steps, interface, step, order -# Optional fields (❌): disableRamdisk, public, owner, extra, args +# Required fields: cloudCredentialsRef, runbookName, steps, interface, step, order +# Optional fields: description, traits, disableRamdisk, public, owner, extra, args apiVersion: baremetal.ironicproject.org/v1alpha1 kind: IronicRunbook @@ -18,16 +18,28 @@ metadata: annotations: description: "Complete example showing all available fields" spec: - # ✅ REQUIRED: Runbook name (must match CUSTOM_* pattern) - runbookName: CUSTOM_COMPLETE + # REQUIRED: Secret holding the clouds.yaml, and the cloud entry to use + cloudCredentialsRef: + secretName: infrasetup + cloudName: understack - # ✅ REQUIRED: Ordered list of steps (minimum 1 step) + # REQUIRED: Runbook name. Any URL-safe string (letters, digits, - . _ ~) + runbookName: complete-example + + # OPTIONAL: Human-readable description (max 255 characters) + description: "Complete example runbook exercising every field" + + # OPTIONAL: Traits deciding which nodes this runbook may act on + traits: + - CUSTOM_COMPLETE_EXAMPLE + + # REQUIRED: Ordered list of steps (minimum 1 step) steps: # Step 1: BIOS Configuration - - interface: bios # ✅ REQUIRED - step: apply_configuration # ✅ REQUIRED - order: 1 # ✅ REQUIRED - args: # ❌ OPTIONAL + - interface: bios # REQUIRED + step: apply_configuration # REQUIRED + order: 1 # REQUIRED + args: # OPTIONAL settings: - name: LogicalProc value: Enabled @@ -37,10 +49,10 @@ spec: value: Enabled # Step 2: RAID Configuration - - interface: raid # ✅ REQUIRED - step: create_configuration # ✅ REQUIRED - order: 2 # ✅ REQUIRED - args: # ❌ OPTIONAL + - interface: raid # REQUIRED + step: create_configuration # REQUIRED + order: 2 # REQUIRED + args: # OPTIONAL logical_disks: - size_gb: 100 raid_level: "1" @@ -50,24 +62,24 @@ spec: is_root_volume: false # Step 3: Disk Cleaning - - interface: deploy # ✅ REQUIRED - step: erase_devices # ✅ REQUIRED - order: 3 # ✅ REQUIRED - args: # ❌ OPTIONAL + - interface: deploy # REQUIRED + step: erase_devices # REQUIRED + order: 3 # REQUIRED + args: # OPTIONAL erase_skip_list: [] - # ❌ OPTIONAL: Skip ramdisk booting (default: false) + # OPTIONAL: Skip ramdisk booting (default: false) disableRamdisk: false - # ❌ OPTIONAL: Make runbook public (default: false) + # OPTIONAL: Make runbook public (default: false) # Note: Cannot be true if owner is set public: false - # ❌ OPTIONAL: Project/tenant owner (default: null) + # OPTIONAL: Project/tenant owner (default: null) # Note: Cannot be set if public is true owner: "project-123" - # ❌ OPTIONAL: Additional metadata (default: {}) + # OPTIONAL: Additional metadata (default: {}) extra: description: "Complete example runbook with all fields" version: "1.0.0" diff --git a/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_v1alpha1_minimal.yaml b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_v1alpha1_minimal.yaml new file mode 100644 index 000000000..7e80d4a5a --- /dev/null +++ b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_v1alpha1_minimal.yaml @@ -0,0 +1,33 @@ +# Minimal Runbook Example - Required Fields Only +# +# This example shows the absolute minimum required to create a valid runbook. +# It includes only the 6 required fields: +# 1. spec.cloudCredentialsRef (secretName + cloudName) +# 2. spec.runbookName +# 3. spec.steps (array with min 1 step) +# 4. steps[].interface +# 5. steps[].step +# 6. steps[].order +# +# Use this as a starting point and add optional fields as needed. + +apiVersion: baremetal.ironicproject.org/v1alpha1 +kind: IronicRunbook +metadata: + name: minimal-runbook + namespace: default +spec: + # REQUIRED: Secret holding the clouds.yaml, and the cloud entry to use + cloudCredentialsRef: + secretName: infrasetup + cloudName: understack + + # REQUIRED: Runbook name. Any URL-safe string. + # Without spec.traits this runbook matches no nodes; see the other samples. + runbookName: minimal-example + + # REQUIRED: At least one step + steps: + - interface: deploy # REQUIRED: Hardware interface + step: erase_devices # REQUIRED: Step name + order: 1 # REQUIRED: Execution order (unique) diff --git a/components/openstack-sync-plugins/ironic-runbooks/kustomization.yaml b/components/openstack-sync-plugins/ironic-runbooks/kustomization.yaml new file mode 100644 index 000000000..c884452e2 --- /dev/null +++ b/components/openstack-sync-plugins/ironic-runbooks/kustomization.yaml @@ -0,0 +1,6 @@ +--- +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization + +resources: + - bmc_maintenance.yaml diff --git a/components/openstack-sync-plugins/kustomization.yaml b/components/openstack-sync-plugins/kustomization.yaml index d6a869e7a..b079cb854 100644 --- a/components/openstack-sync-plugins/kustomization.yaml +++ b/components/openstack-sync-plugins/kustomization.yaml @@ -4,3 +4,4 @@ kind: Kustomization resources: - neutron-router-flavors + - ironic-runbooks diff --git a/containers/openstack-sync-operator/Dockerfile b/containers/openstack-sync-operator/Dockerfile index 698876cad..25cd950c5 100644 --- a/containers/openstack-sync-operator/Dockerfile +++ b/containers/openstack-sync-operator/Dockerfile @@ -18,3 +18,4 @@ RUN --mount=type=cache,target=/root/.cache/uv \ COPY --chmod=755 python/openstack-sync/openstack_sync/hooks/placeholder.py /hooks/placeholder.py COPY --chmod=755 python/openstack-sync/openstack_sync/hooks/router_flavors.py /hooks/router_flavors.py +COPY --chmod=755 python/openstack-sync/openstack_sync/hooks/ironic_runbooks.py /hooks/ironic_runbooks.py diff --git a/containers/shell-operator-ironic/Dockerfile b/containers/shell-operator-ironic/Dockerfile deleted file mode 100644 index 5af84f2e6..000000000 --- a/containers/shell-operator-ironic/Dockerfile +++ /dev/null @@ -1,11 +0,0 @@ -FROM ghcr.io/flant/shell-operator:v1.13.1 AS prod -LABEL org.opencontainers.image.description="shell-operator for Ironic Runbooks" - -RUN --mount=type=cache,target=/var/cache/apk apk add python3 -RUN python3 -m venv /opt/venv -ENV PATH="/opt/venv/bin:$PATH" - -COPY containers/shell-operator-ironic/requirements.txt requirements.txt -RUN pip install --no-cache --upgrade -r requirements.txt - -COPY containers/shell-operator-ironic/hooks /hooks diff --git a/containers/shell-operator-ironic/hooks/create_runbook.sh b/containers/shell-operator-ironic/hooks/create_runbook.sh deleted file mode 100755 index f7f5a030b..000000000 --- a/containers/shell-operator-ironic/hooks/create_runbook.sh +++ /dev/null @@ -1,174 +0,0 @@ -#!/usr/bin/env bash - -if [[ $1 == "--config" ]] ; then - cat </dev/null || \ - echo "[create_runbook] WARNING: failed to patch status for ${name}" - } - - sync_runbook() { - # obj_path is a jq filter expression (e.g. "." or ".[0].object") - # pointing at the IronicRunbook object within BINDING_CONTEXT_PATH. - local obj_path="$1" - local resource_name namespace kind runbook_name description public owner - - resource_name=$(jq -r "${obj_path} | .metadata.name" "${BINDING_CONTEXT_PATH}") - namespace=$(jq -r "${obj_path} | .metadata.namespace" "${BINDING_CONTEXT_PATH}") - kind=$(jq -r "${obj_path} | .kind" "${BINDING_CONTEXT_PATH}") - runbook_name=$(jq -r "${obj_path} | .spec.runbookName" "${BINDING_CONTEXT_PATH}") - description=$(jq -r "${obj_path} | .spec.description // empty" "${BINDING_CONTEXT_PATH}") - public=$(jq -r "${obj_path} | .spec.public // empty" "${BINDING_CONTEXT_PATH}") - owner=$(jq -r "${obj_path} | .spec.owner // empty" "${BINDING_CONTEXT_PATH}") - - echo "[create_runbook] Creating runbook kind=${kind} name=${resource_name} namespace=${namespace} runbookName=${runbook_name} description=${description} public=${public} owner=${owner}" - - jq -r "${obj_path} | .spec.steps" "${BINDING_CONTEXT_PATH}" > /tmp/steps.json - - if ! jq -e 'type == "array" and length > 0' /tmp/steps.json >/dev/null 2>&1; then - echo "[create_runbook] FAILED: name=${resource_name} error=spec.steps is missing, null, or empty" >&2 - patch_status "${namespace}" "${resource_name}" "Failed" "spec.steps must be a non-empty array" - return 1 - fi - - command_args=(baremetal runbook create --name "${runbook_name}" --steps /tmp/steps.json) - - if [[ -n "${description}" ]]; then - command_args+=(--description "${description}") - fi - if [[ -n "${public}" ]]; then - command_args+=(--public "${public}") - fi - if [[ -n "${owner}" ]]; then - command_args+=(--owner "${owner}") - fi - - echo "[create_runbook] Running: openstack ${command_args[*]}" - - if output=$(openstack "${command_args[@]}" 2>&1); then - echo "[create_runbook] SUCCESS: Runbook created in Ironic name=${resource_name} output=${output}" - - traits_json=$(jq -c "${obj_path} | .spec.traits // []" "${BINDING_CONTEXT_PATH}") - if [[ "${traits_json}" != "[]" ]]; then - echo "[create_runbook] Setting traits name=${resource_name} traits=${traits_json}" - ironic_endpoint=$(openstack endpoint list --service baremetal --interface internal -f value -c URL 2>/dev/null | head -1) - if [[ -n "${ironic_endpoint}" ]]; then - token=$(openstack token issue -f value -c id) - echo "[create_runbook] PUT ${ironic_endpoint}/v1/runbooks/${runbook_name}/traits" - trait_response=$(curl -s -X PUT \ - -H "Content-Type: application/json" \ - -H "X-Auth-Token: ${token}" \ - -H "X-OpenStack-Ironic-API-Version: 1.112" \ - -d "{\"traits\": ${traits_json}}" \ - "${ironic_endpoint}/v1/runbooks/${runbook_name}/traits") - echo "[create_runbook] Traits response name=${resource_name} response=${trait_response}" - else - echo "[create_runbook] WARNING: Could not determine Ironic endpoint for traits" - fi - else - echo "[create_runbook] No traits to set name=${resource_name}" - fi - - patch_status "${namespace}" "${resource_name}" "Synced" "Successfully created runbook in Ironic" - echo "[create_runbook] Completed name=${resource_name} status=Synced" - else - # If it already exists, that's OK during sync - not an error - if echo "${output}" | grep -qi "already exists\|Conflict\|409"; then - echo "[create_runbook] Runbook already exists in Ironic name=${resource_name}, skipping create" - patch_status "${namespace}" "${resource_name}" "Synced" "Runbook already exists in Ironic" - else - echo "[create_runbook] FAILED: name=${resource_name} error=${output}" >&2 - patch_status "${namespace}" "${resource_name}" "Failed" "${output}" - return 1 - fi - fi - } - - echo "[create_runbook] Hook invoked, processing binding contexts" - binding_count=$(jq -r 'length' "${BINDING_CONTEXT_PATH}") - echo "[create_runbook] Found ${binding_count} binding context(s)" - - for ((i = 0; i < binding_count; i++)); do - type=$(jq -r ".[$i].type" "${BINDING_CONTEXT_PATH}") - echo "[create_runbook] Processing context=${i} type=${type}" - - if [[ $type == "Synchronization" ]] ; then - echo "[create_runbook] Synchronization event, reconciling existing resources" - objects_count=$(jq -r ".[$i].objects | length" "${BINDING_CONTEXT_PATH}") - echo "[create_runbook] Found ${objects_count} existing IronicRunbook(s) to reconcile" - for ((j = 0; j < objects_count; j++)); do - obj_name=$(jq -r ".[$i].objects[$j].object.metadata.name" "${BINDING_CONTEXT_PATH}") - obj_sync=$(jq -r ".[$i].objects[$j].object.status.syncStatus // empty" "${BINDING_CONTEXT_PATH}") - echo "[create_runbook] Checking name=${obj_name} syncStatus=${obj_sync}" - if [[ -z "${obj_sync}" || "${obj_sync}" == "null" ]]; then - echo "[create_runbook] Resource name=${obj_name} has no syncStatus, needs reconciliation" - # Re-map the jq path to point at the object within the sync event - ORIG_BINDING_CONTEXT_PATH="${BINDING_CONTEXT_PATH}" - jq -r ".[$i].objects[$j].object" "${BINDING_CONTEXT_PATH}" > /tmp/sync_object.json - BINDING_CONTEXT_PATH=/tmp/sync_object.json - sync_runbook "." - BINDING_CONTEXT_PATH="${ORIG_BINDING_CONTEXT_PATH}" - else - echo "[create_runbook] Resource name=${obj_name} already synced, skipping" - fi - done - continue - fi - - if [[ $type == "Event" ]] ; then - if ! sync_runbook ".[$i].object"; then - exit 1 - fi - fi - done - echo "[create_runbook] Hook finished" -fi diff --git a/containers/shell-operator-ironic/hooks/delete_runbook.sh b/containers/shell-operator-ironic/hooks/delete_runbook.sh deleted file mode 100755 index ec4c36d88..000000000 --- a/containers/shell-operator-ironic/hooks/delete_runbook.sh +++ /dev/null @@ -1,51 +0,0 @@ -#!/usr/bin/env bash - -if [[ $1 == "--config" ]] ; then - cat <&1); then - echo "[delete_runbook] SUCCESS: Runbook deleted from Ironic name=${resource_name} output=${output}" - else - echo "[delete_runbook] FAILED: name=${resource_name} error=${output}" >&2 - exit 1 - fi - fi - done - echo "[delete_runbook] Hook finished" -fi diff --git a/containers/shell-operator-ironic/hooks/update_runbook.sh b/containers/shell-operator-ironic/hooks/update_runbook.sh deleted file mode 100755 index 400f7fdc6..000000000 --- a/containers/shell-operator-ironic/hooks/update_runbook.sh +++ /dev/null @@ -1,179 +0,0 @@ -#!/usr/bin/env bash - -if [[ $1 == "--config" ]] ; then - cat </dev/null || \ - echo "[update_runbook] WARNING: failed to patch status for ${name}" - } - - sync_runbook() { - # obj_path is a jq filter expression (e.g. "." or ".[0].object") - # pointing at the IronicRunbook object within BINDING_CONTEXT_PATH. - local obj_path="$1" - local resource_name namespace kind runbook_name description public owner runbook_uuid - - resource_name=$(jq -r "${obj_path} | .metadata.name" "${BINDING_CONTEXT_PATH}") - namespace=$(jq -r "${obj_path} | .metadata.namespace" "${BINDING_CONTEXT_PATH}") - kind=$(jq -r "${obj_path} | .kind" "${BINDING_CONTEXT_PATH}") - runbook_name=$(jq -r "${obj_path} | .spec.runbookName" "${BINDING_CONTEXT_PATH}") - description=$(jq -r "${obj_path} | .spec.description // empty" "${BINDING_CONTEXT_PATH}") - public=$(jq -r "${obj_path} | .spec.public // empty" "${BINDING_CONTEXT_PATH}") - owner=$(jq -r "${obj_path} | .spec.owner // empty" "${BINDING_CONTEXT_PATH}") - - echo "[update_runbook] Updating runbook kind=${kind} name=${resource_name} namespace=${namespace} runbookName=${runbook_name} description=${description} public=${public} owner=${owner}" - - jq -r "${obj_path} | .spec.steps" "${BINDING_CONTEXT_PATH}" > /tmp/steps.json - - if ! jq -e 'type == "array" and length > 0' /tmp/steps.json >/dev/null 2>&1; then - echo "[update_runbook] FAILED: name=${resource_name} error=spec.steps is missing, null, or empty" >&2 - patch_status "${namespace}" "${resource_name}" "Failed" "spec.steps must be a non-empty array" - return 1 - fi - - # Look up the existing runbook by name to get its UUID. If the show fails - # the runbook does not exist yet and we need to create it instead of set. - if runbook_uuid=$(openstack baremetal runbook show "${runbook_name}" -f value -c uuid 2>/dev/null); then - echo "[update_runbook] Found existing runbook name=${runbook_name} uuid=${runbook_uuid}" - command_args=(baremetal runbook set "${runbook_uuid}") - else - echo "[update_runbook] Runbook name=${runbook_name} not found, creating" - runbook_uuid="" - command_args=(baremetal runbook create) - fi - command_args+=(--name "${runbook_name}" --steps /tmp/steps.json) - - if [[ -n "${description}" ]]; then - command_args+=(--description "${description}") - fi - if [[ -n "${owner}" ]]; then - command_args+=(--owner "${owner}") - fi - - echo "[update_runbook] Running: openstack ${command_args[*]}" - - if output=$(openstack "${command_args[@]}" 2>&1); then - echo "[update_runbook] SUCCESS: Runbook updated in Ironic name=${resource_name} output=${output}" - - traits_json=$(jq -c "${obj_path} | .spec.traits // []" "${BINDING_CONTEXT_PATH}") - if [[ "${traits_json}" != "[]" ]]; then - echo "[update_runbook] Setting traits name=${resource_name} traits=${traits_json}" - # The traits endpoint requires the UUID; look it up if we just created - # the runbook and don't have it yet. - if [[ -z "${runbook_uuid}" ]]; then - runbook_uuid=$(openstack baremetal runbook show "${runbook_name}" -f value -c uuid 2>/dev/null) - fi - ironic_endpoint=$(openstack endpoint list --service baremetal --interface internal -f value -c URL 2>/dev/null | head -1) - if [[ -n "${ironic_endpoint}" && -n "${runbook_uuid}" ]]; then - token=$(openstack token issue -f value -c id) - echo "[update_runbook] PUT ${ironic_endpoint}/v1/runbooks/${runbook_uuid}/traits" - trait_response=$(curl -s -X PUT \ - -H "Content-Type: application/json" \ - -H "X-Auth-Token: ${token}" \ - -H "X-OpenStack-Ironic-API-Version: 1.112" \ - -d "{\"traits\": ${traits_json}}" \ - "${ironic_endpoint}/v1/runbooks/${runbook_uuid}/traits") - echo "[update_runbook] Traits response name=${resource_name} response=${trait_response}" - else - echo "[update_runbook] WARNING: Could not determine Ironic endpoint or runbook UUID for traits" - fi - else - echo "[update_runbook] No traits to set name=${resource_name}" - fi - - patch_status "${namespace}" "${resource_name}" "Synced" "Successfully updated runbook in Ironic" - echo "[update_runbook] Completed name=${resource_name} status=Synced" - else - echo "[update_runbook] FAILED: name=${resource_name} error=${output}" >&2 - patch_status "${namespace}" "${resource_name}" "Failed" "${output}" - return 1 - fi - } - - echo "[update_runbook] Hook invoked, processing binding contexts" - binding_count=$(jq -r 'length' "${BINDING_CONTEXT_PATH}") - echo "[update_runbook] Found ${binding_count} binding context(s)" - - for ((i = 0; i < binding_count; i++)); do - type=$(jq -r ".[$i].type" "${BINDING_CONTEXT_PATH}") - echo "[update_runbook] Processing context=${i} type=${type}" - - if [[ $type == "Synchronization" ]] ; then - echo "[update_runbook] Synchronization event, reconciling existing resources" - objects_count=$(jq -r ".[$i].objects | length" "${BINDING_CONTEXT_PATH}") - echo "[update_runbook] Found ${objects_count} existing IronicRunbook(s) to reconcile" - for ((j = 0; j < objects_count; j++)); do - obj_name=$(jq -r ".[$i].objects[$j].object.metadata.name" "${BINDING_CONTEXT_PATH}") - obj_sync=$(jq -r ".[$i].objects[$j].object.status.syncStatus // empty" "${BINDING_CONTEXT_PATH}") - echo "[update_runbook] Checking name=${obj_name} syncStatus=${obj_sync}" - if [[ -z "${obj_sync}" || "${obj_sync}" == "null" || "${obj_sync}" == "Failed" ]]; then - echo "[update_runbook] Resource name=${obj_name} needs sync (syncStatus=${obj_sync}), reconciling" - ORIG_BINDING_CONTEXT_PATH="${BINDING_CONTEXT_PATH}" - jq -r ".[$i].objects[$j].object" "${BINDING_CONTEXT_PATH}" > /tmp/sync_object.json - BINDING_CONTEXT_PATH=/tmp/sync_object.json - sync_runbook "." - BINDING_CONTEXT_PATH="${ORIG_BINDING_CONTEXT_PATH}" - else - echo "[update_runbook] Resource name=${obj_name} already synced, skipping" - fi - done - continue - fi - - if [[ $type == "Event" ]] ; then - if ! sync_runbook ".[$i].object"; then - exit 1 - fi - fi - done - echo "[update_runbook] Hook finished" -fi diff --git a/containers/shell-operator-ironic/requirements.txt b/containers/shell-operator-ironic/requirements.txt deleted file mode 100644 index e48726a9c..000000000 --- a/containers/shell-operator-ironic/requirements.txt +++ /dev/null @@ -1,4 +0,0 @@ -pip -kubernetes -python-openstackclient -python-ironicclient diff --git a/docs/deploy-guide/components/openstack-sync-operator.md b/docs/deploy-guide/components/openstack-sync-operator.md index 00484be79..9c733b11d 100644 --- a/docs/deploy-guide/components/openstack-sync-operator.md +++ b/docs/deploy-guide/components/openstack-sync-operator.md @@ -63,13 +63,13 @@ Important behavior: - The plugin Application applies every CR listed in `components/openstack-sync-plugins/kustomization.yaml` and `//openstack-sync-plugins/kustomization.yaml`. -- `plugins.neutronRouterFlavors` does not control CR creation. It only controls - the operator runtime for that hook: enablement env vars, hook RBAC, and the - `verify-hooks` initContainer. +- `plugins.` does not control CR creation. It only controls the operator + runtime for that hook: enablement env vars, hook RBAC, and the `verify-hooks` + initContainer. -Because of that split, `NeutronRouterFlavor` CRs can exist while -`plugins.neutronRouterFlavors: false`. In that state ArgoCD can be Synced, but -the operator will not reconcile those CRs into OpenStack. +Because of that split, plugin CRs can exist while their hook is disabled. In +that state ArgoCD can be Synced, but the operator will not reconcile those CRs +into OpenStack. ## Enablement @@ -96,18 +96,18 @@ intend to run in `//openstack-sync-operator/values.yaml`. Built-in hooks are declared in `components/openstack-sync-operator/values.yaml`. -For Neutron router flavors, the default is: +For each built-in CRD hook, the chart values use this shape: ```yaml plugins: - neutronRouterFlavors: false + : false pluginData: - neutronRouterFlavors: + : hook: - path: /hooks/router_flavors.py - crd: crds/neutron.understack.rackspace.net_neutronrouterflavors.yaml - envPrefix: NEUTRON_ROUTER_FLAVOR + path: /hooks/.py + crd: crds/_.yaml + envPrefix: ``` Enable the hook from the deployment repo after the site is pinned to an @@ -115,17 +115,16 @@ operator image built from this code: ```yaml title="$CLUSTER_NAME/openstack-sync-operator/values.yaml" plugins: - neutronRouterFlavors: true + : true ``` -The image build in `containers/openstack-sync-operator/Dockerfile` copies both -`python/openstack-sync/openstack_sync/hooks/placeholder.py` and -`python/openstack-sync/openstack_sync/hooks/router_flavors.py` into `/hooks/`. +The image build in `containers/openstack-sync-operator/Dockerfile` copies the +enabled hook executables into `/hooks/`. -When `plugins.neutronRouterFlavors: false`, the router-flavor hook still exists -in the image but publishes only a no-op startup binding. That keeps -shell-operator startup valid while preventing any watch, schedule, OpenStack -sync, or hook-specific RBAC for router flavors. +When `plugins.: false`, that hook may still exist in the image but +publishes only a no-op startup binding. That keeps shell-operator startup valid +while preventing any watch, schedule, OpenStack sync, or hook-specific RBAC for +that resource. When a hook is enabled, the chart: @@ -141,7 +140,7 @@ They declare the hook path in `pluginData..hook.path`, and the chart generates one startup check for each enabled hook. The plugin author must still copy the hook executable into the operator image at that path. -Rendered example for Neutron router flavors: +Rendered shape: ```yaml initContainers: @@ -152,18 +151,15 @@ initContainers: - -ec - | missing=0 - if [ ! -x "/hooks/router_flavors.py" ]; then - echo "enabled hook neutronRouterFlavors missing or not executable: /hooks/router_flavors.py" >&2 + if [ ! -x "/hooks/.py" ]; then + echo "enabled hook missing or not executable: /hooks/.py" >&2 missing=1 fi exit "${missing}" ``` -For Neutron router flavors, the enabled hook registers a `kubernetes` binding -that watches `NeutronRouterFlavor` CRs and a `schedule` binding for periodic -sync. Reconciliation logic (reading CRs, calling `openstacksdk`, and patching CR -status) is not yet implemented; the hook currently exits 0 without taking action -on events. +Hook-specific OpenStack behavior belongs with the plugin's CR examples or schema +docs. This page documents only the operator deployment contract. When no hook is enabled, the operator can still start. In that state the Role has no custom-resource permissions and no OpenStack sync work is expected. @@ -194,33 +190,31 @@ The plugin Application should continue to apply only CR manifests. ## CRDs and Validation -The Neutron router flavor CRD is in: -`components/openstack-sync-operator/crds/neutron.understack.rackspace.net_neutronrouterflavors.yaml` +The CRDs live under `components/openstack-sync-operator/crds/`. -It defines: +Each plugin CRD defines: -- API version: `neutron.understack.rackspace.net/v1alpha1` -- Kind: `NeutronRouterFlavor` -- Resource: `neutronrouterflavors` - Scope: namespaced - Status subresource: enabled +- Required `spec.cloudCredentialsRef.secretName` +- Required `spec.cloudCredentialsRef.cloudName` The chart reads this CRD through `components/openstack-sync-operator/templates/_crd.tpl` so RBAC and hook environment variables are derived from the same schema Kubernetes applies. -Neutron router flavor CR files also reference the editor schema at: -`schema/openstack-sync/neutron-router-flavor.schema.json` +Plugin CR files can also reference editor schemas under: +`schema/openstack-sync/` -That schema focuses on the flavor data under `spec`. Kubernetes validates the +That schema focuses on plugin data under `spec`. Kubernetes validates the full custom resource through the operator-owned CRD when ArgoCD applies it. -## Current Neutron Router Flavor Data +## Plugin CR Data -Shared Neutron router flavor CRs live here: +Shared plugin CRs live under: -`components/openstack-sync-plugins/neutron-router-flavors/` +`components/openstack-sync-plugins/` Site-specific additions live in the deploy repo: diff --git a/docs/deploy-guide/components/openstack-sync-plugins.md b/docs/deploy-guide/components/openstack-sync-plugins.md index 40d9a7aab..4b3b70d09 100644 --- a/docs/deploy-guide/components/openstack-sync-plugins.md +++ b/docs/deploy-guide/components/openstack-sync-plugins.md @@ -31,33 +31,35 @@ Enable the Application with `site.openstack_sync_plugins.enabled`. {{ secrets_disclaimer }} -Shared Neutron router flavor CRs that should apply to all clusters live in the -understack repo under: +Shared plugin CRs that should apply to all clusters live in the understack repo +under: -`components/openstack-sync-plugins/neutron-router-flavors/` +`components/openstack-sync-plugins/` The shared data entrypoint is: `components/openstack-sync-plugins/kustomization.yaml` -Cluster-specific CRs live in the deployment repo under +Site-specific CRs live in the deployment repo under `//openstack-sync-plugins/` and are listed by that directory's `kustomization.yaml`. -Hook enablement is separate. Set `plugins.neutronRouterFlavors: true` in -`//openstack-sync-operator/values.yaml` only after the site -is pinned to an operator image built with `/hooks/router_flavors.py`. - -`plugins.neutronRouterFlavors: false` does not stop this Application from -creating `NeutronRouterFlavor` CRs. It only disables the operator hook that -reconciles those CRs into OpenStack. To stop creating the CRs, disable -`site.openstack_sync_plugins.enabled` or remove the CR files from the relevant -`kustomization.yaml`. - -Neutron router flavor CR files use the published editor schema -`schema/openstack-sync/neutron-router-flavor.schema.json`. That schema validates -the flavor data under `spec`, not the Kubernetes wrapper fields. Kubernetes -validates required fields, types, enums, and defaults through the operator-owned -CRD when ArgoCD applies the CR. The editor schema is stricter about unknown spec -fields, so add new schema fields with the matching operator hook/CRD change when -a driver needs new service-profile data. +Hook enablement is separate. Set `plugins.: true` in +`//openstack-sync-operator/values.yaml` only after the site is +pinned to an operator image built with the matching hook under `/hooks/`. + +`plugins.: false` does not stop this Application from creating that +plugin's CRs. It only disables the operator hook that processes those CRs. To +stop creating the CRs, disable `site.openstack_sync_plugins.enabled` or remove +the CR files from the relevant `kustomization.yaml`. + +Plugin CR files use published editor schemas under `schema/openstack-sync/`. +Those schemas validate the data under `spec`, not the Kubernetes wrapper fields. +Kubernetes validates required fields, types, enums, and defaults through the +operator-owned CRD when ArgoCD applies the CR. The editor schemas are stricter +about unknown spec fields, so add new schema fields with the matching +operator hook/CRD change when a plugin needs new data. + +Syncing a plugin CR only converges the OpenStack resource described by that CR. +Any separate operation that uses the synced resource belongs in the plugin's own +examples or operational documentation. diff --git a/docs/operator-guide/baremetal-ironic-cleanup-runbook.md b/docs/operator-guide/baremetal-ironic-cleanup-runbook.md index a78922f00..24f11cde0 100644 --- a/docs/operator-guide/baremetal-ironic-cleanup-runbook.md +++ b/docs/operator-guide/baremetal-ironic-cleanup-runbook.md @@ -9,7 +9,7 @@ happens, we need a safe process to inspect the machine again, verify the data, and then clean or return it to service. Operator rule of thumb: start with node state, tenant ownership, the last error, -and the next safe command. The repository/config details are kept later as +and the next safe command. The repository/config details are kept below as evidence so the process stays auditable without making the first page feel like a config review. @@ -21,8 +21,8 @@ This document is only for Ironic baremetal box cleanup. - how to decide whether a node is safe to inspect or clean - how to handle stale/out-of-sync data before retrying cleanup - which Ironic states require manual investigation -- where existing Ironic runbooks fit today -- what is not currently automated or configured +- where Ironic runbooks fit +- what is automated and what still needs an operator ## Manual vs Automated @@ -204,9 +204,7 @@ No Port found for How to read this: -- This is not an example of the historical wrong-secondary-switch PXE issue. - That older issue came from stale/old inspection behavior and is not expected - as a normal cleanup failure pattern. +- This is not the wrong-secondary-switch PXE issue. - In this example, the PXE-enabled port points at the expected primary `-1` switch. - Ironic still has a cleaning VIF recorded, but Neutron no longer has that @@ -340,7 +338,7 @@ Check Neutron/Undersync and the Ironic port data before retrying inspection. ### Enrollment -The enrollment flow is implemented in +The enrollment flow lives in `enroll_server.py`. Operator-level flow: @@ -358,6 +356,7 @@ flowchart TD I[available] A --> B --> C --> D --> E --> F --> G --> H --> I +``` The final state transition in code is: @@ -394,13 +393,9 @@ maintenance. ## Runbooks Operators May Encounter -Firmware update cleanup is already modeled with Ironic runbooks. - -The workflow is defined in -`server-firmware-update.yaml`. - -It finds node traits matching `CUSTOM_FIRMWARE_UPDATE_`, looks up the matching -Ironic runbook, and runs: +Firmware update cleanup uses Ironic runbooks through the +`server-firmware-update.yaml` workflow. It finds node traits matching +`CUSTOM_FIRMWARE_UPDATE_`, looks up the matching Ironic runbook, and runs: ```text openstack baremetal node clean --runbook --wait 0 @@ -409,7 +404,7 @@ openstack baremetal node clean --runbook --wait 0 The guide for this is [server-firmware-update.md](server-firmware-update.md). -There is also an existing manual-clean example in +There is also a manual-clean example in [openstack-ironic-change-boot-interface.md](openstack-ironic-change-boot-interface.md): ```text @@ -422,11 +417,11 @@ box cleanup path. ## Repo Evidence We do not need this section for every cleanup, but it explains why the -commands above are the current supported process. +commands above are the supported process. ### Cleaning Configuration -The current Ironic cleaning configuration is in +Ironic cleaning configuration is in `values.yaml`: ```yaml @@ -453,10 +448,10 @@ conf: inspection_hooks: "validate-interfaces,ports,port-bios-name,architecture,pci-devices,resource-class" ``` -Current meaning: +Configuration meaning: - Ironic automated cleaning is enabled with `automated_clean: true`. -- The older default disk erase priorities are set to `0`. +- Default disk erase steps are disabled by setting their priorities to `0`. - `deploy.erase_devices_express` is enabled through `clean_step_priority_override: deploy.erase_devices_express:95`. - The default box cleanup path is not configured as an Ironic runbook in this @@ -498,10 +493,9 @@ or event-source logs when verifying this behavior in an environment. ### Runbook CRD -The runbook CRD is defined under -`runbook-crd`, and the shell operator hook -that syncs Kubernetes `IronicRunbook` objects into Ironic is -`create_runbook.sh`. +The runbook CRD is defined under `components/openstack-sync-operator/crds/`. +The `ironic_runbooks.py` hook reconciles Kubernetes `IronicRunbook` objects +with the Ironic API and patches sync status. Checked-in sample runbooks include: @@ -518,11 +512,11 @@ they are deployed unless the environment confirms that. | File | Why it matters | | --- | --- | -| `values.yaml` | Current Ironic cleaning, inspector, DHCP, and Redfish hook configuration | +| `values.yaml` | Ironic cleaning, inspector, DHCP, and Redfish hook configuration | | `enroll_server.py` | Enrollment order and final `provide` transition | | `ironic_node.py` | Helper functions for Ironic state transitions, RAID clean steps, and firmware runbooks | -| `reclean-server.yaml` | Existing Argo reclean workflow | -| `sensor-ironic-node-reclean.yaml` | Existing clean-failed event sensor | -| `pr-clean-failed-servers.yaml` | Existing clean-failed Prometheus alert | -| `server-firmware-update.yaml` | Existing firmware runbook workflow | -| `runbook-crd/samples` | Sample runbook manifests, not assumed deployed | +| `reclean-server.yaml` | Argo reclean workflow | +| `sensor-ironic-node-reclean.yaml` | Clean-failed event sensor | +| `pr-clean-failed-servers.yaml` | Clean-failed Prometheus alert | +| `server-firmware-update.yaml` | Firmware runbook workflow | +| `components/openstack-sync-operator/crds` | OpenStack sync CRD manifests | diff --git a/docs/operator-guide/server-firmware-update.md b/docs/operator-guide/server-firmware-update.md index 836d025ad..038ff74ac 100644 --- a/docs/operator-guide/server-firmware-update.md +++ b/docs/operator-guide/server-firmware-update.md @@ -1,56 +1,72 @@ # Server Firmware Updates -Server firmware updates are done via executing Ironic Runbooks against target nodes. The node must have a trait matching the name of the runbook for the runbook to execute. +Server firmware updates are done by executing Ironic runbooks against target +nodes. The firmware workflow looks for node traits matching +`CUSTOM_FIRMWARE_UPDATE_*` and runs the Ironic runbook with the same name. ## Inspection rules -Traits are applied to a node during inspection. Ironic Inspection Rules can be used to define which traits are applied during inspection time. These Inspection Rules are currently deployed with Ironic Conductor, in a yaml file located in `/etc/ironic/inspection-rules/inspection-rules.yaml`. An example inspection-rules.yaml file: +Traits are applied to a node during inspection. Ironic inspection rules define +which traits are added, and Ironic Conductor loads them from +`/etc/ironic/inspection-rules/inspection-rules.yaml`. + +Example inspection rules: ```yaml --- - description: Set R7615 Firmware Traits - phase: main - conditions: + phase: main + conditions: - op: "contains" - args: ["{inventory[system_vendor][product_name]}", "PowerEdge R7615"] - actions: + args: ["{inventory[system_vendor][product_name]}", "PowerEdge R7615"] + actions: - op: "add-trait" - args: ["CUSTOM_FIRMWARE_UPDATE_R7615"] + args: ["CUSTOM_FIRMWARE_UPDATE_R7615"] - description: Set R7515 Firmware Traits - phase: main - conditions: + phase: main + conditions: - op: "contains" - args: ["{inventory[system_vendor][product_name]}", "PowerEdge R7515"] - actions: + args: ["{inventory[system_vendor][product_name]}", "PowerEdge R7515"] + actions: - op: "add-trait" - args: ["CUSTOM_FIRMWARE_UPDATE_R7515"] + args: ["CUSTOM_FIRMWARE_UPDATE_R7515"] - description: Set R740xd Firmware Traits - phase: main - conditions: + phase: main + conditions: - op: "contains" - args: ["{inventory[system_vendor][product_name]}", "PowerEdge R740xd"] + args: ["{inventory[system_vendor][product_name]}", "PowerEdge R740xd"] - op: "!contains" - args: ["{inventory[system_vendor][product_name]}", "(?i)R740xd2"] - actions: + args: ["{inventory[system_vendor][product_name]}", "(?i)R740xd2"] + actions: - op: "add-trait" - args: ["CUSTOM_FIRMWARE_UPDATE_R740XD"] + args: ["CUSTOM_FIRMWARE_UPDATE_R740XD"] ``` ## Ironic Runbooks -Deployment of the Ironic Runbooks are done via Kubernetes manifests. A kubernetes Runbook CRD has been created to define a Runbook resource. To sync and maintain the state of these Runbook resources to the Openstack API, a Kubernetes Runbook operator was created. +Ironic runbooks are managed as `IronicRunbook` Kubernetes CRs. The +`ironicRunbooks` openstack-sync hook reconciles those CRs into the Ironic API, +patches CR status, and prunes operator-owned runbooks when their CRs are +removed. + +For firmware updates, set `spec.runbookName` to the matching +`CUSTOM_FIRMWARE_UPDATE_*` trait name. Ironic only runs a runbook on a node that +has at least one of the runbook's `spec.traits`; a runbook with no traits +matches no nodes. ## Workflows -An Argo Workflow, named `server-firmware-update`, was created to handle execution of the server firmware updates. This workflow will take a node in either `manageable` or `available` state and do the following: +The `server-firmware-update` Argo Workflow handles server firmware updates for +a node in either `manageable` or `available` state. It: -- Move the node to `manageable` state (if necessary) -- Identify all traits matching `^CUSTOM_FIRMWARE_UPDATE_.*` -- Attempt to execute a Runbook for all matching traits that were found -- Sequentially install firmwares defined in all runbooks -- Return the node to original state (if necessary) +- moves the node to `manageable` state if needed +- identifies traits matching `^CUSTOM_FIRMWARE_UPDATE_.*` +- executes the Ironic runbook for each matching trait +- installs firmware from the selected runbooks in sequence +- returns the node to its original state if needed -This workflow can also optionally be run from within the `enroll-server` workflow, immediately after the final inspection, by passing in `firmware_update=true`. +The `enroll-server` workflow can run firmware updates after final inspection by +passing `firmware_update=true`. ```mermaid flowchart TB @@ -62,9 +78,11 @@ flowchart TB E --> F(Run Matching Runbooks) ``` -## Runbook Operator +## Runbook Sync -The Ironic Runbook Operator was written using [shell-operator](https://github.com/flant/shell-operator). Essentially it listens for create, update or delete events on any Runbook resources, and then issues the appropriate calls to the Openstack Ironic API. These operations are defined by basic shell hooks, which can be found [here](https://github.com/rackerlabs/understack/tree/main/containers/shell-operator-ironic/hooks) +`IronicRunbook` resources are owned by the `openstack-sync-operator` framework. +The hook creates, updates, and deletes operator-owned Ironic runbooks through the +Ironic API. ```mermaid architecture-beta diff --git a/python/openstack-sync/README.md b/python/openstack-sync/README.md index ff610b1f6..aa1f6b6d3 100644 --- a/python/openstack-sync/README.md +++ b/python/openstack-sync/README.md @@ -15,14 +15,15 @@ openstack_sync/ 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 + .py CRD hook entry point plugins/ common.py OpenStack helpers shared by all plugins - neutron/router_flavors/ + // config.py plugin constants + client.py OpenStack API calls, if needed markers.py ownership markers reconcile.py converge one CR - prune.py delete resources whose CR was removed + prune.py delete resources whose CR was removed, if safe ``` ## What the framework does for you @@ -31,7 +32,7 @@ openstack_sync/ 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, or any CR could not be read at all, it **skips the prune -entirely** — either way the desired state is unknown, so deleting anything would +entirely** - either way the desired state is unknown, so deleting anything would be unsafe. A CR whose spec does not satisfy the framework's contract is named in the log and @@ -45,47 +46,44 @@ reading the binding context, and the exit code. 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 + `secretName` and `cloudName` - the framework relies on both. Put validation (`required`, `enum`, `minLength`, `default`) in the schema so the API server rejects bad CRs at admission. - Schema validation is not a guarantee about what a reconcile receives, though. - Kubernetes validates on write, so a CR admitted before a field became - required keeps being served by the watch exactly as stored — tightening a CRD - neither invalidates nor migrates what already exists. Read schema-optional - fields with a default, and treat a missing schema-required field as a reason - to fail that one CR loudly and by name, not as impossible. + Read optional fields with explicit defaults. Missing required fields are + rejected by the CRD schema. 2. **Register it** in `components/openstack-sync-operator/values.yaml`: ```yaml plugins: - myResource: false # opt in per site + : false # opt in per site pluginData: - myResource: + : hook: - path: /hooks/my_resource.py + path: /hooks/.py crd: crds/_.yaml - envPrefix: MY_RESOURCE + envPrefix: env: SYNC_CRONTAB: "0 * * * *" ``` - The chart derives `MY_RESOURCE_ENABLED`, `_CRD_API_VERSION`, `_CRD_KIND`, + The chart derives `_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 only the framework + key into `_`. `HookConfig.from_env` reads only the framework keys, such as `PRUNE`, `SYNC_CRONTAB`, `READY_RETRIES` and `READY_DELAY`. Plugins read custom prefixed env vars directly. -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`. +3. **Write the plugin package** under `plugins///`. + `config.py` and `reconcile.py` are the usual minimum. Add `markers.py` when + the plugin stamps ownership into OpenStack resources, and `prune.py` only + when deleting resources after CR removal is safe and implemented. -4. **Write the hook** — subclass `SyncPlugin` and wire it up: +4. **Write the hook** - subclass `SyncPlugin` and wire it up: ```python - class MyResourcePlugin(SyncPlugin): - noun = "my resource" + class ResourcePlugin(SyncPlugin): + noun = "" def wait_for_api(self, conn) -> None: ... @@ -102,7 +100,7 @@ reading the binding context, and the exit code. 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_sync(ResourcePlugin(config), hook_inputs(contexts, config)) return run_hook(lambda: build_crd_hook_config(ENV_PREFIX, BINDING_NAME), run) ``` @@ -120,10 +118,9 @@ for a hand-made resource unless transferring it to the operator is intentional. **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. +correct. 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 @@ -134,4 +131,4 @@ uv run pytest ``` `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. +OpenStack at all - read it first to understand the contract a plugin gets. diff --git a/python/openstack-sync/openstack_sync/hooks/framework.py b/python/openstack-sync/openstack_sync/hooks/framework.py index 9f2c50bd2..12b8f8f39 100644 --- a/python/openstack-sync/openstack_sync/hooks/framework.py +++ b/python/openstack-sync/openstack_sync/hooks/framework.py @@ -174,14 +174,8 @@ def display_name(self) -> str: class HookInputs: """Binding context split by reconciliation purpose. - The 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. - - ``unreadable_resources`` names the CRs the binding context described but - that could not be read (see :class:`_ResourceReader`). They are absent from - every other field, so the desired set is not known to be complete while it - is non-empty. + Event runs reconcile changed CRs and prune against the snapshot. Unreadable + CRs are omitted from the resource lists and make the desired set incomplete. """ resources_to_reconcile: list[SyncResource] @@ -223,15 +217,7 @@ def _resource_identity(obj: dict[str, Any]) -> str: def _resource_from_object(obj: dict[str, Any]) -> SyncResource: - """Build a :class:`SyncResource` from a Kubernetes object. - - The spec is validated rather than assumed. The CRD marks - ``spec.cloudCredentialsRef`` required and its ``secretName`` / ``cloudName`` - ``minLength: 1``, but that only binds writes: Kubernetes validates on - admission, so an object stored before the schema required those fields is - still served by the watch exactly as stored. Tightening a CRD neither - invalidates nor migrates what already exists. - """ + """Build a resource from a Kubernetes object and validate required spec fields.""" spec = obj.get("spec") if not isinstance(spec, dict): raise _MalformedResourceError("spec is missing or not an object") @@ -265,17 +251,7 @@ def _resource_from_object(obj: dict[str, Any]) -> SyncResource: class _ResourceReader: - """Reads watched objects into resources, naming the ones it cannot read. - - An object that fails validation is reported and dropped rather than raised - past the batch, so one unusable CR does not stop the others from - reconciling. Its identity is retained because a dropped CR leaves the - desired set incomplete, which the caller needs in order to decide whether - pruning is safe. - - One reader spans a whole binding context, so a CR that appears in both an - event and the accompanying snapshot is reported once. - """ + """Reads watched objects into resources and records unreadable CRs.""" def __init__(self) -> None: self.unreadable: set[str] = set() @@ -323,20 +299,23 @@ def _status_is_current(resource: SyncResource) -> bool: def _split_events( contexts: list[dict[str, Any]], config: HookConfig, reader: _ResourceReader -) -> tuple[list[SyncResource], list[SyncResource], frozenset[str]]: +) -> tuple[list[SyncResource], list[SyncResource], bool]: """Split this binding's Event contexts into changed and deleted resources.""" changed: list[SyncResource] = [] deleted: list[SyncResource] = [] - watch_events: set[str] = set() + saw_event_context = False for context in contexts: if context.get("binding") != config.binding_name: continue if context.get("type") != "Event": continue + saw_event_context = True - watch_event = context["watchEvent"] - watch_events.add(watch_event) + watch_event = context.get("watchEvent") + if not watch_event: + LOG.warning("%s event carries no watchEvent; ignoring it", config.crd_kind) + continue obj = context.get("object") if not obj: @@ -363,7 +342,7 @@ def _split_events( changed.append(resource) changed.sort(key=lambda r: str(r.spec.get("name", ""))) - return changed, deleted, frozenset(watch_events) + return changed, deleted, saw_event_context def hook_inputs(contexts: list[dict[str, Any]], config: HookConfig) -> HookInputs: @@ -374,22 +353,17 @@ def hook_inputs(contexts: list[dict[str, Any]], config: HookConfig) -> HookInput runs reconcile everything they are given. """ reader = _ResourceReader() - changed, deleted, watch_events = _split_events(contexts, config, reader) + changed, deleted, saw_event_context = _split_events(contexts, config, reader) items = snapshot_items(contexts, config.binding_name) - if watch_events: + if saw_event_context: if items is None: raise ConfigError( f"Shell-operator {config.binding_name} event context does not " f"contain {config.binding_name} snapshot objects" ) desired = reader.read_all(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() + prune_credentials = _credentials(changed) | _credentials(deleted) return HookInputs( changed, desired, deleted, prune_credentials, frozenset(reader.unreadable) ) diff --git a/python/openstack-sync/openstack_sync/hooks/ironic_runbooks.py b/python/openstack-sync/openstack_sync/hooks/ironic_runbooks.py new file mode 100644 index 000000000..e9101e74f --- /dev/null +++ b/python/openstack-sync/openstack_sync/hooks/ironic_runbooks.py @@ -0,0 +1,63 @@ +#!/usr/bin/env python3 +"""Shell-operator hook for Ironic runbook reconciliation.""" + +from __future__ import annotations + +import sys +from typing import Any + +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.ironic.runbooks import client +from openstack_sync.plugins.ironic.runbooks import prune as prune_module +from openstack_sync.plugins.ironic.runbooks import reconcile as reconcile_module +from openstack_sync.plugins.ironic.runbooks.config import BINDING_NAME +from openstack_sync.plugins.ironic.runbooks.config import ENV_PREFIX + + +class IronicRunbookPlugin(SyncPlugin): + """Sync IronicRunbook CRs into Ironic runbooks.""" + + noun = "ironic runbook" + + def wait_for_api(self, conn: Any) -> None: + client.wait_for_runbook_api( + conn, + retries=self.config.ready_retries, + delay=self.config.ready_delay, + ) + + def reconcile(self, conn: Any, spec: dict[str, Any], cache: Any) -> list[str]: + return reconcile_module.sync_runbook(conn, spec, cache) + + def prune( + self, + conn: Any, + desired_specs: list[dict[str, Any]], + *, + authoritative_empty: bool, + ) -> None: + if not self.config.prune: + return + prune_module.prune_removed_runbooks( + conn, desired_specs, authoritative_empty=authoritative_empty + ) + + +def main() -> int: + 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(IronicRunbookPlugin(config), hook_inputs(contexts, config)) + + return run_hook(lambda: build_crd_hook_config(ENV_PREFIX, BINDING_NAME), run) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/python/openstack-sync/openstack_sync/plugins/common.py b/python/openstack-sync/openstack_sync/plugins/common.py index 887486798..c4f6552f0 100644 --- a/python/openstack-sync/openstack_sync/plugins/common.py +++ b/python/openstack-sync/openstack_sync/plugins/common.py @@ -1,10 +1,4 @@ -"""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. -""" +"""Generic utilities shared across openstack-sync plugins.""" from __future__ import annotations @@ -12,6 +6,7 @@ import logging import os import time +from collections.abc import Callable from typing import Any from openstack import exceptions as openstack_exceptions @@ -111,6 +106,34 @@ def resource_id(resource: Any) -> str: return str(get_value(resource, "id")) +# --------------------------------------------------------------------------- +# API pagination +# --------------------------------------------------------------------------- + + +def paginated_collection( + fetch_page: Callable[[dict[str, Any]], dict[str, Any]], + *, + collection_key: str, + marker_key: str, + page_limit: int, +) -> list[Any]: + """Return every item from a marker-paginated OpenStack collection.""" + items: list[Any] = [] + marker: Any = None + + while True: + params: dict[str, Any] = {"limit": page_limit} + if marker is not None: + params["marker"] = marker + + page = fetch_page(params).get(collection_key, []) + items.extend(page) + if len(page) < page_limit: + return items + marker = page[-1][marker_key] + + # --------------------------------------------------------------------------- # meta_info helpers # --------------------------------------------------------------------------- @@ -146,10 +169,32 @@ def meta_info_payload(value: Any) -> str: # --------------------------------------------------------------------------- -# Neutron network readiness probe +# API readiness probes # --------------------------------------------------------------------------- +def wait_for_openstack_api( + service: str, + probe: Callable[[], Any], + retries: int = 30, + delay: float = 10.0, +) -> None: + """Poll *probe* until it succeeds, or raise after *retries*.""" + for attempt in range(1, retries + 1): + try: + probe() + return + except ConfigError: + raise + except Exception as exc: + if attempt >= retries: + raise RuntimeError( + f"{service} API did not become ready after {retries} attempt(s)" + ) from exc + LOG.info("Waiting for %s API (%s/%s): %s", service, attempt, retries, exc) + time.sleep(delay) + + def wait_for_openstack_network( conn: Any, retries: int = 30, @@ -165,17 +210,12 @@ def wait_for_openstack_network( 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) + wait_for_openstack_api( + "Neutron", + lambda: next(iter(conn.network.flavors()), None), + retries=retries, + delay=delay, + ) # --------------------------------------------------------------------------- diff --git a/python/openstack-sync/openstack_sync/plugins/ironic/__init__.py b/python/openstack-sync/openstack_sync/plugins/ironic/__init__.py new file mode 100644 index 000000000..1286fb768 --- /dev/null +++ b/python/openstack-sync/openstack_sync/plugins/ironic/__init__.py @@ -0,0 +1 @@ +"""Ironic sync plugins.""" diff --git a/python/openstack-sync/openstack_sync/plugins/ironic/runbooks/__init__.py b/python/openstack-sync/openstack_sync/plugins/ironic/runbooks/__init__.py new file mode 100644 index 000000000..3009f2b42 --- /dev/null +++ b/python/openstack-sync/openstack_sync/plugins/ironic/runbooks/__init__.py @@ -0,0 +1 @@ +"""Ironic runbook sync package.""" diff --git a/python/openstack-sync/openstack_sync/plugins/ironic/runbooks/client.py b/python/openstack-sync/openstack_sync/plugins/ironic/runbooks/client.py new file mode 100644 index 000000000..bd44a481f --- /dev/null +++ b/python/openstack-sync/openstack_sync/plugins/ironic/runbooks/client.py @@ -0,0 +1,151 @@ +"""Ironic runbook API calls through the baremetal proxy.""" + +from __future__ import annotations + +import logging +from typing import Any + +from openstack import exceptions as openstack_exceptions +from openstack import utils as openstack_utils + +from openstack_sync.plugins.common import ConfigError +from openstack_sync.plugins.common import paginated_collection +from openstack_sync.plugins.common import wait_for_openstack_api +from openstack_sync.plugins.ironic.runbooks.config import RUNBOOK_MICROVERSION + +LOG = logging.getLogger(__name__) + +_RUNBOOKS_PATH = "/runbooks" +_RUNBOOK_PAGE_LIMIT = 100 + + +# --------------------------------------------------------------------------- +# Readiness +# --------------------------------------------------------------------------- + + +def _version_tuple(microversion: str) -> tuple[int, ...]: + """Return *microversion* as a comparable tuple of ints.""" + try: + return tuple(int(part) for part in str(microversion).split(".")) + except ValueError as exc: + raise ConfigError( + f"Ironic reported an unusable API microversion {microversion!r}" + ) from exc + + +def check_microversion(conn: Any) -> None: + """Raise unless the cloud can serve :data:`RUNBOOK_MICROVERSION`.""" + supported = openstack_utils.maximum_supported_microversion( + conn.baremetal, RUNBOOK_MICROVERSION + ) + if supported is None: + raise ConfigError( + "Could not determine the Ironic API microversion; the baremetal " + "endpoint did not report its supported versions, so the runbook " + f"API cannot be used (requires {RUNBOOK_MICROVERSION})" + ) + if _version_tuple(supported) < _version_tuple(RUNBOOK_MICROVERSION): + raise ConfigError( + f"Ironic supports API microversion {supported} but this hook " + f"requires {RUNBOOK_MICROVERSION} for runbook descriptions and " + "traits; upgrade Ironic or disable the ironicRunbooks hook" + ) + + +def wait_for_runbook_api( + conn: Any, + retries: int = 30, + delay: float = 10.0, +) -> None: + """Poll until the runbook API is reachable and listable.""" + + def probe() -> None: + check_microversion(conn) + list_runbooks(conn, limit=1) + + wait_for_openstack_api("Ironic", probe, retries=retries, delay=delay) + + +# --------------------------------------------------------------------------- +# Requests +# --------------------------------------------------------------------------- + + +def _request(conn: Any, method: str, path: str, **kwargs: Any) -> Any: + """Send one baremetal request and raise for any non-2xx response.""" + response = conn.baremetal.request( + path, method, microversion=RUNBOOK_MICROVERSION, **kwargs + ) + openstack_exceptions.raise_from_response(response) + return response + + +def _json_body(response: Any) -> dict[str, Any]: + """Return the JSON body of *response*, or an empty dict when it has none.""" + if not response.content: + return {} + body = response.json() + return body if isinstance(body, dict) else {} + + +def list_runbooks(conn: Any, limit: int | None = None) -> list[dict[str, Any]]: + """Return every runbook visible to these credentials, with all fields.""" + if limit is not None: + response = _request( + conn, "GET", _RUNBOOKS_PATH, params={"detail": "true", "limit": limit} + ) + runbooks = _json_body(response).get("runbooks", []) + return [runbook for runbook in runbooks if isinstance(runbook, dict)] + + def fetch_page(params: dict[str, Any]) -> dict[str, Any]: + return _json_body( + _request( + conn, + "GET", + _RUNBOOKS_PATH, + params={"detail": "true", **params}, + ) + ) + + runbooks = paginated_collection( + fetch_page, + collection_key="runbooks", + marker_key="uuid", + page_limit=_RUNBOOK_PAGE_LIMIT, + ) + return [runbook for runbook in runbooks if isinstance(runbook, dict)] + + +def get_runbook(conn: Any, name: str) -> dict[str, Any] | None: + """Return the runbook named *name*, or None when Ironic does not have it.""" + try: + response = _request(conn, "GET", f"{_RUNBOOKS_PATH}/{name}") + except openstack_exceptions.NotFoundException: + return None + return _json_body(response) + + +def create_runbook(conn: Any, payload: dict[str, Any]) -> dict[str, Any]: + """Create a runbook from *payload* and return it as Ironic stored it.""" + response = _request(conn, "POST", _RUNBOOKS_PATH, json=payload) + return _json_body(response) + + +def patch_runbook(conn: Any, name: str, patch: list[dict[str, Any]]) -> dict[str, Any]: + """Apply a JSON patch to the runbook named *name*.""" + response = _request(conn, "PATCH", f"{_RUNBOOKS_PATH}/{name}", json=patch) + return _json_body(response) + + +def delete_runbook(conn: Any, name: str) -> None: + """Delete the runbook named *name*, treating an absent one as success.""" + try: + _request(conn, "DELETE", f"{_RUNBOOKS_PATH}/{name}") + except openstack_exceptions.NotFoundException: + LOG.info("Runbook %s is already absent from Ironic", name) + + +def set_traits(conn: Any, name: str, traits: list[str]) -> None: + """Replace every trait on the runbook named *name* with *traits*.""" + _request(conn, "PUT", f"{_RUNBOOKS_PATH}/{name}/traits", json={"traits": traits}) diff --git a/python/openstack-sync/openstack_sync/plugins/ironic/runbooks/config.py b/python/openstack-sync/openstack_sync/plugins/ironic/runbooks/config.py new file mode 100644 index 000000000..c07b06adb --- /dev/null +++ b/python/openstack-sync/openstack_sync/plugins/ironic/runbooks/config.py @@ -0,0 +1,17 @@ +"""Ironic runbook plugin constants. + +Runtime configuration comes from :class:`openstack_sync.hooks.framework.HookConfig`, +built from the ``IRONIC_RUNBOOK`` env prefix the Helm chart injects. +""" + +from __future__ import annotations + +#: The Ironic API microversion this plugin requires. It is the first with runbook +#: descriptions and the traits sub-resource, both of which the CRD exposes. +RUNBOOK_MICROVERSION = "1.112" + +#: Env prefix the Helm chart uses for this plugin's variables. +ENV_PREFIX = "IRONIC_RUNBOOK" + +#: shell-operator binding label for the CRD watch. +BINDING_NAME = "ironic-runbooks" diff --git a/python/openstack-sync/openstack_sync/plugins/ironic/runbooks/markers.py b/python/openstack-sync/openstack_sync/plugins/ironic/runbooks/markers.py new file mode 100644 index 000000000..d141416b5 --- /dev/null +++ b/python/openstack-sync/openstack_sync/plugins/ironic/runbooks/markers.py @@ -0,0 +1,48 @@ +"""Ownership markers for operator-managed Ironic runbooks. + +An IronicRunbook CR is an ownership claim for the Ironic runbook of the same +name. Runbooks the operator creates or adopts carry these markers in ``extra``, +Ironic's arbitrary metadata field; prune only deletes runbooks that have already +entered that managed set. +""" + +from __future__ import annotations + +from typing import Any + +from openstack_sync.plugins.common import get_value + +MANAGED_EXTRA_KEY = "_understack_runbook_operator" +MANAGED_EXTRA_VALUE = "managed" +MARKER_VERSION_EXTRA_KEY = "_understack_runbook_marker_version" +MARKER_VERSION_EXTRA_VALUE = "v1" +MARKER_SOURCE_EXTRA_KEY = "_understack_runbook_source" +MARKER_SOURCE_EXTRA_VALUE = "IronicRunbook" + +#: Marker keys stamped into a managed runbook's ``extra``. +OPERATOR_EXTRA_MARKERS = { + MANAGED_EXTRA_KEY: MANAGED_EXTRA_VALUE, + MARKER_VERSION_EXTRA_KEY: MARKER_VERSION_EXTRA_VALUE, + MARKER_SOURCE_EXTRA_KEY: MARKER_SOURCE_EXTRA_VALUE, +} + + +def runbook_extra(runbook: Any) -> dict[str, Any]: + """Return the ``extra`` of *runbook* as a dict. + + Ironic models ``extra`` as nullable, so a runbook without one comes back as + ``None``; an empty dict is the safe reading of that. + """ + extra = get_value(runbook, "extra", default={}) + return extra if isinstance(extra, dict) else {} + + +def managed_extra(value: Any) -> dict[str, Any]: + """Return *value* with the operator ownership markers merged in.""" + extra = value if isinstance(value, dict) else {} + return {**extra, **OPERATOR_EXTRA_MARKERS} + + +def is_managed_runbook(runbook: Any) -> bool: + """Return True when *runbook* carries the operator ownership marker.""" + return runbook_extra(runbook).get(MANAGED_EXTRA_KEY) == MANAGED_EXTRA_VALUE diff --git a/python/openstack-sync/openstack_sync/plugins/ironic/runbooks/prune.py b/python/openstack-sync/openstack_sync/plugins/ironic/runbooks/prune.py new file mode 100644 index 000000000..d20a5d499 --- /dev/null +++ b/python/openstack-sync/openstack_sync/plugins/ironic/runbooks/prune.py @@ -0,0 +1,66 @@ +"""Delete Ironic runbooks whose CR was removed. + +Everything here is gated on the operator's ownership marker. A hand-made runbook +is untouched until a CR causes the operator to create or adopt it; a runbook +carrying the marker is in the operator-managed set, which makes any further +filtering redundant. + +There is no in-use check to make: a runbook is named in a clean or service +request as that request is made, and Ironic keeps no reference from a node back +to a runbook. +""" + +from __future__ import annotations + +import logging +from typing import Any + +from openstack import exceptions as openstack_exceptions + +from openstack_sync.plugins.common import get_value +from openstack_sync.plugins.ironic.runbooks import client +from openstack_sync.plugins.ironic.runbooks.markers import is_managed_runbook + +LOG = logging.getLogger(__name__) + + +def _delete_runbook(conn: Any, name: str) -> None: + LOG.info("Deleting removed Ironic runbook %s", name) + try: + client.delete_runbook(conn, name) + except openstack_exceptions.ConflictException: + LOG.info("Ironic runbook %s is still in use; skipping delete", name) + + +def prune_removed_runbooks( + conn: Any, + desired_specs: list[dict[str, Any]], + *, + authoritative_empty: bool = False, +) -> None: + """Delete operator-owned runbooks 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 runbook. + """ + if not desired_specs and not authoritative_empty: + LOG.warning( + "No desired Ironic runbooks found; skipping prune to avoid deleting " + "all managed runbooks" + ) + return + + desired_names = { + str(spec["runbookName"]) for spec in desired_specs if spec.get("runbookName") + } + + LOG.info("Pruning removed Ironic runbooks") + for runbook in client.list_runbooks(conn): + name = get_value(runbook, "name") + if not name or name in desired_names: + continue + if not is_managed_runbook(runbook): + LOG.info("Keeping Ironic runbook %s; it is not operator-owned", name) + continue + _delete_runbook(conn, str(name)) diff --git a/python/openstack-sync/openstack_sync/plugins/ironic/runbooks/reconcile.py b/python/openstack-sync/openstack_sync/plugins/ironic/runbooks/reconcile.py new file mode 100644 index 000000000..6806f0bdd --- /dev/null +++ b/python/openstack-sync/openstack_sync/plugins/ironic/runbooks/reconcile.py @@ -0,0 +1,257 @@ +"""Reconcile an IronicRunbook CR onto Ironic.""" + +from __future__ import annotations + +import json +import logging +from typing import Any + +from openstack_sync.plugins.common import ConfigError +from openstack_sync.plugins.common import get_value +from openstack_sync.plugins.ironic.runbooks import client +from openstack_sync.plugins.ironic.runbooks.markers import is_managed_runbook +from openstack_sync.plugins.ironic.runbooks.markers import managed_extra +from openstack_sync.plugins.ironic.runbooks.markers import runbook_extra + +LOG = logging.getLogger(__name__) + + +# --------------------------------------------------------------------------- +# Spec -> Ironic payload +# --------------------------------------------------------------------------- + + +def validate_spec(spec: dict[str, Any]) -> str: + """Return the runbook name once the spec is valid.""" + name = str(spec.get("runbookName") or "") + if not name: + raise ConfigError("spec.runbookName must be set") + if spec.get("public") and spec.get("owner"): + raise ConfigError( + f"Runbook {name!r} sets both public and owner. Ironic does not allow " + "an owner on a public runbook. Drop spec.owner to share it with every " + "project, or set spec.public to false to keep it owned." + ) + return name + + +def _step_payload(index: int, step: Any) -> dict[str, Any]: + """Return one CR step as Ironic's runbook step.""" + if not isinstance(step, dict): + raise ConfigError(f"spec.steps[{index}] must be an object, got {step!r}") + + missing = [key for key in ("interface", "step", "order") if step.get(key) is None] + if missing: + raise ConfigError( + f"spec.steps[{index}] is missing required field(s): {', '.join(missing)}" + ) + + try: + order = int(step["order"]) + except (TypeError, ValueError) as exc: + raise ConfigError( + f"spec.steps[{index}].order must be an integer, got {step['order']!r}" + ) from exc + + return { + "interface": str(step["interface"]), + "step": str(step["step"]), + "args": step.get("args") or {}, + "order": order, + } + + +def desired_steps(spec: dict[str, Any]) -> list[dict[str, Any]]: + """Return the runbook steps *spec* describes, in Ironic's shape.""" + steps = spec.get("steps") + if not isinstance(steps, list) or not steps: + raise ConfigError("spec.steps must be a non-empty list") + return [_step_payload(index, step) for index, step in enumerate(steps)] + + +def canonical_steps(steps: Any) -> list[tuple[str, str, str, str]]: + """Return *steps* as an order-insensitive comparison key.""" + if not isinstance(steps, list): + return [] + return sorted( + ( + str(step.get("interface", "")), + str(step.get("step", "")), + str(step.get("order", "")), + json.dumps(step.get("args") or {}, sort_keys=True), + ) + for step in steps + if isinstance(step, dict) + ) + + +def desired_extra(spec: dict[str, Any]) -> dict[str, Any]: + """Return the ``extra`` to store, with the ownership markers merged in.""" + return managed_extra(spec.get("extra") or {}) + + +def desired_traits(spec: dict[str, Any]) -> list[str]: + """Return the traits *spec* asks for.""" + return [str(trait) for trait in spec.get("traits") or []] + + +def build_payload(spec: dict[str, Any]) -> dict[str, Any]: + """Return the body that creates the runbook *spec* describes.""" + payload: dict[str, Any] = { + "name": spec["runbookName"], + "steps": desired_steps(spec), + "public": bool(spec.get("public", False)), + "disable_ramdisk": bool(spec.get("disableRamdisk", False)), + "extra": desired_extra(spec), + "owner": str(spec["owner"]) if spec.get("owner") else None, + } + if spec.get("description"): + payload["description"] = str(spec["description"]) + return payload + + +# --------------------------------------------------------------------------- +# The runbook +# --------------------------------------------------------------------------- + + +def _patch_operations( + existing: dict[str, Any], spec: dict[str, Any] +) -> list[dict[str, Any]]: + """Return the JSON patch that converges *existing* onto *spec*.""" + operations: list[dict[str, Any]] = [] + + def set_field(field: str, value: Any) -> None: + operations.append({"op": "add", "path": f"/{field}", "value": value}) + + steps = desired_steps(spec) + if canonical_steps(existing.get("steps")) != canonical_steps(steps): + set_field("steps", steps) + + extra = desired_extra(spec) + if runbook_extra(existing) != extra: + set_field("extra", extra) + + public = bool(spec.get("public", False)) + if bool(existing.get("public", False)) != public: + set_field("public", public) + + disable_ramdisk = bool(spec.get("disableRamdisk", False)) + if bool(existing.get("disable_ramdisk", False)) != disable_ramdisk: + set_field("disable_ramdisk", disable_ramdisk) + + description = str(spec.get("description") or "") + if str(existing.get("description") or "") != description: + set_field("description", description) + + if spec.get("owner"): + owner = str(spec["owner"]) + if str(existing.get("owner") or "") != owner: + set_field("owner", owner) + elif not public and existing.get("owner") is not None: + set_field("owner", None) + + return operations + + +def ensure_runbook(conn: Any, spec: dict[str, Any]) -> dict[str, Any]: + """Create or converge the runbook *spec* describes, and return it.""" + name = str(spec["runbookName"]) + existing = client.get_runbook(conn, name) + + if existing is None: + payload = build_payload(spec) + LOG.info( + "Creating Ironic runbook %s with %s step(s)", name, len(payload["steps"]) + ) + return client.create_runbook(conn, payload) + + if is_managed_runbook(existing): + LOG.info("Ironic runbook %s already exists and is operator-owned", name) + else: + LOG.info( + "Adopting existing Ironic runbook %s; the CR is an ownership claim " + "for it, so the operator markers are being written to its extra", + name, + ) + + operations = _patch_operations(existing, spec) + if not operations: + return existing + + LOG.info( + "Updating Ironic runbook %s: %s", + name, + ", ".join(operation["path"] for operation in operations), + ) + return client.patch_runbook(conn, name, operations) + + +# --------------------------------------------------------------------------- +# Traits +# --------------------------------------------------------------------------- + + +def reconcile_traits( + conn: Any, runbook: dict[str, Any], spec: dict[str, Any] +) -> list[str]: + """Converge the traits of *runbook* onto *spec*, and return the result.""" + name = str(spec["runbookName"]) + desired = desired_traits(spec) + current = [str(trait) for trait in runbook.get("traits") or []] + if sorted(current) == sorted(desired): + return current + + LOG.info( + "Setting traits on Ironic runbook %s: have=%s want=%s", + name, + sorted(current), + sorted(desired), + ) + client.set_traits(conn, name, desired) + return desired + + +# --------------------------------------------------------------------------- +# Entry point +# --------------------------------------------------------------------------- + + +def render_runbook(runbook: dict[str, Any]) -> dict[str, Any]: + """Return the reconciled runbook as a loggable dict. + + Step arguments are summarised, not logged: they carry hardware settings and, + for some interfaces, credentials. + """ + steps = runbook.get("steps") if isinstance(runbook.get("steps"), list) else [] + return { + "uuid": get_value(runbook, "uuid"), + "name": get_value(runbook, "name"), + "description": get_value(runbook, "description"), + "public": get_value(runbook, "public"), + "owner": get_value(runbook, "owner"), + "disable_ramdisk": get_value(runbook, "disable_ramdisk"), + "traits": sorted(str(trait) for trait in runbook.get("traits") or []), + "steps": [ + f"{step.get('order')}:{step.get('interface')}.{step.get('step')}" + for step in steps + if isinstance(step, dict) + ], + "extra_keys": sorted(runbook_extra(runbook)), + } + + +def sync_runbook(conn: Any, spec: dict[str, Any], _cache: Any = None) -> list[str]: + """Converge one IronicRunbook spec.""" + name = validate_spec(spec) + + LOG.info("Reconciling Ironic runbook %s", name) + runbook = ensure_runbook(conn, spec) + traits = reconcile_traits(conn, runbook, spec) + + LOG.info( + "Reconciled Ironic runbook: %s", + # The traits the PUT just set are not in the body it answered with. + json.dumps(render_runbook({**runbook, "traits": traits}), sort_keys=True), + ) + return [] diff --git a/python/openstack-sync/tests/test_framework.py b/python/openstack-sync/tests/test_framework.py index 6bd9fe0b4..bd140b086 100644 --- a/python/openstack-sync/tests/test_framework.py +++ b/python/openstack-sync/tests/test_framework.py @@ -285,7 +285,13 @@ def test_enabled_hook_config_omits_namespace_without_pod_namespace(monkeypatch): # --------------------------------------------------------------------------- -def _cr(name: str, generation: int = 3, status: dict | None = None) -> dict: +def _cr( + name: str, + generation: int = 3, + status: dict | None = None, + secret: str = "infrasetup", + cloud: str = "understack", +) -> dict: obj = { "apiVersion": CRD_API_VERSION, "kind": CRD_KIND, @@ -293,8 +299,8 @@ def _cr(name: str, generation: int = 3, status: dict | None = None) -> dict: "spec": { "name": name, "cloudCredentialsRef": { - "secretName": "infrasetup", - "cloudName": "understack", + "secretName": secret, + "cloudName": cloud, }, }, } @@ -394,6 +400,29 @@ def test_added_event_reconciles_only_the_changed_resource(): ] +def test_event_prune_credentials_are_limited_to_changed_resource(): + config = make_hook_config() + changed = _cr("changed", secret="group-a", cloud="cloud-a") + other = _cr("other", secret="group-b", cloud="cloud-b") + contexts = [ + { + "binding": BINDING, + "type": "Event", + "watchEvent": "Modified", + "object": changed, + "snapshots": {BINDING: [{"object": changed}, {"object": other}]}, + } + ] + + inputs = hook_inputs(contexts, config) + + assert [r.spec["name"] for r in inputs.desired_resources_for_prune] == [ + "changed", + "other", + ] + assert inputs.prune_credentials == frozenset({("group-a", "cloud-a")}) + + def test_deleted_event_reconciles_nothing_but_prunes(): config = make_hook_config() contexts = [ @@ -413,6 +442,25 @@ def test_deleted_event_reconciles_nothing_but_prunes(): assert inputs.prune_credentials == frozenset({("infrasetup", "understack")}) +def test_event_without_watch_event_is_ignored_without_snapshot_reconcile(caplog): + config = make_hook_config() + contexts = [ + { + "binding": BINDING, + "type": "Event", + "object": _cr("bad"), + "snapshots": {BINDING: [{"object": _cr("kept")}]}, + } + ] + + inputs = hook_inputs(contexts, config) + + assert inputs.resources_to_reconcile == [] + assert [r.spec["name"] for r in inputs.desired_resources_for_prune] == ["kept"] + assert inputs.prune_credentials == frozenset() + assert "event carries no watchEvent" in caplog.text + + def test_modified_event_skipped_when_status_already_current(): """The hook's own status patch must not trigger another reconcile.""" config = make_hook_config() @@ -471,17 +519,10 @@ def test_unrecognised_context_is_an_error(): # --------------------------------------------------------------------------- # Unreadable CRs -# -# A CRD's required fields bind writes only. Kubernetes validates on admission, -# so an object stored before the schema required a field is still served by the -# watch exactly as stored, and admission is no guarantee about what a hook -# reads. The contract: name the offending CR, drop it, reconcile the rest, and -# never prune against the resulting incomplete desired set. # --------------------------------------------------------------------------- def _cr_without_credentials(name: str, generation: int = 1) -> dict: - """A CR stored before the CRD required spec.cloudCredentialsRef.""" return { "apiVersion": CRD_API_VERSION, "kind": CRD_KIND, @@ -501,7 +542,6 @@ def _snapshot_context(*objects: dict) -> list[dict]: def test_unreadable_cr_does_not_discard_the_readable_ones(): - """One malformed CR must not take down the whole batch.""" config = make_hook_config() contexts = _snapshot_context( _cr("good"), _cr_without_credentials("legacy"), _cr("also-good") @@ -539,7 +579,6 @@ def test_unreadable_cr_is_reported_by_namespace_and_name(caplog): ], ) def test_incomplete_cloud_credentials_ref_is_unreadable(creds, reason): - """minLength: 1 in the CRD does not constrain what is already stored.""" config = make_hook_config() obj = _cr_without_credentials("legacy") obj["spec"]["cloudCredentialsRef"] = creds @@ -643,12 +682,6 @@ def test_readable_crs_leave_nothing_unreadable(): def test_unreadable_crs_do_not_stall_a_whole_namespace(): - """Enabling a hook on a namespace that predates the CRD's required fields. - - The mixture that matters: several CRs stored under the older schema - alongside conforming ones. The conforming CRs must converge, the run must - still report failure, and prune must not act on the partial desired set. - """ config = make_hook_config(prune=True) plugin = StubPlugin(config) contexts = [ @@ -887,11 +920,6 @@ def test_run_sync_returns_error_when_prune_fails(): def test_run_sync_skips_prune_when_a_cr_was_unreadable(): - """An unreadable CR is missing from the desired set. - - Pruning against that set would delete the resource the unreadable CR still - describes, the same hazard as pruning after a failed reconcile. - """ plugin = StubPlugin(make_hook_config(prune=True)) inputs = _inputs([_resource("a")], unreadable=frozenset({"openstack/legacy"})) diff --git a/python/openstack-sync/tests/test_ironic_runbooks_hook.py b/python/openstack-sync/tests/test_ironic_runbooks_hook.py new file mode 100644 index 000000000..35a699007 --- /dev/null +++ b/python/openstack-sync/tests/test_ironic_runbooks_hook.py @@ -0,0 +1,384 @@ +"""Tests for the Ironic runbook hook wiring. + +The hook registers the right CRD watch, delegates reconcile and prune to the +plugin package, and processes CRs through the shared framework. What each +delegate does is covered in ``test_ironic_runbooks_reconcile.py`` and +``test_ironic_runbooks_prune.py``. +""" + +from __future__ import annotations + +import importlib +import json +import types +from pathlib import Path +from typing import Any +from unittest import mock + +import pytest + +from openstack_sync.hooks import ironic_runbooks as hook +from openstack_sync.hooks.framework import HookConfig +from openstack_sync.plugins.common import ConfigError +from openstack_sync.plugins.ironic.runbooks import markers +from openstack_sync.plugins.ironic.runbooks.config import BINDING_NAME +from openstack_sync.plugins.ironic.runbooks.config import ENV_PREFIX +from openstack_sync.plugins.ironic.runbooks.config import RUNBOOK_MICROVERSION +from tests.test_ironic_runbooks_reconcile import FakeBaremetal + +CRD_API_VERSION = "baremetal.ironicproject.org/v1alpha1" +CRD_KIND = "IronicRunbook" +CRD_RESOURCE = "ironicrunbooks.baremetal.ironicproject.org" + +RUNBOOK_NAME = "firmware-r740xd" + +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", + f"{ENV_PREFIX}_CRD_API_VERSION", + f"{ENV_PREFIX}_CRD_KIND", + f"{ENV_PREFIX}_CRD_RESOURCE", + "POD_NAMESPACE", +) + + +def clear_env(monkeypatch: pytest.MonkeyPatch) -> None: + for name in ENV_NAMES: + monkeypatch.delenv(name, raising=False) + + +def set_crd_identity(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv(f"{ENV_PREFIX}_CRD_API_VERSION", CRD_API_VERSION) + monkeypatch.setenv(f"{ENV_PREFIX}_CRD_KIND", CRD_KIND) + monkeypatch.setenv(f"{ENV_PREFIX}_CRD_RESOURCE", CRD_RESOURCE) + + +def make_ironic_config(**overrides: Any) -> HookConfig: + 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": True, + "prune": False, + "sync_crontab": "", + "ready_retries": 30, + "ready_delay": 10.0, + } + return HookConfig(**{**defaults, **overrides}) + + +def ironic_runbook_object(name: str, spec: dict[str, Any] | None = None) -> dict: + runbook_spec: dict[str, Any] = { + "cloudCredentialsRef": { + "secretName": "infrasetup", + "cloudName": "understack", + }, + "runbookName": name, + "description": f"{name} description", + "public": True, + "traits": ["CUSTOM_DELL_POWEREDGE_R740XD"], + "steps": [ + { + "interface": "firmware", + "step": "update", + "args": {"settings": [{"component": "bios", "wait": 1200}]}, + "order": 1, + } + ], + } + runbook_spec.update(spec or {}) + return { + "apiVersion": CRD_API_VERSION, + "kind": CRD_KIND, + "metadata": {"name": name, "namespace": "openstack", "generation": 3}, + "spec": runbook_spec, + } + + +def write_binding_context(path: Path, contexts: list[dict[str, Any]]) -> str: + context_path = path / "binding-context.json" + context_path.write_text(json.dumps(contexts), encoding="utf-8") + return str(context_path) + + +def schedule_context(*names: str) -> list[dict[str, Any]]: + return [ + { + "binding": BINDING_NAME, + "type": "Schedule", + "snapshots": { + BINDING_NAME: [{"object": ironic_runbook_object(n)} for n in names] + }, + } + ] + + +def test_module_import_is_safe_with_bad_runtime_env(monkeypatch: pytest.MonkeyPatch): + 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_disabled_startup_config( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +): + clear_env(monkeypatch) + monkeypatch.setattr(hook.sys, "argv", ["ironic_runbooks.py", "--config"]) + + assert hook.main() == 0 + assert json.loads(capsys.readouterr().out)["onStartup"] == 10 + + +def test_enabled_config_flag_watches_ironic_runbook_crd( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +): + clear_env(monkeypatch) + set_crd_identity(monkeypatch) + monkeypatch.setenv(f"{ENV_PREFIX}_ENABLED", "true") + monkeypatch.setenv("POD_NAMESPACE", "openstack") + monkeypatch.setattr(hook.sys, "argv", ["ironic_runbooks.py", "--config"]) + + assert hook.main() == 0 + config = json.loads(capsys.readouterr().out) + (binding,) = config["kubernetes"] + assert binding["name"] == BINDING_NAME + assert binding["apiVersion"] == CRD_API_VERSION + assert binding["kind"] == CRD_KIND + assert binding["namespace"] == {"nameSelector": {"matchNames": ["openstack"]}} + assert "schedule" not in config + + +def test_enabled_config_flag_adds_schedule_when_configured( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +): + clear_env(monkeypatch) + set_crd_identity(monkeypatch) + monkeypatch.setenv(f"{ENV_PREFIX}_ENABLED", "true") + monkeypatch.setenv(f"{ENV_PREFIX}_SYNC_CRONTAB", "*/10 * * * *") + monkeypatch.setattr(hook.sys, "argv", ["ironic_runbooks.py", "--config"]) + + assert hook.main() == 0 + config = json.loads(capsys.readouterr().out) + (schedule,) = config["schedule"] + assert schedule["crontab"] == "*/10 * * * *" + assert schedule["includeSnapshotsFrom"] == [BINDING_NAME] + assert schedule["queue"] == BINDING_NAME + + +def test_plugin_reconcile_delegates_to_sync_runbook(): + plugin = hook.IronicRunbookPlugin(make_ironic_config()) + conn = mock.MagicMock() + cache: dict[str, Any] = {} + spec = {"runbookName": "CUSTOM_BIOS_R740XD", "steps": []} + + with mock.patch.object( + hook.reconcile_module, "sync_runbook", return_value=["a note"] + ) as sync_runbook: + notes = plugin.reconcile(conn, spec, cache) + + assert notes == ["a note"] + sync_runbook.assert_called_once_with(conn, spec, cache) + + +def test_plugin_waits_for_the_runbook_api_with_the_configured_budget(): + plugin = hook.IronicRunbookPlugin( + make_ironic_config(ready_retries=5, ready_delay=2) + ) + conn = mock.MagicMock() + + with mock.patch.object(hook.client, "wait_for_runbook_api") as wait: + plugin.wait_for_api(conn) + + wait.assert_called_once_with(conn, retries=5, delay=2) + + +def test_plugin_prunes_only_when_the_chart_enabled_it(): + """PRUNE is opt-in: deleting a runbook is not undone by re-adding the CR.""" + conn = mock.MagicMock() + specs = [{"runbookName": "CUSTOM_KEEP", "steps": []}] + + with mock.patch.object(hook.prune_module, "prune_removed_runbooks") as do_prune: + hook.IronicRunbookPlugin(make_ironic_config(prune=False)).prune( + conn, specs, authoritative_empty=False + ) + do_prune.assert_not_called() + + hook.IronicRunbookPlugin(make_ironic_config(prune=True)).prune( + conn, specs, authoritative_empty=True + ) + do_prune.assert_called_once_with(conn, specs, authoritative_empty=True) + + +def test_main_returns_zero_when_hook_disabled( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +): + clear_env(monkeypatch) + set_crd_identity(monkeypatch) + monkeypatch.setenv( + "BINDING_CONTEXT_PATH", + write_binding_context(tmp_path, schedule_context(RUNBOOK_NAME)), + ) + + with ( + mock.patch.object(hook.sys, "argv", ["ironic_runbooks.py"]), + mock.patch( + "openstack_sync.hooks.framework.get_openstack_connection" + ) as connect, + mock.patch("openstack_sync.hooks.framework.patch_resource_status") as status, + ): + assert hook.main() == 0 + + connect.assert_not_called() + status.assert_not_called() + + +def test_main_reconciles_the_runbook_and_reports_synced( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +): + clear_env(monkeypatch) + set_crd_identity(monkeypatch) + monkeypatch.setenv(f"{ENV_PREFIX}_ENABLED", "true") + monkeypatch.setenv(f"{ENV_PREFIX}_STATUS_ENABLED", "true") + monkeypatch.setenv( + "BINDING_CONTEXT_PATH", + write_binding_context(tmp_path, schedule_context(RUNBOOK_NAME)), + ) + + with ( + mock.patch.object(hook.sys, "argv", ["ironic_runbooks.py"]), + mock.patch( + "openstack_sync.hooks.framework.get_openstack_connection", + return_value=mock.MagicMock(), + ) as connect, + mock.patch("openstack_sync.hooks.framework.patch_resource_status") as status, + mock.patch.object(hook.client, "wait_for_runbook_api"), + mock.patch.object( + hook.reconcile_module, "sync_runbook", return_value=[] + ) as sync_runbook, + ): + assert hook.main() == 0 + + connect.assert_called_once_with("infrasetup", "understack") + assert sync_runbook.call_args.args[1]["runbookName"] == RUNBOOK_NAME + assert status.call_args.kwargs["sync_status"] == "Synced" + assert status.call_args.kwargs["crd_kind"] == CRD_KIND + assert status.call_args.kwargs["crd_resource"] == CRD_RESOURCE + assert ( + status.call_args.kwargs["message"] == "Successfully reconciled ironic runbook" + ) + + +def test_main_reports_failed_when_the_runbook_cannot_be_reconciled( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +): + clear_env(monkeypatch) + set_crd_identity(monkeypatch) + monkeypatch.setenv(f"{ENV_PREFIX}_ENABLED", "true") + monkeypatch.setenv(f"{ENV_PREFIX}_STATUS_ENABLED", "true") + monkeypatch.setenv(f"{ENV_PREFIX}_PRUNE", "true") + monkeypatch.setenv( + "BINDING_CONTEXT_PATH", + write_binding_context(tmp_path, schedule_context(RUNBOOK_NAME)), + ) + + with ( + mock.patch.object(hook.sys, "argv", ["ironic_runbooks.py"]), + mock.patch( + "openstack_sync.hooks.framework.get_openstack_connection", + return_value=mock.MagicMock(), + ), + mock.patch("openstack_sync.hooks.framework.patch_resource_status") as status, + mock.patch.object(hook.client, "wait_for_runbook_api"), + mock.patch.object( + hook.reconcile_module, + "sync_runbook", + side_effect=ConfigError("steps must be a non-empty list"), + ), + mock.patch.object(hook.prune_module, "prune_removed_runbooks") as do_prune, + ): + assert hook.main() == 1 + + assert status.call_args.kwargs["sync_status"] == "Failed" + assert status.call_args.kwargs["message"] == "steps must be a non-empty list" + # The desired set is unknown once a CR failed, so nothing may be deleted. + do_prune.assert_not_called() + + +# --------------------------------------------------------------------------- +# End to end +# --------------------------------------------------------------------------- + + +def test_main_creates_then_prunes_against_a_fake_ironic( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +): + """One pass through the whole chain with nothing below the hook mocked. + + Binding context -> framework -> reconcile -> runbook client -> Ironic routes, + then the same for prune once the CR is gone. Only the connection, the + microversion discovery and kubectl are stood in for. + """ + fake = FakeBaremetal() + conn = types.SimpleNamespace(baremetal=fake) + + def run(contexts: list[dict[str, Any]], prune: str) -> int: + monkeypatch.setenv( + "BINDING_CONTEXT_PATH", write_binding_context(tmp_path, contexts) + ) + monkeypatch.setenv(f"{ENV_PREFIX}_PRUNE", prune) + with ( + mock.patch.object(hook.sys, "argv", ["ironic_runbooks.py"]), + mock.patch( + "openstack_sync.hooks.framework.get_openstack_connection", + return_value=conn, + ), + mock.patch("openstack_sync.hooks.framework.patch_resource_status"), + mock.patch.object( + hook.client.openstack_utils, + "maximum_supported_microversion", + return_value=RUNBOOK_MICROVERSION, + ), + ): + return hook.main() + + clear_env(monkeypatch) + set_crd_identity(monkeypatch) + monkeypatch.setenv(f"{ENV_PREFIX}_ENABLED", "true") + + assert run(schedule_context(RUNBOOK_NAME), "false") == 0 + created = fake.runbooks[RUNBOOK_NAME] + assert created["steps"][0]["args"] == { + "settings": [{"component": "bios", "wait": 1200}] + } + assert created["description"] == f"{RUNBOOK_NAME} description" + assert created["traits"] == ["CUSTOM_DELL_POWEREDGE_R740XD"] + assert markers.is_managed_runbook(created) + + # A second pass over an unchanged CR must not write anything. + fake.calls.clear() + assert run(schedule_context(RUNBOOK_NAME), "false") == 0 + assert fake.calls_for("PATCH") == [] + assert fake.calls_for("POST") == [] + assert fake.calls_for("PUT") == [] + + # The CR is deleted: with PRUNE on, the runbook goes with it. + deleted = [ + { + "binding": BINDING_NAME, + "type": "Event", + "watchEvent": "Deleted", + "object": ironic_runbook_object(RUNBOOK_NAME), + "snapshots": {BINDING_NAME: []}, + } + ] + assert run(deleted, "true") == 0 + assert fake.runbooks == {} diff --git a/python/openstack-sync/tests/test_ironic_runbooks_prune.py b/python/openstack-sync/tests/test_ironic_runbooks_prune.py new file mode 100644 index 000000000..741aa0e61 --- /dev/null +++ b/python/openstack-sync/tests/test_ironic_runbooks_prune.py @@ -0,0 +1,144 @@ +"""Tests for Ironic runbook prune behaviour.""" + +from __future__ import annotations + +from typing import Any +from unittest import mock + +import pytest +from openstack import exceptions as openstack_exceptions + +from openstack_sync.plugins.ironic.runbooks import client +from openstack_sync.plugins.ironic.runbooks import markers +from openstack_sync.plugins.ironic.runbooks import prune +from openstack_sync.plugins.ironic.runbooks.config import RUNBOOK_MICROVERSION +from tests.test_ironic_runbooks_reconcile import FakeBaremetal +from tests.test_ironic_runbooks_reconcile import _conn + + +def _owned(name: str) -> dict[str, Any]: + return { + "uuid": f"{name}-uuid", + "name": name, + "steps": [], + "extra": markers.managed_extra({"version": "1.0.0"}), + } + + +def _unowned(name: str) -> dict[str, Any]: + return {"uuid": f"{name}-uuid", "name": name, "steps": [], "extra": {}} + + +def _spec(name: str) -> dict[str, Any]: + return {"runbookName": name, "steps": []} + + +def _prune(fake: FakeBaremetal, specs: list[dict[str, Any]], **kwargs: Any) -> None: + prune.prune_removed_runbooks(_conn(fake), specs, **kwargs) + + +def test_owned_runbook_absent_from_the_desired_set_is_deleted(): + fake = FakeBaremetal([_owned("CUSTOM_KEEP"), _owned("CUSTOM_GONE")]) + + _prune(fake, [_spec("CUSTOM_KEEP")]) + + assert sorted(fake.runbooks) == ["CUSTOM_KEEP"] + assert fake.calls_for("DELETE") == ["/runbooks/CUSTOM_GONE"] + + +def test_owned_runbook_on_the_second_page_is_deleted(): + fake = FakeBaremetal([_owned("CUSTOM_KEEP"), _owned("CUSTOM_GONE")]) + + with mock.patch.object(client, "_RUNBOOK_PAGE_LIMIT", 1): + _prune(fake, [_spec("CUSTOM_KEEP")]) + + assert sorted(fake.runbooks) == ["CUSTOM_KEEP"] + get_params = [ + params + for (method, _), params in zip(fake.calls, fake.params, strict=True) + if method == "GET" + ] + assert get_params == [ + {"detail": "true", "limit": 1}, + {"detail": "true", "limit": 1, "marker": "CUSTOM_KEEP-uuid"}, + {"detail": "true", "limit": 1, "marker": "CUSTOM_GONE-uuid"}, + ] + assert fake.calls_for("DELETE") == ["/runbooks/CUSTOM_GONE"] + + +def test_runbook_the_operator_does_not_own_is_kept(): + """A hand-made runbook is not the operator's to delete.""" + fake = FakeBaremetal([_unowned("CUSTOM_HANDMADE")]) + + _prune(fake, [_spec("CUSTOM_KEEP")]) + + assert sorted(fake.runbooks) == ["CUSTOM_HANDMADE"] + assert fake.calls_for("DELETE") == [] + + +def test_runbook_without_a_name_is_skipped(): + fake = FakeBaremetal() + fake.runbooks["unnamed"] = {"uuid": "u", "extra": markers.managed_extra({})} + + _prune(fake, [_spec("CUSTOM_KEEP")]) + + assert fake.calls_for("DELETE") == [] + + +def test_empty_desired_set_is_refused_unless_a_cr_was_deleted(): + """An unreadable snapshot must not read as "delete everything".""" + fake = FakeBaremetal([_owned("CUSTOM_GONE")]) + + _prune(fake, []) + assert sorted(fake.runbooks) == ["CUSTOM_GONE"] + assert fake.calls == [] + + _prune(fake, [], authoritative_empty=True) + assert fake.runbooks == {} + + +def test_a_runbook_deleted_out_of_band_is_not_an_error(): + fake = FakeBaremetal([_owned("CUSTOM_GONE")]) + + def vanish(path: str, method: str, **kwargs: Any) -> Any: + if method == "DELETE": + fake.calls.append((method, path)) + fake.bodies.append(None) + fake.microversions.append(RUNBOOK_MICROVERSION) + raise openstack_exceptions.NotFoundException("already gone") + return FakeBaremetal.request(fake, path, method, **kwargs) + + with mock.patch.object(fake, "request", side_effect=vanish): + _prune(fake, [_spec("CUSTOM_KEEP")]) + + assert fake.calls_for("DELETE") == ["/runbooks/CUSTOM_GONE"] + + +def test_a_conflict_leaves_the_runbook_in_place(): + fake = FakeBaremetal([_owned("CUSTOM_GONE")]) + + def conflict(path: str, method: str, **kwargs: Any) -> Any: + if method == "DELETE": + raise openstack_exceptions.ConflictException("still in use") + return FakeBaremetal.request(fake, path, method, **kwargs) + + with mock.patch.object(fake, "request", side_effect=conflict): + _prune(fake, [_spec("CUSTOM_KEEP")]) + + assert sorted(fake.runbooks) == ["CUSTOM_GONE"] + + +def test_a_failure_other_than_conflict_or_not_found_stops_the_prune(): + """The framework reports a failed prune as a non-zero exit.""" + fake = FakeBaremetal([_owned("CUSTOM_GONE")]) + + def forbidden(path: str, method: str, **kwargs: Any) -> Any: + if method == "DELETE": + raise openstack_exceptions.ForbiddenException("not allowed") + return FakeBaremetal.request(fake, path, method, **kwargs) + + with ( + mock.patch.object(fake, "request", side_effect=forbidden), + pytest.raises(openstack_exceptions.ForbiddenException), + ): + _prune(fake, [_spec("CUSTOM_KEEP")]) diff --git a/python/openstack-sync/tests/test_ironic_runbooks_reconcile.py b/python/openstack-sync/tests/test_ironic_runbooks_reconcile.py new file mode 100644 index 000000000..c2bb3751b --- /dev/null +++ b/python/openstack-sync/tests/test_ironic_runbooks_reconcile.py @@ -0,0 +1,653 @@ +"""Tests for Ironic runbook reconciliation.""" + +from __future__ import annotations + +import json +import types +from typing import Any +from unittest import mock + +import pytest +import requests +from openstack import exceptions as openstack_exceptions + +from openstack_sync.plugins.common import ConfigError +from openstack_sync.plugins.ironic.runbooks import client +from openstack_sync.plugins.ironic.runbooks import markers +from openstack_sync.plugins.ironic.runbooks import reconcile +from openstack_sync.plugins.ironic.runbooks.config import RUNBOOK_MICROVERSION + +_NAME = "bmc-maintenance" + + +# --------------------------------------------------------------------------- +# Fake Ironic +# --------------------------------------------------------------------------- + + +def _response(status_code: int, body: Any = None) -> requests.Response: + response = requests.Response() + response.status_code = status_code + response.reason = "fake" + if body is not None: + response.headers["content-type"] = "application/json" + response._content = json.dumps(body).encode("utf-8") + else: + response._content = b"" + return response + + +class FakeBaremetal: + """In-memory stand-in for Ironic's runbook endpoints.""" + + def __init__(self, runbooks: list[dict[str, Any]] | None = None) -> None: + self.runbooks = {book["name"]: dict(book) for book in runbooks or []} + self.calls: list[tuple[str, str]] = [] + self.bodies: list[Any] = [] + self.params: list[dict[str, Any] | None] = [] + self.microversions: list[str] = [] + + # -- helpers for assertions --------------------------------------------- + + def calls_for(self, method: str) -> list[str]: + return [path for call_method, path in self.calls if call_method == method] + + def _bodies_for(self, method: str) -> list[Any]: + return [ + body + for (call_method, _), body in zip(self.calls, self.bodies, strict=True) + if call_method == method + ] + + @property + def patches(self) -> list[Any]: + return self._bodies_for("PATCH") + + @property + def trait_writes(self) -> list[Any]: + return self._bodies_for("PUT") + + @property + def created(self) -> Any: + posted = self._bodies_for("POST") + return posted[0] if posted else None + + def traits_of(self, name: str) -> list[str]: + return list(self.runbooks[name].get("traits") or []) + + # -- the API ------------------------------------------------------------ + + def request( + self, + path: str, + method: str, + microversion: str | None = None, + params: dict[str, Any] | None = None, + json: Any = None, + ) -> requests.Response: + self.calls.append((method, path)) + self.bodies.append(json) + self.params.append(params) + self.microversions.append(str(microversion)) + + parts = path.strip("/").split("/") + if parts[0] != "runbooks": + return _response(404, {"error_message": f"no route {path}"}) + + if len(parts) == 1: + if method == "GET": + runbooks = list(self.runbooks.values()) + if params and "marker" in params: + marker = str(params["marker"]) + start = next( + index + 1 + for index, runbook in enumerate(runbooks) + if runbook["uuid"] == marker + ) + runbooks = runbooks[start:] + if params and "limit" in params: + runbooks = runbooks[: int(params["limit"])] + return _response(200, {"runbooks": runbooks}) + if method == "POST": + book = dict(json) + if "traits" in book: + return _response(400, {"error_message": "traits not allowed"}) + book["traits"] = [] + self.runbooks[book["name"]] = book + return _response(201, book) + + if len(parts) == 2: + name = parts[1] + book = self.runbooks.get(name) + if book is None: + return _response(404, {"error_message": f"no runbook {name}"}) + if method == "GET": + return _response(200, book) + if method == "PATCH": + for operation in json: + field = operation["path"].lstrip("/") + if field == "traits": + return _response(400, {"error_message": "traits not patchable"}) + book[field] = operation["value"] + if field == "public" and operation["value"] is True: + book["owner"] = None + return _response(200, book) + if method == "DELETE": + del self.runbooks[name] + return _response(204) + + if len(parts) == 3 and parts[2] == "traits": + name = parts[1] + book = self.runbooks.get(name) + if book is None: + return _response(404, {"error_message": f"no runbook {name}"}) + if method == "PUT": + book["traits"] = list((json or {}).get("traits") or []) + return _response(204) + + return _response(405, {"error_message": f"{method} {path} not allowed"}) + + +def _conn(fake: FakeBaremetal) -> Any: + return types.SimpleNamespace(baremetal=fake) + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +def _spec(**overrides: Any) -> dict[str, Any]: + """A CR spec as the API server materialises it, with defaults applied.""" + spec: dict[str, Any] = { + "runbookName": _NAME, + "description": "Performs BMC maintenance", + "public": True, + "disableRamdisk": True, + "traits": ["CUSTOM_DELL_IDRAC"], + "steps": [ + {"interface": "management", "step": "clear_job_queue", "order": 1}, + {"interface": "management", "step": "set_bmc_clock", "order": 2}, + ], + "extra": {"version": "1.0.0"}, + } + spec.update(overrides) + return spec + + +def _runbook(**overrides: Any) -> dict[str, Any]: + """An Ironic runbook that matches ``_spec()`` exactly.""" + book: dict[str, Any] = { + "uuid": "runbook-uuid", + "name": _NAME, + "description": "Performs BMC maintenance", + "public": True, + "owner": None, + "disable_ramdisk": True, + "traits": ["CUSTOM_DELL_IDRAC"], + "steps": [ + { + "interface": "management", + "step": "clear_job_queue", + "args": {}, + "order": 1, + }, + { + "interface": "management", + "step": "set_bmc_clock", + "args": {}, + "order": 2, + }, + ], + "extra": markers.managed_extra({"version": "1.0.0"}), + } + book.update(overrides) + return book + + +# --------------------------------------------------------------------------- +# Spec validation +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "name", ["bmc-maintenance", "CUSTOM_BMC_MAINTENANCE", "firmware.r740xd_2.23~0"] +) +def test_any_url_safe_runbook_name_is_accepted(name: str): + """A runbook name is a logical name; eligibility comes from spec.traits.""" + assert reconcile.validate_spec(_spec(runbookName=name)) == name + + +@pytest.mark.parametrize("name", ["", None]) +def test_a_runbook_without_a_name_fails_the_cr(name: Any): + with pytest.raises(ConfigError, match="spec.runbookName must be set"): + reconcile.validate_spec(_spec(runbookName=name)) + + +def test_a_public_runbook_may_not_also_have_an_owner(): + """Ironic's runbook PATCH refuses an owner on a public runbook. + + Such a CR would create once and fail on every update after that, so it is + refused up front where the message can name the CR fields. + """ + fake = FakeBaremetal() + + with pytest.raises(ConfigError, match="both public and owner"): + reconcile.sync_runbook(_conn(fake), _spec(owner="project-123")) + + assert fake.calls == [] + + +def test_an_owned_private_runbook_is_fine(): + assert reconcile.validate_spec(_spec(public=False, owner="project-123")) == _NAME + + +# --------------------------------------------------------------------------- +# Spec -> payload +# --------------------------------------------------------------------------- + + +def test_steps_always_carry_args(): + """Ironic stores step args NOT NULL with no default.""" + steps = reconcile.desired_steps(_spec()) + + assert [step["args"] for step in steps] == [{}, {}] + assert steps[0] == { + "interface": "management", + "step": "clear_job_queue", + "args": {}, + "order": 1, + } + + +def test_steps_keep_supplied_args_and_coerce_order(): + steps = reconcile.desired_steps( + _spec( + steps=[ + { + "interface": "bios", + "step": "apply_configuration", + "order": "3", + "args": {"settings": [{"name": "LogicalProc"}]}, + } + ] + ) + ) + + assert steps == [ + { + "interface": "bios", + "step": "apply_configuration", + "args": {"settings": [{"name": "LogicalProc"}]}, + "order": 3, + } + ] + + +@pytest.mark.parametrize( + ("steps", "match"), + [ + ([], "non-empty list"), + (None, "non-empty list"), + (["not-an-object"], "must be an object"), + ([{"interface": "bios", "order": 1}], "missing required field"), + ([{"interface": "bios", "step": "x", "order": "later"}], "must be an integer"), + ], +) +def test_step_problems_fail_the_cr_by_name(steps: Any, match: str): + with pytest.raises(ConfigError, match=match): + reconcile.desired_steps(_spec(steps=steps)) + + +def test_payload_sets_owner_null_when_the_spec_does_not_claim_one(): + payload = reconcile.build_payload(_spec()) + + assert payload["owner"] is None + assert payload["public"] is True + assert payload["disable_ramdisk"] is True + assert payload["description"] == "Performs BMC maintenance" + assert payload["extra"] == markers.managed_extra({"version": "1.0.0"}) + + +def test_payload_carries_the_owner_the_spec_claims(): + payload = reconcile.build_payload(_spec(public=False, owner="project-123")) + + assert payload["owner"] == "project-123" + + +def test_payload_never_sends_traits(): + """Ironic rejects traits in a create or patch body.""" + assert "traits" not in reconcile.build_payload(_spec()) + + +# --------------------------------------------------------------------------- +# Create +# --------------------------------------------------------------------------- + + +def test_create_when_the_runbook_is_absent(): + fake = FakeBaremetal() + + assert reconcile.sync_runbook(_conn(fake), _spec()) == [] + + assert fake.calls_for("POST") == ["/runbooks"] + assert fake.created["name"] == _NAME + assert fake.created["extra"][markers.MANAGED_EXTRA_KEY] == ( + markers.MANAGED_EXTRA_VALUE + ) + assert fake.microversions == [RUNBOOK_MICROVERSION] * len(fake.calls) + + +def test_create_sets_traits_through_the_sub_resource(): + fake = FakeBaremetal() + + reconcile.sync_runbook(_conn(fake), _spec()) + + assert fake.calls_for("PUT") == [f"/runbooks/{_NAME}/traits"] + assert fake.trait_writes == [{"traits": ["CUSTOM_DELL_IDRAC"]}] + assert fake.traits_of(_NAME) == ["CUSTOM_DELL_IDRAC"] + + +def test_create_without_traits_writes_none(): + fake = FakeBaremetal() + + reconcile.sync_runbook(_conn(fake), _spec(traits=[])) + + assert fake.calls_for("PUT") == [] + + +# --------------------------------------------------------------------------- +# Converged +# --------------------------------------------------------------------------- + + +def test_converged_runbook_is_not_written_to_at_all(): + """A needless PATCH is a Modified event the hook watches, so it requeues.""" + fake = FakeBaremetal([_runbook()]) + + assert reconcile.sync_runbook(_conn(fake), _spec()) == [] + assert fake.calls == [("GET", f"/runbooks/{_NAME}")] + + +def test_step_order_from_ironic_does_not_count_as_drift(): + """Ironic does not promise to return steps in the order they were sent.""" + book = _runbook() + book["steps"] = list(reversed(book["steps"])) + fake = FakeBaremetal([book]) + + reconcile.sync_runbook(_conn(fake), _spec()) + + assert fake.patches == [] + + +def test_trait_order_from_ironic_does_not_count_as_drift(): + fake = FakeBaremetal([_runbook(traits=["CUSTOM_B", "CUSTOM_A"])]) + + reconcile.sync_runbook(_conn(fake), _spec(traits=["CUSTOM_A", "CUSTOM_B"])) + + assert fake.calls_for("PUT") == [] + + +# --------------------------------------------------------------------------- +# Drift +# --------------------------------------------------------------------------- + + +def test_a_runbook_with_no_steps_at_all_is_patched_back(): + """Ironic omits ``steps`` from a fields-limited response; treat it as empty.""" + book = _runbook() + del book["steps"] + fake = FakeBaremetal([book]) + + reconcile.sync_runbook(_conn(fake), _spec()) + + (patch,) = fake.patches + assert patch[0]["path"] == "/steps" + + +def test_step_drift_is_patched(): + book = _runbook() + book["steps"] = book["steps"][:1] + fake = FakeBaremetal([book]) + + reconcile.sync_runbook(_conn(fake), _spec()) + + (patch,) = fake.patches + assert patch == [ + {"op": "add", "path": "/steps", "value": reconcile.desired_steps(_spec())} + ] + + +def test_extra_drift_is_patched_with_the_markers_intact(): + fake = FakeBaremetal([_runbook(extra=markers.managed_extra({"version": "0.9.0"}))]) + + reconcile.sync_runbook(_conn(fake), _spec()) + + (patch,) = fake.patches + assert patch[0]["path"] == "/extra" + assert patch[0]["value"] == markers.managed_extra({"version": "1.0.0"}) + + +def test_unowned_runbook_is_adopted_by_stamping_its_extra(): + """The CR is an ownership claim; adoption is what makes prune safe later.""" + fake = FakeBaremetal([_runbook(extra={"version": "1.0.0"})]) + + reconcile.sync_runbook(_conn(fake), _spec()) + + (patch,) = fake.patches + assert patch == [ + { + "op": "add", + "path": "/extra", + "value": markers.managed_extra({"version": "1.0.0"}), + } + ] + assert markers.is_managed_runbook(fake.runbooks[_NAME]) + + +def test_public_drift_is_patched(): + fake = FakeBaremetal([_runbook(public=False)]) + + reconcile.sync_runbook(_conn(fake), _spec()) + + (patch,) = fake.patches + assert patch == [{"op": "add", "path": "/public", "value": True}] + + +def test_disable_ramdisk_drift_is_patched(): + fake = FakeBaremetal([_runbook(disable_ramdisk=False)]) + + assert reconcile.sync_runbook(_conn(fake), _spec()) == [] + + (patch,) = fake.patches + assert patch == [{"op": "add", "path": "/disable_ramdisk", "value": True}] + assert fake.runbooks[_NAME]["disable_ramdisk"] is True + + +def test_description_drift_is_patched(): + fake = FakeBaremetal([_runbook(description="stale")]) + + reconcile.sync_runbook(_conn(fake), _spec(description="fresh")) + + (patch,) = fake.patches + assert patch == [{"op": "add", "path": "/description", "value": "fresh"}] + + +def test_a_dropped_description_is_cleared(): + fake = FakeBaremetal([_runbook()]) + spec = _spec() + del spec["description"] + + reconcile.sync_runbook(_conn(fake), spec) + + (patch,) = fake.patches + assert patch == [{"op": "add", "path": "/description", "value": ""}] + + +def test_owner_is_cleared_when_the_spec_drops_it(): + fake = FakeBaremetal([_runbook(public=False, owner="project-123")]) + private = _spec(public=False) + + reconcile.sync_runbook(_conn(fake), private) + (patch,) = fake.patches + assert patch == [{"op": "add", "path": "/owner", "value": None}] + assert fake.runbooks[_NAME]["owner"] is None + + +def test_owner_is_patched_when_the_spec_claims_one(): + fake = FakeBaremetal([_runbook(public=False, owner="project-123")]) + private = _spec(public=False) + + reconcile.sync_runbook(_conn(fake), {**private, "owner": "project-456"}) + (patch,) = fake.patches + assert patch == [{"op": "add", "path": "/owner", "value": "project-456"}] + + +def test_switching_to_public_clears_owner_through_ironic(): + fake = FakeBaremetal([_runbook(public=False, owner="project-123")]) + + reconcile.sync_runbook(_conn(fake), _spec(public=True)) + + (patch,) = fake.patches + assert patch == [{"op": "add", "path": "/public", "value": True}] + assert fake.runbooks[_NAME]["owner"] is None + + +def test_patch_uses_add_so_it_works_on_fields_ironic_omits(): + """``replace`` on a member Ironic does not return is rejected by the patch.""" + fake = FakeBaremetal([_runbook(public=False, description="stale")]) + + reconcile.sync_runbook(_conn(fake), _spec()) + + assert {operation["op"] for patch in fake.patches for operation in patch} == {"add"} + + +# --------------------------------------------------------------------------- +# Traits +# --------------------------------------------------------------------------- + + +def test_traits_are_replaced_in_one_request(): + """One PUT for the whole set, so no node sees a half-applied runbook.""" + fake = FakeBaremetal([_runbook(traits=["CUSTOM_STALE", "CUSTOM_DELL_IDRAC"])]) + + reconcile.sync_runbook( + _conn(fake), _spec(traits=["CUSTOM_DELL_IDRAC", "CUSTOM_NEW"]) + ) + + assert fake.calls_for("PUT") == [f"/runbooks/{_NAME}/traits"] + assert fake.trait_writes == [{"traits": ["CUSTOM_DELL_IDRAC", "CUSTOM_NEW"]}] + assert fake.traits_of(_NAME) == ["CUSTOM_DELL_IDRAC", "CUSTOM_NEW"] + + +def test_dropping_every_trait_clears_them(): + fake = FakeBaremetal([_runbook()]) + + reconcile.sync_runbook(_conn(fake), _spec(traits=[])) + + assert fake.trait_writes == [{"traits": []}] + assert fake.traits_of(_NAME) == [] + + +# --------------------------------------------------------------------------- +# Microversion +# --------------------------------------------------------------------------- + + +def _check(reported: str | None) -> None: + with mock.patch.object( + client.openstack_utils, + "maximum_supported_microversion", + return_value=reported, + ): + client.check_microversion(_conn(FakeBaremetal())) + + +def test_check_microversion_accepts_a_cloud_at_the_required_version(): + _check(RUNBOOK_MICROVERSION) + + +def test_check_microversion_rejects_a_cloud_that_is_too_old(): + with pytest.raises(ConfigError, match=f"requires {RUNBOOK_MICROVERSION}"): + _check("1.101") + + +def test_check_microversion_rejects_an_undiscoverable_endpoint(): + with pytest.raises(ConfigError, match="Could not determine"): + _check(None) + + +def test_check_microversion_rejects_a_version_it_cannot_compare(): + with pytest.raises(ConfigError, match="unusable API microversion"): + _check("latest") + + +def test_readiness_probe_does_not_retry_a_cloud_that_cannot_be_fixed(): + """A too-old Ironic will not become new by waiting retries * delay seconds.""" + conn = _conn(FakeBaremetal()) + + with ( + mock.patch.object( + client.openstack_utils, + "maximum_supported_microversion", + return_value="1.101", + ), + mock.patch("openstack_sync.plugins.common.time.sleep") as sleep, + pytest.raises(ConfigError), + ): + client.wait_for_runbook_api(conn, retries=5, delay=0) + + sleep.assert_not_called() + + +def test_readiness_probe_lists_runbooks_so_policy_failures_surface_early(): + fake = FakeBaremetal() + + with mock.patch.object( + client.openstack_utils, + "maximum_supported_microversion", + return_value=RUNBOOK_MICROVERSION, + ): + client.wait_for_runbook_api(_conn(fake), retries=1, delay=0) + + assert fake.calls == [("GET", "/runbooks")] + + +# --------------------------------------------------------------------------- +# Client edges +# --------------------------------------------------------------------------- + + +def test_get_runbook_returns_none_for_an_absent_name(): + assert client.get_runbook(_conn(FakeBaremetal()), _NAME) is None + + +def test_client_raises_typed_errors_for_other_failures(): + fake = FakeBaremetal() + # POST /runbooks//traits is not a route Ironic serves. + with pytest.raises(openstack_exceptions.HttpException): + client._request(_conn(fake), "POST", f"/runbooks/{_NAME}/traits") + + +def test_delete_runbook_treats_an_absent_runbook_as_done(): + client.delete_runbook(_conn(FakeBaremetal()), _NAME) + + +# --------------------------------------------------------------------------- +# Logging +# --------------------------------------------------------------------------- + + +def test_render_runbook_summarises_steps_without_their_args(): + """Step args carry hardware settings and, for some interfaces, secrets.""" + rendered = reconcile.render_runbook( + _runbook( + steps=[{"interface": "bios", "step": "apply", "args": {"p": "s3cret"}}] + ) + ) + + assert rendered["steps"] == ["None:bios.apply"] + assert "s3cret" not in json.dumps(rendered) + assert rendered["traits"] == ["CUSTOM_DELL_IDRAC"] + assert rendered["description"] == "Performs BMC maintenance" + assert rendered["extra_keys"] == sorted(markers.managed_extra({"version": "1.0.0"})) diff --git a/python/openstack-sync/tests/test_plugins_common.py b/python/openstack-sync/tests/test_plugins_common.py index 8e4b2368f..855ace443 100644 --- a/python/openstack-sync/tests/test_plugins_common.py +++ b/python/openstack-sync/tests/test_plugins_common.py @@ -2,6 +2,8 @@ from __future__ import annotations +from unittest import mock + import pytest from openstack import exceptions as sdk_exceptions from openstack.network.v2 import flavor as sdk_flavor @@ -111,3 +113,75 @@ def test_meta_info_payload_canonicalizes_json_strings(): def test_normalize_meta_info_leaves_non_json_strings_unchanged(): assert common.normalize_meta_info("{'b': 2, 'a': 1}") == "{'b': 2, 'a': 1}" + + +# --------------------------------------------------------------------------- +# API readiness +# --------------------------------------------------------------------------- + + +def test_wait_for_openstack_api_returns_as_soon_as_the_probe_succeeds(): + probe = mock.Mock(side_effect=[RuntimeError("not yet"), None]) + + with mock.patch.object(common.time, "sleep") as sleep: + common.wait_for_openstack_api("Ironic", probe, retries=5, delay=1) + + assert probe.call_count == 2 + sleep.assert_called_once_with(1) + + +def test_wait_for_openstack_api_gives_up_after_retries(): + probe = mock.Mock(side_effect=RuntimeError("down")) + + with ( + mock.patch.object(common.time, "sleep"), + pytest.raises(RuntimeError, match="Ironic API did not become ready after 3"), + ): + common.wait_for_openstack_api("Ironic", probe, retries=3, delay=0) + + assert probe.call_count == 3 + + +def test_wait_for_openstack_api_does_not_retry_a_config_error(): + """A misconfigured or too-old API does not become ready by waiting.""" + probe = mock.Mock(side_effect=common.ConfigError("this cloud is too old")) + + with ( + mock.patch.object(common.time, "sleep") as sleep, + pytest.raises(common.ConfigError), + ): + common.wait_for_openstack_api("Ironic", probe, retries=30, delay=10) + + assert probe.call_count == 1 + sleep.assert_not_called() + + +def test_wait_for_openstack_network_probes_neutron_flavors(): + conn = mock.MagicMock() + + common.wait_for_openstack_network(conn, retries=1, delay=0) + + conn.network.flavors.assert_called_once_with() + + +def test_paginated_collection_uses_the_last_item_marker_for_the_next_page(): + pages = [ + {"runbooks": [{"uuid": "runbook-1"}, {"uuid": "runbook-2"}]}, + {"runbooks": [{"uuid": "runbook-3"}]}, + ] + params_seen = [] + + def fetch(params): + params_seen.append(dict(params)) + return pages.pop(0) + + assert common.paginated_collection( + fetch, + collection_key="runbooks", + marker_key="uuid", + page_limit=2, + ) == [{"uuid": "runbook-1"}, {"uuid": "runbook-2"}, {"uuid": "runbook-3"}] + assert params_seen == [ + {"limit": 2}, + {"limit": 2, "marker": "runbook-2"}, + ] diff --git a/schema/openstack-sync/ironic-runbook.schema.json b/schema/openstack-sync/ironic-runbook.schema.json new file mode 100644 index 000000000..047af686f --- /dev/null +++ b/schema/openstack-sync/ironic-runbook.schema.json @@ -0,0 +1,142 @@ +{ + "$schema": "https://json-schema.org/draft-07/schema#", + "$id": "https://rackerlabs.github.io/understack/schema/openstack-sync/ironic-runbook.schema.json", + "title": "OpenStack Sync Ironic Runbook Spec", + "description": "Schema for Ironic runbook spec data consumed by openstack-sync. When attached to an IronicRunbook custom resource, only spec is constrained.", + "oneOf": [ + { + "$ref": "#/definitions/ironicRunbookSpec" + }, + { + "type": "object", + "additionalProperties": true, + "properties": { + "spec": { + "$ref": "#/definitions/ironicRunbookSpec" + } + }, + "required": [ + "spec" + ] + } + ], + "definitions": { + "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 + } + } + }, + "ironicRunbookSpec": { + "description": "Ironic runbook data stored under spec.", + "type": "object", + "additionalProperties": false, + "required": ["cloudCredentialsRef", "runbookName", "steps"], + "properties": { + "cloudCredentialsRef": { + "$ref": "#/definitions/cloudCredentialsRef" + }, + "runbookName": { + "description": "Runbook name, and the identity the operator syncs by. Renaming creates a new runbook rather than renaming the existing one.", + "type": "string", + "minLength": 1, + "maxLength": 255, + "pattern": "^[A-Za-z0-9._~-]+$" + }, + "description": { + "description": "Human-readable runbook description.", + "type": "string", + "maxLength": 255 + }, + "traits": { + "description": "Traits deciding which nodes this runbook may act on. A node must carry at least one; a runbook with no traits matches no nodes.", + "type": "array", + "items": { + "type": "string", + "minLength": 1, + "maxLength": 255, + "pattern": "^CUSTOM_[A-Z0-9_]+$" + }, + "default": [] + }, + "steps": { + "description": "Ordered runbook steps.", + "type": "array", + "minItems": 1, + "items": { + "$ref": "#/definitions/runbookStep" + } + }, + "disableRamdisk": { + "description": "Whether to run without booting the cleaning ramdisk.", + "type": "boolean", + "default": false + }, + "public": { + "description": "Whether the runbook is available to all projects. A public runbook cannot have an owner.", + "type": "boolean", + "default": false + }, + "owner": { + "description": "Project that owns this runbook. Leave unset to let Ironic assign the credentials' own project.", + "type": "string", + "maxLength": 255 + }, + "extra": { + "description": "Additional runbook metadata. The operator also keeps its ownership markers here, under _understack_runbook_ keys.", + "type": "object", + "additionalProperties": true + } + } + }, + "runbookStep": { + "description": "A single Ironic runbook step.", + "type": "object", + "additionalProperties": false, + "required": ["interface", "step", "order"], + "properties": { + "interface": { + "description": "Interface that owns this cleaning step.", + "type": "string", + "enum": [ + "bios", + "deploy", + "firmware", + "management", + "power", + "raid", + "vendor" + ] + }, + "step": { + "description": "Step name for the selected interface.", + "type": "string", + "minLength": 1, + "maxLength": 255 + }, + "args": { + "description": "Step-specific arguments.", + "type": "object", + "additionalProperties": true + }, + "order": { + "description": "Execution order. Lower numbers run first.", + "type": "integer", + "minimum": 0 + } + } + } + } +}