Only Aggregate contoller changes aggregates and store uuids

For Cortex, we want the uuids in addition to the names, so
keep track of them in the status.

Let's keep only the aggregates controller changing aggregates.

Author: fwiesel

api/v1/hypervisor_types.go

@@ -61,10 +61,17 @@ const (
 	ConditionReasonReadyEvicting    = "Evicting"
 
 	// ConditionTypeOnboarding reasons
-	ConditionReasonInitial    = "Initial"
-	ConditionReasonOnboarding = "Onboarding"
-	ConditionReasonTesting    = "Testing"
-	ConditionReasonAborted    = "Aborted"
+	ConditionReasonInitial               = "Initial"
+	ConditionReasonOnboarding            = "Onboarding"
+	ConditionReasonTesting               = "Testing"
+	ConditionReasonRemovingTestAggregate = "RemovingTestAggregate"
+	ConditionReasonAborted               = "Aborted"
+
+	// ConditionTypeAggregatesUpdated reasons
+	// Note: ConditionReasonSucceeded and ConditionReasonFailed are shared with eviction_types.go
+	ConditionReasonTestAggregates     = "TestAggregates"
+	ConditionReasonTerminating        = "Terminating"
+	ConditionReasonEvictionInProgress = "EvictionInProgress"
 )
 
 // HypervisorSpec defines the desired state of Hypervisor
@@ -347,6 +354,10 @@ type HypervisorStatus struct {
 	// Aggregates are the applied aggregates of the hypervisor.
 	Aggregates []string `json:"aggregates,omitempty"`
 
+	// +kubebuilder:default:={}
+	// The UUIDs of the aggregates are used to apply aggregates to the hypervisor.
+	AggregateUUIDs []string `json:"aggregateUUIDs,omitempty"`
+
 	// InternalIP is the internal IP address of the hypervisor.
 	InternalIP string `json:"internalIp,omitempty"`
 

api/v1/zz_generated.deepcopy.go

@@ -200,6 +200,13 @@ spec:
           status:
             description: HypervisorStatus defines the observed state of Hypervisor
             properties:
+              aggregateUUIDs:
+                default: []
+                description: The UUIDs of the aggregates are used to apply aggregates
+                  to the hypervisor.
+                items:
+                  type: string
+                type: array
               aggregates:
                 description: Aggregates are the applied aggregates of the hypervisor.
                 items:

applyconfigurations/api/v1/hypervisorstatus.go

@@ -201,6 +201,13 @@ spec:
           status:
             description: HypervisorStatus defines the observed state of Hypervisor
             properties:
+              aggregateUUIDs:
+                default: []
+                description: The UUIDs of the aggregates are used to apply aggregates
+                  to the hypervisor.
+                items:
+                  type: string
+                type: array
               aggregates:
                 description: Aggregates are the applied aggregates of the hypervisor.
                 items:

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

@@ -23,6 +23,8 @@ import (
 	"fmt"
 	"slices"
 
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -57,56 +59,111 @@ func (ac *AggregatesController) Reconcile(ctx context.Context, req ctrl.Request)
 		return ctrl.Result{}, k8sclient.IgnoreNotFound(err)
 	}
 
-	/// On- and off-boarding need to mess with the aggregates, so let's get out of their way
-	if !meta.IsStatusConditionFalse(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) ||
-		meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeTerminating) {
+	// Wait for onboarding controller to populate HypervisorID and ServiceID
+	// before attempting to modify aggregates
+	if hv.Status.HypervisorID == "" || hv.Status.ServiceID == "" {
 		return ctrl.Result{}, nil
 	}
 
-	if slices.Equal(hv.Spec.Aggregates, hv.Status.Aggregates) {
-		// Nothing to be done
-		return ctrl.Result{}, nil
+	base := hv.DeepCopy()
+	desiredAggregates, desiredCondition := ac.determineDesiredState(hv)
+
+	if !slices.Equal(desiredAggregates, hv.Status.Aggregates) {
+		// Apply aggregates to OpenStack and update status
+		uuids, err := openstack.ApplyAggregates(ctx, ac.computeClient, hv.Name, desiredAggregates)
+		if err != nil {
+			// Set error condition
+			condition := metav1.Condition{
+				Type:    kvmv1.ConditionTypeAggregatesUpdated,
+				Status:  metav1.ConditionFalse,
+				Reason:  kvmv1.ConditionReasonFailed,
+				Message: fmt.Errorf("failed to apply aggregates: %w", err).Error(),
+			}
+
+			if meta.SetStatusCondition(&hv.Status.Conditions, condition) {
+				if err2 := ac.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base,
+					k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(AggregatesControllerName)); err2 != nil {
+					return ctrl.Result{}, errors.Join(err, err2)
+				}
+			}
+			return ctrl.Result{}, err
+		}
+
+		hv.Status.Aggregates = desiredAggregates
+		hv.Status.AggregateUUIDs = uuids
 	}
 
-	err := openstack.ApplyAggregates(ctx, ac.computeClient, hv.Name, hv.Spec.Aggregates)
-	if err != nil {
-		err = fmt.Errorf("failed to apply aggregates: %w", err)
-		if err2 := ac.setErrorCondition(ctx, hv, err.Error()); err2 != nil {
-			return ctrl.Result{}, errors.Join(err, err2)
-		}
-		return ctrl.Result{}, err
+	// Set the condition based on the determined desired state
+	meta.SetStatusCondition(&hv.Status.Conditions, desiredCondition)
+
+	if equality.Semantic.DeepEqual(base, hv) {
+		return ctrl.Result{}, nil
 	}
 
-	base := hv.DeepCopy()
-	hv.Status.Aggregates = hv.Spec.Aggregates
-	meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
-		Type:    kvmv1.ConditionTypeAggregatesUpdated,
-		Status:  metav1.ConditionTrue,
-		Reason:  kvmv1.ConditionReasonSucceeded,
-		Message: "Aggregates updated successfully",
-	})
 	return ctrl.Result{}, ac.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base,
 		k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(AggregatesControllerName))
 }
 
-// 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:    kvmv1.ConditionTypeAggregatesUpdated,
-		Status:  metav1.ConditionFalse,
-		Reason:  kvmv1.ConditionReasonFailed,
-		Message: msg,
+// determineDesiredState returns the desired aggregates and the corresponding condition
+// based on the hypervisor's current state. The condition status is True only when
+// spec aggregates are being applied. Otherwise, it's False with a reason explaining
+// why different aggregates are applied.
+func (ac *AggregatesController) determineDesiredState(hv *kvmv1.Hypervisor) ([]string, metav1.Condition) {
+	// If terminating AND evicted, remove from all aggregates
+	// We must wait for eviction to complete before removing aggregates
+	if meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeTerminating) {
+		evictingCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeEvicting)
+		// Only remove aggregates if eviction is complete (Evicting=False)
+		// If Evicting condition is not set or still True, keep current aggregates
+		if evictingCondition != nil && evictingCondition.Status == metav1.ConditionFalse {
+			return []string{}, metav1.Condition{
+				Type:    kvmv1.ConditionTypeAggregatesUpdated,
+				Status:  metav1.ConditionFalse,
+				Reason:  kvmv1.ConditionReasonTerminating,
+				Message: "Aggregates cleared due to termination after eviction",
+			}
+		}
+		// Still evicting or eviction not started - keep current aggregates
+		return hv.Status.Aggregates, metav1.Condition{
+			Type:    kvmv1.ConditionTypeAggregatesUpdated,
+			Status:  metav1.ConditionFalse,
+			Reason:  kvmv1.ConditionReasonEvictionInProgress,
+			Message: "Aggregates unchanged while terminating and eviction in progress",
+		}
 	}
 
-	base := hv.DeepCopy()
-	if meta.SetStatusCondition(&hv.Status.Conditions, condition) {
-		if err := ac.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base,
-			k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(AggregatesControllerName)); err != nil {
-			return err
+	// If onboarding is in progress (Initial or Testing), add test aggregate
+	onboardingCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding)
+	if onboardingCondition != nil && onboardingCondition.Status == metav1.ConditionTrue {
+		if onboardingCondition.Reason == kvmv1.ConditionReasonInitial ||
+			onboardingCondition.Reason == kvmv1.ConditionReasonTesting {
+			zone := hv.Labels[corev1.LabelTopologyZone]
+			return []string{zone, testAggregateName}, metav1.Condition{
+				Type:    kvmv1.ConditionTypeAggregatesUpdated,
+				Status:  metav1.ConditionFalse,
+				Reason:  kvmv1.ConditionReasonTestAggregates,
+				Message: "Test aggregate applied during onboarding instead of spec aggregates",
+			}
+		}
+
+		// If removing test aggregate, use Spec.Aggregates (no test aggregate)
+		if onboardingCondition.Reason == kvmv1.ConditionReasonRemovingTestAggregate {
+			return hv.Spec.Aggregates, metav1.Condition{
+				Type:    kvmv1.ConditionTypeAggregatesUpdated,
+				Status:  metav1.ConditionTrue,
+				Reason:  kvmv1.ConditionReasonSucceeded,
+				Message: "Aggregates from spec applied successfully",
+			}
 		}
 	}
 
-	return nil
+	// Normal operations or onboarding complete: use Spec.Aggregates
+	return hv.Spec.Aggregates, metav1.Condition{
+		Type:    kvmv1.ConditionTypeAggregatesUpdated,
+		Status:  metav1.ConditionTrue,
+		Reason:  kvmv1.ConditionReasonSucceeded,
+		Message: "Aggregates from spec applied successfully",
+	}
 }
 
 // SetupWithManager sets up the controller with the Manager.
@@ -118,7 +175,6 @@ func (ac *AggregatesController) SetupWithManager(mgr ctrl.Manager) error {
 	if ac.computeClient, err = openstack.GetServiceClient(ctx, "compute", nil); err != nil {
 		return err
 	}
-	ac.computeClient.Microversion = "2.40" // gophercloud only supports numeric ids
 
 	return ctrl.NewControllerManagedBy(mgr).
 		Named(AggregatesControllerName).