[hypervisor-controller] one-time transport aggregate and custom traits

Previous commit changed the aggregate- and custom-traits handling to the
hypervisor-crd. This removed the persistence over new gardener rollouts.
This PR reintroduces the persistence by transporting the aggregates/traits
from server labels/annotationts to the new hypervisor CRO.

Also changes the existing code from using `controllerutil.CreateOrPatch`
to explictly checking if the objects is already there (and only
updating status in this case). Manual overrides to the CRO would have
been overwritten else.

controllerutil.CreateOrPatch behaves unpredicted in a way that it won't update
status for newly created CRO, only for existing ones.

Author: notandy

internal/controller/hypervisor_controller.go

@@ -20,8 +20,11 @@ package controller
 import (
 	"context"
 	"fmt"
+	"slices"
+	"strings"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -36,7 +39,9 @@ import (
 )
 
 const (
-	labelLifecycleMode = "cobaltcore.cloud.sap/node-hypervisor-lifecycle"
+	labelLifecycleMode     = "cobaltcore.cloud.sap/node-hypervisor-lifecycle"
+	annotationAggregates   = "nova.openstack.cloud.sap/aggregates"
+	annotationCustomTraits = "nova.openstack.cloud.sap/custom-traits"
 )
 
 var transferLabels = []string{
@@ -80,24 +85,14 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request)
 		},
 	}
 
-	// Ensure corresponding hypervisor exists or update it
-	op, err := controllerutil.CreateOrPatch(ctx, hv.Client, hypervisor, func() error {
-		// Transfer Labels
-		for _, label := range transferLabels {
-			if nodeLabels.Has(label) {
-				hypervisor.Labels[label] = nodeLabels.Get(label)
-			}
-		}
-
-		if nodeLabels.Has(labelLifecycleMode) {
-			hypervisor.Spec.LifecycleEnabled = true
-			hypervisor.Spec.SkipTests = nodeLabels.Get(labelLifecycleMode) == "skip-tests"
+	// Check if hypervisor already exists
+	if err := hv.Get(ctx, k8sclient.ObjectKeyFromObject(hypervisor), hypervisor); err != nil {
+		if !errors.IsNotFound(err) {
+			return ctrl.Result{}, fmt.Errorf("failed to get hypervisor: %w", err)
 		}
-
-		if err := controllerutil.SetControllerReference(node, hypervisor, hv.Scheme); err != nil {
-			return fmt.Errorf("failed setting controller reference: %w", err)
-		}
-
+		// continue with creation
+	} else {
+		// update Status if needed
 		nodeTerminationCondition := FindNodeStatusCondition(node.Status.Conditions, "Terminating")
 		if nodeTerminationCondition != nil && nodeTerminationCondition.Status == corev1.ConditionTrue {
 			// Node might be terminating, propagate condition to hypervisor
@@ -113,16 +108,60 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request)
 				Reason:  nodeTerminationCondition.Reason,
 				Message: nodeTerminationCondition.Message,
 			})
+			return ctrl.Result{}, hv.Status().Update(ctx, hypervisor)
 		}
 
-		return nil
-	})
-	if err != nil {
-		log.Error(err, "Hypervisor reconcile failed")
-	} else if op != controllerutil.OperationResultNone {
-		log.Info("Hypervisor successfully reconciled", "operation", op)
+		return ctrl.Result{}, nil
+	}
+
+	// transfer labels
+	for _, label := range transferLabels {
+		if nodeLabels.Has(label) {
+			hypervisor.Labels[label] = nodeLabels.Get(label)
+		}
+	}
+
+	// transport lifecycle label to hypervisor spec
+	if nodeLabels.Has(labelLifecycleMode) {
+		hypervisor.Spec.LifecycleEnabled = true
+		hypervisor.Spec.SkipTests = nodeLabels.Get(labelLifecycleMode) == "skip-tests"
+	}
+
+	// transport aggregates annotation to hypervisor spec
+	if aggregates, found := node.Annotations[annotationAggregates]; found {
+		// split aggregates string
+		hypervisor.Spec.Aggregates = slices.Collect(func(yield func(string) bool) {
+			for _, agg := range strings.Split(aggregates, ",") {
+				trimmed := strings.TrimSpace(agg)
+				if trimmed != "" && yield(trimmed) {
+					return
+				}
+			}
+		})
+	}
+
+	// transport custom traits annotation to hypervisor spec
+	if customTraits, found := node.Annotations[annotationCustomTraits]; found {
+		// split custom traits string
+		hypervisor.Spec.CustomTraits = slices.Collect(func(yield func(string) bool) {
+			for _, trait := range strings.Split(customTraits, ",") {
+				trimmed := strings.TrimSpace(trait)
+				if trimmed != "" && yield(trimmed) {
+					return
+				}
+			}
+		})
+	}
+
+	if err := controllerutil.SetControllerReference(node, hypervisor, hv.Scheme); err != nil {
+		return ctrl.Result{}, fmt.Errorf("failed setting controller reference: %w", err)
+	}
+
+	if err := hv.Create(ctx, hypervisor); err != nil {
+		return ctrl.Result{}, err
 	}
 
+	log.Info("Created Hypervisor resource", "hypervisor", hypervisor.Name)
 	return ctrl.Result{}, nil
 }
 

internal/controller/hypervisor_controller_test.go

@@ -47,7 +47,7 @@ var _ = Describe("Hypervisor Controller", func() {
 		// pregenerate the resource
 		resource = &corev1.Node{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:   "node-test",
+				Name:   "other-node",
 				Labels: map[string]string{corev1.LabelTopologyZone: "test-zone"}, //nolint:goconst
 			},
 		}
@@ -57,22 +57,24 @@ var _ = Describe("Hypervisor Controller", func() {
 		node := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: resource.Name}}
 		By("Cleanup the specific node")
 		Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, node))).To(Succeed())
+
+		hypervisor := &kvmv1.Hypervisor{ObjectMeta: metav1.ObjectMeta{Name: resource.Name}}
+		By("Cleanup the specific hypervisor")
+		Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, hypervisor))).To(Succeed())
 	})
 
 	Context("When reconciling a node", func() {
-		BeforeEach(func() {
-			Expect(k8sClient.Create(ctx, resource)).To(Succeed())
-		})
 
 		It("should successfully reconcile the node", func(ctx SpecContext) {
+			Expect(k8sClient.Create(ctx, resource)).To(Succeed())
+
 			By("Reconciling the created resource")
 			_, err := hypervisorController.Reconcile(ctx, ctrl.Request{
 				NamespacedName: types.NamespacedName{Name: resource.Name},
 			})
 			Expect(err).NotTo(HaveOccurred())
-		})
 
-		It("should have created the Hypervisor resource", func(ctx SpecContext) {
+			By("should have created the Hypervisor resource")
 			// Get the Hypervisor resource
 			hypervisor := &kvmv1.Hypervisor{}
 			hypervisorName := types.NamespacedName{Name: resource.Name}
@@ -84,7 +86,7 @@ var _ = Describe("Hypervisor Controller", func() {
 	})
 
 	Context("When reconciling a terminating node", func() {
-		BeforeEach(func(ctx SpecContext) {
+		It("should successfully reconcile the terminating node", func(ctx SpecContext) {
 			// Mark the node as terminating
 			resource.DeletionTimestamp = &metav1.Time{}
 			Expect(k8sClient.Create(ctx, resource)).To(Succeed())
@@ -97,17 +99,16 @@ var _ = Describe("Hypervisor Controller", func() {
 				Message: "Node is terminating",
 			})
 			Expect(k8sClient.Status().Update(ctx, resource)).To(Succeed())
-		})
 
-		It("should successfully reconcile the terminating node", func(ctx SpecContext) {
 			By("Reconciling the created resource")
-			_, err := hypervisorController.Reconcile(ctx, ctrl.Request{
-				NamespacedName: types.NamespacedName{Name: resource.Name},
-			})
-			Expect(err).NotTo(HaveOccurred())
-		})
-
-		It("should have set the terminating condition on the Hypervisor resource", func(ctx SpecContext) {
+			for i := 0; i < 2; i++ {
+				_, err := hypervisorController.Reconcile(ctx, ctrl.Request{
+					NamespacedName: types.NamespacedName{Name: resource.Name},
+				})
+				Expect(err).NotTo(HaveOccurred())
+			}
+
+			By("should have set the terminating condition on the Hypervisor resource")
 			// Get the Hypervisor resource
 			hypervisor := &kvmv1.Hypervisor{}
 			hypervisorName := types.NamespacedName{Name: resource.Name}