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.

Author: notandy

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 {