Refactor conditions, introduce terminating maintenance

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"
 

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"`

charts/openstack-hypervisor-operator/crds/hypervisor-crd.yaml

@@ -149,6 +149,7 @@ spec:
                 - manual
                 - auto
                 - ha
+                - termination
                 type: string
               reboot:
                 default: false

config/crd/bases/kvm.cloud.sap_hypervisors.yaml

@@ -150,6 +150,7 @@ spec:
                 - manual
                 - auto
                 - ha
+                - termination
                 type: string
               reboot:
                 default: false

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,
 	}
 

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())
 		})
 	})
 })

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"

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 {

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)

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 {

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",

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 {

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())