From b72cd200499421b359b007df95efd537ecf5ad0d Mon Sep 17 00:00:00 2001 From: Paul Wallrabe Date: Mon, 1 Jun 2026 13:35:05 +0200 Subject: [PATCH] test: add e2e regression tests for router graceful shutdown patterns Generated-By: Forge/20260601_152141_879822_af7227a0 --- .../test/e2e/router_graceful_shutdown_test.go | 296 ++++++++++++++++++ 1 file changed, 296 insertions(+) create mode 100644 controller/deploy/operator/test/e2e/router_graceful_shutdown_test.go diff --git a/controller/deploy/operator/test/e2e/router_graceful_shutdown_test.go b/controller/deploy/operator/test/e2e/router_graceful_shutdown_test.go new file mode 100644 index 000000000..cd374f612 --- /dev/null +++ b/controller/deploy/operator/test/e2e/router_graceful_shutdown_test.go @@ -0,0 +1,296 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package e2e + +import ( + "fmt" + "math" + "os" + "strconv" + "strings" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" + + operatorv1alpha1 "github.com/jumpstarter-dev/jumpstarter-controller/deploy/operator/api/v1alpha1" +) + +// These tests verify the router deployment configuration that prevents the +// failure patterns described in issue #28: updating the router kills existing +// connections because the deployment lacks proper rollout strategy, health +// probes, and termination grace period. +var _ = Describe("Router graceful shutdown regression (issue #28)", Ordered, Pending, func() { + const baseDomain = "graceful.127.0.0.1.nip.io" + const jumpstarterName = "jumpstarter-graceful" + var gracefulTestNamespace string + + BeforeAll(func() { + gracefulTestNamespace = CreateTestNamespace() + + By("creating a Jumpstarter CR to produce a router deployment") + image := os.Getenv("IMG") + if image == "" { + image = defaultControllerImage + } + + jumpstarterYAML := fmt.Sprintf(`apiVersion: operator.jumpstarter.dev/v1alpha1 +kind: Jumpstarter +metadata: + name: %s + namespace: %s +spec: + baseDomain: %s + authentication: + internal: + prefix: "internal:" + enabled: true + controller: + image: %s + imagePullPolicy: IfNotPresent + replicas: 1 + grpc: + endpoints: + - address: grpc.%s:8082 + nodeport: + enabled: true + port: 30080 + routers: + image: %s + imagePullPolicy: IfNotPresent + replicas: 1 + resources: + requests: + cpu: 100m + memory: 100Mi + grpc: + endpoints: + - address: router.%s:8083 + nodeport: + enabled: true + port: 30081 +`, jumpstarterName, gracefulTestNamespace, baseDomain, image, baseDomain, image, baseDomain) + + err := applyYAML(jumpstarterYAML) + Expect(err).NotTo(HaveOccurred(), "Failed to create Jumpstarter CR") + + By("waiting for the router deployment to exist") + Eventually(func(g Gomega) { + deployment := &appsv1.Deployment{} + err := k8sClient.Get(ctx, types.NamespacedName{ + Name: fmt.Sprintf("%s-router-0", jumpstarterName), + Namespace: gracefulTestNamespace, + }, deployment) + g.Expect(err).NotTo(HaveOccurred()) + }, 2*time.Minute).Should(Succeed()) + }) + + getRouterDeployment := func() *appsv1.Deployment { + deployment := &appsv1.Deployment{} + err := k8sClient.Get(ctx, types.NamespacedName{ + Name: fmt.Sprintf("%s-router-0", jumpstarterName), + Namespace: gracefulTestNamespace, + }, deployment) + Expect(err).NotTo(HaveOccurred()) + return deployment + } + + getRouterContainer := func(deployment *appsv1.Deployment) corev1.Container { + Expect(deployment.Spec.Template.Spec.Containers).NotTo(BeEmpty(), + "router deployment should have at least one container") + return deployment.Spec.Template.Spec.Containers[0] + } + + Context("Rollout strategy prevents connection loss during updates", func() { + It("should use RollingUpdate strategy type", func() { + deployment := getRouterDeployment() + Expect(deployment.Spec.Strategy.Type).To( + Equal(appsv1.RollingUpdateDeploymentStrategyType), + "router deployment must use RollingUpdate strategy to avoid downtime") + }) + + It("should set maxUnavailable to 0 so the old router stays running until the new one is ready", func() { + deployment := getRouterDeployment() + Expect(deployment.Spec.Strategy.RollingUpdate).NotTo(BeNil(), + "RollingUpdate configuration must be specified") + Expect(deployment.Spec.Strategy.RollingUpdate.MaxUnavailable).NotTo(BeNil(), + "maxUnavailable must be explicitly set") + + maxUnavailable := deployment.Spec.Strategy.RollingUpdate.MaxUnavailable + Expect(maxUnavailable).To(Equal(&intstr.IntOrString{Type: intstr.Int, IntVal: 0}), + "maxUnavailable must be 0 so existing router pods are not terminated "+ + "before their replacement is ready -- otherwise active connections are dropped") + }) + + It("should set maxSurge to at least 1 so the new router starts before the old one stops", func() { + deployment := getRouterDeployment() + Expect(deployment.Spec.Strategy.RollingUpdate).NotTo(BeNil(), + "RollingUpdate configuration must be specified") + Expect(deployment.Spec.Strategy.RollingUpdate.MaxSurge).NotTo(BeNil(), + "maxSurge must be explicitly set") + + maxSurge := deployment.Spec.Strategy.RollingUpdate.MaxSurge + if maxSurge.Type == intstr.Int { + Expect(maxSurge.IntVal).To(BeNumerically(">=", 1), + "maxSurge must be >= 1 so a new router pod is created before the old one is terminated") + } else { + pctStr := strings.TrimSuffix(maxSurge.StrVal, "%") + pct, err := strconv.ParseFloat(pctStr, 64) + Expect(err).NotTo(HaveOccurred(), "maxSurge percentage must be a valid number") + + replicas := int32(1) + if deployment.Spec.Replicas != nil { + replicas = *deployment.Spec.Replicas + } + effectiveSurge := int(math.Ceil(float64(replicas) * pct / 100.0)) + Expect(effectiveSurge).To(BeNumerically(">=", 1), + "maxSurge percentage must resolve to at least 1 pod of surge "+ + "(got %q with %d replicas = %d effective surge)", + maxSurge.StrVal, replicas, effectiveSurge) + } + }) + }) + + Context("Health probes allow the cluster to route traffic correctly", func() { + It("should have a readiness probe so the cluster stops sending new connections to a stopping router", func() { + deployment := getRouterDeployment() + container := getRouterContainer(deployment) + Expect(container.ReadinessProbe).NotTo(BeNil(), + "router container must have a readiness probe -- without it the cluster "+ + "cannot detect when a router is shutting down and will continue sending "+ + "new connections to it") + }) + + It("should have a liveness probe so the cluster can detect and restart a stuck router", func() { + deployment := getRouterDeployment() + container := getRouterContainer(deployment) + Expect(container.LivenessProbe).NotTo(BeNil(), + "router container must have a liveness probe -- without it a hung router "+ + "will never be restarted, silently dropping connections") + }) + }) + + Context("Connection draining mechanism", func() { + It("should have a preStop hook to initiate connection draining", func() { + deployment := getRouterDeployment() + container := getRouterContainer(deployment) + Expect(container.Lifecycle).NotTo(BeNil(), + "lifecycle hooks must be configured for graceful shutdown") + Expect(container.Lifecycle.PreStop).NotTo(BeNil(), + "preStop hook is needed to drain connections before the container "+ + "receives SIGTERM -- without it the router stops accepting new "+ + "connections abruptly when killed") + }) + }) + + Context("Termination grace period allows existing connections to drain", func() { + It("should have terminationGracePeriodSeconds long enough for connections to finish", func() { + deployment := getRouterDeployment() + gracePeriod := deployment.Spec.Template.Spec.TerminationGracePeriodSeconds + Expect(gracePeriod).NotTo(BeNil(), + "terminationGracePeriodSeconds must be set explicitly") + + // Issue #28 recommends 1-2 hours (3600-7200 seconds) to allow + // long-running lease connections to complete before the router is killed. + Expect(*gracePeriod).To(BeNumerically(">=", 3600), + "terminationGracePeriodSeconds must be at least 3600 (1 hour) to allow "+ + "existing long-running connections to complete before the router is "+ + "forcibly terminated -- 30 seconds is insufficient for lease-bound sessions") + }) + }) + + Context("Parity with controller deployment health configuration", func() { + It("should configure health probes on the same port and paths as the controller", func() { + By("fetching the controller deployment for comparison") + controllerDeployment := &appsv1.Deployment{} + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, types.NamespacedName{ + Name: fmt.Sprintf("%s-controller", jumpstarterName), + Namespace: gracefulTestNamespace, + }, controllerDeployment) + g.Expect(err).NotTo(HaveOccurred()) + }, 2*time.Minute).Should(Succeed()) + + controllerContainer := controllerDeployment.Spec.Template.Spec.Containers[0] + Expect(controllerContainer.ReadinessProbe).NotTo(BeNil(), + "controller should have readiness probe for comparison baseline") + Expect(controllerContainer.LivenessProbe).NotTo(BeNil(), + "controller should have liveness probe for comparison baseline") + + By("verifying the router uses the same health configuration as the controller") + routerDeployment := getRouterDeployment() + routerContainer := getRouterContainer(routerDeployment) + + Expect(routerContainer.ReadinessProbe).NotTo(BeNil(), + "router must have a readiness probe like the controller does") + Expect(routerContainer.LivenessProbe).NotTo(BeNil(), + "router must have a liveness probe like the controller does") + + if controllerContainer.ReadinessProbe.HTTPGet != nil { + Expect(routerContainer.ReadinessProbe.HTTPGet).NotTo(BeNil(), + "router readiness probe should use HTTPGet like the controller") + Expect(routerContainer.ReadinessProbe.HTTPGet.Path).To( + Equal(controllerContainer.ReadinessProbe.HTTPGet.Path), + "router readiness probe path should match controller readiness probe path") + } + + if controllerContainer.LivenessProbe.HTTPGet != nil { + Expect(routerContainer.LivenessProbe.HTTPGet).NotTo(BeNil(), + "router liveness probe should use HTTPGet like the controller") + Expect(routerContainer.LivenessProbe.HTTPGet.Path).To( + Equal(controllerContainer.LivenessProbe.HTTPGet.Path), + "router liveness probe path should match controller liveness probe path") + } + + routerHealthPort := int32(0) + for _, port := range routerContainer.Ports { + if port.Name == "health" { + routerHealthPort = port.ContainerPort + break + } + } + Expect(routerHealthPort).NotTo(BeZero(), + "router should expose a health port for probes to target") + }) + }) + + AfterAll(func() { + By("cleaning up the Jumpstarter CR") + jumpstarter := &operatorv1alpha1.Jumpstarter{} + err := k8sClient.Get(ctx, types.NamespacedName{ + Name: jumpstarterName, + Namespace: gracefulTestNamespace, + }, jumpstarter) + if err == nil { + Expect(k8sClient.Delete(ctx, jumpstarter)).To(Succeed()) + Eventually(func(g Gomega) { + getErr := k8sClient.Get(ctx, types.NamespacedName{ + Name: jumpstarterName, + Namespace: gracefulTestNamespace, + }, jumpstarter) + g.Expect(apierrors.IsNotFound(getErr)).To(BeTrue()) + }, 2*time.Minute).Should(Succeed()) + } + + DeleteTestNamespace(gracefulTestNamespace) + }) +})