Focus additional volume mounting on preexisting PVCs

Author: benjaminjb

config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml

@@ -1183,10 +1183,6 @@ func (r *Reconciler) reconcileInstance(
 	if err == nil {
 		tablespaceVolumes, err = r.reconcileTablespaceVolumes(ctx, cluster, spec, instance, clusterVolumes)
 	}
-	if err == nil {
-		_, err = r.reconcileAdditionalVolumes(ctx,
-			cluster, spec.Volumes.Additional)
-	}
 	if err == nil {
 		postgres.InstancePod(
 			ctx, cluster, spec,
@@ -1259,7 +1255,7 @@ func (r *Reconciler) reconcileInstance(
 
 	// mount additional volumes to the Postgres instance containers
 	if err == nil && spec.Volumes != nil && spec.Volumes.Additional != nil {
-		addAdditionalVolumes(&instance.Spec.Template, cluster, spec.Volumes.Additional)
+		addAdditionalVolumesToSpecifiedContainers(&instance.Spec.Template, spec.Volumes.Additional)
 	}
 
 	if err == nil {

internal/controller/postgrescluster/instance.go

@@ -289,41 +289,39 @@ func safeHash32(content func(w io.Writer) error) (string, error) {
 }
 
 // AdditionalVolumeMount returns the name and mount path of the additional volume.
-func AdditionalVolumeMount(cluster *v1beta1.PostgresCluster,
-	additionalVolume *v1beta1.AdditionalVolume) corev1.VolumeMount {
-
-	volumeName := naming.AdditionalVolume(cluster, additionalVolume).Name
-
+func AdditionalVolumeMount(volumeName, claimName string) corev1.VolumeMount {
 	return corev1.VolumeMount{
-		Name:      volumeName,
-		MountPath: "volumes" + "/" + additionalVolume.Name}
+		Name:      claimName,
+		MountPath: "/volumes" + "/" + volumeName}
 }
 
-// addAdditionalVolumes adds additional volumes to the Postgres pod
-func addAdditionalVolumes(template *corev1.PodTemplateSpec,
-	inCluster *v1beta1.PostgresCluster,
+// addAdditionalVolumesToSpecifiedContainers adds additional volumes to the specified
+// containers in the specified pod
+func addAdditionalVolumesToSpecifiedContainers(template *corev1.PodTemplateSpec,
 	additionalVolumes []*v1beta1.AdditionalVolume) {
 
-	for _, additionalVolume := range additionalVolumes {
+	for _, additionalVolumeRequest := range additionalVolumes {
 
 		additionalVolumeMount := AdditionalVolumeMount(
-			inCluster,
-			additionalVolume)
+			additionalVolumeRequest.Name,
+			additionalVolumeRequest.ClaimName)
 
-		additionalVolumeSpec := corev1.Volume{
+		additionalVolume := corev1.Volume{
 			Name: additionalVolumeMount.Name,
 			VolumeSource: corev1.VolumeSource{
 				PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
+					// TODO: Using the claim name as the claim name here means this has to be unique
+					// We could add the index if we want someone to mount the same PVC in multiple dirs?
 					ClaimName: additionalVolumeMount.Name,
-					ReadOnly:  additionalVolume.ReadOnly,
+					ReadOnly:  additionalVolumeRequest.ReadOnly,
 				},
 			},
 		}
 
 		for i := range template.Spec.Containers {
-			if len(additionalVolume.Containers) == 0 ||
+			if len(additionalVolumeRequest.Containers) == 0 ||
 				slices.Contains(
-					additionalVolume.Containers,
+					additionalVolumeRequest.Containers,
 					template.Spec.Containers[i].Name) {
 				template.Spec.Containers[i].VolumeMounts = append(
 					template.Spec.Containers[i].VolumeMounts,
@@ -332,9 +330,9 @@ func addAdditionalVolumes(template *corev1.PodTemplateSpec,
 		}
 
 		for i := range template.Spec.InitContainers {
-			if len(additionalVolume.Containers) == 0 ||
+			if len(additionalVolumeRequest.Containers) == 0 ||
 				slices.Contains(
-					additionalVolume.Containers,
+					additionalVolumeRequest.Containers,
 					template.Spec.InitContainers[i].Name) {
 				template.Spec.InitContainers[i].VolumeMounts = append(
 					template.Spec.InitContainers[i].VolumeMounts,
@@ -344,6 +342,6 @@ func addAdditionalVolumes(template *corev1.PodTemplateSpec,
 
 		template.Spec.Volumes = append(
 			template.Spec.Volumes,
-			additionalVolumeSpec)
+			additionalVolume)
 	}
 }

internal/controller/postgrescluster/util.go

@@ -906,64 +906,3 @@ func getPVCName(volumes []*corev1.PersistentVolumeClaim, selector labels.Selecto
 	}
 	return ""
 }
-
-// +kubebuilder:rbac:groups="",resources="persistentvolumeclaims",verbs={get}
-// +kubebuilder:rbac:groups="",resources="persistentvolumeclaims",verbs={create,delete,patch}
-
-// reconcileAdditionalVolumes writes the PersistentVolumeClaim for additional volumes.
-func (r *Reconciler) reconcileAdditionalVolumes(
-	ctx context.Context,
-	cluster *v1beta1.PostgresCluster,
-	additionalVolumes []*v1beta1.AdditionalVolume,
-	// instance *appsv1.StatefulSet,
-	// observed *Instance,
-) (additionalVolumePVCs []*corev1.PersistentVolumeClaim, err error) {
-
-	if len(additionalVolumes) == 0 {
-		return
-	}
-
-	for _, additionalVolume := range additionalVolumes {
-		// If the user provides a PVC, we leave it alone here
-		// and use it to mount the volume
-		if additionalVolume.ClaimName != "" {
-			continue
-		}
-
-		// TODO: What labels do we want to add
-		labelMap := map[string]string{
-			naming.LabelCluster: cluster.Name,
-		}
-
-		pvc := &corev1.PersistentVolumeClaim{ObjectMeta: naming.AdditionalVolume(cluster, additionalVolume)}
-
-		pvc.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"))
-
-		err = errors.WithStack(r.setControllerReference(cluster, pvc))
-
-		// TODO: Do we have other labels/annotation sources we want to merge?
-		pvc.Annotations = naming.Merge(
-			cluster.Spec.Metadata.GetAnnotationsOrNil())
-
-		pvc.Labels = naming.Merge(
-			cluster.Spec.Metadata.GetLabelsOrNil(),
-			labelMap,
-		)
-
-		pvc.Spec = additionalVolume.ClaimTemplate.AsPersistentVolumeClaimSpec()
-
-		if err == nil {
-			err = r.handlePersistentVolumeClaimError(cluster,
-				errors.WithStack(r.apply(ctx, pvc)))
-		}
-
-		if err != nil {
-			return nil, err
-		}
-
-		additionalVolumePVCs = append(additionalVolumePVCs, pvc)
-	}
-
-	return
-
-}

internal/controller/postgrescluster/volumes.go

@@ -350,21 +350,6 @@ func InstancePostgresWALVolume(instance *appsv1.StatefulSet) metav1.ObjectMeta {
 	}
 }
 
-// AdditionalVolume returns the ObjectMeta for the additional
-// volume for instance.
-func AdditionalVolume(cluster *v1beta1.PostgresCluster,
-	volume *v1beta1.AdditionalVolume) metav1.ObjectMeta {
-	// TODO: What's the name for the PVC if not given?
-	volumeName := volume.ClaimName
-	if volumeName == "" {
-		volumeName = cluster.GetName() + "-additional-something"
-	}
-	return metav1.ObjectMeta{
-		Namespace: cluster.GetNamespace(),
-		Name:      volumeName,
-	}
-}
-
 // MonitoringUserSecret returns ObjectMeta necessary to lookup the Secret
 // containing authentication credentials for monitoring tools.
 func MonitoringUserSecret(cluster *v1beta1.PostgresCluster) metav1.ObjectMeta {

internal/naming/names.go

@@ -535,14 +535,12 @@ type PostgresVolumesSpec struct {
 	// +optional
 	Temp *v1beta1.VolumeClaimSpec `json:"temp,omitempty"`
 
-	// TODO: Update when v1beta1 is worked out
-	// Additional volumes to add to the pod.
+	// Additional pre-existing volumes to add to the pod.
 	// ---
 	// +optional
 	// +listType=map
 	// +listMapKey=name
 	// +kubebuilder:validation:MaxItems=10
-	// // +kubebuilder:validation:items:XValidation:rule=`[has(self.claimName), has(self.claimTemplate)].filter(x, x).size() <= 1`,message=`can only have one of claimName, claimTemplate`
 	Additional []*v1beta1.AdditionalVolume `json:"additional,omitempty"`
 }
 

pkg/apis/postgres-operator.crunchydata.com/v1/postgrescluster_types.go

@@ -532,42 +532,33 @@ type PostgresVolumesSpec struct {
 	// +optional
 	Temp *VolumeClaimSpec `json:"temp,omitempty"`
 
-	// These CRD xvalidation rules were over the complexity budget
-	// so I turned them off -- what's wrong here
-	// Additional volumes to add to the pod.
+	// Additional pre-existing volumes to add to the pod.
 	// ---
 	// +optional
 	// +listType=map
 	// +listMapKey=name
-	// +kubebuilder:validation:MaxItems=1
-	// // +kubebuilder:validation:items:XValidation:rule=`[has(self.claimName), has(self.claimTemplate)].filter(x, x).size() <= 1`,message=`can only have one of claimName, claimTemplate`
-	// // +kubebuilder:validation:items:XValidation:rule=`has(oldSelf.claimTemplate) != has(self.claimTemplate)`,message=`testing`
+	// +kubebuilder:validation:MaxItems=10
 	Additional []*AdditionalVolume `json:"additional,omitempty"`
 }
 
 type AdditionalVolume struct {
 	// The name of the volume used for mounting path.
+	// Volumes are mounted in the pods at `volumes/<NAME>`
 	// Must be unique.
 	// ---
+	// The `Name` field is a `DNS1123Subdomain` type to enforce
+	// the max length and also allow us to more easily transition
+	// to CPK-provisioned volumes.
 	// +kubebuilder:validation:Required
-	// TODO: CHANGE THE MAX LENGTH
-	// +kubebuilder:validation:MaxLength=10
 	Name DNS1123Subdomain `json:"name,omitempty"`
 
 	// A reference to a preexisting PVC.
 	// ---
-	// +optional
-	// TODO: CHANGE THE MAX LENGTH
-	// +kubebuilder:validation:MaxLength=10
-	// // +kubebuilder:validation:XValidation:rule=`self == oldSelf`,message="immutable"
+	// +kubebuilder:validation:Required
 	ClaimName string `json:"claimName,omitempty"`
 
-	// A PVC request.
-	// ---
-	// +optional
-	ClaimTemplate *VolumeClaimSpec `json:"claimTemplate,omitempty"`
-
 	// The containers to attach this volume to.
+	// A blank/unset `Containers` field matches all containers.
 	// ---
 	// +optional
 	// +listType=set