diff --git a/components/ironic/kustomization.yaml b/components/ironic/kustomization.yaml index 897b75690..cf3215d57 100644 --- a/components/ironic/kustomization.yaml +++ b/components/ironic/kustomization.yaml @@ -11,7 +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 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/examples/README.md b/components/openstack-sync-plugins/ironic-runbooks/examples/README.md new file mode 100644 index 000000000..c4281f84d --- /dev/null +++ b/components/openstack-sync-plugins/ironic-runbooks/examples/README.md @@ -0,0 +1,53 @@ +# 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. + +## Removing one + +Deleting the CR does not delete the Ironic runbook. The hook only prunes when +`PRUNE` is enabled for it, and the chart default is `false`, so a removed CR +leaves the runbook in Ironic with nothing reconciling it. Delete both, or turn +pruning on deliberately — see +`docs/operator-guide/server-firmware-update.md#removing-a-runbook`. + +## 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/openstack-sync-plugins/ironic-runbooks/examples/bmc_maintenance.yaml b/components/openstack-sync-plugins/ironic-runbooks/examples/bmc_maintenance.yaml new file mode 100644 index 000000000..7f01e700c --- /dev/null +++ b/components/openstack-sync-plugins/ironic-runbooks/examples/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/runbook_bios_config.yaml b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_bios_config.yaml new file mode 100644 index 000000000..f6aca4361 --- /dev/null +++ b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_bios_config.yaml @@ -0,0 +1,69 @@ +# BIOS Configuration Runbook +# +# This runbook configures BIOS settings for compute nodes. +# Common use case: Enabling virtualization features for hypervisor nodes. +# +# Selects nodes carrying the trait: CUSTOM_COMPUTE_BIOS + +apiVersion: baremetal.ironicproject.org/v1alpha1 +kind: IronicRunbook +metadata: + name: compute-bios-config + namespace: baremetal-system + labels: + use-case: bios-configuration + hardware-type: compute +spec: + # REQUIRED: Secret holding the clouds.yaml, and the cloud entry to use. + cloudCredentialsRef: + secretName: infrasetup-system + 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 + order: 1 + args: + settings: + # Enable logical processors (hyperthreading) + - name: LogicalProc + value: Enabled + + # Enable virtualization technology + - name: VirtualizationTechnology + value: Enabled + + # Enable Intel VT-d (IOMMU) + - name: VtForDirectIo + value: Enabled + + # Enable SR-IOV support + - name: SRIOV + value: Enabled + + # Set boot mode to UEFI + - name: BootMode + value: Uefi + + # Enable secure boot + - name: SecureBoot + value: Enabled + + extra: + description: "BIOS configuration for compute nodes with virtualization support" + version: "1.0.0" + use_case: "Hypervisor node preparation" + hardware_compatibility: + - "Dell PowerEdge R740" + - "Dell PowerEdge R640" + - "HPE ProLiant DL380 Gen10" + notes: | + This runbook enables common virtualization features required for + running KVM/QEMU workloads. Adjust settings based on your specific + hardware and requirements. diff --git a/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_disk_cleaning.yaml b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_disk_cleaning.yaml new file mode 100644 index 000000000..53f2c64e3 --- /dev/null +++ b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_disk_cleaning.yaml @@ -0,0 +1,54 @@ +# Disk Cleaning Runbook +# +# This runbook performs secure disk erasure for node reuse. +# Common use case: Preparing nodes for redeployment or decommissioning. +# +# Selects nodes carrying the trait: CUSTOM_DISK_CLEAN + +apiVersion: baremetal.ironicproject.org/v1alpha1 +kind: IronicRunbook +metadata: + name: disk-cleaning + namespace: baremetal-system + labels: + use-case: disk-cleaning + security-level: standard +spec: + # REQUIRED: Secret holding the clouds.yaml, and the cloud entry to use. + cloudCredentialsRef: + secretName: infrasetup-system + 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 + step: erase_devices + order: 1 + args: + # Empty list means erase all devices + erase_skip_list: [] + + extra: + description: "Standard disk cleaning for node reuse" + version: "1.0.0" + use_case: "Secure disk erasure before redeployment" + security_level: "standard" + notes: | + This runbook performs a standard disk erase on all storage devices. + + Erase method depends on Ironic configuration: + - ATA Secure Erase (if supported by drive) + - NVMe Format (for NVMe drives) + - Software-based shred (fallback) + + For high-security environments, consider: + - Multiple pass overwrite + - DoD 5220.22-M standard + - Physical destruction for decommissioning + estimated_duration: "30-120 minutes depending on disk size and method" diff --git a/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_firmware_update.yaml b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_firmware_update.yaml new file mode 100644 index 000000000..5d6b1e6aa --- /dev/null +++ b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_firmware_update.yaml @@ -0,0 +1,91 @@ +# Firmware Update Runbook +# +# This runbook updates firmware components on baremetal nodes. +# Common use case: Updating BIOS, BMC, and NIC firmware. +# +# Selects nodes carrying the trait: CUSTOM_FIRMWARE_UPDATE + +apiVersion: baremetal.ironicproject.org/v1alpha1 +kind: IronicRunbook +metadata: + name: firmware-update + namespace: baremetal-system + labels: + use-case: firmware-update + hardware-type: general +spec: + # REQUIRED: Secret holding the clouds.yaml, and the cloud entry to use. + cloudCredentialsRef: + secretName: infrasetup-system + 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 + step: update_firmware + order: 1 + args: + component: bios + firmware_images: + - url: "http://firmware-repo.example.com/bios/R740_BIOS_2.15.0.bin" + checksum: "sha256:abc123..." + version: "2.15.0" + + # Step 2: Update BMC (iDRAC/iLO) firmware + - interface: management + step: update_firmware + order: 2 + args: + component: bmc + firmware_images: + - url: "http://firmware-repo.example.com/idrac/iDRAC9_4.40.00.00.bin" + checksum: "sha256:def456..." + version: "4.40.00.00" + + # Step 3: Update NIC firmware + - interface: management + step: update_firmware + order: 3 + args: + component: nic + firmware_images: + - url: "http://firmware-repo.example.com/nic/BCM5720_7.14.76.bin" + checksum: "sha256:ghi789..." + version: "7.14.76" + device_id: "14e4:165f" # Broadcom BCM5720 + + # Firmware updates typically don't need ramdisk + disableRamdisk: false + + extra: + description: "Firmware update runbook for BIOS, BMC, and NIC components" + version: "1.0.0" + use_case: "Firmware maintenance and security updates" + hardware_compatibility: + - "Dell PowerEdge R740" + - "Dell PowerEdge R640" + firmware_versions: + bios: "2.15.0" + bmc: "4.40.00.00" + nic: "7.14.76" + notes: | + This runbook updates critical firmware components: + 1. BIOS - System firmware + 2. BMC (iDRAC/iLO) - Management controller + 3. NIC - Network interface card + + Important: + - Ensure firmware images are accessible from the nodes + - Verify checksums match the official firmware releases + - Test on a single node before rolling out to production + - Some updates may require a reboot + warnings: + - "Firmware updates can take 10-30 minutes per component" + - "Do not power off nodes during firmware updates" + - "Verify hardware compatibility before applying updates" diff --git a/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_gpu_node_setup.yaml b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_gpu_node_setup.yaml new file mode 100644 index 000000000..df297f294 --- /dev/null +++ b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_gpu_node_setup.yaml @@ -0,0 +1,100 @@ +# GPU Node Setup Runbook +# +# This runbook configures nodes for GPU workloads. +# Common use case: Preparing nodes for ML/AI or GPU compute workloads. +# +# Selects nodes carrying the trait: CUSTOM_GPU_SETUP + +apiVersion: baremetal.ironicproject.org/v1alpha1 +kind: IronicRunbook +metadata: + name: gpu-node-setup + namespace: baremetal-system + labels: + use-case: gpu-configuration + hardware-type: gpu-compute +spec: + # REQUIRED: Secret holding the clouds.yaml, and the cloud entry to use. + cloudCredentialsRef: + secretName: infrasetup-system + 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 + step: apply_configuration + order: 1 + args: + settings: + # Enable virtualization for GPU passthrough + - name: VirtualizationTechnology + value: Enabled + + # Enable VT-d for IOMMU + - name: VtForDirectIo + value: Enabled + + # Enable SR-IOV for GPU virtualization + - name: SRIOV + value: Enabled + + # Enable Above 4G Decoding for large GPU memory + - name: Above4GDecoding + value: Enabled + + # Set PCIe speed to maximum + - name: PcieSpeed + value: Auto + + # Enable NUMA for optimal GPU-CPU affinity + - name: NumaMode + value: Enabled + + # Step 2: Update GPU firmware (optional) + - interface: management + step: update_firmware + order: 2 + args: + component: gpu + firmware_images: + - url: "http://firmware-repo.example.com/gpu/nvidia-vbios-latest.bin" + checksum: "sha256:xyz123..." + version: "latest" + + extra: + description: "GPU node BIOS and firmware configuration" + version: "1.0.0" + use_case: "Preparing nodes for GPU compute workloads" + hardware_compatibility: + - "Dell PowerEdge R740 with NVIDIA GPUs" + - "HPE ProLiant DL380 Gen10 with NVIDIA GPUs" + gpu_support: + - "NVIDIA Tesla V100" + - "NVIDIA A100" + - "NVIDIA H100" + features_enabled: + - "GPU passthrough (VT-d)" + - "SR-IOV for GPU virtualization" + - "NUMA for optimal performance" + - "Above 4G decoding for large GPU memory" + notes: | + This runbook prepares nodes for GPU workloads by: + 1. Enabling virtualization features for GPU passthrough + 2. Configuring IOMMU (VT-d) for device assignment + 3. Enabling SR-IOV for GPU virtualization + 4. Optimizing PCIe and NUMA settings + + After running this runbook: + - Verify GPU visibility with 'lspci | grep -i nvidia' + - Install GPU drivers appropriate for your workload + - Configure GPU device plugins for Kubernetes + recommended_next_steps: + - "Install NVIDIA drivers" + - "Install NVIDIA Container Toolkit" + - "Deploy NVIDIA GPU Operator (for Kubernetes)" diff --git a/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_raid_config.yaml b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_raid_config.yaml new file mode 100644 index 000000000..a72919016 --- /dev/null +++ b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_raid_config.yaml @@ -0,0 +1,74 @@ +# RAID Configuration Runbook +# +# This runbook configures RAID arrays for storage nodes. +# Common use case: Setting up RAID 1 for OS and RAID 6 for data. +# +# Selects nodes carrying the trait: CUSTOM_STORAGE_RAID + +apiVersion: baremetal.ironicproject.org/v1alpha1 +kind: IronicRunbook +metadata: + name: storage-raid-config + namespace: baremetal-system + labels: + use-case: raid-configuration + hardware-type: storage +spec: + # REQUIRED: Secret holding the clouds.yaml, and the cloud entry to use. + cloudCredentialsRef: + secretName: infrasetup-system + 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 + step: delete_configuration + order: 1 + args: {} + + # Step 2: Create new RAID configuration + - interface: raid + step: create_configuration + order: 2 + args: + logical_disks: + # RAID 1 for OS (root volume) + - size_gb: 500 + raid_level: "1" + is_root_volume: true + controller: "RAID.Integrated.1-1" + disk_type: "ssd" + interface_type: "sata" + volume_name: "OS" + + # RAID 6 for data storage + - size_gb: MAX + raid_level: "6" + is_root_volume: false + controller: "RAID.Integrated.1-1" + disk_type: "hdd" + interface_type: "sas" + volume_name: "DATA" + number_of_physical_disks: 8 + + extra: + description: "RAID configuration for storage nodes with OS and data volumes" + version: "1.0.0" + use_case: "Storage node RAID setup" + hardware_compatibility: + - "Dell PowerEdge R740xd" + - "Dell PowerEdge R7525" + raid_layout: | + - RAID 1 (500GB): Operating system on 2x SSDs + - RAID 6 (remaining): Data storage on 8x HDDs + notes: | + This configuration provides: + - High availability for OS with RAID 1 mirroring + - Large capacity with redundancy for data with RAID 6 + - Optimal performance by separating OS (SSD) and data (HDD) diff --git a/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_v1alpha1_complete.yaml b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_v1alpha1_complete.yaml new file mode 100644 index 000000000..c4b2a1751 --- /dev/null +++ b/components/openstack-sync-plugins/ironic-runbooks/examples/runbook_v1alpha1_complete.yaml @@ -0,0 +1,95 @@ +# Complete Runbook Example - All Fields +# +# This example demonstrates all available fields in a runbook, +# including both required and optional fields. +# +# Required fields: cloudCredentialsRef, runbookName, steps, interface, step, order +# Optional fields: description, traits, disableRamdisk, public, owner, extra, args + +apiVersion: baremetal.ironicproject.org/v1alpha1 +kind: IronicRunbook +metadata: + name: complete-runbook + namespace: baremetal-system + labels: + environment: production + hardware-type: compute + version: v1.0.0 + annotations: + description: "Complete example showing all available fields" +spec: + # REQUIRED: Secret holding the clouds.yaml, and the cloud entry to use + cloudCredentialsRef: + secretName: infrasetup-system + cloudName: understack + + # 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 + settings: + - name: LogicalProc + value: Enabled + - name: VirtualizationTechnology + value: Enabled + - name: SRIOV + value: Enabled + + # Step 2: RAID Configuration + - interface: raid # REQUIRED + step: create_configuration # REQUIRED + order: 2 # REQUIRED + args: # OPTIONAL + logical_disks: + - size_gb: 100 + raid_level: "1" + is_root_volume: true + - size_gb: 500 + raid_level: "5" + is_root_volume: false + + # Step 3: Disk Cleaning + - interface: deploy # REQUIRED + step: erase_devices # REQUIRED + order: 3 # REQUIRED + args: # OPTIONAL + erase_skip_list: [] + + # OPTIONAL: Skip ramdisk booting (default: false) + disableRamdisk: false + + # OPTIONAL: Make runbook public (default: false) + # Note: Cannot be true if owner is set + public: false + + # OPTIONAL: Project/tenant owner (default: null) + # Note: Cannot be set if public is true + owner: "project-123" + + # OPTIONAL: Additional metadata (default: {}) + extra: + description: "Complete example runbook with all fields" + version: "1.0.0" + maintainer: "ops-team@example.com" + documentation: "https://docs.example.com/runbooks/complete" + tags: + - production + - compute + - complete-example + changelog: + - version: "1.0.0" + date: "2024-01-14" + changes: "Initial version" 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..519608b3b --- /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-system + 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..9efcfde31 --- /dev/null +++ b/components/openstack-sync-plugins/ironic-runbooks/kustomization.yaml @@ -0,0 +1,5 @@ +--- +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization + +resources: [] 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/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/python/openstack-sync/README.md b/python/openstack-sync/README.md index ff610b1f6..dc0b0d54d 100644 --- a/python/openstack-sync/README.md +++ b/python/openstack-sync/README.md @@ -15,24 +15,25 @@ 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 `run_sync` groups CRs by the credentials in `spec.cloudCredentialsRef`, opens one connection per credential group, waits for the OpenStack service, reconciles each -CR, patches `Synced`/`Failed` onto the CR status, and then prunes. If any -reconcile fails, 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 -be unsafe. +CR, patches `Synced`/`Failed` onto the CR status, and then calls the plugin's +prune step, which most plugins gate on `PRUNE`. 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 be unsafe. A CR whose spec does not satisfy the framework's contract is named in the log and dropped, and the run exits non-zero. The remaining CRs still reconcile: one @@ -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..7f5ce943d --- /dev/null +++ b/python/openstack-sync/openstack_sync/plugins/ironic/runbooks/client.py @@ -0,0 +1,166 @@ +"""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, runbook_uuid: str, patch: list[dict[str, Any]] +) -> dict[str, Any]: + """Apply a JSON patch to the runbook with UUID *runbook_uuid*. + + Writes address the runbook by UUID, not by name. Ironic resolves either in + the path, but the UUID is the identifier that cannot be renamed out from + under the request. + """ + response = _request(conn, "PATCH", f"{_RUNBOOKS_PATH}/{runbook_uuid}", 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, runbook_uuid: str, traits: list[str]) -> None: + """Replace every trait on the runbook with UUID *runbook_uuid*. + + Addressed by UUID for the same reason as :func:`patch_runbook`. + """ + _request( + conn, + "PUT", + f"{_RUNBOOKS_PATH}/{runbook_uuid}/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..93d819b61 --- /dev/null +++ b/python/openstack-sync/openstack_sync/plugins/ironic/runbooks/reconcile.py @@ -0,0 +1,274 @@ +"""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 _runbook_uuid(runbook: dict[str, Any], name: str) -> str: + """Return the UUID Ironic assigned to *runbook*. + + Every write goes to the UUID rather than the name. Ironic accepts either in + the path, but the UUID is what the runbook keeps across a rename, so a + write can never land on whatever else answers to that name. + """ + uuid = str(runbook.get("uuid") or "") + if not uuid: + raise ConfigError( + f"Ironic returned runbook {name!r} without a uuid, so it cannot be " + "updated; the response was truncated or the API is not serving " + "runbooks as expected" + ) + return uuid + + +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, _runbook_uuid(existing, 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, _runbook_uuid(runbook, 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..336b085b0 --- /dev/null +++ b/python/openstack-sync/tests/test_ironic_runbooks_reconcile.py @@ -0,0 +1,701 @@ +"""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 []) + + def uuid_of(self, name: str) -> str: + """The UUID Ironic holds for *name*; writes must be addressed to it.""" + return str(self.runbooks[name]["uuid"]) + + def _lookup(self, ident: str) -> dict[str, Any] | None: + """Resolve *ident* the way Ironic does: as a UUID first, then a name.""" + for book in self.runbooks.values(): + if book.get("uuid") == ident: + return book + return self.runbooks.get(ident) + + # -- 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"] = [] + # Ironic generates the UUID on create and returns it, which is + # what the caller then addresses its traits PUT to. + book.setdefault("uuid", f"{book['name']}-uuid") + self.runbooks[book["name"]] = book + return _response(201, book) + + if len(parts) == 2: + ident = parts[1] + book = self._lookup(ident) + if book is None: + return _response(404, {"error_message": f"no runbook {ident}"}) + 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[book["name"]] + return _response(204) + + if len(parts) == 3 and parts[2] == "traits": + ident = parts[1] + book = self._lookup(ident) + if book is None: + return _response(404, {"error_message": f"no runbook {ident}"}) + 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/{fake.uuid_of(_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/{fake.uuid_of(_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) == [] + + +# --------------------------------------------------------------------------- +# Identifiers +# --------------------------------------------------------------------------- + + +def test_writes_are_addressed_to_the_uuid_not_the_name(): + """The CR supplies a name, so the read is by name and every write by UUID. + + Ironic resolves either identifier in the path, so a name-addressed write + works right up until the name stops belonging to the runbook the operator + read; the UUID it already holds has no such window. + """ + fake = FakeBaremetal([_runbook(public=False, traits=["CUSTOM_STALE"])]) + + reconcile.sync_runbook(_conn(fake), _spec()) + + uuid = fake.uuid_of(_NAME) + assert fake.calls_for("GET") == [f"/runbooks/{_NAME}"] + assert fake.calls_for("PATCH") == [f"/runbooks/{uuid}"] + assert fake.calls_for("PUT") == [f"/runbooks/{uuid}/traits"] + + +def test_a_runbook_with_no_uuid_fails_the_cr_instead_of_writing(): + """Better a failed CR than a PATCH to /runbooks/None.""" + book = _runbook(public=False) + del book["uuid"] + fake = FakeBaremetal([book]) + + with pytest.raises(ConfigError, match="without a uuid"): + reconcile.sync_runbook(_conn(fake), _spec()) + + assert fake.calls_for("PATCH") == [] + + +# --------------------------------------------------------------------------- +# 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 + } + } + } + } +}