diff --git a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml index 9fe1ccf43..4db85ca84 100644 --- a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml +++ b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml @@ -11027,6 +11027,50 @@ spec: type: array volumes: properties: + additional: + description: Additional pre-existing volumes to add to the + pod. + items: + properties: + claimName: + description: A reference to a preexisting PVC. + maxLength: 253 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?([.][a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + containers: + description: |- + The containers to attach this volume to. + An omitted `Containers` field matches all containers. + An empty `Containers` field matches no containers. + items: + type: string + maxItems: 10 + type: array + x-kubernetes-list-type: atomic + name: + allOf: + - maxLength: 63 + - maxLength: 55 + description: |- + The name of the volume used for mounting path. + Volumes are mounted in the pods at `volumes/` + Must be unique. + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ + type: string + readOnly: + description: Sets the write/read mode of the volume + type: boolean + required: + - claimName + - name + type: object + maxItems: 10 + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map temp: description: |- An ephemeral volume for temporary files. @@ -29598,6 +29642,50 @@ spec: type: array volumes: properties: + additional: + description: Additional pre-existing volumes to add to the + pod. + items: + properties: + claimName: + description: A reference to a preexisting PVC. + maxLength: 253 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?([.][a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + containers: + description: |- + The containers to attach this volume to. + An omitted `Containers` field matches all containers. + An empty `Containers` field matches no containers. + items: + type: string + maxItems: 10 + type: array + x-kubernetes-list-type: atomic + name: + allOf: + - maxLength: 63 + - maxLength: 55 + description: |- + The name of the volume used for mounting path. + Volumes are mounted in the pods at `volumes/` + Must be unique. + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ + type: string + readOnly: + description: Sets the write/read mode of the volume + type: boolean + required: + - claimName + - name + type: object + maxItems: 10 + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map temp: description: |- An ephemeral volume for temporary files. diff --git a/internal/controller/postgrescluster/instance.go b/internal/controller/postgrescluster/instance.go index 0c91ca715..f9b8e12cd 100644 --- a/internal/controller/postgrescluster/instance.go +++ b/internal/controller/postgrescluster/instance.go @@ -1253,6 +1253,16 @@ func (r *Reconciler) reconcileInstance( addDevSHM(&instance.Spec.Template) } + // 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) + + if len(missingContainers) > 0 { + r.Recorder.Eventf(cluster, corev1.EventTypeWarning, "SpecifiedContainerNotFound", + "The following containers were specified for additional volumes but cannot be found: %s.", missingContainers) + } + } + if err == nil { err = errors.WithStack(r.apply(ctx, instance)) } diff --git a/internal/controller/postgrescluster/util.go b/internal/controller/postgrescluster/util.go index a1ba6ce08..2fc849956 100644 --- a/internal/controller/postgrescluster/util.go +++ b/internal/controller/postgrescluster/util.go @@ -13,9 +13,11 @@ 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") @@ -285,3 +287,81 @@ 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 8e7d5c434..0dde296ae 100644 --- a/internal/controller/postgrescluster/util_test.go +++ b/internal/controller/postgrescluster/util_test.go @@ -16,6 +16,7 @@ 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) { @@ -378,3 +379,271 @@ 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/pkg/apis/postgres-operator.crunchydata.com/v1/postgrescluster_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1/postgrescluster_types.go index abd23670c..9463c2361 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1/postgrescluster_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1/postgrescluster_types.go @@ -529,6 +529,14 @@ type PostgresInstanceSetSpec struct { } type PostgresVolumesSpec struct { + // Additional pre-existing volumes to add to the pod. + // --- + // +optional + // +listType=map + // +listMapKey=name + // +kubebuilder:validation:MaxItems=10 + Additional []v1beta1.AdditionalVolume `json:"additional,omitempty"` + // An ephemeral volume for temporary files. // More info: https://kubernetes.io/docs/concepts/storage/ephemeral-volumes // --- diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1/zz_generated.deepcopy.go b/pkg/apis/postgres-operator.crunchydata.com/v1/zz_generated.deepcopy.go index 94a6ed338..4c5826c02 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1/zz_generated.deepcopy.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1/zz_generated.deepcopy.go @@ -652,6 +652,13 @@ func (in *PostgresUserInterfaceStatus) DeepCopy() *PostgresUserInterfaceStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PostgresVolumesSpec) DeepCopyInto(out *PostgresVolumesSpec) { *out = *in + if in.Additional != nil { + in, out := &in.Additional, &out.Additional + *out = make([]v1beta1.AdditionalVolume, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.Temp != nil { in, out := &in.Temp, &out.Temp *out = (*in).DeepCopy() diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go index 07c6d4c80..4e9af31b5 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go @@ -526,6 +526,14 @@ type PostgresInstanceSetSpec struct { } type PostgresVolumesSpec struct { + // Additional pre-existing volumes to add to the pod. + // --- + // +optional + // +listType=map + // +listMapKey=name + // +kubebuilder:validation:MaxItems=10 + Additional []AdditionalVolume `json:"additional,omitempty"` + // An ephemeral volume for temporary files. // More info: https://kubernetes.io/docs/concepts/storage/ephemeral-volumes // --- @@ -533,6 +541,38 @@ type PostgresVolumesSpec struct { Temp *VolumeClaimSpec `json:"temp,omitempty"` } +type AdditionalVolume struct { + // A reference to a preexisting PVC. + // --- + // +required + ClaimName DNS1123Subdomain `json:"claimName"` + + // The containers to attach this volume to. + // An omitted `Containers` field matches all containers. + // An empty `Containers` field matches no containers. + // --- + // +optional + // +listType=atomic + // +kubebuilder:validation:MaxItems=10 + Containers []string `json:"containers,omitempty"` + + // The name of the volume used for mounting path. + // Volumes are mounted in the pods at `volumes/` + // Must be unique. + // --- + // The `Name` field is a `DNS1123Label` type to enforce + // the max length. + // +required + // Max length is less than max 63 to allow prepending `volumes-` to name + // +kubebuilder:validation:MaxLength=55 + Name DNS1123Label `json:"name"` + + // Sets the write/read mode of the volume + // --- + // +optional + ReadOnly bool `json:"readOnly,omitempty"` +} + type TablespaceVolume struct { // This value goes into // a. the name of a corev1.PersistentVolumeClaim, 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 c185cd4b2..7a7d55427 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types.go @@ -32,6 +32,16 @@ type ConfigDataKey = string // +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?([.][a-z0-9]([-a-z0-9]*[a-z0-9])?)*$` type DNS1123Subdomain = string +// --- +// https://docs.k8s.io/concepts/overview/working-with-objects/names#dns-label-names +// https://pkg.go.dev/k8s.io/apimachinery/pkg/util/validation#IsDNS1123Label +// https://pkg.go.dev/k8s.io/apiserver/pkg/cel/library#Format +// +// +kubebuilder:validation:MinLength=1 +// +kubebuilder:validation:MaxLength=63 +// +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$` +type DNS1123Label = string + // --- // Duration represents a string accepted by the Kubernetes API in the "duration" // [format]. This format extends the "duration" [defined by OpenAPI] by allowing diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go index 747e36385..02dd91b82 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go @@ -33,6 +33,26 @@ func (in *APIResponses) DeepCopy() *APIResponses { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AdditionalVolume) DeepCopyInto(out *AdditionalVolume) { + *out = *in + if in.Containers != nil { + in, out := &in.Containers, &out.Containers + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AdditionalVolume. +func (in *AdditionalVolume) DeepCopy() *AdditionalVolume { + if in == nil { + return nil + } + out := new(AdditionalVolume) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BackupJobs) DeepCopyInto(out *BackupJobs) { *out = *in @@ -2510,6 +2530,13 @@ func (in *PostgresUserSpec) DeepCopy() *PostgresUserSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PostgresVolumesSpec) DeepCopyInto(out *PostgresVolumesSpec) { *out = *in + if in.Additional != nil { + in, out := &in.Additional, &out.Additional + *out = make([]AdditionalVolume, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.Temp != nil { in, out := &in.Temp, &out.Temp *out = (*in).DeepCopy()