GardenerNodeLifecycleController: add timeout for HA-disabled wait

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 (
@@ -105,9 +116,25 @@ func (r *GardenerNodeLifecycleController) Reconcile(ctx context.Context, req ctr
 
 		onboardingCompleted := meta.IsStatusConditionFalse(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding)
 		if onboardingCompleted && hv.Spec.Maintenance == kvmv1.MaintenanceTermination {
-			// 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)
 			}
 		}
 	}
@@ -242,6 +269,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).

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")
@@ -197,6 +200,7 @@ var _ = Describe("Gardener Maintenance Controller", func() {
 
 	Context("When hypervisor is in termination maintenance and offboarded", func() {
 		BeforeEach(func(ctx SpecContext) {
+
 			hypervisor := &kvmv1.Hypervisor{}
 			Expect(k8sClient.Get(ctx, name, hypervisor)).To(Succeed())
 			hypervisor.Spec.Maintenance = kvmv1.MaintenanceTermination
@@ -228,17 +232,100 @@ 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) {
-			// 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())
+		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) {
+				// 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())
+				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(ctx SpecContext) {
+				// Override: set HaEnabled=True so the timeout path is exercised.
+				hypervisor := &kvmv1.Hypervisor{}
+				Expect(k8sClient.Get(ctx, name, hypervisor)).To(Succeed())
+				meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{
+					Type:    kvmv1.ConditionTypeHaEnabled,
+					Status:  metav1.ConditionTrue,
+					Reason:  "Enabled",
+					Message: "HA still enabled",
+				})
+				Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
+				// 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) {
+				// First reconcile applies the offboarding taint and returns early.
+				_, err := controller.Reconcile(ctx, reconcileReq)
+				Expect(err).NotTo(HaveOccurred())
+				// Second reconcile reaches the HA timeout check and requeues.
+				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) {
+				// Override: set HaEnabled=True and push Offboarded's LastTransitionTime
+				// 2h into the past so that deadline = (now-2h) + 1h = now-1h (elapsed).
+				hypervisor := &kvmv1.Hypervisor{}
+				Expect(k8sClient.Get(ctx, name, hypervisor)).To(Succeed())
+				meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{
+					Type:    kvmv1.ConditionTypeHaEnabled,
+					Status:  metav1.ConditionTrue,
+					Reason:  "Enabled",
+					Message: "HA still enabled",
+				})
+				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) {
+				// First reconcile applies the offboarding taint and returns early.
+				_, err := controller.Reconcile(ctx, reconcileReq)
+				Expect(err).NotTo(HaveOccurred())
+				// Second reconcile: timeout elapsed, proceeds past HA check to update PDB.
+				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))))
+			})
 		})
 	})