diff --git a/api/v1/hypervisor_types.go b/api/v1/hypervisor_types.go index baf8301..6c7cba7 100644 --- a/api/v1/hypervisor_types.go +++ b/api/v1/hypervisor_types.go @@ -178,7 +178,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"` @@ -211,6 +211,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/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_maintenance_controller.go b/internal/controller/hypervisor_maintenance_controller.go index 1d0da1a..aa49e99 100644 --- a/internal/controller/hypervisor_maintenance_controller.go +++ b/internal/controller/hypervisor_maintenance_controller.go @@ -17,8 +17,7 @@ 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" @@ -32,12 +31,8 @@ import ( k8sclient "sigs.k8s.io/controller-runtime/pkg/client" 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" 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" ) @@ -47,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 @@ -77,10 +71,6 @@ func (hec *HypervisorMaintenanceController) Reconcile(ctx context.Context, req c statusCfg.WithConditions(utils.ConditionFromStatus(*c)) } - if err := hec.reconcileComputeService(ctx, hv, statusCfg); err != nil { - return ctrl.Result{}, err - } - if err := hec.reconcileEviction(ctx, hv, statusCfg); err != nil { return ctrl.Result{}, err } @@ -90,66 +80,6 @@ func (hec *HypervisorMaintenanceController) Reconcile(ctx context.Context, req c k8sclient.ForceOwnership, k8sclient.FieldOwner(HypervisorMaintenanceControllerName)) } -// reconcileComputeService enables/disables the nova-compute service based on -// hv.Spec.Maintenance and sets the HypervisorDisabled condition on statusCfg. -func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context.Context, hv *kvmv1.Hypervisor, statusCfg *apiv1.HypervisorStatusApplyConfiguration) error { - log := logger.FromContext(ctx) - serviceId := hv.Status.ServiceID - - if serviceId == "" { - // We can only do something here, if there is a service to begin with. - // The onboarding should take care of that. - return nil - } - - switch hv.Spec.Maintenance { - case kvmv1.MaintenanceUnset: - existing := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) - if existing == nil || existing.Status != metav1.ConditionFalse { - // We need to enable the host as per spec. - // Also clear forced_down in case a previous HA event set it. - falseVal := false - enableService := openstack.UpdateServiceOpts{ - Status: services.ServiceEnabled, - ForcedDown: &falseVal, - } - log.Info("Enabling hypervisor", "id", serviceId) - if _, err := services.Update(ctx, hec.computeClient, serviceId, enableService).Extract(); err != nil { - return fmt.Errorf("failed to enable hypervisor due to %w", err) - } - } - utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, - *k8sacmetav1.Condition(). - WithType(kvmv1.ConditionTypeHypervisorDisabled). - WithStatus(metav1.ConditionFalse). - WithMessage("Hypervisor is enabled"). - WithReason(kvmv1.ConditionReasonSucceeded)) - - case kvmv1.MaintenanceManual, kvmv1.MaintenanceAuto, kvmv1.MaintenanceTermination: - // Disable the compute service. - existing := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) - if existing == nil || existing.Status != metav1.ConditionTrue { - disableService := services.UpdateOpts{ - Status: services.ServiceDisabled, - DisabledReason: "Hypervisor CRD: spec.maintenance=" + hv.Spec.Maintenance, - } - // We need to disable the host as per spec - log.Info("Disabling hypervisor", "id", serviceId) - if _, err := services.Update(ctx, hec.computeClient, serviceId, disableService).Extract(); err != nil { - return fmt.Errorf("failed to disable hypervisor due to %w", err) - } - } - utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, - *k8sacmetav1.Condition(). - WithType(kvmv1.ConditionTypeHypervisorDisabled). - WithStatus(metav1.ConditionTrue). - WithMessage("Hypervisor is disabled"). - WithReason(kvmv1.ConditionReasonSucceeded)) - } - - return nil -} - // 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. @@ -159,7 +89,7 @@ func (hec *HypervisorMaintenanceController) reconcileEviction(ctx context.Contex } switch hv.Spec.Maintenance { - case kvmv1.MaintenanceUnset: + case kvmv1.MaintenanceUnset, kvmv1.MaintenanceNoSchedule: // Avoid deleting the eviction over and over. if !hv.Status.Evicted && meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeEvicting) == nil { return nil @@ -265,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/onboarding_controller.go b/internal/controller/onboarding_controller.go index b238a51..1509bfa 100644 --- a/internal/controller/onboarding_controller.go +++ b/internal/controller/onboarding_controller.go @@ -320,6 +320,11 @@ func (r *OnboardingController) completeOnboarding(ctx context.Context, host stri return ctrl.Result{RequeueAfter: r.getRequeueInterval()}, nil } + // Wait for HypervisorComputeService controller to enable the compute service + if !meta.IsStatusConditionFalse(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) { + return ctrl.Result{}, nil + } + // Wait for HypervisorInstanceHa controller to enable HA if hv.Spec.HighAvailability && !meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeHaEnabled) { return ctrl.Result{}, nil 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{