Skip to content

Commit c8a6ba2

Browse files
committed
Add ImageVolumeSource to AdditionalVolumes.
Add validation to ensure that user must choose between an image volume source and a pvc claim. Add validation to ensure that user cannot set readOnly to false when using an image volume source. Add validation to ensure that user sets a reference when using an image volume source. Add tests for using ImageVolumeSource in AdditionalVolume feature.
1 parent 313f2a7 commit c8a6ba2

File tree

8 files changed

+740
-18
lines changed

8 files changed

+740
-18
lines changed

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

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2620,6 +2620,27 @@ spec:
26202620
maxItems: 10
26212621
type: array
26222622
x-kubernetes-list-type: set
2623+
image:
2624+
description: Details for adding an image volume
2625+
properties:
2626+
pullPolicy:
2627+
description: |-
2628+
Policy for pulling OCI objects. Possible values are:
2629+
Always: the kubelet always attempts to pull the reference. Container creation will fail If the pull fails.
2630+
Never: the kubelet never pulls the reference and only uses a local image or artifact. Container creation will fail if the reference isn't present.
2631+
IfNotPresent: the kubelet pulls if the reference isn't already present on disk. Container creation will fail if the reference isn't present and the pull fails.
2632+
Defaults to Always if :latest tag is specified, or IfNotPresent otherwise.
2633+
type: string
2634+
reference:
2635+
description: |-
2636+
Required: Image or artifact reference to be used.
2637+
Behaves in the same way as pod.spec.containers[*].image.
2638+
Pull secrets will be assembled in the same way as for the container image by looking up node credentials, SA image pull secrets, and pod spec image pull secrets.
2639+
More info: https://kubernetes.io/docs/concepts/containers/images
2640+
This field is optional to allow higher level config management to default or override
2641+
container images in workload controllers like Deployments and StatefulSets.
2642+
type: string
2643+
type: object
26232644
name:
26242645
description: |-
26252646
The name of the directory in which to mount this volume.
@@ -2633,10 +2654,17 @@ spec:
26332654
read-write. Defaults to false.
26342655
type: boolean
26352656
required:
2636-
- claimName
26372657
- name
26382658
type: object
26392659
x-kubernetes-map-type: atomic
2660+
x-kubernetes-validations:
2661+
- message: you must set only one of image or claimName
2662+
rule: has(self.claimName) != has(self.image)
2663+
- message: readOnly cannot be set false when using an ImageVolumeSource
2664+
rule: '!has(self.image) || !has(self.readOnly) || self.readOnly'
2665+
- message: if using an ImageVolumeSource, you must set a reference
2666+
rule: '!has(self.image) || (self.?image.reference.hasValue()
2667+
&& self.image.reference.size() > 0)'
26402668
maxItems: 10
26412669
type: array
26422670
x-kubernetes-list-map-keys:

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

Lines changed: 446 additions & 14 deletions
Large diffs are not rendered by default.

internal/testing/validation/postgrescluster_test.go

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ import (
99
"testing"
1010

1111
"gotest.tools/v3/assert"
12+
apierrors "k8s.io/apimachinery/pkg/api/errors"
13+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1214
"sigs.k8s.io/controller-runtime/pkg/client"
15+
"sigs.k8s.io/yaml"
1316

1417
"github.com/crunchydata/postgres-operator/internal/testing/require"
1518
v1 "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1"
@@ -103,3 +106,165 @@ func TestPostgresUserInterfaceAcrossVersions(t *testing.T) {
103106
"userInterface not available in v1")
104107
})
105108
}
109+
110+
func TestAdditionalVolumes(t *testing.T) {
111+
ctx := context.Background()
112+
cc := require.KubernetesAtLeast(t, "1.30")
113+
t.Parallel()
114+
115+
namespace := require.Namespace(t, cc)
116+
base := v1beta1.NewPostgresCluster()
117+
118+
base.Namespace = namespace.Name
119+
base.Name = "image-volume-source-test"
120+
// required fields
121+
require.UnmarshalInto(t, &base.Spec, `{
122+
postgresVersion: 16,
123+
instances: [{
124+
dataVolumeClaimSpec: {
125+
accessModes: [ReadWriteOnce],
126+
resources: { requests: { storage: 1Mi } },
127+
},
128+
}],
129+
}`)
130+
131+
assert.NilError(t, cc.Create(ctx, base.DeepCopy(), client.DryRunAll),
132+
"expected this base to be valid")
133+
134+
var unstructuredBase unstructured.Unstructured
135+
require.UnmarshalInto(t, &unstructuredBase, require.Value(yaml.Marshal(base)))
136+
137+
t.Run("Cannot set both image and claimName", func(t *testing.T) {
138+
tmp := unstructuredBase.DeepCopy()
139+
140+
require.UnmarshalIntoField(t, tmp, `[{
141+
dataVolumeClaimSpec: {
142+
accessModes: [ReadWriteOnce],
143+
resources: { requests: { storage: 1Mi } },
144+
},
145+
volumes: {
146+
additional: [{
147+
name: "test",
148+
claimName: "pvc-claim",
149+
image: {
150+
reference: "test-image",
151+
pullPolicy: Always
152+
},
153+
readOnly: true
154+
}]
155+
}
156+
}]`, "spec", "instances")
157+
err := cc.Create(ctx, tmp.DeepCopy(), client.DryRunAll)
158+
assert.Assert(t, apierrors.IsInvalid(err))
159+
assert.ErrorContains(t, err, "you must set only one of image or claimName")
160+
})
161+
162+
t.Run("Cannot set readOnly to false when using image volume", func(t *testing.T) {
163+
tmp := unstructuredBase.DeepCopy()
164+
165+
require.UnmarshalIntoField(t, tmp, `[{
166+
dataVolumeClaimSpec: {
167+
accessModes: [ReadWriteOnce],
168+
resources: { requests: { storage: 1Mi } },
169+
},
170+
volumes: {
171+
additional: [{
172+
name: "test",
173+
image: {
174+
reference: "test-image",
175+
pullPolicy: Always
176+
},
177+
readOnly: false
178+
}]
179+
}
180+
}]`, "spec", "instances")
181+
err := cc.Create(ctx, tmp.DeepCopy(), client.DryRunAll)
182+
assert.Assert(t, apierrors.IsInvalid(err))
183+
assert.ErrorContains(t, err, "readOnly cannot be set false when using an ImageVolumeSource")
184+
})
185+
186+
t.Run("Reference must be set when using image volume", func(t *testing.T) {
187+
tmp := unstructuredBase.DeepCopy()
188+
189+
require.UnmarshalIntoField(t, tmp, `[{
190+
dataVolumeClaimSpec: {
191+
accessModes: [ReadWriteOnce],
192+
resources: { requests: { storage: 1Mi } },
193+
},
194+
volumes: {
195+
additional: [{
196+
name: "test",
197+
image: {
198+
pullPolicy: Always
199+
},
200+
readOnly: true
201+
}]
202+
}
203+
}]`, "spec", "instances")
204+
err := cc.Create(ctx, tmp.DeepCopy(), client.DryRunAll)
205+
assert.Assert(t, apierrors.IsInvalid(err))
206+
assert.ErrorContains(t, err, "if using an ImageVolumeSource, you must set a reference")
207+
})
208+
209+
t.Run("Reference cannot be an empty string when using image volume", func(t *testing.T) {
210+
tmp := unstructuredBase.DeepCopy()
211+
212+
require.UnmarshalIntoField(t, tmp, `[{
213+
dataVolumeClaimSpec: {
214+
accessModes: [ReadWriteOnce],
215+
resources: { requests: { storage: 1Mi } },
216+
},
217+
volumes: {
218+
additional: [{
219+
name: "test",
220+
image: {
221+
reference: "",
222+
pullPolicy: Always
223+
},
224+
readOnly: true
225+
}]
226+
}
227+
}]`, "spec", "instances")
228+
err := cc.Create(ctx, tmp.DeepCopy(), client.DryRunAll)
229+
assert.Assert(t, apierrors.IsInvalid(err))
230+
assert.ErrorContains(t, err, "if using an ImageVolumeSource, you must set a reference")
231+
})
232+
233+
t.Run("ReadOnly can be omitted or set true when using image volume", func(t *testing.T) {
234+
tmp := unstructuredBase.DeepCopy()
235+
236+
require.UnmarshalIntoField(t, tmp, `[{
237+
name: "test-instance",
238+
dataVolumeClaimSpec: {
239+
accessModes: [ReadWriteOnce],
240+
resources: { requests: { storage: 1Mi } },
241+
},
242+
volumes: {
243+
additional: [{
244+
name: "test",
245+
image: {
246+
reference: "test-image",
247+
pullPolicy: Always
248+
},
249+
}]
250+
}
251+
}, {
252+
name: "another-instance",
253+
dataVolumeClaimSpec: {
254+
accessModes: [ReadWriteOnce],
255+
resources: { requests: { storage: 1Mi } },
256+
},
257+
volumes: {
258+
additional: [{
259+
name: "another",
260+
image: {
261+
reference: "another-image",
262+
pullPolicy: Always
263+
},
264+
readOnly: true
265+
}]
266+
}
267+
}]`, "spec", "instances")
268+
assert.NilError(t, cc.Create(ctx, tmp.DeepCopy(), client.DryRunAll))
269+
})
270+
}

internal/util/volumes.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,13 @@ func addVolumesAndMounts(pod *corev1.PodSpec, volumes []v1beta1.AdditionalVolume
5858
missingContainers := []string{}
5959

6060
for _, spec := range volumes {
61-
mount := namer(spec.Name, spec.ReadOnly)
61+
// If it is an image volume, override readOnly to true
62+
readOnly := spec.ReadOnly
63+
if spec.Image != nil {
64+
readOnly = true
65+
}
66+
67+
mount := namer(spec.Name, readOnly)
6268
pod.Volumes = append(pod.Volumes, spec.AsVolume(mount.Name))
6369

6470
// Create a set of all the requested containers,

internal/util/volumes_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,60 @@ func TestAddAdditionalVolumesAndMounts(t *testing.T) {
207207
claimName: required
208208
readOnly: true`,
209209
expectedMissing: []string{},
210+
}, {
211+
tcName: "image volumes - readOnly overridden true",
212+
additionalVolumes: []v1beta1.AdditionalVolume{{
213+
Containers: []string{"database"},
214+
Image: &corev1.ImageVolumeSource{
215+
Reference: "some-image-name",
216+
PullPolicy: corev1.PullAlways,
217+
},
218+
Name: "required",
219+
ReadOnly: true,
220+
}, {
221+
Image: &corev1.ImageVolumeSource{
222+
Reference: "another-image-name",
223+
PullPolicy: corev1.PullAlways,
224+
},
225+
Name: "other",
226+
ReadOnly: false,
227+
}},
228+
expectedContainers: `- name: database
229+
resources: {}
230+
volumeMounts:
231+
- mountPath: /volumes/required
232+
name: volumes-required
233+
readOnly: true
234+
- mountPath: /volumes/other
235+
name: volumes-other
236+
readOnly: true
237+
- name: other
238+
resources: {}
239+
volumeMounts:
240+
- mountPath: /volumes/other
241+
name: volumes-other
242+
readOnly: true`,
243+
expectedInitContainers: `- name: startup
244+
resources: {}
245+
volumeMounts:
246+
- mountPath: /volumes/other
247+
name: volumes-other
248+
readOnly: true
249+
- name: config
250+
resources: {}
251+
volumeMounts:
252+
- mountPath: /volumes/other
253+
name: volumes-other
254+
readOnly: true`,
255+
expectedVolumes: `- image:
256+
pullPolicy: Always
257+
reference: some-image-name
258+
name: volumes-required
259+
- image:
260+
pullPolicy: Always
261+
reference: another-image-name
262+
name: volumes-other`,
263+
expectedMissing: []string{},
210264
}}
211265

212266
for _, tc := range testCases {

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

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -311,14 +311,18 @@ func (meta *Metadata) GetAnnotationsOrNil() map[string]string {
311311
// Only one applier should be managing each volume definition.
312312
// https://docs.k8s.io/reference/using-api/server-side-apply#merge-strategy
313313
// +structType=atomic
314+
//
315+
// +kubebuilder:validation:XValidation:rule=`has(self.claimName) != has(self.image)`,message=`you must set only one of image or claimName`
316+
// +kubebuilder:validation:XValidation:rule=`!has(self.image) || !has(self.readOnly) || self.readOnly`,message=`readOnly cannot be set false when using an ImageVolumeSource`
317+
// +kubebuilder:validation:XValidation:rule=`!has(self.image) || (self.?image.reference.hasValue() && self.image.reference.size() > 0)`,message=`if using an ImageVolumeSource, you must set a reference`
314318
type AdditionalVolume struct {
315319
// Name of an existing PersistentVolumeClaim.
316320
// ---
317321
// https://pkg.go.dev/k8s.io/kubernetes/pkg/apis/core/validation#ValidatePersistentVolumeClaim
318322
// https://pkg.go.dev/k8s.io/kubernetes/pkg/apis/core/validation#ValidatePersistentVolumeName
319323
//
320-
// +required
321-
ClaimName DNS1123Subdomain `json:"claimName"`
324+
// +optional
325+
ClaimName DNS1123Subdomain `json:"claimName,omitempty"`
322326

323327
// The names of containers in which to mount this volume.
324328
// The default mounts the volume in *all* containers. An empty list does not mount the volume to any containers.
@@ -333,6 +337,13 @@ type AdditionalVolume struct {
333337
// +optional
334338
Containers []DNS1123Label `json:"containers"`
335339

340+
// Details for adding an image volume
341+
// ---
342+
// https://docs.k8s.io/concepts/storage/volumes#image
343+
//
344+
// +optional
345+
Image *corev1.ImageVolumeSource `json:"image,omitempty"`
346+
336347
// The name of the directory in which to mount this volume.
337348
// Volumes are mounted in containers at `/volumes/{name}`.
338349
// ---
@@ -366,6 +377,8 @@ func (in *AdditionalVolume) AsVolume(name string) corev1.Volume {
366377
ClaimName: in.ClaimName,
367378
ReadOnly: in.ReadOnly,
368379
}
380+
case in.Image != nil:
381+
out.Image = in.Image.DeepCopy()
369382
}
370383

371384
return out

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,25 @@ func TestAdditionalVolumeAsVolume(t *testing.T) {
4545
assert.DeepEqual(t, out, expected)
4646
})
4747
})
48+
49+
t.Run("Image", func(t *testing.T) {
50+
in := v1beta1.AdditionalVolume{Image: &corev1.ImageVolumeSource{
51+
Reference: "jkl;",
52+
PullPolicy: corev1.PullAlways,
53+
}}
54+
out := in.AsVolume("asdf")
55+
56+
var expected corev1.Volume
57+
assert.NilError(t, yaml.Unmarshal([]byte(`{
58+
name: asdf,
59+
image: {
60+
reference: jkl;,
61+
pullPolicy: Always,
62+
},
63+
}`), &expected))
64+
65+
assert.DeepEqual(t, out, expected)
66+
})
4867
}
4968

5069
func TestDurationAsDuration(t *testing.T) {

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

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

0 commit comments

Comments
 (0)