From b97d3760cd01a81e1caa568a1b3a6f2272b97dc9 Mon Sep 17 00:00:00 2001 From: Kevin Rudde Date: Wed, 29 Jul 2026 12:24:50 +0200 Subject: [PATCH 1/2] feat: add LOCK_DSN support, rework probes & optimize php settings fix: merge volumeMounts feat: add LOCK_DSN support and optimized php settings --- api/v1/store.go | 17 +++++++- api/v1/store_env.go | 55 ++++++++++++++++++++++++-- api/v1/store_test.go | 4 +- api/v1/zz_generated.deepcopy.go | 17 ++++++++ internal/deployment/admin_test.go | 9 +++-- internal/deployment/storefront.go | 24 +++++++++-- internal/deployment/storefront_test.go | 21 +++++++--- internal/deployment/worker.go | 10 ++++- internal/deployment/worker_test.go | 9 +++-- internal/job/migration_test.go | 9 +++-- internal/job/setup_test.go | 9 +++-- 11 files changed, 149 insertions(+), 35 deletions(-) diff --git a/api/v1/store.go b/api/v1/store.go index 6cfe301..eaaf014 100644 --- a/api/v1/store.go +++ b/api/v1/store.go @@ -95,6 +95,9 @@ type StoreSpec struct { // +kubebuilder:default={adapter: "builtin"} AppCache AppCacheSpec `json:"appCache"` + // +kubebuilder:default={adapter: "builtin"} + Lock LockSpec `json:"lock"` + // +kubebuilder:default={adapter: "builtin"} Worker WorkerSpec `json:"worker"` @@ -346,6 +349,16 @@ type AppCacheSpec struct { Adapter string `json:"adapter"` } +// LockSpec configures Symfony's lock store. With adapter "redis" the operator +// sets LOCK_DSN so locks are shared across pods; "builtin" leaves the image +// default (per-pod flock, unsafe with >1 replica). +type LockSpec struct { + RedisSpec `json:",inline"` + + // +kubebuilder:validation:Enum=builtin;redis + Adapter string `json:"adapter"` +} + type RedisSpec struct { RedisDSN string `json:"redisDsn,omitempty"` RedisHost string `json:"redisHost,omitempty"` @@ -486,13 +499,13 @@ func (c *ContainerSpec) Merge(from ContainerMergeSpec) { c.ExtraEnvs = from.ExtraEnvs } if from.VolumeMounts != nil { - c.VolumeMounts = from.VolumeMounts + c.VolumeMounts = append(c.VolumeMounts, from.VolumeMounts...) } if from.ImagePullSecrets != nil { c.ImagePullSecrets = from.ImagePullSecrets } if from.Volumes != nil { - c.Volumes = from.Volumes + c.Volumes = append(c.Volumes, from.Volumes...) } if from.ServiceAccountName != "" { diff --git a/api/v1/store_env.go b/api/v1/store_env.go index 4dfac7a..b9783cf 100644 --- a/api/v1/store_env.go +++ b/api/v1/store_env.go @@ -176,6 +176,30 @@ func (s *Store) getSessionCache() []corev1.EnvVar { } } +// getLock sets LOCK_DSN from the Lock spec so Symfony's lock store uses shared +// Redis/Valkey instead of the image default (per-pod flock, unsafe with >1 +// replica). Only emitted for adapter "redis". +func (s *Store) getLock() []corev1.EnvVar { + if s.Spec.Lock.Adapter != "redis" { + return nil + } + dsn := s.Spec.Lock.RedisDSN + if dsn == "" { + dsn = fmt.Sprintf( + "redis://%s:%d/%d", + s.Spec.Lock.RedisHost, + s.Spec.Lock.RedisPort, + s.Spec.Lock.RedisIndex, + ) + } + return []corev1.EnvVar{ + { + Name: "LOCK_DSN", + Value: dsn, + }, + } +} + func (f *FPMSpec) getFPMConfiguration() []corev1.EnvVar { if f.ProcessManagement != "dynamic" && f.ProcessManagement != "operator" { return []corev1.EnvVar{ @@ -359,6 +383,14 @@ func (s *Store) getOtel() []corev1.EnvVar { Name: "OTEL_EXPORTER_OTLP_ENDPOINT", Value: s.Spec.Otel.ExporterEndpoint, }, + // Skip tracing probe/monitoring requests entirely (no span, no export). + // The readiness probe (/api/_info/health-check) and the fpm-admin + // endpoints hit workers frequently; tracing them stalls workers on span + // export, inflating php-fpm active_processes and breaking autoscaling. + { + Name: "OTEL_PHP_EXCLUDED_URLS", + Value: "/api/_info/health-check,/-/fpm/", + }, } } return []corev1.EnvVar{} @@ -543,9 +575,21 @@ func (s *Store) GetEnv() []corev1.EnvVar { Name: "SHOPWARE_DBAL_TIMEZONE_SUPPORT_ENABLED", Value: "1", }, + // opcache sizing: Shopware ships ~16k PHP files. The PHP image defaults + // (10000 files / 128MB / 20 interned) overflow the cache, forcing constant + // recompilation. These fit the whole class map. Overridable per-shop via + // extraEnvs (MergeEnv lets container ExtraEnvs win). { - Name: "SQL_SET_DEFAULT_SESSION_VARIABLES", - Value: "0", + Name: "PHP_OPCACHE_MAX_ACCELERATED_FILES", + Value: "20000", + }, + { + Name: "PHP_OPCACHE_MEMORY_CONSUMPTION", + Value: "256", + }, + { + Name: "PHP_OPCACHE_INTERNED_STRINGS_BUFFER", + Value: "64", }, { Name: "INSTALL_LOCALE", @@ -559,9 +603,13 @@ func (s *Store) GetEnv() []corev1.EnvVar { Name: "APP_URL", Value: appUrl, }, + // Non-persistent by default: storefront/admin serve many short-lived + // requests, and persistent connections there hoard DB connections (caused + // [1040] Too many connections under load). The worker deployment overrides + // this to "1" (few long-lived processes benefit from persistent conns). { Name: "DATABASE_PERSISTENT_CONNECTION", - Value: "1", + Value: "0", }, } @@ -588,6 +636,7 @@ func (s *Store) GetEnv() []corev1.EnvVar { c = append(c, s.getOldSessionCache()...) c = append(c, s.getSessionCache()...) c = append(c, s.getAppCache()...) + c = append(c, s.getLock()...) c = append(c, s.getOtel()...) c = append(c, s.getBlackfire()...) c = append(c, s.getStorage()...) diff --git a/api/v1/store_test.go b/api/v1/store_test.go index 10361fc..0a1f352 100644 --- a/api/v1/store_test.go +++ b/api/v1/store_test.go @@ -82,9 +82,9 @@ func TestStoreContainer(t *testing.T) { assert.Equal(t, int32(60), baseContainer.Spec.Container.ProgressDeadlineSeconds) assert.Equal(t, corev1.RestartPolicyNever, baseContainer.Spec.Container.RestartPolicy) assert.Equal(t, []corev1.EnvVar{{Name: "ENV2", Value: "value2"}}, baseContainer.Spec.Container.ExtraEnvs) - assert.Equal(t, []corev1.VolumeMount{{Name: "vol2", MountPath: "/path2"}}, baseContainer.Spec.Container.VolumeMounts) + assert.Equal(t, []corev1.VolumeMount{{Name: "vol1", MountPath: "/path1"}, {Name: "vol2", MountPath: "/path2"}}, baseContainer.Spec.Container.VolumeMounts) assert.Equal(t, []corev1.LocalObjectReference{{Name: "secret2"}}, baseContainer.Spec.Container.ImagePullSecrets) - assert.Equal(t, []corev1.Volume{{Name: "vol2"}}, baseContainer.Spec.Container.Volumes) + assert.Equal(t, []corev1.Volume{{Name: "vol1"}, {Name: "vol2"}}, baseContainer.Spec.Container.Volumes) assert.Equal(t, resource.MustParse("2"), baseContainer.Spec.Container.Resources.Limits[corev1.ResourceCPU]) assert.Equal(t, resource.MustParse("2Gi"), baseContainer.Spec.Container.Resources.Requests[corev1.ResourceMemory]) assert.Equal(t, []corev1.Container{{Name: "sidecar2"}}, baseContainer.Spec.Container.ExtraContainers) diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index ce5fd39..2d83d64 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -444,6 +444,22 @@ func (in *Hook) DeepCopy() *Hook { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *LockSpec) DeepCopyInto(out *LockSpec) { + *out = *in + out.RedisSpec = in.RedisSpec +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LockSpec. +func (in *LockSpec) DeepCopy() *LockSpec { + if in == nil { + return nil + } + out := new(LockSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NetworkSpec) DeepCopyInto(out *NetworkSpec) { *out = *in @@ -1148,6 +1164,7 @@ func (in *StoreSpec) DeepCopyInto(out *StoreSpec) { out.ShopConfiguration = in.ShopConfiguration out.SessionCache = in.SessionCache out.AppCache = in.AppCache + out.Lock = in.Lock out.Worker = in.Worker out.AdminCredentials = in.AdminCredentials out.SetupHook = in.SetupHook diff --git a/internal/deployment/admin_test.go b/internal/deployment/admin_test.go index 0f75514..4bcf867 100644 --- a/internal/deployment/admin_test.go +++ b/internal/deployment/admin_test.go @@ -127,10 +127,11 @@ func TestAdminDeployment(t *testing.T) { assert.Equal(t, resource.MustParse("1"), container.Resources.Limits["cpu"]) assert.Equal(t, resource.MustParse("1Gi"), container.Resources.Limits["memory"]) - // Verify volume mounts are replaced - assert.Len(t, container.VolumeMounts, 1) - assert.Equal(t, "admin-volume", container.VolumeMounts[0].Name) - assert.Equal(t, "/admin", container.VolumeMounts[0].MountPath) + // Verify volume mounts are merged + assert.Len(t, container.VolumeMounts, 2) + assert.Equal(t, "container-volume", container.VolumeMounts[0].Name) + assert.Equal(t, "admin-volume", container.VolumeMounts[1].Name) + assert.Equal(t, "/admin", container.VolumeMounts[1].MountPath) // Verify env vars are merged hasAdminEnv := false diff --git a/internal/deployment/storefront.go b/internal/deployment/storefront.go index 6d92853..66aab9d 100644 --- a/internal/deployment/storefront.go +++ b/internal/deployment/storefront.go @@ -17,6 +17,7 @@ import ( ) const DEPLOYMENT_STOREFRONT_CONTAINER_NAME = "shopware-storefront" +const FPM_ADMIN_PORT int32 = 8001 func GetStorefrontDeployment( ctx context.Context, @@ -95,18 +96,33 @@ func StorefrontDeployment(store v1.Store) *appsv1.Deployment { containers := append(util.DefaultContainerSecurityContexts(containerSpec.ExtraContainers), corev1.Container{ Name: DEPLOYMENT_STOREFRONT_CONTAINER_NAME, + StartupProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/-/fpm/ping", + Port: intstr.IntOrString{ + Type: intstr.Int, + IntVal: FPM_ADMIN_PORT, + }, + }, + }, + PeriodSeconds: 5, + TimeoutSeconds: 5, + FailureThreshold: 18, + }, LivenessProbe: &corev1.Probe{ ProbeHandler: corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ - Path: "/api/_info/health-check", + Path: "/-/fpm/ping", Port: intstr.IntOrString{ Type: intstr.Int, - IntVal: containerSpec.Port, + IntVal: FPM_ADMIN_PORT, }, }, }, - TimeoutSeconds: 2, - InitialDelaySeconds: 5, + PeriodSeconds: 10, + TimeoutSeconds: 5, + FailureThreshold: 3, }, ReadinessProbe: &corev1.Probe{ ProbeHandler: corev1.ProbeHandler{ diff --git a/internal/deployment/storefront_test.go b/internal/deployment/storefront_test.go index 7b8310d..928605b 100644 --- a/internal/deployment/storefront_test.go +++ b/internal/deployment/storefront_test.go @@ -123,10 +123,11 @@ func TestStorefrontDeployment(t *testing.T) { assert.Equal(t, resource.MustParse("1"), container.Resources.Limits["cpu"]) assert.Equal(t, resource.MustParse("1Gi"), container.Resources.Limits["memory"]) - // Verify volume mounts are replaced - assert.Len(t, container.VolumeMounts, 1) - assert.Equal(t, "storefront-volume", container.VolumeMounts[0].Name) - assert.Equal(t, "/storefront", container.VolumeMounts[0].MountPath) + // Verify volume mounts are merged + assert.Len(t, container.VolumeMounts, 2) + assert.Equal(t, "container-volume", container.VolumeMounts[0].Name) + assert.Equal(t, "storefront-volume", container.VolumeMounts[1].Name) + assert.Equal(t, "/storefront", container.VolumeMounts[1].MountPath) // Verify env vars are merged hasStorefrontEnv := false @@ -241,11 +242,19 @@ func TestStorefrontDeployment(t *testing.T) { container := result.Spec.Template.Spec.Containers[0] // Verify probes are configured + assert.NotNil(t, container.StartupProbe) assert.NotNil(t, container.LivenessProbe) assert.NotNil(t, container.ReadinessProbe) - assert.Equal(t, "/api/_info/health-check", container.LivenessProbe.HTTPGet.Path) + assert.Equal(t, "/-/fpm/ping", container.StartupProbe.HTTPGet.Path) + assert.Equal(t, int32(8001), container.StartupProbe.HTTPGet.Port.IntVal) + assert.Equal(t, "/-/fpm/ping", container.LivenessProbe.HTTPGet.Path) + assert.Equal(t, int32(8001), container.LivenessProbe.HTTPGet.Port.IntVal) assert.Equal(t, "/api/_info/health-check", container.ReadinessProbe.HTTPGet.Path) - assert.Equal(t, int32(8000), container.LivenessProbe.HTTPGet.Port.IntVal) assert.Equal(t, int32(8000), container.ReadinessProbe.HTTPGet.Port.IntVal) + + assert.Equal(t, int32(5), container.StartupProbe.PeriodSeconds) + assert.Equal(t, int32(18), container.StartupProbe.FailureThreshold) + assert.Equal(t, int32(10), container.LivenessProbe.PeriodSeconds) + assert.Equal(t, int32(3), container.LivenessProbe.FailureThreshold) }) } diff --git a/internal/deployment/worker.go b/internal/deployment/worker.go index a522f47..9a1d5fe 100644 --- a/internal/deployment/worker.go +++ b/internal/deployment/worker.go @@ -78,8 +78,14 @@ func WorkerDeployment(store v1.Store) *appsv1.Deployment { annotations := util.GetDefaultContainerAnnotations(appName, store, store.Spec.WorkerDeploymentContainer.Annotations) - // Merge containerSpec.ExtraEnvs to override with merged values from WorkerDeploymentContainer - envs := util.MergeEnv(store.GetEnv(), containerSpec.ExtraEnvs) + // Worker-specific defaults layered over the shared env, still overridable by + // ExtraEnvs. The worker runs a few long-lived processes, so persistent DB + // connections are beneficial here (storefront/admin default to 0 in GetEnv to + // avoid hoarding connections across many short-lived requests). + workerDefaults := []corev1.EnvVar{ + {Name: "DATABASE_PERSISTENT_CONNECTION", Value: "1"}, + } + envs := util.MergeEnv(util.MergeEnv(store.GetEnv(), workerDefaults), containerSpec.ExtraEnvs) // Set PHP_MEMORY_LIMIT to 90% of the container memory limit phpMemoryLimitMiB := 0 diff --git a/internal/deployment/worker_test.go b/internal/deployment/worker_test.go index e28c131..8e33418 100644 --- a/internal/deployment/worker_test.go +++ b/internal/deployment/worker_test.go @@ -123,10 +123,11 @@ func TestWorkerDeployment(t *testing.T) { assert.Equal(t, resource.MustParse("1"), container.Resources.Limits["cpu"]) assert.Equal(t, resource.MustParse("1Gi"), container.Resources.Limits["memory"]) - // Verify volume mounts are replaced - assert.Len(t, container.VolumeMounts, 1) - assert.Equal(t, "worker-volume", container.VolumeMounts[0].Name) - assert.Equal(t, "/worker", container.VolumeMounts[0].MountPath) + // Verify volume mounts are merged + assert.Len(t, container.VolumeMounts, 2) + assert.Equal(t, "container-volume", container.VolumeMounts[0].Name) + assert.Equal(t, "worker-volume", container.VolumeMounts[1].Name) + assert.Equal(t, "/worker", container.VolumeMounts[1].MountPath) // Verify env vars are merged hasWorkerEnv := false diff --git a/internal/job/migration_test.go b/internal/job/migration_test.go index 14fd90f..bfe82bf 100644 --- a/internal/job/migration_test.go +++ b/internal/job/migration_test.go @@ -126,10 +126,11 @@ func TestMigrationJob(t *testing.T) { assert.Equal(t, resource.MustParse("1"), container.Resources.Limits["cpu"]) assert.Equal(t, resource.MustParse("1Gi"), container.Resources.Limits["memory"]) - // Verify volume mounts are replaced - assert.Len(t, container.VolumeMounts, 1) - assert.Equal(t, "migration-volume", container.VolumeMounts[0].Name) - assert.Equal(t, "/migration", container.VolumeMounts[0].MountPath) + // Verify volume mounts are merged + assert.Len(t, container.VolumeMounts, 2) + assert.Equal(t, "container-volume", container.VolumeMounts[0].Name) + assert.Equal(t, "migration-volume", container.VolumeMounts[1].Name) + assert.Equal(t, "/migration", container.VolumeMounts[1].MountPath) // Verify env vars are merged hasMigrationEnv := false diff --git a/internal/job/setup_test.go b/internal/job/setup_test.go index 2d9c6db..8fad498 100644 --- a/internal/job/setup_test.go +++ b/internal/job/setup_test.go @@ -184,10 +184,11 @@ func TestSetupJob(t *testing.T) { assert.Equal(t, resource.MustParse("1"), container.Resources.Limits["cpu"]) assert.Equal(t, resource.MustParse("1Gi"), container.Resources.Limits["memory"]) - // Verify volume mounts are replaced - assert.Len(t, container.VolumeMounts, 1) - assert.Equal(t, "setup-volume", container.VolumeMounts[0].Name) - assert.Equal(t, "/setup", container.VolumeMounts[0].MountPath) + // Verify volume mounts are merged + assert.Len(t, container.VolumeMounts, 2) + assert.Equal(t, "container-volume", container.VolumeMounts[0].Name) + assert.Equal(t, "setup-volume", container.VolumeMounts[1].Name) + assert.Equal(t, "/setup", container.VolumeMounts[1].MountPath) // Verify env vars are replaced hasSetupEnv := false From a40cefe47d771ccabde37b572ae704cdcf201f7f Mon Sep 17 00:00:00 2001 From: Kevin Rudde Date: Thu, 30 Jul 2026 16:16:22 +0200 Subject: [PATCH 2/2] fix: only merge volume / volumeMounts by name --- api/v1/store.go | 40 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/api/v1/store.go b/api/v1/store.go index eaaf014..648626a 100644 --- a/api/v1/store.go +++ b/api/v1/store.go @@ -478,6 +478,42 @@ func (s *Store) GetSecretName() string { return s.Spec.SecretName } +// mergeVolumeMountsByName appends from onto base, replacing any entry whose +// Name already exists so the merged Pod spec stays free of duplicate mounts. +func mergeVolumeMountsByName(base, from []corev1.VolumeMount) []corev1.VolumeMount { + idx := make(map[string]int, len(base)) + for i, m := range base { + idx[m.Name] = i + } + for _, m := range from { + if i, ok := idx[m.Name]; ok { + base[i] = m + continue + } + idx[m.Name] = len(base) + base = append(base, m) + } + return base +} + +// mergeVolumesByName appends from onto base, replacing any entry whose Name +// already exists so the merged Pod spec stays free of duplicate volumes. +func mergeVolumesByName(base, from []corev1.Volume) []corev1.Volume { + idx := make(map[string]int, len(base)) + for i, v := range base { + idx[v.Name] = i + } + for _, v := range from { + if i, ok := idx[v.Name]; ok { + base[i] = v + continue + } + idx[v.Name] = len(base) + base = append(base, v) + } + return base +} + //nolint:gocyclo func (c *ContainerSpec) Merge(from ContainerMergeSpec) { if from.Image != "" { @@ -499,13 +535,13 @@ func (c *ContainerSpec) Merge(from ContainerMergeSpec) { c.ExtraEnvs = from.ExtraEnvs } if from.VolumeMounts != nil { - c.VolumeMounts = append(c.VolumeMounts, from.VolumeMounts...) + c.VolumeMounts = mergeVolumeMountsByName(c.VolumeMounts, from.VolumeMounts) } if from.ImagePullSecrets != nil { c.ImagePullSecrets = from.ImagePullSecrets } if from.Volumes != nil { - c.Volumes = append(c.Volumes, from.Volumes...) + c.Volumes = mergeVolumesByName(c.Volumes, from.Volumes) } if from.ServiceAccountName != "" {