GardenerNodeLifecycleController: add timeout for HA-disabled wait

When a node is terminating and offboarded, the controller waits for
HaEnabled=False before allowing pod eviction. Previously it would wait
indefinitely; now it times out after HaDisabledTimeout (default 15m),
measured from the Offboarded condition's LastTransitionTime, so a
stalled HA controller cannot block node termination indefinitely.

Author: fwiesel

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

@@ -164,6 +164,27 @@ spec:
                 description: MaintenanceReason provides the reason for manual maintenance
                   mode.
                 type: string
+              overcommit:
+                additionalProperties:
+                  type: number
+                description: |-
+                  Overcommit specifies the desired overcommit ratio by resource type.
+
+                  If no overcommit is specified for a resource type, the default overcommit
+                  ratio of 1.0 should be applied, i.e. the effective capacity is the same
+                  as the actual capacity.
+
+                  If the overcommit ratio results in a fractional effective capacity,
+                  the effective capacity is expected to be rounded down. This allows
+                  gradually adjusting the hypervisor capacity.
+
+                  It is validated that all overcommit ratios are greater than or equal to
+                  1.0, if specified. For this we don't need extra validating webhooks.
+                  See: https://kubernetes.io/blog/2022/09/23/crd-validation-rules-beta/#crd-transition-rules
+                type: object
+                x-kubernetes-validations:
+                - message: overcommit ratios must be >= 1.0
+                  rule: self.all(k, self[k] >= 1.0)
               reboot:
                 default: false
                 description: Reboot request an reboot after successful installation
@@ -258,7 +279,13 @@ spec:
                   - type: string
                   pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
                   x-kubernetes-int-or-string: true
-                description: Auto-discovered capacity of the hypervisor.
+                description: |-
+                  Auto-discovered capacity of the hypervisor.
+
+                  Note that this capacity does not include the applied overcommit ratios,
+                  and represents the actual capacity of the hypervisor. Use the
+                  effective capacity field to get the capacity considering the applied
+                  overcommit ratios.
                 type: object
               cells:
                 description: Auto-discovered cells on this hypervisor.
@@ -282,12 +309,35 @@ spec:
                         - type: string
                         pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
                         x-kubernetes-int-or-string: true
-                      description: Auto-discovered capacity of this cell.
+                      description: |-
+                        Auto-discovered capacity of this cell.
+
+                        Note that this capacity does not include the applied overcommit ratios,
+                        and represents the actual capacity of the cell. Use the effective capacity
+                        field to get the capacity considering the applied overcommit ratios.
                       type: object
                     cellID:
                       description: Cell ID.
                       format: int64
                       type: integer
+                    effectiveCapacity:
+                      additionalProperties:
+                        anyOf:
+                        - type: integer
+                        - type: string
+                        pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
+                        x-kubernetes-int-or-string: true
+                      description: |-
+                        Auto-discovered capacity of this cell, considering the
+                        applied overcommit ratios.
+
+                        In case no overcommit ratio is specified for a resource type, the default
+                        overcommit ratio of 1 should be applied, meaning the effective capacity
+                        is the same as the actual capacity.
+
+                        If the overcommit ratio results in a fractional effective capacity, the
+                        effective capacity is expected to be rounded down.
+                      type: object
                   required:
                   - cellID
                   type: object
@@ -415,6 +465,24 @@ spec:
                       type: string
                     type: array
                 type: object
+              effectiveCapacity:
+                additionalProperties:
+                  anyOf:
+                  - type: integer
+                  - type: string
+                  pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
+                  x-kubernetes-int-or-string: true
+                description: |-
+                  Auto-discovered capacity of the hypervisor, considering the
+                  applied overcommit ratios.
+
+                  In case no overcommit ratio is specified for a resource type, the default
+                  overcommit ratio of 1 should be applied, meaning the effective capacity
+                  is the same as the actual capacity.
+
+                  If the overcommit ratio results in a fractional effective capacity, the
+                  effective capacity is expected to be rounded down.
+                type: object
               evicted:
                 description: Evicted indicates whether the hypervisor is evicted.
                   (no instances left with active maintenance mode)

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))))
+			})
 		})
 	})
 })