diff --git a/internal/controller/gardener_node_lifecycle_controller.go b/internal/controller/gardener_node_lifecycle_controller.go index aeea167..06652de 100644 --- a/internal/controller/gardener_node_lifecycle_controller.go +++ b/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). diff --git a/internal/controller/gardener_node_lifecycle_controller_test.go b/internal/controller/gardener_node_lifecycle_controller_test.go index 45e3175..34c6b84 100644 --- a/internal/controller/gardener_node_lifecycle_controller_test.go +++ b/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)))) + }) }) })