From 76be4fa4e58f9c799b51575045c5ef4f45b01dc0 Mon Sep 17 00:00:00 2001 From: Chris Bandy Date: Fri, 5 Sep 2025 11:34:37 -0500 Subject: [PATCH 1/4] Move AddAdditionalVolumesToSpecifiedContainers to a shared package --- .../controller/postgrescluster/instance.go | 3 +- .../controller/postgrescluster/pgbackrest.go | 6 +- .../controller/postgrescluster/pgbouncer.go | 5 +- internal/controller/postgrescluster/util.go | 80 ------ .../controller/postgrescluster/util_test.go | 269 ------------------ .../standalone_pgadmin/statefulset.go | 3 +- internal/util/volumes.go | 51 ++++ internal/util/volumes_test.go | 225 +++++++++++++++ .../v1beta1/shared_types.go | 16 ++ .../v1beta1/shared_types_test.go | 29 ++ 10 files changed, 331 insertions(+), 356 deletions(-) diff --git a/internal/controller/postgrescluster/instance.go b/internal/controller/postgrescluster/instance.go index 1f5d171e81..8455cc5a48 100644 --- a/internal/controller/postgrescluster/instance.go +++ b/internal/controller/postgrescluster/instance.go @@ -36,6 +36,7 @@ import ( "github.com/crunchydata/postgres-operator/internal/pki" "github.com/crunchydata/postgres-operator/internal/postgres" "github.com/crunchydata/postgres-operator/internal/tracing" + "github.com/crunchydata/postgres-operator/internal/util" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) @@ -1213,7 +1214,7 @@ func (r *Reconciler) reconcileInstance( // mount additional volumes to the Postgres instance containers if err == nil && spec.Volumes != nil && len(spec.Volumes.Additional) > 0 { - missingContainers := AddAdditionalVolumesToSpecifiedContainers(&instance.Spec.Template, spec.Volumes.Additional) + missingContainers := util.AddAdditionalVolumesAndMounts(&instance.Spec.Template, spec.Volumes.Additional) if len(missingContainers) > 0 { r.Recorder.Eventf(cluster, corev1.EventTypeWarning, "SpecifiedContainerNotFound", diff --git a/internal/controller/postgrescluster/pgbackrest.go b/internal/controller/postgrescluster/pgbackrest.go index d1226c4af3..7bc9dc2f5d 100644 --- a/internal/controller/postgrescluster/pgbackrest.go +++ b/internal/controller/postgrescluster/pgbackrest.go @@ -721,7 +721,7 @@ func (r *Reconciler) generateRepoHostIntent(ctx context.Context, postgresCluster // mount additional volumes to the repo host containers if repoHost != nil && repoHost.Volumes != nil && len(repoHost.Volumes.Additional) > 0 { - missingContainers := AddAdditionalVolumesToSpecifiedContainers(&repo.Spec.Template, repoHost.Volumes.Additional) + missingContainers := util.AddAdditionalVolumesAndMounts(&repo.Spec.Template, repoHost.Volumes.Additional) if len(missingContainers) > 0 { r.Recorder.Eventf(postgresCluster, corev1.EventTypeWarning, "SpecifiedContainerNotFound", @@ -908,7 +908,7 @@ func (r *Reconciler) generateBackupJobSpecIntent(ctx context.Context, postgresCl // mount additional volumes to the job containers if jobs != nil && jobs.Volumes != nil && len(jobs.Volumes.Additional) > 0 { - missingContainers := AddAdditionalVolumesToSpecifiedContainers(&jobSpec.Template, jobs.Volumes.Additional) + missingContainers := util.AddAdditionalVolumesAndMounts(&jobSpec.Template, jobs.Volumes.Additional) if len(missingContainers) > 0 { r.Recorder.Eventf(postgresCluster, corev1.EventTypeWarning, "SpecifiedContainerNotFound", @@ -1408,7 +1408,7 @@ func (r *Reconciler) generateRestoreJobIntent(cluster *v1beta1.PostgresCluster, job.Spec.Template.Spec.PriorityClassName = initialize.FromPointer(dataSource.PriorityClassName) if dataSource.Volumes != nil && len(dataSource.Volumes.Additional) > 0 { - missingContainers := AddAdditionalVolumesToSpecifiedContainers(&job.Spec.Template, dataSource.Volumes.Additional) + missingContainers := util.AddAdditionalVolumesAndMounts(&job.Spec.Template, dataSource.Volumes.Additional) if len(missingContainers) > 0 { r.Recorder.Eventf(cluster, corev1.EventTypeWarning, "SpecifiedContainerNotFound", diff --git a/internal/controller/postgrescluster/pgbouncer.go b/internal/controller/postgrescluster/pgbouncer.go index 46b66cdbf5..a952ebbb2a 100644 --- a/internal/controller/postgrescluster/pgbouncer.go +++ b/internal/controller/postgrescluster/pgbouncer.go @@ -25,6 +25,7 @@ import ( "github.com/crunchydata/postgres-operator/internal/pgbouncer" "github.com/crunchydata/postgres-operator/internal/pki" "github.com/crunchydata/postgres-operator/internal/postgres" + "github.com/crunchydata/postgres-operator/internal/util" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) @@ -476,8 +477,8 @@ func (r *Reconciler) generatePGBouncerDeployment( AddTMPEmptyDir(&deploy.Spec.Template) // mount additional volumes to the pgbouncer containers - if err == nil && cluster.Spec.Proxy.PGBouncer.Volumes != nil && len(cluster.Spec.Proxy.PGBouncer.Volumes.Additional) > 0 { - missingContainers := AddAdditionalVolumesToSpecifiedContainers(&deploy.Spec.Template, cluster.Spec.Proxy.PGBouncer.Volumes.Additional) + if volumes := cluster.Spec.Proxy.PGBouncer.Volumes; err == nil && volumes != nil && len(volumes.Additional) > 0 { + missingContainers := util.AddAdditionalVolumesAndMounts(&deploy.Spec.Template, volumes.Additional) if len(missingContainers) > 0 { r.Recorder.Eventf(cluster, corev1.EventTypeWarning, "SpecifiedContainerNotFound", diff --git a/internal/controller/postgrescluster/util.go b/internal/controller/postgrescluster/util.go index fa021f4626..a1ba6ce087 100644 --- a/internal/controller/postgrescluster/util.go +++ b/internal/controller/postgrescluster/util.go @@ -13,11 +13,9 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/util/rand" - "k8s.io/apimachinery/pkg/util/sets" "github.com/crunchydata/postgres-operator/internal/initialize" "github.com/crunchydata/postgres-operator/internal/naming" - "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) var tmpDirSizeLimit = resource.MustParse("16Mi") @@ -287,81 +285,3 @@ func safeHash32(content func(w io.Writer) error) (string, error) { } return rand.SafeEncodeString(fmt.Sprint(hash.Sum32())), nil } - -// AdditionalVolumeMount returns the name and mount path of the additional volume. -func AdditionalVolumeMount(name string, readOnly bool) corev1.VolumeMount { - return corev1.VolumeMount{ - Name: fmt.Sprintf("volumes-%s", name), - MountPath: "/volumes/" + name, - ReadOnly: readOnly, - } -} - -// AddAdditionalVolumesToSpecifiedContainers adds additional volumes to the specified -// containers in the specified pod -// AddAdditionalVolumesToSpecifiedContainers adds the volumes to the pod -// as `volumes-` -// and adds the directory to the path `/volumes/` -func AddAdditionalVolumesToSpecifiedContainers(template *corev1.PodTemplateSpec, - additionalVolumes []v1beta1.AdditionalVolume) []string { - - missingContainers := []string{} - for _, additionalVolumeRequest := range additionalVolumes { - - additionalVolumeMount := AdditionalVolumeMount( - additionalVolumeRequest.Name, - additionalVolumeRequest.ReadOnly, - ) - - additionalVolume := corev1.Volume{ - Name: additionalVolumeMount.Name, - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: additionalVolumeRequest.ClaimName, - ReadOnly: additionalVolumeMount.ReadOnly, - }, - }, - } - - // Create a set of all the requested containers, - // then in the loops below when we attach the volume to a container, - // we can safely remove that container name from the set. - // This gives us a way to track the containers that are requested but not found. - // This relies on `containers` and `initContainers` together being unique. - // - https://github.com/kubernetes/api/blob/b40c1cacbb902b21a7e0c7bf0967321860c1a632/core/v1/types.go#L3895C27-L3896C33 - names := sets.New(additionalVolumeRequest.Containers...) - allContainers := false - // If the containers list is omitted, we add the volume to all containers - if additionalVolumeRequest.Containers == nil { - allContainers = true - } - - for i := range template.Spec.Containers { - if allContainers || names.Has(template.Spec.Containers[i].Name) { - template.Spec.Containers[i].VolumeMounts = append( - template.Spec.Containers[i].VolumeMounts, - additionalVolumeMount) - - names.Delete(template.Spec.Containers[i].Name) - } - } - - for i := range template.Spec.InitContainers { - if allContainers || names.Has(template.Spec.InitContainers[i].Name) { - template.Spec.InitContainers[i].VolumeMounts = append( - template.Spec.InitContainers[i].VolumeMounts, - additionalVolumeMount) - - names.Delete(template.Spec.InitContainers[i].Name) - - } - } - - missingContainers = append(missingContainers, names.UnsortedList()...) - - template.Spec.Volumes = append( - template.Spec.Volumes, - additionalVolume) - } - return missingContainers -} diff --git a/internal/controller/postgrescluster/util_test.go b/internal/controller/postgrescluster/util_test.go index 2912cd489c..8e7d5c434f 100644 --- a/internal/controller/postgrescluster/util_test.go +++ b/internal/controller/postgrescluster/util_test.go @@ -16,7 +16,6 @@ import ( "github.com/crunchydata/postgres-operator/internal/naming" "github.com/crunchydata/postgres-operator/internal/testing/cmp" - "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) func TestSafeHash32(t *testing.T) { @@ -379,271 +378,3 @@ func TestJobFailed(t *testing.T) { }) } } - -func TestAddAdditionalVolumesToSpecifiedContainers(t *testing.T) { - - podTemplate := &corev1.PodTemplateSpec{ - Spec: corev1.PodSpec{ - InitContainers: []corev1.Container{ - {Name: "startup"}, - {Name: "config"}, - }, - Containers: []corev1.Container{ - {Name: "database"}, - {Name: "other"}, - }}} - - testCases := []struct { - tcName string - additionalVolumes []v1beta1.AdditionalVolume - expectedContainers string - expectedInitContainers string - expectedVolumes string - expectedMissing []string - }{{ - tcName: "all", - additionalVolumes: []v1beta1.AdditionalVolume{{ - ClaimName: "required", - Name: "required", - }}, - expectedContainers: `- name: database - resources: {} - volumeMounts: - - mountPath: /volumes/required - name: volumes-required -- name: other - resources: {} - volumeMounts: - - mountPath: /volumes/required - name: volumes-required`, - expectedInitContainers: `- name: startup - resources: {} - volumeMounts: - - mountPath: /volumes/required - name: volumes-required -- name: config - resources: {} - volumeMounts: - - mountPath: /volumes/required - name: volumes-required`, - expectedVolumes: `- name: volumes-required - persistentVolumeClaim: - claimName: required`, - expectedMissing: []string{}, - }, { - tcName: "multiple additional volumes", - additionalVolumes: []v1beta1.AdditionalVolume{{ - ClaimName: "required", - Name: "required", - }, { - ClaimName: "also", - Name: "other", - }}, - expectedContainers: `- name: database - resources: {} - volumeMounts: - - mountPath: /volumes/required - name: volumes-required - - mountPath: /volumes/other - name: volumes-other -- name: other - resources: {} - volumeMounts: - - mountPath: /volumes/required - name: volumes-required - - mountPath: /volumes/other - name: volumes-other`, - expectedInitContainers: `- name: startup - resources: {} - volumeMounts: - - mountPath: /volumes/required - name: volumes-required - - mountPath: /volumes/other - name: volumes-other -- name: config - resources: {} - volumeMounts: - - mountPath: /volumes/required - name: volumes-required - - mountPath: /volumes/other - name: volumes-other`, - expectedVolumes: `- name: volumes-required - persistentVolumeClaim: - claimName: required -- name: volumes-other - persistentVolumeClaim: - claimName: also`, - expectedMissing: []string{}, - }, { - tcName: "none", - additionalVolumes: []v1beta1.AdditionalVolume{{ - Containers: []string{}, - ClaimName: "required", - Name: "required", - }}, - expectedContainers: `- name: database - resources: {} -- name: other - resources: {}`, - expectedInitContainers: `- name: startup - resources: {} -- name: config - resources: {}`, - expectedVolumes: `- name: volumes-required - persistentVolumeClaim: - claimName: required`, - expectedMissing: []string{}, - }, { - tcName: "multiple additional volumes", - additionalVolumes: []v1beta1.AdditionalVolume{{ - ClaimName: "required", - Name: "required", - }, { - ClaimName: "also", - Name: "other", - }}, - expectedContainers: `- name: database - resources: {} - volumeMounts: - - mountPath: /volumes/required - name: volumes-required - - mountPath: /volumes/other - name: volumes-other -- name: other - resources: {} - volumeMounts: - - mountPath: /volumes/required - name: volumes-required - - mountPath: /volumes/other - name: volumes-other`, - expectedInitContainers: `- name: startup - resources: {} - volumeMounts: - - mountPath: /volumes/required - name: volumes-required - - mountPath: /volumes/other - name: volumes-other -- name: config - resources: {} - volumeMounts: - - mountPath: /volumes/required - name: volumes-required - - mountPath: /volumes/other - name: volumes-other`, - expectedVolumes: `- name: volumes-required - persistentVolumeClaim: - claimName: required -- name: volumes-other - persistentVolumeClaim: - claimName: also`, - expectedMissing: []string{}, - }, { - tcName: "database and startup containers only", - additionalVolumes: []v1beta1.AdditionalVolume{{ - Containers: []string{"database", "startup"}, - ClaimName: "required", - Name: "required", - }}, - expectedContainers: `- name: database - resources: {} - volumeMounts: - - mountPath: /volumes/required - name: volumes-required -- name: other - resources: {}`, - expectedInitContainers: `- name: startup - resources: {} - volumeMounts: - - mountPath: /volumes/required - name: volumes-required -- name: config - resources: {}`, - expectedVolumes: `- name: volumes-required - persistentVolumeClaim: - claimName: required`, - expectedMissing: []string{}, - }, { - tcName: "container is missing", - additionalVolumes: []v1beta1.AdditionalVolume{{ - Containers: []string{"database", "startup", "missing", "container"}, - ClaimName: "required", - Name: "required", - }}, - expectedContainers: `- name: database - resources: {} - volumeMounts: - - mountPath: /volumes/required - name: volumes-required -- name: other - resources: {}`, - expectedInitContainers: `- name: startup - resources: {} - volumeMounts: - - mountPath: /volumes/required - name: volumes-required -- name: config - resources: {}`, - expectedVolumes: `- name: volumes-required - persistentVolumeClaim: - claimName: required`, - expectedMissing: []string{"missing", "container"}, - }, { - tcName: "readonly", - additionalVolumes: []v1beta1.AdditionalVolume{{ - Containers: []string{"database"}, - ClaimName: "required", - Name: "required", - ReadOnly: true, - }}, - expectedContainers: `- name: database - resources: {} - volumeMounts: - - mountPath: /volumes/required - name: volumes-required - readOnly: true -- name: other - resources: {}`, - expectedInitContainers: `- name: startup - resources: {} -- name: config - resources: {}`, - expectedVolumes: `- name: volumes-required - persistentVolumeClaim: - claimName: required - readOnly: true`, - expectedMissing: []string{}, - }} - - for _, tc := range testCases { - t.Run(tc.tcName, func(t *testing.T) { - - copyPodTemplate := podTemplate.DeepCopy() - - missingContainers := AddAdditionalVolumesToSpecifiedContainers( - copyPodTemplate, - tc.additionalVolumes, - ) - - assert.Assert(t, cmp.MarshalMatches( - copyPodTemplate.Spec.Containers, - tc.expectedContainers)) - assert.Assert(t, cmp.MarshalMatches( - copyPodTemplate.Spec.InitContainers, - tc.expectedInitContainers)) - assert.Assert(t, cmp.MarshalMatches( - copyPodTemplate.Spec.Volumes, - tc.expectedVolumes)) - if len(tc.expectedMissing) == 0 { - assert.Assert(t, cmp.DeepEqual( - missingContainers, - tc.expectedMissing)) - } else { - for _, mc := range tc.expectedMissing { - assert.Assert(t, cmp.Contains( - missingContainers, - mc)) - } - } - }) - } -} diff --git a/internal/controller/standalone_pgadmin/statefulset.go b/internal/controller/standalone_pgadmin/statefulset.go index 7fddc3a7de..232afa45a0 100644 --- a/internal/controller/standalone_pgadmin/statefulset.go +++ b/internal/controller/standalone_pgadmin/statefulset.go @@ -18,6 +18,7 @@ import ( "github.com/crunchydata/postgres-operator/internal/controller/postgrescluster" "github.com/crunchydata/postgres-operator/internal/initialize" "github.com/crunchydata/postgres-operator/internal/naming" + "github.com/crunchydata/postgres-operator/internal/util" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) @@ -140,7 +141,7 @@ func (r *PGAdminReconciler) statefulset( // mount additional volumes to the Postgres instance containers if pgadmin.Spec.Volumes != nil && len(pgadmin.Spec.Volumes.Additional) > 0 { - missingContainers := postgrescluster.AddAdditionalVolumesToSpecifiedContainers(&sts.Spec.Template, pgadmin.Spec.Volumes.Additional) + missingContainers := util.AddAdditionalVolumesAndMounts(&sts.Spec.Template, pgadmin.Spec.Volumes.Additional) if len(missingContainers) > 0 { r.Recorder.Eventf(pgadmin, corev1.EventTypeWarning, "SpecifiedContainerNotFound", diff --git a/internal/util/volumes.go b/internal/util/volumes.go index 34e2699b54..1bebe6b2c1 100644 --- a/internal/util/volumes.go +++ b/internal/util/volumes.go @@ -8,8 +8,59 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/sets" + + "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) +// AdditionalVolumeMount creates a [corev1.VolumeMount] at `/volumes/{name}` of volume `volumes-{name}`. +func AdditionalVolumeMount(name string, readOnly bool) corev1.VolumeMount { + return corev1.VolumeMount{ + Name: fmt.Sprintf("volumes-%s", name), + MountPath: "/volumes/" + name, + ReadOnly: readOnly, + } +} + +// AddAdditionalVolumesAndMounts adds volumes as [corev1.Volume]s and [corev1.VolumeMount]s in template. +// Volume names are chosen in [AdditionalVolumeMount]. +func AddAdditionalVolumesAndMounts(template *corev1.PodTemplateSpec, volumes []v1beta1.AdditionalVolume) []string { + missingContainers := []string{} + + for _, spec := range volumes { + mount := AdditionalVolumeMount(spec.Name, spec.ReadOnly) + template.Spec.Volumes = append(template.Spec.Volumes, spec.AsVolume(mount.Name)) + + // Create a set of all the requested containers, + // then in the loops below when we attach the volume to a container, + // we can safely remove that container name from the set. + // This gives us a way to track the containers that are requested but not found. + // This relies on `containers` and `initContainers` together being unique. + // - https://github.com/kubernetes/api/blob/b40c1cacbb902b21a7e0c7bf0967321860c1a632/core/v1/types.go#L3895C27-L3896C33 + names := sets.New(spec.Containers...) + + for i, c := range template.Spec.Containers { + if spec.Containers == nil || names.Has(c.Name) { + c.VolumeMounts = append(c.VolumeMounts, mount) + template.Spec.Containers[i] = c + } + names.Delete(c.Name) + } + + for i, c := range template.Spec.InitContainers { + if spec.Containers == nil || names.Has(c.Name) { + c.VolumeMounts = append(c.VolumeMounts, mount) + template.Spec.InitContainers[i] = c + } + names.Delete(c.Name) + } + + missingContainers = append(missingContainers, names.UnsortedList()...) + } + + return missingContainers +} + // AddVolumeAndMountsToPod takes a Pod spec and a PVC and adds a Volume to the Pod spec with // the PVC as the VolumeSource and mounts the volume to all containers and init containers // in the Pod spec. diff --git a/internal/util/volumes_test.go b/internal/util/volumes_test.go index b438943e3a..0ff005943f 100644 --- a/internal/util/volumes_test.go +++ b/internal/util/volumes_test.go @@ -13,8 +13,233 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/crunchydata/postgres-operator/internal/testing/cmp" + "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) +func TestAddAdditionalVolumesAndMounts(t *testing.T) { + t.Parallel() + + podTemplate := &corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + {Name: "startup"}, + {Name: "config"}, + }, + Containers: []corev1.Container{ + {Name: "database"}, + {Name: "other"}, + }}} + + testCases := []struct { + tcName string + additionalVolumes []v1beta1.AdditionalVolume + expectedContainers string + expectedInitContainers string + expectedVolumes string + expectedMissing []string + }{{ + tcName: "all containers", + additionalVolumes: []v1beta1.AdditionalVolume{{ + ClaimName: "required", + Name: "required", + }}, + expectedContainers: `- name: database + resources: {} + volumeMounts: + - mountPath: /volumes/required + name: volumes-required +- name: other + resources: {} + volumeMounts: + - mountPath: /volumes/required + name: volumes-required`, + expectedInitContainers: `- name: startup + resources: {} + volumeMounts: + - mountPath: /volumes/required + name: volumes-required +- name: config + resources: {} + volumeMounts: + - mountPath: /volumes/required + name: volumes-required`, + expectedVolumes: `- name: volumes-required + persistentVolumeClaim: + claimName: required`, + expectedMissing: []string{}, + }, { + tcName: "no containers", + additionalVolumes: []v1beta1.AdditionalVolume{{ + Containers: []string{}, + ClaimName: "required", + Name: "required", + }}, + expectedContainers: `- name: database + resources: {} +- name: other + resources: {}`, + expectedInitContainers: `- name: startup + resources: {} +- name: config + resources: {}`, + expectedVolumes: `- name: volumes-required + persistentVolumeClaim: + claimName: required`, + expectedMissing: []string{}, + }, { + tcName: "multiple volumes", + additionalVolumes: []v1beta1.AdditionalVolume{{ + ClaimName: "required", + Name: "required", + }, { + ClaimName: "also", + Name: "other", + }}, + expectedContainers: `- name: database + resources: {} + volumeMounts: + - mountPath: /volumes/required + name: volumes-required + - mountPath: /volumes/other + name: volumes-other +- name: other + resources: {} + volumeMounts: + - mountPath: /volumes/required + name: volumes-required + - mountPath: /volumes/other + name: volumes-other`, + expectedInitContainers: `- name: startup + resources: {} + volumeMounts: + - mountPath: /volumes/required + name: volumes-required + - mountPath: /volumes/other + name: volumes-other +- name: config + resources: {} + volumeMounts: + - mountPath: /volumes/required + name: volumes-required + - mountPath: /volumes/other + name: volumes-other`, + expectedVolumes: `- name: volumes-required + persistentVolumeClaim: + claimName: required +- name: volumes-other + persistentVolumeClaim: + claimName: also`, + expectedMissing: []string{}, + }, { + tcName: "database and startup containers only", + additionalVolumes: []v1beta1.AdditionalVolume{{ + Containers: []string{"database", "startup"}, + ClaimName: "required", + Name: "required", + }}, + expectedContainers: `- name: database + resources: {} + volumeMounts: + - mountPath: /volumes/required + name: volumes-required +- name: other + resources: {}`, + expectedInitContainers: `- name: startup + resources: {} + volumeMounts: + - mountPath: /volumes/required + name: volumes-required +- name: config + resources: {}`, + expectedVolumes: `- name: volumes-required + persistentVolumeClaim: + claimName: required`, + expectedMissing: []string{}, + }, { + tcName: "container is missing", + additionalVolumes: []v1beta1.AdditionalVolume{{ + Containers: []string{"database", "startup", "missing", "container"}, + ClaimName: "required", + Name: "required", + }}, + expectedContainers: `- name: database + resources: {} + volumeMounts: + - mountPath: /volumes/required + name: volumes-required +- name: other + resources: {}`, + expectedInitContainers: `- name: startup + resources: {} + volumeMounts: + - mountPath: /volumes/required + name: volumes-required +- name: config + resources: {}`, + expectedVolumes: `- name: volumes-required + persistentVolumeClaim: + claimName: required`, + expectedMissing: []string{"missing", "container"}, + }, { + tcName: "readonly", + additionalVolumes: []v1beta1.AdditionalVolume{{ + Containers: []string{"database"}, + ClaimName: "required", + Name: "required", + ReadOnly: true, + }}, + expectedContainers: `- name: database + resources: {} + volumeMounts: + - mountPath: /volumes/required + name: volumes-required + readOnly: true +- name: other + resources: {}`, + expectedInitContainers: `- name: startup + resources: {} +- name: config + resources: {}`, + expectedVolumes: `- name: volumes-required + persistentVolumeClaim: + claimName: required + readOnly: true`, + expectedMissing: []string{}, + }} + + for _, tc := range testCases { + t.Run(tc.tcName, func(t *testing.T) { + copyPodTemplate := podTemplate.DeepCopy() + + missingContainers := AddAdditionalVolumesAndMounts( + copyPodTemplate, + tc.additionalVolumes, + ) + + assert.Assert(t, cmp.MarshalMatches( + copyPodTemplate.Spec.Containers, + tc.expectedContainers)) + assert.Assert(t, cmp.MarshalMatches( + copyPodTemplate.Spec.InitContainers, + tc.expectedInitContainers)) + assert.Assert(t, cmp.MarshalMatches( + copyPodTemplate.Spec.Volumes, + tc.expectedVolumes)) + if len(tc.expectedMissing) == 0 { + assert.Assert(t, cmp.DeepEqual( + missingContainers, + tc.expectedMissing)) + } else { + for _, mc := range tc.expectedMissing { + assert.Assert(t, cmp.Contains( + missingContainers, + mc)) + } + } + }) + } +} + func TestAddVolumeAndMountsToPod(t *testing.T) { pod := &corev1.PodSpec{ Containers: []corev1.Container{ diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types.go index d4ecaba821..4325dae255 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types.go @@ -304,3 +304,19 @@ type AdditionalVolume struct { // +optional ReadOnly bool `json:"readOnly,omitempty"` } + +// AsVolume returns a copy of this as a [corev1.Volume]. +func (in *AdditionalVolume) AsVolume(name string) corev1.Volume { + var out corev1.Volume + out.Name = name + + switch { + case len(in.ClaimName) > 0: + out.PersistentVolumeClaim = &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: in.ClaimName, + ReadOnly: in.ReadOnly, + } + } + + return out +} diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types_test.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types_test.go index c4c2fe65f9..89f0c3f42a 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types_test.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types_test.go @@ -16,6 +16,35 @@ import ( "sigs.k8s.io/yaml" ) +func TestAdditionalVolumeAsVolume(t *testing.T) { + t.Parallel() + + t.Run("ClaimName", func(t *testing.T) { + in := AdditionalVolume{ClaimName: "doot"} + out := in.AsVolume("asdf") + + var expected corev1.Volume + assert.NilError(t, yaml.Unmarshal([]byte(`{ + name: asdf, + persistentVolumeClaim: { + claimName: doot, + }, + }`), &expected)) + + assert.DeepEqual(t, out, expected) + + t.Run("ReadOnly", func(t *testing.T) { + in.ReadOnly = true + out = in.AsVolume("qwerty") + + expected.Name = "qwerty" + expected.PersistentVolumeClaim.ReadOnly = true + + assert.DeepEqual(t, out, expected) + }) + }) +} + func TestDurationAsDuration(t *testing.T) { t.Parallel() From e10746b09d893b0f56a893e42466ec0c6d6767a5 Mon Sep 17 00:00:00 2001 From: Chris Bandy Date: Fri, 5 Sep 2025 11:53:04 -0500 Subject: [PATCH 2/4] Change util.AddAdditionalVolumesAndMounts to take a PodSpec This was the only field of the template it used. --- .../controller/postgrescluster/instance.go | 2 +- .../controller/postgrescluster/pgbackrest.go | 6 +-- .../controller/postgrescluster/pgbouncer.go | 2 +- .../standalone_pgadmin/statefulset.go | 2 +- internal/util/volumes.go | 14 ++--- internal/util/volumes_test.go | 54 +++++++------------ 6 files changed, 32 insertions(+), 48 deletions(-) diff --git a/internal/controller/postgrescluster/instance.go b/internal/controller/postgrescluster/instance.go index 8455cc5a48..924005b896 100644 --- a/internal/controller/postgrescluster/instance.go +++ b/internal/controller/postgrescluster/instance.go @@ -1214,7 +1214,7 @@ func (r *Reconciler) reconcileInstance( // mount additional volumes to the Postgres instance containers if err == nil && spec.Volumes != nil && len(spec.Volumes.Additional) > 0 { - missingContainers := util.AddAdditionalVolumesAndMounts(&instance.Spec.Template, spec.Volumes.Additional) + missingContainers := util.AddAdditionalVolumesAndMounts(&instance.Spec.Template.Spec, spec.Volumes.Additional) if len(missingContainers) > 0 { r.Recorder.Eventf(cluster, corev1.EventTypeWarning, "SpecifiedContainerNotFound", diff --git a/internal/controller/postgrescluster/pgbackrest.go b/internal/controller/postgrescluster/pgbackrest.go index 7bc9dc2f5d..ba9fbc57e4 100644 --- a/internal/controller/postgrescluster/pgbackrest.go +++ b/internal/controller/postgrescluster/pgbackrest.go @@ -721,7 +721,7 @@ func (r *Reconciler) generateRepoHostIntent(ctx context.Context, postgresCluster // mount additional volumes to the repo host containers if repoHost != nil && repoHost.Volumes != nil && len(repoHost.Volumes.Additional) > 0 { - missingContainers := util.AddAdditionalVolumesAndMounts(&repo.Spec.Template, repoHost.Volumes.Additional) + missingContainers := util.AddAdditionalVolumesAndMounts(&repo.Spec.Template.Spec, repoHost.Volumes.Additional) if len(missingContainers) > 0 { r.Recorder.Eventf(postgresCluster, corev1.EventTypeWarning, "SpecifiedContainerNotFound", @@ -908,7 +908,7 @@ func (r *Reconciler) generateBackupJobSpecIntent(ctx context.Context, postgresCl // mount additional volumes to the job containers if jobs != nil && jobs.Volumes != nil && len(jobs.Volumes.Additional) > 0 { - missingContainers := util.AddAdditionalVolumesAndMounts(&jobSpec.Template, jobs.Volumes.Additional) + missingContainers := util.AddAdditionalVolumesAndMounts(&jobSpec.Template.Spec, jobs.Volumes.Additional) if len(missingContainers) > 0 { r.Recorder.Eventf(postgresCluster, corev1.EventTypeWarning, "SpecifiedContainerNotFound", @@ -1408,7 +1408,7 @@ func (r *Reconciler) generateRestoreJobIntent(cluster *v1beta1.PostgresCluster, job.Spec.Template.Spec.PriorityClassName = initialize.FromPointer(dataSource.PriorityClassName) if dataSource.Volumes != nil && len(dataSource.Volumes.Additional) > 0 { - missingContainers := util.AddAdditionalVolumesAndMounts(&job.Spec.Template, dataSource.Volumes.Additional) + missingContainers := util.AddAdditionalVolumesAndMounts(&job.Spec.Template.Spec, dataSource.Volumes.Additional) if len(missingContainers) > 0 { r.Recorder.Eventf(cluster, corev1.EventTypeWarning, "SpecifiedContainerNotFound", diff --git a/internal/controller/postgrescluster/pgbouncer.go b/internal/controller/postgrescluster/pgbouncer.go index a952ebbb2a..c140071f12 100644 --- a/internal/controller/postgrescluster/pgbouncer.go +++ b/internal/controller/postgrescluster/pgbouncer.go @@ -478,7 +478,7 @@ func (r *Reconciler) generatePGBouncerDeployment( // mount additional volumes to the pgbouncer containers if volumes := cluster.Spec.Proxy.PGBouncer.Volumes; err == nil && volumes != nil && len(volumes.Additional) > 0 { - missingContainers := util.AddAdditionalVolumesAndMounts(&deploy.Spec.Template, volumes.Additional) + missingContainers := util.AddAdditionalVolumesAndMounts(&deploy.Spec.Template.Spec, volumes.Additional) if len(missingContainers) > 0 { r.Recorder.Eventf(cluster, corev1.EventTypeWarning, "SpecifiedContainerNotFound", diff --git a/internal/controller/standalone_pgadmin/statefulset.go b/internal/controller/standalone_pgadmin/statefulset.go index 232afa45a0..8e507acdad 100644 --- a/internal/controller/standalone_pgadmin/statefulset.go +++ b/internal/controller/standalone_pgadmin/statefulset.go @@ -141,7 +141,7 @@ func (r *PGAdminReconciler) statefulset( // mount additional volumes to the Postgres instance containers if pgadmin.Spec.Volumes != nil && len(pgadmin.Spec.Volumes.Additional) > 0 { - missingContainers := util.AddAdditionalVolumesAndMounts(&sts.Spec.Template, pgadmin.Spec.Volumes.Additional) + missingContainers := util.AddAdditionalVolumesAndMounts(&sts.Spec.Template.Spec, pgadmin.Spec.Volumes.Additional) if len(missingContainers) > 0 { r.Recorder.Eventf(pgadmin, corev1.EventTypeWarning, "SpecifiedContainerNotFound", diff --git a/internal/util/volumes.go b/internal/util/volumes.go index 1bebe6b2c1..1565f71c55 100644 --- a/internal/util/volumes.go +++ b/internal/util/volumes.go @@ -22,14 +22,14 @@ func AdditionalVolumeMount(name string, readOnly bool) corev1.VolumeMount { } } -// AddAdditionalVolumesAndMounts adds volumes as [corev1.Volume]s and [corev1.VolumeMount]s in template. +// AddAdditionalVolumesAndMounts adds volumes as [corev1.Volume]s and [corev1.VolumeMount]s in pod. // Volume names are chosen in [AdditionalVolumeMount]. -func AddAdditionalVolumesAndMounts(template *corev1.PodTemplateSpec, volumes []v1beta1.AdditionalVolume) []string { +func AddAdditionalVolumesAndMounts(pod *corev1.PodSpec, volumes []v1beta1.AdditionalVolume) []string { missingContainers := []string{} for _, spec := range volumes { mount := AdditionalVolumeMount(spec.Name, spec.ReadOnly) - template.Spec.Volumes = append(template.Spec.Volumes, spec.AsVolume(mount.Name)) + pod.Volumes = append(pod.Volumes, spec.AsVolume(mount.Name)) // Create a set of all the requested containers, // then in the loops below when we attach the volume to a container, @@ -39,18 +39,18 @@ func AddAdditionalVolumesAndMounts(template *corev1.PodTemplateSpec, volumes []v // - https://github.com/kubernetes/api/blob/b40c1cacbb902b21a7e0c7bf0967321860c1a632/core/v1/types.go#L3895C27-L3896C33 names := sets.New(spec.Containers...) - for i, c := range template.Spec.Containers { + for i, c := range pod.Containers { if spec.Containers == nil || names.Has(c.Name) { c.VolumeMounts = append(c.VolumeMounts, mount) - template.Spec.Containers[i] = c + pod.Containers[i] = c } names.Delete(c.Name) } - for i, c := range template.Spec.InitContainers { + for i, c := range pod.InitContainers { if spec.Containers == nil || names.Has(c.Name) { c.VolumeMounts = append(c.VolumeMounts, mount) - template.Spec.InitContainers[i] = c + pod.InitContainers[i] = c } names.Delete(c.Name) } diff --git a/internal/util/volumes_test.go b/internal/util/volumes_test.go index 0ff005943f..f2c8eafe40 100644 --- a/internal/util/volumes_test.go +++ b/internal/util/volumes_test.go @@ -5,6 +5,7 @@ package util import ( + "slices" "testing" "github.com/google/go-cmp/cmp/cmpopts" @@ -19,16 +20,16 @@ import ( func TestAddAdditionalVolumesAndMounts(t *testing.T) { t.Parallel() - podTemplate := &corev1.PodTemplateSpec{ - Spec: corev1.PodSpec{ - InitContainers: []corev1.Container{ - {Name: "startup"}, - {Name: "config"}, - }, - Containers: []corev1.Container{ - {Name: "database"}, - {Name: "other"}, - }}} + podSpec := corev1.PodSpec{ + InitContainers: []corev1.Container{ + {Name: "startup"}, + {Name: "config"}, + }, + Containers: []corev1.Container{ + {Name: "database"}, + {Name: "other"}, + }, + } testCases := []struct { tcName string @@ -209,33 +210,16 @@ func TestAddAdditionalVolumesAndMounts(t *testing.T) { for _, tc := range testCases { t.Run(tc.tcName, func(t *testing.T) { - copyPodTemplate := podTemplate.DeepCopy() + sink := podSpec.DeepCopy() + missingContainers := AddAdditionalVolumesAndMounts(sink, tc.additionalVolumes) - missingContainers := AddAdditionalVolumesAndMounts( - copyPodTemplate, - tc.additionalVolumes, - ) + assert.Assert(t, cmp.MarshalMatches(sink.Containers, tc.expectedContainers)) + assert.Assert(t, cmp.MarshalMatches(sink.InitContainers, tc.expectedInitContainers)) + assert.Assert(t, cmp.MarshalMatches(sink.Volumes, tc.expectedVolumes)) - assert.Assert(t, cmp.MarshalMatches( - copyPodTemplate.Spec.Containers, - tc.expectedContainers)) - assert.Assert(t, cmp.MarshalMatches( - copyPodTemplate.Spec.InitContainers, - tc.expectedInitContainers)) - assert.Assert(t, cmp.MarshalMatches( - copyPodTemplate.Spec.Volumes, - tc.expectedVolumes)) - if len(tc.expectedMissing) == 0 { - assert.Assert(t, cmp.DeepEqual( - missingContainers, - tc.expectedMissing)) - } else { - for _, mc := range tc.expectedMissing { - assert.Assert(t, cmp.Contains( - missingContainers, - mc)) - } - } + slices.Sort(missingContainers) + slices.Sort(tc.expectedMissing) + assert.DeepEqual(t, missingContainers, tc.expectedMissing) }) } } From bc765f09ca2ba3f5123ad210b7672836f31b6861 Mon Sep 17 00:00:00 2001 From: Chris Bandy Date: Fri, 5 Sep 2025 12:04:08 -0500 Subject: [PATCH 3/4] Share logic between AddAdditionalVolumesAndMounts and AddVolumeAndMountsToPod --- internal/util/volumes.go | 57 ++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 32 deletions(-) diff --git a/internal/util/volumes.go b/internal/util/volumes.go index 1565f71c55..ea605128ee 100644 --- a/internal/util/volumes.go +++ b/internal/util/volumes.go @@ -25,10 +25,34 @@ func AdditionalVolumeMount(name string, readOnly bool) corev1.VolumeMount { // AddAdditionalVolumesAndMounts adds volumes as [corev1.Volume]s and [corev1.VolumeMount]s in pod. // Volume names are chosen in [AdditionalVolumeMount]. func AddAdditionalVolumesAndMounts(pod *corev1.PodSpec, volumes []v1beta1.AdditionalVolume) []string { + return addVolumesAndMounts(pod, volumes, AdditionalVolumeMount) +} + +// AddVolumeAndMountsToPod takes a Pod spec and a PVC and adds a Volume to the Pod spec with +// the PVC as the VolumeSource and mounts the volume to all containers and init containers +// in the Pod spec. +func AddVolumeAndMountsToPod(podSpec *corev1.PodSpec, volume *corev1.PersistentVolumeClaim) { + additional := []v1beta1.AdditionalVolume{{ + ClaimName: volume.Name, + Name: volume.Name, + ReadOnly: false, + }} + + addVolumesAndMounts(podSpec, additional, func(string, bool) corev1.VolumeMount { + return corev1.VolumeMount{ + // This name has no prefix and differs from [AdditionalVolumeMount]. + Name: volume.Name, + MountPath: fmt.Sprintf("/volumes/%s", volume.Name), + ReadOnly: false, + } + }) +} + +func addVolumesAndMounts(pod *corev1.PodSpec, volumes []v1beta1.AdditionalVolume, namer func(string, bool) corev1.VolumeMount) []string { missingContainers := []string{} for _, spec := range volumes { - mount := AdditionalVolumeMount(spec.Name, spec.ReadOnly) + mount := namer(spec.Name, spec.ReadOnly) pod.Volumes = append(pod.Volumes, spec.AsVolume(mount.Name)) // Create a set of all the requested containers, @@ -60,34 +84,3 @@ func AddAdditionalVolumesAndMounts(pod *corev1.PodSpec, volumes []v1beta1.Additi return missingContainers } - -// AddVolumeAndMountsToPod takes a Pod spec and a PVC and adds a Volume to the Pod spec with -// the PVC as the VolumeSource and mounts the volume to all containers and init containers -// in the Pod spec. -func AddVolumeAndMountsToPod(podSpec *corev1.PodSpec, volume *corev1.PersistentVolumeClaim) { - - podSpec.Volumes = append(podSpec.Volumes, corev1.Volume{ - Name: volume.Name, - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: volume.Name, - }, - }, - }) - - for i := range podSpec.Containers { - podSpec.Containers[i].VolumeMounts = append(podSpec.Containers[i].VolumeMounts, - corev1.VolumeMount{ - Name: volume.Name, - MountPath: fmt.Sprintf("/volumes/%s", volume.Name), - }) - } - - for i := range podSpec.InitContainers { - podSpec.InitContainers[i].VolumeMounts = append(podSpec.InitContainers[i].VolumeMounts, - corev1.VolumeMount{ - Name: volume.Name, - MountPath: fmt.Sprintf("/volumes/%s", volume.Name), - }) - } -} From 38dd930e054df569a94f4d090787728a8746a485 Mon Sep 17 00:00:00 2001 From: Chris Bandy Date: Fri, 5 Sep 2025 12:25:26 -0500 Subject: [PATCH 4/4] Stop checking that "pgbackrest-cloud-log-volume" PVC exists --- .../controller/postgrescluster/pgbackrest.go | 20 +---- .../postgrescluster/pgbackrest_test.go | 84 +------------------ internal/util/volumes.go | 12 +-- internal/util/volumes_test.go | 9 +- 4 files changed, 10 insertions(+), 115 deletions(-) diff --git a/internal/controller/postgrescluster/pgbackrest.go b/internal/controller/postgrescluster/pgbackrest.go index ba9fbc57e4..0acb86513f 100644 --- a/internal/controller/postgrescluster/pgbackrest.go +++ b/internal/controller/postgrescluster/pgbackrest.go @@ -884,25 +884,9 @@ func (r *Reconciler) generateBackupJobSpecIntent(ctx context.Context, postgresCl jobSpec.Template.Spec.SecurityContext = postgres.PodSecurityContext(postgresCluster) pgbackrest.AddConfigToCloudBackupJob(postgresCluster, &jobSpec.Template) - // If the user has specified a PVC to use as a log volume via the PGBackRestCloudLogVolume - // annotation, check for the PVC. If we find it, mount it to the backup job. - // Otherwise, create a warning event. + // Mount the PVC named in the "pgbackrest-cloud-log-volume" annotation, if any. if logVolumeName := postgresCluster.Annotations[naming.PGBackRestCloudLogVolume]; logVolumeName != "" { - logVolume := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: logVolumeName, - Namespace: postgresCluster.GetNamespace(), - }, - } - err := errors.WithStack(r.Client.Get(ctx, - client.ObjectKeyFromObject(logVolume), logVolume)) - if err != nil { - // PVC not retrieved, create warning event - r.Recorder.Event(postgresCluster, corev1.EventTypeWarning, "PGBackRestCloudLogVolumeNotFound", err.Error()) - } else { - // We successfully found the specified PVC, so we will add it to the backup job - util.AddVolumeAndMountsToPod(&jobSpec.Template.Spec, logVolume) - } + util.AddCloudLogVolumeToPod(&jobSpec.Template.Spec, logVolumeName) } } diff --git a/internal/controller/postgrescluster/pgbackrest_test.go b/internal/controller/postgrescluster/pgbackrest_test.go index 4591f1770f..0eac94ba7e 100644 --- a/internal/controller/postgrescluster/pgbackrest_test.go +++ b/internal/controller/postgrescluster/pgbackrest_test.go @@ -2947,79 +2947,7 @@ volumes: }) }) - t.Run("CloudLogVolumeAnnotationNoPvc", func(t *testing.T) { - recorder := events.NewRecorder(t, runtime.Scheme) - r.Recorder = recorder - - cluster.Namespace = ns.Name - cluster.Annotations = map[string]string{} - cluster.Annotations[naming.PGBackRestCloudLogVolume] = "some-pvc" - spec := r.generateBackupJobSpecIntent(ctx, - &cluster, v1beta1.PGBackRestRepo{}, - "", - nil, nil, - ) - assert.Assert(t, cmp.MarshalMatches(spec.Template.Spec, ` -containers: -- command: - - /bin/pgbackrest - - backup - - --stanza=db - - --repo= - name: pgbackrest - resources: {} - securityContext: - allowPrivilegeEscalation: false - capabilities: - drop: - - ALL - privileged: false - readOnlyRootFilesystem: true - runAsNonRoot: true - seccompProfile: - type: RuntimeDefault - volumeMounts: - - mountPath: /etc/pgbackrest/conf.d - name: pgbackrest-config - readOnly: true - - mountPath: /tmp - name: tmp -enableServiceLinks: false -restartPolicy: Never -securityContext: - fsGroup: 26 - fsGroupChangePolicy: OnRootMismatch -volumes: -- name: pgbackrest-config - projected: - sources: - - configMap: - items: - - key: pgbackrest_cloud.conf - path: pgbackrest_cloud.conf - name: hippo-test-pgbackrest-config - - secret: - items: - - key: pgbackrest.ca-roots - path: ~postgres-operator/tls-ca.crt - - key: pgbackrest-client.crt - path: ~postgres-operator/client-tls.crt - - key: pgbackrest-client.key - mode: 384 - path: ~postgres-operator/client-tls.key - name: hippo-test-pgbackrest -- emptyDir: - sizeLimit: 16Mi - name: tmp - `)) - - assert.Equal(t, len(recorder.Events), 1) - assert.Equal(t, recorder.Events[0].Regarding.Name, cluster.Name) - assert.Equal(t, recorder.Events[0].Reason, "PGBackRestCloudLogVolumeNotFound") - assert.Equal(t, recorder.Events[0].Note, "persistentvolumeclaims \"some-pvc\" not found") - }) - - t.Run("CloudLogVolumeAnnotationPvcInPlace", func(t *testing.T) { + t.Run("CloudLogVolumeAnnotation", func(t *testing.T) { recorder := events.NewRecorder(t, runtime.Scheme) r.Recorder = recorder @@ -3027,16 +2955,6 @@ volumes: cluster.Annotations = map[string]string{} cluster.Annotations[naming.PGBackRestCloudLogVolume] = "another-pvc" - pvc := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "another-pvc", - Namespace: ns.Name, - }, - Spec: corev1.PersistentVolumeClaimSpec(testVolumeClaimSpec()), - } - err := r.Client.Create(ctx, pvc) - assert.NilError(t, err) - spec := r.generateBackupJobSpecIntent(ctx, &cluster, v1beta1.PGBackRestRepo{}, "", diff --git a/internal/util/volumes.go b/internal/util/volumes.go index ea605128ee..9870920a97 100644 --- a/internal/util/volumes.go +++ b/internal/util/volumes.go @@ -28,21 +28,21 @@ func AddAdditionalVolumesAndMounts(pod *corev1.PodSpec, volumes []v1beta1.Additi return addVolumesAndMounts(pod, volumes, AdditionalVolumeMount) } -// AddVolumeAndMountsToPod takes a Pod spec and a PVC and adds a Volume to the Pod spec with +// AddCloudLogVolumeToPod takes a Pod spec and a PVC and adds a Volume to the Pod spec with // the PVC as the VolumeSource and mounts the volume to all containers and init containers // in the Pod spec. -func AddVolumeAndMountsToPod(podSpec *corev1.PodSpec, volume *corev1.PersistentVolumeClaim) { +func AddCloudLogVolumeToPod(podSpec *corev1.PodSpec, pvcName string) { additional := []v1beta1.AdditionalVolume{{ - ClaimName: volume.Name, - Name: volume.Name, + ClaimName: pvcName, + Name: pvcName, ReadOnly: false, }} addVolumesAndMounts(podSpec, additional, func(string, bool) corev1.VolumeMount { return corev1.VolumeMount{ // This name has no prefix and differs from [AdditionalVolumeMount]. - Name: volume.Name, - MountPath: fmt.Sprintf("/volumes/%s", volume.Name), + Name: pvcName, + MountPath: fmt.Sprintf("/volumes/%s", pvcName), ReadOnly: false, } }) diff --git a/internal/util/volumes_test.go b/internal/util/volumes_test.go index f2c8eafe40..81e4f17a3f 100644 --- a/internal/util/volumes_test.go +++ b/internal/util/volumes_test.go @@ -11,7 +11,6 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" "gotest.tools/v3/assert" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/crunchydata/postgres-operator/internal/testing/cmp" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" @@ -237,12 +236,6 @@ func TestAddVolumeAndMountsToPod(t *testing.T) { }, } - volume := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "volume-name", - }, - } - alwaysExpect := func(t testing.TB, result *corev1.PodSpec) { // Only Containers, InitContainers, and Volumes fields have changed. assert.DeepEqual(t, *pod, *result, cmpopts.IgnoreFields(*pod, "Containers", "InitContainers", "Volumes")) @@ -282,6 +275,6 @@ func TestAddVolumeAndMountsToPod(t *testing.T) { } out := pod.DeepCopy() - AddVolumeAndMountsToPod(out, volume) + AddCloudLogVolumeToPod(out, "volume-name") alwaysExpect(t, out) }