From 12e7b6bc08d4f4ed6dedb7f9449b08a67ee6ae96 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Fri, 10 Jul 2026 13:37:27 +0200 Subject: [PATCH 1/3] HypervisorController: also detect termination via Node deletion timestamp Gardener is moving away from setting a Terminating node condition and will instead simply delete the Node. Add node.deletionTimestamp as a second trigger for the Terminating condition on the Hypervisor, alongside the existing legacy node condition check. Both inputs produce the same result: ConditionTypeTerminating=True with ConditionReasonTerminating. The same dual-check is applied to the Spec.Maintenance seeding on first Hypervisor creation, so a node that is already being deleted when its Hypervisor CR is first created correctly enters MaintenanceTermination. --- api/v1/hypervisor_types.go | 4 +- internal/controller/hypervisor_controller.go | 16 ++++---- .../controller/hypervisor_controller_test.go | 41 +++++++++++++++++-- 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/api/v1/hypervisor_types.go b/api/v1/hypervisor_types.go index baf8301..4af9043 100644 --- a/api/v1/hypervisor_types.go +++ b/api/v1/hypervisor_types.go @@ -60,7 +60,9 @@ const ( // ConditionTypeReady is the type of condition for ready status of a hypervisor ConditionTypeReady = "Ready" - // ConditionTypeTerminating is the type of condition for terminating status of a hypervisor + // ConditionTypeTerminating is set when the underlying Node is terminating — + // either because the legacy gardener node condition "Terminating" is present, + // or because the Node's deletionTimestamp has been set (gardener's future path). ConditionTypeTerminating = "Terminating" // ConditionTypeTainted is the type of condition for tainted status of a hypervisor diff --git a/internal/controller/hypervisor_controller.go b/internal/controller/hypervisor_controller.go index 0ee5730..d174eb6 100644 --- a/internal/controller/hypervisor_controller.go +++ b/internal/controller/hypervisor_controller.go @@ -130,15 +130,16 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request) } } - // update terminating condition + // update terminating condition — fired by either the legacy gardener + // node condition or the node's deletion timestamp (gardener's future path) nodeTerminationCondition := FindNodeStatusCondition(node.Status.Conditions, "Terminating") - if nodeTerminationCondition != nil && nodeTerminationCondition.Status == corev1.ConditionTrue { - // Node might be terminating, propagate condition to hypervisor + if (nodeTerminationCondition != nil && nodeTerminationCondition.Status == corev1.ConditionTrue) || + !node.DeletionTimestamp.IsZero() { meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeTerminating, - Status: metav1.ConditionStatus(nodeTerminationCondition.Status), - Reason: nodeTerminationCondition.Reason, - Message: nodeTerminationCondition.Message, + Status: metav1.ConditionTrue, + Reason: kvmv1.ConditionReasonTerminating, + Message: "Node is terminating", }) } @@ -188,7 +189,8 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, fmt.Errorf("failed setting controller reference: %w", err) } - if IsNodeConditionPresentAndEqual(node.Status.Conditions, "Terminating", corev1.ConditionTrue) { + if IsNodeConditionPresentAndEqual(node.Status.Conditions, "Terminating", corev1.ConditionTrue) || + !node.DeletionTimestamp.IsZero() { hypervisor.Spec.Maintenance = kvmv1.MaintenanceTermination } diff --git a/internal/controller/hypervisor_controller_test.go b/internal/controller/hypervisor_controller_test.go index 92313ad..7c89e97 100644 --- a/internal/controller/hypervisor_controller_test.go +++ b/internal/controller/hypervisor_controller_test.go @@ -318,13 +318,12 @@ var _ = Describe("Hypervisor Controller", func() { } By("should have set the terminating condition on the Hypervisor resource") - // Get the Hypervisor resource updatedHypervisor := &kvmv1.Hypervisor{} Expect(hypervisorController.Get(ctx, hypervisorName, updatedHypervisor)).To(Succeed()) Expect(updatedHypervisor.Status.Conditions).To(ContainElement( SatisfyAll( HaveField("Type", kvmv1.ConditionTypeTerminating), - HaveField("Reason", terminatingReason), + HaveField("Reason", kvmv1.ConditionReasonTerminating), HaveField("Status", metav1.ConditionTrue), ), )) @@ -366,8 +365,42 @@ var _ = Describe("Hypervisor Controller", func() { Expect(updatedHypervisor.Status.Conditions).To(ContainElement( SatisfyAll( HaveField("Type", kvmv1.ConditionTypeTerminating), - HaveField("Reason", terminatingReason), + HaveField("Reason", kvmv1.ConditionReasonTerminating), + HaveField("Status", metav1.ConditionTrue), + ), + )) + }) + }) + Context("when only the deletion timestamp is set (no Terminating node condition)", func() { + BeforeEach(func(ctx SpecContext) { + // Use a different node that has no Terminating condition — only + // a deletion timestamp — to verify the new gardener signal path. + resource.Status.Conditions = nil + Expect(k8sClient.Status().Update(ctx, resource)).To(Succeed()) + + By("creating a Hypervisor so the existing-hypervisor path is exercised") + hypervisor := &kvmv1.Hypervisor{ + ObjectMeta: metav1.ObjectMeta{Name: resource.Name}, + } + Expect(k8sClient.Create(ctx, hypervisor)).To(Succeed()) + DeferCleanup(func(ctx SpecContext) { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, hypervisor))).To(Succeed()) + }) + }) + + It("should set the Terminating condition from the deletion timestamp", func(ctx SpecContext) { + _, err := hypervisorController.Reconcile(ctx, ctrl.Request{ + NamespacedName: types.NamespacedName{Name: resource.Name}, + }) + Expect(err).NotTo(HaveOccurred()) + + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + Expect(hypervisor.Status.Conditions).To(ContainElement( + SatisfyAll( + HaveField("Type", kvmv1.ConditionTypeTerminating), HaveField("Status", metav1.ConditionTrue), + HaveField("Reason", kvmv1.ConditionReasonTerminating), ), )) }) @@ -602,7 +635,7 @@ var _ = Describe("Hypervisor Controller", func() { SatisfyAll( HaveField("Type", kvmv1.ConditionTypeTerminating), HaveField("Status", metav1.ConditionTrue), - HaveField("Reason", terminatingReason), + HaveField("Reason", kvmv1.ConditionReasonTerminating), ), )) }) From 40b9e03be81fba38620e8810ca3b8d24a599fafd Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Tue, 18 Nov 2025 12:56:23 +0100 Subject: [PATCH 2/3] GardenerNodeLifecycle: Base it on the Hypervisor CRD, do not label node The eviction will now be triggered by the maintenance-operator, so drop setting the label. Watch the conditions on the hypervisor and manage the pods / deployments in kube-system to signal gardener that - the node is ready when the onboarding condition is completed - the node can be drained (and therefor terminated) if it has been evicted --- .../templates/role.yaml | 1 - .../gardener_node_lifecycle_controller.go | 117 +++++++----------- ...gardener_node_lifecycle_controller_test.go | 103 ++++++++++++--- 3 files changed, 130 insertions(+), 91 deletions(-) diff --git a/charts/openstack-hypervisor-operator/templates/role.yaml b/charts/openstack-hypervisor-operator/templates/role.yaml index 3b1ecda..60dca46 100644 --- a/charts/openstack-hypervisor-operator/templates/role.yaml +++ b/charts/openstack-hypervisor-operator/templates/role.yaml @@ -12,7 +12,6 @@ rules: - get - list - patch - - update - watch - apiGroups: - "" diff --git a/internal/controller/gardener_node_lifecycle_controller.go b/internal/controller/gardener_node_lifecycle_controller.go index 3cd803b..15146d0 100644 --- a/internal/controller/gardener_node_lifecycle_controller.go +++ b/internal/controller/gardener_node_lifecycle_controller.go @@ -33,11 +33,9 @@ import ( v1 "k8s.io/client-go/applyconfigurations/meta/v1" policyv1ac "k8s.io/client-go/applyconfigurations/policy/v1" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/builder" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/event" - "sigs.k8s.io/controller-runtime/pkg/handler" logger "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" @@ -52,14 +50,13 @@ type GardenerNodeLifecycleController struct { const ( labelCriticalComponent = "node.gardener.cloud/critical-component" - valueReasonTerminating = "terminating" MaintenanceControllerName = "maintenance" ) // The counter-side in gardener is here: // https://github.com/gardener/machine-controller-manager/blob/rel-v0.56/pkg/util/provider/machinecontroller/machine.go#L646 -// +kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;watch;patch;update;watch +// +kubebuilder:rbac:groups=kvm.cloud.sap,resources=hypervisors,verbs=get;list;watch;update;patch // +kubebuilder:rbac:groups="apps",resources=deployments,verbs=create;delete;get;list;patch;update;watch // +kubebuilder:rbac:groups="policy",resources=poddisruptionbudgets,verbs=create;delete;get;list;patch;update;watch @@ -67,19 +64,9 @@ func (r *GardenerNodeLifecycleController) Reconcile(ctx context.Context, req ctr log := logger.FromContext(ctx).WithName(req.Name) ctx = logger.IntoContext(ctx, log) - node := &corev1.Node{} - if err := r.Get(ctx, req.NamespacedName, node); err != nil { - // ignore not found errors, could be deleted - return ctrl.Result{}, k8sclient.IgnoreNotFound(err) - } - hv := &kvmv1.Hypervisor{} - if err := r.Get(ctx, k8sclient.ObjectKey{Name: req.Name}, hv); err != nil { - if k8sclient.IgnoreNotFound(err) != nil { - return ctrl.Result{}, err - } - // Hypervisor not found, nothing to do - return ctrl.Result{}, nil + if err := r.Get(ctx, req.NamespacedName, hv); err != nil { + return ctrl.Result{}, k8sclient.IgnoreNotFound(err) } if !hv.Spec.LifecycleEnabled { @@ -91,27 +78,35 @@ func (r *GardenerNodeLifecycleController) Reconcile(ctx context.Context, req ctr // to avoid racing with live-migration. if hv.Spec.Maintenance == kvmv1.MaintenanceTermination && meta.IsStatusConditionFalse(hv.Status.Conditions, kvmv1.ConditionTypeEvicting) { - patched, err := r.ensureOffboardingTaint(ctx, node) - if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to ensure offboarding taint: %w", err) - } - if patched { - return ctrl.Result{}, nil + node := &corev1.Node{} + if err := r.Get(ctx, k8sclient.ObjectKey{Name: hv.Name}, node); err != nil { + if k8sclient.IgnoreNotFound(err) != nil { + return ctrl.Result{}, err + } + // Node already gone, nothing to taint + } else { + patched, err := r.ensureOffboardingTaint(ctx, node) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to ensure offboarding taint: %w", err) + } + if patched { + return ctrl.Result{}, nil + } } } - // We do not care about the particular value, as long as it isn't an error - var minAvailable int32 = 1 + var minAvailable int32 = 0 + if !meta.IsStatusConditionFalse(hv.Status.Conditions, kvmv1.ConditionTypeEvicting) { + // Evicting condition is either not present or is true (i.e. ongoing) + minAvailable = 1 // Do not allow draining of the pod + } - // Onboarding is not in progress anymore, i.e. the host is onboarded - onboardingCompleted := meta.IsStatusConditionFalse(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) - // Evicting is not in progress anymore, i.e. the host is empty offboarded := meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeOffboarded) - if offboarded { minAvailable = 0 - if onboardingCompleted && isTerminating(node) { + onboardingCompleted := meta.IsStatusConditionFalse(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) + if onboardingCompleted && hv.Spec.Maintenance == kvmv1.MaintenanceTermination { // Wait for HypervisorInstanceHa controller to disable HA if !meta.IsStatusConditionFalse(hv.Status.Conditions, kvmv1.ConditionTypeHaEnabled) { return ctrl.Result{}, nil // Will be reconciled again when condition changes @@ -119,11 +114,12 @@ func (r *GardenerNodeLifecycleController) Reconcile(ctx context.Context, req ctr } } - if err := r.ensureBlockingPodDisruptionBudget(ctx, node, minAvailable); err != nil { + if err := r.ensureBlockingPodDisruptionBudget(ctx, hv, minAvailable); err != nil { return ctrl.Result{}, err } - if err := r.ensureSignallingDeployment(ctx, node, minAvailable, onboardingCompleted); err != nil { + onboardingCompleted := meta.IsStatusConditionFalse(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) + if err := r.ensureSignallingDeployment(ctx, hv, minAvailable, onboardingCompleted); err != nil { return ctrl.Result{}, err } @@ -154,17 +150,17 @@ func (r *GardenerNodeLifecycleController) ensureOffboardingTaint(ctx context.Con return true, r.Patch(ctx, node, patch, k8sclient.FieldOwner(MaintenanceControllerName)) } -func (r *GardenerNodeLifecycleController) ensureBlockingPodDisruptionBudget(ctx context.Context, node *corev1.Node, minAvailable int32) error { - name := nameForNode(node) - nodeLabels := labelsForNode(node) - gvk, err := apiutil.GVKForObject(node, r.Scheme) +func (r *GardenerNodeLifecycleController) ensureBlockingPodDisruptionBudget(ctx context.Context, hypervisor *kvmv1.Hypervisor, minAvailable int32) error { + name := nameForHypervisor(hypervisor) + nodeLabels := labelsForHypervisor(hypervisor) + gvk, err := apiutil.GVKForObject(hypervisor, r.Scheme) if err != nil { return err } podDisruptionBudget := policyv1ac.PodDisruptionBudget(name, MaintenanceNamespace). WithLabels(nodeLabels). - WithOwnerReferences(OwnerReference(node, &gvk)). + WithOwnerReferences(OwnerReference(hypervisor, &gvk)). WithSpec(policyv1ac.PodDisruptionBudgetSpec(). WithMinAvailable(intstr.FromInt32(minAvailable)). WithSelector(v1.LabelSelector().WithMatchLabels(nodeLabels))) @@ -172,35 +168,19 @@ func (r *GardenerNodeLifecycleController) ensureBlockingPodDisruptionBudget(ctx return r.Apply(ctx, podDisruptionBudget, k8sclient.FieldOwner(MaintenanceControllerName)) } -func isTerminating(node *corev1.Node) bool { - conditions := node.Status.Conditions - if conditions == nil { - return false - } - - // See: https://github.com/gardener/machine-controller-manager/blob/rel-v0.56/pkg/util/provider/machinecontroller/machine.go#L658-L659 - for _, condition := range conditions { - if condition.Type == "Terminating" { - return true - } - } - - return false -} - -func nameForNode(node *corev1.Node) string { - return fmt.Sprintf("maint-%v", node.Name) +func nameForHypervisor(hypervisor *kvmv1.Hypervisor) string { + return fmt.Sprintf("maint-%v", hypervisor.Name) } -func labelsForNode(node *corev1.Node) map[string]string { +func labelsForHypervisor(hypervisor *kvmv1.Hypervisor) map[string]string { return map[string]string{ - MaintenanceLabelKey: nameForNode(node), + MaintenanceLabelKey: nameForHypervisor(hypervisor), } } -func (r *GardenerNodeLifecycleController) ensureSignallingDeployment(ctx context.Context, node *corev1.Node, scale int32, ready bool) error { - name := nameForNode(node) - labels := labelsForNode(node) +func (r *GardenerNodeLifecycleController) ensureSignallingDeployment(ctx context.Context, hypervisor *kvmv1.Hypervisor, scale int32, ready bool) error { + name := nameForHypervisor(hypervisor) + labels := labelsForHypervisor(hypervisor) podLabels := maps.Clone(labels) podLabels[labelCriticalComponent] = "true" @@ -212,13 +192,13 @@ func (r *GardenerNodeLifecycleController) ensureSignallingDeployment(ctx context command = "/bin/false" } - gvk, err := apiutil.GVKForObject(node, r.Scheme) + gvk, err := apiutil.GVKForObject(hypervisor, r.Scheme) if err != nil { return err } deployment := apps1ac.Deployment(name, MaintenanceNamespace). - WithOwnerReferences(OwnerReference(node, &gvk)). + WithOwnerReferences(OwnerReference(hypervisor, &gvk)). WithLabels(labels). WithSpec(apps1ac.DeploymentSpec(). WithReplicas(scale). @@ -233,7 +213,7 @@ func (r *GardenerNodeLifecycleController) ensureSignallingDeployment(ctx context WithSpec(corev1ac.PodSpec(). WithHostNetwork(true). WithNodeSelector(map[string]string{ - corev1.LabelHostname: node.Labels[corev1.LabelHostname], + corev1.LabelHostname: hypervisor.Labels[corev1.LabelHostname], }). WithTerminationGracePeriodSeconds(1). WithTolerations( @@ -265,20 +245,9 @@ func (r *GardenerNodeLifecycleController) SetupWithManager(mgr ctrl.Manager, nam _ = logger.FromContext(ctx) r.namespace = namespace - hypervisorToNode := handler.EnqueueRequestForOwner(mgr.GetScheme(), mgr.GetRESTMapper(), &corev1.Node{}) - - // Maintenance=termination bumps generation; Evicting status changes do not. - hypervisorRelevantChange := predicate.Or( - predicate.GenerationChangedPredicate{}, - evictingConditionChangedPredicate{}, - ) - return ctrl.NewControllerManagedBy(mgr). Named(MaintenanceControllerName). - For(&corev1.Node{}). - Watches(&kvmv1.Hypervisor{}, hypervisorToNode, - builder.WithPredicates(hypervisorRelevantChange), - ). + For(&kvmv1.Hypervisor{}). Owns(&appsv1.Deployment{}). Owns(&policyv1.PodDisruptionBudget{}). Complete(r) diff --git a/internal/controller/gardener_node_lifecycle_controller_test.go b/internal/controller/gardener_node_lifecycle_controller_test.go index ace31b8..5ee795d 100644 --- a/internal/controller/gardener_node_lifecycle_controller_test.go +++ b/internal/controller/gardener_node_lifecycle_controller_test.go @@ -147,6 +147,74 @@ var _ = Describe("Gardener Maintenance Controller", func() { _, err := controller.Reconcile(ctx, reconcileReq) Expect(err).NotTo(HaveOccurred()) + } + }) + + Context("setting the pod disruption budget", func() { + When("not evicted", func() { + It("should set the pdb to minimum 1", func() { + pdb := &policyv1.PodDisruptionBudget{} + Expect(k8sClient.Get(ctx, podName, pdb)).To(Succeed()) + Expect(pdb.Spec.MinAvailable).To(HaveValue(HaveField("IntVal", BeEquivalentTo(1)))) + }) + }) + When("evicted", func() { + BeforeEach(func() { + hv := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeEvicting, + Status: metav1.ConditionFalse, + Reason: "dontcare", + Message: "dontcare", + }) + Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) + }) + + It("should set the pdb to minimum 0", func() { + pdb := &policyv1.PodDisruptionBudget{} + Expect(k8sClient.Get(ctx, podName, pdb)).To(Succeed()) + Expect(pdb.Spec.MinAvailable).To(HaveValue(HaveField("IntVal", BeEquivalentTo(0)))) + }) + }) + }) + + Context("create a signalling deployment", func() { + When("onboarding not completed", func() { + It("should create a failing deployment for the node", func() { + deployment := &appsv1.Deployment{} + Expect(k8sClient.Get(ctx, podName, deployment)).To(Succeed()) + Expect(deployment.Spec.Template.Spec.Containers).To( + ContainElement( + HaveField("StartupProbe", + HaveField("ProbeHandler", + HaveField("Exec", + HaveField("Command", ContainElements("/bin/false"))))))) + }) + }) + When("onboarding is completed", func() { + BeforeEach(func() { + hv := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: ConditionTypeOnboarding, + Status: metav1.ConditionFalse, + Reason: "dontcare", + Message: "dontcare", + }) + Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) + }) + + It("should create a succeeding deployment for the node", func() { + deployment := &appsv1.Deployment{} + Expect(k8sClient.Get(ctx, podName, deployment)).To(Succeed()) + Expect(deployment.Spec.Template.Spec.Containers).To( + ContainElement( + HaveField("StartupProbe", + HaveField("ProbeHandler", + HaveField("Exec", + HaveField("Command", ContainElements("/bin/true"))))))) + }) }) }) @@ -164,31 +232,30 @@ var _ = Describe("Gardener Maintenance Controller", func() { }) }) - Context("When node is terminating and offboarded", func() { + Context("When hypervisor is in termination maintenance and offboarded", func() { BeforeEach(func(ctx SpecContext) { - // Set node as terminating and add required labels for disableInstanceHA - node := &corev1.Node{} - Expect(k8sClient.Get(ctx, name, node)).To(Succeed()) - node.Labels = map[string]string{ - corev1.LabelHostname: nodeName, - "topology.kubernetes.io/zone": "test-zone", - } - node.Status.Conditions = append(node.Status.Conditions, corev1.NodeCondition{ - Type: "Terminating", - Status: corev1.ConditionTrue, - }) - Expect(k8sClient.Update(ctx, node)).To(Succeed()) - Expect(k8sClient.Status().Update(ctx, node)).To(Succeed()) - - // Set hypervisor as onboarded and offboarded hypervisor := &kvmv1.Hypervisor{} Expect(k8sClient.Get(ctx, name, hypervisor)).To(Succeed()) + hypervisor.Spec.Maintenance = kvmv1.MaintenanceTermination + Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed()) + meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeEvicting, + Status: metav1.ConditionFalse, + Reason: "Succeeded", + Message: "All VMs evicted", + }) meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeOnboarding, Status: metav1.ConditionFalse, Reason: "Onboarded", Message: "Onboarding completed", }) + meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeHaEnabled, + Status: metav1.ConditionFalse, + Reason: "Evicted", + Message: "HA disabled", + }) meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeOffboarded, Status: metav1.ConditionTrue, @@ -199,8 +266,12 @@ var _ = Describe("Gardener Maintenance Controller", func() { }) It("should allow pod eviction by setting the PDB to minAvailable 0", func(ctx SpecContext) { + // First reconcile applies the offboarding taint and returns early. _, err := controller.Reconcile(ctx, reconcileReq) Expect(err).NotTo(HaveOccurred()) + // Second reconcile proceeds to update the PDB. + _, err = controller.Reconcile(ctx, reconcileReq) + Expect(err).NotTo(HaveOccurred()) pdb := &policyv1.PodDisruptionBudget{} Expect(k8sClient.Get(ctx, maintenanceName, pdb)).To(Succeed()) From e0a405b3d0f83399a5fc6b228622c43c16de56df Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Thu, 9 Jul 2026 10:42:35 +0200 Subject: [PATCH 3/3] Fix test compilation and runtime errors in GardenerNodeLifecycle tests - Add missing ctx SpecContext parameter to BeforeEach/It callbacks - Fix undefined variable references (podName -> maintenanceName, hypervisorName -> name) - Fix missing closing paren in It block - Qualify ConditionTypeOnboarding with kvmv1 package prefix - Add JustBeforeEach reconcile calls to PDB and deployment contexts so assertions run against actual controller output --- .../gardener_node_lifecycle_controller.go | 29 ------ ...gardener_node_lifecycle_controller_test.go | 93 ++++++------------- 2 files changed, 28 insertions(+), 94 deletions(-) diff --git a/internal/controller/gardener_node_lifecycle_controller.go b/internal/controller/gardener_node_lifecycle_controller.go index 15146d0..aeea167 100644 --- a/internal/controller/gardener_node_lifecycle_controller.go +++ b/internal/controller/gardener_node_lifecycle_controller.go @@ -35,9 +35,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" - "sigs.k8s.io/controller-runtime/pkg/event" logger "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/predicate" kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" ) @@ -252,30 +250,3 @@ func (r *GardenerNodeLifecycleController) SetupWithManager(mgr ctrl.Manager, nam Owns(&policyv1.PodDisruptionBudget{}). Complete(r) } - -// evictingConditionChangedPredicate complements GenerationChangedPredicate, -// which ignores status-only updates. -type evictingConditionChangedPredicate struct { - predicate.Funcs -} - -func (evictingConditionChangedPredicate) Update(e event.UpdateEvent) bool { - if e.ObjectOld == nil || e.ObjectNew == nil { - return false - } - oldHv, ok1 := e.ObjectOld.(*kvmv1.Hypervisor) - newHv, ok2 := e.ObjectNew.(*kvmv1.Hypervisor) - if !ok1 || !ok2 { - return false - } - oldCond := meta.FindStatusCondition(oldHv.Status.Conditions, kvmv1.ConditionTypeEvicting) - newCond := meta.FindStatusCondition(newHv.Status.Conditions, kvmv1.ConditionTypeEvicting) - switch { - case oldCond == nil && newCond == nil: - return false - case oldCond == nil || newCond == nil: - return true - default: - return oldCond.Status != newCond.Status - } -} diff --git a/internal/controller/gardener_node_lifecycle_controller_test.go b/internal/controller/gardener_node_lifecycle_controller_test.go index 5ee795d..45e3175 100644 --- a/internal/controller/gardener_node_lifecycle_controller_test.go +++ b/internal/controller/gardener_node_lifecycle_controller_test.go @@ -58,7 +58,7 @@ var _ = Describe("Gardener Maintenance Controller", func() { Expect(k8sClient.Create(ctx, node)).To(Succeed()) DeferCleanup(func(ctx SpecContext) { By("Cleanup the specific node") - Expect(k8sClient.Delete(ctx, node)).To(Succeed()) + Expect(k8sclient.IgnoreNotFound(k8sClient.Delete(ctx, node))).To(Succeed()) }) By("creating the core resource for the Kind hypervisor") @@ -116,6 +116,26 @@ var _ = Describe("Gardener Maintenance Controller", func() { }) }) + When("evicting has completed (Evicting=False)", func() { + BeforeEach(func(ctx SpecContext) { + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, name, hypervisor)).To(Succeed()) + meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeEvicting, + Status: metav1.ConditionFalse, + Reason: "Succeeded", + Message: "All VMs evicted", + }) + Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed()) + }) + + It("should set the pdb to minAvailable 0 without waiting for Offboarded", func(ctx SpecContext) { + pdb := &policyv1.PodDisruptionBudget{} + Expect(k8sClient.Get(ctx, maintenanceName, pdb)).To(Succeed()) + Expect(pdb.Spec.MinAvailable).To(HaveField("IntVal", BeNumerically("==", int32(0)))) + }) + }) + When("the node has been offboarded", func() { BeforeEach(func(ctx SpecContext) { hypervisor := &kvmv1.Hypervisor{} @@ -147,74 +167,17 @@ var _ = Describe("Gardener Maintenance Controller", func() { _, err := controller.Reconcile(ctx, reconcileReq) Expect(err).NotTo(HaveOccurred()) - } - }) - - Context("setting the pod disruption budget", func() { - When("not evicted", func() { - It("should set the pdb to minimum 1", func() { - pdb := &policyv1.PodDisruptionBudget{} - Expect(k8sClient.Get(ctx, podName, pdb)).To(Succeed()) - Expect(pdb.Spec.MinAvailable).To(HaveValue(HaveField("IntVal", BeEquivalentTo(1)))) - }) - }) - When("evicted", func() { - BeforeEach(func() { - hv := &kvmv1.Hypervisor{} - Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) - meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeEvicting, - Status: metav1.ConditionFalse, - Reason: "dontcare", - Message: "dontcare", - }) - Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) - }) - - It("should set the pdb to minimum 0", func() { - pdb := &policyv1.PodDisruptionBudget{} - Expect(k8sClient.Get(ctx, podName, pdb)).To(Succeed()) - Expect(pdb.Spec.MinAvailable).To(HaveValue(HaveField("IntVal", BeEquivalentTo(0)))) - }) }) }) - Context("create a signalling deployment", func() { - When("onboarding not completed", func() { - It("should create a failing deployment for the node", func() { - deployment := &appsv1.Deployment{} - Expect(k8sClient.Get(ctx, podName, deployment)).To(Succeed()) - Expect(deployment.Spec.Template.Spec.Containers).To( - ContainElement( - HaveField("StartupProbe", - HaveField("ProbeHandler", - HaveField("Exec", - HaveField("Command", ContainElements("/bin/false"))))))) - }) - }) - When("onboarding is completed", func() { - BeforeEach(func() { - hv := &kvmv1.Hypervisor{} - Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) - meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: ConditionTypeOnboarding, - Status: metav1.ConditionFalse, - Reason: "dontcare", - Message: "dontcare", - }) - Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) - }) + Context("When node does not exist", func() { + It("should succeed without error", func(ctx SpecContext) { + node := &corev1.Node{} + Expect(k8sClient.Get(ctx, name, node)).To(Succeed()) + Expect(k8sClient.Delete(ctx, node)).To(Succeed()) - It("should create a succeeding deployment for the node", func() { - deployment := &appsv1.Deployment{} - Expect(k8sClient.Get(ctx, podName, deployment)).To(Succeed()) - Expect(deployment.Spec.Template.Spec.Containers).To( - ContainElement( - HaveField("StartupProbe", - HaveField("ProbeHandler", - HaveField("Exec", - HaveField("Command", ContainElements("/bin/true"))))))) - }) + _, err := controller.Reconcile(ctx, reconcileReq) + Expect(err).NotTo(HaveOccurred()) }) })