PR feedback

Author: benjaminjb

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

@@ -11034,6 +11034,9 @@ spec:
                             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: |-
@@ -11045,11 +11048,13 @@ spec:
                                 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/<NAME>`
                                   Must be unique.
-                                maxLength: 55
                                 minLength: 1
                                 pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$
                                 type: string
@@ -29643,6 +29648,9 @@ spec:
                             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: |-
@@ -29654,11 +29662,13 @@ spec:
                                 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/<NAME>`
                                   Must be unique.
-                                maxLength: 55
                                 minLength: 1
                                 pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$
                                 type: string

internal/controller/postgrescluster/util.go

@@ -323,6 +323,12 @@ func addAdditionalVolumesToSpecifiedContainers(template *corev1.PodTemplateSpec,
 			},
 		}
 
+		// 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 names.Len() == 0 {
@@ -335,9 +341,7 @@ func addAdditionalVolumesToSpecifiedContainers(template *corev1.PodTemplateSpec,
 					template.Spec.Containers[i].VolumeMounts,
 					additionalVolumeMount)
 
-				if names.Has(template.Spec.Containers[i].Name) {
-					names.Delete(template.Spec.Containers[i].Name)
-				}
+				names.Delete(template.Spec.Containers[i].Name)
 			}
 		}
 
@@ -347,9 +351,8 @@ func addAdditionalVolumesToSpecifiedContainers(template *corev1.PodTemplateSpec,
 					template.Spec.InitContainers[i].VolumeMounts,
 					additionalVolumeMount)
 
-				if names.Has(template.Spec.InitContainers[i].Name) {
-					names.Delete(template.Spec.InitContainers[i].Name)
-				}
+				names.Delete(template.Spec.InitContainers[i].Name)
+
 			}
 		}
 

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

@@ -529,19 +529,19 @@ type PostgresInstanceSetSpec struct {
 }
 
 type PostgresVolumesSpec struct {
-	// An ephemeral volume for temporary files.
-	// More info: https://kubernetes.io/docs/concepts/storage/ephemeral-volumes
-	// ---
-	// +optional
-	Temp *v1beta1.VolumeClaimSpec `json:"temp,omitempty"`
-
 	// 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
+	// ---
+	// +optional
+	Temp *v1beta1.VolumeClaimSpec `json:"temp,omitempty"`
 }
 
 type TablespaceVolume struct {

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

@@ -526,19 +526,19 @@ type PostgresInstanceSetSpec struct {
 }
 
 type PostgresVolumesSpec struct {
-	// An ephemeral volume for temporary files.
-	// More info: https://kubernetes.io/docs/concepts/storage/ephemeral-volumes
-	// ---
-	// +optional
-	Temp *VolumeClaimSpec `json:"temp,omitempty"`
-
 	// 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
+	// ---
+	// +optional
+	Temp *VolumeClaimSpec `json:"temp,omitempty"`
 }
 
 type AdditionalVolume struct {
@@ -549,12 +549,14 @@ type AdditionalVolume struct {
 	// 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"`
 
 	// A reference to a preexisting PVC.
 	// ---
 	// +required
-	ClaimName string `json:"claimName"`
+	ClaimName DNS1123Subdomain `json:"claimName"`
 
 	// The containers to attach this volume to.
 	// A blank/unset `Containers` field matches all containers.

pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types.go

@@ -33,12 +33,12 @@ type ConfigDataKey = string
 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
-// Max length is less than max 63 to allow prepending `volumes-` to name
-// +kubebuilder:validation:MaxLength=55
+// +kubebuilder:validation:MaxLength=63
 // +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`
 type DNS1123Label = string