Attach additional volume for postgres,

[benjaminjb]

Checklist:

• Have you added an explanation of what your changes do and why you'd like them to be included?
• Have you updated or added documentation for the change, as applicable?
• Have you tested your changes on all related environments with successful results, as applicable?
  • Have you added automated tests?

Type of Changes:

• New feature
• Bug fix
• Documentation
• Testing enhancement
• Other

What is the current behavior (link to any open issues here)?

Only can add additional volumes to database container through abuse of the tablespace volumes.

What is the new behavior (if this is a feature change)?

• Breaking change (fix or feature that would cause existing functionality to change)

Ability to add PVC (either BYO or create from template) to multiple containers in the Postgres pod

Other Information:
Issues: [PGO-2556]

[cbandy]

🔧 The name field, I imagine. That field should have validation that rejects weird filepath stuff (e.g. slash) and is also valid for K8s API name (subdomain?). We have a few predefined types for names.

postgres-operator/internal/naming/limitations.md

Line 80 in 899c4a0

[PersistentVolumeClaim (PVC) names are DNS subdomains](https://releases.k8s.io/v1.23.0/pkg/apis/core/validation/validation.go#L2066).

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

Lines 32 to 33 in 899c4a0

	// +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?([.][a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`
	type DNS1123Subdomain = string

[cbandy]

🤔 Ah, hmm. Volume names must be unique in a pod. What happens If someone uses postgres-data for their volume name? Maybe we should prefix with volume-?

🤔 🤔 What other (Pod) validation happens on this name in K8s? Maybe a limit on the length?

postgres-operator/internal/postgres/reconcile.go

Lines 26 to 28 in 6a30d72

	func DataVolumeMount() corev1.VolumeMount {
	return corev1.VolumeMount{Name: "postgres-data", MountPath: dataMountPath}
	}

[benjaminjb]

For the pod.spec.volumes, its says "Must be a DNS_LABEL and unique within the pod." (cite)

That's what the code comment says, at least.

+1 for a prefix that prevents other problems

[cbandy]

❓ I'm getting a little mixed up with which name is where. Should this be additionalVolumeRequest.claimName ?

[benjaminjb]

This is a little confusing because I'm switching from using the derived Mount spec to our AdditionalVolume spec -- I think I can clean this up by (a) using the request name above, (b) creating the Mount spec with the ReadOnly field, and (c) sticking to the Mount spec here.

[benjaminjb]

Eh, tackling this and the comment re: naming the volumes, the code is less intuitive than I wanted

[cbandy]

👍🏻 I like that this takes a template.