GardenerNodeLifecycleController: add timeout for HA-disabled wait

go.mod

@@ -104,7 +104,7 @@ require (
 	k8s.io/component-base v0.35.0 // indirect
 	k8s.io/klog/v2 v2.130.1 // indirect
 	k8s.io/kube-openapi v0.0.0-20250910181357-589584f1c912 // indirect
-	k8s.io/utils v0.0.0-20251002143259-bc988d571ff4 // indirect
+	k8s.io/utils v0.0.0-20251002143259-bc988d571ff4
 	sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.33.0 // indirect
 	sigs.k8s.io/gateway-api v1.4.0 // indirect
 	sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 // indirect

internal/controller/gardener_node_lifecycle_controller.go

@@ -21,6 +21,7 @@ import (
 	"context"
 	"fmt"
 	"maps"
+	"time"
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -32,6 +33,7 @@ import (
 	corev1ac "k8s.io/client-go/applyconfigurations/core/v1"
 	v1 "k8s.io/client-go/applyconfigurations/meta/v1"
 	policyv1ac "k8s.io/client-go/applyconfigurations/policy/v1"
+	"k8s.io/utils/clock"
 	ctrl "sigs.k8s.io/controller-runtime"
 	k8sclient "sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
@@ -40,10 +42,19 @@ import (
 	kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1"
 )
 
+const defaultHaDisabledTimeout = 15 * time.Minute
+
 type GardenerNodeLifecycleController struct {
 	k8sclient.Client
 	Scheme    *runtime.Scheme
 	namespace string
+	// HaDisabledTimeout is the maximum time to wait for ConditionTypeHaEnabled to
+	// become False after the node is offboarded, measured from the lastTransitionTime
+	// of the ConditionTypeOffboarded condition. After the deadline, Unknown and unset
+	// are also accepted. Defaults to defaultHaDisabledTimeout when zero.
+	HaDisabledTimeout time.Duration
+	// Clock is used to determine the current time. Defaults to clock.RealClock{}.
+	Clock clock.Clock
 }
 
 const (
@@ -84,18 +95,34 @@ func (r *GardenerNodeLifecycleController) Reconcile(ctx context.Context, req ctr
 	// We do not care about the particular value, as long as it isn't an error
 	var minAvailable int32 = 1
 
-	// Onboarding is not in progress anymore, i.e. the host is onboarded
+	// Onboarding is not in progress, 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
+	// Evicting is not in progress, i.e. the host is empty
 	offboarded := meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeOffboarded)
 
 	if offboarded {
 		minAvailable = 0
 
 		if onboardingCompleted && isTerminating(node) {
-			// Wait for HypervisorInstanceHa controller to disable HA
+			// Wait for HypervisorInstanceHa controller to disable HA.
+			// After the deadline (measured from the lastTransitionTime of the
+			// Offboarded condition) we also accept Unknown and unset, so that a
+			// stalled HA controller cannot block node termination indefinitely.
 			if !meta.IsStatusConditionFalse(hv.Status.Conditions, kvmv1.ConditionTypeHaEnabled) {
-				return ctrl.Result{}, nil // Will be reconciled again when condition changes
+				offboardedCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOffboarded)
+				timeout := r.HaDisabledTimeout
+				if timeout == 0 {
+					timeout = defaultHaDisabledTimeout
+				}
+				now := r.Clock.Now()
+				offboardedAt := offboardedCondition.LastTransitionTime.Time
+				deadline := offboardedAt.Add(timeout)
+				if now.Before(deadline) {
+					return ctrl.Result{RequeueAfter: deadline.Sub(now)}, nil
+				}
+				log.Info("HA disabled timeout exceeded, proceeding without waiting for HaEnabled=False",
+					"timeout", timeout,
+					"offboardedAt", offboardedAt)
 			}
 		}
 	}
@@ -221,7 +248,9 @@ func (r *GardenerNodeLifecycleController) SetupWithManager(mgr ctrl.Manager, nam
 	ctx := context.Background()
 	_ = logger.FromContext(ctx)
 	r.namespace = namespace
-
+	if r.Clock == nil {
+		r.Clock = clock.RealClock{}
+	}
 	return ctrl.NewControllerManagedBy(mgr).
 		Named(MaintenanceControllerName).
 		For(&corev1.Node{}).

internal/controller/gardener_node_lifecycle_controller_test.go

@@ -19,6 +19,7 @@ package controller
 
 import (
 	"fmt"
+	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -28,6 +29,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/utils/clock"
 	ctrl "sigs.k8s.io/controller-runtime"
 	k8sclient "sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -47,6 +49,7 @@ var _ = Describe("Gardener Maintenance Controller", func() {
 		controller = &GardenerNodeLifecycleController{
 			Client: k8sClient,
 			Scheme: k8sClient.Scheme(),
+			Clock:  clock.RealClock{},
 		}
 
 		By("creating the core resource for the Kind Node")
@@ -166,18 +169,21 @@ var _ = Describe("Gardener Maintenance Controller", func() {
 
 	Context("When node is terminating and offboarded", func() {
 		BeforeEach(func(ctx SpecContext) {
-			// Set node as terminating and add required labels for disableInstanceHA
+			// Set node labels (spec/metadata update)
 			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",
 			}
+			Expect(k8sClient.Update(ctx, node)).To(Succeed())
+			// Set node Terminating condition separately via the status subresource,
+			// using a fresh Get to avoid the spec Update overwriting the status.
+			Expect(k8sClient.Get(ctx, name, node)).To(Succeed())
 			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
@@ -198,13 +204,70 @@ var _ = Describe("Gardener Maintenance Controller", func() {
 			Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
 		})
 
-		It("should allow pod eviction by setting the PDB to minAvailable 0", func(ctx SpecContext) {
-			_, err := controller.Reconcile(ctx, reconcileReq)
-			Expect(err).NotTo(HaveOccurred())
+		When("HaEnabled is explicitly 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.ConditionTypeHaEnabled,
+					Status:  metav1.ConditionFalse,
+					Reason:  "Evicted",
+					Message: "HA disabled due to eviction",
+				})
+				Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
+			})
 
-			pdb := &policyv1.PodDisruptionBudget{}
-			Expect(k8sClient.Get(ctx, maintenanceName, pdb)).To(Succeed())
-			Expect(pdb.Spec.MinAvailable).To(HaveField("IntVal", BeNumerically("==", int32(0))))
+			It("should allow pod eviction immediately by setting the PDB to minAvailable 0", func(ctx SpecContext) {
+				result, err := controller.Reconcile(ctx, reconcileReq)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(result.RequeueAfter).To(BeZero())
+
+				pdb := &policyv1.PodDisruptionBudget{}
+				Expect(k8sClient.Get(ctx, maintenanceName, pdb)).To(Succeed())
+				Expect(pdb.Spec.MinAvailable).To(HaveField("IntVal", BeNumerically("==", int32(0))))
+			})
+		})
+
+		When("HaEnabled is not yet False and the timeout has not elapsed", func() {
+			BeforeEach(func() {
+				// LastTransitionTime ≈ now (set by meta.SetStatusCondition above),
+				// so deadline = now + 1h is in the future.
+				controller.HaDisabledTimeout = time.Hour
+			})
+
+			It("should requeue and not proceed", func(ctx SpecContext) {
+				result, err := controller.Reconcile(ctx, reconcileReq)
+				Expect(err).NotTo(HaveOccurred())
+				// Should requeue before the deadline rather than returning immediately.
+				Expect(result.RequeueAfter).To(BeNumerically(">", 0))
+			})
+		})
+
+		When("HaEnabled is not False but the timeout has elapsed", func() {
+			BeforeEach(func(ctx SpecContext) {
+				// Push LastTransitionTime 2h into the past so that
+				// deadline = (now - 2h) + 1h = now - 1h, which has already elapsed.
+				hypervisor := &kvmv1.Hypervisor{}
+				Expect(k8sClient.Get(ctx, name, hypervisor)).To(Succeed())
+				for i := range hypervisor.Status.Conditions {
+					if hypervisor.Status.Conditions[i].Type == kvmv1.ConditionTypeOffboarded {
+						hypervisor.Status.Conditions[i].LastTransitionTime = metav1.NewTime(time.Now().Add(-2 * time.Hour))
+						break
+					}
+				}
+				Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
+				controller.HaDisabledTimeout = time.Hour
+			})
+
+			It("should allow pod eviction by setting the PDB to minAvailable 0", func(ctx SpecContext) {
+				result, err := controller.Reconcile(ctx, reconcileReq)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(result.RequeueAfter).To(BeZero())
+
+				pdb := &policyv1.PodDisruptionBudget{}
+				Expect(k8sClient.Get(ctx, maintenanceName, pdb)).To(Succeed())
+				Expect(pdb.Spec.MinAvailable).To(HaveField("IntVal", BeNumerically("==", int32(0))))
+			})
 		})
 	})
 })