From 7bcb73a40271a5d0a287e3c7dee933076ec6b5fd Mon Sep 17 00:00:00 2001 From: Samuel Mutel <12967891+smutel@users.noreply.github.com> Date: Fri, 24 May 2024 11:55:22 +0200 Subject: [PATCH] feat: Add SubPathExpr option for additionalVolumes (#2463) --- .../postgres-operator/crds/postgresqls.yaml | 2 + docs/reference/cluster_manifest.md | 1 + manifests/complete-postgres-manifest.yaml | 10 ++++ manifests/postgresql.crd.yaml | 2 + pkg/apis/acid.zalan.do/v1/crds.go | 3 ++ pkg/apis/acid.zalan.do/v1/postgresql_type.go | 1 + pkg/cluster/k8sres.go | 13 +++-- pkg/cluster/k8sres_test.go | 53 +++++++++++++++++-- 8 files changed, 78 insertions(+), 7 deletions(-) diff --git a/charts/postgres-operator/crds/postgresqls.yaml b/charts/postgres-operator/crds/postgresqls.yaml index afb6ee618..5539ae3cc 100644 --- a/charts/postgres-operator/crds/postgresqls.yaml +++ b/charts/postgres-operator/crds/postgresqls.yaml @@ -101,6 +101,8 @@ spec: x-kubernetes-preserve-unknown-fields: true subPath: type: string + isSubPathExpr: + type: boolean allowedSourceRanges: type: array nullable: true diff --git a/docs/reference/cluster_manifest.md b/docs/reference/cluster_manifest.md index 48334724a..665d5aece 100644 --- a/docs/reference/cluster_manifest.md +++ b/docs/reference/cluster_manifest.md @@ -242,6 +242,7 @@ These parameters are grouped directly under the `spec` key in the manifest. It allows you to mount existing PersistentVolumeClaims, ConfigMaps and Secrets inside the StatefulSet. Also an `emptyDir` volume can be shared between initContainer and statefulSet. Additionaly, you can provide a `SubPath` for volume mount (a file in a configMap source volume, for example). + Set `isSubPathExpr` to true if you want to include [API environment variables](https://kubernetes.io/docs/concepts/storage/volumes/#using-subpath-expanded-environment). You can also specify in which container the additional Volumes will be mounted with the `targetContainers` array option. If `targetContainers` is empty, additional volumes will be mounted only in the `postgres` container. If you set the `all` special item, it will be mounted in all containers (postgres + sidecars). diff --git a/manifests/complete-postgres-manifest.yaml b/manifests/complete-postgres-manifest.yaml index a306b4477..a2a1ebe8f 100644 --- a/manifests/complete-postgres-manifest.yaml +++ b/manifests/complete-postgres-manifest.yaml @@ -83,6 +83,16 @@ spec: # PersistentVolumeClaim: # claimName: pvc-postgresql-data-partitions # readyOnly: false +# - name: data +# mountPath: /home/postgres/pgdata/partitions +# subPath: $(NODE_NAME)/$(POD_NAME) +# isSubPathExpr: true +# targetContainers: +# - postgres +# volumeSource: +# PersistentVolumeClaim: +# claimName: pvc-postgresql-data-partitions +# readyOnly: false # - name: conf # mountPath: /etc/telegraf # subPath: telegraf.conf diff --git a/manifests/postgresql.crd.yaml b/manifests/postgresql.crd.yaml index 26011596a..e5d83b11d 100644 --- a/manifests/postgresql.crd.yaml +++ b/manifests/postgresql.crd.yaml @@ -99,6 +99,8 @@ spec: x-kubernetes-preserve-unknown-fields: true subPath: type: string + isSubPathExpr: + type: boolean allowedSourceRanges: type: array nullable: true diff --git a/pkg/apis/acid.zalan.do/v1/crds.go b/pkg/apis/acid.zalan.do/v1/crds.go index b4fc8776b..116ed7a14 100644 --- a/pkg/apis/acid.zalan.do/v1/crds.go +++ b/pkg/apis/acid.zalan.do/v1/crds.go @@ -168,6 +168,9 @@ var PostgresCRDResourceValidation = apiextv1.CustomResourceValidation{ "subPath": { Type: "string", }, + "isSubPathExpr": { + Type: "boolean", + }, }, }, }, diff --git a/pkg/apis/acid.zalan.do/v1/postgresql_type.go b/pkg/apis/acid.zalan.do/v1/postgresql_type.go index 9797feabf..63662255a 100644 --- a/pkg/apis/acid.zalan.do/v1/postgresql_type.go +++ b/pkg/apis/acid.zalan.do/v1/postgresql_type.go @@ -143,6 +143,7 @@ type AdditionalVolume struct { Name string `json:"name"` MountPath string `json:"mountPath"` SubPath string `json:"subPath,omitempty"` + IsSubPathExpr bool `json:"isSubPathExpr,omitemtpy"` TargetContainers []string `json:"targetContainers"` VolumeSource v1.VolumeSource `json:"volumeSource"` } diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 3b7d452b9..3e6ee4fd7 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -1820,11 +1820,18 @@ func (c *Cluster) addAdditionalVolumes(podSpec *v1.PodSpec, for _, additionalVolume := range additionalVolumes { for _, target := range additionalVolume.TargetContainers { if podSpec.Containers[i].Name == target || target == "all" { - mounts = append(mounts, v1.VolumeMount{ + v := v1.VolumeMount{ Name: additionalVolume.Name, MountPath: additionalVolume.MountPath, - SubPath: additionalVolume.SubPath, - }) + } + + if additionalVolume.IsSubPathExpr { + v.SubPathExpr = additionalVolume.SubPath + } else { + v.SubPath = additionalVolume.SubPath + } + + mounts = append(mounts, v) } } } diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go index a4e3eadde..ba6b16d26 100644 --- a/pkg/cluster/k8sres_test.go +++ b/pkg/cluster/k8sres_test.go @@ -1889,6 +1889,25 @@ func TestAdditionalVolume(t *testing.T) { EmptyDir: &v1.EmptyDirVolumeSource{}, }, }, + { + Name: "test5", + MountPath: "/test5", + SubPath: "subpath", + TargetContainers: nil, // should mount only to postgres + VolumeSource: v1.VolumeSource{ + EmptyDir: &v1.EmptyDirVolumeSource{}, + }, + }, + { + Name: "test6", + MountPath: "/test6", + SubPath: "$(POD_NAME)", + IsSubPathExpr: true, + TargetContainers: nil, // should mount only to postgres + VolumeSource: v1.VolumeSource{ + EmptyDir: &v1.EmptyDirVolumeSource{}, + }, + }, } pg := acidv1.Postgresql{ @@ -1935,9 +1954,10 @@ func TestAdditionalVolume(t *testing.T) { assert.NoError(t, err) tests := []struct { - subTest string - container string - expectedMounts []string + subTest string + container string + expectedMounts []string + expectedSubPath []string }{ { subTest: "checking volume mounts of postgres container", @@ -1949,6 +1969,17 @@ func TestAdditionalVolume(t *testing.T) { container: "sidecar", expectedMounts: []string{"pgdata", "test1", "test2"}, }, + { + subTest: "checking volume mounts with subPath", + container: constants.PostgresContainerName, + expectedMounts: []string{"test5"}, + expectedSubPath: []string{"subpath"}, + }, + { + subTest: "checking volume mounts with subPathExpr", + container: constants.PostgresContainerName, + expectedMounts: []string{"test6"}, + }, } for _, tt := range tests { @@ -1957,12 +1988,26 @@ func TestAdditionalVolume(t *testing.T) { continue } mounts := []string{} + subPaths := []string{} + subPathExprs := []string{} for _, volumeMounts := range container.VolumeMounts { mounts = append(mounts, volumeMounts.Name) + subPaths = append(subPaths, volumeMounts.SubPath) + subPathExprs = append(subPathExprs, volumeMounts.SubPathExpr) } if !util.IsEqualIgnoreOrder(mounts, tt.expectedMounts) { - t.Errorf("%s %s: different volume mounts: got %v, epxected %v", + t.Errorf("%s %s: different volume mounts: got %v, expected %v", + t.Name(), tt.subTest, mounts, tt.expectedMounts) + } + + if !util.IsEqualIgnoreOrder(subPaths, tt.expectedSubPath) { + t.Errorf("%s %s: different volume subPaths: got %v, expected %v", + t.Name(), tt.subTest, mounts, tt.expectedSubPath) + } + + if !util.IsEqualIgnoreOrder(subPathExprs, []string{container.Name}) { + t.Errorf("%s %s: different volume subPathExprs: got %v, expected %v", t.Name(), tt.subTest, mounts, tt.expectedMounts) } }