Add volume selector (zalando#1385)

jcrood · John Rood · web-flow · commit 2d2ce6197b89 · 2021-08-26T14:57:54.000+02:00
* Add volume selector
* Add slightly better documentation and gofmt changes
* Update generated deepcopy
* Add test for PV selector

Co-authored-by: John Rood &lt;j.rood@picturae.com&gt;
diff --git a/charts/postgres-operator/crds/postgresqls.yaml b/charts/postgres-operator/crds/postgresqls.yaml
@@ -561,6 +561,24 @@ spec:
                 properties:
                   iops:
                     type: integer
+                  selector:
+                    type: object
+                    properties:
+                      matchExpressions:
+                        type: array
+                        items:
+                          type: object
+                          properties:
+                            key:
+                              type: string
+                            operator:
+                              type: string
+                            values:
+                              type: array
+                              items:
+                                type: string
+                      matchLabels:
+                        type: object
                   size:
                     type: string
                     pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$'
diff --git a/docs/reference/cluster_manifest.md b/docs/reference/cluster_manifest.md
@@ -399,6 +399,11 @@ properties of the persistent storage that stores Postgres data.
   When running the operator on AWS the latest generation of EBS volumes (`gp3`)
   allows for configuring the throughput in MB/s. Maximum is 1000. Optional.
 
+* **selector**
+  A label query over PVs to consider for binding. See the [Kubernetes 
+  documentation](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/)
+  for details on using `matchLabels` and `matchExpressions`. Optional
+
 ## Sidecar definitions
 
 Those parameters are defined under the `sidecars` key. They consist of a list
diff --git a/manifests/complete-postgres-manifest.yaml b/manifests/complete-postgres-manifest.yaml
@@ -46,6 +46,12 @@ spec:
 #    storageClass: my-sc
 #    iops: 1000  # for EBS gp3
 #    throughput: 250  # in MB/s for EBS gp3
+#    selector:
+#      matchExpressions:
+#        - { key: flavour, operator: In, values: [ "banana", "chocolate" ] }
+#      matchLabels:
+#        environment: dev
+#        service: postgres
   additionalVolumes:
     - name: empty
       mountPath: /opt/empty
diff --git a/manifests/postgresql.crd.yaml b/manifests/postgresql.crd.yaml
@@ -557,6 +557,24 @@ spec:
                 properties:
                   iops:
                     type: integer
+                  selector:
+                    type: object
+                    properties:
+                      matchExpressions:
+                        type: array
+                        items:
+                          type: object
+                          properties:
+                            key:
+                              type: string
+                            operator:
+                              type: string
+                            values:
+                              type: array
+                              items:
+                                type: string
+                      matchLabels:
+                        type: object
                   size:
                     type: string
                     pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$'
diff --git a/pkg/apis/acid.zalan.do/v1/crds.go b/pkg/apis/acid.zalan.do/v1/crds.go
@@ -841,6 +841,54 @@ var PostgresCRDResourceValidation = apiextv1.CustomResourceValidation{
 							"iops": {
 								Type: "integer",
 							},
+							"selector": {
+								Type: "object",
+								Properties: map[string]apiextv1.JSONSchemaProps{
+									"matchExpressions": {
+										Type: "array",
+										Items: &apiextv1.JSONSchemaPropsOrArray{
+											Schema: &apiextv1.JSONSchemaProps{
+												Type:     "object",
+												Required: []string{"key", "operator", "values"},
+												Properties: map[string]apiextv1.JSONSchemaProps{
+													"key": {
+														Type: "string",
+													},
+													"operator": {
+														Type: "string",
+														Enum: []apiextv1.JSON{
+															{
+																Raw: []byte(`"In"`),
+															},
+															{
+																Raw: []byte(`"NotIn"`),
+															},
+															{
+																Raw: []byte(`"Exists"`),
+															},
+															{
+																Raw: []byte(`"DoesNotExist"`),
+															},
+														},
+													},
+													"values": {
+														Type: "array",
+														Items: &apiextv1.JSONSchemaPropsOrArray{
+															Schema: &apiextv1.JSONSchemaProps{
+																Type: "string",
+															},
+														},
+													},
+												},
+											},
+										},
+									},
+									"matchLabels": {
+										Type:                   "object",
+										XPreserveUnknownFields: util.True(),
+									},
+								},
+							},
 							"size": {
 								Type:        "string",
 								Description: "Value must not be zero",
diff --git a/pkg/apis/acid.zalan.do/v1/postgresql_type.go b/pkg/apis/acid.zalan.do/v1/postgresql_type.go
@@ -114,12 +114,13 @@ type MaintenanceWindow struct {
 
 // Volume describes a single volume in the manifest.
 type Volume struct {
-	Size         string `json:"size"`
-	StorageClass string `json:"storageClass,omitempty"`
-	SubPath      string `json:"subPath,omitempty"`
-	Iops         *int64 `json:"iops,omitempty"`
-	Throughput   *int64 `json:"throughput,omitempty"`
-	VolumeType   string `json:"type,omitempty"`
+	Selector     *metav1.LabelSelector `json:"selector,omitempty"`
+	Size         string                `json:"size"`
+	StorageClass string                `json:"storageClass,omitempty"`
+	SubPath      string                `json:"subPath,omitempty"`
+	Iops         *int64                `json:"iops,omitempty"`
+	Throughput   *int64                `json:"throughput,omitempty"`
+	VolumeType   string                `json:"type,omitempty"`
 }
 
 // AdditionalVolume specs additional optional volumes for statefulset
diff --git a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go
diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go
@@ -1272,7 +1272,7 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef
 	}
 
 	if volumeClaimTemplate, err = generatePersistentVolumeClaimTemplate(spec.Volume.Size,
-		spec.Volume.StorageClass); err != nil {
+		spec.Volume.StorageClass, spec.Volume.Selector); err != nil {
 		return nil, fmt.Errorf("could not generate volume claim template: %v", err)
 	}
 
@@ -1520,7 +1520,8 @@ func (c *Cluster) addAdditionalVolumes(podSpec *v1.PodSpec,
 	podSpec.Volumes = volumes
 }
 
-func generatePersistentVolumeClaimTemplate(volumeSize, volumeStorageClass string) (*v1.PersistentVolumeClaim, error) {
+func generatePersistentVolumeClaimTemplate(volumeSize, volumeStorageClass string,
+	volumeSelector *metav1.LabelSelector) (*v1.PersistentVolumeClaim, error) {
 
 	var storageClassName *string
 
@@ -1553,6 +1554,7 @@ func generatePersistentVolumeClaimTemplate(volumeSize, volumeStorageClass string
 			},
 			StorageClassName: storageClassName,
 			VolumeMode:       &volumeMode,
+			Selector:         volumeSelector,
 		},
 	}
 
diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go
@@ -1509,3 +1509,106 @@ func TestGenerateCapabilities(t *testing.T) {
 		}
 	}
 }
+
+func TestVolumeSelector(t *testing.T) {
+	testName := "TestVolumeSelector"
+	makeSpec := func(volume acidv1.Volume) acidv1.PostgresSpec {
+		return acidv1.PostgresSpec{
+			TeamID:            "myapp",
+			NumberOfInstances: 0,
+			Resources: acidv1.Resources{
+				ResourceRequests: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
+				ResourceLimits:   acidv1.ResourceDescription{CPU: "1", Memory: "10"},
+			},
+			Volume: volume,
+		}
+	}
+
+	tests := []struct {
+		subTest      string
+		volume       acidv1.Volume
+		wantSelector *metav1.LabelSelector
+	}{
+		{
+			subTest: "PVC template has no selector",
+			volume: acidv1.Volume{
+				Size: "1G",
+			},
+			wantSelector: nil,
+		},
+		{
+			subTest: "PVC template has simple label selector",
+			volume: acidv1.Volume{
+				Size: "1G",
+				Selector: &metav1.LabelSelector{
+					MatchLabels: map[string]string{"environment": "unittest"},
+				},
+			},
+			wantSelector: &metav1.LabelSelector{
+				MatchLabels: map[string]string{"environment": "unittest"},
+			},
+		},
+		{
+			subTest: "PVC template has full selector",
+			volume: acidv1.Volume{
+				Size: "1G",
+				Selector: &metav1.LabelSelector{
+					MatchLabels: map[string]string{"environment": "unittest"},
+					MatchExpressions: []metav1.LabelSelectorRequirement{
+						{
+							Key:      "flavour",
+							Operator: metav1.LabelSelectorOpIn,
+							Values:   []string{"banana", "chocolate"},
+						},
+					},
+				},
+			},
+			wantSelector: &metav1.LabelSelector{
+				MatchLabels: map[string]string{"environment": "unittest"},
+				MatchExpressions: []metav1.LabelSelectorRequirement{
+					{
+						Key:      "flavour",
+						Operator: metav1.LabelSelectorOpIn,
+						Values:   []string{"banana", "chocolate"},
+					},
+				},
+			},
+		},
+	}
+
+	cluster := New(
+		Config{
+			OpConfig: config.Config{
+				PodManagementPolicy: "ordered_ready",
+				ProtectedRoles:      []string{"admin"},
+				Auth: config.Auth{
+					SuperUsername:       superUserName,
+					ReplicationUsername: replicationUserName,
+				},
+			},
+		}, k8sutil.KubernetesClient{}, acidv1.Postgresql{}, logger, eventRecorder)
+
+	for _, tt := range tests {
+		pgSpec := makeSpec(tt.volume)
+		sts, err := cluster.generateStatefulSet(&pgSpec)
+		if err != nil {
+			t.Fatalf("%s %s: no statefulset created %v", testName, tt.subTest, err)
+		}
+
+		volIdx := len(sts.Spec.VolumeClaimTemplates)
+		for i, ct := range sts.Spec.VolumeClaimTemplates {
+			if ct.ObjectMeta.Name == constants.DataVolumeName {
+				volIdx = i
+				break
+			}
+		}
+		if volIdx == len(sts.Spec.VolumeClaimTemplates) {
+			t.Errorf("%s %s: no datavolume found in sts", testName, tt.subTest)
+		}
+
+		selector := sts.Spec.VolumeClaimTemplates[volIdx].Spec.Selector
+		if !reflect.DeepEqual(selector, tt.wantSelector) {
+			t.Errorf("%s %s: expected: %#v but got: %#v", testName, tt.subTest, tt.wantSelector, selector)
+		}
+	}
+}

Original file line number	Diff line number	Diff line change
`@@ -1272,7 +1272,7 @@ func (c Cluster) generateStatefulSet(spec acidv1.PostgresSpec) (*appsv1.Statef`
`1272`	`1272`	`}`
`1273`	`1273`
`1274`	`1274`	`if volumeClaimTemplate, err = generatePersistentVolumeClaimTemplate(spec.Volume.Size,`
`1275`		`- spec.Volume.StorageClass); err != nil {`
	`1275`	`+ spec.Volume.StorageClass, spec.Volume.Selector); err != nil {`
`1276`	`1276`	`return nil, fmt.Errorf("could not generate volume claim template: %v", err)`
`1277`	`1277`	`}`
`1278`	`1278`
`@@ -1520,7 +1520,8 @@ func (c Cluster) addAdditionalVolumes(podSpec v1.PodSpec,`
`1520`	`1520`	`podSpec.Volumes = volumes`
`1521`	`1521`	`}`
`1522`	`1522`
`1523`		`-func generatePersistentVolumeClaimTemplate(volumeSize, volumeStorageClass string) (*v1.PersistentVolumeClaim, error) {`
	`1523`	`+func generatePersistentVolumeClaimTemplate(volumeSize, volumeStorageClass string,`
	`1524`	`+ volumeSelector metav1.LabelSelector) (v1.PersistentVolumeClaim, error) {`
`1524`	`1525`
`1525`	`1526`	`var storageClassName *string`
`1526`	`1527`
`@@ -1553,6 +1554,7 @@ func generatePersistentVolumeClaimTemplate(volumeSize, volumeStorageClass string`
`1553`	`1554`	`},`
`1554`	`1555`	`StorageClassName: storageClassName,`
`1555`	`1556`	`VolumeMode: &volumeMode,`
	`1557`	`+ Selector: volumeSelector,`
`1556`	`1558`	`},`
`1557`	`1559`	`}`
`1558`	`1560`