diff --git a/api/v1/hypervisor_types.go b/api/v1/hypervisor_types.go index 4af9043..aa5195f 100644 --- a/api/v1/hypervisor_types.go +++ b/api/v1/hypervisor_types.go @@ -180,7 +180,7 @@ type HypervisorSpec struct { InstallCertificate bool `json:"installCertificate"` // +kubebuilder:optional - // +kubebuilder:validation:Enum:="";manual;auto;ha;termination + // +kubebuilder:validation:Enum:="";manual;auto;ha;no-schedule;termination // Maintenance indicates whether the hypervisor is in maintenance mode. Maintenance string `json:"maintenance,omitempty"` @@ -213,6 +213,7 @@ const ( MaintenanceManual = "manual" // manual maintenance mode by external user MaintenanceAuto = "auto" // automatic maintenance mode MaintenanceHA = "ha" // high availability maintenance mode + MaintenanceNoSchedule = "no-schedule" // compute service disabled, no eviction MaintenanceTermination = "termination" // internal use only, when node is terminating state ) diff --git a/charts/openstack-hypervisor-operator/crds/hypervisor-crd.yaml b/charts/openstack-hypervisor-operator/crds/hypervisor-crd.yaml new file mode 100644 index 0000000..ed6ec21 --- /dev/null +++ b/charts/openstack-hypervisor-operator/crds/hypervisor-crd.yaml @@ -0,0 +1,554 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.19.0 + name: hypervisors.kvm.cloud.sap +spec: + group: kvm.cloud.sap + names: + kind: Hypervisor + listKind: HypervisorList + plural: hypervisors + shortNames: + - hv + singular: hypervisor + scope: Cluster + versions: + - additionalPrinterColumns: + - jsonPath: .metadata.labels.topology\.kubernetes\.io/zone + name: Zone + priority: 2 + type: string + - jsonPath: .metadata.labels.kubernetes\.metal\.cloud\.sap/bb + name: Building Block + priority: 2 + type: string + - jsonPath: .metadata.labels.worker\.garden\.sapcloud\.io/group + name: Group + priority: 2 + type: string + - jsonPath: .status.conditions[?(@.type=="Ready")].status + name: Ready + type: string + - jsonPath: .status.conditions[?(@.type=="Ready")].reason + name: State + type: string + - jsonPath: .status.conditions[?(@.type=="Tainted")].message + name: Taint + type: string + - jsonPath: .spec.lifecycleEnabled + name: Lifecycle + type: boolean + - jsonPath: .spec.highAvailability + name: High Availability + type: boolean + - jsonPath: .spec.skipTests + name: Skip Tests + type: boolean + - jsonPath: .status.operatingSystem.prettyVersion + name: Version + type: string + - jsonPath: .status.internalIp + name: IP + type: string + - jsonPath: .status.numInstances + name: Instances + type: integer + - jsonPath: .status.operatingSystem.hardwareModel + name: Hardware + priority: 2 + type: string + - jsonPath: .status.operatingSystem.kernelRelease + name: Kernel + priority: 2 + type: string + - jsonPath: .status.conditions[?(@.type=="Onboarding")].reason + name: Onboarding + priority: 3 + type: string + - jsonPath: .status.serviceId + name: Service ID + priority: 3 + type: string + - jsonPath: .status.hypervisorId + name: Hypervisor ID + priority: 3 + type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1 + schema: + openAPIV3Schema: + description: Hypervisor is the Schema for the hypervisors API + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: HypervisorSpec defines the desired state of Hypervisor + properties: + aggregates: + default: [] + description: Aggregates are used to apply aggregates to the hypervisor. + items: + type: string + type: array + allowedProjects: + default: [] + description: |- + AllowedProjects defines which openstack projects are allowed to schedule + instances on this hypervisor. The values of this list should be project + uuids. If left empty, all projects are allowed. + items: + type: string + type: array + createCertManagerCertificate: + default: false + description: |- + Require to issue a certificate from cert-manager for the hypervisor, to be used for + secure communication with the libvirt API. + type: boolean + customTraits: + default: [] + description: CustomTraits are used to apply custom traits to the hypervisor. + items: + type: string + type: array + evacuateOnReboot: + default: true + description: EvacuateOnReboot request an evacuation of all instances + before reboot. + type: boolean + highAvailability: + default: true + description: HighAvailability is used to enable the high availability + handling of the hypervisor. + type: boolean + installCertificate: + default: true + description: InstallCertificate is used to enable the installations + of the certificates via kvm-node-agent. + type: boolean + lifecycleEnabled: + default: true + description: LifecycleEnabled enables the lifecycle management of + the hypervisor via hypervisor-operator. + type: boolean + maintenance: + description: Maintenance indicates whether the hypervisor is in maintenance + mode. + enum: + - "" + - manual + - auto + - ha + - no-schedule + - termination + type: string + maintenanceReason: + description: MaintenanceReason provides the reason for manual maintenance + mode. + type: string + reboot: + default: false + description: Reboot request an reboot after successful installation + of an upgrade. + type: boolean + skipTests: + default: false + description: SkipTests skips the tests during the onboarding process. + type: boolean + version: + description: OperatingSystemVersion represents the desired operating + system version. + type: string + required: + - aggregates + - allowedProjects + - createCertManagerCertificate + - customTraits + - evacuateOnReboot + - highAvailability + - installCertificate + - lifecycleEnabled + - reboot + - skipTests + type: object + x-kubernetes-validations: + - message: spec is immutable when maintenance is 'termination'; can only + change maintenance to 'ha' + rule: '!has(oldSelf.maintenance) || oldSelf.maintenance != ''termination'' + || self.maintenance == ''ha'' || self == oldSelf' + - message: maintenanceReason must be non-empty when maintenance is 'manual' + rule: '!has(self.maintenance) || self.maintenance != ''manual'' || (has(self.maintenanceReason) + && self.maintenanceReason.size() > 0)' + status: + description: HypervisorStatus defines the observed state of Hypervisor + properties: + aggregates: + description: Aggregates are the applied aggregates of the hypervisor + with their names and UUIDs. + items: + description: Aggregate represents an OpenStack aggregate with its + name and UUID. + properties: + name: + description: Name is the name of the aggregate. + type: string + uuid: + description: UUID is the unique identifier of the aggregate. + type: string + required: + - name + - uuid + type: object + type: array + allocation: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: Auto-discovered resource allocation of all hosted VMs. + type: object + capabilities: + description: Auto-discovered capabilities as reported by libvirt. + properties: + cpuArch: + default: unknown + description: The hosts CPU architecture (not the guests). + type: string + cpus: + anyOf: + - type: integer + - type: string + description: Total host cpus available as a sum of cpus over all + numa cells. + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + memory: + anyOf: + - type: integer + - type: string + description: Total host memory available as a sum of memory over + all numa cells. + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + type: object + capacity: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: Auto-discovered capacity of the hypervisor. + type: object + cells: + description: Auto-discovered cells on this hypervisor. + items: + description: Cell represents a NUMA cell on the hypervisor. + properties: + allocation: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: Auto-discovered resource allocation of all hosted + VMs in this cell. + type: object + capacity: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: Auto-discovered capacity of this cell. + type: object + cellID: + description: Cell ID. + format: int64 + type: integer + required: + - cellID + type: object + type: array + conditions: + description: Represents the Hypervisor node conditions. + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + domainCapabilities: + description: |- + Auto-discovered domain capabilities relevant to check if a VM + can be scheduled on the hypervisor. + properties: + arch: + default: unknown + description: The available domain cpu architecture. + type: string + hypervisorType: + default: unknown + description: The supported type of virtualization for domains, + such as "ch". + type: string + supportedCpuModes: + default: [] + description: |- + Supported cpu modes for domains. + + The format of this list is cpu mode, and if specified, a specific + submode. For example, the take the following xml domain cpu definition: + + + + + + The corresponding entries in this list would be "host-passthrough" and + "host-passthrough/migratable". + items: + type: string + type: array + supportedDevices: + default: [] + description: |- + Supported devices for domains. + + The format of this list is the device type, and if specified, a specific + model. For example, the take the following xml domain device definition: + + + + The corresponding entries in this list would be "video" and "video/nvidia". + items: + type: string + type: array + supportedFeatures: + default: [] + description: |- + Supported features for domains, such as "sev" or "sgx". + + This is a flat list of supported features, meaning the following xml: + + + + + + + Would correspond to the entries "sev" and "sgx" in this list. + items: + type: string + type: array + type: object + evicted: + description: Evicted indicates whether the hypervisor is evicted. + (no instances left with active maintenance mode) + type: boolean + hypervisorId: + description: HypervisorID is the unique identifier of the hypervisor + in OpenStack. + type: string + hypervisorVersion: + default: unknown + description: Represents the Hypervisor version + type: string + instances: + description: Represents the Hypervisor hosted Virtual Machines + items: + properties: + active: + description: Represents the instance state. + type: boolean + id: + description: Represents the instance ID (uuidv4). + type: string + name: + description: Represents the instance name. + type: string + required: + - active + - id + - name + type: object + type: array + internalIp: + description: InternalIP is the internal IP address of the hypervisor. + type: string + libVirtVersion: + default: unknown + description: Represents the LibVirt version. + type: string + numInstances: + default: 0 + description: Represent the num of instances + type: integer + operatingSystem: + description: Represents the Operating System status. + properties: + firmwareDate: + description: FirmwareDate + format: date-time + type: string + firmwareVendor: + description: FirmwareVendor + type: string + firmwareVersion: + description: FirmwareVersion + type: string + gardenLinuxCommitID: + description: Represents the Garden Linux build commit id + type: string + gardenLinuxFeatures: + description: Represents the Garden Linux Feature Set + items: + type: string + type: array + hardwareModel: + description: HardwareModel + type: string + hardwareSerial: + description: HardwareSerial + type: string + hardwareVendor: + description: HardwareVendor + type: string + kernelCommandLine: + description: KernelCommandLine contains the raw kernel boot parameters + from /proc/cmdline. + type: string + kernelName: + description: KernelName + type: string + kernelRelease: + description: KernelRelease + type: string + kernelVersion: + description: KernelVersion + type: string + prettyVersion: + description: PrettyVersion + type: string + variantID: + description: Identifying a specific variant or edition of the + operating system + type: string + version: + description: Represents the Operating System version. + type: string + type: object + serviceId: + description: ServiceID is the unique identifier of the compute service + in OpenStack. + type: string + specHash: + type: string + traits: + description: Traits are the applied traits of the hypervisor. + items: + type: string + type: array + updateStatus: + description: Represents the Hypervisor update status. + properties: + inProgress: + default: false + description: Represents a running Operating System update. + type: boolean + installed: + default: unknown + description: Represents the Operating System installed update + version. + type: string + retry: + default: 3 + description: Represents the number of retries. + type: integer + required: + - inProgress + - retry + type: object + required: + - numInstances + type: object + type: object + served: true + storage: true + subresources: + status: {} + diff --git a/charts/openstack-hypervisor-operator/crds/kvm.cloud.sap_hypervisors.yaml b/charts/openstack-hypervisor-operator/crds/kvm.cloud.sap_hypervisors.yaml index e01399a..47224d7 100644 --- a/charts/openstack-hypervisor-operator/crds/kvm.cloud.sap_hypervisors.yaml +++ b/charts/openstack-hypervisor-operator/crds/kvm.cloud.sap_hypervisors.yaml @@ -222,6 +222,7 @@ spec: - manual - auto - ha + - no-schedule - termination type: string maintenanceReason: diff --git a/cmd/main.go b/cmd/main.go index a179780..840a05e 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -304,7 +304,6 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "Eviction") os.Exit(1) } - if err = (&controller.HypervisorOffboardingReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), @@ -313,6 +312,14 @@ func main() { os.Exit(1) } + if err = (&controller.HypervisorComputeServiceController{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", controller.HypervisorComputeServiceControllerName) + os.Exit(1) + } + if err = (&controller.OnboardingController{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), diff --git a/internal/controller/aggregates_controller.go b/internal/controller/aggregates_controller.go index 30aa47e..1f4b999 100644 --- a/internal/controller/aggregates_controller.go +++ b/internal/controller/aggregates_controller.go @@ -19,6 +19,7 @@ package controller import ( "context" + "encoding/json" "errors" "fmt" @@ -26,6 +27,8 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -33,6 +36,7 @@ import ( "github.com/gophercloud/gophercloud/v2" kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" + apiv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/applyconfigurations/api/v1" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/openstack" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils" ) @@ -76,17 +80,32 @@ func (ac *AggregatesController) Reconcile(ctx context.Context, req ctrl.Request) // Apply aggregates to OpenStack and update status aggregates, err := openstack.ApplyAggregates(ctx, ac.computeClient, hv.Name, desiredAggregateNames) if err != nil { - // Set error condition with retry on conflict - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeAggregatesUpdated, - Status: metav1.ConditionFalse, - Reason: kvmv1.ConditionReasonFailed, - Message: fmt.Errorf("failed to apply aggregates: %w", err).Error(), + // Set error condition, preserving current aggregate ownership + statusCfg := apiv1.HypervisorStatus() + if c := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeAggregatesUpdated); c != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*c)) + } + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeAggregatesUpdated). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonFailed). + WithMessage(fmt.Errorf("failed to apply aggregates: %w", err).Error())) + if len(hv.Status.Aggregates) > 0 { + aggCfgs := make([]apiv1.AggregateApplyConfiguration, len(hv.Status.Aggregates)) + for i, agg := range hv.Status.Aggregates { + a := apiv1.Aggregate().WithName(agg.Name).WithUUID(agg.UUID) + if len(agg.Metadata) > 0 { + a.WithMetadata(agg.Metadata) + } + aggCfgs[i] = *a + } + statusCfg.Aggregates = aggCfgs } - if err2 := utils.PatchHypervisorStatusWithRetry(ctx, ac.Client, hv.Name, AggregatesControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }); err2 != nil { + if err2 := ac.Status().Apply(ctx, + apiv1.Hypervisor(hv.Name).WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(AggregatesControllerName)); err2 != nil { return ctrl.Result{}, errors.Join(err, err2) } return ctrl.Result{}, err @@ -106,15 +125,54 @@ func (ac *AggregatesController) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, nil } - // Patch status with retry on conflict - err := utils.PatchHypervisorStatusWithRetry(ctx, ac.Client, hv.Name, AggregatesControllerName, func(h *kvmv1.Hypervisor) { - if aggregatesChanged { - h.Status.Aggregates = newAggregates + statusCfg := apiv1.HypervisorStatus() + if c := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeAggregatesUpdated); c != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*c)) + } + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(desiredCondition.Type). + WithStatus(desiredCondition.Status). + WithReason(desiredCondition.Reason). + WithMessage(desiredCondition.Message)) + + if aggregatesChanged && len(newAggregates) > 0 { + aggCfgs := make([]apiv1.AggregateApplyConfiguration, len(newAggregates)) + for i, agg := range newAggregates { + a := apiv1.Aggregate().WithName(agg.Name).WithUUID(agg.UUID) + if len(agg.Metadata) > 0 { + a.WithMetadata(agg.Metadata) + } + aggCfgs[i] = *a + } + statusCfg.Aggregates = aggCfgs + } + + if err := ac.Status().Apply(ctx, + apiv1.Hypervisor(hv.Name).WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(AggregatesControllerName)); err != nil { + return ctrl.Result{}, err + } + + // The Aggregates field uses omitempty in the generated apply config, so an + // empty slice cannot be sent via SSA. Use a targeted merge patch to clear it. + if aggregatesChanged && len(newAggregates) == 0 { + patch, err := json.Marshal(map[string]any{ + "status": map[string]any{ + "aggregates": []kvmv1.Aggregate{}, + }, + }) + if err != nil { + return ctrl.Result{}, err + } + fresh := &kvmv1.Hypervisor{} + if err := ac.Get(ctx, k8sclient.ObjectKey{Name: hv.Name}, fresh); err != nil { + return ctrl.Result{}, err } - meta.SetStatusCondition(&h.Status.Conditions, desiredCondition) - }) + return ctrl.Result{}, ac.Status().Patch(ctx, fresh, k8sclient.RawPatch(types.MergePatchType, patch)) + } - return ctrl.Result{}, err + return ctrl.Result{}, nil } // determineDesiredState returns the desired aggregates and the corresponding condition diff --git a/internal/controller/eviction_controller.go b/internal/controller/eviction_controller.go index b0fa33c..acb5c95 100644 --- a/internal/controller/eviction_controller.go +++ b/internal/controller/eviction_controller.go @@ -32,12 +32,13 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/util/retry" + k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" logger "sigs.k8s.io/controller-runtime/pkg/log" kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" + apiv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/applyconfigurations/api/v1" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/openstack" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils" ) @@ -85,6 +86,24 @@ func moveToBack(instances []string) []string { return instances } +// evictionStatusCfg builds an EvictionStatusApplyConfiguration pre-populated +// with the current scalar fields and the conditions this controller owns. +// Call sites upsert the single condition they are setting via +// utils.SetApplyConfigurationStatusCondition, then apply. +func evictionStatusCfg(eviction *kvmv1.Eviction) *apiv1.EvictionStatusApplyConfiguration { + cfg := apiv1.EvictionStatus(). + WithHypervisorServiceId(eviction.Status.HypervisorServiceId). + WithOutstandingRamMb(eviction.Status.OutstandingRamMb) + cfg.OutstandingInstances = eviction.Status.OutstandingInstances + // Seed all conditions this controller owns so they are not pruned on Apply. + for _, t := range []string{kvmv1.ConditionTypeEvicting, kvmv1.ConditionTypePreflight, kvmv1.ConditionTypeMigration} { + if c := meta.FindStatusCondition(eviction.Status.Conditions, t); c != nil { + cfg.WithConditions(utils.ConditionFromStatus(*c)) + } + } + return cfg +} + // +kubebuilder:rbac:groups=kvm.cloud.sap,resources=evictions,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=kvm.cloud.sap,resources=evictions/status,verbs=get;update;patch // +kubebuilder:rbac:groups=kvm.cloud.sap,resources=evictions/finalizers,verbs=update @@ -118,14 +137,16 @@ func (r *EvictionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c statusCondition := meta.FindStatusCondition(eviction.Status.Conditions, kvmv1.ConditionTypeEvicting) if statusCondition == nil { - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeEvicting, - Status: metav1.ConditionTrue, - Message: "Running", - Reason: kvmv1.ConditionReasonRunning, - }) - - return ctrl.Result{}, r.updateStatus(ctx, eviction) + statusCfg := evictionStatusCfg(eviction) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeEvicting). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonRunning). + WithMessage("Running")) + return ctrl.Result{}, r.Status().Apply(ctx, + apiv1.Eviction(eviction.Name).WithStatus(statusCfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } switch statusCondition.Status { @@ -157,36 +178,18 @@ func (r *EvictionReconciler) handleRunning(ctx context.Context, eviction *kvmv1. return r.evictNext(ctx, eviction) } - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeEvicting, - Status: metav1.ConditionFalse, - Message: "eviction completed successfully", - Reason: kvmv1.ConditionReasonSucceeded, - }) - - eviction.Status.OutstandingRamMb = 0 logger.FromContext(ctx).Info("succeeded") - return ctrl.Result{}, r.updateStatus(ctx, eviction) -} - -func (r *EvictionReconciler) updateStatus(ctx context.Context, eviction *kvmv1.Eviction) error { - desiredStatus := eviction.Status.DeepCopy() - return retry.RetryOnConflict(utils.StatusPatchBackoff, func() error { - freshEviction := &kvmv1.Eviction{} - if err := r.Get(ctx, client.ObjectKeyFromObject(eviction), freshEviction); err != nil { - return err - } - freshBase := freshEviction.DeepCopy() - // Apply desired conditions and scalar fields onto the fresh status - for _, c := range desiredStatus.Conditions { - meta.SetStatusCondition(&freshEviction.Status.Conditions, c) - } - freshEviction.Status.OutstandingInstances = desiredStatus.OutstandingInstances - freshEviction.Status.OutstandingRamMb = desiredStatus.OutstandingRamMb - freshEviction.Status.HypervisorServiceId = desiredStatus.HypervisorServiceId - return r.Status().Patch(ctx, freshEviction, client.MergeFromWithOptions(freshBase, - client.MergeFromWithOptimisticLock{}), client.FieldOwner(EvictionControllerName)) - }) + statusCfg := evictionStatusCfg(eviction) + statusCfg.WithOutstandingRamMb(0) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeEvicting). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonSucceeded). + WithMessage("eviction completed successfully")) + return ctrl.Result{}, r.Status().Apply(ctx, + apiv1.Eviction(eviction.Name).WithStatus(statusCfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv1.Eviction, hv *kvmv1.Hypervisor) (ctrl.Result, error) { @@ -194,14 +197,22 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv // If the hypervisor should exist, then we need to ensure it is disabled before we start evicting if expectHypervisor && !meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) { - // Hypervisor is not disabled (yet?), reflect that as a failing preflight check - if meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypePreflight, - Status: metav1.ConditionFalse, - Message: "hypervisor not disabled", - Reason: kvmv1.ConditionReasonFailed, - }) { - return ctrl.Result{}, r.updateStatus(ctx, eviction) + // Hypervisor is not disabled (yet?), reflect that as a failing preflight check. + // Only apply if the condition is new or changed, to avoid unnecessary API calls on repeated requeues. + existing := meta.FindStatusCondition(eviction.Status.Conditions, kvmv1.ConditionTypePreflight) + if existing == nil || existing.Status != metav1.ConditionFalse { + statusCfg := evictionStatusCfg(eviction) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypePreflight). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonFailed). + WithMessage("hypervisor not disabled")) + if err := r.Status().Apply(ctx, + apiv1.Eviction(eviction.Name).WithStatus(statusCfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)); err != nil { + return ctrl.Result{}, err + } } return ctrl.Result{RequeueAfter: defaultPollTime}, nil // Wait for hypervisor to be disabled } @@ -216,47 +227,56 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv if expectHypervisor { // Abort eviction - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeEvicting, - Status: metav1.ConditionFalse, - Message: fmt.Sprintf("failed to get hypervisor %v", err), - Reason: kvmv1.ConditionReasonFailed, - }) - return ctrl.Result{}, r.updateStatus(ctx, eviction) - } else { - // That is (likely) an eviction for a node that never registered - // so we are good to go - msg := "eviction completed successfully due to expected case of no hypervisor" - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeEvicting, - Status: metav1.ConditionFalse, - Message: msg, - Reason: kvmv1.ConditionReasonSucceeded, - }) - eviction.Status.OutstandingRamMb = 0 - logger.FromContext(ctx).Info(msg) - return ctrl.Result{}, r.updateStatus(ctx, eviction) + statusCfg := evictionStatusCfg(eviction) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeEvicting). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonFailed). + WithMessage(fmt.Sprintf("failed to get hypervisor %v", err))) + return ctrl.Result{}, r.Status().Apply(ctx, + apiv1.Eviction(eviction.Name).WithStatus(statusCfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } + + // That is (likely) an eviction for a node that never registered + // so we are good to go + msg := "eviction completed successfully due to expected case of no hypervisor" + logger.FromContext(ctx).Info(msg) + statusCfg := evictionStatusCfg(eviction) + statusCfg.WithOutstandingRamMb(0) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeEvicting). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonSucceeded). + WithMessage(msg)) + return ctrl.Result{}, r.Status().Apply(ctx, + apiv1.Eviction(eviction.Name).WithStatus(statusCfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } + var instances []string if hypervisor.Servers != nil { - uuids := make([]string, len(*hypervisor.Servers)) + instances = make([]string, len(*hypervisor.Servers)) for i, server := range *hypervisor.Servers { - uuids[i] = server.UUID + instances[i] = server.UUID } - eviction.Status.OutstandingInstances = uuids } - // Update status - eviction.Status.HypervisorServiceId = hypervisor.ID - eviction.Status.OutstandingRamMb = int64(hypervisor.MemoryMBUsed) - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypePreflight, - Status: metav1.ConditionTrue, - Message: "Preflight checks passed, hypervisor is disabled and ready for eviction", - Reason: kvmv1.ConditionReasonSucceeded, - }) - return ctrl.Result{}, r.updateStatus(ctx, eviction) + statusCfg := evictionStatusCfg(eviction) + statusCfg.WithHypervisorServiceId(hypervisor.ID) + statusCfg.WithOutstandingRamMb(int64(hypervisor.MemoryMBUsed)) + statusCfg.OutstandingInstances = instances + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypePreflight). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonSucceeded). + WithMessage("Preflight checks passed, hypervisor is disabled and ready for eviction")) + return ctrl.Result{}, r.Status().Apply(ctx, + apiv1.Eviction(eviction.Name).WithStatus(statusCfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } // Tries to handle the NotFound-error by updating the status @@ -265,18 +285,22 @@ func (r *EvictionReconciler) handleNotFound(ctx context.Context, eviction *kvmv1 return err } logger.FromContext(ctx).Info("Instance is gone") - var uuid string - eviction.Status.OutstandingInstances, uuid = popInstance(eviction.Status.OutstandingInstances) + remaining, uuid := popInstance(eviction.Status.OutstandingInstances) if uuid == "" { return nil } - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeMigration, - Status: metav1.ConditionFalse, - Message: fmt.Sprintf("Instance %s is gone", uuid), - Reason: kvmv1.ConditionReasonSucceeded, - }) - return r.updateStatus(ctx, eviction) + + statusCfg := evictionStatusCfg(eviction) + statusCfg.OutstandingInstances = remaining + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeMigration). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonSucceeded). + WithMessage(fmt.Sprintf("Instance %s is gone", uuid))) + return r.Status().Apply(ctx, + apiv1.Eviction(eviction.Name).WithStatus(statusCfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Eviction) (ctrl.Result, error) { @@ -309,41 +333,27 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic case "ERROR": // Needs manual intervention (or another operator fixes it) // put it at the end of the list (beginning of array) - eviction.Status.OutstandingInstances = moveToBack(eviction.Status.OutstandingInstances) + reordered := moveToBack(eviction.Status.OutstandingInstances) log.Info("error", "faultMessage", vm.Fault.Message) - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeMigration, - Status: metav1.ConditionFalse, - Message: fmt.Sprintf("Migration of instance %s failed: %s", vm.ID, vm.Fault.Message), - Reason: kvmv1.ConditionReasonFailed, - }) - - return ctrl.Result{}, errors.Join(fmt.Errorf("error migrating instance %v", uuid), - r.updateStatus(ctx, eviction)) + + statusCfg := evictionStatusCfg(eviction) + statusCfg.OutstandingInstances = reordered + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeMigration). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonFailed). + WithMessage(fmt.Sprintf("Migration of instance %s failed: %s", vm.ID, vm.Fault.Message))) + applyErr := r.Status().Apply(ctx, + apiv1.Eviction(eviction.Name).WithStatus(statusCfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)) + return ctrl.Result{}, errors.Join(fmt.Errorf("error migrating instance %v", uuid), applyErr) } currentHypervisor, _, _ := strings.Cut(vm.HypervisorHostname, ".") if currentHypervisor != eviction.Spec.Hypervisor { log.Info("migrated") - // Don't overwrite a sticky Failed migration condition with Succeeded - // while there are still other outstanding VMs - an earlier ERROR VM - // has been moved to the back of the queue and the eviction is not - // actually clean. The condition is reset only when the whole - // eviction completes (OutstandingInstances becomes empty). - remaining, _ := popInstance(eviction.Status.OutstandingInstances) - prior := meta.FindStatusCondition(eviction.Status.Conditions, kvmv1.ConditionTypeMigration) - stickyFailure := len(remaining) > 0 && prior != nil && - prior.Status == metav1.ConditionFalse && - prior.Reason == kvmv1.ConditionReasonFailed - if !stickyFailure { - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeMigration, - Status: metav1.ConditionFalse, - Message: fmt.Sprintf("Migration of instance %s finished", vm.ID), - Reason: kvmv1.ConditionReasonSucceeded, - }) - } // So, it is already off this one, do we need to verify it? if vm.Status == "VERIFY_RESIZE" { @@ -358,27 +368,54 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic } // All done - eviction.Status.OutstandingInstances, _ = popInstance(eviction.Status.OutstandingInstances) - return ctrl.Result{}, r.updateStatus(ctx, eviction) + popped, _ := popInstance(eviction.Status.OutstandingInstances) + + // Don't overwrite a sticky Failed migration condition with Succeeded + // while there are still other outstanding VMs - an earlier ERROR VM + // has been moved to the back of the queue and the eviction is not + // actually clean. The condition is reset only when the whole + // eviction completes (OutstandingInstances becomes empty). + prior := meta.FindStatusCondition(eviction.Status.Conditions, kvmv1.ConditionTypeMigration) + stickyFailure := len(popped) > 0 && prior != nil && + prior.Status == metav1.ConditionFalse && + prior.Reason == kvmv1.ConditionReasonFailed + + statusCfg := evictionStatusCfg(eviction) + statusCfg.OutstandingInstances = popped + if !stickyFailure { + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeMigration). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonSucceeded). + WithMessage(fmt.Sprintf("Migration of instance %s finished", vm.ID))) + } + return ctrl.Result{}, r.Status().Apply(ctx, + apiv1.Eviction(eviction.Name).WithStatus(statusCfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } if vm.TaskState == "deleting" { //nolint:gocritic // We just have to wait for it to be gone. Try the next one. - eviction.Status.OutstandingInstances = moveToBack(eviction.Status.OutstandingInstances) - - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeMigration, - Status: metav1.ConditionFalse, - Message: fmt.Sprintf("Live migration of terminating instance %s skipped", vm.ID), - Reason: kvmv1.ConditionReasonFailed, - }) - if err := r.updateStatus(ctx, eviction); err != nil { + reordered := moveToBack(eviction.Status.OutstandingInstances) + + statusCfg := evictionStatusCfg(eviction) + statusCfg.OutstandingInstances = reordered + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeMigration). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonFailed). + WithMessage(fmt.Sprintf("Live migration of terminating instance %s skipped", vm.ID))) + if err := r.Status().Apply(ctx, + apiv1.Eviction(eviction.Name).WithStatus(statusCfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)); err != nil { return ctrl.Result{}, fmt.Errorf("could not update status due to %w", err) } return ctrl.Result{RequeueAfter: defaultPollTime}, nil } else if vm.Status == "ACTIVE" || vm.PowerState == 1 { log.Info("trigger live-migration") - if err := r.liveMigrate(ctx, vm.ID, eviction); err != nil { + if err := r.liveMigrate(ctx, vm.ID, eviction.Spec.Hypervisor); err != nil { if err2 := r.handleNotFound(ctx, eviction, err); err2 != nil { return ctrl.Result{}, err2 } @@ -386,7 +423,7 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic } } else { log.Info("trigger cold-migration") - if err := r.coldMigrate(ctx, vm.ID, eviction); err != nil { + if err := r.coldMigrate(ctx, vm.ID, eviction.Spec.Hypervisor); err != nil { if err2 := r.handleNotFound(ctx, eviction, err); err2 != nil { return ctrl.Result{}, err2 } @@ -400,7 +437,7 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic return ctrl.Result{RequeueAfter: defaultPollTime}, nil } -func (r *EvictionReconciler) liveMigrate(ctx context.Context, uuid string, eviction *kvmv1.Eviction) error { +func (r *EvictionReconciler) liveMigrate(ctx context.Context, uuid, hypervisorName string) error { log := logger.FromContext(ctx) liveMigrateOpts := servers.LiveMigrateOpts{ @@ -409,36 +446,22 @@ func (r *EvictionReconciler) liveMigrate(ctx context.Context, uuid string, evict res := servers.LiveMigrate(ctx, r.computeClient, uuid, liveMigrateOpts) if res.Err != nil { - err := fmt.Errorf("failed to evict VM %s due to %w", uuid, res.Err) - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeMigration, - Status: metav1.ConditionFalse, - Message: err.Error(), - Reason: kvmv1.ConditionReasonFailed, - }) - return err + return fmt.Errorf("failed to evict VM %s due to %w", uuid, res.Err) } - log.Info("Live migrating server", "server", uuid, "source", eviction.Spec.Hypervisor, "X-Openstack-Request-Id", res.Header.Get("X-Openstack-Request-Id")) + log.Info("Live migrating server", "server", uuid, "source", hypervisorName, "X-Openstack-Request-Id", res.Header.Get("X-Openstack-Request-Id")) return nil } -func (r *EvictionReconciler) coldMigrate(ctx context.Context, uuid string, eviction *kvmv1.Eviction) error { +func (r *EvictionReconciler) coldMigrate(ctx context.Context, uuid, hypervisorName string) error { log := logger.FromContext(ctx) res := servers.Migrate(ctx, r.computeClient, uuid) if res.Err != nil { - err := fmt.Errorf("failed to evict stopped server %s due to %w", uuid, res.Err) - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeMigration, - Status: metav1.ConditionFalse, - Message: err.Error(), - Reason: kvmv1.ConditionReasonFailed, - }) - return err + return fmt.Errorf("failed to evict stopped server %s due to %w", uuid, res.Err) } - log.Info("Cold-migrating server", "server", uuid, "source", eviction.Spec.Hypervisor, "X-Openstack-Request-Id", res.Header.Get("X-Openstack-Request-Id")) + log.Info("Cold-migrating server", "server", uuid, "source", hypervisorName, "X-Openstack-Request-Id", res.Header.Get("X-Openstack-Request-Id")) return nil } diff --git a/internal/controller/hypervisor_compute_service_controller.go b/internal/controller/hypervisor_compute_service_controller.go new file mode 100644 index 0000000..f6f37e0 --- /dev/null +++ b/internal/controller/hypervisor_compute_service_controller.go @@ -0,0 +1,211 @@ +/* +SPDX-FileCopyrightText: Copyright 2024 SAP SE or an SAP affiliate company and cobaltcore-dev contributors +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "errors" + "fmt" + + "github.com/gophercloud/gophercloud/v2" + "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/services" + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + k8sclient "sigs.k8s.io/controller-runtime/pkg/client" + logger "sigs.k8s.io/controller-runtime/pkg/log" + + kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" + "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/openstack" +) + +const ( + HypervisorComputeServiceControllerName = "HypervisorComputeService" +) + +type HypervisorComputeServiceController struct { + k8sclient.Client + Scheme *runtime.Scheme + computeClient *gophercloud.ServiceClient +} + +// +kubebuilder:rbac:groups=kvm.cloud.sap,resources=hypervisors,verbs=get;list;watch;update;patch +// +kubebuilder:rbac:groups=kvm.cloud.sap,resources=hypervisors/status,verbs=get;list;watch;update;patch +func (r *HypervisorComputeServiceController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log := logger.FromContext(ctx).WithName(req.Name) + ctx = logger.IntoContext(ctx, log) + + hv := &kvmv1.Hypervisor{} + if err := r.Get(ctx, req.NamespacedName, hv); err != nil { + // ignore not found errors, could be deleted + return ctrl.Result{}, k8sclient.IgnoreNotFound(err) + } + + // We can only control the compute service if there is a serviceID + serviceId := hv.Status.ServiceID + if serviceId == "" { + return ctrl.Result{}, nil // Service not yet registered, nothing to do + } + + // If onboarding hasn't started yet, no compute service exists + onboardingCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) + if onboardingCondition == nil { + return ctrl.Result{}, nil // Onboarding not started, so no compute service + } + + old := hv.DeepCopy() + + // Determine if compute service should be disabled and the reason + onboardingInProgress := onboardingCondition.Status == metav1.ConditionTrue && + (onboardingCondition.Reason == kvmv1.ConditionReasonTesting || + onboardingCondition.Reason == kvmv1.ConditionReasonHandover) + maintenanceDisabling := hv.Spec.Maintenance == kvmv1.MaintenanceNoSchedule || + hv.Spec.Maintenance == kvmv1.MaintenanceManual || + hv.Spec.Maintenance == kvmv1.MaintenanceAuto || + hv.Spec.Maintenance == kvmv1.MaintenanceTermination + aborted := onboardingCondition.Status == metav1.ConditionFalse && + onboardingCondition.Reason == kvmv1.ConditionReasonAborted + + // During onboarding (testing/handover), always enable the compute service + // Otherwise, disable if maintenance mode requires it or onboarding was aborted + shouldDisable := !onboardingInProgress && (maintenanceDisabling || aborted) + + if shouldDisable { + // Determine the reason based on why compute service is being disabled + var reason string + var message string + switch { + case aborted: + reason = kvmv1.ConditionReasonAborted + message = "Compute service disabled due to aborted onboarding" + case hv.Spec.Maintenance == kvmv1.MaintenanceNoSchedule: + reason = kvmv1.ConditionReasonSucceeded + message = "Compute service disabled per spec (no-schedule)" + case hv.Spec.Maintenance == kvmv1.MaintenanceTermination: + reason = kvmv1.ConditionReasonTerminating + message = "Compute service disabled due to termination" + default: + reason = kvmv1.ConditionReasonSucceeded + message = fmt.Sprintf("Compute service disabled per spec (maintenance: %s)", hv.Spec.Maintenance) + } + + if !meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeHypervisorDisabled, + Status: metav1.ConditionTrue, + Message: message, + Reason: reason, + }) { + // Desired state already achieved + return ctrl.Result{}, nil + } + + // Update Ready condition + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeReady, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonReadyMaintenance, + Message: "Hypervisor compute service is disabled", + }) + + log.Info("Disabling compute service", "id", serviceId, "reason", reason) + opts := services.UpdateOpts{ + Status: services.ServiceDisabled, + DisabledReason: message, + } + if _, err := services.Update(ctx, r.computeClient, serviceId, opts).Extract(); err != nil { + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeHypervisorDisabled, + Status: metav1.ConditionUnknown, + Message: err.Error(), + Reason: kvmv1.ConditionReasonFailed, + }) + + if patchErr := r.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(old, k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(HypervisorComputeServiceControllerName)); patchErr != nil { + return ctrl.Result{}, errors.Join(err, patchErr) + } + return ctrl.Result{}, err + } + } else { + if !meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeHypervisorDisabled, + Status: metav1.ConditionFalse, + Message: "Compute service is enabled", + Reason: kvmv1.ConditionReasonSucceeded, + }) { + // Desired state already achieved + return ctrl.Result{}, nil + } + + // Only update Ready condition if we're not in other maintenance states + if hv.Spec.Maintenance == kvmv1.MaintenanceUnset || hv.Spec.Maintenance == kvmv1.MaintenanceHA { + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeReady, + Status: metav1.ConditionTrue, + Reason: kvmv1.ConditionReasonReadyReady, + Message: "Hypervisor is ready", + }) + } + + log.Info("Enabling compute service", "id", serviceId) + // The service may be forced down previously due to an HA event, + // so we need to ensure it not only enabled, but also not forced to be down. + falseVal := false + opts := openstack.UpdateServiceOpts{ + Status: services.ServiceEnabled, + ForcedDown: &falseVal, + } + if _, err := services.Update(ctx, r.computeClient, serviceId, opts).Extract(); err != nil { + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeHypervisorDisabled, + Status: metav1.ConditionUnknown, + Message: err.Error(), + Reason: kvmv1.ConditionReasonFailed, + }) + + if patchErr := r.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(old, k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(HypervisorComputeServiceControllerName)); patchErr != nil { + return ctrl.Result{}, errors.Join(err, patchErr) + } + return ctrl.Result{}, err + } + } + + if equality.Semantic.DeepEqual(hv, old) { + return ctrl.Result{}, nil + } + + return ctrl.Result{}, r.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(old, k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(HypervisorComputeServiceControllerName)) +} + +// SetupWithManager sets up the controller with the Manager. +func (r *HypervisorComputeServiceController) SetupWithManager(mgr ctrl.Manager) error { + ctx := context.Background() + _ = logger.FromContext(ctx) + + var err error + if r.computeClient, err = openstack.GetServiceClient(ctx, "compute", nil); err != nil { + return err + } + r.computeClient.Microversion = "2.90" // Xena (or later) + + return ctrl.NewControllerManagedBy(mgr). + Named(HypervisorComputeServiceControllerName). + For(&kvmv1.Hypervisor{}). // trigger the r.Reconcile whenever a hypervisor is created/updated/deleted. + Complete(r) +} diff --git a/internal/controller/hypervisor_compute_service_controller_test.go b/internal/controller/hypervisor_compute_service_controller_test.go new file mode 100644 index 0000000..06a74ef --- /dev/null +++ b/internal/controller/hypervisor_compute_service_controller_test.go @@ -0,0 +1,361 @@ +/* +SPDX-FileCopyrightText: Copyright 2024 SAP SE or an SAP affiliate company and cobaltcore-dev contributors +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "fmt" + "net/http" + + "github.com/gophercloud/gophercloud/v2/testhelper" + "github.com/gophercloud/gophercloud/v2/testhelper/client" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + + kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" +) + +var _ = Describe("HypervisorComputeServiceController", Label("test"), func() { + const ( + EOF = "EOF" + hostName = "test-hostname" + region = "region" + zone = "zone" + serviceId = "test-service-id" + ) + var ( + controller *HypervisorComputeServiceController + fakeServer testhelper.FakeServer + hypervisorName = types.NamespacedName{Name: "test-node"} + numApiRequest = 0 + ) + + mockApi := func(expectedStatus string, expectedReason string) { + fakeServer.Mux.HandleFunc(fmt.Sprintf("/os-services/%v", serviceId), func(w http.ResponseWriter, r *http.Request) { + numApiRequest++ + Expect(r.Method).To(Equal("PUT")) + Expect(r.Header.Get("Content-Type")).To(Equal("application/json")) + body := make([]byte, r.ContentLength) + _, err := r.Body.Read(body) + Expect(err == nil || err.Error() == EOF).To(BeTrue()) + + bodyStr := string(body) + Expect(bodyStr).To(ContainSubstring(fmt.Sprintf(`"status":"%s"`, expectedStatus))) + if expectedReason != "" { + Expect(bodyStr).To(ContainSubstring(expectedReason)) + } + + w.Header().Add("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, err = fmt.Fprintf(w, `{"service": {"id": "%v", "status": "%s"}}`, serviceId, expectedStatus) + Expect(err).NotTo(HaveOccurred()) + }) + } + + mockApiEnable := func() { + mockApi("enabled", "") + } + + mockApiDisable := func(reason string) { + mockApi("disabled", reason) + } + + BeforeEach(func(ctx SpecContext) { + fakeServer = testhelper.SetupHTTP() + numApiRequest = 0 + DeferCleanup(fakeServer.Teardown) + + controller = &HypervisorComputeServiceController{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + computeClient: client.ServiceClient(fakeServer), + } + + By("creating the hypervisor resource") + hypervisor := &kvmv1.Hypervisor{ + ObjectMeta: metav1.ObjectMeta{ + Name: hypervisorName.Name, + Labels: map[string]string{ + corev1.LabelHostname: hostName, + corev1.LabelTopologyRegion: region, + corev1.LabelTopologyZone: zone, + }, + }, + Spec: kvmv1.HypervisorSpec{ + LifecycleEnabled: true, + }, + } + Expect(k8sClient.Create(ctx, hypervisor)).To(Succeed()) + DeferCleanup(func(ctx SpecContext) { + Expect(k8sClient.Delete(ctx, hypervisor)).To(Succeed()) + }) + + By("setting service ID and onboarding to completed successfully") + hypervisor.Status.ServiceID = serviceId + meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOnboarding, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonSucceeded, + Message: "Onboarding succeeded", + }) + Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed()) + }) + + Context("Happy path", func() { + JustBeforeEach(func(ctx context.Context) { + By("reconciling twice") + for range 2 { + _, err := controller.Reconcile(ctx, ctrl.Request{NamespacedName: hypervisorName}) + Expect(err).To(Succeed()) + } + }) + + Context("with maintenance=no-schedule", func() { + BeforeEach(func(ctx SpecContext) { + hv := &kvmv1.Hypervisor{} + By("setting hv.Spec.Maintenance=no-schedule") + Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) + hv.Spec.Maintenance = kvmv1.MaintenanceNoSchedule + Expect(k8sClient.Update(ctx, hv)).To(Succeed()) + + mockApiDisable("no-schedule") + }) + + It("should have called the api server once", func() { + Expect(numApiRequest).To(Equal(1)) + }) + + It("should reflect the disabled state in the status", func(ctx SpecContext) { + hv := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) + + condition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) + Expect(condition).NotTo(BeNil()) + Expect(condition.Status).To(Equal(metav1.ConditionTrue)) + Expect(condition.Reason).To(Equal(kvmv1.ConditionReasonSucceeded)) + Expect(condition.Message).To(ContainSubstring("no-schedule")) + }) + }) + + Context("with maintenance=manual", func() { + BeforeEach(func(ctx SpecContext) { + hv := &kvmv1.Hypervisor{} + By("setting hv.Spec.Maintenance=manual") + Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) + hv.Spec.Maintenance = kvmv1.MaintenanceManual + hv.Spec.MaintenanceReason = "testing" + Expect(k8sClient.Update(ctx, hv)).To(Succeed()) + + mockApiDisable("manual") + }) + + It("should have called the api server once", func() { + Expect(numApiRequest).To(Equal(1)) + }) + + It("should reflect the disabled state in the status", func(ctx SpecContext) { + hv := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) + + condition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) + Expect(condition).NotTo(BeNil()) + Expect(condition.Status).To(Equal(metav1.ConditionTrue)) + Expect(condition.Reason).To(Equal(kvmv1.ConditionReasonSucceeded)) + }) + }) + + Context("with onboarding aborted", func() { + BeforeEach(func(ctx SpecContext) { + hv := &kvmv1.Hypervisor{} + By("setting onboarding to aborted") + Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOnboarding, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonAborted, + Message: "Onboarding aborted", + }) + Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) + + mockApiDisable("aborted") + }) + + It("should have called the api server once", func() { + Expect(numApiRequest).To(Equal(1)) + }) + + It("should reflect the disabled state in the status", func(ctx SpecContext) { + hv := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) + + condition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) + Expect(condition).NotTo(BeNil()) + Expect(condition.Status).To(Equal(metav1.ConditionTrue)) + Expect(condition.Reason).To(Equal(kvmv1.ConditionReasonAborted)) + }) + }) + + Context("with onboarding in testing phase", func() { + BeforeEach(func(ctx SpecContext) { + hv := &kvmv1.Hypervisor{} + By("setting onboarding to testing (in progress)") + Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOnboarding, + Status: metav1.ConditionTrue, + Reason: kvmv1.ConditionReasonTesting, + Message: "Testing in progress", + }) + Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) + + mockApiEnable() + }) + + It("should have called the api server once", func() { + Expect(numApiRequest).To(Equal(1)) + }) + + It("should reflect the enabled state in the status", func(ctx SpecContext) { + hv := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) + + condition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) + Expect(condition).NotTo(BeNil()) + Expect(condition.Status).To(Equal(metav1.ConditionFalse)) + Expect(condition.Reason).To(Equal(kvmv1.ConditionReasonSucceeded)) + }) + }) + + Context("with onboarding in handover phase", func() { + BeforeEach(func(ctx SpecContext) { + hv := &kvmv1.Hypervisor{} + By("setting onboarding to handover (ready for completion)") + Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOnboarding, + Status: metav1.ConditionTrue, + Reason: kvmv1.ConditionReasonHandover, + Message: "Handover in progress", + }) + Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) + + mockApiEnable() + }) + + It("should have called the api server once", func() { + Expect(numApiRequest).To(Equal(1)) + }) + + It("should reflect the enabled state in the status", func(ctx SpecContext) { + hv := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) + + condition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) + Expect(condition).NotTo(BeNil()) + Expect(condition.Status).To(Equal(metav1.ConditionFalse)) + Expect(condition.Reason).To(Equal(kvmv1.ConditionReasonSucceeded)) + }) + }) + + Context("with compute service enabled (default)", func() { + BeforeEach(func() { + mockApiEnable() + }) + + It("should have called the api server once", func() { + Expect(numApiRequest).To(Equal(1)) + }) + + It("should reflect the enabled state in the status", func(ctx SpecContext) { + hv := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) + + condition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) + Expect(condition).NotTo(BeNil()) + Expect(condition.Status).To(Equal(metav1.ConditionFalse)) + Expect(condition.Reason).To(Equal(kvmv1.ConditionReasonSucceeded)) + Expect(condition.Message).To(Equal("Compute service is enabled")) + }) + }) + }) + + Context("Failure modes", func() { + JustBeforeEach(func(ctx context.Context) { + By("reconciling once (expecting error)") + //nolint:errcheck // Ignoring error - failure tests expect errors + controller.Reconcile(ctx, ctrl.Request{NamespacedName: hypervisorName}) + }) + + Context("when enabling compute service fails", func() { + BeforeEach(func(ctx SpecContext) { + // Mock API to return error + fakeServer.Mux.HandleFunc(fmt.Sprintf("/os-services/%v", serviceId), func(w http.ResponseWriter, r *http.Request) { + numApiRequest++ + w.WriteHeader(http.StatusInternalServerError) + _, err := fmt.Fprint(w, `{"error": "internal server error"}`) + Expect(err).NotTo(HaveOccurred()) + }) + }) + + It("should reflect the error in status with Unknown condition", func(ctx SpecContext) { + hv := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) + + condition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) + Expect(condition).NotTo(BeNil()) + Expect(condition.Status).To(Equal(metav1.ConditionUnknown)) + Expect(condition.Reason).To(Equal(kvmv1.ConditionReasonFailed)) + Expect(numApiRequest).To(BeNumerically(">", 0)) + }) + }) + + Context("when disabling compute service fails", func() { + BeforeEach(func(ctx SpecContext) { + hv := &kvmv1.Hypervisor{} + By("setting hv.Spec.Maintenance=no-schedule") + Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) + hv.Spec.Maintenance = kvmv1.MaintenanceNoSchedule + Expect(k8sClient.Update(ctx, hv)).To(Succeed()) + + // Mock API to return error + fakeServer.Mux.HandleFunc(fmt.Sprintf("/os-services/%v", serviceId), func(w http.ResponseWriter, r *http.Request) { + numApiRequest++ + w.WriteHeader(http.StatusInternalServerError) + _, err := fmt.Fprint(w, `{"error": "internal server error"}`) + Expect(err).NotTo(HaveOccurred()) + }) + }) + + It("should reflect the error in status with Unknown condition", func(ctx SpecContext) { + hv := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) + + condition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) + Expect(condition).NotTo(BeNil()) + Expect(condition.Status).To(Equal(metav1.ConditionUnknown)) + Expect(condition.Reason).To(Equal(kvmv1.ConditionReasonFailed)) + Expect(numApiRequest).To(BeNumerically(">", 0)) + }) + }) + }) +}) diff --git a/internal/controller/hypervisor_controller.go b/internal/controller/hypervisor_controller.go index d174eb6..c673092 100644 --- a/internal/controller/hypervisor_controller.go +++ b/internal/controller/hypervisor_controller.go @@ -34,6 +34,7 @@ import ( "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" ctrl "sigs.k8s.io/controller-runtime" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -41,6 +42,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" + apiv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/applyconfigurations/api/v1" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/global" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils" ) @@ -105,27 +107,13 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request) } // continue with creation } else { - // First, propagate spec/metadata derived from the Node (labels, - // annotations -> aggregates/traits, lifecycle). This must run on - // every reconcile, including those where status will also change - // (e.g. AgentPodsEvicted=False during termination); otherwise the - // Hypervisor spec/labels go stale. - specBase := hypervisor.DeepCopy() - syncLabelsAndAnnotations(nodeLabels, hypervisor, node) - if !equality.Semantic.DeepEqual(hypervisor, specBase) { - if err := hv.Patch(ctx, hypervisor, k8sclient.MergeFromWithOptions(specBase, - k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(HypervisorControllerName)); err != nil { - return ctrl.Result{}, err - } - } - - // Then, compute and persist any status changes derived from the Node. - statusBase := hypervisor.DeepCopy() + // update Status if needed // transfer internal IP + var newInternalIP string for _, address := range node.Status.Addresses { - if address.Type == corev1.NodeInternalIP && hypervisor.Status.InternalIP != address.Address { - hypervisor.Status.InternalIP = address.Address + if address.Type == corev1.NodeInternalIP { + newInternalIP = address.Address break } } @@ -133,14 +121,31 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request) // update terminating condition — fired by either the legacy gardener // node condition or the node's deletion timestamp (gardener's future path) nodeTerminationCondition := FindNodeStatusCondition(node.Status.Conditions, "Terminating") + + // Capture values to apply - only mutate fields this controller owns. + // Only set InternalIP when non-empty: an empty string combined with + // ForceOwnership would overwrite a valid previously-stored IP during + // transient node states (kubelet restart, bootstrap) where no + // NodeInternalIP address is present yet. + statusCfg := apiv1.HypervisorStatus() + if newInternalIP != "" { + statusCfg.WithInternalIP(newInternalIP) + } + // Seed the conditions this controller owns so they are not pruned. + for _, t := range []string{kvmv1.ConditionTypeTerminating, kvmv1.ConditionTypeAgentPodsEvicted} { + if c := meta.FindStatusCondition(hypervisor.Status.Conditions, t); c != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*c)) + } + } + // Node might be terminating, propagate condition to hypervisor if (nodeTerminationCondition != nil && nodeTerminationCondition.Status == corev1.ConditionTrue) || !node.DeletionTimestamp.IsZero() { - meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeTerminating, - Status: metav1.ConditionTrue, - Reason: kvmv1.ConditionReasonTerminating, - Message: "Node is terminating", - }) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeTerminating). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonTerminating). + WithMessage("Node is terminating")) } // Only evaluate after VM eviction; a spurious True on a fresh node @@ -153,28 +158,39 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request) if err != nil { return ctrl.Result{}, fmt.Errorf("failed to compute %s condition: %w", kvmv1.ConditionTypeAgentPodsEvicted, err) } - meta.SetStatusCondition(&hypervisor.Status.Conditions, cond) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(cond.Type). + WithStatus(cond.Status). + WithReason(cond.Reason). + WithMessage(cond.Message)) if cond.Status == metav1.ConditionFalse { // No pod watch — rely on periodic requeue. statusRequeueAfter = defaultPollTime } } - if !equality.Semantic.DeepEqual(hypervisor, statusBase) { - // Capture values to apply - only mutate fields this controller owns - newInternalIP := hypervisor.Status.InternalIP - terminatingCondition := meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeTerminating) - agentPodsCondition := meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeAgentPodsEvicted) + if err := hv.Status().Apply(ctx, + apiv1.Hypervisor(hypervisor.Name).WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(HypervisorControllerName)); err != nil { + return ctrl.Result{}, err + } - if err := utils.PatchHypervisorStatusWithRetry(ctx, hv.Client, hypervisor.Name, HypervisorControllerName, func(h *kvmv1.Hypervisor) { - h.Status.InternalIP = newInternalIP - if terminatingCondition != nil { - meta.SetStatusCondition(&h.Status.Conditions, *terminatingCondition) - } - if agentPodsCondition != nil { - meta.SetStatusCondition(&h.Status.Conditions, *agentPodsCondition) - } - }); err != nil { + // Re-fetch after status apply so the spec patch has a fresh resourceVersion + if err := hv.Get(ctx, k8sclient.ObjectKeyFromObject(hypervisor), hypervisor); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to re-fetch hypervisor after status apply: %w", err) + } + + // First, propagate spec/metadata derived from the Node (labels, + // annotations -> aggregates/traits, lifecycle). This must run on + // every reconcile, including those where status will also change + // (e.g. AgentPodsEvicted=False during termination); otherwise the + // Hypervisor spec/labels go stale. + base := hypervisor.DeepCopy() + syncLabelsAndAnnotations(nodeLabels, hypervisor, node) + if !equality.Semantic.DeepEqual(hypervisor, base) { + if err := hv.Patch(ctx, hypervisor, k8sclient.MergeFromWithOptions(base, + k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(HypervisorControllerName)); err != nil { return ctrl.Result{}, err } } diff --git a/internal/controller/hypervisor_maintenance_controller.go b/internal/controller/hypervisor_maintenance_controller.go index 5c82487..aa49e99 100644 --- a/internal/controller/hypervisor_maintenance_controller.go +++ b/internal/controller/hypervisor_maintenance_controller.go @@ -17,28 +17,22 @@ limitations under the License. package controller -// This controller only takes care of enabling or disabling the compute -// service depending on the hypervisor spec Maintenance field +// This controller takes care of triggering evictions based on the maintenance field import ( "context" "fmt" - "k8s.io/apimachinery/pkg/api/equality" - k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" ctrl "sigs.k8s.io/controller-runtime" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" logger "sigs.k8s.io/controller-runtime/pkg/log" - "github.com/gophercloud/gophercloud/v2" - "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/services" - kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" - "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/openstack" + apiv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/applyconfigurations/api/v1" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils" ) @@ -48,8 +42,7 @@ const ( type HypervisorMaintenanceController struct { k8sclient.Client - Scheme *runtime.Scheme - computeClient *gophercloud.ServiceClient + Scheme *runtime.Scheme } // +kubebuilder:rbac:groups=kvm.cloud.sap,resources=hypervisors,verbs=get;list;watch @@ -58,7 +51,6 @@ type HypervisorMaintenanceController struct { func (hec *HypervisorMaintenanceController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { hv := &kvmv1.Hypervisor{} if err := hec.Get(ctx, req.NamespacedName, hv); err != nil { - // OnboardingReconciler not found errors, could be deleted return ctrl.Result{}, k8sclient.IgnoreNotFound(err) } @@ -69,116 +61,46 @@ func (hec *HypervisorMaintenanceController) Reconcile(ctx context.Context, req c return ctrl.Result{}, nil } - old := hv.DeepCopy() - - if err := hec.reconcileComputeService(ctx, hv); err != nil { - return ctrl.Result{}, err + // Build status apply config upfront; sub-functions mutate it directly. + // Seed only the HypervisorDisabled condition (the only one always retained). + // reconcileEviction conditionally seeds ConditionTypeEvicting: it is included + // when maintenance is active, and intentionally omitted when MaintenanceUnset + // so that SSA prunes it from the field manager's managed fields. + statusCfg := apiv1.HypervisorStatus().WithEvicted(hv.Status.Evicted) + if c := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled); c != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*c)) } - if err := hec.reconcileEviction(ctx, hv); err != nil { + if err := hec.reconcileEviction(ctx, hv, statusCfg); err != nil { return ctrl.Result{}, err } - if equality.Semantic.DeepEqual(hv, old) { - return ctrl.Result{}, nil - } - - // Capture only the fields this controller owns - disabledCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) - evictingCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeEvicting) - evicted := hv.Status.Evicted - - return ctrl.Result{}, utils.PatchHypervisorStatusWithRetry(ctx, hec.Client, hv.Name, HypervisorMaintenanceControllerName, func(h *kvmv1.Hypervisor) { - if disabledCondition != nil { - meta.SetStatusCondition(&h.Status.Conditions, *disabledCondition) - } - if evictingCondition != nil { - meta.SetStatusCondition(&h.Status.Conditions, *evictingCondition) - } else { - meta.RemoveStatusCondition(&h.Status.Conditions, kvmv1.ConditionTypeEvicting) - } - h.Status.Evicted = evicted - }) + return ctrl.Result{}, hec.Status().Apply(ctx, + apiv1.Hypervisor(hv.Name).WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(HypervisorMaintenanceControllerName)) } -func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context.Context, hv *kvmv1.Hypervisor) error { - log := logger.FromContext(ctx) - serviceId := hv.Status.ServiceID - - // We can only do something here, if there is a service to begin with. - // The onboarding should take care of that - if serviceId == "" { - return nil - } - - switch hv.Spec.Maintenance { - case kvmv1.MaintenanceUnset: - // Enable the compute service (in case we haven't done so already) - if !meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeHypervisorDisabled, - Status: metav1.ConditionFalse, - Message: "Hypervisor is enabled", - Reason: kvmv1.ConditionReasonSucceeded, - }) { - // Spec matches status - return nil - } - - // We need to enable the host as per spec and clear forced_down - // in case it was set by the HA service during maintenance. - falseVal := false - enableService := openstack.UpdateServiceOpts{ - Status: services.ServiceEnabled, - ForcedDown: &falseVal, - } - log.Info("Enabling hypervisor", "id", serviceId) - _, err := services.Update(ctx, hec.computeClient, serviceId, enableService).Extract() - if err != nil { - return fmt.Errorf("failed to enable hypervisor due to %w", err) - } - case kvmv1.MaintenanceManual, kvmv1.MaintenanceAuto, kvmv1.MaintenanceTermination: - // Disable the compute service. - if !meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeHypervisorDisabled, - Status: metav1.ConditionTrue, - Message: "Hypervisor is disabled", - Reason: kvmv1.ConditionReasonSucceeded, - }) { - // Spec matches status - return nil - } - - // We need to disable the host as per spec - enableService := services.UpdateOpts{ - Status: services.ServiceDisabled, - DisabledReason: "Hypervisor CRD: spec.maintenance=" + hv.Spec.Maintenance, - } - log.Info("Disabling hypervisor", "id", serviceId) - _, err := services.Update(ctx, hec.computeClient, serviceId, enableService).Extract() - if err != nil { - return fmt.Errorf("failed to disable hypervisor due to %w", err) - } - } - - return nil -} - -func (hec *HypervisorMaintenanceController) reconcileEviction(ctx context.Context, hv *kvmv1.Hypervisor) error { +// reconcileEviction creates/deletes the Eviction CR and sets the ConditionTypeEvicting +// condition and Evicted scalar on statusCfg. When eviction should be removed, the +// condition entry is filtered out so SSA prunes it. +func (hec *HypervisorMaintenanceController) reconcileEviction(ctx context.Context, hv *kvmv1.Hypervisor, statusCfg *apiv1.HypervisorStatusApplyConfiguration) error { eviction := &kvmv1.Eviction{ - ObjectMeta: metav1.ObjectMeta{ - Name: hv.Name, - }, + ObjectMeta: metav1.ObjectMeta{Name: hv.Name}, } switch hv.Spec.Maintenance { - case kvmv1.MaintenanceUnset: + case kvmv1.MaintenanceUnset, kvmv1.MaintenanceNoSchedule: // Avoid deleting the eviction over and over. - if hv.Status.Evicted || meta.RemoveStatusCondition(&hv.Status.Conditions, kvmv1.ConditionTypeEvicting) { - err := k8sclient.IgnoreNotFound(hec.Delete(ctx, eviction)) - hv.Status.Evicted = false + if !hv.Status.Evicted && meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeEvicting) == nil { + return nil + } + if err := k8sclient.IgnoreNotFound(hec.Delete(ctx, eviction)); err != nil { return err } - return nil + // ConditionTypeEvicting is intentionally absent from statusCfg — SSA + // will prune it from this field manager's managed fields on Apply. + statusCfg.WithEvicted(false) + case kvmv1.MaintenanceManual, kvmv1.MaintenanceAuto, kvmv1.MaintenanceTermination: // In case of "ha", the host gets emptied from the HA service if cond := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeEvicting); cond != nil { @@ -186,31 +108,32 @@ func (hec *HypervisorMaintenanceController) reconcileEviction(ctx context.Contex // We are done here, no need to look at the eviction any more return nil } + // Seed the existing evicting condition so it is not pruned + statusCfg.WithConditions(utils.ConditionFromStatus(*cond)) } + status, err := hec.ensureEviction(ctx, eviction, hv) if err != nil { return err } - var reason, message string + var reason, message string if status == metav1.ConditionFalse { message = "Evicted" reason = kvmv1.ConditionReasonSucceeded - hv.Status.Evicted = true + statusCfg.WithEvicted(true) } else { message = "Evicting" reason = kvmv1.ConditionReasonRunning - hv.Status.Evicted = false + statusCfg.WithEvicted(false) } - meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeEvicting, - Status: status, - Reason: reason, - Message: message, - }) - - return nil + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeEvicting). + WithStatus(status). + WithReason(reason). + WithMessage(message)) } return nil @@ -218,33 +141,46 @@ func (hec *HypervisorMaintenanceController) reconcileEviction(ctx context.Contex func (hec *HypervisorMaintenanceController) ensureEviction(ctx context.Context, eviction *kvmv1.Eviction, hypervisor *kvmv1.Hypervisor) (metav1.ConditionStatus, error) { log := logger.FromContext(ctx) - if err := hec.Get(ctx, k8sclient.ObjectKeyFromObject(eviction), eviction); err != nil { - if !k8serrors.IsNotFound(err) { - return metav1.ConditionUnknown, fmt.Errorf("failed to get eviction due to %w", err) - } - if err := controllerutil.SetControllerReference(hypervisor, eviction, hec.Scheme); err != nil { - return metav1.ConditionUnknown, err - } - log.Info("Creating new eviction", "name", eviction.Name) - eviction.Spec = kvmv1.EvictionSpec{ - Hypervisor: hypervisor.Name, - Reason: "openstack-hypervisor-operator maintenance", + + // Build labels to transport from hypervisor (e.g. label-selector, if set) + evictionLabels := make(map[string]string) + for _, label := range transferLabels { + if v, ok := hypervisor.Labels[label]; ok { + evictionLabels[label] = v } + } - // This also transports the label-selector, if set - transportLabels(&hypervisor.ObjectMeta, &eviction.ObjectMeta) + ownerRef := k8sacmetav1.OwnerReference(). + WithAPIVersion(kvmv1.GroupVersion.String()). + WithKind("Hypervisor"). + WithName(hypervisor.Name). + WithUID(hypervisor.UID). + WithController(true). + WithBlockOwnerDeletion(true) + + evictionApplyCfg := apiv1.Eviction(eviction.Name). + WithLabels(evictionLabels). + WithOwnerReferences(ownerRef). + WithSpec(apiv1.EvictionSpec(). + WithHypervisor(hypervisor.Name). + WithReason("openstack-hypervisor-operator maintenance")) + + log.Info("Applying eviction", "name", eviction.Name) + if err := hec.Apply(ctx, evictionApplyCfg, + k8sclient.ForceOwnership, k8sclient.FieldOwner(HypervisorMaintenanceControllerName)); err != nil { + return metav1.ConditionUnknown, fmt.Errorf("failed to apply eviction due to %w", err) + } - if err = hec.Create(ctx, eviction); err != nil { - return metav1.ConditionUnknown, fmt.Errorf("failed to create eviction due to %w", err) - } + // Re-fetch to read current eviction status + if err := hec.Get(ctx, k8sclient.ObjectKeyFromObject(eviction), eviction); err != nil { + return metav1.ConditionUnknown, fmt.Errorf("failed to get eviction status due to %w", err) } // check if we are still evicting (defaulting to yes) if meta.IsStatusConditionFalse(eviction.Status.Conditions, kvmv1.ConditionTypeEvicting) { return metav1.ConditionFalse, nil - } else { - return metav1.ConditionTrue, nil } + return metav1.ConditionTrue, nil } // registerWithManager registers the controller with the Manager without acquiring OpenStack clients. @@ -259,13 +195,5 @@ func (hec *HypervisorMaintenanceController) registerWithManager(mgr ctrl.Manager // SetupWithManager sets up the controller with the Manager. func (hec *HypervisorMaintenanceController) SetupWithManager(mgr ctrl.Manager) error { - ctx := context.Background() - - var err error - if hec.computeClient, err = openstack.GetServiceClient(ctx, "compute", nil); err != nil { - return err - } - hec.computeClient.Microversion = "2.90" // Xena (or later) - return hec.registerWithManager(mgr) } diff --git a/internal/controller/hypervisor_maintenance_controller_test.go b/internal/controller/hypervisor_maintenance_controller_test.go index bdbe615..e7b9724 100644 --- a/internal/controller/hypervisor_maintenance_controller_test.go +++ b/internal/controller/hypervisor_maintenance_controller_test.go @@ -22,7 +22,6 @@ import ( "net/http" "github.com/gophercloud/gophercloud/v2/testhelper" - "github.com/gophercloud/gophercloud/v2/testhelper/client" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/meta" @@ -85,9 +84,8 @@ var _ = Describe("HypervisorMaintenanceController", func() { By("Creating the HypervisorMaintenanceController") controller = &HypervisorMaintenanceController{ - Client: k8sClient, - Scheme: k8sClient.Scheme(), - computeClient: client.ServiceClient(fakeServer), + Client: k8sClient, + Scheme: k8sClient.Scheme(), } By("Creating a blank Hypervisor resource") @@ -141,64 +139,6 @@ var _ = Describe("HypervisorMaintenanceController", func() { Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed()) }) - Describe("Enabling or Disabling the Nova Service", func() { - Context("Spec.Maintenance=\"\"", func() { - BeforeEach(func(ctx SpecContext) { - hypervisor := &kvmv1.Hypervisor{} - Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) - hypervisor.Spec.Maintenance = "" - Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed()) - expectedBody := `{"status": "enabled", "forced_down": false}` - mockServiceUpdate(expectedBody) - }) - - It("should set the ConditionTypeHypervisorDisabled to false", func(ctx SpecContext) { - updated := &kvmv1.Hypervisor{} - Expect(k8sClient.Get(ctx, hypervisorName, updated)).To(Succeed()) - Expect(meta.IsStatusConditionFalse(updated.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled)).To(BeTrue()) - }) - }) // Spec.Maintenance="" - }) - - for _, mode := range []string{"auto", "manual"} { - Context(fmt.Sprintf("Spec.Maintenance=\"%v\"", mode), func() { - BeforeEach(func(ctx SpecContext) { - hypervisor := &kvmv1.Hypervisor{} - Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) - hypervisor.Spec.Maintenance = mode - if mode == "manual" { - hypervisor.Spec.MaintenanceReason = "Test maintenance reason" - } - Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed()) - expectedBody := fmt.Sprintf(`{"disabled_reason": "Hypervisor CRD: spec.maintenance=%v", "status": "disabled"}`, mode) - mockServiceUpdate(expectedBody) - }) - - It("should set the ConditionTypeHypervisorDisabled to true", func(ctx SpecContext) { - updated := &kvmv1.Hypervisor{} - Expect(k8sClient.Get(ctx, hypervisorName, updated)).To(Succeed()) - Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled)).To(BeTrue()) - }) - }) // Spec.Maintenance="" - } - - Context("Spec.Maintenance=\"ha\"", func() { - BeforeEach(func(ctx SpecContext) { - hypervisor := &kvmv1.Hypervisor{} - Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) - hypervisor.Spec.Maintenance = kvmv1.MaintenanceHA - Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed()) - }) - - It("should not call the Nova API (kvm-ha-service owns enable/disable for ha mode)", func(ctx SpecContext) { - // The controller takes no action for ha mode; kvm-ha-service handles it. - // Verify the hypervisor is still accessible and no condition is set by this controller. - updated := &kvmv1.Hypervisor{} - Expect(k8sClient.Get(ctx, hypervisorName, updated)).To(Succeed()) - Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled)).To(BeFalse()) - }) - }) // Spec.Maintenance="ha" - Describe("Eviction reconciliation", func() { Context("Spec.Maintenance=\"\"", func() { BeforeEach(func(ctx SpecContext) { diff --git a/internal/controller/hypervisor_taint_controller.go b/internal/controller/hypervisor_taint_controller.go index 7822b34..d15fc6e 100644 --- a/internal/controller/hypervisor_taint_controller.go +++ b/internal/controller/hypervisor_taint_controller.go @@ -20,16 +20,17 @@ package controller import ( "context" - "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/predicate" kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" + apiv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/applyconfigurations/api/v1" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils" ) @@ -46,52 +47,47 @@ type HypervisorTaintController struct { // +kubebuilder:rbac:groups=kvm.cloud.sap,resources=hypervisors/status,verbs=get;update;patch func (r *HypervisorTaintController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - hypervisor := &kvmv1.Hypervisor{ - ObjectMeta: metav1.ObjectMeta{ - Name: req.Name, - Labels: map[string]string{}, - }, - Spec: kvmv1.HypervisorSpec{ - HighAvailability: true, - InstallCertificate: true, - }, - } - - // Check if hypervisor already exists + hypervisor := &kvmv1.Hypervisor{} if err := r.Get(ctx, req.NamespacedName, hypervisor); err != nil { return ctrl.Result{}, k8sclient.IgnoreNotFound(err) } - before := hypervisor.DeepCopy() + var condStatus metav1.ConditionStatus + var reason, message string if HasKubectlManagedFields(&hypervisor.ObjectMeta) { - meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeTainted, - Status: metav1.ConditionTrue, - Reason: "Kubectl", - Message: "⚠️", - ObservedGeneration: hypervisor.Generation, - }) + condStatus = metav1.ConditionTrue + reason = "Kubectl" + message = "⚠️" } else { - meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeTainted, - Status: metav1.ConditionFalse, - Reason: "NoKubectl", - Message: "🟢", - ObservedGeneration: hypervisor.Generation, - }) + condStatus = metav1.ConditionFalse + reason = "NoKubectl" + message = "🟢" } - if equality.Semantic.DeepEqual(hypervisor, before) { + // Skip if already at the desired state + existing := meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeTainted) + if existing != nil && existing.Status == condStatus && + existing.Reason == reason && existing.Message == message && + existing.ObservedGeneration == hypervisor.Generation { return ctrl.Result{}, nil } // Only set the Tainted condition this controller owns - taintedCondition := meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeTainted) - return ctrl.Result{}, utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hypervisor.Name, HypervisorTaintControllerName, func(h *kvmv1.Hypervisor) { - if taintedCondition != nil { - meta.SetStatusCondition(&h.Status.Conditions, *taintedCondition) - } - }) + statusCfg := apiv1.HypervisorStatus() + if c := meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeTainted); c != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*c)) + } + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeTainted). + WithStatus(condStatus). + WithReason(reason). + WithMessage(message). + WithObservedGeneration(hypervisor.Generation)) + + return ctrl.Result{}, r.Status().Apply(ctx, + apiv1.Hypervisor(hypervisor.Name).WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(HypervisorTaintControllerName)) } func (r *HypervisorTaintController) SetupWithManager(mgr ctrl.Manager) error { diff --git a/internal/controller/node_certificate_controller.go b/internal/controller/node_certificate_controller.go index b8da375..8287c48 100644 --- a/internal/controller/node_certificate_controller.go +++ b/internal/controller/node_certificate_controller.go @@ -24,14 +24,14 @@ import ( "time" cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" - cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" + cmapplyv1 "github.com/cert-manager/cert-manager/pkg/client/applyconfigurations/certmanager/v1" + cmapplymetav1 "github.com/cert-manager/cert-manager/pkg/client/applyconfigurations/meta/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/util/retry" + k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" ctrl "sigs.k8s.io/controller-runtime" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" logger "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" ) @@ -59,95 +59,77 @@ func (r *NodeCertificateController) ensureCertificate(ctx context.Context, node secretName, certName := getSecretAndCertName(node.Name) - certificate := &cmapi.Certificate{ - TypeMeta: metav1.TypeMeta{ - Kind: cmapi.CertificateKind, - APIVersion: cmapi.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: certName, - Namespace: r.namespace, - }, - } - - update, err := controllerutil.CreateOrUpdate(ctx, r.Client, certificate, func() error { - if err := controllerutil.SetOwnerReference(node, certificate, r.Scheme); err != nil { - return err - } - - ipAddressSet := make(map[string]bool) - dnsNameSet := make(map[string]bool) - - dnsNameSet[computeHost] = true + ipAddressSet := make(map[string]bool) + dnsNameSet := make(map[string]bool) - for _, addr := range node.Status.Addresses { - if addr.Address == "" { - continue - } + dnsNameSet[computeHost] = true - switch addr.Type { - case corev1.NodeHostName, corev1.NodeInternalDNS, corev1.NodeExternalDNS: - dnsNameSet[addr.Address] = true - case corev1.NodeInternalIP, corev1.NodeExternalIP: - ipAddressSet[addr.Address] = true - } + for _, addr := range node.Status.Addresses { + if addr.Address == "" { + continue } - ipAddresses := make([]string, 0, len(ipAddressSet)) - for k := range ipAddressSet { - ipAddresses = append(ipAddresses, k) - } - - slices.Sort(ipAddresses) - - dnsNames := make([]string, 0, len(dnsNameSet)) - for k := range dnsNameSet { - dnsNames = append(dnsNames, k) + switch addr.Type { + case corev1.NodeHostName, corev1.NodeInternalDNS, corev1.NodeExternalDNS: + dnsNameSet[addr.Address] = true + case corev1.NodeInternalIP, corev1.NodeExternalIP: + ipAddressSet[addr.Address] = true } + } - slices.Sort(dnsNames) + ipAddresses := make([]string, 0, len(ipAddressSet)) + for k := range ipAddressSet { + ipAddresses = append(ipAddresses, k) + } + slices.Sort(ipAddresses) - certificate.Spec = cmapi.CertificateSpec{ - SecretName: secretName, - PrivateKey: &cmapi.CertificatePrivateKey{ - Algorithm: cmapi.RSAKeyAlgorithm, - Encoding: cmapi.PKCS1, - Size: 4096, - }, + dnsNames := make([]string, 0, len(dnsNameSet)) + for k := range dnsNameSet { + dnsNames = append(dnsNames, k) + } + slices.Sort(dnsNames) + + ownerRef := k8sacmetav1.OwnerReference(). + WithAPIVersion(corev1.SchemeGroupVersion.String()). + WithKind("Node"). + WithName(node.Name). + WithUID(node.UID) + + certApplyCfg := cmapplyv1.Certificate(certName, r.namespace). + WithOwnerReferences(ownerRef). + WithSpec(cmapplyv1.CertificateSpec(). + WithSecretName(secretName). + WithPrivateKey(cmapplyv1.CertificatePrivateKey(). + WithAlgorithm(cmapi.RSAKeyAlgorithm). + WithEncoding(cmapi.PKCS1). + WithSize(4096)). // Matching the CA/Browser Forum's maximum duration for 2029 - Duration: &metav1.Duration{Duration: 47 * 24 * time.Hour}, - RenewBefore: &metav1.Duration{Duration: 37 * 24 * time.Hour}, - IsCA: false, - Usages: []cmapi.KeyUsage{ + WithDuration(metav1.Duration{Duration: 47 * 24 * time.Hour}). + WithRenewBefore(metav1.Duration{Duration: 37 * 24 * time.Hour}). + WithIsCA(false). + WithUsages( cmapi.UsageServerAuth, cmapi.UsageClientAuth, cmapi.UsageCertSign, // Really? cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment, - }, - Subject: &cmapi.X509Subject{ - Organizations: []string{"nova"}, - }, - CommonName: computeHost, - DNSNames: dnsNames, - IPAddresses: ipAddresses, - IssuerRef: cmmeta.IssuerReference{ - Name: r.issuerName, - Kind: cmapi.IssuerKind, - Group: cmapi.SchemeGroupVersion.Group, - }, - } - return nil - }) - - if err != nil { + ). + WithSubject(cmapplyv1.X509Subject(). + WithOrganizations("nova")). + WithCommonName(computeHost). + WithDNSNames(dnsNames...). + WithIPAddresses(ipAddresses...). + WithIssuerRef(cmapplymetav1.IssuerReference(). + WithName(r.issuerName). + WithKind(cmapi.IssuerKind). + WithGroup(cmapi.SchemeGroupVersion.Group))) + + if err := r.Apply(ctx, certApplyCfg, + k8sclient.ForceOwnership, k8sclient.FieldOwner(NodeCertificateControllerName)); err != nil { return err } - if update != controllerutil.OperationResultNone { - log.Info(fmt.Sprintf("Certificate %s %s", certName, update)) - } - + log.Info("Applied Certificate " + certName) return nil } @@ -166,12 +148,8 @@ func (r *NodeCertificateController) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, k8sclient.IgnoreNotFound(err) } - err := retry.RetryOnConflict(retry.DefaultRetry, func() error { - return r.ensureCertificate(ctx, node, node.Name) - }) - - if err != nil { - return ctrl.Result{}, fmt.Errorf("could not create certificate: %w", err) + if err := r.ensureCertificate(ctx, node, node.Name); err != nil { + return ctrl.Result{}, fmt.Errorf("could not apply certificate: %w", err) } return ctrl.Result{}, nil diff --git a/internal/controller/offboarding_controller.go b/internal/controller/offboarding_controller.go index eee2cfe..776aa93 100644 --- a/internal/controller/offboarding_controller.go +++ b/internal/controller/offboarding_controller.go @@ -29,11 +29,13 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" ctrl "sigs.k8s.io/controller-runtime" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" logger "sigs.k8s.io/controller-runtime/pkg/log" kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" + apiv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/applyconfigurations/api/v1" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/openstack" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils" ) @@ -157,15 +159,23 @@ func (r *HypervisorOffboardingReconciler) Reconcile(ctx context.Context, req ctr return ctrl.Result{}, r.markOffboarded(ctx, hv) } +func (r *HypervisorOffboardingReconciler) applyStatus(ctx context.Context, hv *kvmv1.Hypervisor, cond *k8sacmetav1.ConditionApplyConfiguration) error { + statusCfg := apiv1.HypervisorStatus() + if c := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOffboarded); c != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*c)) + } + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, *cond) + return r.Status().Apply(ctx, + apiv1.Hypervisor(hv.Name).WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(OffboardingControllerName)) +} + func (r *HypervisorOffboardingReconciler) setOffboardingCondition(ctx context.Context, hv *kvmv1.Hypervisor, message string) (ctrl.Result, error) { - err := utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OffboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeOffboarded, - Status: metav1.ConditionFalse, - Reason: "Offboarding", - Message: message, - }) - }) + err := r.applyStatus(ctx, hv, k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOffboarded). + WithStatus(metav1.ConditionFalse). + WithReason("Offboarding"). + WithMessage(message)) if err != nil { return ctrl.Result{}, fmt.Errorf("cannot update hypervisor status due to %w", err) } @@ -173,14 +183,11 @@ func (r *HypervisorOffboardingReconciler) setOffboardingCondition(ctx context.Co } func (r *HypervisorOffboardingReconciler) markOffboarded(ctx context.Context, hv *kvmv1.Hypervisor) error { - err := utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OffboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeOffboarded, - Status: metav1.ConditionTrue, - Reason: "Offboarded", - Message: "Offboarding successful", - }) - }) + err := r.applyStatus(ctx, hv, k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOffboarded). + WithStatus(metav1.ConditionTrue). + WithReason("Offboarded"). + WithMessage("Offboarding successful")) if err != nil { return fmt.Errorf("cannot update hypervisor status due to %w", err) } diff --git a/internal/controller/onboarding_controller.go b/internal/controller/onboarding_controller.go index 1f52d0e..1509bfa 100644 --- a/internal/controller/onboarding_controller.go +++ b/internal/controller/onboarding_controller.go @@ -26,11 +26,11 @@ import ( "time" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" "k8s.io/utils/clock" ctrl "sigs.k8s.io/controller-runtime" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -45,6 +45,7 @@ import ( "github.com/gophercloud/utils/v2/openstack/clientconfig" kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" + apiv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/applyconfigurations/api/v1" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/openstack" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils" ) @@ -95,110 +96,110 @@ func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request) computeHost := hv.Name + // Build the status apply config upfront; sub-functions mutate it. + // HypervisorID and ServiceID are always retained so they are never pruned. + statusCfg := apiv1.HypervisorStatus(). + WithHypervisorID(hv.Status.HypervisorID). + WithServiceID(hv.Status.ServiceID) + // Seed the Onboarding condition this controller owns so it is not pruned. + if c := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding); c != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*c)) + } + + apply := func() error { + return r.Status().Apply(ctx, + apiv1.Hypervisor(hv.Name).WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(OnboardingControllerName)) + } + // check if lifecycle management is enabled if !hv.Spec.LifecycleEnabled { - return ctrl.Result{}, r.abortOnboarding(ctx, hv, computeHost, "Aborted due to LifecycleEnabled being false") + return ctrl.Result{}, r.abortOnboarding(ctx, hv, computeHost, "Aborted due to LifecycleEnabled being false", statusCfg, apply) } // check if hv is terminating if hv.Spec.Maintenance == kvmv1.MaintenanceTermination { - return ctrl.Result{}, r.abortOnboarding(ctx, hv, computeHost, "Aborted due to MaintenanceTermination") + return ctrl.Result{}, r.abortOnboarding(ctx, hv, computeHost, "Aborted due to MaintenanceTermination", statusCfg, apply) } // We bail here out, because the openstack api is not the best to poll if hv.Status.HypervisorID == "" || hv.Status.ServiceID == "" { - if err := r.ensureNovaProperties(ctx, hv); err != nil { + hypervisorID, serviceID, err := r.lookupNovaProperties(ctx, hv) + if err != nil { if errors.Is(err, errRequeue) { return ctrl.Result{RequeueAfter: r.getRequeueInterval()}, nil } return ctrl.Result{}, err } + statusCfg.WithHypervisorID(hypervisorID).WithServiceID(serviceID) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonInitial). + WithMessage("Initial onboarding")) + return ctrl.Result{}, apply() } // check condition reason status := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) - if status == nil { - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionTrue, - Reason: kvmv1.ConditionReasonInitial, - Message: "Initial onboarding", - } - return ctrl.Result{}, utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }) - } - switch status.Reason { case kvmv1.ConditionReasonInitial: - return ctrl.Result{}, r.initialOnboarding(ctx, hv) + return ctrl.Result{}, r.initialOnboarding(ctx, hv, statusCfg, apply) case kvmv1.ConditionReasonTesting: if hv.Spec.SkipTests { - return r.completeOnboarding(ctx, computeHost, hv) + return r.completeOnboarding(ctx, computeHost, hv, statusCfg, apply) } else { - return r.smokeTest(ctx, hv, computeHost) + return r.smokeTest(ctx, hv, computeHost, statusCfg, apply) } case kvmv1.ConditionReasonHandover: - return r.completeOnboarding(ctx, computeHost, hv) + return r.completeOnboarding(ctx, computeHost, hv, statusCfg, apply) default: // Nothing to be done return ctrl.Result{}, nil } } -func (r *OnboardingController) abortOnboarding(ctx context.Context, hv *kvmv1.Hypervisor, computeHost, message string) error { +func (r *OnboardingController) abortOnboarding(ctx context.Context, hv *kvmv1.Hypervisor, computeHost, message string, statusCfg *apiv1.HypervisorStatusApplyConfiguration, apply func() error) error { status := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) // Never onboarded if status == nil { return nil } - base := hv.DeepCopy() - - meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionFalse, - Reason: kvmv1.ConditionReasonAborted, - Message: message, - }) - - if equality.Semantic.DeepEqual(hv, base) { - // Already aborted + // Already aborted with the same message — nothing to do + if status.Status == metav1.ConditionFalse && + status.Reason == kvmv1.ConditionReasonAborted && + status.Message == message { return nil } - condition := *meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) if err := r.deleteTestServers(ctx, computeHost); err != nil { - condition = metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionTrue, // No cleanup, so we are still "onboarding" - Reason: kvmv1.ConditionReasonAborted, - Message: err.Error(), - } - - meta.SetStatusCondition(&hv.Status.Conditions, condition) - if equality.Semantic.DeepEqual(hv, base) { - return err - } - - return errors.Join(err, utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - })) - } - - return utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionTrue). // No cleanup, so we are still "onboarding" + WithReason(kvmv1.ConditionReasonAborted). + WithMessage(err.Error())) + return errors.Join(err, apply()) + } + + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonAborted). + WithMessage(message)) + return apply() } -func (r *OnboardingController) initialOnboarding(ctx context.Context, hv *kvmv1.Hypervisor) error { +func (r *OnboardingController) initialOnboarding(ctx context.Context, hv *kvmv1.Hypervisor, statusCfg *apiv1.HypervisorStatusApplyConfiguration, apply func() error) error { zone, found := hv.Labels[corev1.LabelTopologyZone] if !found || zone == "" { return fmt.Errorf("cannot find availability-zone label %v on node", corev1.LabelTopologyZone) } // Wait for aggregates controller to apply the desired state (zone and test aggregate) - // Extract aggregate names for comparison currentAggregateNames := make([]string, len(hv.Status.Aggregates)) for i, agg := range hv.Status.Aggregates { currentAggregateNames[i] = agg.Name @@ -220,18 +221,16 @@ func (r *OnboardingController) initialOnboarding(ctx context.Context, hv *kvmv1. return result.Err } - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionTrue, - Reason: kvmv1.ConditionReasonTesting, - Message: "Running onboarding tests", - } - return utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonTesting). + WithMessage("Running onboarding tests")) + return apply() } -func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervisor, host string) (ctrl.Result, error) { +func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervisor, host string, statusCfg *apiv1.HypervisorStatusApplyConfiguration, apply func() error) (ctrl.Result, error) { log := logger.FromContext(ctx) zone := hv.Labels[corev1.LabelTopologyZone] server, err := r.createOrGetTestServer(ctx, zone, host, hv.UID) @@ -239,27 +238,28 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis return ctrl.Result{}, fmt.Errorf("failed to create or get test instance: %w", err) } + setTestingCondition := func(message string) error { + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonTesting). + WithMessage(message)) + return apply() + } + switch server.Status { case "ERROR": // servers.List doesn't provide the fault field, so fetch the server again id := server.ID server, err = servers.Get(ctx, r.testComputeClient, id).Extract() if err != nil { - // should not happened log.Error(err, "failed to get test instance, instance vanished", "id", id) return ctrl.Result{RequeueAfter: r.getRequeueInterval()}, nil } // Set condition back to testing - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionTrue, - Reason: kvmv1.ConditionReasonTesting, - Message: "Server ended up in error state: " + server.Fault.Message, - } - if err = utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }); err != nil { + if err = setTestingCondition("Server ended up in error state: " + server.Fault.Message); err != nil { return ctrl.Result{}, err } @@ -268,38 +268,23 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis log.Error(err, "failed to delete test instance", "id", id) } - return ctrl.Result{RequeueAfter: r.getRequeueInterval()}, nil + return ctrl.Result{RequeueAfter: defaultWaitTime}, nil + case "ACTIVE": consoleOutput, err := servers. ShowConsoleOutput(ctx, r.testComputeClient, server.ID, servers.ShowConsoleOutputOpts{Length: 11}). Extract() if err != nil { - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionTrue, - Reason: kvmv1.ConditionReasonTesting, - Message: fmt.Sprintf("could not get console output %v", err), - } - if err := utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }); err != nil { - return ctrl.Result{}, err + if err2 := setTestingCondition(fmt.Sprintf("could not get console output %v", err)); err2 != nil { + return ctrl.Result{}, err2 } return ctrl.Result{RequeueAfter: r.getRequeueInterval()}, nil } if !strings.Contains(consoleOutput, server.Name) { if !server.LaunchedAt.IsZero() && r.Clock.Now().After(server.LaunchedAt.Add(smokeTestTimeout)) { - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionTrue, - Reason: kvmv1.ConditionReasonTesting, - Message: fmt.Sprintf("timeout waiting for console output since %v", server.LaunchedAt), - } - if err := utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }); err != nil { - return ctrl.Result{}, err + if err2 := setTestingCondition(fmt.Sprintf("timeout waiting for console output since %v", server.LaunchedAt)); err2 != nil { + return ctrl.Result{}, err2 } if err = servers.Delete(ctx, r.testComputeClient, server.ID).ExtractErr(); err != nil { if !gophercloud.ResponseCodeIs(err, http.StatusNotFound) { @@ -311,28 +296,20 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis } if err = servers.Delete(ctx, r.testComputeClient, server.ID).ExtractErr(); err != nil { - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionTrue, - Reason: kvmv1.ConditionReasonTesting, - Message: fmt.Sprintf("failed to terminate instance %v", err), - } - if err := utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }); err != nil { - return ctrl.Result{}, err + if err2 := setTestingCondition(fmt.Sprintf("failed to terminate instance %v", err)); err2 != nil { + return ctrl.Result{}, err2 } return ctrl.Result{RequeueAfter: r.getRequeueInterval()}, nil } - return r.completeOnboarding(ctx, host, hv) + return r.completeOnboarding(ctx, host, hv, statusCfg, apply) + default: return ctrl.Result{RequeueAfter: r.getRequeueInterval()}, nil } } -func (r *OnboardingController) completeOnboarding(ctx context.Context, host string, hv *kvmv1.Hypervisor) (ctrl.Result, error) { - // Check if we're in the RemovingTestAggregate phase +func (r *OnboardingController) completeOnboarding(ctx context.Context, host string, hv *kvmv1.Hypervisor, statusCfg *apiv1.HypervisorStatusApplyConfiguration, apply func() error) (ctrl.Result, error) { onboardingCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) if onboardingCondition != nil && onboardingCondition.Reason == kvmv1.ConditionReasonHandover { // We're waiting for aggregates and traits controllers to sync @@ -343,49 +320,45 @@ func (r *OnboardingController) completeOnboarding(ctx context.Context, host stri return ctrl.Result{RequeueAfter: r.getRequeueInterval()}, nil } - // Wait for HypervisorInstanceHa controller to enable HA - if hv.Spec.HighAvailability && !meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeHaEnabled) { + // Wait for HypervisorComputeService controller to enable the compute service + if !meta.IsStatusConditionFalse(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) { return ctrl.Result{}, nil } - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionFalse, - Reason: kvmv1.ConditionReasonSucceeded, - Message: "Onboarding completed", + // Wait for HypervisorInstanceHa controller to enable HA + if hv.Spec.HighAvailability && !meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeHaEnabled) { + return ctrl.Result{}, nil } - return ctrl.Result{}, utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonSucceeded). + WithMessage("Onboarding completed")) + return ctrl.Result{}, apply() } // First time in completeOnboarding - clean up and prepare for aggregate sync - err := r.deleteTestServers(ctx, host) - if err != nil { - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionTrue, // No cleanup, so we are still "onboarding" - Reason: kvmv1.ConditionReasonAborted, - Message: err.Error(), - } - return ctrl.Result{}, errors.Join(err, utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - })) + if err := r.deleteTestServers(ctx, host); err != nil { + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionTrue). // No cleanup, so we are still "onboarding" + WithReason(kvmv1.ConditionReasonAborted). + WithMessage(err.Error())) + return ctrl.Result{}, errors.Join(err, apply()) } // Mark onboarding as almost complete, triggers other controllers to do their part - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionTrue, - Reason: kvmv1.ConditionReasonHandover, - Message: "Waiting for other controllers to take over", - } - // Patch status to signal aggregates controller - return ctrl.Result{}, utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonHandover). + WithMessage("Waiting for other controllers to take over")) + return ctrl.Result{}, apply() } func (r *OnboardingController) deleteTestServers(ctx context.Context, host string) error { @@ -417,10 +390,10 @@ func (r *OnboardingController) deleteTestServers(ctx context.Context, host strin return errors.Join(errs...) } -func (r *OnboardingController) ensureNovaProperties(ctx context.Context, hv *kvmv1.Hypervisor) error { +func (r *OnboardingController) lookupNovaProperties(ctx context.Context, hv *kvmv1.Hypervisor) (hypervisorID, serviceID string, err error) { hypervisorAddress := hv.Labels[corev1.LabelHostname] if hypervisorAddress == "" { - return errRequeue + return "", "", errRequeue } shortHypervisorAddress := strings.SplitN(hypervisorAddress, ".", 2)[0] @@ -429,41 +402,27 @@ func (r *OnboardingController) ensureNovaProperties(ctx context.Context, hv *kvm hypervisorPages, err := hypervisors.List(r.computeClient, hypervisorQuery).AllPages(ctx) if err != nil { if gophercloud.ResponseCodeIs(err, http.StatusNotFound) { - return errRequeue + return "", "", errRequeue } - return err + return "", "", err } hs, err := hypervisors.ExtractHypervisors(hypervisorPages) if err != nil { - return err + return "", "", err } if len(hs) < 1 { - return errRequeue + return "", "", errRequeue } - var found = false - var myHypervisor hypervisors.Hypervisor for _, h := range hs { - short := strings.SplitN(h.HypervisorHostname, ".", 2)[0] - if short == shortHypervisorAddress { - myHypervisor = h - found = true - break + if strings.SplitN(h.HypervisorHostname, ".", 2)[0] == shortHypervisorAddress { + return h.ID, h.Service.ID, nil } } - if !found { - return fmt.Errorf("could not find exact match for %v", shortHypervisorAddress) - } - - hypervisorID := myHypervisor.ID - serviceID := myHypervisor.Service.ID - return utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - h.Status.HypervisorID = hypervisorID - h.Status.ServiceID = serviceID - }) + return "", "", fmt.Errorf("could not find exact match for %v", shortHypervisorAddress) } func (r *OnboardingController) createOrGetTestServer(ctx context.Context, zone, computeHost string, nodeUid types.UID) (*servers.Server, error) { @@ -484,9 +443,9 @@ func (r *OnboardingController) createOrGetTestServer(ctx context.Context, zone, } log := logger.FromContext(ctx) + var foundServer *servers.Server // Cleanup all other server with the same test prefix, except for the exact match // as the cleanup after onboarding may leak resources - var foundServer *servers.Server for _, server := range serverList { // The query is a substring search, we are looking for a prefix if !strings.HasPrefix(server.Name, serverPrefix) { diff --git a/internal/controller/onboarding_controller_test.go b/internal/controller/onboarding_controller_test.go index 8c0003d..1f9a730 100644 --- a/internal/controller/onboarding_controller_test.go +++ b/internal/controller/onboarding_controller_test.go @@ -454,6 +454,16 @@ var _ = Describe("Onboarding Controller", func() { }) Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) + By("Simulating HypervisorComputeService controller enabling compute service") + Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed()) + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeHypervisorDisabled, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonSucceeded, + Message: "Compute service is enabled", + }) + Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) + By("Reconciling again to call completeOnboarding and set RemovingTestAggregate") err = reconcileLoop(ctx, 1) Expect(err).NotTo(HaveOccurred()) @@ -533,6 +543,16 @@ var _ = Describe("Onboarding Controller", func() { }) Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) + By("Simulating HypervisorComputeService controller enabling compute service") + Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed()) + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeHypervisorDisabled, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonSucceeded, + Message: "Compute service is enabled", + }) + Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) + By("Simulating aggregates controller setting condition after removing test aggregate") Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed()) hv.Status.Aggregates = []kvmv1.Aggregate{ diff --git a/internal/controller/ready/controller.go b/internal/controller/ready/controller.go index 813d583..5ef66fb 100644 --- a/internal/controller/ready/controller.go +++ b/internal/controller/ready/controller.go @@ -24,6 +24,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -32,6 +33,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" + apiv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/applyconfigurations/api/v1" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils" ) @@ -104,20 +106,23 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, k8sclient.IgnoreNotFound(err) } - base := hv.DeepCopy() - // Compute Ready condition based on other conditions readyCondition := ComputeReadyCondition(hv) - meta.SetStatusCondition(&hv.Status.Conditions, readyCondition) - - if equality.Semantic.DeepEqual(hv.Status, base.Status) { - return ctrl.Result{}, nil - } log.Info("Updating Ready condition", "status", readyCondition.Status, "reason", readyCondition.Reason) - return ctrl.Result{}, utils.PatchHypervisorStatusWithRetry(ctx, r.Client, req.Name, ControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, readyCondition) - }) + statusCfg := apiv1.HypervisorStatus() + if c := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeReady); c != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*c)) + } + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(readyCondition.Type). + WithStatus(readyCondition.Status). + WithReason(readyCondition.Reason). + WithMessage(readyCondition.Message)) + return ctrl.Result{}, r.Status().Apply(ctx, + apiv1.Hypervisor(hv.Name).WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(ControllerName)) } // ComputeReadyCondition determines the Ready condition based on other conditions diff --git a/internal/controller/traits_controller.go b/internal/controller/traits_controller.go index 4e91e21..40c84d0 100644 --- a/internal/controller/traits_controller.go +++ b/internal/controller/traits_controller.go @@ -27,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" ctrl "sigs.k8s.io/controller-runtime" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -34,6 +35,7 @@ import ( "github.com/gophercloud/gophercloud/v2/openstack/placement/v1/resourceproviders" kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" + apiv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/applyconfigurations/api/v1" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/openstack" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils" ) @@ -103,11 +105,8 @@ func (tc *TraitsController) Reconcile(ctx context.Context, req ctrl.Request) (ct // fetch current traits, to ensure we don't add duplicates current, err := resourceproviders.GetTraits(ctx, tc.serviceClient, hv.Status.HypervisorID).Extract() if err != nil { - condition := getTraitCondition(err, "Failed to get current traits from placement") - patchErr := utils.PatchHypervisorStatusWithRetry(ctx, tc.Client, hv.Name, TraitsControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }) - return ctrl.Result{}, errors.Join(err, patchErr) + return ctrl.Result{}, errors.Join(err, + tc.applyTraitsStatus(ctx, hv, hv.Status.Traits, getTraitCondition(err, "Failed to get current traits from placement"))) } var targetTraits []string @@ -131,22 +130,33 @@ func (tc *TraitsController) Reconcile(ctx context.Context, req ctrl.Request) (ct ResourceProviderGeneration: current.ResourceProviderGeneration, Traits: targetTraits, }) - err = result.Err - if err != nil { - condition := getTraitCondition(err, "Failed to update traits in placement") - patchErr := utils.PatchHypervisorStatusWithRetry(ctx, tc.Client, hv.Name, TraitsControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }) - return ctrl.Result{}, errors.Join(err, patchErr) + if result.Err != nil { + return ctrl.Result{}, errors.Join(result.Err, + tc.applyTraitsStatus(ctx, hv, hv.Status.Traits, getTraitCondition(result.Err, "Failed to update traits in placement"))) } } // update status unconditionally, since we want always to propagate the current traits - err = utils.PatchHypervisorStatusWithRetry(ctx, tc.Client, hv.Name, TraitsControllerName, func(h *kvmv1.Hypervisor) { - h.Status.Traits = targetTraits - meta.SetStatusCondition(&h.Status.Conditions, getTraitCondition(nil, "Traits successfully updated")) - }) - return ctrl.Result{}, err + return ctrl.Result{}, tc.applyTraitsStatus(ctx, hv, targetTraits, getTraitCondition(nil, "Traits successfully updated")) +} + +func (tc *TraitsController) applyTraitsStatus(ctx context.Context, hv *kvmv1.Hypervisor, traits []string, cond metav1.Condition) error { + statusCfg := apiv1.HypervisorStatus() + if c := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeTraitsUpdated); c != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*c)) + } + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(cond.Type). + WithStatus(cond.Status). + WithReason(cond.Reason). + WithMessage(cond.Message)) + if traits != nil { + statusCfg.WithTraits(traits...) + } + return tc.Status().Apply(ctx, + apiv1.Hypervisor(hv.Name).WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(TraitsControllerName)) } // getTraitCondition creates a Condition object for trait updates diff --git a/internal/utils/conditions.go b/internal/utils/conditions.go new file mode 100644 index 0000000..d4f4a89 --- /dev/null +++ b/internal/utils/conditions.go @@ -0,0 +1,138 @@ +/* +SPDX-FileCopyrightText: Copyright 2024 SAP SE or an SAP affiliate company and cobaltcore-dev contributors +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package utils + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" +) + +// ConditionFromStatus converts a single metav1.Condition into a +// *k8sacmetav1.ConditionApplyConfiguration, preserving all fields verbatim +// (including LastTransitionTime and ObservedGeneration). +// +// Controllers must seed their apply-config payload with the conditions they +// own before mutating them. With +listType=map / +listMapKey=type on the CRD, +// SSA merges conditions by the "type" key: conditions owned by other field +// managers are left untouched, and conditions previously written by this +// field manager that are absent from the payload are pruned. Therefore each +// controller should include all conditions it has previously set — and only +// those — in every Apply call. +func ConditionFromStatus(c metav1.Condition) *k8sacmetav1.ConditionApplyConfiguration { + return &k8sacmetav1.ConditionApplyConfiguration{ + Type: &c.Type, + Status: &c.Status, + Reason: &c.Reason, + Message: &c.Message, + LastTransitionTime: &c.LastTransitionTime, + ObservedGeneration: &c.ObservedGeneration, + } +} + +// SetApplyConfigurationStatusCondition sets the corresponding condition in +// conditions to newCondition and returns true if the conditions are changed by +// this call. It mirrors the behaviour of k8s.io/apimachinery/pkg/api/meta.SetStatusCondition: +// +// 1. If a condition of the specified type does not exist, newCondition is +// appended. LastTransitionTime is set to now if not provided. +// 2. If a condition of the specified type already exists, all fields are +// updated. LastTransitionTime is updated to now (or the provided value) +// only when Status changes; otherwise the existing LastTransitionTime is +// preserved. +func SetApplyConfigurationStatusCondition(conditions *[]k8sacmetav1.ConditionApplyConfiguration, newCondition k8sacmetav1.ConditionApplyConfiguration) (changed bool) { + if conditions == nil { + return false + } + if newCondition.Type == nil || *newCondition.Type == "" { + return false + } + + // Find existing entry by type + var existing *k8sacmetav1.ConditionApplyConfiguration + for i := range *conditions { + if (*conditions)[i].Type != nil && *(*conditions)[i].Type == *newCondition.Type { + existing = &(*conditions)[i] + break + } + } + + if existing == nil { + // New condition: set LastTransitionTime if not provided + if newCondition.LastTransitionTime == nil { + now := metav1.Now() + newCondition.LastTransitionTime = &now + } + *conditions = append(*conditions, newCondition) + return true + } + + // Existing condition: update fields, handle LastTransitionTime + statusChanged := existing.Status == nil || newCondition.Status == nil || + *existing.Status != *newCondition.Status + + if statusChanged { + existing.Status = newCondition.Status + if newCondition.LastTransitionTime != nil { + existing.LastTransitionTime = newCondition.LastTransitionTime + } else { + now := metav1.Now() + existing.LastTransitionTime = &now + } + changed = true + } + // When status is unchanged, LastTransitionTime is intentionally preserved. + + if strChanged(&existing.Reason, newCondition.Reason) { + existing.Reason = newCondition.Reason + changed = true + } + if strChanged(&existing.Message, newCondition.Message) { + existing.Message = newCondition.Message + changed = true + } + if int64Changed(&existing.ObservedGeneration, newCondition.ObservedGeneration) { + existing.ObservedGeneration = newCondition.ObservedGeneration + changed = true + } + + return changed +} + +// strChanged reports whether the pointer value of b differs from the current +// value at a. +func strChanged(a **string, b *string) bool { + if *a == nil && b == nil { + return false + } + if *a == nil || b == nil { + return true + } + return **a != *b +} + +// int64Changed reports whether the pointer value of b differs from the current +// value at a. +func int64Changed(a **int64, b *int64) bool { + if *a == nil && b == nil { + return false + } + if *a == nil || b == nil { + return true + } + return **a != *b +} diff --git a/internal/utils/status_patch.go b/internal/utils/status_patch.go deleted file mode 100644 index e353f87..0000000 --- a/internal/utils/status_patch.go +++ /dev/null @@ -1,55 +0,0 @@ -/* -SPDX-FileCopyrightText: Copyright 2025 SAP SE or an SAP affiliate company and cobaltcore-dev contributors -SPDX-License-Identifier: Apache-2.0 - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package utils - -import ( - "context" - "time" - - "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/client-go/util/retry" - k8sclient "sigs.k8s.io/controller-runtime/pkg/client" - - kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" -) - -// StatusPatchBackoff is a retry backoff for status patches that may conflict -// with other controllers. Uses exponential backoff with more retries than the -// default to handle high contention scenarios. -var StatusPatchBackoff = wait.Backoff{ - Steps: 10, - Duration: 50 * time.Millisecond, - Factor: 2.0, - Jitter: 0.2, -} - -// PatchHypervisorStatusWithRetry patches hypervisor status with retry on conflict. -// The updateFn receives a fresh copy of the hypervisor and should apply status changes to it. -// It re-fetches the resource before each retry attempt to get the latest resourceVersion. -func PatchHypervisorStatusWithRetry(ctx context.Context, c k8sclient.Client, name, fieldOwner string, updateFn func(*kvmv1.Hypervisor)) error { - return retry.RetryOnConflict(StatusPatchBackoff, func() error { - hv := &kvmv1.Hypervisor{} - if err := c.Get(ctx, k8sclient.ObjectKey{Name: name}, hv); err != nil { - return err - } - base := hv.DeepCopy() - updateFn(hv) - return c.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base, - k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(fieldOwner)) - }) -}