From 853945655e71237fbef16b6f7dcc91ec8528f007 Mon Sep 17 00:00:00 2001 From: Andrew Karpow Date: Tue, 16 Dec 2025 15:07:21 -0500 Subject: [PATCH] Refactor conditions, introduce terminating maintenance Consolidate all condition to their respective API spec, this makes it easier and clearer for external componenets to identify condition and reason, also reuse conditions that are similar (Complated -> Successful). Also introducing a new maintenance enum value "terminating", that is set by the maintenance-operator in case the node is terminating - removing the need to monitor nodes status. --- api/v1/eviction_types.go | 5 ++ api/v1/hypervisor_types.go | 53 ++++++++++---- .../crds/hypervisor-crd.yaml | 1 + .../crd/bases/kvm.cloud.sap_hypervisors.yaml | 1 + internal/controller/aggregates_controller.go | 15 ++-- .../controller/aggregates_controller_test.go | 10 +-- internal/controller/consts.go | 1 - internal/controller/eviction_controller.go | 2 +- .../gardener_node_lifecycle_controller.go | 2 +- .../hypervisor_maintenance_controller.go | 6 +- .../hypervisor_maintenance_controller_test.go | 2 +- .../node_eviction_label_controller.go | 2 +- .../node_eviction_label_controller_test.go | 4 +- internal/controller/onboarding_controller.go | 73 +++++++++---------- .../controller/onboarding_controller_test.go | 26 +++---- internal/controller/traits_controller.go | 15 ++-- internal/controller/traits_controller_test.go | 10 +-- 17 files changed, 124 insertions(+), 104 deletions(-) diff --git a/api/v1/eviction_types.go b/api/v1/eviction_types.go index 4d39b704..d6f503b3 100644 --- a/api/v1/eviction_types.go +++ b/api/v1/eviction_types.go @@ -37,6 +37,8 @@ type EvictionSpec struct { Reason string `json:"reason"` } +// Eviction Condition Types +// type of condition in CamelCase or in foo.example.com/CamelCase. const ( // ConditionTypeMigration is the type of condition for migration status of a server ConditionTypeMigration = "MigratingInstance" @@ -52,7 +54,10 @@ const ( // ConditionTypeEvicting is the type of condition for eviction status ConditionTypeEvicting = "Evicting" +) +// Condition Reasons +const ( // ConditionReasonRunning means the eviction is currently running ConditionReasonRunning string = "Running" diff --git a/api/v1/hypervisor_types.go b/api/v1/hypervisor_types.go index 752a1ef8..f2917f2e 100644 --- a/api/v1/hypervisor_types.go +++ b/api/v1/hypervisor_types.go @@ -26,23 +26,41 @@ import ( // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster // Important: Run "make" to regenerate code after modifying this file +// Hypervisor Condition Types +// type of condition in CamelCase or in foo.example.com/CamelCase. const ( + // ConditionTypeOnboarding is the type of condition for onboarding status + ConditionTypeOnboarding = "Onboarding" + // ConditionTypeReady is the type of condition for ready status of a hypervisor - ConditionTypeReady = "Ready" + ConditionTypeReady = "Ready" + + // ConditionTypeTerminating is the type of condition for terminating status of a hypervisor ConditionTypeTerminating = "Terminating" - ConditionTypeTainted = "Tainted" - // Reasons for the various being ready... - ConditionReasonReadyReady = "ready" - // or not - ConditionReasonReadyMaintenance = "maintenance" - ConditionReasonReadyEvicted = "evicted" + // ConditionTypeTainted is the type of condition for tainted status of a hypervisor + ConditionTypeTainted = "Tainted" - // HypervisorMaintenance "enum" - MaintenanceUnset = "" - MaintenanceManual = "manual" - MaintenanceAuto = "auto" - MaintenanceHA = "ha" + // ConditionTypeTraitsUpdated is the type of condition for traits updated status of a hypervisor + ConditionTypeTraitsUpdated = "TraitsUpdated" + + // ConditionTypeAggregatesUpdated is the type of condition for aggregates updated status of a hypervisor + ConditionTypeAggregatesUpdated = "AggregatesUpdated" +) + +// Condition Reasons +// The value should be a CamelCase string. +const ( + // ConditionTypeReady reasons + ConditionReasonReadyReady = "Ready" + ConditionReasonReadyMaintenance = "Maintenance" + ConditionReasonReadyEvicted = "Evicted" + + // ConditionTypeOnboarding reasons + ConditionReasonInitial = "Initial" + ConditionReasonOnboarding = "Onboarding" + ConditionReasonTesting = "Testing" + ConditionReasonAborted = "Aborted" ) // HypervisorSpec defines the desired state of Hypervisor @@ -89,11 +107,20 @@ type HypervisorSpec struct { InstallCertificate bool `json:"installCertificate"` // +kubebuilder:optional - // +kubebuilder:validation:Enum:="";manual;auto;ha + // +kubebuilder:validation:Enum:="";manual;auto;ha;termination // Maintenance indicates whether the hypervisor is in maintenance mode. Maintenance string `json:"maintenance,omitempty"` } +const ( + // HypervisorMaintenance "enum" + MaintenanceUnset = "" + MaintenanceManual = "manual" // manual maintenance mode by external user + MaintenanceAuto = "auto" // automatic maintenance mode + MaintenanceHA = "ha" // high availability maintenance mode + MaintenanceTermination = "termination" // internal use only, when node is terminating state +) + type Instance struct { // Represents the instance ID (uuidv4). ID string `json:"id"` diff --git a/charts/openstack-hypervisor-operator/crds/hypervisor-crd.yaml b/charts/openstack-hypervisor-operator/crds/hypervisor-crd.yaml index 2b7da6d6..ac60036d 100644 --- a/charts/openstack-hypervisor-operator/crds/hypervisor-crd.yaml +++ b/charts/openstack-hypervisor-operator/crds/hypervisor-crd.yaml @@ -149,6 +149,7 @@ spec: - manual - auto - ha + - termination type: string reboot: default: false diff --git a/config/crd/bases/kvm.cloud.sap_hypervisors.yaml b/config/crd/bases/kvm.cloud.sap_hypervisors.yaml index a8d005bc..9a9569cb 100644 --- a/config/crd/bases/kvm.cloud.sap_hypervisors.yaml +++ b/config/crd/bases/kvm.cloud.sap_hypervisors.yaml @@ -150,6 +150,7 @@ spec: - manual - auto - ha + - termination type: string reboot: default: false diff --git a/internal/controller/aggregates_controller.go b/internal/controller/aggregates_controller.go index 24118f9d..50185b74 100644 --- a/internal/controller/aggregates_controller.go +++ b/internal/controller/aggregates_controller.go @@ -40,10 +40,7 @@ import ( ) const ( - ConditionTypeAggregatesUpdated = "AggregatesUpdated" - ConditionAggregatesSuccess = "Success" - ConditionAggregatesFailed = "Failed" - AggregatesControllerName = "aggregates" + AggregatesControllerName = "aggregates" ) type AggregatesController struct { @@ -63,7 +60,7 @@ func (ac *AggregatesController) Reconcile(ctx context.Context, req ctrl.Request) } /// On- and off-boarding need to mess with the aggregates, so let's get out of their way - if !meta.IsStatusConditionFalse(hv.Status.Conditions, ConditionTypeOnboarding) || + if !meta.IsStatusConditionFalse(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) || meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeTerminating) { return ctrl.Result{}, nil } @@ -119,9 +116,9 @@ func (ac *AggregatesController) Reconcile(ctx context.Context, req ctrl.Request) hv.Status.Aggregates = hv.Spec.Aggregates meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: ConditionTypeAggregatesUpdated, + Type: kvmv1.ConditionTypeAggregatesUpdated, Status: metav1.ConditionTrue, - Reason: ConditionAggregatesSuccess, + Reason: kvmv1.ConditionReasonSucceeded, Message: "Aggregates updated successfully", }) return ctrl.Result{}, ac.Status().Update(ctx, hv) @@ -130,9 +127,9 @@ func (ac *AggregatesController) Reconcile(ctx context.Context, req ctrl.Request) // setErrorCondition sets the error condition on the Hypervisor status, returns error if update fails func (ac *AggregatesController) setErrorCondition(ctx context.Context, hv *kvmv1.Hypervisor, msg string) error { condition := metav1.Condition{ - Type: ConditionTypeAggregatesUpdated, + Type: kvmv1.ConditionTypeAggregatesUpdated, Status: metav1.ConditionFalse, - Reason: ConditionAggregatesFailed, + Reason: kvmv1.ConditionReasonFailed, Message: msg, } diff --git a/internal/controller/aggregates_controller_test.go b/internal/controller/aggregates_controller_test.go index d0a04e3d..3996fe04 100644 --- a/internal/controller/aggregates_controller_test.go +++ b/internal/controller/aggregates_controller_test.go @@ -121,7 +121,7 @@ var _ = Describe("AggregatesController", func() { Expect(k8sClient.Create(ctx, hypervisor)).To(Succeed()) Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) meta.SetStatusCondition(&hypervisor.Status.Conditions, v1.Condition{ - Type: ConditionTypeOnboarding, + Type: kvmv1.ConditionTypeOnboarding, Status: v1.ConditionFalse, Reason: "dontcare", Message: "dontcare", @@ -193,7 +193,7 @@ var _ = Describe("AggregatesController", func() { updated := &kvmv1.Hypervisor{} Expect(tc.Client.Get(ctx, hypervisorName, updated)).To(Succeed()) Expect(updated.Status.Aggregates).To(ContainElements("test-aggregate1")) - Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, ConditionTypeAggregatesUpdated)).To(BeTrue()) + Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, kvmv1.ConditionTypeAggregatesUpdated)).To(BeTrue()) }) }) @@ -237,7 +237,7 @@ var _ = Describe("AggregatesController", func() { updated := &kvmv1.Hypervisor{} Expect(tc.Client.Get(ctx, hypervisorName, updated)).To(Succeed()) Expect(updated.Status.Aggregates).To(BeEmpty()) - Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, ConditionTypeAggregatesUpdated)).To(BeTrue()) + Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, kvmv1.ConditionTypeAggregatesUpdated)).To(BeTrue()) }) }) @@ -254,7 +254,7 @@ var _ = Describe("AggregatesController", func() { updated := &kvmv1.Hypervisor{} Expect(tc.Client.Get(ctx, hypervisorName, updated)).To(Succeed()) Expect(updated.Status.Aggregates).To(BeEmpty()) - Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, ConditionTypeAggregatesUpdated)).To(BeFalse()) + Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, kvmv1.ConditionTypeAggregatesUpdated)).To(BeFalse()) }) }) @@ -276,7 +276,7 @@ var _ = Describe("AggregatesController", func() { updated := &kvmv1.Hypervisor{} Expect(tc.Client.Get(ctx, hypervisorName, updated)).To(Succeed()) Expect(updated.Status.Aggregates).To(BeEmpty()) - Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, ConditionTypeAggregatesUpdated)).To(BeFalse()) + Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, kvmv1.ConditionTypeAggregatesUpdated)).To(BeFalse()) }) }) }) diff --git a/internal/controller/consts.go b/internal/controller/consts.go index ed6b1259..2042ec0c 100644 --- a/internal/controller/consts.go +++ b/internal/controller/consts.go @@ -19,7 +19,6 @@ package controller // This should contain consts shared between controllers const ( - labelHypervisorID = "nova.openstack.cloud.sap/hypervisor-id" labelEvictionRequired = "cloud.sap/hypervisor-eviction-required" labelEvictionApproved = "cloud.sap/hypervisor-eviction-succeeded" labelHypervisor = "nova.openstack.cloud.sap/virt-driver" diff --git a/internal/controller/eviction_controller.go b/internal/controller/eviction_controller.go index d59f8970..528d4f6f 100644 --- a/internal/controller/eviction_controller.go +++ b/internal/controller/eviction_controller.go @@ -164,7 +164,7 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv } if hv.Name != "" { - expectHypervisor = HasStatusCondition(hv.Status.Conditions, ConditionTypeOnboarding) + expectHypervisor = HasStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) } if expectHypervisor { diff --git a/internal/controller/gardener_node_lifecycle_controller.go b/internal/controller/gardener_node_lifecycle_controller.go index fb1a1136..91d04fbf 100644 --- a/internal/controller/gardener_node_lifecycle_controller.go +++ b/internal/controller/gardener_node_lifecycle_controller.go @@ -101,7 +101,7 @@ func (r *GardenerNodeLifecycleController) Reconcile(ctx context.Context, req ctr return ctrl.Result{}, err } - onboardingCompleted := meta.IsStatusConditionFalse(hv.Status.Conditions, ConditionTypeOnboarding) + onboardingCompleted := meta.IsStatusConditionFalse(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) if err := retry.RetryOnConflict(retry.DefaultRetry, func() error { return r.ensureSignallingDeployment(ctx, node, minAvailable, onboardingCompleted) diff --git a/internal/controller/hypervisor_maintenance_controller.go b/internal/controller/hypervisor_maintenance_controller.go index 56f87b20..0f3160f2 100644 --- a/internal/controller/hypervisor_maintenance_controller.go +++ b/internal/controller/hypervisor_maintenance_controller.go @@ -63,7 +63,7 @@ func (hec *HypervisorMaintenanceController) Reconcile(ctx context.Context, req c } // is onboarding completed? - if !meta.IsStatusConditionFalse(hv.Status.Conditions, ConditionTypeOnboarding) { + if !meta.IsStatusConditionFalse(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) { return ctrl.Result{}, nil } @@ -120,7 +120,7 @@ func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context. if err != nil { return fmt.Errorf("failed to enable hypervisor due to %w", err) } - case kvmv1.MaintenanceManual, kvmv1.MaintenanceAuto, kvmv1.MaintenanceHA: + case kvmv1.MaintenanceManual, kvmv1.MaintenanceAuto, kvmv1.MaintenanceHA, kvmv1.MaintenanceTermination: // Disable the compute service: // Also in case of HA, as it doesn't hurt to disable it twice, and this // allows us to enable the service again, when the maintenance field is @@ -173,7 +173,7 @@ func (hec *HypervisorMaintenanceController) reconcileEviction(ctx context.Contex return err } return nil - case kvmv1.MaintenanceManual, kvmv1.MaintenanceAuto: + 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 { if cond.Reason == kvmv1.ConditionReasonSucceeded { diff --git a/internal/controller/hypervisor_maintenance_controller_test.go b/internal/controller/hypervisor_maintenance_controller_test.go index 18533062..1a56a4bf 100644 --- a/internal/controller/hypervisor_maintenance_controller_test.go +++ b/internal/controller/hypervisor_maintenance_controller_test.go @@ -133,7 +133,7 @@ var _ = Describe("HypervisorMaintenanceController", func() { hypervisor.Status.ServiceID = "1234" meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ - Type: ConditionTypeOnboarding, + Type: kvmv1.ConditionTypeOnboarding, Status: metav1.ConditionFalse, Reason: metav1.StatusSuccess, Message: "random text", diff --git a/internal/controller/node_eviction_label_controller.go b/internal/controller/node_eviction_label_controller.go index 3afeb53e..bbc45829 100644 --- a/internal/controller/node_eviction_label_controller.go +++ b/internal/controller/node_eviction_label_controller.go @@ -94,7 +94,7 @@ func (r *NodeEvictionLabelReconciler) Reconcile(ctx context.Context, req ctrl.Re return ctrl.Result{}, err } - if !HasStatusCondition(hv.Status.Conditions, ConditionTypeOnboarding) { + if !HasStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) { // Hasn't even started to onboard that node, so nothing to evict for sure value = "true" } else { diff --git a/internal/controller/node_eviction_label_controller_test.go b/internal/controller/node_eviction_label_controller_test.go index 3309d1af..c7c9769e 100644 --- a/internal/controller/node_eviction_label_controller_test.go +++ b/internal/controller/node_eviction_label_controller_test.go @@ -119,9 +119,9 @@ var _ = Describe("Node Eviction Label Controller", func() { Expect(k8sClient.Get(ctx, types.NamespacedName{Name: nodeName}, hypervisor)).To(Succeed()) By("updating the hypervisor status sub-resource") meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ - Type: ConditionTypeOnboarding, + Type: kvmv1.ConditionTypeOnboarding, Status: metav1.ConditionTrue, - Reason: ConditionReasonInitial, + Reason: kvmv1.ConditionReasonInitial, Message: "Initial onboarding", }) Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed()) diff --git a/internal/controller/onboarding_controller.go b/internal/controller/onboarding_controller.go index 6ee6906a..52d9392e 100644 --- a/internal/controller/onboarding_controller.go +++ b/internal/controller/onboarding_controller.go @@ -51,21 +51,14 @@ import ( var errRequeue = errors.New("requeue requested") const ( - defaultWaitTime = 1 * time.Minute - ConditionTypeOnboarding = "Onboarding" - ConditionReasonAborted = "aborted" - ConditionReasonError = "error" - ConditionReasonInitial = "initial" - ConditionReasonOnboarding = "onboarding" - ConditionReasonTesting = "testing" - ConditionReasonCompleted = "completed" - testAggregateName = "tenant_filter_tests" - testProjectName = "test" - testDomainName = "cc3test" - testImageName = "cirros-d240801-kvm" - testPrefixName = "ohooc-" - testVolumeType = "kvm-pilot" - OnboardingControllerName = "onboarding" + defaultWaitTime = 1 * time.Minute + testAggregateName = "tenant_filter_tests" + testProjectName = "test" + testDomainName = "cc3test" + testImageName = "cirros-d240801-kvm" + testPrefixName = "ohooc-" + testVolumeType = "kvm-pilot" + OnboardingControllerName = "onboarding" ) type OnboardingController struct { @@ -112,28 +105,28 @@ func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request) } // check condition reason - status := meta.FindStatusCondition(hv.Status.Conditions, ConditionTypeOnboarding) + status := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) if status == nil { meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeReady, Status: metav1.ConditionFalse, - Reason: ConditionReasonOnboarding, + Reason: kvmv1.ConditionReasonOnboarding, Message: "Onboarding in progress", }) meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: ConditionTypeOnboarding, + Type: kvmv1.ConditionTypeOnboarding, Status: metav1.ConditionTrue, - Reason: ConditionReasonInitial, + Reason: kvmv1.ConditionReasonInitial, Message: "Initial onboarding", }) return ctrl.Result{}, r.Status().Update(ctx, hv) } switch status.Reason { - case ConditionReasonInitial: + case kvmv1.ConditionReasonInitial: return ctrl.Result{}, r.initialOnboarding(ctx, hv, computeHost) - case ConditionReasonTesting: + case kvmv1.ConditionReasonTesting: if hv.Spec.SkipTests { return r.completeOnboarding(ctx, computeHost, hv) } else { @@ -146,7 +139,7 @@ func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request) } func (r *OnboardingController) abortOnboarding(ctx context.Context, hv *kvmv1.Hypervisor, computeHost string) error { - status := meta.FindStatusCondition(hv.Status.Conditions, ConditionTypeOnboarding) + status := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) // Never onboarded if status == nil { return nil @@ -156,11 +149,11 @@ func (r *OnboardingController) abortOnboarding(ctx context.Context, hv *kvmv1.Hy ready := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeReady) if ready != nil { // Only undo ones own readiness status reporting - if ready.Reason == ConditionReasonOnboarding { + if ready.Reason == kvmv1.ConditionReasonOnboarding { meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeReady, Status: metav1.ConditionFalse, - Reason: ConditionReasonOnboarding, + Reason: kvmv1.ConditionReasonOnboarding, Message: "Onboarding aborted", }) changed = true @@ -168,9 +161,9 @@ func (r *OnboardingController) abortOnboarding(ctx context.Context, hv *kvmv1.Hy } if meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: ConditionTypeOnboarding, + Type: kvmv1.ConditionTypeOnboarding, Status: metav1.ConditionFalse, - Reason: ConditionReasonAborted, + Reason: kvmv1.ConditionReasonAborted, Message: "Aborted due to LifecycleEnabled being false", }) { changed = true @@ -182,9 +175,9 @@ func (r *OnboardingController) abortOnboarding(ctx context.Context, hv *kvmv1.Hy } if err := r.deleteTestServers(ctx, computeHost); err != nil { meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: ConditionTypeOnboarding, + Type: kvmv1.ConditionTypeOnboarding, Status: metav1.ConditionTrue, // No cleanup, so we are still "onboarding" - Reason: ConditionReasonAborted, + Reason: kvmv1.ConditionReasonAborted, Message: err.Error(), }) @@ -241,9 +234,9 @@ func (r *OnboardingController) initialOnboarding(ctx context.Context, hv *kvmv1. } meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: ConditionTypeOnboarding, + Type: kvmv1.ConditionTypeOnboarding, Status: metav1.ConditionTrue, - Reason: ConditionReasonTesting, + Reason: kvmv1.ConditionReasonTesting, Message: "Running onboarding tests", }) return r.Status().Update(ctx, hv) @@ -270,9 +263,9 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis // Set condition back to testing meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: ConditionTypeOnboarding, + Type: kvmv1.ConditionTypeOnboarding, Status: metav1.ConditionTrue, - Reason: ConditionReasonTesting, + Reason: kvmv1.ConditionReasonTesting, Message: "Server ended up in error state: " + server.Fault.Message, }) if err = r.Status().Update(ctx, hv); err != nil { @@ -289,9 +282,9 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis consoleOutput, err := servers.ShowConsoleOutput(ctx, r.testComputeClient, server.ID, servers.ShowConsoleOutputOpts{Length: 11}).Extract() if err != nil { meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: ConditionTypeOnboarding, + Type: kvmv1.ConditionTypeOnboarding, Status: metav1.ConditionTrue, - Reason: ConditionReasonTesting, + Reason: kvmv1.ConditionReasonTesting, Message: fmt.Sprintf("could not get console output %v", err), }) if err := r.Status().Update(ctx, hv); err != nil { @@ -306,9 +299,9 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis if err = servers.Delete(ctx, r.testComputeClient, server.ID).ExtractErr(); err != nil { meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: ConditionTypeOnboarding, + Type: kvmv1.ConditionTypeOnboarding, Status: metav1.ConditionTrue, - Reason: ConditionReasonTesting, + Reason: kvmv1.ConditionReasonTesting, Message: fmt.Sprintf("failed to terminate instance %v", err), }) if err := r.Status().Update(ctx, hv); err != nil { @@ -329,9 +322,9 @@ func (r *OnboardingController) completeOnboarding(ctx context.Context, host stri err := r.deleteTestServers(ctx, host) if err != nil { if meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: ConditionTypeOnboarding, + Type: kvmv1.ConditionTypeOnboarding, Status: metav1.ConditionTrue, // No cleanup, so we are still "onboarding" - Reason: ConditionReasonAborted, + Reason: kvmv1.ConditionReasonAborted, Message: err.Error(), }) { return ctrl.Result{}, errors.Join(err, r.Status().Update(ctx, hv)) @@ -367,9 +360,9 @@ func (r *OnboardingController) completeOnboarding(ctx context.Context, host stri // set onboarding condition completed meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: ConditionTypeOnboarding, + Type: kvmv1.ConditionTypeOnboarding, Status: metav1.ConditionFalse, - Reason: ConditionReasonCompleted, + Reason: kvmv1.ConditionReasonSucceeded, Message: "Onboarding completed", }) return ctrl.Result{}, r.Status().Update(ctx, hv) diff --git a/internal/controller/onboarding_controller_test.go b/internal/controller/onboarding_controller_test.go index 00dc82d1..2f50d935 100644 --- a/internal/controller/onboarding_controller_test.go +++ b/internal/controller/onboarding_controller_test.go @@ -328,12 +328,12 @@ var _ = Describe("Onboarding Controller", func() { SatisfyAll( HaveField("Type", kvmv1.ConditionTypeReady), HaveField("Status", metav1.ConditionFalse), - HaveField("Reason", ConditionReasonOnboarding), + HaveField("Reason", kvmv1.ConditionReasonOnboarding), ), SatisfyAll( - HaveField("Type", ConditionTypeOnboarding), + HaveField("Type", kvmv1.ConditionTypeOnboarding), HaveField("Status", metav1.ConditionTrue), - HaveField("Reason", ConditionReasonTesting), + HaveField("Reason", kvmv1.ConditionReasonTesting), ), )) }) @@ -376,12 +376,12 @@ var _ = Describe("Onboarding Controller", func() { SatisfyAll( HaveField("Type", kvmv1.ConditionTypeReady), HaveField("Status", metav1.ConditionFalse), - HaveField("Reason", ConditionReasonOnboarding), + HaveField("Reason", kvmv1.ConditionReasonOnboarding), ), SatisfyAll( - HaveField("Type", ConditionTypeOnboarding), + HaveField("Type", kvmv1.ConditionTypeOnboarding), HaveField("Status", metav1.ConditionTrue), - HaveField("Reason", ConditionReasonTesting), + HaveField("Reason", kvmv1.ConditionReasonTesting), ), )) }) @@ -397,12 +397,12 @@ var _ = Describe("Onboarding Controller", func() { meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeReady, Status: metav1.ConditionFalse, - Reason: ConditionReasonOnboarding, + Reason: kvmv1.ConditionReasonOnboarding, }) meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: ConditionTypeOnboarding, + Type: kvmv1.ConditionTypeOnboarding, Status: metav1.ConditionTrue, - Reason: ConditionReasonInitial, + Reason: kvmv1.ConditionReasonInitial, }) Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) @@ -516,9 +516,9 @@ var _ = Describe("Onboarding Controller", func() { HaveField("Status", metav1.ConditionTrue), ), SatisfyAll( - HaveField("Type", ConditionTypeOnboarding), + HaveField("Type", kvmv1.ConditionTypeOnboarding), HaveField("Status", metav1.ConditionFalse), - HaveField("Reason", ConditionReasonCompleted), + HaveField("Reason", kvmv1.ConditionReasonSucceeded), ), )) }) @@ -543,9 +543,9 @@ var _ = Describe("Onboarding Controller", func() { HaveField("Status", metav1.ConditionTrue), ), SatisfyAll( - HaveField("Type", ConditionTypeOnboarding), + HaveField("Type", kvmv1.ConditionTypeOnboarding), HaveField("Status", metav1.ConditionFalse), - HaveField("Reason", ConditionReasonCompleted), + HaveField("Reason", kvmv1.ConditionReasonSucceeded), ), )) }) diff --git a/internal/controller/traits_controller.go b/internal/controller/traits_controller.go index 4e98e467..40a72ad4 100644 --- a/internal/controller/traits_controller.go +++ b/internal/controller/traits_controller.go @@ -39,11 +39,8 @@ import ( ) const ( - customPrefix = "CUSTOM_" - ConditionTypeTraitsUpdated = "TraitsUpdated" - ConditionTraitsSuccess = "Success" - ConditionTraitsFailed = "Failed" - TraitsControllerName = "traits" + customPrefix = "CUSTOM_" + TraitsControllerName = "traits" ) type TraitsController struct { @@ -72,7 +69,7 @@ func (tc *TraitsController) Reconcile(ctx context.Context, req ctrl.Request) (ct return ctrl.Result{RequeueAfter: 10 * time.Second}, nil } - if !meta.IsStatusConditionFalse(hv.Status.Conditions, ConditionTypeOnboarding) || + if !meta.IsStatusConditionFalse(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) || meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeTerminating) { return ctrl.Result{}, nil } @@ -145,14 +142,14 @@ func (tc *TraitsController) Reconcile(ctx context.Context, req ctrl.Request) (ct func getTraitCondition(err error, msg string) metav1.Condition { // set status condition var ( - reason = ConditionTraitsSuccess + reason = kvmv1.ConditionReasonSucceeded message = msg status = metav1.ConditionTrue ) if err != nil { status = metav1.ConditionFalse - reason = ConditionTraitsFailed + reason = kvmv1.ConditionReasonFailed if msg != "" { message = msg + ": " + err.Error() } else { @@ -161,7 +158,7 @@ func getTraitCondition(err error, msg string) metav1.Condition { } return metav1.Condition{ - Type: ConditionTypeTraitsUpdated, + Type: kvmv1.ConditionTypeTraitsUpdated, Status: status, Reason: reason, Message: message, diff --git a/internal/controller/traits_controller_test.go b/internal/controller/traits_controller_test.go index 791d53d9..3c14aacb 100644 --- a/internal/controller/traits_controller_test.go +++ b/internal/controller/traits_controller_test.go @@ -137,7 +137,7 @@ var _ = Describe("TraitsController", func() { hypervisor := &kvmv1.Hypervisor{} Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) meta.SetStatusCondition(&hypervisor.Status.Conditions, v1.Condition{ - Type: ConditionTypeOnboarding, + Type: kvmv1.ConditionTypeOnboarding, Status: v1.ConditionFalse, Reason: "UnitTest", }) @@ -154,7 +154,7 @@ var _ = Describe("TraitsController", func() { updated := &kvmv1.Hypervisor{} Expect(tc.Client.Get(ctx, hypervisorName, updated)).To(Succeed()) Expect(updated.Status.Traits).To(ContainElements("CUSTOM_FOO", "CUSTOM_BAR", "HW_CPU_X86_VMX")) - Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, ConditionTypeTraitsUpdated)).To(BeTrue()) + Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, kvmv1.ConditionTypeTraitsUpdated)).To(BeTrue()) }) }) @@ -186,7 +186,7 @@ var _ = Describe("TraitsController", func() { updated := &kvmv1.Hypervisor{} Expect(tc.Client.Get(ctx, hypervisorName, updated)).To(Succeed()) Expect(updated.Status.Traits).NotTo(ContainElements("CUSTOM_FOO", "CUSTOM_BAR", "HW_CPU_X86_VMX")) - Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, ConditionTypeTraitsUpdated)).To(BeFalse()) + Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, kvmv1.ConditionTypeTraitsUpdated)).To(BeFalse()) }) }) @@ -206,7 +206,7 @@ var _ = Describe("TraitsController", func() { hypervisor := &kvmv1.Hypervisor{} Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) meta.SetStatusCondition(&hypervisor.Status.Conditions, v1.Condition{ - Type: ConditionTypeOnboarding, + Type: kvmv1.ConditionTypeOnboarding, Status: v1.ConditionFalse, Reason: "UnitTest", }) @@ -229,7 +229,7 @@ var _ = Describe("TraitsController", func() { updated := &kvmv1.Hypervisor{} Expect(tc.Client.Get(ctx, hypervisorName, updated)).To(Succeed()) Expect(updated.Status.Traits).NotTo(ContainElements("CUSTOM_FOO", "CUSTOM_BAR", "HW_CPU_X86_VMX")) - Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, ConditionTypeTraitsUpdated)).To(BeFalse()) + Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, kvmv1.ConditionTypeTraitsUpdated)).To(BeFalse()) }) }) })