diff --git a/go.mod b/go.mod index 6953213..5f34b70 100644 --- a/go.mod +++ b/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 diff --git a/internal/controller/gardener_node_lifecycle_controller.go b/internal/controller/gardener_node_lifecycle_controller.go index a59ed9b..373fe8f 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 ( @@ -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{}). diff --git a/internal/controller/gardener_node_lifecycle_controller_test.go b/internal/controller/gardener_node_lifecycle_controller_test.go index 205b51f..75a4423 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") @@ -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)))) + }) }) }) })