From 455482d4dac572b9bacf91347f73b18f9fbdb532 Mon Sep 17 00:00:00 2001 From: Hemanth kumar Pannem Date: Thu, 25 Jun 2026 17:38:00 -0700 Subject: [PATCH 1/3] extraconfig tests address comments sdd specs for e2e doc update --- .sdd/INDEX.md | 1 + .../specs/002-vm-extraconfig-reconcile/e2e.md | 73 ++ .../002-vm-extraconfig-reconcile/tasks.md | 54 ++ api/v1alpha6/virtualmachine_types.go | 4 + docs/concepts/workloads/vm.md | 56 +- pkg/vmconfig/extraconfig/reconciler.go | 15 +- pkg/vmconfig/extraconfig/reconciler_test.go | 91 +- test/e2e/vmservice/config/wcp.yaml | 3 +- test/e2e/vmservice/consts/consts.go | 1 + .../virtualmachine/vm_extraconfig.go | 873 ++++++++++++++++++ test/e2e/vmservice/vmservice_test.go | 14 +- 11 files changed, 1146 insertions(+), 39 deletions(-) create mode 100644 .sdd/specs/002-vm-extraconfig-reconcile/e2e.md create mode 100644 .sdd/specs/002-vm-extraconfig-reconcile/tasks.md create mode 100644 test/e2e/vmservice/vmservice/virtualmachine/vm_extraconfig.go diff --git a/.sdd/INDEX.md b/.sdd/INDEX.md index 20cbdade3..ad56c9aa5 100644 --- a/.sdd/INDEX.md +++ b/.sdd/INDEX.md @@ -32,6 +32,7 @@ Each spec lives under `specs/NNN-slug/`. Standard artifacts: `spec.md` (behavior |---|------|-------|--------|------| | 000 | [sdd](specs/000-sdd/) | VM Operator Specification Driven Development | In Progress | vmop-3820 | | 001 | [class-policy-resize](specs/001-class-policy-resize/) | VM Service Class Policy and Resize — Policy + Environment Browser Pipeline | In Progress | vmop-3331 | +| 002 | [vm-extraconfig-reconcile](specs/002-vm-extraconfig-reconcile/) | VM ExtraConfig Reconciliation — E2E Coverage (`spec.md`/`plan.md` pending) | In Progress | vmop-3782 | ### Finding the right spec diff --git a/.sdd/specs/002-vm-extraconfig-reconcile/e2e.md b/.sdd/specs/002-vm-extraconfig-reconcile/e2e.md new file mode 100644 index 000000000..ff0acaddb --- /dev/null +++ b/.sdd/specs/002-vm-extraconfig-reconcile/e2e.md @@ -0,0 +1,73 @@ +# E2E Test Plan: VM ExtraConfig Reconciliation + +- **Epic**: vmop-3782 +- **Tasks**: [`tasks.md`](./tasks.md) + +> `spec.md`, `plan.md`, and `model.md` for this feature are not written yet. This +> document specifies the E2E acceptance criteria directly; fold it under a +> `spec.md`'s "User stories" section when that spec is authored, rather than +> duplicating the scenario tables. + +This document specifies the end-to-end test suite that guards `spec.advanced` +ExtraConfig reconciliation on a real Supervisor: the set of scenarios the +reconciler and its `VirtualMachineExtraConfigSynced` condition must satisfy, +grouped by the capability each one guards. + +## Suite + +- `VMExtraConfigSpec`, in `test/e2e/vmservice/vmservice/virtualmachine/vm_extraconfig.go`, registered from `test/e2e/vmservice/vmservice_test.go` under a `Context("VM-EXTRACONFIG", ...)` block. +- Every scenario carries `Label("extraconfig", ...)` plus either `"core-functional"` or `"extended-functional"` — the latter for scenarios that need direct `govmomi` access to vSphere (out-of-band power-off, out-of-band destroy) and are more disruptive to the test VM's lifecycle. +- The suite requires one new wait-interval key in `test/e2e/vmservice/config/wcp.yaml`: `default/wait-vm-extraconfig-synced` (5m/10s), used for every wait on the `ExtraConfigSynced` condition. Power-state transitions reuse the existing `default/wait-virtual-machine-powerstate` key (via `vmoperator.WaitForVirtualMachinePowerState`/`UpdateVirtualMachinePowerState`) — do not add a second, extraconfig-specific power-state interval; those helpers hardcode the shared key and are used by many other suites. +- The suite requires a capability constant in `test/e2e/vmservice/consts/consts.go`: `TelcoVMServiceAPICapabilityName = "supports_telco_vm_service_api"`. +- Each scenario MUST create its own VM and clean it up independently (`DeferCleanup`), so every `It` block is independently runnable via `TEST_FOCUS`/`LABEL_FILTER`. +- Unless a scenario is specifically testing interaction with another reconciler, every VM it creates MUST set `spec.promoteDisksMode: Disabled` and `spec.bootstrap.disabled: true`. This keeps disk-promotion (SvMotion) and cloud-init customization from racing with or delaying ExtraConfig reconciliation, which would make `ExtraConfigSynced` timing non-deterministic. The one deliberate exception is the disk-promotion-concurrency scenario below, which leaves `promoteDisksMode` at its default (`Online`) specifically to exercise that race. + +## VMX key reference + +First-class `spec.advanced` fields surface in `status.extraConfig` under the VMX key their `vmxmode`/vmx struct tag maps to, not their Go field name. Assertions against `status.extraConfig` MUST use these literal keys (mirror the vmx struct tags in the active `api/v1alphaN` `VirtualMachineAdvancedSpec`; verify against source if the API version has moved on): + +| `spec.advanced` field | VMX key | Mode | +|---|---|---| +| `preferHtEnabled` | `numa.vcpu.preferHT` | PowerCycle | +| `timeTrackerLowLatencyEnabled` | `timeTracker.lowLatency` | PowerCycle | +| `cpuAffinityExclusiveNoStatsEnabled` | `sched.cpu.affinity.exclusiveNoStats` | PowerCycle | +| `vmxSwapEnabled` | `sched.swap.vmxSwapEnabled` | PowerCycle | +| `pnumaNodeAffinity` (`[]int32`, comma-separated encoding) | `numa.nodeAffinity` | PowerCycle | +| `hugePages1GEnabled` | `sched.mem.lpage.enable1GPage` | PowerOff | + +The condition under test is `vmopv1.VirtualMachineExtraConfigSynced`, with reasons `"PowerCyclePending"` and `"PowerOffRequired"` (mirror the reconciler's condition-reason constants; verify against source if renamed). + +## Gating + +The suite MUST skip entirely when the `supports_telco_vm_service_api` Supervisor capability is disabled — `spec.advanced`'s first-class VMX fields are inert without it, so there is nothing to observe. + +## Scenarios + +### Capability: applying first-class VMX fields and bag keys, reflected in status + +| Scenario | Verifies | +|----------|----------| +| creates VM with PowerCycle-mode first-class fields and bag keys, syncs immediately, reflects bag key CRUD | On create, first-class VMX fields (`preferHtEnabled`, `timeTrackerLowLatencyEnabled`, `cpuAffinityExclusiveNoStatsEnabled`, `vmxSwapEnabled`) and `spec.advanced.extraConfig` bag keys land in `status.extraConfig` and `ExtraConfigSynced` reaches `True`; a subsequent patch that adds a key, updates a key's value, and omits a previously-set key is reflected in `status.extraConfig` — the omitted key is removed, not merely left stale | +| applies extraConfig correctly while disk promotion runs concurrently (default `PromoteDisksMode=Online`) | `ExtraConfigSynced` reaches `True` and every first-class/bag key is present even when the disk-promotion (SvMotion) reconciler may still be in flight on the same VM — no ordering or resource-conflict regression between the two reconcilers | + +### Capability: PowerCycle-mode vs PowerOff-mode field semantics, surfaced via `ExtraConfigSynced` + +| Scenario | Verifies | +|----------|----------| +| marks `PowerCyclePending` when a PowerCycle-mode field changes while the VM is powered on | Flipping a `vmxmode:"powercycle"` field (e.g. `vmxSwapEnabled`) on a running VM applies the value immediately but sets `ExtraConfigSynced=False/PowerCyclePending`; a manual power-off then power-on clears the condition to `True` and the new value is confirmed in `status.extraConfig` | +| defers a PowerOff-mode field while the VM is powered on, applies it after power-off | Adding a `vmxmode:"poweroff"` field (e.g. `hugePages1GEnabled`) to a running VM sets `ExtraConfigSynced=False/PowerOffRequired`, names the deferred VMX key in the condition message, and the key MUST NOT appear in `status.extraConfig` while deferred; powering the VM off applies it, and every other first-class/bag key remains intact afterward | +| `PowerOffRequired` takes priority over `PowerCyclePending` when a PowerOff-mode and a PowerCycle-mode field change simultaneously | Patching both a `poweroff`-mode field and a `powercycle`-mode field in the same update yields `ExtraConfigSynced=False/PowerOffRequired` — never `PowerCyclePending` — and the message names the PowerOff-mode field | +| handles the `[]int32` first-class field (`pnumaNodeAffinity`) correctly | A comma-separated encoding (e.g. `"0"`) is visible in `status.extraConfig` after create; clearing the field to `nil` on a running VM is treated as a PowerCycle-mode change (`PowerCyclePending`), and the key is absent from `status.extraConfig` once the clear is applied after a power-off | + +### Capability: resilience — extraConfig survives VM recreation and out-of-band vSphere drift + +| Scenario | Verifies | +|----------|----------| +| re-applies all extraConfig keys when the VM is deleted via Kubernetes and recreated with an identical spec | First-class fields and bag keys are fully re-applied from spec on the recreated VM — no key is silently skipped because it was already applied to the previous underlying vSphere VM | +| resolves `PowerCyclePending` after an out-of-band vSphere power-off | Powering the VM off directly via `govmomi` (bypassing VM Operator) is detected, the pending PowerCycle-mode change applies while the VM is off, `ExtraConfigSynced` reaches `True`, and the operator's normal drift recovery restores `spec.powerState`'s desired `PoweredOn` state afterward — with extraConfig still intact post-recovery | +| re-applies extraConfig after the vSphere VM is destroyed out-of-band | Destroying the underlying vSphere VM directly (the Kubernetes object survives) and then toggling `spec.powerState` to `PoweredOn` MUST cause the operator to recreate the VM (`status.uniqueID` changes) and re-apply every first-class/bag key from spec on the new VM — not silently leave the object orphaned or extraConfig-less | + +## Scope boundaries (not E2E-tested by design or as a follow-up) + +- NIC-level network ExtraConfig (`pkg/vmconfig/networkextraconfig`, reconciling `spec.network.interfaces[].vmxnet3` VMX properties) has no E2E coverage in this suite today. It is exercised at the unit/integration layer only. Per `e2e-sync-with-changes.md`, this is a gap to close in a follow-up spec/task, not a deliberate scope exclusion — call it out explicitly rather than letting it read as covered. +- The `supports_telco_vm_service_api` webhook-rejection behavior when the capability is disabled is not exercised here; this suite's whole-suite skip (see "Gating" above) is the only E2E-level signal for the disabled case. Rejection itself belongs at the webhook unit-test layer. diff --git a/.sdd/specs/002-vm-extraconfig-reconcile/tasks.md b/.sdd/specs/002-vm-extraconfig-reconcile/tasks.md new file mode 100644 index 000000000..89737b2c0 --- /dev/null +++ b/.sdd/specs/002-vm-extraconfig-reconcile/tasks.md @@ -0,0 +1,54 @@ +# Tasks: VM ExtraConfig Reconciliation — E2E Coverage + +- **E2E plan**: [`e2e.md`](./e2e.md) +- **Epic**: vmop-3782 + + + +This task list produces the E2E suite specified in `e2e.md`: coverage for +`spec.advanced` ExtraConfig reconciliation — first-class VMX fields, bag key +CRUD, PowerCycle/PowerOff-mode semantics and their condition precedence, and +resilience to VM recreation and out-of-band vSphere drift. It assumes the +`VirtualMachineExtraConfigSynced` condition and its `PowerOffRequired`/ +`PowerCyclePending` reasons already exist as reconciler surface to test +against; no product-code task is included here. + +## Phase 1 — Setup + +- [ ] T001 [P] Add `TelcoVMServiceAPICapabilityName = "supports_telco_vm_service_api"` to `test/e2e/vmservice/consts/consts.go` +- [ ] T002 [P] Add `default/wait-vm-extraconfig-synced` (`5m`/`10s`) interval to `test/e2e/vmservice/config/wcp.yaml`; power-state waits reuse the existing `default/wait-virtual-machine-powerstate` key + +## Phase 2 — Foundational + +- [ ] T003 Scaffold `VMExtraConfigSpecInput`/`VMExtraConfigSpec` and the `BeforeEach` (capability skip, namespace/class/image lookup, pod-log watch) in `test/e2e/vmservice/vmservice/virtualmachine/vm_extraconfig.go` +- [ ] T004 [P] Add shared helpers in the same file: `buildExtraConfigVM`, `waitForExtraConfigSynced`, `statusExtraConfigValue`, `statusExtraConfigKeys`, `getExtraConfigVM`, `deleteExtraConfigVM` +- [ ] T005 [P] Add out-of-band vSphere helpers in the same file: `waitForBiosUUID`, `findVSphereVMByBiosUUID` (used by Phase 4's out-of-band scenarios) +- [ ] T006 Register `Context("VM-EXTRACONFIG", ...)` calling `virtualmachine.VMExtraConfigSpec` from `test/e2e/vmservice/vmservice_test.go` + +## Phase 3 — User Story: applying first-class VMX fields and bag keys, reflected in status + +- [ ] T007 [US1] Add scenario: create VM with PowerCycle-mode first-class fields + two bag keys, wait for `ExtraConfigSynced=True`, assert both are visible in `status.extraConfig`, then patch bag keys (add/update/omit) and assert the change lands in `status.extraConfig` (`vm_extraconfig.go`) +- [ ] T008 [P] [US1] Add scenario: create VM with default `PromoteDisksMode=Online` plus first-class fields and a bag key, assert `ExtraConfigSynced=True` and all keys present despite concurrent disk promotion (`vm_extraconfig.go`) + +## Phase 4 — User Story: PowerCycle-mode vs PowerOff-mode semantics and condition precedence + +- [ ] T009 [US2] Add scenario: flip a PowerCycle-mode field on a running VM, assert `ExtraConfigSynced=False/PowerCyclePending`, power-cycle the VM, assert `True` and the new value in `status.extraConfig` (`vm_extraconfig.go`) +- [ ] T010 [P] [US2] Add scenario: add a PowerOff-mode field to a running VM, assert `ExtraConfigSynced=False/PowerOffRequired` naming the field, assert the key is absent from `status.extraConfig` while deferred, power off, assert `True` and every key present (`vm_extraconfig.go`) +- [ ] T011 [P] [US2] Add scenario: change a PowerOff-mode and a PowerCycle-mode field simultaneously on a running VM, assert `ExtraConfigSynced=False/PowerOffRequired` (not `PowerCyclePending`) and the message names the PowerOff-mode field (`vm_extraconfig.go`) +- [ ] T012 [P] [US2] Add scenario: create VM with `pnumaNodeAffinity` set, assert the comma-separated encoding in `status.extraConfig`; clear the field on a running VM, assert `PowerCyclePending`; power off, assert the key is removed from `status.extraConfig` (`vm_extraconfig.go`) + +## Phase 5 — User Story: resilience to VM recreation and out-of-band vSphere drift + +- [ ] T013 [US3] Add scenario: power off, delete the VM via the Kubernetes API, recreate with an identical spec, assert `ExtraConfigSynced=True` and every first-class/bag key re-applied (`vm_extraconfig.go`) +- [ ] T014 [P] [US3] Add scenario: create a `PowerCyclePending` condition, power the VM off out-of-band via `govmomi`, assert the pending change applies and `ExtraConfigSynced=True`, assert the operator restores `PoweredOn` afterward with extraConfig intact (`vm_extraconfig.go`) +- [ ] T015 [P] [US3] Add scenario: power off cleanly, destroy the vSphere VM out-of-band via `govmomi` (Kubernetes object survives), set `spec.powerState=PoweredOn`, assert `status.uniqueID` changes (proving recreation), assert `VirtualMachineCreated=True` and `ExtraConfigSynced=True` with every key re-applied (`vm_extraconfig.go`) + +## Phase Final — Polish + +- [ ] T016 Run every scenario in this file against a real Supervisor at least once; drop the `"experimental"` label per `e2e-testing.md` only after that run passes +- [ ] T017 File a follow-up spec/task for E2E coverage of `pkg/vmconfig/networkextraconfig` (see `e2e.md` "Scope boundaries") — not part of this task list's scope diff --git a/api/v1alpha6/virtualmachine_types.go b/api/v1alpha6/virtualmachine_types.go index 0db767ec7..712e7f1a0 100644 --- a/api/v1alpha6/virtualmachine_types.go +++ b/api/v1alpha6/virtualmachine_types.go @@ -196,6 +196,10 @@ const ( // VirtualMachineNetworkErrorReason indicates that reconciliation of // VirtualMachineNetworkConfigSynced failed with an unexpected error. VirtualMachineNetworkErrorReason = "Error" + + // VirtualMachineExtraConfigErrorReason indicates that reconciliation of + // VirtualMachineExtraConfigSynced failed with an unexpected error. + VirtualMachineExtraConfigErrorReason = "ExtraConfigError" ) const ( diff --git a/docs/concepts/workloads/vm.md b/docs/concepts/workloads/vm.md index 61dff6334..aec2149be 100644 --- a/docs/concepts/workloads/vm.md +++ b/docs/concepts/workloads/vm.md @@ -910,26 +910,48 @@ The above data does _not_ represent the _observed_ network configuration of the ## Advanced VM settings -`spec.advanced` groups optional VM-wide settings that are not covered by the VM class, image, or basic hardware blocks. Under the default VM Service configuration, these fields are available on the `VirtualMachine` CRD. Use per-interface `spec.network.interfaces[]` fields (and `advancedProperties` there) for adapter-specific VMX keys; use `spec.advanced.extraConfig` only for VM-wide keys that do not have a first-class field below. +VM advanced settings can be configured using optional fields under `spec.advanced`. For adapter-specific VMX keys, use the per-interface `spec.network.interfaces[]` fields along with their respective `advancedProperties`. The `spec.advanced.extraConfig` field should be reserved exclusively for VM-wide keys that lack a first-class field below. -| Field | Description | -|-------|-------------| -| `bootDiskCapacity` | Desired capacity of the boot disk from the VM image. Ignored when deploying from an ISO with CD-ROM devices. Resizing has guest and risk implications. | -| `defaultVolumeProvisioningMode` | Default provisioning mode for PVCs owned by this VM. | -| `changeBlockTracking` | Enables change block tracking for backup integrations. | -| `preferHtEnabled` | Prefer scheduling vCPUs on hyperthreads of the same core for locality. | -| `hugePages1GEnabled` | Use 1 GiB huge pages for VM memory. Cannot be changed while the VM is powered on. | -| `timeTrackerLowLatencyEnabled` | Low-latency time tracking; often used with high latency sensitivity. | -| `cpuAffinityExclusiveNoStatsEnabled` | Disable per-VM CPU accounting statistics; often used with high latency sensitivity. | -| `vmxSwapEnabled` | Allow or disallow VMX process swap; set `false` to reduce swap-related jitter. | -| `pnumaNodeAffinity` | Pin the VM to listed physical host NUMA node IDs (distinct from per-NIC `vnumaNodeID`). | -| `extraConfig` | Fallback list of VM-wide VMX key/value pairs not modeled above. Keys that duplicate a first-class field or reserved prefixes are rejected. Per-adapter keys belong under `spec.network.interfaces[]`. | - -`status.extraConfig` lists the effective VM-wide VMX map the operator is managing for observation. Conditions such as `VirtualMachineExtraConfigSynced` and `VirtualMachineNetworkConfigSynced` report whether VM-wide and per-interface advanced settings have been applied. +| Field | Description | Mode | +|-------|-------------|:----:| +| `bootDiskCapacity` | Desired capacity of the boot disk from the VM image. Ignored when deploying from an ISO with CD-ROM devices. Resizing has guest and risk implications. | _NA_ | +| `defaultVolumeProvisioningMode` | Default provisioning mode for PVCs owned by this VM. | _NA_ | +| `changeBlockTracking` | Enables change block tracking for backup integrations. | _NA_ | +| `preferHtEnabled` | Prefer scheduling vCPUs on hyperthreads of the same core for locality. | PowerCycle | +| `hugePages1GEnabled` | Use 1 GiB huge pages for VM memory. | PowerOff | +| `timeTrackerLowLatencyEnabled` | Low-latency time tracking; often used with high latency sensitivity. | PowerCycle | +| `cpuAffinityExclusiveNoStatsEnabled` | Disable per-VM CPU accounting statistics; often used with high latency sensitivity. | PowerCycle | +| `vmxSwapEnabled` | Allow or disallow VMX process swap; set `false` to reduce swap-related jitter. | PowerCycle | +| `pnumaNodeAffinity` | Pin the VM to listed physical host NUMA node IDs (distinct from per-NIC `vnumaNodeID`). | PowerCycle | +| `extraConfig` | Fallback list of VM-wide VMX key/value pairs not modeled above. Keys that duplicate a first-class field, or that match a reserved key or the `vmservice.`/`guestinfo.` prefixes, are rejected by the admission webhook. | _NA_* | -## Compute Reconfiguration +The `Mode` column reflects how a change to that field is applied when the VM is already powered on: + +* **PowerCycle** — the new value is written immediately, but the change is not considered fully synced until the VM is next power-cycled (powered off, then on). +* **PowerOff** — the change cannot be applied while the VM is powered on; it is deferred until the VM is powered off, at which point it is applied. +* **_NA_** — not subject to this mode; applies without a power transition. + +`*` `extraConfig` bag keys are always written immediately, regardless of power state, and are never deferred. VM Operator has no semantic knowledge of an arbitrary VMX key, so it cannot know whether that key actually requires a power cycle to take effect on the running VM — determining that, and power-cycling the VM if needed, is the user's responsibility. + +Removing a key from `spec.advanced.extraConfig` removes it from the VM's VMX and from `status.extraConfig` — it is not left stale. + +### ExtraConfigSynced condition + +`status.extraConfig` reflects the effective VMX extraConfig currently applied to the VM, merged from the VM Image, VM Class, `spec.network.interfaces[]` advanced properties, and `spec.advanced.extraConfig`. -> **Capability required:** `TelcoVMServiceAPI`. Fields in this section are absent from the CRD schema when the capability is not activated on the Supervisor. +The `VirtualMachineExtraConfigSynced` condition (and the analogous `VirtualMachineNetworkConfigSynced` for per-interface settings) reports whether these VM-wide and per-interface advanced settings have been applied. When `status: False`, the `reason` field explains why: + +| Reason | Description | +|--------|-------------| +| `PowerCyclePending` | A `PowerCycle`-mode first-class field changed while the VM was powered on. The value applies immediately, but a power-cycle is required to fully converge. | +| `PowerOffRequired` | A `PowerOff`-mode first-class field changed while the VM was powered on. The change is deferred, and the key is absent from `status.extraConfig`, until the VM is powered off. | +| `ExtraConfigError` | The reconciler failed to apply the desired configuration. | + +If a `PowerOff`-mode field and a `PowerCycle`-mode field both change in the same update, `PowerOffRequired` takes priority and is reported instead of `PowerCyclePending`. + +These reasons are only ever produced by first-class fields. A `spec.advanced.extraConfig` bag key change is applied immediately and the condition reports `True` right away, even if that particular key actually requires a power cycle to take effect — VM Operator does not track this for arbitrary keys. + +## Compute Reconfiguration `spec.resources`, `spec.cpuAdvanced`, and `spec.memoryAdvanced` enable field-level override of compute settings for Telco VNF workloads requiring deterministic scheduling or hardware-pinned resources. diff --git a/pkg/vmconfig/extraconfig/reconciler.go b/pkg/vmconfig/extraconfig/reconciler.go index 68d314f07..1750f796f 100644 --- a/pkg/vmconfig/extraconfig/reconciler.go +++ b/pkg/vmconfig/extraconfig/reconciler.go @@ -19,8 +19,8 @@ import ( vmopv1common "github.com/vmware-tanzu/vm-operator/api/v1alpha6/common" "github.com/vmware-tanzu/vm-operator/pkg/conditions" pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config" - pkgerr "github.com/vmware-tanzu/vm-operator/pkg/errors" ctxgen "github.com/vmware-tanzu/vm-operator/pkg/context/generic" + pkgerr "github.com/vmware-tanzu/vm-operator/pkg/errors" vsphereconst "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants" "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/virtualmachine/extraconfig" pkgutil "github.com/vmware-tanzu/vm-operator/pkg/util" @@ -28,13 +28,6 @@ import ( "github.com/vmware-tanzu/vm-operator/pkg/vmconfig" ) -// Reason constants for VirtualMachineConditionExtraConfigSynced. -const ( - ReasonPowerOffRequired = "PowerOffRequired" - ReasonPowerCyclePending = "PowerCyclePending" - ReasonError = "Error" -) - type contextKeyType uint8 const contextKeyValue contextKeyType = 0 @@ -253,7 +246,7 @@ func (r reconciler) OnResult( conditions.MarkFalse( vm, vmopv1.VirtualMachineExtraConfigSynced, - ReasonError, + vmopv1.VirtualMachineExtraConfigErrorReason, "%v", resultErr) return nil } @@ -294,13 +287,13 @@ func (r reconciler) OnResult( conditions.MarkFalse( vm, vmopv1.VirtualMachineExtraConfigSynced, - ReasonPowerOffRequired, + vmopv1.VirtualMachinePowerOffRequiredReason, "VM power off required to apply: %s", strings.Join(s.Deferred, ", ")) case s.PowerCyclePending || powerCycleOnVM: conditions.MarkFalse( vm, vmopv1.VirtualMachineExtraConfigSynced, - ReasonPowerCyclePending, + vmopv1.VirtualMachinePowerCyclePendingReason, "applied changes take effect on next power cycle") default: conditions.MarkTrue(vm, vmopv1.VirtualMachineExtraConfigSynced) diff --git a/pkg/vmconfig/extraconfig/reconciler_test.go b/pkg/vmconfig/extraconfig/reconciler_test.go index 66eff84c6..40f46a7f6 100644 --- a/pkg/vmconfig/extraconfig/reconciler_test.go +++ b/pkg/vmconfig/extraconfig/reconciler_test.go @@ -247,6 +247,26 @@ var _ = Describe("Reconcile", func() { Expect(r.Reconcile(ctx, nil, nil, vm, moVM, configSpec)).To(Succeed()) Expect(getECVal(configSpec.ExtraConfig, "numa.vcpu.preferHT")).To(Equal("TRUE")) }) + + It("injects vmx.reboot.powerCycle when PNUMANodeAffinity changes on powered-on VM", func() { + vm.Status.PowerState = vmopv1.VirtualMachinePowerStateOn + vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{PNUMANodeAffinity: []int32{0}} + moVM = moVMWithEC("numa.nodeAffinity", "1,2") + Expect(r.Reconcile(ctx, nil, nil, vm, moVM, configSpec)).To(Succeed()) + Expect(getECVal(configSpec.ExtraConfig, "numa.nodeAffinity")).To(Equal("0")) + Expect(getECVal(configSpec.ExtraConfig, + vsphereconst.ExtraConfigReservedKeyVMXRebootPowerCycle)).To(Equal("TRUE")) + }) + + It("defers clearing a PowerOff-mode key when VM is powered on", func() { + vm.Status.PowerState = vmopv1.VirtualMachinePowerStateOn + // Spec sets HugePages to nil (clear intent), observed still has the key. + vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{HugePages1GEnabled: nil} + moVM = moVMWithEC("sched.mem.lpage.enable1GPage", "TRUE") + Expect(r.Reconcile(ctx, nil, nil, vm, moVM, configSpec)).To(Succeed()) + // The clear is deferred — no entries emitted. + Expect(configSpec.ExtraConfig).To(BeEmpty()) + }) }) var _ = Describe("OnResult", func() { @@ -292,11 +312,17 @@ var _ = Describe("OnResult", func() { cond := findCond(vm) Expect(cond).ToNot(BeNil()) Expect(cond.Status).To(Equal(metav1.ConditionFalse)) - Expect(cond.Reason).To(Equal(vmconfextraconfig.ReasonError)) + Expect(cond.Reason).To(Equal(vmopv1.VirtualMachineExtraConfigErrorReason)) }) - It("does not set Error condition on NoRequeueNoErr sentinel", func() { - Expect(r.OnResult(ctx, vm, moVM, pkgerr.NoRequeueNoErr("created vm"))).To(Succeed()) + It("does not set Error condition on NoRequeueNoError", func() { + // NoRequeueNoError is a sentinel used for non-error stop conditions (e.g. + // snapshot revert, VM just created). OnResult must not mark ExtraConfigSynced + // false in these cases; it should proceed to evaluate the condition normally. + vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{PreferHTEnabled: ptr.To(true)} + moVM = moVMWithEC("numa.vcpu.preferHT", "TRUE") + Expect(r.Reconcile(ctx, nil, nil, vm, moVM, configSpec)).To(Succeed()) + Expect(r.OnResult(ctx, vm, moVM, pkgerr.NoRequeueNoErr("updated vm"))).To(Succeed()) cond := findCond(vm) Expect(cond).ToNot(BeNil()) Expect(cond.Status).To(Equal(metav1.ConditionTrue)) @@ -326,7 +352,7 @@ var _ = Describe("OnResult", func() { cond := findCond(vm) Expect(cond).ToNot(BeNil()) Expect(cond.Status).To(Equal(metav1.ConditionFalse)) - Expect(cond.Reason).To(Equal(vmconfextraconfig.ReasonPowerOffRequired)) + Expect(cond.Reason).To(Equal(vmopv1.VirtualMachinePowerOffRequiredReason)) Expect(cond.Message).To(ContainSubstring("sched.mem.lpage.enable1GPage")) }) @@ -337,7 +363,7 @@ var _ = Describe("OnResult", func() { reconcileAndOnResult(nil) cond := findCond(vm) Expect(cond).ToNot(BeNil()) - Expect(cond.Reason).To(Equal(vmconfextraconfig.ReasonPowerCyclePending)) + Expect(cond.Reason).To(Equal(vmopv1.VirtualMachinePowerCyclePendingReason)) }) It("persists PowerCyclePending when vmx.reboot.powerCycle already on powered-on VM", func() { @@ -350,7 +376,7 @@ var _ = Describe("OnResult", func() { reconcileAndOnResult(nil) cond := findCond(vm) Expect(cond).ToNot(BeNil()) - Expect(cond.Reason).To(Equal(vmconfextraconfig.ReasonPowerCyclePending)) + Expect(cond.Reason).To(Equal(vmopv1.VirtualMachinePowerCyclePendingReason)) }) It("sets True when vmx.reboot.powerCycle is on VM but VM is powered off", func() { @@ -380,8 +406,8 @@ var _ = Describe("OnResult", func() { It("status.ExtraConfig is sorted by key when multiple first-class keys are present", func() { vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{ - PreferHTEnabled: ptr.To(true), - TimeTrackerLowLatencyEnabled: ptr.To(true), + PreferHTEnabled: ptr.To(true), + TimeTrackerLowLatencyEnabled: ptr.To(true), CPUAffinityExclusiveNoStatsEnabled: ptr.To(true), } moVM = moVMWithEC( @@ -397,6 +423,55 @@ var _ = Describe("OnResult", func() { })) }) + It("sets PowerCyclePending when PNUMANodeAffinity changes on powered-on VM", func() { + vm.Status.PowerState = vmopv1.VirtualMachinePowerStateOn + vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{PNUMANodeAffinity: []int32{0}} + moVM = moVMWithEC("numa.nodeAffinity", "1,2") + reconcileAndOnResult(nil) + cond := findCond(vm) + Expect(cond).ToNot(BeNil()) + Expect(cond.Reason).To(Equal(vmopv1.VirtualMachinePowerCyclePendingReason)) + }) + + It("sets PowerOffRequired when clearing a PowerOff-mode key on powered-on VM", func() { + vm.Status.PowerState = vmopv1.VirtualMachinePowerStateOn + vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{HugePages1GEnabled: nil} + moVM = moVMWithEC("sched.mem.lpage.enable1GPage", "TRUE") + reconcileAndOnResult(nil) + cond := findCond(vm) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(vmopv1.VirtualMachinePowerOffRequiredReason)) + Expect(cond.Message).To(ContainSubstring("sched.mem.lpage.enable1GPage")) + }) + + It("leaves status.ExtraConfig unchanged when moVM.Config is nil", func() { + existing := []vmopv1common.KeyValuePair{{Key: "numa.vcpu.preferHT", Value: "TRUE"}} + vm.Status.ExtraConfig = existing + vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{PreferHTEnabled: ptr.To(true)} + moVM.Config = nil + reconcileAndOnResult(nil) + // OnResult must not overwrite status.ExtraConfig when moVM.Config is nil. + Expect(vm.Status.ExtraConfig).To(Equal(existing)) + }) + + It("PowerOffRequired takes priority over PowerCyclePending when both are pending", func() { + vm.Status.PowerState = vmopv1.VirtualMachinePowerStateOn + vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{ + HugePages1GEnabled: ptr.To(true), // poweroff → deferred + VMXSwapEnabled: ptr.To(false), // powercycle → applied + } + moVM = moVMWithEC( + "sched.mem.lpage.enable1GPage", "FALSE", // HugePages: differs from spec + "sched.swap.vmxSwapEnabled", "TRUE", // VMXSwap: differs from spec + ) + reconcileAndOnResult(nil) + cond := findCond(vm) + Expect(cond).ToNot(BeNil()) + Expect(cond.Reason).To(Equal(vmopv1.VirtualMachinePowerOffRequiredReason)) + Expect(cond.Message).To(ContainSubstring("sched.mem.lpage.enable1GPage")) + }) + It("populates status.ExtraConfig with only user-controlled keys", func() { vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{ PreferHTEnabled: ptr.To(true), diff --git a/test/e2e/vmservice/config/wcp.yaml b/test/e2e/vmservice/config/wcp.yaml index 3d7c748ac..d1ea27c8a 100644 --- a/test/e2e/vmservice/config/wcp.yaml +++ b/test/e2e/vmservice/config/wcp.yaml @@ -118,4 +118,5 @@ intervals: default/wait-security-policy-creation: ["2m", "10s"] default/wait-pvc-attachment: ["5m", "10s"] default/wait-pvc-deletion: ["2m", "5s"] - default/wait-jumpbox-sshpass-ready: ["30m", "10s"] \ No newline at end of file + default/wait-jumpbox-sshpass-ready: ["30m", "10s"] + default/wait-vm-extraconfig-synced: ["5m", "10s"] \ No newline at end of file diff --git a/test/e2e/vmservice/consts/consts.go b/test/e2e/vmservice/consts/consts.go index ba12ea4e3..79fb913f3 100644 --- a/test/e2e/vmservice/consts/consts.go +++ b/test/e2e/vmservice/consts/consts.go @@ -38,4 +38,5 @@ const ( MultiWriterDiskVMotionCapabilityName = "supports_multiwriter_disk_vmotion" IaaSComputePoliciesCapabilityName = "supports_iaas_compute_policies" VirtualMachineConfigPolicyCapabilityName = "supports_vm_service_vm_config_policy" + TelcoVMServiceAPICapabilityName = "supports_telco_vm_service_api" ) diff --git a/test/e2e/vmservice/vmservice/virtualmachine/vm_extraconfig.go b/test/e2e/vmservice/vmservice/virtualmachine/vm_extraconfig.go new file mode 100644 index 000000000..e94f39781 --- /dev/null +++ b/test/e2e/vmservice/vmservice/virtualmachine/vm_extraconfig.go @@ -0,0 +1,873 @@ +// © Broadcom. All Rights Reserved. +// The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries. +// SPDX-License-Identifier: Apache-2.0 + +package virtualmachine + +import ( + "context" + "fmt" + "os" + "path/filepath" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/vmware/govmomi/object" + "github.com/vmware/govmomi/vim25" + vimtypes "github.com/vmware/govmomi/vim25/types" + e2eframework "k8s.io/kubernetes/test/e2e/framework" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + capiutil "sigs.k8s.io/cluster-api/util" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha6" + vmopv1common "github.com/vmware-tanzu/vm-operator/api/v1alpha6/common" + "github.com/vmware-tanzu/vm-operator/pkg/util/ptr" + + "github.com/vmware-tanzu/vm-operator/test/e2e/framework" + "github.com/vmware-tanzu/vm-operator/test/e2e/infrastructure/vsphere/vcenter" + e2eConfig "github.com/vmware-tanzu/vm-operator/test/e2e/vmservice/config" + "github.com/vmware-tanzu/vm-operator/test/e2e/vmservice/common" + "github.com/vmware-tanzu/vm-operator/test/e2e/vmservice/consts" + "github.com/vmware-tanzu/vm-operator/test/e2e/vmservice/lib/vmoperator" + "github.com/vmware-tanzu/vm-operator/test/e2e/vmservice/skipper" + "github.com/vmware-tanzu/vm-operator/test/e2e/vmservice/vmservice" + "github.com/vmware-tanzu/vm-operator/test/e2e/wcpframework" +) + +// VMX key names for first-class advanced fields (mirrors the vmx struct tags). +const ( + vmxPreferHT = "numa.vcpu.preferHT" + vmxHugePages = "sched.mem.lpage.enable1GPage" + vmxTimeTracker = "timeTracker.lowLatency" + vmxCPUAffinity = "sched.cpu.affinity.exclusiveNoStats" + vmxVMXSwap = "sched.swap.vmxSwapEnabled" + vmxPNUMANodeAffinity = "numa.nodeAffinity" +) + +// Condition reason values for VirtualMachineExtraConfigSynced (mirrors reconciler constants). +const ( + extraConfigReasonPowerOffRequired = "PowerOffRequired" + extraConfigReasonPowerCyclePending = "PowerCyclePending" +) + +// VMExtraConfigSpecInput is the input for the ExtraConfig test spec. +type VMExtraConfigSpecInput struct { + ClusterProxy wcpframework.WCPClusterProxyInterface + Config *e2eConfig.E2EConfig + ArtifactFolder string + SkipCleanup bool + WCPNamespaceName string +} + +// VMExtraConfigSpec validates the ExtraConfig reconciler end-to-end on a live WCP cluster. +// +// Each It block creates its own VM and cleans up after itself, so all blocks are +// independently runnable via TEST_FOCUS or LABEL_FILTER. +// +// All blocks are skipped when the TelcoVMServiceAPI supervisor capability +// (supports_telco_vm_service_api) is not enabled on the cluster. +func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpecInput) { + const specName = "vm-extraconfig" + + var ( + input VMExtraConfigSpecInput + config *e2eConfig.E2EConfig + clusterProxy *common.VMServiceClusterProxy + svClusterClient ctrlclient.Client + vCenterClient *vim25.Client + vmClassName string + storageClass string + linuxVMIName string + ) + + BeforeEach(func() { + input = inputGetter() + + Expect(input.Config).NotTo(BeNil(), + "Invalid argument. input.Config can't be nil when calling %s spec", specName) + Expect(input.Config.InfraConfig).NotTo(BeNil(), + "Invalid argument. input.Config.InfraConfig can't be nil when calling %s spec", specName) + Expect(input.ClusterProxy).NotTo(BeNil(), + "Invalid argument. input.ClusterProxy can't be nil when calling %s spec", specName) + Expect(input.WCPNamespaceName).NotTo(BeEmpty(), + "Invalid argument. input.WCPNamespaceName can't be empty when calling %s spec", specName) + Expect(os.MkdirAll(input.ArtifactFolder, 0o755)).To(Succeed(), + "Invalid argument. input.ArtifactFolder can't be created for %s spec", specName) + + skipper.SkipUnlessInfraIs(input.Config.InfraConfig.InfraName, consts.WCP) + skipper.SkipUnlessSupervisorCapabilityEnabled( + ctx, + input.ClusterProxy.(*common.VMServiceClusterProxy), + consts.TelcoVMServiceAPICapabilityName, + ) + + config = input.Config + clusterProxy = input.ClusterProxy.(*common.VMServiceClusterProxy) + svClusterClient = clusterProxy.GetClient() + + cancelPodWatches := framework.WatchPodLogsAndEventsInNamespaces( + ctx, + []string{config.GetVariable("VMOPNamespace")}, + clusterProxy.GetClientSet(), + filepath.Join(input.ArtifactFolder, specName), + ) + DeferCleanup(cancelPodWatches) + + vCenterClient = vcenter.NewVimClientFromKubeconfig(ctx, clusterProxy.GetKubeconfigPath()) + DeferCleanup(func() { + vcenter.LogoutVimClient(vCenterClient) + }) + + clusterResources := config.InfraConfig.ManagementClusterConfig.Resources + vmClassName = clusterResources.VMClassName + storageClass = clusterResources.StorageClassName + + linuxImageDisplayName := vmservice.GetDefaultImageDisplayName(clusterResources) + var err error + linuxVMIName, err = vmoperator.WaitForVirtualMachineImageName( + ctx, &config.Config, svClusterClient, + input.WCPNamespaceName, linuxImageDisplayName) + Expect(err).NotTo(HaveOccurred(), + "failed to get VMI name for display name %q in namespace %q", + linuxImageDisplayName, input.WCPNamespaceName) + }) + + // ── It block 1: phases 1-2 ──────────────────────────────────────────────── + // Creates a VM with PowerCycle first-class fields and two bag keys, waits for + // ExtraConfigSynced=True, then exercises bag key CRUD and verifies status.extraConfig + // reflects the changes. + It("creates VM with first-class fields and bag keys, syncs immediately, reflects bag key CRUD", + Label("extraconfig", "core-functional"), func() { + + vmName := fmt.Sprintf("%s-core-%s", specName, capiutil.RandomString(4)) + vmKey := types.NamespacedName{Name: vmName, Namespace: input.WCPNamespaceName} + + By("Creating VM with PowerCycle first-class fields and two bag keys") + vm := buildExtraConfigVM( + vmName, input.WCPNamespaceName, vmClassName, linuxVMIName, storageClass, + &vmopv1.VirtualMachineAdvancedSpec{ + PreferHTEnabled: ptr.To(true), + TimeTrackerLowLatencyEnabled: ptr.To(true), + CPUAffinityExclusiveNoStatsEnabled: ptr.To(false), + VMXSwapEnabled: ptr.To(true), + ExtraConfig: []vmopv1common.KeyValuePair{ + {Key: "custom.test.foo", Value: "bar"}, + {Key: "custom.test.baz", Value: "qux"}, + }, + }) + Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to create VM %s", vmName) + DeferCleanup(func() { + if !input.SkipCleanup { + deleteExtraConfigVM(ctx, svClusterClient, config, vmKey) + } + }) + + By("Waiting for VM to be created in vSphere") + vmoperator.WaitForVirtualMachineConditionCreated( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName) + + By("Waiting for ExtraConfigSynced=True") + waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, metav1.ConditionTrue, "") + + By("Asserting first-class VMX keys are present in status.extraConfig") + vm = getExtraConfigVM(ctx, svClusterClient, vmKey) + Expect(statusExtraConfigValue(vm, vmxPreferHT)).To(Equal("TRUE"), + "expected %s=TRUE", vmxPreferHT) + Expect(statusExtraConfigValue(vm, vmxTimeTracker)).To(Equal("TRUE"), + "expected %s=TRUE", vmxTimeTracker) + Expect(statusExtraConfigValue(vm, vmxCPUAffinity)).To(Equal("FALSE"), + "expected %s=FALSE", vmxCPUAffinity) + Expect(statusExtraConfigValue(vm, vmxVMXSwap)).To(Equal("TRUE"), + "expected %s=TRUE", vmxVMXSwap) + + By("Asserting bag keys are visible in status.extraConfig") + Expect(statusExtraConfigKeys(vm)).To(ContainElements("custom.test.foo", "custom.test.baz")) + Expect(statusExtraConfigValue(vm, "custom.test.foo")).To(Equal("bar")) + + // Phase 2: bag key CRUD. + By("Patching spec: add new bag key, update existing value, omit one to trigger deletion") + Eventually(func(g Gomega) { + g.Expect(svClusterClient.Get(ctx, vmKey, vm)).To(Succeed()) + vm.Spec.Advanced.ExtraConfig = []vmopv1common.KeyValuePair{ + {Key: "custom.test.foo", Value: "updated"}, + {Key: "custom.test.new", Value: "newval"}, + } + g.Expect(svClusterClient.Update(ctx, vm)).To(Succeed()) + }, config.GetIntervals("default", "wait-vm-extraconfig-synced")...).Should(Succeed(), + "failed to patch VM %s bag keys", vmName) + + By("Waiting for status.extraConfig to reflect the bag key changes") + Eventually(func(g Gomega) { + g.Expect(svClusterClient.Get(ctx, vmKey, vm)).To(Succeed()) + g.Expect(statusExtraConfigValue(vm, "custom.test.foo")).To(Equal("updated")) + g.Expect(statusExtraConfigValue(vm, "custom.test.new")).To(Equal("newval")) + g.Expect(statusExtraConfigKeys(vm)).NotTo(ContainElement("custom.test.baz")) + }, config.GetIntervals("default", "wait-vm-extraconfig-synced")...).Should(Succeed(), + "timed out waiting for bag key CRUD to be reflected in status.extraConfig") + }) + + // ── It block 2: phase 3 ─────────────────────────────────────────────────── + // Flips a PowerCycle-mode field on a running VM, verifies PowerCyclePending, + // then power-cycles to apply and verifies the new value. + It("marks PowerCyclePending when a PowerCycle-mode field changes while VM is powered on", + Label("extraconfig", "core-functional"), func() { + + vmName := fmt.Sprintf("%s-pc-%s", specName, capiutil.RandomString(4)) + vmKey := types.NamespacedName{Name: vmName, Namespace: input.WCPNamespaceName} + + By("Creating VM with PowerCycle first-class fields") + vm := buildExtraConfigVM( + vmName, input.WCPNamespaceName, vmClassName, linuxVMIName, storageClass, + &vmopv1.VirtualMachineAdvancedSpec{ + PreferHTEnabled: ptr.To(true), + TimeTrackerLowLatencyEnabled: ptr.To(true), + CPUAffinityExclusiveNoStatsEnabled: ptr.To(false), + VMXSwapEnabled: ptr.To(true), + }) + Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to create VM %s", vmName) + DeferCleanup(func() { + if !input.SkipCleanup { + deleteExtraConfigVM(ctx, svClusterClient, config, vmKey) + } + }) + + vmoperator.WaitForVirtualMachineConditionCreated( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName) + waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, metav1.ConditionTrue, "") + + By("Flipping VMXSwapEnabled=false (PowerCycle-mode field) while VM is powered on") + Eventually(func(g Gomega) { + g.Expect(svClusterClient.Get(ctx, vmKey, vm)).To(Succeed()) + vm.Spec.Advanced.VMXSwapEnabled = ptr.To(false) + g.Expect(svClusterClient.Update(ctx, vm)).To(Succeed()) + }, config.GetIntervals("default", "wait-vm-extraconfig-synced")...).Should(Succeed()) + + By("Waiting for ExtraConfigSynced=False/PowerCyclePending") + cond := waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, + metav1.ConditionFalse, extraConfigReasonPowerCyclePending) + Expect(cond).NotTo(BeNil()) + + By("Power-cycling VM via spec to apply the pending change") + vmoperator.UpdateVirtualMachinePowerState( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName, "PoweredOff") + vmoperator.WaitForVirtualMachinePowerState( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName, "PoweredOff") + + By("Waiting for ExtraConfigSynced=True once VM is powered off") + waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, metav1.ConditionTrue, "") + + By("Powering VM back on") + vmoperator.UpdateVirtualMachinePowerState( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName, "PoweredOn") + vmoperator.WaitForVirtualMachinePowerState( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName, "PoweredOn") + + By("Asserting VMXSwap VMX key reflects FALSE after the power cycle") + Eventually(func(g Gomega) { + g.Expect(svClusterClient.Get(ctx, vmKey, vm)).To(Succeed()) + g.Expect(statusExtraConfigValue(vm, vmxVMXSwap)).To(Equal("FALSE")) + }, config.GetIntervals("default", "wait-vm-extraconfig-synced")...).Should(Succeed()) + }) + + // ── It block 3: phases 4-5 ──────────────────────────────────────────────── + // Adds a PowerOff-mode field on a running VM, verifies PowerOffRequired and + // that the key is absent from status while deferred, then powers off and verifies + // all keys applied. + It("defers a PowerOff-mode field while VM is powered on, applies it after power-off", + Label("extraconfig", "core-functional"), func() { + + vmName := fmt.Sprintf("%s-po-%s", specName, capiutil.RandomString(4)) + vmKey := types.NamespacedName{Name: vmName, Namespace: input.WCPNamespaceName} + + By("Creating VM with first-class fields and one bag key") + vm := buildExtraConfigVM( + vmName, input.WCPNamespaceName, vmClassName, linuxVMIName, storageClass, + &vmopv1.VirtualMachineAdvancedSpec{ + PreferHTEnabled: ptr.To(true), + TimeTrackerLowLatencyEnabled: ptr.To(true), + CPUAffinityExclusiveNoStatsEnabled: ptr.To(false), + VMXSwapEnabled: ptr.To(true), + ExtraConfig: []vmopv1common.KeyValuePair{ + {Key: "custom.test.foo", Value: "bar"}, + }, + }) + Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to create VM %s", vmName) + DeferCleanup(func() { + if !input.SkipCleanup { + deleteExtraConfigVM(ctx, svClusterClient, config, vmKey) + } + }) + + vmoperator.WaitForVirtualMachineConditionCreated( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName) + waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, metav1.ConditionTrue, "") + + By("Adding HugePages1GEnabled=true (PowerOff-mode) while VM is powered on") + Eventually(func(g Gomega) { + g.Expect(svClusterClient.Get(ctx, vmKey, vm)).To(Succeed()) + vm.Spec.Advanced.HugePages1GEnabled = ptr.To(true) + g.Expect(svClusterClient.Update(ctx, vm)).To(Succeed()) + }, config.GetIntervals("default", "wait-vm-extraconfig-synced")...).Should(Succeed()) + + By("Waiting for ExtraConfigSynced=False/PowerOffRequired") + cond := waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, + metav1.ConditionFalse, extraConfigReasonPowerOffRequired) + Expect(cond).NotTo(BeNil()) + Expect(cond.Message).To(ContainSubstring(vmxHugePages), + "condition message should name the deferred VMX key") + + By("Asserting HugePages key is absent from status.extraConfig while deferred") + vm = getExtraConfigVM(ctx, svClusterClient, vmKey) + Expect(statusExtraConfigKeys(vm)).NotTo(ContainElement(vmxHugePages), + "deferred key %s should not appear in status.extraConfig while VM is on", vmxHugePages) + + By("Powering off VM via spec to apply the deferred PowerOff-mode key") + vmoperator.UpdateVirtualMachinePowerState( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName, "PoweredOff") + vmoperator.WaitForVirtualMachinePowerState( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName, "PoweredOff") + + By("Waiting for ExtraConfigSynced=True after power-off") + waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, metav1.ConditionTrue, "") + + By("Asserting all first-class keys and HugePages are present after power-off") + vm = getExtraConfigVM(ctx, svClusterClient, vmKey) + Expect(statusExtraConfigValue(vm, vmxPreferHT)).To(Equal("TRUE")) + Expect(statusExtraConfigValue(vm, vmxTimeTracker)).To(Equal("TRUE")) + Expect(statusExtraConfigValue(vm, vmxCPUAffinity)).To(Equal("FALSE")) + Expect(statusExtraConfigValue(vm, vmxVMXSwap)).To(Equal("TRUE")) + Expect(statusExtraConfigValue(vm, vmxHugePages)).To(Equal("TRUE")) + Expect(statusExtraConfigValue(vm, "custom.test.foo")).To(Equal("bar")) + }) + + // ── It block 4: phase 6 ─────────────────────────────────────────────────── + // Deletes a VM via the Kubernetes API and recreates it with an identical spec, + // verifying that the operator re-applies all extraConfig keys from scratch. + It("re-applies all extraConfig keys when VM is deleted via Kubernetes and recreated", + Label("extraconfig", "core-functional"), func() { + + vmName := fmt.Sprintf("%s-recreate-%s", specName, capiutil.RandomString(4)) + vmKey := types.NamespacedName{Name: vmName, Namespace: input.WCPNamespaceName} + + advanced := &vmopv1.VirtualMachineAdvancedSpec{ + PreferHTEnabled: ptr.To(true), + TimeTrackerLowLatencyEnabled: ptr.To(true), + CPUAffinityExclusiveNoStatsEnabled: ptr.To(false), + VMXSwapEnabled: ptr.To(true), + ExtraConfig: []vmopv1common.KeyValuePair{ + {Key: "custom.test.foo", Value: "bar"}, + {Key: "custom.test.baz", Value: "qux"}, + }, + } + + By("Creating VM with first-class fields and bag keys") + vm := buildExtraConfigVM( + vmName, input.WCPNamespaceName, vmClassName, linuxVMIName, storageClass, advanced) + Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to create VM %s", vmName) + + vmoperator.WaitForVirtualMachineConditionCreated( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName) + waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, metav1.ConditionTrue, "") + + By("Powering off VM via spec before deletion") + vmoperator.UpdateVirtualMachinePowerState( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName, "PoweredOff") + vmoperator.WaitForVirtualMachinePowerState( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName, "PoweredOff") + + By("Deleting VM via Kubernetes API") + deleteExtraConfigVM(ctx, svClusterClient, config, vmKey) + + By("Recreating VM with identical spec") + vm = buildExtraConfigVM( + vmName, input.WCPNamespaceName, vmClassName, linuxVMIName, storageClass, advanced) + Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to recreate VM %s", vmName) + DeferCleanup(func() { + if !input.SkipCleanup { + deleteExtraConfigVM(ctx, svClusterClient, config, vmKey) + } + }) + + By("Waiting for recreated VM to be created in vSphere") + vmoperator.WaitForVirtualMachineConditionCreated( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName) + + By("Waiting for ExtraConfigSynced=True on the recreated VM") + waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, metav1.ConditionTrue, "") + + By("Asserting first-class keys and bag keys are fully re-applied on the recreated VM") + vm = getExtraConfigVM(ctx, svClusterClient, vmKey) + Expect(statusExtraConfigValue(vm, vmxPreferHT)).To(Equal("TRUE")) + Expect(statusExtraConfigValue(vm, vmxTimeTracker)).To(Equal("TRUE")) + Expect(statusExtraConfigValue(vm, "custom.test.foo")).To(Equal("bar")) + Expect(statusExtraConfigValue(vm, "custom.test.baz")).To(Equal("qux")) + }) + + // ── It block 5: phase 7 ─────────────────────────────────────────────────── + // Creates a PowerCyclePending condition, then powers the VM off out-of-band + // through vSphere directly. Verifies that the operator detects the power-off, + // applies the pending change, and restores the VM to PoweredOn. + It("resolves PowerCyclePending after an out-of-band vSphere power-off", + Label("extraconfig", "extended-functional"), func() { + + vmName := fmt.Sprintf("%s-oob-off-%s", specName, capiutil.RandomString(4)) + vmKey := types.NamespacedName{Name: vmName, Namespace: input.WCPNamespaceName} + + By("Creating VM with first-class fields, all synced") + vm := buildExtraConfigVM( + vmName, input.WCPNamespaceName, vmClassName, linuxVMIName, storageClass, + &vmopv1.VirtualMachineAdvancedSpec{ + PreferHTEnabled: ptr.To(true), + TimeTrackerLowLatencyEnabled: ptr.To(true), + CPUAffinityExclusiveNoStatsEnabled: ptr.To(false), + VMXSwapEnabled: ptr.To(true), + }) + Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to create VM %s", vmName) + DeferCleanup(func() { + if !input.SkipCleanup { + deleteExtraConfigVM(ctx, svClusterClient, config, vmKey) + } + }) + + vmoperator.WaitForVirtualMachineConditionCreated( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName) + waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, metav1.ConditionTrue, "") + + By("Flipping PreferHTEnabled=false (PowerCycle-mode) to create PowerCyclePending") + Eventually(func(g Gomega) { + g.Expect(svClusterClient.Get(ctx, vmKey, vm)).To(Succeed()) + vm.Spec.Advanced.PreferHTEnabled = ptr.To(false) + g.Expect(svClusterClient.Update(ctx, vm)).To(Succeed()) + }, config.GetIntervals("default", "wait-vm-extraconfig-synced")...).Should(Succeed()) + + waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, + metav1.ConditionFalse, extraConfigReasonPowerCyclePending) + + By("Getting VM BiosUUID for vSphere lookup") + biosUUID := waitForBiosUUID(ctx, svClusterClient, config, vmKey) + + By("Powering off VM out-of-band via vSphere (bypassing the operator)") + vsphereVM := findVSphereVMByBiosUUID(ctx, vCenterClient, biosUUID) + Expect(vsphereVM).NotTo(BeNil(), "VM with BiosUUID %s not found in vSphere", biosUUID) + powerOffTask, err := vsphereVM.PowerOff(ctx) + Expect(err).NotTo(HaveOccurred(), "failed to start out-of-band power-off task") + Expect(powerOffTask.Wait(ctx)).To(Succeed(), "out-of-band power-off task failed") + + By("Waiting for ExtraConfigSynced=True (pending change applied while VM is off)") + waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, metav1.ConditionTrue, "") + + By("Asserting PreferHT key is updated to FALSE in status.extraConfig") + vm = getExtraConfigVM(ctx, svClusterClient, vmKey) + Expect(statusExtraConfigValue(vm, vmxPreferHT)).To(Equal("FALSE")) + + By("Waiting for operator to restore VM to PoweredOn (drift recovery)") + vmoperator.WaitForVirtualMachinePowerState( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName, "PoweredOn") + + By("Asserting extraConfig is intact after drift recovery") + vm = getExtraConfigVM(ctx, svClusterClient, vmKey) + Expect(statusExtraConfigValue(vm, vmxPreferHT)).To(Equal("FALSE")) + Expect(statusExtraConfigValue(vm, vmxTimeTracker)).To(Equal("TRUE")) + }) + + // ── It block 6: phase 8 ─────────────────────────────────────────────────── + // Destroys the VM in vSphere without touching the Kubernetes object. Verifies + // that the operator detects the orphaned K8s object, recreates the VM in vSphere, + // and re-applies all extraConfig keys from the spec. + It("re-applies extraConfig after the vSphere VM is destroyed out-of-band", + Label("extraconfig", "extended-functional"), func() { + + vmName := fmt.Sprintf("%s-oob-del-%s", specName, capiutil.RandomString(4)) + vmKey := types.NamespacedName{Name: vmName, Namespace: input.WCPNamespaceName} + + By("Creating VM with first-class fields and a bag key") + vm := buildExtraConfigVM( + vmName, input.WCPNamespaceName, vmClassName, linuxVMIName, storageClass, + &vmopv1.VirtualMachineAdvancedSpec{ + PreferHTEnabled: ptr.To(true), + TimeTrackerLowLatencyEnabled: ptr.To(true), + CPUAffinityExclusiveNoStatsEnabled: ptr.To(false), + VMXSwapEnabled: ptr.To(true), + ExtraConfig: []vmopv1common.KeyValuePair{ + {Key: "custom.test.foo", Value: "bar"}, + }, + }) + Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to create VM %s", vmName) + DeferCleanup(func() { + if !input.SkipCleanup { + deleteExtraConfigVM(ctx, svClusterClient, config, vmKey) + } + }) + + vmoperator.WaitForVirtualMachineConditionCreated( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName) + waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, metav1.ConditionTrue, "") + + By("Powering off VM cleanly via spec before destroy") + vmoperator.UpdateVirtualMachinePowerState( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName, "PoweredOff") + vmoperator.WaitForVirtualMachinePowerState( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName, "PoweredOff") + + By("Recording BiosUUID and UniqueID before the vSphere destroy") + originalBiosUUID := waitForBiosUUID(ctx, svClusterClient, config, vmKey) + Expect(svClusterClient.Get(ctx, vmKey, vm)).To(Succeed()) + originalUniqueID := vm.Status.UniqueID + + By("Confirming vSphere reports the VM as powered off before destroying") + vsphereVM := findVSphereVMByBiosUUID(ctx, vCenterClient, originalBiosUUID) + Expect(vsphereVM).NotTo(BeNil(), "VM with BiosUUID %s not found in vSphere", originalBiosUUID) + Eventually(func(g Gomega) { + powerState, err := vsphereVM.PowerState(ctx) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(powerState).To(Equal(vimtypes.VirtualMachinePowerStatePoweredOff)) + }, config.GetIntervals("default", "wait-virtual-machine-powerstate")...).Should(Succeed(), + "timed out waiting for vSphere to confirm VM %s is powered off", vmName) + + By("Destroying VM in vSphere out-of-band (Kubernetes object survives)") + destroyTask, err := vsphereVM.Destroy(ctx) + Expect(err).NotTo(HaveOccurred(), "failed to start vSphere destroy task") + Expect(destroyTask.Wait(ctx)).To(Succeed(), "vSphere VM destroy task failed") + + By("Setting spec.powerState=PoweredOn so the operator powers on the recreated VM") + Eventually(func(g Gomega) { + g.Expect(svClusterClient.Get(ctx, vmKey, vm)).To(Succeed()) + vm.Spec.PowerState = vmopv1.VirtualMachinePowerStateOn + g.Expect(svClusterClient.Update(ctx, vm)).To(Succeed()) + }, config.GetIntervals("default", "wait-vm-extraconfig-synced")...).Should(Succeed()) + + By("Waiting for operator to recreate the VM (UniqueID change proves a new vSphere VM)") + Eventually(func(g Gomega) { + g.Expect(svClusterClient.Get(ctx, vmKey, vm)).To(Succeed()) + g.Expect(vm.Status.UniqueID).NotTo(BeEmpty()) + g.Expect(vm.Status.UniqueID).NotTo(Equal(originalUniqueID)) + }, config.GetIntervals("default", "wait-virtual-machine-creation")...).Should(Succeed(), + "timed out waiting for UniqueID to change after vSphere destroy") + + By("Waiting for VirtualMachineCreated=True on the recreated VM") + vmoperator.WaitForVirtualMachineConditionCreated( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName) + + By("Waiting for ExtraConfigSynced=True on the recreated VM") + waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, metav1.ConditionTrue, "") + + By("Asserting extraConfig is fully re-applied from spec on the recreated VM") + vm = getExtraConfigVM(ctx, svClusterClient, vmKey) + Expect(statusExtraConfigValue(vm, vmxPreferHT)).To(Equal("TRUE")) + Expect(statusExtraConfigValue(vm, vmxTimeTracker)).To(Equal("TRUE")) + Expect(statusExtraConfigValue(vm, "custom.test.foo")).To(Equal("bar")) + }) + + // ── It block 7 ─────────────────────────────────────────────────────────── + // Verifies that the PNUMANodeAffinity field (the only []int32 first-class field) + // is encoded correctly as a comma-separated string, triggers PowerCyclePending + // when changed while the VM is running, and resolves after a power cycle. + It("handles PNUMANodeAffinity ([]int32 field): syncs, marks PowerCyclePending on change, resolves after power-off", + Label("extraconfig", "core-functional"), func() { + + vmName := fmt.Sprintf("%s-pnuma-%s", specName, capiutil.RandomString(4)) + vmKey := types.NamespacedName{Name: vmName, Namespace: input.WCPNamespaceName} + + By("Creating VM with PNUMANodeAffinity pinned to NUMA node 0") + vm := buildExtraConfigVM( + vmName, input.WCPNamespaceName, vmClassName, linuxVMIName, storageClass, + &vmopv1.VirtualMachineAdvancedSpec{ + PNUMANodeAffinity: []int32{0}, + }) + Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to create VM %s", vmName) + DeferCleanup(func() { + if !input.SkipCleanup { + deleteExtraConfigVM(ctx, svClusterClient, config, vmKey) + } + }) + + vmoperator.WaitForVirtualMachineConditionCreated( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName) + waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, metav1.ConditionTrue, "") + + By("Asserting numa.nodeAffinity=0 is visible in status.extraConfig") + vm = getExtraConfigVM(ctx, svClusterClient, vmKey) + Expect(statusExtraConfigValue(vm, vmxPNUMANodeAffinity)).To(Equal("0"), + "expected %s=0 in status", vmxPNUMANodeAffinity) + + By("Clearing PNUMANodeAffinity (nil) while VM is powered on — should mark PowerCyclePending") + Eventually(func(g Gomega) { + g.Expect(svClusterClient.Get(ctx, vmKey, vm)).To(Succeed()) + vm.Spec.Advanced.PNUMANodeAffinity = nil + g.Expect(svClusterClient.Update(ctx, vm)).To(Succeed()) + }, config.GetIntervals("default", "wait-vm-extraconfig-synced")...).Should(Succeed()) + + By("Waiting for ExtraConfigSynced=False/PowerCyclePending") + waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, + metav1.ConditionFalse, extraConfigReasonPowerCyclePending) + + By("Powering off VM to apply the pending clear") + vmoperator.UpdateVirtualMachinePowerState( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName, "PoweredOff") + vmoperator.WaitForVirtualMachinePowerState( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName, "PoweredOff") + + By("Waiting for ExtraConfigSynced=True after power-off") + waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, metav1.ConditionTrue, "") + + By("Asserting numa.nodeAffinity is no longer in status.extraConfig after the clear") + vm = getExtraConfigVM(ctx, svClusterClient, vmKey) + Expect(statusExtraConfigKeys(vm)).NotTo(ContainElement(vmxPNUMANodeAffinity), + "cleared key %s should not appear in status.extraConfig", vmxPNUMANodeAffinity) + }) + + // ── It block 8 ─────────────────────────────────────────────────────────── + // Creates a VM with the default PromoteDisksMode (Online) so that disk + // promotion runs concurrently with extraConfig application. Verifies that + // ExtraConfigSynced reaches True even while SvMotion may be in flight, + // catching ordering or resource-conflict bugs between the two reconcilers. + It("applies extraConfig correctly while disk promotion runs (default PromoteDisksMode=Online)", + Label("extraconfig", "core-functional"), func() { + + vmName := fmt.Sprintf("%s-promo-%s", specName, capiutil.RandomString(4)) + vmKey := types.NamespacedName{Name: vmName, Namespace: input.WCPNamespaceName} + + By("Creating VM with default PromoteDisksMode and first-class fields plus a bag key") + vm := &vmopv1.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: vmName, + Namespace: input.WCPNamespaceName, + Labels: map[string]string{ + "e2e.vmoperator.vmware.com/extraconfig-test": "true", + }, + }, + Spec: vmopv1.VirtualMachineSpec{ + ClassName: vmClassName, + ImageName: linuxVMIName, + StorageClass: storageClass, + PowerState: vmopv1.VirtualMachinePowerStateOn, + Bootstrap: &vmopv1.VirtualMachineBootstrapSpec{ + Disabled: true, + }, + Advanced: &vmopv1.VirtualMachineAdvancedSpec{ + PreferHTEnabled: ptr.To(true), + TimeTrackerLowLatencyEnabled: ptr.To(true), + ExtraConfig: []vmopv1common.KeyValuePair{ + {Key: "custom.test.foo", Value: "bar"}, + }, + }, + }, + } + Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to create VM %s", vmName) + DeferCleanup(func() { + if !input.SkipCleanup { + deleteExtraConfigVM(ctx, svClusterClient, config, vmKey) + } + }) + + By("Waiting for VM to be created in vSphere") + vmoperator.WaitForVirtualMachineConditionCreated( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName) + + By("Waiting for ExtraConfigSynced=True while disk promotion may be running concurrently") + waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, metav1.ConditionTrue, "") + + By("Asserting extraConfig keys are present despite concurrent disk promotion") + vm = getExtraConfigVM(ctx, svClusterClient, vmKey) + Expect(statusExtraConfigValue(vm, vmxPreferHT)).To(Equal("TRUE")) + Expect(statusExtraConfigValue(vm, vmxTimeTracker)).To(Equal("TRUE")) + Expect(statusExtraConfigValue(vm, "custom.test.foo")).To(Equal("bar")) + }) + + // ── It block 9 ─────────────────────────────────────────────────────────── + // When a PowerOff-mode key (HugePages) and a PowerCycle-mode key (VMXSwap) + // both change simultaneously on a running VM, PowerOffRequired must take + // priority over PowerCyclePending in the ExtraConfigSynced condition. + It("PowerOffRequired takes priority over PowerCyclePending when both key types change simultaneously", + Label("extraconfig", "core-functional"), func() { + + vmName := fmt.Sprintf("%s-prio-%s", specName, capiutil.RandomString(4)) + vmKey := types.NamespacedName{Name: vmName, Namespace: input.WCPNamespaceName} + + By("Creating VM with VMXSwapEnabled=true, no HugePages") + vm := buildExtraConfigVM( + vmName, input.WCPNamespaceName, vmClassName, linuxVMIName, storageClass, + &vmopv1.VirtualMachineAdvancedSpec{ + VMXSwapEnabled: ptr.To(true), + }) + Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to create VM %s", vmName) + DeferCleanup(func() { + if !input.SkipCleanup { + deleteExtraConfigVM(ctx, svClusterClient, config, vmKey) + } + }) + + vmoperator.WaitForVirtualMachineConditionCreated( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName) + waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, metav1.ConditionTrue, "") + + By("Simultaneously adding HugePages1GEnabled=true (PowerOff) and flipping VMXSwapEnabled=false (PowerCycle)") + Eventually(func(g Gomega) { + g.Expect(svClusterClient.Get(ctx, vmKey, vm)).To(Succeed()) + vm.Spec.Advanced.HugePages1GEnabled = ptr.To(true) + vm.Spec.Advanced.VMXSwapEnabled = ptr.To(false) + g.Expect(svClusterClient.Update(ctx, vm)).To(Succeed()) + }, config.GetIntervals("default", "wait-vm-extraconfig-synced")...).Should(Succeed()) + + By("Asserting ExtraConfigSynced=False/PowerOffRequired (PowerOff takes priority)") + cond := waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, + metav1.ConditionFalse, extraConfigReasonPowerOffRequired) + Expect(cond).NotTo(BeNil()) + Expect(cond.Message).To(ContainSubstring(vmxHugePages), + "condition message should name the deferred PowerOff key") + }) +} + +// ── helpers ─────────────────────────────────────────────────────────────────── + +// buildExtraConfigVM constructs a v1alpha6 VirtualMachine with the given advanced spec. +// Bootstrap is disabled to avoid cloud-init customization delays. +func buildExtraConfigVM( + name, namespace, className, imageName, storageClass string, + advanced *vmopv1.VirtualMachineAdvancedSpec, +) *vmopv1.VirtualMachine { + return &vmopv1.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Labels: map[string]string{ + "e2e.vmoperator.vmware.com/extraconfig-test": "true", + }, + }, + Spec: vmopv1.VirtualMachineSpec{ + ClassName: className, + ImageName: imageName, + StorageClass: storageClass, + PowerState: vmopv1.VirtualMachinePowerStateOn, + PromoteDisksMode: vmopv1.VirtualMachinePromoteDisksModeDisabled, + Bootstrap: &vmopv1.VirtualMachineBootstrapSpec{ + Disabled: true, + }, + Advanced: advanced, + }, + } +} + +// waitForExtraConfigSynced polls until VirtualMachineExtraConfigSynced reaches +// wantStatus (and optionally wantReason when non-empty). Returns the matched condition. +func waitForExtraConfigSynced( + ctx context.Context, + client ctrlclient.Client, + config *e2eConfig.E2EConfig, + vmKey types.NamespacedName, + wantStatus metav1.ConditionStatus, + wantReason string, +) *metav1.Condition { + desc := string(wantStatus) + if wantReason != "" { + desc = fmt.Sprintf("%s/%s", wantStatus, wantReason) + } + + var matched *metav1.Condition + Eventually(func(g Gomega) { + vm := &vmopv1.VirtualMachine{} + g.Expect(client.Get(ctx, vmKey, vm)).To(Succeed()) + + var cond *metav1.Condition + for i := range vm.Status.Conditions { + if vm.Status.Conditions[i].Type == vmopv1.VirtualMachineExtraConfigSynced { + cond = &vm.Status.Conditions[i] + break + } + } + g.Expect(cond).NotTo(BeNil(), + "%s condition not yet present on VM %s", vmopv1.VirtualMachineExtraConfigSynced, vmKey) + g.Expect(cond.Status).To(Equal(wantStatus), + "%s: got status=%s reason=%s message=%s", + vmopv1.VirtualMachineExtraConfigSynced, cond.Status, cond.Reason, cond.Message) + if wantReason != "" { + g.Expect(cond.Reason).To(Equal(wantReason), + "%s: got reason=%s, want=%s", + vmopv1.VirtualMachineExtraConfigSynced, cond.Reason, wantReason) + } + matched = cond + }, config.GetIntervals("default", "wait-vm-extraconfig-synced")...).Should(Succeed(), + "timed out waiting for %s=%s on VM %s", + vmopv1.VirtualMachineExtraConfigSynced, desc, vmKey) + + return matched +} + +// statusExtraConfigValue returns the value for key in vm.Status.ExtraConfig, or "" if absent. +func statusExtraConfigValue(vm *vmopv1.VirtualMachine, key string) string { + for _, kv := range vm.Status.ExtraConfig { + if kv.Key == key { + return kv.Value + } + } + return "" +} + +// statusExtraConfigKeys returns all keys present in vm.Status.ExtraConfig. +func statusExtraConfigKeys(vm *vmopv1.VirtualMachine) []string { + keys := make([]string, 0, len(vm.Status.ExtraConfig)) + for _, kv := range vm.Status.ExtraConfig { + keys = append(keys, kv.Key) + } + return keys +} + +// getExtraConfigVM fetches the VirtualMachine or fails the test immediately. +func getExtraConfigVM(ctx context.Context, client ctrlclient.Client, key types.NamespacedName) *vmopv1.VirtualMachine { + vm := &vmopv1.VirtualMachine{} + Expect(client.Get(ctx, key, vm)).To(Succeed(), "failed to get VM %s", key) + return vm +} + +// deleteExtraConfigVM deletes the VM and waits for it to be fully removed from Kubernetes. +func deleteExtraConfigVM( + ctx context.Context, + client ctrlclient.Client, + config *e2eConfig.E2EConfig, + key types.NamespacedName, +) { + vm := &vmopv1.VirtualMachine{} + if err := client.Get(ctx, key, vm); err != nil { + e2eframework.Logf("VM %s not found during cleanup (may already be deleted): %v", key, err) + return + } + if err := client.Delete(ctx, vm); err != nil { + e2eframework.Logf("failed to delete VM %s: %v", key, err) + return + } + vmoperator.WaitForVirtualMachineToBeDeleted(ctx, config, client, key.Namespace, key.Name) +} + +// waitForBiosUUID polls until vm.Status.BiosUUID is non-empty and returns it. +func waitForBiosUUID( + ctx context.Context, + client ctrlclient.Client, + config *e2eConfig.E2EConfig, + vmKey types.NamespacedName, +) string { + var biosUUID string + Eventually(func(g Gomega) { + vm := &vmopv1.VirtualMachine{} + g.Expect(client.Get(ctx, vmKey, vm)).To(Succeed()) + g.Expect(vm.Status.BiosUUID).NotTo(BeEmpty(), + "BiosUUID not yet populated on VM %s", vmKey) + biosUUID = vm.Status.BiosUUID + }, config.GetIntervals("default", "wait-virtual-machine-moid")...).Should(Succeed(), + "timed out waiting for BiosUUID on VM %s", vmKey) + return biosUUID +} + +// findVSphereVMByBiosUUID locates a govmomi VirtualMachine object by its BIOS UUID. +// Returns nil when the VM is not found. +func findVSphereVMByBiosUUID(ctx context.Context, vimClient *vim25.Client, biosUUID string) *object.VirtualMachine { + si := object.NewSearchIndex(vimClient) + // instanceUuid=false searches by BIOS UUID (not instance UUID). + ref, err := si.FindByUuid(ctx, nil, biosUUID, true, vimtypes.NewBool(false)) + if err != nil || ref == nil { + e2eframework.Logf("VM with BiosUUID %s not found in vSphere: %v", biosUUID, err) + return nil + } + return object.NewVirtualMachine(vimClient, ref.Reference()) +} diff --git a/test/e2e/vmservice/vmservice_test.go b/test/e2e/vmservice/vmservice_test.go index d8eb64561..74e0bd5e5 100644 --- a/test/e2e/vmservice/vmservice_test.go +++ b/test/e2e/vmservice/vmservice_test.go @@ -249,14 +249,24 @@ var _ = Describe("Testing VM Services", Label("devops"), Label("viadmin"), Label Context("VM-LOCATION", func() { virtualmachine.VMLocationSpec(context.TODO(), func() virtualmachine.VMLocationSpecInput { return virtualmachine.VMLocationSpecInput{ + ClusterProxy: svClusterProxy, + Config: config, + WCPClient: wcpClient, + ArtifactFolder: artifactFolder, + } + }) + }) + + Context("VM-EXTRACONFIG", func() { + virtualmachine.VMExtraConfigSpec(context.TODO(), func() virtualmachine.VMExtraConfigSpecInput { + return virtualmachine.VMExtraConfigSpecInput{ ClusterProxy: svClusterProxy, Config: config, - WCPClient: wcpClient, ArtifactFolder: artifactFolder, + SkipCleanup: skipCleanup, WCPNamespaceName: wcpNamespaceName, } }) }) - }) }) From 837d0f64e92a50619d2498b5fd1094b126e37444 Mon Sep 17 00:00:00 2001 From: Hemanth kumar Pannem Date: Thu, 16 Jul 2026 11:49:50 -0700 Subject: [PATCH 2/3] addressed comments --- .../specs/002-vm-extraconfig-reconcile/e2e.md | 4 +- .../002-vm-extraconfig-reconcile/tasks.md | 30 +- docs/concepts/workloads/vm.md | 4 +- .../virtualmachine/vm_extraconfig.go | 309 +++++++++++------- 4 files changed, 202 insertions(+), 145 deletions(-) diff --git a/.sdd/specs/002-vm-extraconfig-reconcile/e2e.md b/.sdd/specs/002-vm-extraconfig-reconcile/e2e.md index ff0acaddb..7187ff811 100644 --- a/.sdd/specs/002-vm-extraconfig-reconcile/e2e.md +++ b/.sdd/specs/002-vm-extraconfig-reconcile/e2e.md @@ -16,7 +16,7 @@ grouped by the capability each one guards. ## Suite - `VMExtraConfigSpec`, in `test/e2e/vmservice/vmservice/virtualmachine/vm_extraconfig.go`, registered from `test/e2e/vmservice/vmservice_test.go` under a `Context("VM-EXTRACONFIG", ...)` block. -- Every scenario carries `Label("extraconfig", ...)` plus either `"core-functional"` or `"extended-functional"` — the latter for scenarios that need direct `govmomi` access to vSphere (out-of-band power-off, out-of-band destroy) and are more disruptive to the test VM's lifecycle. +- Every scenario carries either `"core-functional"` or `"extended-functional"` — the latter for scenarios that need direct `govmomi` access to vSphere (out-of-band power-off, out-of-band destroy) and are more disruptive to the test VM's lifecycle. Every scenario also carries `"experimental"` per `e2e-testing.md`, to be dropped once verified on a real Supervisor (see `tasks.md` T016). - The suite requires one new wait-interval key in `test/e2e/vmservice/config/wcp.yaml`: `default/wait-vm-extraconfig-synced` (5m/10s), used for every wait on the `ExtraConfigSynced` condition. Power-state transitions reuse the existing `default/wait-virtual-machine-powerstate` key (via `vmoperator.WaitForVirtualMachinePowerState`/`UpdateVirtualMachinePowerState`) — do not add a second, extraconfig-specific power-state interval; those helpers hardcode the shared key and are used by many other suites. - The suite requires a capability constant in `test/e2e/vmservice/consts/consts.go`: `TelcoVMServiceAPICapabilityName = "supports_telco_vm_service_api"`. - Each scenario MUST create its own VM and clean it up independently (`DeferCleanup`), so every `It` block is independently runnable via `TEST_FOCUS`/`LABEL_FILTER`. @@ -63,7 +63,7 @@ The suite MUST skip entirely when the `supports_telco_vm_service_api` Supervisor | Scenario | Verifies | |----------|----------| -| re-applies all extraConfig keys when the VM is deleted via Kubernetes and recreated with an identical spec | First-class fields and bag keys are fully re-applied from spec on the recreated VM — no key is silently skipped because it was already applied to the previous underlying vSphere VM | +| re-applies all extraConfig keys when the VM object is deleted through the Kubernetes API (normal finalizer-driven vSphere teardown — contrast with the out-of-band vSphere deletion below) and recreated with an identical spec | First-class fields and bag keys are fully re-applied from spec on the recreated VM — no key is silently skipped because it was already applied to the previous underlying vSphere VM | | resolves `PowerCyclePending` after an out-of-band vSphere power-off | Powering the VM off directly via `govmomi` (bypassing VM Operator) is detected, the pending PowerCycle-mode change applies while the VM is off, `ExtraConfigSynced` reaches `True`, and the operator's normal drift recovery restores `spec.powerState`'s desired `PoweredOn` state afterward — with extraConfig still intact post-recovery | | re-applies extraConfig after the vSphere VM is destroyed out-of-band | Destroying the underlying vSphere VM directly (the Kubernetes object survives) and then toggling `spec.powerState` to `PoweredOn` MUST cause the operator to recreate the VM (`status.uniqueID` changes) and re-apply every first-class/bag key from spec on the new VM — not silently leave the object orphaned or extraConfig-less | diff --git a/.sdd/specs/002-vm-extraconfig-reconcile/tasks.md b/.sdd/specs/002-vm-extraconfig-reconcile/tasks.md index 89737b2c0..c8698c861 100644 --- a/.sdd/specs/002-vm-extraconfig-reconcile/tasks.md +++ b/.sdd/specs/002-vm-extraconfig-reconcile/tasks.md @@ -20,33 +20,33 @@ against; no product-code task is included here. ## Phase 1 — Setup -- [ ] T001 [P] Add `TelcoVMServiceAPICapabilityName = "supports_telco_vm_service_api"` to `test/e2e/vmservice/consts/consts.go` -- [ ] T002 [P] Add `default/wait-vm-extraconfig-synced` (`5m`/`10s`) interval to `test/e2e/vmservice/config/wcp.yaml`; power-state waits reuse the existing `default/wait-virtual-machine-powerstate` key +- [x] T001 [P] Add `TelcoVMServiceAPICapabilityName = "supports_telco_vm_service_api"` to `test/e2e/vmservice/consts/consts.go` +- [x] T002 [P] Add `default/wait-vm-extraconfig-synced` (`5m`/`10s`) interval to `test/e2e/vmservice/config/wcp.yaml`; power-state waits reuse the existing `default/wait-virtual-machine-powerstate` key ## Phase 2 — Foundational -- [ ] T003 Scaffold `VMExtraConfigSpecInput`/`VMExtraConfigSpec` and the `BeforeEach` (capability skip, namespace/class/image lookup, pod-log watch) in `test/e2e/vmservice/vmservice/virtualmachine/vm_extraconfig.go` -- [ ] T004 [P] Add shared helpers in the same file: `buildExtraConfigVM`, `waitForExtraConfigSynced`, `statusExtraConfigValue`, `statusExtraConfigKeys`, `getExtraConfigVM`, `deleteExtraConfigVM` -- [ ] T005 [P] Add out-of-band vSphere helpers in the same file: `waitForBiosUUID`, `findVSphereVMByBiosUUID` (used by Phase 4's out-of-band scenarios) -- [ ] T006 Register `Context("VM-EXTRACONFIG", ...)` calling `virtualmachine.VMExtraConfigSpec` from `test/e2e/vmservice/vmservice_test.go` +- [x] T003 Scaffold `VMExtraConfigSpecInput`/`VMExtraConfigSpec` and the `BeforeEach` (capability skip, namespace/class/image lookup, pod-log watch) in `test/e2e/vmservice/vmservice/virtualmachine/vm_extraconfig.go` +- [x] T004 [P] Add shared helpers in the same file: `buildExtraConfigVM`, `waitForExtraConfigSynced`, `statusExtraConfigValue`, `statusExtraConfigKeys`, `getExtraConfigVM` +- [x] T005 [P] Add out-of-band vSphere helpers in the same file: `waitForBiosUUID`, `findVSphereVMByBiosUUID` (used by Phase 4's out-of-band scenarios) +- [x] T006 Register `Context("VM-EXTRACONFIG", ...)` calling `virtualmachine.VMExtraConfigSpec` from `test/e2e/vmservice/vmservice_test.go` ## Phase 3 — User Story: applying first-class VMX fields and bag keys, reflected in status -- [ ] T007 [US1] Add scenario: create VM with PowerCycle-mode first-class fields + two bag keys, wait for `ExtraConfigSynced=True`, assert both are visible in `status.extraConfig`, then patch bag keys (add/update/omit) and assert the change lands in `status.extraConfig` (`vm_extraconfig.go`) -- [ ] T008 [P] [US1] Add scenario: create VM with default `PromoteDisksMode=Online` plus first-class fields and a bag key, assert `ExtraConfigSynced=True` and all keys present despite concurrent disk promotion (`vm_extraconfig.go`) +- [x] T007 [US1] Add scenario: create VM with PowerCycle-mode first-class fields + two bag keys, wait for `ExtraConfigSynced=True`, assert both are visible in `status.extraConfig`, then patch bag keys (add/update/omit) and assert the change lands in `status.extraConfig` (`vm_extraconfig.go`) +- [x] T008 [P] [US1] Add scenario: create VM with default `PromoteDisksMode=Online` plus first-class fields and a bag key, assert `ExtraConfigSynced=True` and all keys present despite concurrent disk promotion (`vm_extraconfig.go`) ## Phase 4 — User Story: PowerCycle-mode vs PowerOff-mode semantics and condition precedence -- [ ] T009 [US2] Add scenario: flip a PowerCycle-mode field on a running VM, assert `ExtraConfigSynced=False/PowerCyclePending`, power-cycle the VM, assert `True` and the new value in `status.extraConfig` (`vm_extraconfig.go`) -- [ ] T010 [P] [US2] Add scenario: add a PowerOff-mode field to a running VM, assert `ExtraConfigSynced=False/PowerOffRequired` naming the field, assert the key is absent from `status.extraConfig` while deferred, power off, assert `True` and every key present (`vm_extraconfig.go`) -- [ ] T011 [P] [US2] Add scenario: change a PowerOff-mode and a PowerCycle-mode field simultaneously on a running VM, assert `ExtraConfigSynced=False/PowerOffRequired` (not `PowerCyclePending`) and the message names the PowerOff-mode field (`vm_extraconfig.go`) -- [ ] T012 [P] [US2] Add scenario: create VM with `pnumaNodeAffinity` set, assert the comma-separated encoding in `status.extraConfig`; clear the field on a running VM, assert `PowerCyclePending`; power off, assert the key is removed from `status.extraConfig` (`vm_extraconfig.go`) +- [x] T009 [US2] Add scenario: flip a PowerCycle-mode field on a running VM, assert `ExtraConfigSynced=False/PowerCyclePending`, power-cycle the VM, assert `True` and the new value in `status.extraConfig` (`vm_extraconfig.go`) +- [x] T010 [P] [US2] Add scenario: add a PowerOff-mode field to a running VM, assert `ExtraConfigSynced=False/PowerOffRequired` naming the field, assert the key is absent from `status.extraConfig` while deferred, power off, assert `True` and every key present (`vm_extraconfig.go`) +- [x] T011 [P] [US2] Add scenario: change a PowerOff-mode and a PowerCycle-mode field simultaneously on a running VM, assert `ExtraConfigSynced=False/PowerOffRequired` (not `PowerCyclePending`) and the message names the PowerOff-mode field (`vm_extraconfig.go`) +- [x] T012 [P] [US2] Add scenario: create VM with `pnumaNodeAffinity` set, assert the comma-separated encoding in `status.extraConfig`; clear the field on a running VM, assert `PowerCyclePending`; power off, assert the key is removed from `status.extraConfig` (`vm_extraconfig.go`) ## Phase 5 — User Story: resilience to VM recreation and out-of-band vSphere drift -- [ ] T013 [US3] Add scenario: power off, delete the VM via the Kubernetes API, recreate with an identical spec, assert `ExtraConfigSynced=True` and every first-class/bag key re-applied (`vm_extraconfig.go`) -- [ ] T014 [P] [US3] Add scenario: create a `PowerCyclePending` condition, power the VM off out-of-band via `govmomi`, assert the pending change applies and `ExtraConfigSynced=True`, assert the operator restores `PoweredOn` afterward with extraConfig intact (`vm_extraconfig.go`) -- [ ] T015 [P] [US3] Add scenario: power off cleanly, destroy the vSphere VM out-of-band via `govmomi` (Kubernetes object survives), set `spec.powerState=PoweredOn`, assert `status.uniqueID` changes (proving recreation), assert `VirtualMachineCreated=True` and `ExtraConfigSynced=True` with every key re-applied (`vm_extraconfig.go`) +- [x] T013 [US3] Add scenario: power off, delete the VM via the Kubernetes API, recreate with an identical spec, assert `ExtraConfigSynced=True` and every first-class/bag key re-applied (`vm_extraconfig.go`) +- [x] T014 [P] [US3] Add scenario: create a `PowerCyclePending` condition, power the VM off out-of-band via `govmomi`, assert the pending change applies and `ExtraConfigSynced=True`, assert the operator restores `PoweredOn` afterward with extraConfig intact (`vm_extraconfig.go`) +- [x] T015 [P] [US3] Add scenario: power off cleanly, destroy the vSphere VM out-of-band via `govmomi` (Kubernetes object survives), set `spec.powerState=PoweredOn`, assert `status.uniqueID` changes (proving recreation), assert `VirtualMachineCreated=True` and `ExtraConfigSynced=True` with every key re-applied (`vm_extraconfig.go`) ## Phase Final — Polish diff --git a/docs/concepts/workloads/vm.md b/docs/concepts/workloads/vm.md index aec2149be..2ca9f5425 100644 --- a/docs/concepts/workloads/vm.md +++ b/docs/concepts/workloads/vm.md @@ -912,7 +912,7 @@ The above data does _not_ represent the _observed_ network configuration of the VM advanced settings can be configured using optional fields under `spec.advanced`. For adapter-specific VMX keys, use the per-interface `spec.network.interfaces[]` fields along with their respective `advancedProperties`. The `spec.advanced.extraConfig` field should be reserved exclusively for VM-wide keys that lack a first-class field below. -| Field | Description | Mode | +| Field | Description | Change applies on | |-------|-------------|:----:| | `bootDiskCapacity` | Desired capacity of the boot disk from the VM image. Ignored when deploying from an ISO with CD-ROM devices. Resizing has guest and risk implications. | _NA_ | | `defaultVolumeProvisioningMode` | Default provisioning mode for PVCs owned by this VM. | _NA_ | @@ -925,7 +925,7 @@ VM advanced settings can be configured using optional fields under `spec.advance | `pnumaNodeAffinity` | Pin the VM to listed physical host NUMA node IDs (distinct from per-NIC `vnumaNodeID`). | PowerCycle | | `extraConfig` | Fallback list of VM-wide VMX key/value pairs not modeled above. Keys that duplicate a first-class field, or that match a reserved key or the `vmservice.`/`guestinfo.` prefixes, are rejected by the admission webhook. | _NA_* | -The `Mode` column reflects how a change to that field is applied when the VM is already powered on: +The `Change applies on` column reflects how a change to that field is applied when the VM is already powered on: * **PowerCycle** — the new value is written immediately, but the change is not considered fully synced until the VM is next power-cycled (powered off, then on). * **PowerOff** — the change cannot be applied while the VM is powered on; it is deferred until the VM is powered off, at which point it is applied. diff --git a/test/e2e/vmservice/vmservice/virtualmachine/vm_extraconfig.go b/test/e2e/vmservice/vmservice/virtualmachine/vm_extraconfig.go index e94f39781..e4d39f04f 100644 --- a/test/e2e/vmservice/vmservice/virtualmachine/vm_extraconfig.go +++ b/test/e2e/vmservice/vmservice/virtualmachine/vm_extraconfig.go @@ -15,9 +15,9 @@ import ( "github.com/vmware/govmomi/object" "github.com/vmware/govmomi/vim25" vimtypes "github.com/vmware/govmomi/vim25/types" - e2eframework "k8s.io/kubernetes/test/e2e/framework" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + e2eframework "k8s.io/kubernetes/test/e2e/framework" capiutil "sigs.k8s.io/cluster-api/util" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -27,8 +27,8 @@ import ( "github.com/vmware-tanzu/vm-operator/test/e2e/framework" "github.com/vmware-tanzu/vm-operator/test/e2e/infrastructure/vsphere/vcenter" - e2eConfig "github.com/vmware-tanzu/vm-operator/test/e2e/vmservice/config" "github.com/vmware-tanzu/vm-operator/test/e2e/vmservice/common" + e2eConfig "github.com/vmware-tanzu/vm-operator/test/e2e/vmservice/config" "github.com/vmware-tanzu/vm-operator/test/e2e/vmservice/consts" "github.com/vmware-tanzu/vm-operator/test/e2e/vmservice/lib/vmoperator" "github.com/vmware-tanzu/vm-operator/test/e2e/vmservice/skipper" @@ -38,11 +38,11 @@ import ( // VMX key names for first-class advanced fields (mirrors the vmx struct tags). const ( - vmxPreferHT = "numa.vcpu.preferHT" - vmxHugePages = "sched.mem.lpage.enable1GPage" - vmxTimeTracker = "timeTracker.lowLatency" - vmxCPUAffinity = "sched.cpu.affinity.exclusiveNoStats" - vmxVMXSwap = "sched.swap.vmxSwapEnabled" + vmxPreferHT = "numa.vcpu.preferHT" + vmxHugePages = "sched.mem.lpage.enable1GPage" + vmxTimeTracker = "timeTracker.lowLatency" + vmxCPUAffinity = "sched.cpu.affinity.exclusiveNoStats" + vmxVMXSwap = "sched.swap.vmxSwapEnabled" vmxPNUMANodeAffinity = "numa.nodeAffinity" ) @@ -139,15 +139,19 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec // ExtraConfigSynced=True, then exercises bag key CRUD and verifies status.extraConfig // reflects the changes. It("creates VM with first-class fields and bag keys, syncs immediately, reflects bag key CRUD", - Label("extraconfig", "core-functional"), func() { + Label("core-functional", "experimental"), func() { vmName := fmt.Sprintf("%s-core-%s", specName, capiutil.RandomString(4)) vmKey := types.NamespacedName{Name: vmName, Namespace: input.WCPNamespaceName} By("Creating VM with PowerCycle first-class fields and two bag keys") - vm := buildExtraConfigVM( - vmName, input.WCPNamespaceName, vmClassName, linuxVMIName, storageClass, - &vmopv1.VirtualMachineAdvancedSpec{ + vm := buildExtraConfigVM(buildExtraConfigVMOpts{ + Name: vmName, + Namespace: input.WCPNamespaceName, + ClassName: vmClassName, + ImageName: linuxVMIName, + StorageClass: storageClass, + Advanced: &vmopv1.VirtualMachineAdvancedSpec{ PreferHTEnabled: ptr.To(true), TimeTrackerLowLatencyEnabled: ptr.To(true), CPUAffinityExclusiveNoStatsEnabled: ptr.To(false), @@ -156,11 +160,13 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec {Key: "custom.test.foo", Value: "bar"}, {Key: "custom.test.baz", Value: "qux"}, }, - }) + }, + }) Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to create VM %s", vmName) DeferCleanup(func() { if !input.SkipCleanup { - deleteExtraConfigVM(ctx, svClusterClient, config, vmKey) + vmoperator.DeleteVirtualMachine(ctx, svClusterClient, vmKey.Namespace, vmKey.Name) + vmoperator.WaitForVirtualMachineToBeDeleted(ctx, config, svClusterClient, vmKey.Namespace, vmKey.Name) } }) @@ -188,15 +194,15 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec // Phase 2: bag key CRUD. By("Patching spec: add new bag key, update existing value, omit one to trigger deletion") - Eventually(func(g Gomega) { - g.Expect(svClusterClient.Get(ctx, vmKey, vm)).To(Succeed()) - vm.Spec.Advanced.ExtraConfig = []vmopv1common.KeyValuePair{ - {Key: "custom.test.foo", Value: "updated"}, - {Key: "custom.test.new", Value: "newval"}, - } - g.Expect(svClusterClient.Update(ctx, vm)).To(Succeed()) - }, config.GetIntervals("default", "wait-vm-extraconfig-synced")...).Should(Succeed(), + vm = getExtraConfigVM(ctx, svClusterClient, vmKey) + vmPatch := vm.DeepCopy() + vmPatch.Spec.Advanced.ExtraConfig = []vmopv1common.KeyValuePair{ + {Key: "custom.test.foo", Value: "updated"}, + {Key: "custom.test.new", Value: "newval"}, + } + Expect(svClusterClient.Patch(ctx, vmPatch, ctrlclient.MergeFrom(vm))).To(Succeed(), "failed to patch VM %s bag keys", vmName) + vm = vmPatch By("Waiting for status.extraConfig to reflect the bag key changes") Eventually(func(g Gomega) { @@ -212,37 +218,46 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec // Flips a PowerCycle-mode field on a running VM, verifies PowerCyclePending, // then power-cycles to apply and verifies the new value. It("marks PowerCyclePending when a PowerCycle-mode field changes while VM is powered on", - Label("extraconfig", "core-functional"), func() { + Label("core-functional", "experimental"), func() { vmName := fmt.Sprintf("%s-pc-%s", specName, capiutil.RandomString(4)) vmKey := types.NamespacedName{Name: vmName, Namespace: input.WCPNamespaceName} By("Creating VM with PowerCycle first-class fields") - vm := buildExtraConfigVM( - vmName, input.WCPNamespaceName, vmClassName, linuxVMIName, storageClass, - &vmopv1.VirtualMachineAdvancedSpec{ + vm := buildExtraConfigVM(buildExtraConfigVMOpts{ + Name: vmName, + Namespace: input.WCPNamespaceName, + ClassName: vmClassName, + ImageName: linuxVMIName, + StorageClass: storageClass, + Advanced: &vmopv1.VirtualMachineAdvancedSpec{ PreferHTEnabled: ptr.To(true), TimeTrackerLowLatencyEnabled: ptr.To(true), CPUAffinityExclusiveNoStatsEnabled: ptr.To(false), VMXSwapEnabled: ptr.To(true), - }) + }, + }) Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to create VM %s", vmName) DeferCleanup(func() { if !input.SkipCleanup { - deleteExtraConfigVM(ctx, svClusterClient, config, vmKey) + vmoperator.DeleteVirtualMachine(ctx, svClusterClient, vmKey.Namespace, vmKey.Name) + vmoperator.WaitForVirtualMachineToBeDeleted(ctx, config, svClusterClient, vmKey.Namespace, vmKey.Name) } }) vmoperator.WaitForVirtualMachineConditionCreated( ctx, config, svClusterClient, input.WCPNamespaceName, vmName) waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, metav1.ConditionTrue, "") + vmoperator.WaitForVirtualMachinePowerState( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName, "PoweredOn") By("Flipping VMXSwapEnabled=false (PowerCycle-mode field) while VM is powered on") - Eventually(func(g Gomega) { - g.Expect(svClusterClient.Get(ctx, vmKey, vm)).To(Succeed()) - vm.Spec.Advanced.VMXSwapEnabled = ptr.To(false) - g.Expect(svClusterClient.Update(ctx, vm)).To(Succeed()) - }, config.GetIntervals("default", "wait-vm-extraconfig-synced")...).Should(Succeed()) + vm = getExtraConfigVM(ctx, svClusterClient, vmKey) + vmPatch := vm.DeepCopy() + vmPatch.Spec.Advanced.VMXSwapEnabled = ptr.To(false) + Expect(svClusterClient.Patch(ctx, vmPatch, ctrlclient.MergeFrom(vm))).To(Succeed(), + "failed to patch VM %s VMXSwapEnabled", vmName) + vm = vmPatch By("Waiting for ExtraConfigSynced=False/PowerCyclePending") cond := waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, @@ -276,15 +291,19 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec // that the key is absent from status while deferred, then powers off and verifies // all keys applied. It("defers a PowerOff-mode field while VM is powered on, applies it after power-off", - Label("extraconfig", "core-functional"), func() { + Label("core-functional", "experimental"), func() { vmName := fmt.Sprintf("%s-po-%s", specName, capiutil.RandomString(4)) vmKey := types.NamespacedName{Name: vmName, Namespace: input.WCPNamespaceName} By("Creating VM with first-class fields and one bag key") - vm := buildExtraConfigVM( - vmName, input.WCPNamespaceName, vmClassName, linuxVMIName, storageClass, - &vmopv1.VirtualMachineAdvancedSpec{ + vm := buildExtraConfigVM(buildExtraConfigVMOpts{ + Name: vmName, + Namespace: input.WCPNamespaceName, + ClassName: vmClassName, + ImageName: linuxVMIName, + StorageClass: storageClass, + Advanced: &vmopv1.VirtualMachineAdvancedSpec{ PreferHTEnabled: ptr.To(true), TimeTrackerLowLatencyEnabled: ptr.To(true), CPUAffinityExclusiveNoStatsEnabled: ptr.To(false), @@ -292,24 +311,29 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec ExtraConfig: []vmopv1common.KeyValuePair{ {Key: "custom.test.foo", Value: "bar"}, }, - }) + }, + }) Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to create VM %s", vmName) DeferCleanup(func() { if !input.SkipCleanup { - deleteExtraConfigVM(ctx, svClusterClient, config, vmKey) + vmoperator.DeleteVirtualMachine(ctx, svClusterClient, vmKey.Namespace, vmKey.Name) + vmoperator.WaitForVirtualMachineToBeDeleted(ctx, config, svClusterClient, vmKey.Namespace, vmKey.Name) } }) vmoperator.WaitForVirtualMachineConditionCreated( ctx, config, svClusterClient, input.WCPNamespaceName, vmName) waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, metav1.ConditionTrue, "") + vmoperator.WaitForVirtualMachinePowerState( + ctx, config, svClusterClient, input.WCPNamespaceName, vmName, "PoweredOn") By("Adding HugePages1GEnabled=true (PowerOff-mode) while VM is powered on") - Eventually(func(g Gomega) { - g.Expect(svClusterClient.Get(ctx, vmKey, vm)).To(Succeed()) - vm.Spec.Advanced.HugePages1GEnabled = ptr.To(true) - g.Expect(svClusterClient.Update(ctx, vm)).To(Succeed()) - }, config.GetIntervals("default", "wait-vm-extraconfig-synced")...).Should(Succeed()) + vm = getExtraConfigVM(ctx, svClusterClient, vmKey) + vmPatch := vm.DeepCopy() + vmPatch.Spec.Advanced.HugePages1GEnabled = ptr.To(true) + Expect(svClusterClient.Patch(ctx, vmPatch, ctrlclient.MergeFrom(vm))).To(Succeed(), + "failed to patch VM %s HugePages1GEnabled", vmName) + vm = vmPatch By("Waiting for ExtraConfigSynced=False/PowerOffRequired") cond := waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, @@ -346,7 +370,7 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec // Deletes a VM via the Kubernetes API and recreates it with an identical spec, // verifying that the operator re-applies all extraConfig keys from scratch. It("re-applies all extraConfig keys when VM is deleted via Kubernetes and recreated", - Label("extraconfig", "core-functional"), func() { + Label("core-functional", "experimental"), func() { vmName := fmt.Sprintf("%s-recreate-%s", specName, capiutil.RandomString(4)) vmKey := types.NamespacedName{Name: vmName, Namespace: input.WCPNamespaceName} @@ -363,8 +387,14 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec } By("Creating VM with first-class fields and bag keys") - vm := buildExtraConfigVM( - vmName, input.WCPNamespaceName, vmClassName, linuxVMIName, storageClass, advanced) + vm := buildExtraConfigVM(buildExtraConfigVMOpts{ + Name: vmName, + Namespace: input.WCPNamespaceName, + ClassName: vmClassName, + ImageName: linuxVMIName, + StorageClass: storageClass, + Advanced: advanced, + }) Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to create VM %s", vmName) vmoperator.WaitForVirtualMachineConditionCreated( @@ -378,15 +408,23 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec ctx, config, svClusterClient, input.WCPNamespaceName, vmName, "PoweredOff") By("Deleting VM via Kubernetes API") - deleteExtraConfigVM(ctx, svClusterClient, config, vmKey) + vmoperator.DeleteVirtualMachine(ctx, svClusterClient, vmKey.Namespace, vmKey.Name) + vmoperator.WaitForVirtualMachineToBeDeleted(ctx, config, svClusterClient, vmKey.Namespace, vmKey.Name) By("Recreating VM with identical spec") - vm = buildExtraConfigVM( - vmName, input.WCPNamespaceName, vmClassName, linuxVMIName, storageClass, advanced) + vm = buildExtraConfigVM(buildExtraConfigVMOpts{ + Name: vmName, + Namespace: input.WCPNamespaceName, + ClassName: vmClassName, + ImageName: linuxVMIName, + StorageClass: storageClass, + Advanced: advanced, + }) Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to recreate VM %s", vmName) DeferCleanup(func() { if !input.SkipCleanup { - deleteExtraConfigVM(ctx, svClusterClient, config, vmKey) + vmoperator.DeleteVirtualMachine(ctx, svClusterClient, vmKey.Namespace, vmKey.Name) + vmoperator.WaitForVirtualMachineToBeDeleted(ctx, config, svClusterClient, vmKey.Namespace, vmKey.Name) } }) @@ -410,24 +448,30 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec // through vSphere directly. Verifies that the operator detects the power-off, // applies the pending change, and restores the VM to PoweredOn. It("resolves PowerCyclePending after an out-of-band vSphere power-off", - Label("extraconfig", "extended-functional"), func() { + Label("extended-functional", "experimental"), func() { vmName := fmt.Sprintf("%s-oob-off-%s", specName, capiutil.RandomString(4)) vmKey := types.NamespacedName{Name: vmName, Namespace: input.WCPNamespaceName} By("Creating VM with first-class fields, all synced") - vm := buildExtraConfigVM( - vmName, input.WCPNamespaceName, vmClassName, linuxVMIName, storageClass, - &vmopv1.VirtualMachineAdvancedSpec{ + vm := buildExtraConfigVM(buildExtraConfigVMOpts{ + Name: vmName, + Namespace: input.WCPNamespaceName, + ClassName: vmClassName, + ImageName: linuxVMIName, + StorageClass: storageClass, + Advanced: &vmopv1.VirtualMachineAdvancedSpec{ PreferHTEnabled: ptr.To(true), TimeTrackerLowLatencyEnabled: ptr.To(true), CPUAffinityExclusiveNoStatsEnabled: ptr.To(false), VMXSwapEnabled: ptr.To(true), - }) + }, + }) Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to create VM %s", vmName) DeferCleanup(func() { if !input.SkipCleanup { - deleteExtraConfigVM(ctx, svClusterClient, config, vmKey) + vmoperator.DeleteVirtualMachine(ctx, svClusterClient, vmKey.Namespace, vmKey.Name) + vmoperator.WaitForVirtualMachineToBeDeleted(ctx, config, svClusterClient, vmKey.Namespace, vmKey.Name) } }) @@ -436,11 +480,12 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, metav1.ConditionTrue, "") By("Flipping PreferHTEnabled=false (PowerCycle-mode) to create PowerCyclePending") - Eventually(func(g Gomega) { - g.Expect(svClusterClient.Get(ctx, vmKey, vm)).To(Succeed()) - vm.Spec.Advanced.PreferHTEnabled = ptr.To(false) - g.Expect(svClusterClient.Update(ctx, vm)).To(Succeed()) - }, config.GetIntervals("default", "wait-vm-extraconfig-synced")...).Should(Succeed()) + vm = getExtraConfigVM(ctx, svClusterClient, vmKey) + vmPatch := vm.DeepCopy() + vmPatch.Spec.Advanced.PreferHTEnabled = ptr.To(false) + Expect(svClusterClient.Patch(ctx, vmPatch, ctrlclient.MergeFrom(vm))).To(Succeed(), + "failed to patch VM %s PreferHTEnabled", vmName) + vm = vmPatch waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, metav1.ConditionFalse, extraConfigReasonPowerCyclePending) @@ -477,15 +522,19 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec // that the operator detects the orphaned K8s object, recreates the VM in vSphere, // and re-applies all extraConfig keys from the spec. It("re-applies extraConfig after the vSphere VM is destroyed out-of-band", - Label("extraconfig", "extended-functional"), func() { + Label("extended-functional", "experimental"), func() { vmName := fmt.Sprintf("%s-oob-del-%s", specName, capiutil.RandomString(4)) vmKey := types.NamespacedName{Name: vmName, Namespace: input.WCPNamespaceName} By("Creating VM with first-class fields and a bag key") - vm := buildExtraConfigVM( - vmName, input.WCPNamespaceName, vmClassName, linuxVMIName, storageClass, - &vmopv1.VirtualMachineAdvancedSpec{ + vm := buildExtraConfigVM(buildExtraConfigVMOpts{ + Name: vmName, + Namespace: input.WCPNamespaceName, + ClassName: vmClassName, + ImageName: linuxVMIName, + StorageClass: storageClass, + Advanced: &vmopv1.VirtualMachineAdvancedSpec{ PreferHTEnabled: ptr.To(true), TimeTrackerLowLatencyEnabled: ptr.To(true), CPUAffinityExclusiveNoStatsEnabled: ptr.To(false), @@ -493,11 +542,13 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec ExtraConfig: []vmopv1common.KeyValuePair{ {Key: "custom.test.foo", Value: "bar"}, }, - }) + }, + }) Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to create VM %s", vmName) DeferCleanup(func() { if !input.SkipCleanup { - deleteExtraConfigVM(ctx, svClusterClient, config, vmKey) + vmoperator.DeleteVirtualMachine(ctx, svClusterClient, vmKey.Namespace, vmKey.Name) + vmoperator.WaitForVirtualMachineToBeDeleted(ctx, config, svClusterClient, vmKey.Namespace, vmKey.Name) } }) @@ -532,11 +583,12 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec Expect(destroyTask.Wait(ctx)).To(Succeed(), "vSphere VM destroy task failed") By("Setting spec.powerState=PoweredOn so the operator powers on the recreated VM") - Eventually(func(g Gomega) { - g.Expect(svClusterClient.Get(ctx, vmKey, vm)).To(Succeed()) - vm.Spec.PowerState = vmopv1.VirtualMachinePowerStateOn - g.Expect(svClusterClient.Update(ctx, vm)).To(Succeed()) - }, config.GetIntervals("default", "wait-vm-extraconfig-synced")...).Should(Succeed()) + vm = getExtraConfigVM(ctx, svClusterClient, vmKey) + vmPatch := vm.DeepCopy() + vmPatch.Spec.PowerState = vmopv1.VirtualMachinePowerStateOn + Expect(svClusterClient.Patch(ctx, vmPatch, ctrlclient.MergeFrom(vm))).To(Succeed(), + "failed to patch VM %s spec.powerState", vmName) + vm = vmPatch By("Waiting for operator to recreate the VM (UniqueID change proves a new vSphere VM)") Eventually(func(g Gomega) { @@ -565,21 +617,27 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec // is encoded correctly as a comma-separated string, triggers PowerCyclePending // when changed while the VM is running, and resolves after a power cycle. It("handles PNUMANodeAffinity ([]int32 field): syncs, marks PowerCyclePending on change, resolves after power-off", - Label("extraconfig", "core-functional"), func() { + Label("core-functional", "experimental"), func() { vmName := fmt.Sprintf("%s-pnuma-%s", specName, capiutil.RandomString(4)) vmKey := types.NamespacedName{Name: vmName, Namespace: input.WCPNamespaceName} By("Creating VM with PNUMANodeAffinity pinned to NUMA node 0") - vm := buildExtraConfigVM( - vmName, input.WCPNamespaceName, vmClassName, linuxVMIName, storageClass, - &vmopv1.VirtualMachineAdvancedSpec{ + vm := buildExtraConfigVM(buildExtraConfigVMOpts{ + Name: vmName, + Namespace: input.WCPNamespaceName, + ClassName: vmClassName, + ImageName: linuxVMIName, + StorageClass: storageClass, + Advanced: &vmopv1.VirtualMachineAdvancedSpec{ PNUMANodeAffinity: []int32{0}, - }) + }, + }) Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to create VM %s", vmName) DeferCleanup(func() { if !input.SkipCleanup { - deleteExtraConfigVM(ctx, svClusterClient, config, vmKey) + vmoperator.DeleteVirtualMachine(ctx, svClusterClient, vmKey.Namespace, vmKey.Name) + vmoperator.WaitForVirtualMachineToBeDeleted(ctx, config, svClusterClient, vmKey.Namespace, vmKey.Name) } }) @@ -593,11 +651,12 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec "expected %s=0 in status", vmxPNUMANodeAffinity) By("Clearing PNUMANodeAffinity (nil) while VM is powered on — should mark PowerCyclePending") - Eventually(func(g Gomega) { - g.Expect(svClusterClient.Get(ctx, vmKey, vm)).To(Succeed()) - vm.Spec.Advanced.PNUMANodeAffinity = nil - g.Expect(svClusterClient.Update(ctx, vm)).To(Succeed()) - }, config.GetIntervals("default", "wait-vm-extraconfig-synced")...).Should(Succeed()) + vm = getExtraConfigVM(ctx, svClusterClient, vmKey) + vmPatch := vm.DeepCopy() + vmPatch.Spec.Advanced.PNUMANodeAffinity = nil + Expect(svClusterClient.Patch(ctx, vmPatch, ctrlclient.MergeFrom(vm))).To(Succeed(), + "failed to patch VM %s PNUMANodeAffinity", vmName) + vm = vmPatch By("Waiting for ExtraConfigSynced=False/PowerCyclePending") waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, @@ -624,7 +683,7 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec // ExtraConfigSynced reaches True even while SvMotion may be in flight, // catching ordering or resource-conflict bugs between the two reconcilers. It("applies extraConfig correctly while disk promotion runs (default PromoteDisksMode=Online)", - Label("extraconfig", "core-functional"), func() { + Label("core-functional", "experimental"), func() { vmName := fmt.Sprintf("%s-promo-%s", specName, capiutil.RandomString(4)) vmKey := types.NamespacedName{Name: vmName, Namespace: input.WCPNamespaceName} @@ -658,7 +717,8 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to create VM %s", vmName) DeferCleanup(func() { if !input.SkipCleanup { - deleteExtraConfigVM(ctx, svClusterClient, config, vmKey) + vmoperator.DeleteVirtualMachine(ctx, svClusterClient, vmKey.Namespace, vmKey.Name) + vmoperator.WaitForVirtualMachineToBeDeleted(ctx, config, svClusterClient, vmKey.Namespace, vmKey.Name) } }) @@ -681,21 +741,27 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec // both change simultaneously on a running VM, PowerOffRequired must take // priority over PowerCyclePending in the ExtraConfigSynced condition. It("PowerOffRequired takes priority over PowerCyclePending when both key types change simultaneously", - Label("extraconfig", "core-functional"), func() { + Label("core-functional", "experimental"), func() { vmName := fmt.Sprintf("%s-prio-%s", specName, capiutil.RandomString(4)) vmKey := types.NamespacedName{Name: vmName, Namespace: input.WCPNamespaceName} By("Creating VM with VMXSwapEnabled=true, no HugePages") - vm := buildExtraConfigVM( - vmName, input.WCPNamespaceName, vmClassName, linuxVMIName, storageClass, - &vmopv1.VirtualMachineAdvancedSpec{ + vm := buildExtraConfigVM(buildExtraConfigVMOpts{ + Name: vmName, + Namespace: input.WCPNamespaceName, + ClassName: vmClassName, + ImageName: linuxVMIName, + StorageClass: storageClass, + Advanced: &vmopv1.VirtualMachineAdvancedSpec{ VMXSwapEnabled: ptr.To(true), - }) + }, + }) Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to create VM %s", vmName) DeferCleanup(func() { if !input.SkipCleanup { - deleteExtraConfigVM(ctx, svClusterClient, config, vmKey) + vmoperator.DeleteVirtualMachine(ctx, svClusterClient, vmKey.Namespace, vmKey.Name) + vmoperator.WaitForVirtualMachineToBeDeleted(ctx, config, svClusterClient, vmKey.Namespace, vmKey.Name) } }) @@ -704,12 +770,13 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, metav1.ConditionTrue, "") By("Simultaneously adding HugePages1GEnabled=true (PowerOff) and flipping VMXSwapEnabled=false (PowerCycle)") - Eventually(func(g Gomega) { - g.Expect(svClusterClient.Get(ctx, vmKey, vm)).To(Succeed()) - vm.Spec.Advanced.HugePages1GEnabled = ptr.To(true) - vm.Spec.Advanced.VMXSwapEnabled = ptr.To(false) - g.Expect(svClusterClient.Update(ctx, vm)).To(Succeed()) - }, config.GetIntervals("default", "wait-vm-extraconfig-synced")...).Should(Succeed()) + vm = getExtraConfigVM(ctx, svClusterClient, vmKey) + vmPatch := vm.DeepCopy() + vmPatch.Spec.Advanced.HugePages1GEnabled = ptr.To(true) + vmPatch.Spec.Advanced.VMXSwapEnabled = ptr.To(false) + Expect(svClusterClient.Patch(ctx, vmPatch, ctrlclient.MergeFrom(vm))).To(Succeed(), + "failed to patch VM %s HugePages1GEnabled/VMXSwapEnabled", vmName) + vm = vmPatch By("Asserting ExtraConfigSynced=False/PowerOffRequired (PowerOff takes priority)") cond := waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, @@ -722,30 +789,39 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec // ── helpers ─────────────────────────────────────────────────────────────────── +// buildExtraConfigVMOpts holds the parameters for buildExtraConfigVM. Using a +// struct instead of a long positional parameter list means adding an optional +// field later won't require touching every call site. +type buildExtraConfigVMOpts struct { + Name string + Namespace string + ClassName string + ImageName string + StorageClass string + Advanced *vmopv1.VirtualMachineAdvancedSpec +} + // buildExtraConfigVM constructs a v1alpha6 VirtualMachine with the given advanced spec. // Bootstrap is disabled to avoid cloud-init customization delays. -func buildExtraConfigVM( - name, namespace, className, imageName, storageClass string, - advanced *vmopv1.VirtualMachineAdvancedSpec, -) *vmopv1.VirtualMachine { +func buildExtraConfigVM(opts buildExtraConfigVMOpts) *vmopv1.VirtualMachine { return &vmopv1.VirtualMachine{ ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, + Name: opts.Name, + Namespace: opts.Namespace, Labels: map[string]string{ "e2e.vmoperator.vmware.com/extraconfig-test": "true", }, }, Spec: vmopv1.VirtualMachineSpec{ - ClassName: className, - ImageName: imageName, - StorageClass: storageClass, + ClassName: opts.ClassName, + ImageName: opts.ImageName, + StorageClass: opts.StorageClass, PowerState: vmopv1.VirtualMachinePowerStateOn, PromoteDisksMode: vmopv1.VirtualMachinePromoteDisksModeDisabled, Bootstrap: &vmopv1.VirtualMachineBootstrapSpec{ Disabled: true, }, - Advanced: advanced, + Advanced: opts.Advanced, }, } } @@ -821,25 +897,6 @@ func getExtraConfigVM(ctx context.Context, client ctrlclient.Client, key types.N return vm } -// deleteExtraConfigVM deletes the VM and waits for it to be fully removed from Kubernetes. -func deleteExtraConfigVM( - ctx context.Context, - client ctrlclient.Client, - config *e2eConfig.E2EConfig, - key types.NamespacedName, -) { - vm := &vmopv1.VirtualMachine{} - if err := client.Get(ctx, key, vm); err != nil { - e2eframework.Logf("VM %s not found during cleanup (may already be deleted): %v", key, err) - return - } - if err := client.Delete(ctx, vm); err != nil { - e2eframework.Logf("failed to delete VM %s: %v", key, err) - return - } - vmoperator.WaitForVirtualMachineToBeDeleted(ctx, config, client, key.Namespace, key.Name) -} - // waitForBiosUUID polls until vm.Status.BiosUUID is non-empty and returns it. func waitForBiosUUID( ctx context.Context, From b9711fc182bc19a926e1ff135dccf5fdc2156f63 Mon Sep 17 00:00:00 2001 From: Hemanth kumar Pannem Date: Thu, 16 Jul 2026 17:27:34 -0700 Subject: [PATCH 3/3] move status to update_status() --- api/v1alpha6/virtualmachine_types.go | 8 + .../virtualmachine/extraconfig/managed.go | 110 +++++ .../extraconfig/managed_test.go | 95 +++++ .../vsphere/vmlifecycle/update_status.go | 127 ++++++ .../vsphere/vmlifecycle/update_status_test.go | 397 ++++++++++++++++++ pkg/util/vmopv1/extraconfig.go | 15 + pkg/vmconfig/extraconfig/reconciler.go | 222 ++-------- pkg/vmconfig/extraconfig/reconciler_test.go | 213 +--------- .../virtualmachine/vm_extraconfig.go | 38 +- 9 files changed, 819 insertions(+), 406 deletions(-) diff --git a/api/v1alpha6/virtualmachine_types.go b/api/v1alpha6/virtualmachine_types.go index 712e7f1a0..3f174a0c7 100644 --- a/api/v1alpha6/virtualmachine_types.go +++ b/api/v1alpha6/virtualmachine_types.go @@ -200,6 +200,14 @@ const ( // VirtualMachineExtraConfigErrorReason indicates that reconciliation of // VirtualMachineExtraConfigSynced failed with an unexpected error. VirtualMachineExtraConfigErrorReason = "ExtraConfigError" + + // VirtualMachineExtraConfigMismatchReason is used on the ExtraConfigSynced + // condition when the VM's observed ExtraConfig does not yet match + // spec.advanced, but the pending change is neither deferred (see + // VirtualMachinePowerOffRequiredReason) nor power-cycle-pending (see + // VirtualMachinePowerCyclePendingReason). This is expected to resolve to + // True once a later reconcile observes the applied change. + VirtualMachineExtraConfigMismatchReason = "ExtraConfigMismatch" ) const ( diff --git a/pkg/providers/vsphere/virtualmachine/extraconfig/managed.go b/pkg/providers/vsphere/virtualmachine/extraconfig/managed.go index 467e06357..9d347c5a2 100644 --- a/pkg/providers/vsphere/virtualmachine/extraconfig/managed.go +++ b/pkg/providers/vsphere/virtualmachine/extraconfig/managed.go @@ -49,6 +49,116 @@ func loadManagedKeys(observed pkgutil.OptionValues, key string) []string { return out } +// SpecBagKeys returns the set of valid bag-key names in advanced.ExtraConfig, +// skipping system-reserved and first-class keys. Returns an empty (non-nil) +// map when advanced is nil. +func SpecBagKeys(advanced *vmopv1.VirtualMachineAdvancedSpec) map[string]bool { + keys := make(map[string]bool) + if advanced == nil { + return keys + } + + firstClassKeys := vmopv1util.AdvancedVMXKeyMap() + for _, kv := range advanced.ExtraConfig { + if vmopv1util.IsSystemReservedExtraConfigKey(kv.Key) { + continue + } + if _, isFC := firstClassKeys[kv.Key]; isFC { + continue + } + keys[kv.Key] = true + } + return keys +} + +// DesiredVMExtraConfig assembles the full desired set of VM-level ExtraConfig +// entries for spec.advanced: first-class VMX translations (including "" +// clear-if-present entries for nil/zero fields) plus user bag keys, with "" +// clear entries for previously-managed bag keys no longer in spec. +// +// managedKeys is the previously-tracked bag-key set (see LoadVMManagedKeys), +// used to detect and clear bag keys that were removed from spec. +func DesiredVMExtraConfig( + ctx context.Context, + advanced *vmopv1.VirtualMachineAdvancedSpec, + managedKeys []string, +) pkgutil.OptionValues { + if advanced == nil { + advanced = &vmopv1.VirtualMachineAdvancedSpec{} + } + + specBagKeys := SpecBagKeys(advanced) + var desired pkgutil.OptionValues + + for _, kv := range advanced.ExtraConfig { + if specBagKeys[kv.Key] { + desired = append(desired, &vimtypes.OptionValue{Key: kv.Key, Value: kv.Value}) + } + } + for _, mk := range managedKeys { + if !specBagKeys[mk] { + desired = append(desired, &vimtypes.OptionValue{Key: mk, Value: ""}) + } + } + + return append(desired, TranslateFirstClass(ctx, advanced)...) +} + +// RouteByVMXMode routes a semantic diff (see SemanticDiff) by vmxmode: +// PowerOff-mode entries cannot be applied while the VM is powered on, so they +// are excluded from applied and returned in deferred instead. PowerCycle-mode +// entries are included in applied but flagged via powerCyclePending, since +// they only take effect after a guest power cycle. Non-first-class entries +// (e.g. bag keys) are always included in applied. +func RouteByVMXMode( + diff pkgutil.OptionValues, + modeMap map[string]vmopv1util.VMXMode, + poweredOn bool, +) (applied pkgutil.OptionValues, deferred []string, powerCyclePending bool) { + + for _, entry := range diff { + kv := entry.GetOptionValue() + if kv == nil { + continue + } + mode, isFC := modeMap[kv.Key] + switch { + case isFC && mode == vmopv1util.VMXModePowerOff && poweredOn: + deferred = append(deferred, kv.Key) + default: + applied = append(applied, entry) + if isFC && mode == vmopv1util.VMXModePowerCycle && poweredOn { + powerCyclePending = true + } + } + } + return applied, deferred, powerCyclePending +} + +// VMExtraConfigDiff computes the desired-vs-observed diff for spec.advanced's +// ExtraConfig and classifies it by vmxmode, shared by the config-mutation +// path (Reconcile) and the status/condition path (reconcileStatusExtraConfig +// in pkg/providers/vsphere/vmlifecycle). +// +// existingEC is any already-assembled ExtraConfig from other reconcilers to +// merge onto before diffing (e.g. configSpec.ExtraConfig, so the config path +// can still submit a single Reconfigure covering every reconciler's changes); +// pass nil to scope the diff to only spec.advanced's own keys, as the status +// path does so an unrelated reconciler's pending change can't be mistaken for +// an ExtraConfig mismatch. +func VMExtraConfigDiff( + ctx context.Context, + vm *vmopv1.VirtualMachine, + observed pkgutil.OptionValues, + managedKeys []string, + existingEC pkgutil.OptionValues, +) (applied pkgutil.OptionValues, deferred []string, powerCyclePending bool) { + desired := DesiredVMExtraConfig(ctx, vm.Spec.Advanced, managedKeys) + semanticResult := SemanticDiff(ctx, observed, existingEC.Merge(desired...)) + poweredOn := vm.Status.PowerState == vmopv1.VirtualMachinePowerStateOn + return RouteByVMXMode(semanticResult, vmopv1util.AdvancedVMXModeMap(), poweredOn) +} + // SemanticDiff filters the assembled configSpec ExtraConfig before it is sent // to vSphere, dropping entries that are semantically identical to what the VM // already has. It is the final deduplication step before configSpec is submitted. diff --git a/pkg/providers/vsphere/virtualmachine/extraconfig/managed_test.go b/pkg/providers/vsphere/virtualmachine/extraconfig/managed_test.go index b1f091aa0..8fce1aed3 100644 --- a/pkg/providers/vsphere/virtualmachine/extraconfig/managed_test.go +++ b/pkg/providers/vsphere/virtualmachine/extraconfig/managed_test.go @@ -209,3 +209,98 @@ var _ = Describe("TranslateFirstClass + SemanticDiff round-trip", func() { Expect(out).To(BeNil(), "expected no diff for semantically identical state") }) }) + +var _ = Describe("DesiredVMExtraConfig", func() { + ctx := context.Background() + + It("returns only first-class clears when advanced is nil", func() { + out := extraconfig.DesiredVMExtraConfig(ctx, nil, nil) + v, ok := out.GetString("numa.vcpu.preferHT") + Expect(ok).To(BeTrue()) + Expect(v).To(Equal("")) + }) + + It("includes first-class translations", func() { + adv := &vmopv1.VirtualMachineAdvancedSpec{PreferHTEnabled: ptr.To(true)} + out := extraconfig.DesiredVMExtraConfig(ctx, adv, nil) + v, ok := out.GetString("numa.vcpu.preferHT") + Expect(ok).To(BeTrue()) + Expect(v).To(Equal("TRUE")) + }) + + It("includes bag keys from spec, skipping reserved and first-class keys", func() { + adv := &vmopv1.VirtualMachineAdvancedSpec{ + ExtraConfig: []vmopv1common.KeyValuePair{ + {Key: "foo", Value: "bar"}, + {Key: vsphereconst.EnableDiskUUIDExtraConfigKey, Value: "TRUE"}, + }, + } + out := extraconfig.DesiredVMExtraConfig(ctx, adv, nil) + v, ok := out.GetString("foo") + Expect(ok).To(BeTrue()) + Expect(v).To(Equal("bar")) + _, ok = out.GetString(vsphereconst.EnableDiskUUIDExtraConfigKey) + Expect(ok).To(BeFalse()) + }) + + It("emits a clear for a managed key no longer in spec", func() { + out := extraconfig.DesiredVMExtraConfig(ctx, &vmopv1.VirtualMachineAdvancedSpec{}, []string{"foo"}) + v, ok := out.GetString("foo") + Expect(ok).To(BeTrue()) + Expect(v).To(Equal("")) + }) + + It("does not clear a managed key still present in spec", func() { + adv := &vmopv1.VirtualMachineAdvancedSpec{ + ExtraConfig: []vmopv1common.KeyValuePair{{Key: "foo", Value: "bar"}}, + } + out := extraconfig.DesiredVMExtraConfig(ctx, adv, []string{"foo"}) + v, ok := out.GetString("foo") + Expect(ok).To(BeTrue()) + Expect(v).To(Equal("bar")) + }) +}) + +var _ = Describe("RouteByVMXMode", func() { + modeMap := vmopv1util.AdvancedVMXModeMap() + + It("returns everything applied, nothing deferred, when VM is powered off", func() { + diff := ovList("sched.mem.lpage.enable1GPage", "TRUE") + applied, deferred, powerCyclePending := extraconfig.RouteByVMXMode(diff, modeMap, false) + Expect(applied).To(HaveLen(1)) + Expect(deferred).To(BeEmpty()) + Expect(powerCyclePending).To(BeFalse()) + }) + + It("defers a PowerOff-mode key while powered on", func() { + diff := ovList("sched.mem.lpage.enable1GPage", "TRUE") + applied, deferred, powerCyclePending := extraconfig.RouteByVMXMode(diff, modeMap, true) + Expect(applied).To(BeEmpty()) + Expect(deferred).To(ConsistOf("sched.mem.lpage.enable1GPage")) + Expect(powerCyclePending).To(BeFalse()) + }) + + It("applies a PowerCycle-mode key and flags powerCyclePending while powered on", func() { + diff := ovList("numa.vcpu.preferHT", "TRUE") + applied, deferred, powerCyclePending := extraconfig.RouteByVMXMode(diff, modeMap, true) + Expect(applied).To(HaveLen(1)) + Expect(deferred).To(BeEmpty()) + Expect(powerCyclePending).To(BeTrue()) + }) + + It("applies a PowerCycle-mode key without flagging powerCyclePending while powered off", func() { + diff := ovList("numa.vcpu.preferHT", "TRUE") + applied, deferred, powerCyclePending := extraconfig.RouteByVMXMode(diff, modeMap, false) + Expect(applied).To(HaveLen(1)) + Expect(deferred).To(BeEmpty()) + Expect(powerCyclePending).To(BeFalse()) + }) + + It("always applies non-first-class (bag) keys regardless of power state", func() { + diff := ovList("custom.bag.key", "v") + applied, deferred, powerCyclePending := extraconfig.RouteByVMXMode(diff, modeMap, true) + Expect(applied).To(HaveLen(1)) + Expect(deferred).To(BeEmpty()) + Expect(powerCyclePending).To(BeFalse()) + }) +}) diff --git a/pkg/providers/vsphere/vmlifecycle/update_status.go b/pkg/providers/vsphere/vmlifecycle/update_status.go index a2a6cb56f..6ae584f40 100644 --- a/pkg/providers/vsphere/vmlifecycle/update_status.go +++ b/pkg/providers/vsphere/vmlifecycle/update_status.go @@ -29,12 +29,14 @@ import ( "github.com/vmware-tanzu/vm-operator/api/v1alpha6/common" "github.com/vmware-tanzu/vm-operator/pkg/conditions" pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config" + pkgconst "github.com/vmware-tanzu/vm-operator/pkg/constants" pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context" pkglog "github.com/vmware-tanzu/vm-operator/pkg/log" "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants" "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/network" "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/vcenter" "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/virtualmachine" + "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/virtualmachine/extraconfig" vmoprecord "github.com/vmware-tanzu/vm-operator/pkg/record" "github.com/vmware-tanzu/vm-operator/pkg/topology" pkgutil "github.com/vmware-tanzu/vm-operator/pkg/util" @@ -75,6 +77,10 @@ func ReconcileStatus( errs = append(errs, reconcileStatusNodeName(vmCtx, k8sClient, vcVM, data)...) errs = append(errs, reconcileStatusController(vmCtx, k8sClient, vcVM, data)...) + if pkgcfg.FromContext(vmCtx).Features.TelcoVMServiceAPI { + errs = append(errs, reconcileStatusExtraConfig(vmCtx, k8sClient, vcVM, data)...) + } + if pkgcfg.FromContext(vmCtx).Features.VMSharedDisks { errs = append(errs, reconcileHardwareCondition(vmCtx, k8sClient, vcVM, data)...) } @@ -2096,3 +2102,124 @@ func reconcileStatusController( return nil } + +// reconcileStatusExtraConfig populates status.ExtraConfig with the +// first-class advanced VMX keys and spec.advanced.extraConfig bag keys +// currently observed on the VM, and sets ExtraConfigSynced from that same +// observed state: True once it fully matches spec.advanced and no power +// cycle is pending, or False with a specific reason (PowerOffRequired, +// PowerCyclePending, or ExtraConfigMismatch for a pending change that is +// neither) otherwise. It always reflects moVM as fetched at the start of +// this reconcile, so the condition is only ever set from vSphere's confirmed +// state — never optimistically from a configSpec this reconcile is still in +// the process of applying. +// +// This function is the sole source of the condition for every case except a +// real Reconfigure task failure, which only extraconfig.OnResult can see (it +// runs after the Reconfigure attempt, with the resulting error). So there is +// no correctness dependency between the two on ordering or on OnResult +// running in the same pass: whichever last touches the condition this cycle +// wins, and both independently agree on non-error cases. +func reconcileStatusExtraConfig( + vmCtx pkgctx.VirtualMachineContext, + _ ctrlclient.Client, + _ *object.VirtualMachine, + _ ReconcileStatusData) []error { //nolint:unparam + + vm := vmCtx.VM + moVM := vmCtx.MoVM + if moVM.Config == nil { + return nil + } + + observed := pkgutil.OptionValues(moVM.Config.ExtraConfig) + managedKeys := extraconfig.LoadVMManagedKeys(observed) + + var statusEC []common.KeyValuePair + for _, k := range vmopv1util.SortedAdvancedVMXKeys() { + if v, ok := observed.GetString(k); ok && v != "" { + statusEC = append(statusEC, common.KeyValuePair{Key: k, Value: v}) + } + } + + // Bag keys have no fixed candidate list like first-class keys do (their + // names are arbitrary), so the previously-tracked managedKeys marker is + // normally what tells us which keys to look up in observed. That marker + // is empty until this reconciler has applied a given bag key at least + // once, so also check any bag key currently requested in spec — covering + // a key that already exists on the VM (e.g. pre-set by the class) before + // this reconciler has ever applied it, matching how a first-class key's + // current value is reported regardless of whether it matches desired. + bagKeyCandidates := extraconfig.SpecBagKeys(vm.Spec.Advanced) + for _, k := range managedKeys { + bagKeyCandidates[k] = true + } + for _, k := range sets.List(sets.KeySet(bagKeyCandidates)) { + if v, ok := observed.GetString(k); ok && v != "" { + statusEC = append(statusEC, common.KeyValuePair{Key: k, Value: v}) + } + } + vm.Status.ExtraConfig = statusEC + + // Comparing spec.advanced against observed below is only meaningful once + // this VM has been through the one-time TelcoVMServiceAPI schema-upgrade + // backfill (pkg/providers/vsphere/upgrade/virtualmachine/backfill). Before + // that, spec.advanced may still be nil/zero for a field a VM Class already + // applied directly to vSphere, and comparing against it here would + // misreport a pending clear for a value nothing actually wants cleared. + fv := vmopv1util.ParseFeatureVersion(vm.Annotations[pkgconst.UpgradedToFeatureVersionAnnotationKey]) + if !fv.Has(vmopv1util.FeatureVersionTelcoVMServiceAPI) { + return nil + } + + poweredOn := vm.Status.PowerState == vmopv1.VirtualMachinePowerStateOn + + // A prior cycle may have left vmx.reboot.powerCycle=TRUE on the VM: the + // value already matches (so it won't appear in semanticResult below), but + // the change only takes effect once the guest actually reboots. While the + // VM is on, that flag — not value-matching alone — is authoritative for + // "synced". Once the VM is off, all config is applied and ESXi clears the + // flag automatically on next boot, so it's no longer treated as pending. + var powerCycleOnVM bool + if poweredOn { + _, powerCycleOnVM = observed.GetString(constants.ExtraConfigReservedKeyVMXRebootPowerCycle) + } + + // Diff spec.advanced against observed (scoped to only its own keys, via a + // nil existingEC, so an unrelated reconciler's pending change can't be + // mistaken for an ExtraConfig mismatch) and route it by vmxmode, mirroring + // extraconfig.Reconcile's use of the same diff. + applied, deferred, powerCyclePending := extraconfig.VMExtraConfigDiff( + vmCtx, vm, observed, managedKeys, nil) + + switch { + case len(deferred) > 0: + conditions.MarkFalse( + vm, + vmopv1.VirtualMachineExtraConfigSynced, + vmopv1.VirtualMachinePowerOffRequiredReason, + "VM power off required to apply: %s", strings.Join(deferred, ", ")) + case powerCyclePending || powerCycleOnVM: + conditions.MarkFalse( + vm, + vmopv1.VirtualMachineExtraConfigSynced, + vmopv1.VirtualMachinePowerCyclePendingReason, + "Applied changes take effect on next power cycle") + case len(applied) == 0: + conditions.MarkTrue(vm, vmopv1.VirtualMachineExtraConfigSynced) + default: + // A genuine pending change that is neither deferred nor + // power-cycle-mode (e.g. a bag key just added). Mark it explicitly + // rather than leaving a stale True or a stale reason from a prior + // cycle in place — it resolves to True on the reconcile after this + // one's Reconfigure completes and a fresh Properties() read + // confirms it. + conditions.MarkFalse( + vm, + vmopv1.VirtualMachineExtraConfigSynced, + vmopv1.VirtualMachineExtraConfigMismatchReason, + "Spec extraconfig differs from the live vSphere configuration") + } + + return nil +} diff --git a/pkg/providers/vsphere/vmlifecycle/update_status_test.go b/pkg/providers/vsphere/vmlifecycle/update_status_test.go index c14123a0b..018adad86 100644 --- a/pkg/providers/vsphere/vmlifecycle/update_status_test.go +++ b/pkg/providers/vsphere/vmlifecycle/update_status_test.go @@ -30,6 +30,7 @@ import ( vmopv1common "github.com/vmware-tanzu/vm-operator/api/v1alpha6/common" "github.com/vmware-tanzu/vm-operator/pkg/conditions" pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config" + pkgconst "github.com/vmware-tanzu/vm-operator/pkg/constants" pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context" "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere" "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants" @@ -39,6 +40,7 @@ import ( pkgutil "github.com/vmware-tanzu/vm-operator/pkg/util" kubeutil "github.com/vmware-tanzu/vm-operator/pkg/util/kube" "github.com/vmware-tanzu/vm-operator/pkg/util/ptr" + vmopv1util "github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1" "github.com/vmware-tanzu/vm-operator/test/builder" ) @@ -5947,6 +5949,401 @@ var _ = Describe("Hardware status", func() { }) }) +var _ = Describe("ExtraConfig status", func() { + var ( + ctx *builder.TestContextForVCSim + vmCtx pkgctx.VirtualMachineContext + data vmlifecycle.ReconcileStatusData + vcVM *object.VirtualMachine + vm *vmopv1.VirtualMachine + ) + + BeforeEach(func() { + ctx = suite.NewTestContextForVCSim(builder.VCSimTestConfig{}) + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.TelcoVMServiceAPI = true + }) + + vm = builder.DummyVirtualMachine() + vm.Name = "extraconfig-status-test" + // Most tests below exercise the spec-vs-observed comparison, which + // only runs once this VM has been through the one-time + // TelcoVMServiceAPI schema-upgrade backfill. The "has not yet been + // backfilled" case is covered separately. + vm.SetAnnotation( + pkgconst.UpgradedToFeatureVersionAnnotationKey, + vmopv1util.FeatureVersionTelcoVMServiceAPI.String()) + + vmCtx = pkgctx.VirtualMachineContext{ + Context: ctx, + Logger: suite.GetLogger().WithValues("vmName", vm.Name), + VM: vm, + } + + var err error + vcVM, err = ctx.Finder.VirtualMachine(ctx, "DC0_C0_RP0_VM0") + Expect(err).ToNot(HaveOccurred()) + + Expect(vcVM.Properties( + ctx, + vcVM.Reference(), + vsphere.VMUpdatePropertiesSelector, + &vmCtx.MoVM)).To(Succeed()) + + data = vmlifecycle.ReconcileStatusData{ + NetworkDeviceKeysToSpecIdx: map[int32]int{}, + } + }) + + AfterEach(func() { + ctx.AfterEach() + ctx = nil + }) + + When("TelcoVMServiceAPI is disabled", func() { + BeforeEach(func() { + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.TelcoVMServiceAPI = false + }) + vmCtx.MoVM.Config.ExtraConfig = []vimtypes.BaseOptionValue{ + &vimtypes.OptionValue{Key: "numa.vcpu.preferHT", Value: "TRUE"}, + } + }) + + It("does not populate status.ExtraConfig", func() { + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + Expect(vmCtx.VM.Status.ExtraConfig).To(BeEmpty()) + }) + }) + + When("this VM has not yet been through the TelcoVMServiceAPI schema-upgrade backfill", func() { + BeforeEach(func() { + // No UpgradedToFeatureVersionAnnotationKey annotation at all, + // simulating a VM whose class set a value directly on vSphere + // before spec.advanced.PreferHTEnabled has been backfilled from it. + delete(vm.Annotations, pkgconst.UpgradedToFeatureVersionAnnotationKey) + vmCtx.MoVM.Config.ExtraConfig = []vimtypes.BaseOptionValue{ + &vimtypes.OptionValue{Key: "numa.vcpu.preferHT", Value: "TRUE"}, + } + }) + + It("still populates status.ExtraConfig from observed", func() { + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + Expect(vmCtx.VM.Status.ExtraConfig).To(ContainElement( + vmopv1common.KeyValuePair{Key: "numa.vcpu.preferHT", Value: "TRUE"})) + }) + + It("does not touch ExtraConfigSynced (spec.advanced isn't backfilled yet, so a comparison would be meaningless)", func() { + // spec.advanced.PreferHTEnabled is nil here even though vSphere + // already has numa.vcpu.preferHT=TRUE (from the class). Without + // the guard this would misreport a pending clear. + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + Expect(conditions.Get(vmCtx.VM, vmopv1.VirtualMachineExtraConfigSynced)).To(BeNil()) + }) + }) + + When("moVM.Config is nil", func() { + BeforeEach(func() { + existing := []vmopv1common.KeyValuePair{{Key: "numa.vcpu.preferHT", Value: "TRUE"}} + vmCtx.VM.Status.ExtraConfig = existing + vmCtx.MoVM.Config = nil + }) + + It("leaves status.ExtraConfig unchanged", func() { + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + Expect(vmCtx.VM.Status.ExtraConfig).To(Equal( + []vmopv1common.KeyValuePair{{Key: "numa.vcpu.preferHT", Value: "TRUE"}})) + }) + }) + + When("vm.Spec.Advanced is nil and nothing is observed", func() { + BeforeEach(func() { + vm.Spec.Advanced = nil + vmCtx.MoVM.Config.ExtraConfig = nil + }) + + It("marks ExtraConfigSynced=True", func() { + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + cond := conditions.Get(vmCtx.VM, vmopv1.VirtualMachineExtraConfigSynced) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + }) + }) + + When("first-class and managed bag keys are observed on the VM", func() { + BeforeEach(func() { + vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{ + PreferHTEnabled: ptr.To(true), + TimeTrackerLowLatencyEnabled: ptr.To(true), + CPUAffinityExclusiveNoStatsEnabled: ptr.To(true), + ExtraConfig: []vmopv1common.KeyValuePair{{Key: "foo", Value: "bar"}}, + } + vmCtx.MoVM.Config.ExtraConfig = []vimtypes.BaseOptionValue{ + &vimtypes.OptionValue{Key: "numa.vcpu.preferHT", Value: "TRUE"}, + &vimtypes.OptionValue{Key: "timeTracker.lowLatency", Value: "TRUE"}, + &vimtypes.OptionValue{Key: "sched.cpu.affinity.exclusiveNoStats", Value: "TRUE"}, + &vimtypes.OptionValue{Key: "foo", Value: "bar"}, + &vimtypes.OptionValue{Key: constants.ExtraConfigManagedKeysKey, Value: "foo"}, + &vimtypes.OptionValue{Key: "tools.guest.desktop.autolock", Value: "FALSE"}, // class-derived, not managed + &vimtypes.OptionValue{Key: "disk.enableUUID", Value: "TRUE"}, // internal, not managed + } + }) + + It("populates status.ExtraConfig with only user-controlled keys, first-class keys sorted first", func() { + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + Expect(vmCtx.VM.Status.ExtraConfig).To(Equal([]vmopv1common.KeyValuePair{ + {Key: "numa.vcpu.preferHT", Value: "TRUE"}, + {Key: "sched.cpu.affinity.exclusiveNoStats", Value: "TRUE"}, + {Key: "timeTracker.lowLatency", Value: "TRUE"}, + {Key: "foo", Value: "bar"}, + })) + }) + + It("marks ExtraConfigSynced=True since observed fully matches spec.advanced", func() { + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + cond := conditions.Get(vmCtx.VM, vmopv1.VirtualMachineExtraConfigSynced) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + }) + + When("ESXi returns a first-class bool value in a different case", func() { + BeforeEach(func() { + // "true" vs "TRUE": must still be treated as a semantic match, + // not a pending change, matching extraconfig.SemanticDiff. + for _, ov := range vmCtx.MoVM.Config.ExtraConfig { + if kv := ov.GetOptionValue(); kv != nil && kv.Key == "numa.vcpu.preferHT" { + kv.Value = "true" + } + } + }) + + It("still marks ExtraConfigSynced=True", func() { + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + cond := conditions.Get(vmCtx.VM, vmopv1.VirtualMachineExtraConfigSynced) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + }) + }) + }) + + When("a first-class key is desired but not yet observed on the VM", func() { + BeforeEach(func() { + // Simulates the pass right after Reconfigure submitted the change: + // moVM here predates that Reconfigure, so the key is not observed + // yet. VM is off, so this is a plain pending mismatch — neither + // deferred nor power-cycle-mode routing applies. + vmCtx.MoVM.Runtime.PowerState = vimtypes.VirtualMachinePowerStatePoweredOff + vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{PreferHTEnabled: ptr.To(true)} + vmCtx.MoVM.Config.ExtraConfig = nil + }) + + It("omits the key until vSphere actually reports it", func() { + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + for _, kv := range vmCtx.VM.Status.ExtraConfig { + Expect(kv.Key).ToNot(Equal("numa.vcpu.preferHT")) + } + }) + + It("marks ExtraConfigSynced=False with ExtraConfigMismatch (resolves once a later reconcile observes it)", func() { + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + cond := conditions.Get(vmCtx.VM, vmopv1.VirtualMachineExtraConfigSynced) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(vmopv1.VirtualMachineExtraConfigMismatchReason)) + }) + }) + + When("a bag key is desired but not yet observed on the VM (first pass, nothing applied yet)", func() { + BeforeEach(func() { + // Brand new bag key in spec; vSphere has neither the value nor + // the managed-keys marker yet. managedKeys is read from the + // observed marker, not spec, so on this pass it's empty too. + vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{ + ExtraConfig: []vmopv1common.KeyValuePair{{Key: "foo", Value: "bar"}}, + } + vmCtx.MoVM.Config.ExtraConfig = nil + }) + + It("omits the key until vSphere actually reports it", func() { + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + Expect(vmCtx.VM.Status.ExtraConfig).ToNot(ContainElement( + vmopv1common.KeyValuePair{Key: "foo", Value: "bar"})) + }) + + It("marks ExtraConfigSynced=False with ExtraConfigMismatch", func() { + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + cond := conditions.Get(vmCtx.VM, vmopv1.VirtualMachineExtraConfigSynced) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(vmopv1.VirtualMachineExtraConfigMismatchReason)) + }) + }) + + When("a bag key is desired and already exists on the VM, but not yet tracked as managed", func() { + BeforeEach(func() { + // Simulates a bag key pre-set on vSphere by something other than + // this reconciler (e.g. the VM Class) before this reconciler has + // ever applied it: the value is observed, but the managed-keys + // marker doesn't mention it yet. + vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{ + ExtraConfig: []vmopv1common.KeyValuePair{{Key: "foo", Value: "bar"}}, + } + vmCtx.MoVM.Config.ExtraConfig = []vimtypes.BaseOptionValue{ + &vimtypes.OptionValue{Key: "foo", Value: "old-value"}, + } + }) + + It("reports the key's current observed value in status.ExtraConfig", func() { + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + Expect(vmCtx.VM.Status.ExtraConfig).To(ContainElement( + vmopv1common.KeyValuePair{Key: "foo", Value: "old-value"})) + }) + + It("marks ExtraConfigSynced=False with ExtraConfigMismatch, since the value differs from desired", func() { + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + cond := conditions.Get(vmCtx.VM, vmopv1.VirtualMachineExtraConfigSynced) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(vmopv1.VirtualMachineExtraConfigMismatchReason)) + }) + }) + + When("a bag key removed from spec is still observed on the VM (managed cleanup pending)", func() { + BeforeEach(func() { + // Spec no longer has "foo", but it is still observed and tracked as + // managed — desiredVMExtraConfig must emit a clear for it so this + // isn't mistaken for already-synced. + vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{} + vmCtx.MoVM.Config.ExtraConfig = []vimtypes.BaseOptionValue{ + &vimtypes.OptionValue{Key: "foo", Value: "bar"}, + &vimtypes.OptionValue{Key: constants.ExtraConfigManagedKeysKey, Value: "foo"}, + } + }) + + It("marks ExtraConfigSynced=False with ExtraConfigMismatch", func() { + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + cond := conditions.Get(vmCtx.VM, vmopv1.VirtualMachineExtraConfigSynced) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(vmopv1.VirtualMachineExtraConfigMismatchReason)) + }) + + It("still reports the key in status.ExtraConfig, since it is genuinely still on the VM", func() { + // managedKeys is read from the observed marker, not from + // spec.Advanced.ExtraConfig — if it were spec-derived, this key + // would vanish from status the instant it's removed from spec, + // even though vSphere hasn't cleared it yet. + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + Expect(vmCtx.VM.Status.ExtraConfig).To(ContainElement( + vmopv1common.KeyValuePair{Key: "foo", Value: "bar"})) + }) + }) + + When("a PowerOff-mode key differs from spec on a powered-on VM (deferred)", func() { + BeforeEach(func() { + // Unlike the leftover vmx.reboot.powerCycle flag, a deferred key's + // "not yet applied" state IS a value mismatch — extraconfig.Reconcile + // excludes it from configSpec precisely because observed != desired. + // So SemanticDiff (called independently here, not via OnResult) must + // already catch it on its own. + vmCtx.MoVM.Runtime.PowerState = vimtypes.VirtualMachinePowerStatePoweredOn + vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{HugePages1GEnabled: ptr.To(true)} + vmCtx.MoVM.Config.ExtraConfig = []vimtypes.BaseOptionValue{ + &vimtypes.OptionValue{Key: "sched.mem.lpage.enable1GPage", Value: "FALSE"}, + } + }) + + It("marks PowerOffRequired, without extraconfig.OnResult ever running", func() { + // This test calls only vmlifecycle.ReconcileStatus, never + // extraconfig.OnResult — proving reconcileStatusExtraConfig alone, + // with no help from OnResult, correctly reports deferred. + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + cond := conditions.Get(vmCtx.VM, vmopv1.VirtualMachineExtraConfigSynced) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(vmopv1.VirtualMachinePowerOffRequiredReason)) + Expect(cond.Message).To(ContainSubstring("sched.mem.lpage.enable1GPage")) + }) + }) + + When("a power cycle is still pending on a powered-on VM", func() { + BeforeEach(func() { + // Values already match spec (a prior cycle applied them), but the + // leftover vmx.reboot.powerCycle flag means the guest hasn't + // actually rebooted to pick up the change yet. + // reconcileStatusPowerState (which runs before reconcileStatusExtraConfig) + // derives vm.Status.PowerState from this, so set it here rather + // than on vm.Status directly. + vmCtx.MoVM.Runtime.PowerState = vimtypes.VirtualMachinePowerStatePoweredOn + vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{PreferHTEnabled: ptr.To(true)} + vmCtx.MoVM.Config.ExtraConfig = []vimtypes.BaseOptionValue{ + &vimtypes.OptionValue{Key: "numa.vcpu.preferHT", Value: "TRUE"}, + &vimtypes.OptionValue{Key: constants.ExtraConfigReservedKeyVMXRebootPowerCycle, Value: "TRUE"}, + } + }) + + It("marks PowerCyclePending even though observed values match", func() { + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + cond := conditions.Get(vmCtx.VM, vmopv1.VirtualMachineExtraConfigSynced) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(vmopv1.VirtualMachinePowerCyclePendingReason)) + }) + + When("the VM is powered off", func() { + BeforeEach(func() { + vmCtx.MoVM.Runtime.PowerState = vimtypes.VirtualMachinePowerStatePoweredOff + }) + + It("marks ExtraConfigSynced=True since ESXi clears the flag on next boot", func() { + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + cond := conditions.Get(vmCtx.VM, vmopv1.VirtualMachineExtraConfigSynced) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + }) + }) + }) + + When("PNUMANodeAffinity ([]int32 field) changes on a powered-on VM", func() { + BeforeEach(func() { + vmCtx.MoVM.Runtime.PowerState = vimtypes.VirtualMachinePowerStatePoweredOn + vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{PNUMANodeAffinity: []int32{0}} + vmCtx.MoVM.Config.ExtraConfig = []vimtypes.BaseOptionValue{ + &vimtypes.OptionValue{Key: "numa.nodeAffinity", Value: "1,2"}, + } + }) + + It("marks PowerCyclePending", func() { + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + cond := conditions.Get(vmCtx.VM, vmopv1.VirtualMachineExtraConfigSynced) + Expect(cond).ToNot(BeNil()) + Expect(cond.Reason).To(Equal(vmopv1.VirtualMachinePowerCyclePendingReason)) + }) + }) + + When("a PowerOff-mode key and a PowerCycle-mode key both differ on a powered-on VM", func() { + BeforeEach(func() { + vmCtx.MoVM.Runtime.PowerState = vimtypes.VirtualMachinePowerStatePoweredOn + vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{ + HugePages1GEnabled: ptr.To(true), // poweroff -> deferred + VMXSwapEnabled: ptr.To(false), // powercycle -> pending + } + vmCtx.MoVM.Config.ExtraConfig = []vimtypes.BaseOptionValue{ + &vimtypes.OptionValue{Key: "sched.mem.lpage.enable1GPage", Value: "FALSE"}, + &vimtypes.OptionValue{Key: "sched.swap.vmxSwapEnabled", Value: "TRUE"}, + } + }) + + It("marks PowerOffRequired, taking priority over PowerCyclePending", func() { + Expect(vmlifecycle.ReconcileStatus(vmCtx, ctx.Client, vcVM, data)).To(Succeed()) + cond := conditions.Get(vmCtx.VM, vmopv1.VirtualMachineExtraConfigSynced) + Expect(cond).ToNot(BeNil()) + Expect(cond.Reason).To(Equal(vmopv1.VirtualMachinePowerOffRequiredReason)) + Expect(cond.Message).To(ContainSubstring("sched.mem.lpage.enable1GPage")) + }) + }) +}) + var _ = Describe("Guest status", func() { var ( ctx *builder.TestContextForVCSim diff --git a/pkg/util/vmopv1/extraconfig.go b/pkg/util/vmopv1/extraconfig.go index e17cf2f1a..675e83b69 100644 --- a/pkg/util/vmopv1/extraconfig.go +++ b/pkg/util/vmopv1/extraconfig.go @@ -15,6 +15,8 @@ import ( "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/util/sets" + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha6" pkglog "github.com/vmware-tanzu/vm-operator/pkg/log" vsphereconst "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants" @@ -72,6 +74,13 @@ type vmxTagMaps struct { var ( cachedAdvancedVMXMaps = sync.OnceValue(buildAdvancedVMXMaps[vmopv1.VirtualMachineAdvancedSpec]) cachedVMXNet3NICMaps = sync.OnceValue(buildVMXNet3NICMaps[vmopv1.VirtualMachineNetworkInterfaceVMXNet3Spec]) + + // cachedSortedAdvancedVMXKeys is the sorted list of VMX keys mapped to + // first-class VirtualMachineAdvancedSpec fields. Precomputed once since + // callers (e.g. status reconciliation) rebuild it on every call otherwise. + cachedSortedAdvancedVMXKeys = sync.OnceValue(func() []string { + return sets.List(sets.KeySet(AdvancedVMXKeyMap())) + }) ) // AdvancedVMXKeyMap returns the shared, lazily-built map of vmx struct tags @@ -80,6 +89,12 @@ func AdvancedVMXKeyMap() map[string]int { return cachedAdvancedVMXMaps().keys } +// SortedAdvancedVMXKeys returns the shared, lazily-built, sorted list of vmx +// struct tags for VirtualMachineAdvancedSpec (the keys of AdvancedVMXKeyMap). +func SortedAdvancedVMXKeys() []string { + return cachedSortedAdvancedVMXKeys() +} + // AdvancedVMXModeMap returns the shared, lazily-built map of vmx tag value → // VMXMode for VirtualMachineAdvancedSpec. func AdvancedVMXModeMap() map[string]VMXMode { diff --git a/pkg/vmconfig/extraconfig/reconciler.go b/pkg/vmconfig/extraconfig/reconciler.go index 1750f796f..f3dbfb4fc 100644 --- a/pkg/vmconfig/extraconfig/reconciler.go +++ b/pkg/vmconfig/extraconfig/reconciler.go @@ -6,60 +6,31 @@ package extraconfig import ( "context" - "sort" "strings" - "sync" "github.com/vmware/govmomi/vim25" "github.com/vmware/govmomi/vim25/mo" vimtypes "github.com/vmware/govmomi/vim25/types" + + "k8s.io/apimachinery/pkg/util/sets" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha6" - vmopv1common "github.com/vmware-tanzu/vm-operator/api/v1alpha6/common" "github.com/vmware-tanzu/vm-operator/pkg/conditions" pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config" - ctxgen "github.com/vmware-tanzu/vm-operator/pkg/context/generic" pkgerr "github.com/vmware-tanzu/vm-operator/pkg/errors" vsphereconst "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants" "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/virtualmachine/extraconfig" pkgutil "github.com/vmware-tanzu/vm-operator/pkg/util" - vmopv1util "github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1" "github.com/vmware-tanzu/vm-operator/pkg/vmconfig" ) -type contextKeyType uint8 - -const contextKeyValue contextKeyType = 0 - -// sortedFirstClassVMXKeys is the sorted list of VMX keys mapped to first-class -// API fields. Precomputed once to avoid rebuilding on every OnResult call. -var sortedFirstClassVMXKeys = sync.OnceValue(func() []string { - keys := vmopv1util.AdvancedVMXKeyMap() - out := make([]string, 0, len(keys)) - for k := range keys { - out = append(out, k) - } - sort.Strings(out) - return out -}) - -// state is shared between Reconcile and OnResult via context. -type state struct { - // NextManagedKeys is the sorted list of spec.advanced.extraConfig bag keys. - NextManagedKeys []string - // Deferred holds first-class VMX keys skipped due to PowerOff mode while VM is on. - Deferred []string - // PowerCyclePending is true when vmx.reboot.powerCycle was injected this cycle. - PowerCyclePending bool -} - type reconciler struct{} -var _ vmconfig.ReconcilerWithContext = reconciler{} +var _ vmconfig.Reconciler = reconciler{} -// New returns a new ReconcilerWithContext for spec.advanced ExtraConfig. -func New() vmconfig.ReconcilerWithContext { +// New returns a new Reconciler for spec.advanced ExtraConfig. +func New() vmconfig.Reconciler { return reconciler{} } @@ -76,13 +47,6 @@ func Reconcile( func (r reconciler) Name() string { return "extraconfig" } -func (r reconciler) WithContext(parent context.Context) context.Context { - return ctxgen.WithContext( - parent, - contextKeyValue, - func() state { return state{} }) -} - func (r reconciler) Reconcile( ctx context.Context, _ ctrlclient.Client, @@ -111,98 +75,37 @@ func (r reconciler) Reconcile( } observed := pkgutil.OptionValues(moVM.Config.ExtraConfig) - poweredOn := vm.Status.PowerState == vmopv1.VirtualMachinePowerStateOn - firstClassKeys := vmopv1util.AdvancedVMXKeyMap() - modeMap := vmopv1util.AdvancedVMXModeMap() - - // Collect overlay entries to apply on top of configSpec.ExtraConfig. - var overlay pkgutil.OptionValues - // Overlay bag keys from spec.advanced.extraConfig, skipping reserved and first-class keys. - // The webhook rejects reserved and first-class keys before they reach the reconciler; - // this guard is a defence-in-depth measure. - specBagKeys := make(map[string]bool) - if vm.Spec.Advanced != nil { - for _, kv := range vm.Spec.Advanced.ExtraConfig { - if vmopv1util.IsSystemReservedExtraConfigKey(kv.Key) || isFirstClassKey(kv.Key, firstClassKeys) { - continue - } - specBagKeys[kv.Key] = true - overlay = append(overlay, &vimtypes.OptionValue{Key: kv.Key, Value: kv.Value}) - } - } + // Current bag-key set from spec (skips reserved and first-class keys; + // the webhook rejects those before they reach the reconciler, so this is + // a defence-in-depth measure). Used below to detect bag-key + // additions/removals against the previously-tracked managed-keys marker. + specBagKeys := extraconfig.SpecBagKeys(vm.Spec.Advanced) - // Clear bag keys previously tracked but no longer in spec. managed := extraconfig.LoadVMManagedKeys(observed) - for _, mk := range managed { - if !specBagKeys[mk] { - overlay = append(overlay, &vimtypes.OptionValue{Key: mk, Value: ""}) - } - } + + // Diff spec.advanced against observed, merged onto whatever other + // reconcilers already set on configSpec, and route it by vmxmode: + // PowerOff entries cannot be applied while the VM is on (they are + // deferred here, but this reconciler has no further use for that list — + // see reconcileStatusExtraConfig for the status/condition use of the same + // diff). PowerCycle entries are applied but take effect only after a + // guest power cycle. + result, _, powerCyclePending := extraconfig.VMExtraConfigDiff( + ctx, vm, observed, managed, pkgutil.OptionValues(configSpec.ExtraConfig)) // Update the managed keys tracking entry when the bag key set changes. - nextManagedKeys := make([]string, 0, len(specBagKeys)) - for k := range specBagKeys { - nextManagedKeys = append(nextManagedKeys, k) - } - sort.Strings(nextManagedKeys) + // Not part of the diff above since it's pure bookkeeping, not a + // vmxmode-routed spec.advanced field. + nextManagedKeys := sets.List(sets.KeySet(specBagKeys)) nextMKStr := strings.Join(nextManagedKeys, ",") if nextMKStr != strings.Join(managed, ",") { - overlay = append(overlay, &vimtypes.OptionValue{ + result = append(result, &vimtypes.OptionValue{ Key: vsphereconst.ExtraConfigManagedKeysKey, Value: nextMKStr, }) } - // Overlay all first-class translations (including "" for nil/zero fields). - // vmxmode routing happens after SemanticDiff, not here. - advanced := vm.Spec.Advanced - if advanced == nil { - advanced = &vmopv1.VirtualMachineAdvancedSpec{} - } - for _, entry := range extraconfig.TranslateFirstClass(ctx, advanced) { - kv := entry.GetOptionValue() - if kv == nil { - continue - } - desiredStr, _ := kv.Value.(string) - overlay = append(overlay, &vimtypes.OptionValue{Key: kv.Key, Value: desiredStr}) - } - - // Assemble: apply our overlay on top of what other reconcilers already set. - var allDesiredEC pkgutil.OptionValues - if len(overlay) > 0 { - allDesiredEC = pkgutil.OptionValues(configSpec.ExtraConfig).Merge(overlay...) - } else { - allDesiredEC = pkgutil.OptionValues(configSpec.ExtraConfig) - } - - // Suppress entries semantically identical to what vSphere already has. - // Only entries representing genuine changes remain. - semanticResult := extraconfig.SemanticDiff(ctx, observed, allDesiredEC) - - // Route result by vmxmode: PowerOff entries cannot be applied while the VM is - // on (they are deferred), PowerCycle entries are applied but take effect only - // after a guest power cycle. - var deferred []string - powerCyclePending := false - var result pkgutil.OptionValues - for _, entry := range semanticResult { - kv := entry.GetOptionValue() - if kv == nil { - continue - } - mode, isFC := modeMap[kv.Key] - switch { - case isFC && mode == vmopv1util.VMXModePowerOff && poweredOn: - deferred = append(deferred, kv.Key) - default: - result = append(result, entry) - if isFC && mode == vmopv1util.VMXModePowerCycle && poweredOn { - powerCyclePending = true - } - } - } if powerCyclePending { result = append(result, &vimtypes.OptionValue{ Key: vsphereconst.ExtraConfigReservedKeyVMXRebootPowerCycle, @@ -212,20 +115,22 @@ func (r reconciler) Reconcile( configSpec.ExtraConfig = result - ctxgen.SetContext(ctx, contextKeyValue, func(s state) state { - s.NextManagedKeys = nextManagedKeys - s.Deferred = deferred - s.PowerCyclePending = powerCyclePending - return s - }) - return nil } +// OnResult marks ExtraConfigSynced=False when the Reconfigure task this cycle +// submitted actually failed. That is the only thing this function has +// definitive knowledge of that reconcileStatusExtraConfig +// (pkg/providers/vsphere/vmlifecycle) cannot derive on its own: it runs +// before Reconcile/doReconfigure even attempt the change, using vmCtx.MoVM as +// fetched at the start of this reconcile. reconcileStatusExtraConfig is the +// sole source of True/False+reason otherwise — computed fresh from that same +// moVM plus spec.advanced every reconcile, so it is correct whether or not +// this function also runs in a given pass. func (r reconciler) OnResult( ctx context.Context, vm *vmopv1.VirtualMachine, - moVM mo.VirtualMachine, + _ mo.VirtualMachine, resultErr error) error { if ctx == nil { @@ -239,72 +144,13 @@ func (r reconciler) OnResult( return nil } - s := ctxgen.FromContext(ctx, contextKeyValue, func(s state) state { return s }) - - // On task error, mark condition false and bail. if resultErr != nil && !pkgerr.IsNoRequeueNoError(resultErr) { conditions.MarkFalse( vm, vmopv1.VirtualMachineExtraConfigSynced, vmopv1.VirtualMachineExtraConfigErrorReason, "%v", resultErr) - return nil - } - - // Compute status.ExtraConfig: reflect what the VM currently has for user-controlled keys. - if moVM.Config != nil { - observed := pkgutil.OptionValues(moVM.Config.ExtraConfig) - sortedKeys := sortedFirstClassVMXKeys() - var statusEC []vmopv1common.KeyValuePair - for _, k := range sortedKeys { - if v, ok := observed.GetString(k); ok && v != "" { - statusEC = append(statusEC, vmopv1common.KeyValuePair{Key: k, Value: v}) - } - } - for _, k := range s.NextManagedKeys { - if v, ok := observed.GetString(k); ok && v != "" { - statusEC = append(statusEC, vmopv1common.KeyValuePair{Key: k, Value: v}) - } - } - vm.Status.ExtraConfig = statusEC - } - - // Set VirtualMachineExtraConfigSynced condition. - // - // powerCycleOnVM tracks whether a prior cycle left vmx.reboot.powerCycle=TRUE - // on the VM. When the VM is already powered off, all config has been applied - // and ESXi will clear the flag automatically on the next boot — nothing more - // is required from the user, so we treat it as synced. - vmPoweredOn := vm.Status.PowerState == vmopv1.VirtualMachinePowerStateOn - var powerCycleOnVM bool - if vmPoweredOn && moVM.Config != nil { - _, powerCycleOnVM = pkgutil.OptionValues(moVM.Config.ExtraConfig).GetString( - vsphereconst.ExtraConfigReservedKeyVMXRebootPowerCycle) - } - - switch { - case len(s.Deferred) > 0: - conditions.MarkFalse( - vm, - vmopv1.VirtualMachineExtraConfigSynced, - vmopv1.VirtualMachinePowerOffRequiredReason, - "VM power off required to apply: %s", strings.Join(s.Deferred, ", ")) - case s.PowerCyclePending || powerCycleOnVM: - conditions.MarkFalse( - vm, - vmopv1.VirtualMachineExtraConfigSynced, - vmopv1.VirtualMachinePowerCyclePendingReason, - "applied changes take effect on next power cycle") - default: - conditions.MarkTrue(vm, vmopv1.VirtualMachineExtraConfigSynced) } return nil } - -// isFirstClassKey returns true when the key maps to a vmx-tagged first-class -// field in VirtualMachineAdvancedSpec. -func isFirstClassKey(key string, firstClassKeys map[string]int) bool { - _, ok := firstClassKeys[key] - return ok -} diff --git a/pkg/vmconfig/extraconfig/reconciler_test.go b/pkg/vmconfig/extraconfig/reconciler_test.go index 40f46a7f6..ea99a2460 100644 --- a/pkg/vmconfig/extraconfig/reconciler_test.go +++ b/pkg/vmconfig/extraconfig/reconciler_test.go @@ -53,7 +53,7 @@ func telcoCtx() context.Context { } var _ = Describe("New", func() { - It("returns a non-nil ReconcilerWithContext", func() { + It("returns a non-nil Reconciler", func() { Expect(vmconfextraconfig.New()).ToNot(BeNil()) }) It("has name 'extraconfig'", func() { @@ -68,12 +68,12 @@ var _ = Describe("Reconcile", func() { vm *vmopv1.VirtualMachine moVM mo.VirtualMachine configSpec *vimtypes.VirtualMachineConfigSpec - r vmconfig.ReconcilerWithContext + r vmconfig.Reconciler ) BeforeEach(func() { r = vmconfextraconfig.New() - ctx = r.WithContext(telcoCtx()) + ctx = telcoCtx() vm = makeVM() moVM = moVMWithEC() configSpec = &vimtypes.VirtualMachineConfigSpec{} @@ -91,7 +91,6 @@ var _ = Describe("Reconcile", func() { It("no-ops when TelcoVMServiceAPI is off", func() { offCtx := vmconfig.WithContext(pkgcfg.NewContextWithDefaultConfig()) - offCtx = vmconfextraconfig.New().WithContext(offCtx) vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{PreferHTEnabled: ptr.To(true)} Expect(vmconfextraconfig.Reconcile(offCtx, nil, nil, vm, moVM, configSpec)).To(Succeed()) Expect(configSpec.ExtraConfig).To(BeEmpty()) @@ -269,30 +268,28 @@ var _ = Describe("Reconcile", func() { }) }) +// OnResult now only ever marks ExtraConfigSynced=False, and only for a real +// Reconfigure task failure. Everything else — True, PowerOffRequired, +// PowerCyclePending — is decided by reconcileStatusExtraConfig +// (pkg/providers/vsphere/vmlifecycle), computed fresh from moVM and +// spec.advanced every reconcile. See that package's "ExtraConfig status" +// tests for those cases. var _ = Describe("OnResult", func() { var ( - ctx context.Context - vm *vmopv1.VirtualMachine - moVM mo.VirtualMachine - r vmconfig.ReconcilerWithContext - configSpec *vimtypes.VirtualMachineConfigSpec + ctx context.Context + vm *vmopv1.VirtualMachine + moVM mo.VirtualMachine + r vmconfig.Reconciler ) BeforeEach(func() { r = vmconfextraconfig.New() - ctx = r.WithContext(telcoCtx()) + ctx = telcoCtx() vm = makeVM() moVM = moVMWithEC() - configSpec = &vimtypes.VirtualMachineConfigSpec{} }) - reconcileAndOnResult := func(resultErr error) { - GinkgoHelper() - Expect(r.Reconcile(ctx, nil, nil, vm, moVM, configSpec)).To(Succeed()) - Expect(r.OnResult(ctx, vm, moVM, resultErr)).To(Succeed()) - } - It("panics when ctx is nil", func() { Expect(func() { _ = vmconfextraconfig.New().OnResult(nil, vm, moVM, nil) }).To(Panic()) //nolint:staticcheck }) @@ -302,7 +299,6 @@ var _ = Describe("OnResult", func() { It("no-ops when TelcoVMServiceAPI is off", func() { offCtx := vmconfig.WithContext(pkgcfg.NewContextWithDefaultConfig()) - offCtx = vmconfextraconfig.New().WithContext(offCtx) Expect(vmconfextraconfig.New().OnResult(offCtx, vm, moVM, nil)).To(Succeed()) Expect(findCond(vm)).To(BeNil()) }) @@ -316,183 +312,16 @@ var _ = Describe("OnResult", func() { }) It("does not set Error condition on NoRequeueNoError", func() { - // NoRequeueNoError is a sentinel used for non-error stop conditions (e.g. - // snapshot revert, VM just created). OnResult must not mark ExtraConfigSynced - // false in these cases; it should proceed to evaluate the condition normally. - vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{PreferHTEnabled: ptr.To(true)} - moVM = moVMWithEC("numa.vcpu.preferHT", "TRUE") - Expect(r.Reconcile(ctx, nil, nil, vm, moVM, configSpec)).To(Succeed()) + // NoRequeueNoError is a sentinel used for non-error stop conditions + // (e.g. snapshot revert, VM just created); it must not be treated as + // a Reconfigure failure. Expect(r.OnResult(ctx, vm, moVM, pkgerr.NoRequeueNoErr("updated vm"))).To(Succeed()) - cond := findCond(vm) - Expect(cond).ToNot(BeNil()) - Expect(cond.Status).To(Equal(metav1.ConditionTrue)) - }) - - It("sets True condition when spec matches observed", func() { - vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{PreferHTEnabled: ptr.To(true)} - moVM = moVMWithEC("numa.vcpu.preferHT", "TRUE") - reconcileAndOnResult(nil) - cond := findCond(vm) - Expect(cond).ToNot(BeNil()) - Expect(cond.Status).To(Equal(metav1.ConditionTrue)) - }) - - It("sets True condition with no advanced spec", func() { - reconcileAndOnResult(nil) - cond := findCond(vm) - Expect(cond).ToNot(BeNil()) - Expect(cond.Status).To(Equal(metav1.ConditionTrue)) - }) - - It("sets PowerOffRequired when HugePages deferred on powered-on VM", func() { - vm.Status.PowerState = vmopv1.VirtualMachinePowerStateOn - vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{HugePages1GEnabled: ptr.To(true)} - moVM = moVMWithEC("sched.mem.lpage.enable1GPage", "FALSE") - reconcileAndOnResult(nil) - cond := findCond(vm) - Expect(cond).ToNot(BeNil()) - Expect(cond.Status).To(Equal(metav1.ConditionFalse)) - Expect(cond.Reason).To(Equal(vmopv1.VirtualMachinePowerOffRequiredReason)) - Expect(cond.Message).To(ContainSubstring("sched.mem.lpage.enable1GPage")) - }) - - It("sets PowerCyclePending when first-class key injected this cycle", func() { - vm.Status.PowerState = vmopv1.VirtualMachinePowerStateOn - vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{PreferHTEnabled: ptr.To(false)} - moVM = moVMWithEC("numa.vcpu.preferHT", "TRUE") - reconcileAndOnResult(nil) - cond := findCond(vm) - Expect(cond).ToNot(BeNil()) - Expect(cond.Reason).To(Equal(vmopv1.VirtualMachinePowerCyclePendingReason)) - }) - - It("persists PowerCyclePending when vmx.reboot.powerCycle already on powered-on VM", func() { - vm.Status.PowerState = vmopv1.VirtualMachinePowerStateOn - vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{PreferHTEnabled: ptr.To(true)} - moVM = moVMWithEC( - "numa.vcpu.preferHT", "TRUE", - vsphereconst.ExtraConfigReservedKeyVMXRebootPowerCycle, "TRUE", - ) - reconcileAndOnResult(nil) - cond := findCond(vm) - Expect(cond).ToNot(BeNil()) - Expect(cond.Reason).To(Equal(vmopv1.VirtualMachinePowerCyclePendingReason)) - }) - - It("sets True when vmx.reboot.powerCycle is on VM but VM is powered off", func() { - // VM is off: config fully applied, ESXi clears the flag on next boot automatically. - vm.Status.PowerState = vmopv1.VirtualMachinePowerStateOff - vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{PreferHTEnabled: ptr.To(true)} - moVM = moVMWithEC( - "numa.vcpu.preferHT", "TRUE", - vsphereconst.ExtraConfigReservedKeyVMXRebootPowerCycle, "TRUE", - ) - reconcileAndOnResult(nil) - cond := findCond(vm) - Expect(cond).ToNot(BeNil()) - Expect(cond.Status).To(Equal(metav1.ConditionTrue)) - }) - - It("status.ExtraConfig omits first-class keys not yet on the VM (observed-based)", func() { - // Spec sets preferHT=true but the VM doesn't have it yet (e.g. first apply). - vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{PreferHTEnabled: ptr.To(true)} - moVM = moVMWithEC() // preferHT not in observed yet - reconcileAndOnResult(nil) - for _, kv := range vm.Status.ExtraConfig { - Expect(kv.Key).ToNot(Equal("numa.vcpu.preferHT"), - "status should reflect observed VM, not desired") - } - }) - - It("status.ExtraConfig is sorted by key when multiple first-class keys are present", func() { - vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{ - PreferHTEnabled: ptr.To(true), - TimeTrackerLowLatencyEnabled: ptr.To(true), - CPUAffinityExclusiveNoStatsEnabled: ptr.To(true), - } - moVM = moVMWithEC( - "numa.vcpu.preferHT", "TRUE", - "timeTracker.lowLatency", "TRUE", - "sched.cpu.affinity.exclusiveNoStats", "TRUE", - ) - reconcileAndOnResult(nil) - Expect(vm.Status.ExtraConfig).To(Equal([]vmopv1common.KeyValuePair{ - {Key: "numa.vcpu.preferHT", Value: "TRUE"}, - {Key: "sched.cpu.affinity.exclusiveNoStats", Value: "TRUE"}, - {Key: "timeTracker.lowLatency", Value: "TRUE"}, - })) - }) - - It("sets PowerCyclePending when PNUMANodeAffinity changes on powered-on VM", func() { - vm.Status.PowerState = vmopv1.VirtualMachinePowerStateOn - vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{PNUMANodeAffinity: []int32{0}} - moVM = moVMWithEC("numa.nodeAffinity", "1,2") - reconcileAndOnResult(nil) - cond := findCond(vm) - Expect(cond).ToNot(BeNil()) - Expect(cond.Reason).To(Equal(vmopv1.VirtualMachinePowerCyclePendingReason)) - }) - - It("sets PowerOffRequired when clearing a PowerOff-mode key on powered-on VM", func() { - vm.Status.PowerState = vmopv1.VirtualMachinePowerStateOn - vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{HugePages1GEnabled: nil} - moVM = moVMWithEC("sched.mem.lpage.enable1GPage", "TRUE") - reconcileAndOnResult(nil) - cond := findCond(vm) - Expect(cond).ToNot(BeNil()) - Expect(cond.Status).To(Equal(metav1.ConditionFalse)) - Expect(cond.Reason).To(Equal(vmopv1.VirtualMachinePowerOffRequiredReason)) - Expect(cond.Message).To(ContainSubstring("sched.mem.lpage.enable1GPage")) - }) - - It("leaves status.ExtraConfig unchanged when moVM.Config is nil", func() { - existing := []vmopv1common.KeyValuePair{{Key: "numa.vcpu.preferHT", Value: "TRUE"}} - vm.Status.ExtraConfig = existing - vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{PreferHTEnabled: ptr.To(true)} - moVM.Config = nil - reconcileAndOnResult(nil) - // OnResult must not overwrite status.ExtraConfig when moVM.Config is nil. - Expect(vm.Status.ExtraConfig).To(Equal(existing)) - }) - - It("PowerOffRequired takes priority over PowerCyclePending when both are pending", func() { - vm.Status.PowerState = vmopv1.VirtualMachinePowerStateOn - vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{ - HugePages1GEnabled: ptr.To(true), // poweroff → deferred - VMXSwapEnabled: ptr.To(false), // powercycle → applied - } - moVM = moVMWithEC( - "sched.mem.lpage.enable1GPage", "FALSE", // HugePages: differs from spec - "sched.swap.vmxSwapEnabled", "TRUE", // VMXSwap: differs from spec - ) - reconcileAndOnResult(nil) - cond := findCond(vm) - Expect(cond).ToNot(BeNil()) - Expect(cond.Reason).To(Equal(vmopv1.VirtualMachinePowerOffRequiredReason)) - Expect(cond.Message).To(ContainSubstring("sched.mem.lpage.enable1GPage")) + Expect(findCond(vm)).To(BeNil()) }) - It("populates status.ExtraConfig with only user-controlled keys", func() { - vm.Spec.Advanced = &vmopv1.VirtualMachineAdvancedSpec{ - PreferHTEnabled: ptr.To(true), - ExtraConfig: []vmopv1common.KeyValuePair{{Key: "foo", Value: "bar"}}, - } - moVM = moVMWithEC( - "numa.vcpu.preferHT", "TRUE", - "tools.guest.desktop.autolock", "FALSE", // class-derived, not in spec - "disk.enableUUID", "TRUE", // internal - "foo", "bar", - vsphereconst.ExtraConfigManagedKeysKey, "foo", - ) - reconcileAndOnResult(nil) - statusKeys := make(map[string]string) - for _, kv := range vm.Status.ExtraConfig { - statusKeys[kv.Key] = kv.Value - } - Expect(statusKeys).To(HaveKey("numa.vcpu.preferHT")) - Expect(statusKeys).To(HaveKey("foo")) - Expect(statusKeys).ToNot(HaveKey("tools.guest.desktop.autolock")) - Expect(statusKeys).ToNot(HaveKey("disk.enableUUID")) + It("leaves the condition untouched on success", func() { + Expect(r.OnResult(ctx, vm, moVM, nil)).To(Succeed()) + Expect(findCond(vm)).To(BeNil()) }) }) diff --git a/test/e2e/vmservice/vmservice/virtualmachine/vm_extraconfig.go b/test/e2e/vmservice/vmservice/virtualmachine/vm_extraconfig.go index e4d39f04f..6facbf8fb 100644 --- a/test/e2e/vmservice/vmservice/virtualmachine/vm_extraconfig.go +++ b/test/e2e/vmservice/vmservice/virtualmachine/vm_extraconfig.go @@ -46,12 +46,6 @@ const ( vmxPNUMANodeAffinity = "numa.nodeAffinity" ) -// Condition reason values for VirtualMachineExtraConfigSynced (mirrors reconciler constants). -const ( - extraConfigReasonPowerOffRequired = "PowerOffRequired" - extraConfigReasonPowerCyclePending = "PowerCyclePending" -) - // VMExtraConfigSpecInput is the input for the ExtraConfig test spec. type VMExtraConfigSpecInput struct { ClusterProxy wcpframework.WCPClusterProxyInterface @@ -125,13 +119,9 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec storageClass = clusterResources.StorageClassName linuxImageDisplayName := vmservice.GetDefaultImageDisplayName(clusterResources) - var err error - linuxVMIName, err = vmoperator.WaitForVirtualMachineImageName( + linuxVMIName = vmoperator.WaitForVirtualMachineImageName( ctx, &config.Config, svClusterClient, input.WCPNamespaceName, linuxImageDisplayName) - Expect(err).NotTo(HaveOccurred(), - "failed to get VMI name for display name %q in namespace %q", - linuxImageDisplayName, input.WCPNamespaceName) }) // ── It block 1: phases 1-2 ──────────────────────────────────────────────── @@ -261,7 +251,7 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec By("Waiting for ExtraConfigSynced=False/PowerCyclePending") cond := waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, - metav1.ConditionFalse, extraConfigReasonPowerCyclePending) + metav1.ConditionFalse, vmopv1.VirtualMachinePowerCyclePendingReason) Expect(cond).NotTo(BeNil()) By("Power-cycling VM via spec to apply the pending change") @@ -333,11 +323,10 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec vmPatch.Spec.Advanced.HugePages1GEnabled = ptr.To(true) Expect(svClusterClient.Patch(ctx, vmPatch, ctrlclient.MergeFrom(vm))).To(Succeed(), "failed to patch VM %s HugePages1GEnabled", vmName) - vm = vmPatch By("Waiting for ExtraConfigSynced=False/PowerOffRequired") cond := waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, - metav1.ConditionFalse, extraConfigReasonPowerOffRequired) + metav1.ConditionFalse, vmopv1.VirtualMachinePowerOffRequiredReason) Expect(cond).NotTo(BeNil()) Expect(cond.Message).To(ContainSubstring(vmxHugePages), "condition message should name the deferred VMX key") @@ -396,6 +385,12 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec Advanced: advanced, }) Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to create VM %s", vmName) + DeferCleanup(func() { + if !input.SkipCleanup { + vmoperator.DeleteVirtualMachine(ctx, svClusterClient, vmKey.Namespace, vmKey.Name) + vmoperator.WaitForVirtualMachineToBeDeleted(ctx, config, svClusterClient, vmKey.Namespace, vmKey.Name) + } + }) vmoperator.WaitForVirtualMachineConditionCreated( ctx, config, svClusterClient, input.WCPNamespaceName, vmName) @@ -421,12 +416,6 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec Advanced: advanced, }) Expect(svClusterClient.Create(ctx, vm)).To(Succeed(), "failed to recreate VM %s", vmName) - DeferCleanup(func() { - if !input.SkipCleanup { - vmoperator.DeleteVirtualMachine(ctx, svClusterClient, vmKey.Namespace, vmKey.Name) - vmoperator.WaitForVirtualMachineToBeDeleted(ctx, config, svClusterClient, vmKey.Namespace, vmKey.Name) - } - }) By("Waiting for recreated VM to be created in vSphere") vmoperator.WaitForVirtualMachineConditionCreated( @@ -485,10 +474,9 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec vmPatch.Spec.Advanced.PreferHTEnabled = ptr.To(false) Expect(svClusterClient.Patch(ctx, vmPatch, ctrlclient.MergeFrom(vm))).To(Succeed(), "failed to patch VM %s PreferHTEnabled", vmName) - vm = vmPatch waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, - metav1.ConditionFalse, extraConfigReasonPowerCyclePending) + metav1.ConditionFalse, vmopv1.VirtualMachinePowerCyclePendingReason) By("Getting VM BiosUUID for vSphere lookup") biosUUID := waitForBiosUUID(ctx, svClusterClient, config, vmKey) @@ -656,11 +644,10 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec vmPatch.Spec.Advanced.PNUMANodeAffinity = nil Expect(svClusterClient.Patch(ctx, vmPatch, ctrlclient.MergeFrom(vm))).To(Succeed(), "failed to patch VM %s PNUMANodeAffinity", vmName) - vm = vmPatch By("Waiting for ExtraConfigSynced=False/PowerCyclePending") waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, - metav1.ConditionFalse, extraConfigReasonPowerCyclePending) + metav1.ConditionFalse, vmopv1.VirtualMachinePowerCyclePendingReason) By("Powering off VM to apply the pending clear") vmoperator.UpdateVirtualMachinePowerState( @@ -776,11 +763,10 @@ func VMExtraConfigSpec(ctx context.Context, inputGetter func() VMExtraConfigSpec vmPatch.Spec.Advanced.VMXSwapEnabled = ptr.To(false) Expect(svClusterClient.Patch(ctx, vmPatch, ctrlclient.MergeFrom(vm))).To(Succeed(), "failed to patch VM %s HugePages1GEnabled/VMXSwapEnabled", vmName) - vm = vmPatch By("Asserting ExtraConfigSynced=False/PowerOffRequired (PowerOff takes priority)") cond := waitForExtraConfigSynced(ctx, svClusterClient, config, vmKey, - metav1.ConditionFalse, extraConfigReasonPowerOffRequired) + metav1.ConditionFalse, vmopv1.VirtualMachinePowerOffRequiredReason) Expect(cond).NotTo(BeNil()) Expect(cond.Message).To(ContainSubstring(vmxHugePages), "condition message should name the deferred PowerOff key")