diff --git a/internal/controller/postgres/assets_test.go b/internal/controller/postgres/assets_test.go index a26866cd5..b7e98ebde 100644 --- a/internal/controller/postgres/assets_test.go +++ b/internal/controller/postgres/assets_test.go @@ -87,6 +87,18 @@ var _ = Describe("App postgres server assets", func() { Expect(dep.Spec.Selector.MatchLabels).To(Equal(utils.GeneratePostgresSelectorLabels())) Expect(dep.Spec.RevisionHistoryLimit).To(Equal(&revisionHistoryLimit)) Expect(dep.Spec.Replicas).To(Equal(&replicas)) + // Verify preStop lifecycle hook for clean postgres shutdown + Expect(dep.Spec.Template.Spec.Containers[0].Lifecycle).NotTo(BeNil()) + Expect(dep.Spec.Template.Spec.Containers[0].Lifecycle.PreStop).NotTo(BeNil()) + Expect(dep.Spec.Template.Spec.Containers[0].Lifecycle.PreStop.Exec).NotTo(BeNil()) + Expect(dep.Spec.Template.Spec.Containers[0].Lifecycle.PreStop.Exec.Command).To(Equal([]string{ + "/bin/sh", "-c", + "pg_ctl stop -D /var/lib/pgsql/data/userdata -m fast -w -t 55", + })) + // Verify terminationGracePeriodSeconds gives postgres time to shut down + Expect(dep.Spec.Template.Spec.TerminationGracePeriodSeconds).NotTo(BeNil()) + Expect(*dep.Spec.Template.Spec.TerminationGracePeriodSeconds).To(Equal(int64(60))) + Expect(dep.Spec.Template.Spec.Containers[0].VolumeMounts).To(Equal([]corev1.VolumeMount{ { Name: "secret-" + utils.PostgresCertsSecretName, diff --git a/internal/controller/postgres/deployment.go b/internal/controller/postgres/deployment.go index fcc2e5351..7811cf5ce 100644 --- a/internal/controller/postgres/deployment.go +++ b/internal/controller/postgres/deployment.go @@ -234,10 +234,21 @@ func GeneratePostgresDeployment(r reconciler.Reconciler, ctx context.Context, cr Value: strconv.Itoa(cr.Spec.OLSConfig.ConversationCache.Postgres.MaxConnections), }, }, + Lifecycle: &corev1.Lifecycle{ + PreStop: &corev1.LifecycleHandler{ + Exec: &corev1.ExecAction{ + Command: []string{ + "/bin/sh", "-c", + "pg_ctl stop -D /var/lib/pgsql/data/userdata -m fast -w -t 55", + }, + }, + }, + }, }, }, - Volumes: volumes, - ServiceAccountName: utils.PostgreServiceAccountName, + Volumes: volumes, + ServiceAccountName: utils.PostgreServiceAccountName, + TerminationGracePeriodSeconds: &[]int64{60}[0], }, }, RevisionHistoryLimit: &revisionHistoryLimit, diff --git a/internal/controller/utils/utils.go b/internal/controller/utils/utils.go index 9993602d6..19f70526c 100644 --- a/internal/controller/utils/utils.go +++ b/internal/controller/utils/utils.go @@ -199,7 +199,8 @@ func DeploymentSpecEqual(a, b *appsv1.DeploymentSpec, compareInitContainers bool !apiequality.Semantic.DeepEqual(a.Strategy, b.Strategy) || // check strategy !PodVolumeEqual(a.Template.Spec.Volumes, b.Template.Spec.Volumes) || // check volumes *a.Replicas != *b.Replicas || // check replicas - a.Template.Spec.ServiceAccountName != b.Template.Spec.ServiceAccountName { // check service account name + a.Template.Spec.ServiceAccountName != b.Template.Spec.ServiceAccountName || // check service account name + !apiequality.Semantic.DeepEqual(a.Template.Spec.TerminationGracePeriodSeconds, b.Template.Spec.TerminationGracePeriodSeconds) { // check termination grace period return false } @@ -244,7 +245,8 @@ func ContainerSpecEqual(a, b *corev1.Container) bool { a.ImagePullPolicy == b.ImagePullPolicy && // check image pull policy ProbeEqual(a.LivenessProbe, b.LivenessProbe) && // check liveness probe ProbeEqual(a.ReadinessProbe, b.ReadinessProbe) && // check readiness probe - ProbeEqual(a.StartupProbe, b.StartupProbe)) // check startup probe + ProbeEqual(a.StartupProbe, b.StartupProbe) && // check startup probe + apiequality.Semantic.DeepEqual(a.Lifecycle, b.Lifecycle)) // check lifecycle hooks } // EnvEqual compares two EnvVar slices ignoring order diff --git a/internal/controller/utils/utils_comparison_test.go b/internal/controller/utils/utils_comparison_test.go index f1130d5f6..e0df314e8 100644 --- a/internal/controller/utils/utils_comparison_test.go +++ b/internal/controller/utils/utils_comparison_test.go @@ -127,6 +127,46 @@ var _ = Describe("Container Comparison", func() { It("should handle empty container lists", func() { Expect(ContainersEqual([]corev1.Container{}, []corev1.Container{})).To(BeTrue()) }) + + It("should return false when lifecycle hooks differ", func() { + containers1 := []corev1.Container{ + { + Name: "app", + Image: "myapp:v1", + Lifecycle: &corev1.Lifecycle{ + PreStop: &corev1.LifecycleHandler{ + Exec: &corev1.ExecAction{ + Command: []string{"/bin/sh", "-c", "pg_ctl stop"}, + }, + }, + }, + }, + } + containers2 := []corev1.Container{ + { + Name: "app", + Image: "myapp:v1", + }, + } + Expect(ContainersEqual(containers1, containers2)).To(BeFalse()) + }) + + It("should return true when lifecycle hooks are equal", func() { + lifecycle := &corev1.Lifecycle{ + PreStop: &corev1.LifecycleHandler{ + Exec: &corev1.ExecAction{ + Command: []string{"/bin/sh", "-c", "pg_ctl stop"}, + }, + }, + } + containers1 := []corev1.Container{ + {Name: "app", Image: "myapp:v1", Lifecycle: lifecycle}, + } + containers2 := []corev1.Container{ + {Name: "app", Image: "myapp:v1", Lifecycle: lifecycle.DeepCopy()}, + } + Expect(ContainersEqual(containers1, containers2)).To(BeTrue()) + }) }) }) @@ -270,6 +310,21 @@ var _ = Describe("Resource Comparison Functions", func() { Expect(DeploymentSpecEqual(deployment1, deployment2, true)).To(BeFalse()) }) + + It("should return false when terminationGracePeriodSeconds differs", func() { + gracePeriod := int64(60) + deployment2.Template.Spec.TerminationGracePeriodSeconds = &gracePeriod + + Expect(DeploymentSpecEqual(deployment1, deployment2, true)).To(BeFalse()) + }) + + It("should return true when terminationGracePeriodSeconds is equal", func() { + gracePeriod := int64(60) + deployment1.Template.Spec.TerminationGracePeriodSeconds = &gracePeriod + deployment2.Template.Spec.TerminationGracePeriodSeconds = &gracePeriod + + Expect(DeploymentSpecEqual(deployment1, deployment2, true)).To(BeTrue()) + }) }) Describe("ServiceEqual", func() { diff --git a/test/e2e/all_features_test.go b/test/e2e/all_features_test.go index 5b97d7d71..dcc70ba79 100644 --- a/test/e2e/all_features_test.go +++ b/test/e2e/all_features_test.go @@ -720,6 +720,20 @@ var _ = Describe("All Features Enabled", Ordered, Label("AllFeatures"), func() { } Expect(foundSharedBuffers).To(BeTrue(), "POSTGRESQL_SHARED_BUFFERS should be set to 512MB") Expect(foundMaxConnections).To(BeTrue(), "POSTGRESQL_MAX_CONNECTIONS should be set to 3000") + + By("Verifying postgres deployment has preStop lifecycle hook") + postgresContainer := postgresDeployment.Spec.Template.Spec.Containers[0] + Expect(postgresContainer.Lifecycle).NotTo(BeNil(), "postgres container should have a lifecycle configuration") + Expect(postgresContainer.Lifecycle.PreStop).NotTo(BeNil(), "postgres container should have a preStop hook") + Expect(postgresContainer.Lifecycle.PreStop.Exec).NotTo(BeNil(), "preStop hook should use exec") + Expect(postgresContainer.Lifecycle.PreStop.Exec.Command).To(ContainElement(ContainSubstring("pg_ctl stop")), + "preStop hook should run pg_ctl stop for clean shutdown") + + By("Verifying postgres deployment has terminationGracePeriodSeconds") + Expect(postgresDeployment.Spec.Template.Spec.TerminationGracePeriodSeconds).NotTo(BeNil(), + "postgres pod should have terminationGracePeriodSeconds set") + Expect(*postgresDeployment.Spec.Template.Spec.TerminationGracePeriodSeconds).To(Equal(int64(60)), + "postgres pod should have 60s termination grace period") }) // Test 10: Resource Limits Validation