Cortex aggregates

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
@@ -341,6 +348,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"`
 

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

@@ -188,6 +188,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:

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

@@ -189,6 +189,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:

internal/controller/aggregates_controller.go

@@ -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"
@@ -52,97 +54,116 @@ type AggregatesController struct {
 // +kubebuilder:rbac:groups=kvm.cloud.sap,resources=hypervisors/status,verbs=get;list;watch;create;update;patch;delete
 
 func (ac *AggregatesController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-	log := logger.FromContext(ctx)
 	hv := &kvmv1.Hypervisor{}
 	if err := ac.Get(ctx, req.NamespacedName, hv); err != nil {
 		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(),
+			}
 
-	aggs, err := openstack.GetAggregatesByName(ctx, ac.computeClient)
-	if err != nil {
-		err = fmt.Errorf("failed listing aggregates: %w", err)
-		if err2 := ac.setErrorCondition(ctx, hv, err.Error()); err2 != nil {
-			return ctrl.Result{}, errors.Join(err, err2)
+			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
 		}
-		return ctrl.Result{}, err
+
+		hv.Status.Aggregates = desiredAggregates
+		hv.Status.AggregateUUIDs = uuids
+	}
+
+	// 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
 	}
 
-	toAdd := Difference(hv.Status.Aggregates, hv.Spec.Aggregates)
-	toRemove := Difference(hv.Spec.Aggregates, hv.Status.Aggregates)
-
-	// We need to add first the host to the aggregates, because if we first drop
-	// an aggregate with a filter criterion and then add a new one, we leave the host
-	// open for period of time. Still, this may fail due to a conflict of aggregates
-	// with different availability zones, so we collect all the errors and return them
-	// so it hopefully will converge eventually.
-	var errs []error
-	if len(toAdd) > 0 {
-		log.Info("Adding", "aggregates", toAdd)
-		for item := range slices.Values(toAdd) {
-			if err = openstack.AddToAggregate(ctx, ac.computeClient, aggs, hv.Name, item, ""); err != nil {
-				errs = append(errs, err)
+	return ctrl.Result{}, ac.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base,
+		k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(AggregatesControllerName))
+}
+
+// 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",
+		}
 	}
 
-	if len(toRemove) > 0 {
-		log.Info("Removing", "aggregates", toRemove)
-		for item := range slices.Values(toRemove) {
-			if err = openstack.RemoveFromAggregate(ctx, ac.computeClient, aggs, hv.Name, item); err != nil {
-				errs = append(errs, 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 errs != nil {
-		err = fmt.Errorf("encountered errors during aggregate update: %w", errors.Join(errs...))
-		if err2 := ac.setErrorCondition(ctx, hv, err.Error()); err2 != nil {
-			return ctrl.Result{}, errors.Join(err, err2)
+		// 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 ctrl.Result{}, err
 	}
 
-	base := hv.DeepCopy()
-	hv.Status.Aggregates = hv.Spec.Aggregates
-	meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
+	// 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 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,
+		Message: "Aggregates from spec applied successfully",
 	}
-
-	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
-		}
-	}
-
-	return nil
 }
 
 // SetupWithManager sets up the controller with the Manager.
@@ -154,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).