Skip to content

Commit 8633669

Browse files
benjaminjbbenjaminjb
committed
PR feedback, add KUTTL test
1 parent f11c400 commit 8633669

File tree

12 files changed

+164
-31
lines changed

12 files changed

+164
-31
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11041,6 +11041,7 @@ spec:
1104111041
A blank/unset `Containers` field matches all containers.
1104211042
items:
1104311043
type: string
11044+
maxItems: 10
1104411045
type: array
1104511046
x-kubernetes-list-type: atomic
1104611047
name:
@@ -29649,6 +29650,7 @@ spec:
2964929650
A blank/unset `Containers` field matches all containers.
2965029651
items:
2965129652
type: string
29653+
maxItems: 10
2965229654
type: array
2965329655
x-kubernetes-list-type: atomic
2965429656
name:

‎internal/controller/postgrescluster/util.go

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@ import (
88
"fmt"
99
"hash/fnv"
1010
"io"
11-
"slices"
1211

1312
batchv1 "k8s.io/api/batch/v1"
1413
corev1 "k8s.io/api/core/v1"
1514
"k8s.io/apimachinery/pkg/api/resource"
1615
"k8s.io/apimachinery/pkg/util/rand"
16+
"k8s.io/apimachinery/pkg/util/sets"
1717

1818
"github.com/crunchydata/postgres-operator/internal/initialize"
1919
"github.com/crunchydata/postgres-operator/internal/naming"
@@ -298,7 +298,7 @@ func AdditionalVolumeMount(volumeName, claimName string) corev1.VolumeMount {
298298
// addAdditionalVolumesToSpecifiedContainers adds additional volumes to the specified
299299
// containers in the specified pod
300300
func addAdditionalVolumesToSpecifiedContainers(template *corev1.PodTemplateSpec,
301-
additionalVolumes []*v1beta1.AdditionalVolume) {
301+
additionalVolumes []v1beta1.AdditionalVolume) {
302302

303303
for _, additionalVolumeRequest := range additionalVolumes {
304304

@@ -318,22 +318,18 @@ func addAdditionalVolumesToSpecifiedContainers(template *corev1.PodTemplateSpec,
318318
},
319319
}
320320

321+
names := sets.New(additionalVolumeRequest.Containers...)
322+
321323
for i := range template.Spec.Containers {
322-
if len(additionalVolumeRequest.Containers) == 0 ||
323-
slices.Contains(
324-
additionalVolumeRequest.Containers,
325-
template.Spec.Containers[i].Name) {
324+
if names.Len() == 0 || names.Has(template.Spec.Containers[i].Name) {
326325
template.Spec.Containers[i].VolumeMounts = append(
327326
template.Spec.Containers[i].VolumeMounts,
328327
additionalVolumeMount)
329328
}
330329
}
331330

332331
for i := range template.Spec.InitContainers {
333-
if len(additionalVolumeRequest.Containers) == 0 ||
334-
slices.Contains(
335-
additionalVolumeRequest.Containers,
336-
template.Spec.InitContainers[i].Name) {
332+
if names.Len() == 0 || names.Has(template.Spec.InitContainers[i].Name) {
337333
template.Spec.InitContainers[i].VolumeMounts = append(
338334
template.Spec.InitContainers[i].VolumeMounts,
339335
additionalVolumeMount)

‎internal/controller/postgrescluster/util_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -395,13 +395,13 @@ func TestAddAdditionalVolumesToSpecifiedContainers(t *testing.T) {
395395

396396
testCases := []struct {
397397
tcName string
398-
additionalVolumes []*v1beta1.AdditionalVolume
398+
additionalVolumes []v1beta1.AdditionalVolume
399399
expectedContainers string
400400
expectedInitContainers string
401401
expectedVolumes string
402402
}{{
403403
tcName: "all",
404-
additionalVolumes: []*v1beta1.AdditionalVolume{{
404+
additionalVolumes: []v1beta1.AdditionalVolume{{
405405
Containers: []string{},
406406
ClaimName: "required",
407407
Name: "required",
@@ -431,7 +431,7 @@ func TestAddAdditionalVolumesToSpecifiedContainers(t *testing.T) {
431431
claimName: required`,
432432
}, {
433433
tcName: "multiple additional volumes",
434-
additionalVolumes: []*v1beta1.AdditionalVolume{{
434+
additionalVolumes: []v1beta1.AdditionalVolume{{
435435
Containers: []string{},
436436
ClaimName: "required",
437437
Name: "required",
@@ -476,7 +476,7 @@ func TestAddAdditionalVolumesToSpecifiedContainers(t *testing.T) {
476476
claimName: also`,
477477
}, {
478478
tcName: "database container only",
479-
additionalVolumes: []*v1beta1.AdditionalVolume{{
479+
additionalVolumes: []v1beta1.AdditionalVolume{{
480480
Containers: []string{"database"},
481481
ClaimName: "required",
482482
Name: "required",
@@ -497,7 +497,7 @@ func TestAddAdditionalVolumesToSpecifiedContainers(t *testing.T) {
497497
claimName: required`,
498498
}, {
499499
tcName: "readonly",
500-
additionalVolumes: []*v1beta1.AdditionalVolume{{
500+
additionalVolumes: []v1beta1.AdditionalVolume{{
501501
Containers: []string{"database"},
502502
ClaimName: "required",
503503
Name: "required",

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,7 @@ type PostgresVolumesSpec struct {
541541
// +listType=map
542542
// +listMapKey=name
543543
// +kubebuilder:validation:MaxItems=10
544-
Additional []*v1beta1.AdditionalVolume `json:"additional,omitempty"`
544+
Additional []v1beta1.AdditionalVolume `json:"additional,omitempty"`
545545
}
546546

547547
type TablespaceVolume struct {

‎pkg/apis/postgres-operator.crunchydata.com/v1/zz_generated.deepcopy.go

Lines changed: 2 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -554,15 +554,15 @@ type AdditionalVolume struct {
554554

555555
// A reference to a preexisting PVC.
556556
// ---
557-
// +kubebuilder:validation:Required
558-
ClaimName string `json:"claimName,omitempty"`
557+
// +required
558+
ClaimName string `json:"claimName"`
559559

560560
// The containers to attach this volume to.
561561
// A blank/unset `Containers` field matches all containers.
562562
// ---
563563
// +optional
564-
// +listType=set
565564
// +listType=atomic
565+
// +kubebuilder:validation:MaxItems=10
566566
Containers []string `json:"containers,omitempty"`
567567

568568
// Sets the write/read mode of the volume

‎pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go

Lines changed: 2 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎testing/kuttl/e2e/additional-volumes/00--cluster.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
apiVersion: kuttl.dev/v1beta1
2+
kind: TestStep
3+
apply:
4+
- files/00-create-cluster.yaml
5+
assert:
6+
- files/00-cluster-created.yaml

‎testing/kuttl/e2e/additional-volumes/00-assert.yaml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
apiVersion: kuttl.dev/v1beta1
2+
kind: TestAssert
3+
collectors:
4+
- type: command
5+
command: kubectl -n $NAMESPACE describe pods --selector postgres-operator.crunchydata.com/cluster=additional-vols
6+
- type: command
7+
command: kubectl -n $NAMESPACE describe pvcs --selector created=before
8+
- namespace: $NAMESPACE
9+
selector: postgres-operator.crunchydata.com/cluster=additional-vols

‎testing/kuttl/e2e/additional-volumes/01-assert.yaml

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
apiVersion: kuttl.dev/v1beta1
2+
kind: TestAssert
3+
commands:
4+
- script: |
5+
retry() { bash -ceu 'printf "$1\nSleeping...\n" && sleep 5' - "$@"; }
6+
check_containers_ready() { bash -ceu 'echo "$1" | jq -e ".[] | select(.type==\"ContainersReady\") | .status==\"True\""' - "$@"; }
7+
8+
pod=$(kubectl get pods -o name -n "${NAMESPACE}" \
9+
-l postgres-operator.crunchydata.com/cluster=additional-vols)
10+
[ "$pod" = "" ] && retry "Pod not found" && exit 1
11+
12+
condition_json=$(kubectl get "${pod}" -n "${NAMESPACE}" -o jsonpath="{.status.conditions}")
13+
[ "$condition_json" = "" ] && retry "conditions not found" && exit 1
14+
{ check_containers_ready "$condition_json"; } || {
15+
retry "containers not ready"
16+
exit 1
17+
}
18+
19+
extra_mounted_in_database=$(kubectl get "${pod}" -n "${NAMESPACE}" -o jsonpath="{.status.containerStatuses[?(@.name=='database')].volumeMounts[?(@.name=='additional-vol')]}")
20+
21+
[ "$extra_mounted_in_database" = "" ] && (echo "extra not found in database" && exit 1)
22+
23+
ubiq_mounted_in_database=$(kubectl get "${pod}" -n "${NAMESPACE}" -o jsonpath="{.status.containerStatuses[?(@.name=='database')].volumeMounts[?(@.name=='additional-vol-2')]}")
24+
25+
[ "$ubiq_mounted_in_database" = "" ] && (echo "ubiq not found in database" && exit 1)
26+
27+
extra_mounted_in_rcc=$(kubectl get "${pod}" -n "${NAMESPACE}" -o jsonpath="{.status.containerStatuses[?(@.name=='replication-cert-copy')].volumeMounts[?(@.name=='additional-vol')]}")
28+
29+
[ "$extra_mounted_in_rcc" = "" ] || (echo "extra found in rcc" && exit 1)
30+
31+
ubiq_mounted_in_rcc=$(kubectl get "${pod}" -n "${NAMESPACE}" -o jsonpath="{.status.containerStatuses[?(@.name=='replication-cert-copy')].volumeMounts[?(@.name=='additional-vol-2')]}")
32+
33+
[ "$ubiq_mounted_in_rcc" = "" ] && (echo "ubiq not found in rcc" && exit 1)
34+
35+
extra_mounted_in_nss=$(kubectl get "${pod}" -n "${NAMESPACE}" -o jsonpath="{.status.initContainerStatuses[?(@.name=='nss-wrapper-init')].volumeMounts[?(@.name=='additional-vol')]}")
36+
37+
[ "$extra_mounted_in_nss" = "" ] && (echo "extra not found in nss" && exit 1)
38+
39+
ubiq_mounted_in_nss=$(kubectl get "${pod}" -n "${NAMESPACE}" -o jsonpath="{.status.initContainerStatuses[?(@.name=='nss-wrapper-init')].volumeMounts[?(@.name=='additional-vol-2')]}")
40+
41+
[ "$ubiq_mounted_in_nss" = "" ] && (echo "ubiq not found in nss" && exit 1)
42+
43+
extra_mounted_in_startup=$(kubectl get "${pod}" -n "${NAMESPACE}" -o jsonpath="{.status.initContainerStatuses[?(@.name=='postgres-startup')].volumeMounts[?(@.name=='additional-vol')]}")
44+
45+
[ "$extra_mounted_in_startup" = "" ] || (echo "extra found in startup" && exit 1)
46+
47+
ubiq_mounted_in_startup=$(kubectl get "${pod}" -n "${NAMESPACE}" -o jsonpath="{.status.initContainerStatuses[?(@.name=='postgres-startup')].volumeMounts[?(@.name=='additional-vol-2')]}")
48+
49+
[ "$ubiq_mounted_in_startup" = "" ] && (echo "ubiq not found in startup" && exit 1)
50+
51+
extra_isnt_readonly=$(kubectl get "${pod}" -n "${NAMESPACE}" -o jsonpath="{.spec.volumes[?(@.name=='additional-vol')].persistentVolumeClaim.readOnly}")
52+
53+
[ "$extra_isnt_readonly" = "" ] || (echo "extra is readonly" && exit 1)
54+
55+
ubiq_is_readonly=$(kubectl get "${pod}" -n "${NAMESPACE}" -o jsonpath="{.spec.volumes[?(@.name=='additional-vol-2')].persistentVolumeClaim.readOnly}")
56+
57+
[ "$ubiq_is_readonly" = "" ] && (echo "ubiq is not readonly" && exit 1)
58+
59+
echo "clean bill of health"
60+
61+
collectors:
62+
- type: command
63+
command: kubectl -n $NAMESPACE describe pods --selector postgres-operator.crunchydata.com/cluster=additional-vols
64+
- namespace: $NAMESPACE
65+
selector: postgres-operator.crunchydata.com/cluster=additional-vols

0 commit comments

Comments
 (0)