From b88b7cea1389509fb721cebce2c7ae84e89e4c3d Mon Sep 17 00:00:00 2001 From: Chris Bandy Date: Mon, 3 Mar 2025 13:22:31 -0600 Subject: [PATCH] Define a struct to share validation rules for PVC specs This adds some validation to the PGAdmin data volume spec. Tests show we can simplify these validation rules, which may help keep estimated validation costs low. --- ...res-operator.crunchydata.com_pgadmins.yaml | 6 ++ ...ator.crunchydata.com_postgresclusters.yaml | 39 ++++---- .../postgrescluster/helpers_test.go | 4 +- .../postgrescluster/instance_test.go | 8 +- .../controller/postgrescluster/pgadmin.go | 2 +- .../postgrescluster/pgadmin_test.go | 4 +- .../controller/postgrescluster/pgbackrest.go | 3 +- .../postgrescluster/pgbackrest_test.go | 8 +- .../controller/postgrescluster/postgres.go | 6 +- .../postgrescluster/postgres_test.go | 20 ++--- .../controller/postgrescluster/snapshots.go | 2 +- .../postgrescluster/snapshots_test.go | 3 +- .../controller/postgrescluster/volumes.go | 6 +- .../postgrescluster/volumes_test.go | 14 +-- .../standalone_pgadmin/configmap_test.go | 3 +- .../standalone_pgadmin/controller_test.go | 6 ++ .../standalone_pgadmin/helpers_unit_test.go | 76 ---------------- .../standalone_pgadmin/related_test.go | 69 ++++++++------- .../standalone_pgadmin/statefulset_test.go | 12 +++ .../standalone_pgadmin/users_test.go | 6 ++ .../controller/standalone_pgadmin/volume.go | 2 +- .../standalone_pgadmin/volume_test.go | 23 ++--- internal/pgbackrest/pgbackrest_test.go | 2 +- internal/postgres/config_test.go | 3 +- internal/postgres/reconcile_test.go | 2 +- internal/testing/validation/pgadmin_test.go | 88 +++++++++++++++++++ .../v1beta1/pgadmin_types.go | 5 +- .../v1beta1/pgbackrest_types.go | 16 +--- .../v1beta1/postgrescluster_types.go | 48 ++-------- .../v1beta1/shared_types.go | 30 ++++++- .../v1beta1/shared_types_test.go | 31 +++++++ .../v1beta1/standalone_pgadmin_types.go | 5 +- .../v1beta1/zz_generated.deepcopy.go | 13 ++- 33 files changed, 316 insertions(+), 249 deletions(-) delete mode 100644 internal/controller/standalone_pgadmin/helpers_unit_test.go diff --git a/config/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml b/config/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml index 1d3f1635a8..cf290b0ec6 100644 --- a/config/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml +++ b/config/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml @@ -1560,6 +1560,12 @@ spec: backing this claim. type: string type: object + x-kubernetes-map-type: atomic + x-kubernetes-validations: + - message: missing accessModes + rule: 0 < size(self.accessModes) + - message: missing storage request + rule: has(self.resources.requests.storage) image: description: The image name to use for pgAdmin instance. type: string diff --git a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml index 8b8bfee823..26e1d31154 100644 --- a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml +++ b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml @@ -3197,13 +3197,12 @@ spec: to the PersistentVolume backing this claim. type: string type: object + x-kubernetes-map-type: atomic x-kubernetes-validations: - message: missing accessModes - rule: has(self.accessModes) && size(self.accessModes) - > 0 + rule: 0 < size(self.accessModes) - message: missing storage request - rule: has(self.resources) && has(self.resources.requests) - && has(self.resources.requests.storage) + rule: has(self.resources.requests.storage) required: - volumeClaimSpec type: object @@ -6524,13 +6523,12 @@ spec: to the PersistentVolume backing this claim. type: string type: object + x-kubernetes-map-type: atomic x-kubernetes-validations: - message: missing accessModes - rule: has(self.accessModes) && size(self.accessModes) - > 0 + rule: 0 < size(self.accessModes) - message: missing storage request - rule: has(self.resources) && has(self.resources.requests) - && has(self.resources.requests.storage) + rule: has(self.resources.requests.storage) required: - volumeClaimSpec type: object @@ -10412,12 +10410,12 @@ spec: PersistentVolume backing this claim. type: string type: object + x-kubernetes-map-type: atomic x-kubernetes-validations: - message: missing accessModes - rule: has(self.accessModes) && size(self.accessModes) > 0 + rule: 0 < size(self.accessModes) - message: missing storage request - rule: has(self.resources) && has(self.resources.requests) - && has(self.resources.requests.storage) + rule: has(self.resources.requests.storage) metadata: description: Metadata contains metadata for custom resources properties: @@ -10797,13 +10795,12 @@ spec: the PersistentVolume backing this claim. type: string type: object + x-kubernetes-map-type: atomic x-kubernetes-validations: - message: missing accessModes - rule: has(self.accessModes) && size(self.accessModes) - > 0 + rule: 0 < size(self.accessModes) - message: missing storage request - rule: has(self.resources) && has(self.resources.requests) - && has(self.resources.requests.storage) + rule: has(self.resources.requests.storage) name: description: |- The name for the tablespace, used as the path name for the volume. @@ -11238,12 +11235,12 @@ spec: PersistentVolume backing this claim. type: string type: object + x-kubernetes-map-type: atomic x-kubernetes-validations: - message: missing accessModes - rule: has(self.accessModes) && size(self.accessModes) > 0 + rule: 0 < size(self.accessModes) - message: missing storage request - rule: has(self.resources) && has(self.resources.requests) - && has(self.resources.requests.storage) + rule: has(self.resources.requests.storage) required: - dataVolumeClaimSpec type: object @@ -17328,6 +17325,12 @@ spec: PersistentVolume backing this claim. type: string type: object + x-kubernetes-map-type: atomic + x-kubernetes-validations: + - message: missing accessModes + rule: 0 < size(self.accessModes) + - message: missing storage request + rule: has(self.resources.requests.storage) image: description: |- Name of a container image that can run pgAdmin 4. Changing this value causes diff --git a/internal/controller/postgrescluster/helpers_test.go b/internal/controller/postgrescluster/helpers_test.go index e6709151b4..4542f651a9 100644 --- a/internal/controller/postgrescluster/helpers_test.go +++ b/internal/controller/postgrescluster/helpers_test.go @@ -90,9 +90,9 @@ func setupNamespace(t testing.TB, cc client.Client) *corev1.Namespace { return require.Namespace(t, cc) } -func testVolumeClaimSpec() corev1.PersistentVolumeClaimSpec { +func testVolumeClaimSpec() v1beta1.VolumeClaimSpec { // Defines a volume claim spec that can be used to create instances - return corev1.PersistentVolumeClaimSpec{ + return v1beta1.VolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, Resources: corev1.VolumeResourceRequirements{ Requests: map[corev1.ResourceName]resource.Quantity{ diff --git a/internal/controller/postgrescluster/instance_test.go b/internal/controller/postgrescluster/instance_test.go index 507fa69b85..2381b4cb5b 100644 --- a/internal/controller/postgrescluster/instance_test.go +++ b/internal/controller/postgrescluster/instance_test.go @@ -280,7 +280,7 @@ func TestStoreDesiredRequest(t *testing.T) { InstanceSets: []v1beta1.PostgresInstanceSetSpec{{ Name: "red", Replicas: initialize.Int32(1), - DataVolumeClaimSpec: corev1.PersistentVolumeClaimSpec{ + DataVolumeClaimSpec: v1beta1.VolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, Resources: corev1.VolumeResourceRequirements{ Limits: map[corev1.ResourceName]resource.Quantity{ @@ -1850,7 +1850,7 @@ func TestFindAvailableInstanceNames(t *testing.T) { expectedInstanceNames: []string{"instance1-def"}, }, { set: v1beta1.PostgresInstanceSetSpec{Name: "instance1", - WALVolumeClaimSpec: &corev1.PersistentVolumeClaimSpec{}}, + WALVolumeClaimSpec: &v1beta1.VolumeClaimSpec{}}, fakeObservedInstances: newObservedInstances( &v1beta1.PostgresCluster{Spec: v1beta1.PostgresClusterSpec{ InstanceSets: []v1beta1.PostgresInstanceSetSpec{{Name: "instance1"}}, @@ -1877,7 +1877,7 @@ func TestFindAvailableInstanceNames(t *testing.T) { expectedInstanceNames: []string{}, }, { set: v1beta1.PostgresInstanceSetSpec{Name: "instance1", - WALVolumeClaimSpec: &corev1.PersistentVolumeClaimSpec{}}, + WALVolumeClaimSpec: &v1beta1.VolumeClaimSpec{}}, fakeObservedInstances: newObservedInstances( &v1beta1.PostgresCluster{Spec: v1beta1.PostgresClusterSpec{ InstanceSets: []v1beta1.PostgresInstanceSetSpec{{Name: "instance1"}}, @@ -1901,7 +1901,7 @@ func TestFindAvailableInstanceNames(t *testing.T) { expectedInstanceNames: []string{"instance1-def"}, }, { set: v1beta1.PostgresInstanceSetSpec{Name: "instance1", - WALVolumeClaimSpec: &corev1.PersistentVolumeClaimSpec{}}, + WALVolumeClaimSpec: &v1beta1.VolumeClaimSpec{}}, fakeObservedInstances: newObservedInstances( &v1beta1.PostgresCluster{Spec: v1beta1.PostgresClusterSpec{ InstanceSets: []v1beta1.PostgresInstanceSetSpec{{Name: "instance1"}}, diff --git a/internal/controller/postgrescluster/pgadmin.go b/internal/controller/postgrescluster/pgadmin.go index 87d385becd..dbaaf359ee 100644 --- a/internal/controller/postgrescluster/pgadmin.go +++ b/internal/controller/postgrescluster/pgadmin.go @@ -405,7 +405,7 @@ func (r *Reconciler) reconcilePGAdminDataVolume( cluster.Spec.Metadata.GetLabelsOrNil(), labelMap, ) - pvc.Spec = cluster.Spec.UserInterface.PGAdmin.DataVolumeClaimSpec + pvc.Spec = cluster.Spec.UserInterface.PGAdmin.DataVolumeClaimSpec.AsPersistentVolumeClaimSpec() err := errors.WithStack(r.setControllerReference(cluster, pvc)) diff --git a/internal/controller/postgrescluster/pgadmin_test.go b/internal/controller/postgrescluster/pgadmin_test.go index fd9c656ded..f4be61a8bb 100644 --- a/internal/controller/postgrescluster/pgadmin_test.go +++ b/internal/controller/postgrescluster/pgadmin_test.go @@ -853,7 +853,7 @@ func pgAdminTestCluster(ns corev1.Namespace) *v1beta1.PostgresCluster { Repos: []v1beta1.PGBackRestRepo{{ Name: "repo1", Volume: &v1beta1.RepoPVC{ - VolumeClaimSpec: corev1.PersistentVolumeClaimSpec{ + VolumeClaimSpec: v1beta1.VolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, Resources: corev1.VolumeResourceRequirements{ Requests: corev1.ResourceList{ @@ -868,7 +868,7 @@ func pgAdminTestCluster(ns corev1.Namespace) *v1beta1.PostgresCluster { UserInterface: &v1beta1.UserInterfaceSpec{ PGAdmin: &v1beta1.PGAdminPodSpec{ Image: "test-image", - DataVolumeClaimSpec: corev1.PersistentVolumeClaimSpec{ + DataVolumeClaimSpec: v1beta1.VolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, Resources: corev1.VolumeResourceRequirements{ Requests: corev1.ResourceList{ diff --git a/internal/controller/postgrescluster/pgbackrest.go b/internal/controller/postgrescluster/pgbackrest.go index 54068193af..49d1f8c8ce 100644 --- a/internal/controller/postgrescluster/pgbackrest.go +++ b/internal/controller/postgrescluster/pgbackrest.go @@ -2628,7 +2628,8 @@ func (r *Reconciler) reconcileRepos(ctx context.Context, if repo.Volume == nil { continue } - repo, err := r.applyRepoVolumeIntent(ctx, postgresCluster, repo.Volume.VolumeClaimSpec, + repo, err := r.applyRepoVolumeIntent(ctx, postgresCluster, + repo.Volume.VolumeClaimSpec.AsPersistentVolumeClaimSpec(), repo.Name, repoResources) if err != nil { log.Error(err, errMsg) diff --git a/internal/controller/postgrescluster/pgbackrest_test.go b/internal/controller/postgrescluster/pgbackrest_test.go index 3db4e18b9f..b63120b719 100644 --- a/internal/controller/postgrescluster/pgbackrest_test.go +++ b/internal/controller/postgrescluster/pgbackrest_test.go @@ -64,7 +64,7 @@ func fakePostgresCluster(clusterName, namespace, clusterUID string, Image: "example.com/crunchy-postgres-ha:test", InstanceSets: []v1beta1.PostgresInstanceSetSpec{{ Name: "instance1", - DataVolumeClaimSpec: corev1.PersistentVolumeClaimSpec{ + DataVolumeClaimSpec: v1beta1.VolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}, Resources: corev1.VolumeResourceRequirements{ Requests: corev1.ResourceList{ @@ -115,7 +115,7 @@ func fakePostgresCluster(clusterName, namespace, clusterUID string, postgresCluster.Spec.Backups.PGBackRest.Repos[0] = v1beta1.PGBackRestRepo{ Name: "repo1", Volume: &v1beta1.RepoPVC{ - VolumeClaimSpec: corev1.PersistentVolumeClaimSpec{ + VolumeClaimSpec: v1beta1.VolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}, Resources: corev1.VolumeResourceRequirements{ Requests: map[corev1.ResourceName]resource.Quantity{ @@ -2268,7 +2268,7 @@ func TestCopyConfigurationResources(t *testing.T) { Image: "example.com/crunchy-postgres-ha:test", InstanceSets: []v1beta1.PostgresInstanceSetSpec{{ Name: "instance1", - DataVolumeClaimSpec: corev1.PersistentVolumeClaimSpec{ + DataVolumeClaimSpec: v1beta1.VolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}, Resources: corev1.VolumeResourceRequirements{ Requests: corev1.ResourceList{ @@ -2320,7 +2320,7 @@ func TestCopyConfigurationResources(t *testing.T) { }, InstanceSets: []v1beta1.PostgresInstanceSetSpec{{ Name: "instance1", - DataVolumeClaimSpec: corev1.PersistentVolumeClaimSpec{ + DataVolumeClaimSpec: v1beta1.VolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}, Resources: corev1.VolumeResourceRequirements{ Requests: corev1.ResourceList{ diff --git a/internal/controller/postgrescluster/postgres.go b/internal/controller/postgrescluster/postgres.go index c677d64dbf..6351e18f84 100644 --- a/internal/controller/postgrescluster/postgres.go +++ b/internal/controller/postgrescluster/postgres.go @@ -747,7 +747,7 @@ func (r *Reconciler) reconcilePostgresDataVolume( labelMap, ) - pvc.Spec = instanceSpec.DataVolumeClaimSpec + pvc.Spec = instanceSpec.DataVolumeClaimSpec.AsPersistentVolumeClaimSpec() // If a source cluster was provided and VolumeSnapshots are turned on in the source cluster and // there is a VolumeSnapshot available for the source cluster that is ReadyToUse, use it as the @@ -910,7 +910,7 @@ func (r *Reconciler) reconcileTablespaceVolumes( labelMap, ) - pvc.Spec = vol.DataVolumeClaimSpec + pvc.Spec = vol.DataVolumeClaimSpec.AsPersistentVolumeClaimSpec() if err == nil { err = r.handlePersistentVolumeClaimError(cluster, @@ -1017,7 +1017,7 @@ func (r *Reconciler) reconcilePostgresWALVolume( labelMap, ) - pvc.Spec = *instanceSpec.WALVolumeClaimSpec + pvc.Spec = instanceSpec.WALVolumeClaimSpec.AsPersistentVolumeClaimSpec() if err == nil { err = r.handlePersistentVolumeClaimError(cluster, diff --git a/internal/controller/postgrescluster/postgres_test.go b/internal/controller/postgrescluster/postgres_test.go index 3674d86c3f..db33e7f074 100644 --- a/internal/controller/postgrescluster/postgres_test.go +++ b/internal/controller/postgrescluster/postgres_test.go @@ -883,7 +883,7 @@ func TestSetVolumeSize(t *testing.T) { instanceSetSpec := func(request, limit string) *v1beta1.PostgresInstanceSetSpec { return &v1beta1.PostgresInstanceSetSpec{ Name: "some-instance", - DataVolumeClaimSpec: corev1.PersistentVolumeClaimSpec{ + DataVolumeClaimSpec: v1beta1.VolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, Resources: corev1.VolumeResourceRequirements{ Requests: map[corev1.ResourceName]resource.Quantity{ @@ -911,7 +911,7 @@ func TestSetVolumeSize(t *testing.T) { pvc := &corev1.PersistentVolumeClaim{ObjectMeta: naming.InstancePostgresDataVolume(instance)} spec := instanceSetSpec("4Gi", "3Gi") - pvc.Spec = spec.DataVolumeClaimSpec + pvc.Spec = spec.DataVolumeClaimSpec.AsPersistentVolumeClaimSpec() reconciler.setVolumeSize(ctx, &cluster, pvc, spec.Name) @@ -948,7 +948,7 @@ resources: }}, } - pvc.Spec = spec.DataVolumeClaimSpec + pvc.Spec = spec.DataVolumeClaimSpec.AsPersistentVolumeClaimSpec() reconciler.setVolumeSize(ctx, &cluster, pvc, spec.Name) @@ -984,14 +984,14 @@ resources: pvc := &corev1.PersistentVolumeClaim{ObjectMeta: naming.InstancePostgresDataVolume(instance)} spec := &v1beta1.PostgresInstanceSetSpec{ Name: "some-instance", - DataVolumeClaimSpec: corev1.PersistentVolumeClaimSpec{ + DataVolumeClaimSpec: v1beta1.VolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, Resources: corev1.VolumeResourceRequirements{ Requests: map[corev1.ResourceName]resource.Quantity{ corev1.ResourceStorage: resource.MustParse("1Gi"), }}}} cluster.Status = desiredStatus("2Gi") - pvc.Spec = spec.DataVolumeClaimSpec + pvc.Spec = spec.DataVolumeClaimSpec.AsPersistentVolumeClaimSpec() reconciler.setVolumeSize(ctx, &cluster, pvc, spec.Name) @@ -1016,7 +1016,7 @@ resources: pvc := &corev1.PersistentVolumeClaim{ObjectMeta: naming.InstancePostgresDataVolume(instance)} spec := instanceSetSpec("1Gi", "2Gi") - pvc.Spec = spec.DataVolumeClaimSpec + pvc.Spec = spec.DataVolumeClaimSpec.AsPersistentVolumeClaimSpec() reconciler.setVolumeSize(ctx, &cluster, pvc, spec.Name) @@ -1041,7 +1041,7 @@ resources: pvc := &corev1.PersistentVolumeClaim{ObjectMeta: naming.InstancePostgresDataVolume(instance)} spec := instanceSetSpec("1Gi", "3Gi") cluster.Status = desiredStatus("NotAValidValue") - pvc.Spec = spec.DataVolumeClaimSpec + pvc.Spec = spec.DataVolumeClaimSpec.AsPersistentVolumeClaimSpec() reconciler.setVolumeSize(ctx, &cluster, pvc, spec.Name) @@ -1068,7 +1068,7 @@ resources: pvc := &corev1.PersistentVolumeClaim{ObjectMeta: naming.InstancePostgresDataVolume(instance)} spec := instanceSetSpec("1Gi", "3Gi") cluster.Status = desiredStatus("2Gi") - pvc.Spec = spec.DataVolumeClaimSpec + pvc.Spec = spec.DataVolumeClaimSpec.AsPersistentVolumeClaimSpec() reconciler.setVolumeSize(ctx, &cluster, pvc, spec.Name) @@ -1093,7 +1093,7 @@ resources: pvc := &corev1.PersistentVolumeClaim{ObjectMeta: naming.InstancePostgresDataVolume(instance)} spec := instanceSetSpec("1Gi", "2Gi") cluster.Status = desiredStatus("2Gi") - pvc.Spec = spec.DataVolumeClaimSpec + pvc.Spec = spec.DataVolumeClaimSpec.AsPersistentVolumeClaimSpec() reconciler.setVolumeSize(ctx, &cluster, pvc, spec.Name) @@ -1122,7 +1122,7 @@ resources: pvc := &corev1.PersistentVolumeClaim{ObjectMeta: naming.InstancePostgresDataVolume(instance)} spec := instanceSetSpec("4Gi", "5Gi") cluster.Status = desiredStatus("10Gi") - pvc.Spec = spec.DataVolumeClaimSpec + pvc.Spec = spec.DataVolumeClaimSpec.AsPersistentVolumeClaimSpec() reconciler.setVolumeSize(ctx, &cluster, pvc, spec.Name) diff --git a/internal/controller/postgrescluster/snapshots.go b/internal/controller/postgrescluster/snapshots.go index c639408df2..8f36cefdfc 100644 --- a/internal/controller/postgrescluster/snapshots.go +++ b/internal/controller/postgrescluster/snapshots.go @@ -313,7 +313,7 @@ func (r *Reconciler) createDedicatedSnapshotVolume(ctx context.Context, return pvc, err } - pvc.Spec = instanceSpec.DataVolumeClaimSpec + pvc.Spec = instanceSpec.DataVolumeClaimSpec.AsPersistentVolumeClaimSpec() // Set the snapshot volume to the same size as the pgdata volume. The size should scale with auto-grow. r.setVolumeSize(ctx, cluster, pvc, instanceSpec.Name) diff --git a/internal/controller/postgrescluster/snapshots_test.go b/internal/controller/postgrescluster/snapshots_test.go index 4c56b697fa..4c0ea36761 100644 --- a/internal/controller/postgrescluster/snapshots_test.go +++ b/internal/controller/postgrescluster/snapshots_test.go @@ -388,8 +388,9 @@ func TestReconcileDedicatedSnapshotVolume(t *testing.T) { naming.LabelData: naming.DataPostgres, }, }, - Spec: testVolumeClaimSpec(), } + spec := testVolumeClaimSpec() + pvc.Spec = spec.AsPersistentVolumeClaimSpec() assert.NilError(t, r.setControllerReference(cluster, pvc)) assert.NilError(t, r.apply(ctx, pvc)) diff --git a/internal/controller/postgrescluster/volumes.go b/internal/controller/postgrescluster/volumes.go index aeeeac6166..809b2fe8e1 100644 --- a/internal/controller/postgrescluster/volumes.go +++ b/internal/controller/postgrescluster/volumes.go @@ -254,7 +254,7 @@ func (r *Reconciler) configureExistingPGVolumes( Name: volName, Namespace: cluster.Namespace, }, - Spec: cluster.Spec.InstanceSets[0].DataVolumeClaimSpec, + Spec: cluster.Spec.InstanceSets[0].DataVolumeClaimSpec.AsPersistentVolumeClaimSpec(), } volume.ObjectMeta.Labels = map[string]string{ @@ -307,7 +307,7 @@ func (r *Reconciler) configureExistingPGWALVolume( Name: volName, Namespace: cluster.Namespace, }, - Spec: cluster.Spec.InstanceSets[0].DataVolumeClaimSpec, + Spec: cluster.Spec.InstanceSets[0].DataVolumeClaimSpec.AsPersistentVolumeClaimSpec(), } volume.ObjectMeta.Labels = map[string]string{ @@ -362,7 +362,7 @@ func (r *Reconciler) configureExistingRepoVolumes( cluster.Spec.Backups.PGBackRest.Repos[0].Name), }, Spec: cluster.Spec.Backups.PGBackRest.Repos[0].Volume. - VolumeClaimSpec, + VolumeClaimSpec.AsPersistentVolumeClaimSpec(), } //volume.ObjectMeta = naming.PGBackRestRepoVolume(cluster, cluster.Spec.Backups.PGBackRest.Repos[0].Name) diff --git a/internal/controller/postgrescluster/volumes_test.go b/internal/controller/postgrescluster/volumes_test.go index 3970ee6ccf..85087d079b 100644 --- a/internal/controller/postgrescluster/volumes_test.go +++ b/internal/controller/postgrescluster/volumes_test.go @@ -391,7 +391,7 @@ func TestReconcileConfigureExistingPVCs(t *testing.T) { }, InstanceSets: []v1beta1.PostgresInstanceSetSpec{{ Name: "instance1", - DataVolumeClaimSpec: corev1.PersistentVolumeClaimSpec{ + DataVolumeClaimSpec: v1beta1.VolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{ corev1.ReadWriteMany}, Resources: corev1.VolumeResourceRequirements{ @@ -407,7 +407,7 @@ func TestReconcileConfigureExistingPVCs(t *testing.T) { Repos: []v1beta1.PGBackRestRepo{{ Name: "repo1", Volume: &v1beta1.RepoPVC{ - VolumeClaimSpec: corev1.PersistentVolumeClaimSpec{ + VolumeClaimSpec: v1beta1.VolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{ corev1.ReadWriteMany}, Resources: corev1.VolumeResourceRequirements{ @@ -439,7 +439,7 @@ func TestReconcileConfigureExistingPVCs(t *testing.T) { "somelabel": "labelvalue-pgdata", }, }, - Spec: cluster.Spec.InstanceSets[0].DataVolumeClaimSpec, + Spec: cluster.Spec.InstanceSets[0].DataVolumeClaimSpec.AsPersistentVolumeClaimSpec(), } assert.NilError(t, tClient.Create(ctx, volume)) @@ -504,7 +504,7 @@ func TestReconcileConfigureExistingPVCs(t *testing.T) { "somelabel": "labelvalue-pgwal", }, }, - Spec: cluster.Spec.InstanceSets[0].DataVolumeClaimSpec, + Spec: cluster.Spec.InstanceSets[0].DataVolumeClaimSpec.AsPersistentVolumeClaimSpec(), } assert.NilError(t, tClient.Create(ctx, pgWALVolume)) @@ -570,7 +570,7 @@ func TestReconcileConfigureExistingPVCs(t *testing.T) { "somelabel": "labelvalue-repo", }, }, - Spec: cluster.Spec.InstanceSets[0].DataVolumeClaimSpec, + Spec: cluster.Spec.InstanceSets[0].DataVolumeClaimSpec.AsPersistentVolumeClaimSpec(), } assert.NilError(t, tClient.Create(ctx, volume)) @@ -674,7 +674,7 @@ func TestReconcileMoveDirectories(t *testing.T) { }, }, PriorityClassName: initialize.String("some-priority-class"), - DataVolumeClaimSpec: corev1.PersistentVolumeClaimSpec{ + DataVolumeClaimSpec: v1beta1.VolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{ corev1.ReadWriteMany}, Resources: corev1.VolumeResourceRequirements{ @@ -698,7 +698,7 @@ func TestReconcileMoveDirectories(t *testing.T) { Repos: []v1beta1.PGBackRestRepo{{ Name: "repo1", Volume: &v1beta1.RepoPVC{ - VolumeClaimSpec: corev1.PersistentVolumeClaimSpec{ + VolumeClaimSpec: v1beta1.VolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{ corev1.ReadWriteMany}, Resources: corev1.VolumeResourceRequirements{ diff --git a/internal/controller/standalone_pgadmin/configmap_test.go b/internal/controller/standalone_pgadmin/configmap_test.go index a23ee08d18..267dd77325 100644 --- a/internal/controller/standalone_pgadmin/configmap_test.go +++ b/internal/controller/standalone_pgadmin/configmap_test.go @@ -115,8 +115,9 @@ func TestGenerateConfig(t *testing.T) { func TestGenerateClusterConfig(t *testing.T) { require.ParallelCapacity(t, 0) - cluster := testCluster() + cluster := v1beta1.NewPostgresCluster() cluster.Namespace = "postgres-operator" + cluster.Name = "hippo" clusters := map[string][]*v1beta1.PostgresCluster{ "shared": {cluster, cluster}, "test": {cluster, cluster}, diff --git a/internal/controller/standalone_pgadmin/controller_test.go b/internal/controller/standalone_pgadmin/controller_test.go index 1bd341d54d..4dd984d8ef 100644 --- a/internal/controller/standalone_pgadmin/controller_test.go +++ b/internal/controller/standalone_pgadmin/controller_test.go @@ -29,6 +29,12 @@ func TestDeleteControlled(t *testing.T) { pgadmin := new(v1beta1.PGAdmin) pgadmin.Namespace = ns.Name pgadmin.Name = strings.ToLower(t.Name()) + require.UnmarshalInto(t, &pgadmin.Spec, `{ + dataVolumeClaimSpec: { + accessModes: [ReadWriteOnce], + resources: { requests: { storage: 1Gi } }, + }, + }`) assert.NilError(t, cc.Create(ctx, pgadmin)) t.Run("NoOwnership", func(t *testing.T) { diff --git a/internal/controller/standalone_pgadmin/helpers_unit_test.go b/internal/controller/standalone_pgadmin/helpers_unit_test.go deleted file mode 100644 index 7f4beb5431..0000000000 --- a/internal/controller/standalone_pgadmin/helpers_unit_test.go +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright 2023 - 2025 Crunchy Data Solutions, Inc. -// -// SPDX-License-Identifier: Apache-2.0 - -package standalone_pgadmin - -import ( - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/crunchydata/postgres-operator/internal/initialize" - "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" -) - -// TODO(benjaminjb): This file is duplicated test help functions -// that could probably be put into a separate test_helper package - -var ( - //TODO(tjmoore4): With the new RELATED_IMAGES defaulting behavior, tests could be refactored - // to reference those environment variables instead of hard coded image values - CrunchyPostgresHAImage = "registry.developers.crunchydata.com/crunchydata/crunchy-postgres:ubi8-13.6-1" - CrunchyPGBackRestImage = "registry.developers.crunchydata.com/crunchydata/crunchy-pgbackrest:ubi8-2.38-0" - CrunchyPGBouncerImage = "registry.developers.crunchydata.com/crunchydata/crunchy-pgbouncer:ubi8-1.16-2" -) - -func testCluster() *v1beta1.PostgresCluster { - // Defines a base cluster spec that can be used by tests to generate a - // cluster with an expected number of instances - cluster := v1beta1.PostgresCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "hippo", - }, - Spec: v1beta1.PostgresClusterSpec{ - PostgresVersion: 13, - Image: CrunchyPostgresHAImage, - ImagePullSecrets: []corev1.LocalObjectReference{{ - Name: "myImagePullSecret"}, - }, - InstanceSets: []v1beta1.PostgresInstanceSetSpec{{ - Name: "instance1", - Replicas: initialize.Int32(1), - DataVolumeClaimSpec: testVolumeClaimSpec(), - }}, - Backups: v1beta1.Backups{ - PGBackRest: v1beta1.PGBackRestArchive{ - Image: CrunchyPGBackRestImage, - Repos: []v1beta1.PGBackRestRepo{{ - Name: "repo1", - Volume: &v1beta1.RepoPVC{ - VolumeClaimSpec: testVolumeClaimSpec(), - }, - }}, - }, - }, - Proxy: &v1beta1.PostgresProxySpec{ - PGBouncer: &v1beta1.PGBouncerPodSpec{ - Image: CrunchyPGBouncerImage, - }, - }, - }, - } - return cluster.DeepCopy() -} - -func testVolumeClaimSpec() corev1.PersistentVolumeClaimSpec { - // Defines a volume claim spec that can be used to create instances - return corev1.PersistentVolumeClaimSpec{ - AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, - Resources: corev1.VolumeResourceRequirements{ - Requests: map[corev1.ResourceName]resource.Quantity{ - corev1.ResourceStorage: resource.MustParse("1Gi"), - }, - }, - } -} diff --git a/internal/controller/standalone_pgadmin/related_test.go b/internal/controller/standalone_pgadmin/related_test.go index 649451add6..a14e50d9e2 100644 --- a/internal/controller/standalone_pgadmin/related_test.go +++ b/internal/controller/standalone_pgadmin/related_test.go @@ -41,18 +41,19 @@ func TestFindPGAdminsForSecret(t *testing.T) { pgadmin1 := new(v1beta1.PGAdmin) pgadmin1.Namespace = ns.Name pgadmin1.Name = "first-pgadmin" - pgadmin1.Spec.Users = []v1beta1.PGAdminUser{ - { - PasswordRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "first-password-secret", - }, - Key: "password", - }, - Username: "testuser", - Role: "Administrator", + require.UnmarshalInto(t, &pgadmin1.Spec, `{ + dataVolumeClaimSpec: { + accessModes: [ReadWriteOnce], + resources: { requests: { storage: 1Gi } }, }, - } + users: [ + { + username: testuser, + role: Administrator, + passwordRef: { name: first-password-secret, key: password }, + }, + ], + }`) assert.NilError(t, tClient.Create(ctx, pgadmin1)) pgadmins := reconciler.findPGAdminsForSecret(ctx, secretObjectKey) @@ -65,18 +66,19 @@ func TestFindPGAdminsForSecret(t *testing.T) { pgadmin2 := new(v1beta1.PGAdmin) pgadmin2.Namespace = ns.Name pgadmin2.Name = "second-pgadmin" - pgadmin2.Spec.Users = []v1beta1.PGAdminUser{ - { - PasswordRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "first-password-secret", - }, - Key: "password", - }, - Username: "testuser2", - Role: "Administrator", + require.UnmarshalInto(t, &pgadmin2.Spec, `{ + dataVolumeClaimSpec: { + accessModes: [ReadWriteOnce], + resources: { requests: { storage: 1Gi } }, }, - } + users: [ + { + username: testuser2, + role: Administrator, + passwordRef: { name: first-password-secret, key: password }, + }, + ], + }`) assert.NilError(t, tClient.Create(ctx, pgadmin2)) pgadmins := reconciler.findPGAdminsForSecret(ctx, secretObjectKey) @@ -94,18 +96,19 @@ func TestFindPGAdminsForSecret(t *testing.T) { pgadmin3 := new(v1beta1.PGAdmin) pgadmin3.Namespace = ns.Name pgadmin3.Name = "third-pgadmin" - pgadmin3.Spec.Users = []v1beta1.PGAdminUser{ - { - PasswordRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "other-password-secret", - }, - Key: "password", - }, - Username: "testuser2", - Role: "Administrator", + require.UnmarshalInto(t, &pgadmin3.Spec, `{ + dataVolumeClaimSpec: { + accessModes: [ReadWriteOnce], + resources: { requests: { storage: 1Gi } }, }, - } + users: [ + { + username: testuser2, + role: Administrator, + passwordRef: { name: other-password-secret, key: password }, + }, + ], + }`) assert.NilError(t, tClient.Create(ctx, pgadmin3)) pgadmins := reconciler.findPGAdminsForSecret(ctx, secretObjectKey) diff --git a/internal/controller/standalone_pgadmin/statefulset_test.go b/internal/controller/standalone_pgadmin/statefulset_test.go index 48f0a54a84..9d6b804476 100644 --- a/internal/controller/standalone_pgadmin/statefulset_test.go +++ b/internal/controller/standalone_pgadmin/statefulset_test.go @@ -35,6 +35,12 @@ func TestReconcilePGAdminStatefulSet(t *testing.T) { pgadmin := new(v1beta1.PGAdmin) pgadmin.Name = "test-standalone-pgadmin" pgadmin.Namespace = ns.Name + require.UnmarshalInto(t, &pgadmin.Spec, `{ + dataVolumeClaimSpec: { + accessModes: [ReadWriteOnce], + resources: { requests: { storage: 1Gi } }, + }, + }`) assert.NilError(t, cc.Create(ctx, pgadmin)) t.Cleanup(func() { assert.Check(t, cc.Delete(ctx, pgadmin)) }) @@ -105,6 +111,12 @@ terminationGracePeriodSeconds: 30 // add pod level customizations custompgadmin.Name = "custom-pgadmin" custompgadmin.Namespace = ns.Name + require.UnmarshalInto(t, &custompgadmin.Spec, `{ + dataVolumeClaimSpec: { + accessModes: [ReadWriteOnce], + resources: { requests: { storage: 1Gi } }, + }, + }`) // annotation and label custompgadmin.Spec.Metadata = &v1beta1.Metadata{ diff --git a/internal/controller/standalone_pgadmin/users_test.go b/internal/controller/standalone_pgadmin/users_test.go index 44ad611d8d..fb861e17a7 100644 --- a/internal/controller/standalone_pgadmin/users_test.go +++ b/internal/controller/standalone_pgadmin/users_test.go @@ -236,6 +236,12 @@ func TestWritePGAdminUsers(t *testing.T) { pgadmin := new(v1beta1.PGAdmin) pgadmin.Name = "test-standalone-pgadmin" pgadmin.Namespace = ns.Name + require.UnmarshalInto(t, &pgadmin.Spec, `{ + dataVolumeClaimSpec: { + accessModes: [ReadWriteOnce], + resources: { requests: { storage: 1Gi } }, + }, + }`) assert.NilError(t, cc.Create(ctx, pgadmin)) userPasswordSecret1 := &corev1.Secret{ diff --git a/internal/controller/standalone_pgadmin/volume.go b/internal/controller/standalone_pgadmin/volume.go index ea95e0f22b..dbdfaee649 100644 --- a/internal/controller/standalone_pgadmin/volume.go +++ b/internal/controller/standalone_pgadmin/volume.go @@ -51,7 +51,7 @@ func pvc(pgadmin *v1beta1.PGAdmin) *corev1.PersistentVolumeClaim { pgadmin.Spec.Metadata.GetLabelsOrNil(), naming.StandalonePGAdminDataLabels(pgadmin.Name), ) - pvc.Spec = pgadmin.Spec.DataVolumeClaimSpec + pvc.Spec = pgadmin.Spec.DataVolumeClaimSpec.AsPersistentVolumeClaimSpec() return pvc } diff --git a/internal/controller/standalone_pgadmin/volume_test.go b/internal/controller/standalone_pgadmin/volume_test.go index b0113cba64..6500ac6c42 100644 --- a/internal/controller/standalone_pgadmin/volume_test.go +++ b/internal/controller/standalone_pgadmin/volume_test.go @@ -12,13 +12,11 @@ import ( "gotest.tools/v3/assert" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/crunchydata/postgres-operator/internal/controller/runtime" - "github.com/crunchydata/postgres-operator/internal/initialize" "github.com/crunchydata/postgres-operator/internal/naming" "github.com/crunchydata/postgres-operator/internal/testing/cmp" "github.com/crunchydata/postgres-operator/internal/testing/events" @@ -37,19 +35,16 @@ func TestReconcilePGAdminDataVolume(t *testing.T) { } ns := setupNamespace(t, cc) - pgadmin := &v1beta1.PGAdmin{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-standalone-pgadmin", - Namespace: ns.Name, + pgadmin := v1beta1.NewPGAdmin() + pgadmin.Namespace = ns.Name + pgadmin.Name = "test-standalone-pgadmin" + require.UnmarshalInto(t, &pgadmin.Spec, `{ + dataVolumeClaimSpec: { + accessModes: [ReadWriteOnce], + resources: { requests: { storage: 1Gi } }, + storageClassName: storage-class-for-data, }, - Spec: v1beta1.PGAdminSpec{ - DataVolumeClaimSpec: corev1.PersistentVolumeClaimSpec{ - AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, - Resources: corev1.VolumeResourceRequirements{ - Requests: map[corev1.ResourceName]resource.Quantity{ - corev1.ResourceStorage: resource.MustParse("1Gi")}}, - StorageClassName: initialize.String("storage-class-for-data"), - }}} + }`) assert.NilError(t, cc.Create(ctx, pgadmin)) t.Cleanup(func() { assert.Check(t, cc.Delete(ctx, pgadmin)) }) diff --git a/internal/pgbackrest/pgbackrest_test.go b/internal/pgbackrest/pgbackrest_test.go index 4df29b8449..07ff3d127a 100644 --- a/internal/pgbackrest/pgbackrest_test.go +++ b/internal/pgbackrest/pgbackrest_test.go @@ -61,7 +61,7 @@ fi Repos: []v1beta1.PGBackRestRepo{{ Name: "repo1", Volume: &v1beta1.RepoPVC{ - VolumeClaimSpec: corev1.PersistentVolumeClaimSpec{ + VolumeClaimSpec: v1beta1.VolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}, Resources: corev1.VolumeResourceRequirements{ Requests: map[corev1.ResourceName]resource.Quantity{ diff --git a/internal/postgres/config_test.go b/internal/postgres/config_test.go index 0315072af6..1a7378a50c 100644 --- a/internal/postgres/config_test.go +++ b/internal/postgres/config_test.go @@ -16,7 +16,6 @@ import ( "testing" "gotest.tools/v3/assert" - corev1 "k8s.io/api/core/v1" "sigs.k8s.io/yaml" "github.com/crunchydata/postgres-operator/internal/testing/cmp" @@ -47,7 +46,7 @@ func TestWALDirectory(t *testing.T) { assert.Equal(t, WALDirectory(cluster, instance), "/pgdata/pg13_wal") // with WAL volume - instance.WALVolumeClaimSpec = new(corev1.PersistentVolumeClaimSpec) + instance.WALVolumeClaimSpec = new(v1beta1.VolumeClaimSpec) assert.Equal(t, WALDirectory(cluster, instance), "/pgwal/pg13_wal") } diff --git a/internal/postgres/reconcile_test.go b/internal/postgres/reconcile_test.go index 73fabd3014..a36e3c5368 100644 --- a/internal/postgres/reconcile_test.go +++ b/internal/postgres/reconcile_test.go @@ -608,7 +608,7 @@ volumes: walVolume.Name = "walvol" instance := new(v1beta1.PostgresInstanceSetSpec) - instance.WALVolumeClaimSpec = new(corev1.PersistentVolumeClaimSpec) + instance.WALVolumeClaimSpec = new(v1beta1.VolumeClaimSpec) pod := new(corev1.PodSpec) InstancePod(ctx, cluster, instance, diff --git a/internal/testing/validation/pgadmin_test.go b/internal/testing/validation/pgadmin_test.go index 5d7af6b275..6e50f83deb 100644 --- a/internal/testing/validation/pgadmin_test.go +++ b/internal/testing/validation/pgadmin_test.go @@ -19,6 +19,88 @@ import ( "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) +func TestPGAdminDataVolume(t *testing.T) { + ctx := context.Background() + cc := require.Kubernetes(t) + t.Parallel() + + namespace := require.Namespace(t, cc) + base := v1beta1.NewPGAdmin() + base.Namespace = namespace.Name + base.Name = "pgadmin-data-volume" + require.UnmarshalInto(t, &base.Spec, `{ + dataVolumeClaimSpec: { + accessModes: [ReadWriteOnce], + resources: { requests: { storage: 1Gi } }, + }, + }`) + + assert.NilError(t, cc.Create(ctx, base.DeepCopy(), client.DryRunAll), + "expected this base to be valid") + + t.Run("Required", func(t *testing.T) { + u := require.Value(runtime.ToUnstructuredObject(base)) + unstructured.RemoveNestedField(u.Object, "spec", "dataVolumeClaimSpec") + + err := cc.Create(ctx, u, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + assert.ErrorContains(t, err, "dataVolumeClaimSpec") + assert.ErrorContains(t, err, "Required") + + status := require.StatusError(t, err) + assert.Assert(t, status.Details != nil) + assert.Assert(t, cmp.Len(status.Details.Causes, 2)) + + assert.Equal(t, status.Details.Causes[0].Field, "spec.dataVolumeClaimSpec") + assert.Assert(t, cmp.Contains(status.Details.Causes[0].Message, "Required")) + + assert.Equal(t, string(status.Details.Causes[1].Type), "FieldValueInvalid") + assert.Assert(t, cmp.Contains(status.Details.Causes[1].Message, "rules were not checked")) + }) + + t.Run("AccessModes", func(t *testing.T) { + t.Run("Missing", func(t *testing.T) { + u := require.Value(runtime.ToUnstructuredObject(base)) + unstructured.RemoveNestedField(u.Object, "spec", "dataVolumeClaimSpec", "accessModes") + + err := cc.Create(ctx, u, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + assert.ErrorContains(t, err, "dataVolumeClaimSpec") + assert.ErrorContains(t, err, "accessModes") + }) + + t.Run("Empty", func(t *testing.T) { + pgadmin := base.DeepCopy() + require.UnmarshalInto(t, &pgadmin.Spec.DataVolumeClaimSpec, `{ + accessModes: [], + }`) + + err := cc.Create(ctx, pgadmin, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + assert.ErrorContains(t, err, "dataVolumeClaimSpec") + assert.ErrorContains(t, err, "accessModes") + }) + }) + + t.Run("Resources", func(t *testing.T) { + t.Run("Missing", func(t *testing.T) { + for _, tt := range [][]string{ + {"spec", "dataVolumeClaimSpec", "resources"}, + {"spec", "dataVolumeClaimSpec", "resources", "requests"}, + {"spec", "dataVolumeClaimSpec", "resources", "requests", "storage"}, + } { + u := require.Value(runtime.ToUnstructuredObject(base)) + unstructured.RemoveNestedField(u.Object, tt...) + + err := cc.Create(ctx, u, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + assert.ErrorContains(t, err, "dataVolumeClaimSpec") + assert.ErrorContains(t, err, "storage request") + } + }) + }) +} + func TestPGAdminInstrumentation(t *testing.T) { ctx := context.Background() cc := require.Kubernetes(t) @@ -28,6 +110,12 @@ func TestPGAdminInstrumentation(t *testing.T) { base := v1beta1.NewPGAdmin() base.Namespace = namespace.Name base.Name = "pgadmin-instrumentation" + require.UnmarshalInto(t, &base.Spec, `{ + dataVolumeClaimSpec: { + accessModes: [ReadWriteOnce], + resources: { requests: { storage: 1Gi } }, + }, + }`) assert.NilError(t, cc.Create(ctx, base.DeepCopy(), client.DryRunAll), "expected this base to be valid") diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgadmin_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgadmin_types.go index e9b538368a..f4f04d80b8 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgadmin_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgadmin_types.go @@ -48,8 +48,9 @@ type PGAdminPodSpec struct { // Defines a PersistentVolumeClaim for pgAdmin data. // More info: https://kubernetes.io/docs/concepts/storage/persistent-volumes - // +kubebuilder:validation:Required - DataVolumeClaimSpec corev1.PersistentVolumeClaimSpec `json:"dataVolumeClaimSpec"` + // --- + // +required + DataVolumeClaimSpec VolumeClaimSpec `json:"dataVolumeClaimSpec"` // Name of a container image that can run pgAdmin 4. Changing this value causes // pgAdmin to restart. The image may also be set using the RELATED_IMAGE_PGADMIN diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbackrest_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbackrest_types.go index 877055efd4..5598fd8f6c 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbackrest_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbackrest_types.go @@ -343,20 +343,8 @@ type RepoPVC struct { // Defines a PersistentVolumeClaim spec used to create and/or bind a volume // --- - // +kubebuilder:validation:Required - // - // NOTE(validation): Every PVC must have at least one accessMode. NOTE(KEP-4153) - // TODO(k8s-1.28): fieldPath=`.accessModes`,reason="FieldValueRequired" - // - https://releases.k8s.io/v1.25.0/pkg/apis/core/validation/validation.go#L2098-L2100 - // - https://releases.k8s.io/v1.31.0/pkg/apis/core/validation/validation.go#L2292-L2294 - // +kubebuilder:validation:XValidation:rule=`has(self.accessModes) && size(self.accessModes) > 0`,message=`missing accessModes` - // - // NOTE(validation): Every PVC must have a positive storage request. NOTE(KEP-4153) - // TODO(k8s-1.28): fieldPath=`.resources.requests.storage`,reason="FieldValueRequired" - // - https://releases.k8s.io/v1.25.0/pkg/apis/core/validation/validation.go#L2126-L2133 - // - https://releases.k8s.io/v1.31.0/pkg/apis/core/validation/validation.go#L2318-L2325 - // +kubebuilder:validation:XValidation:rule=`has(self.resources) && has(self.resources.requests) && has(self.resources.requests.storage)`,message=`missing storage request` - VolumeClaimSpec corev1.PersistentVolumeClaimSpec `json:"volumeClaimSpec"` + // +required + VolumeClaimSpec VolumeClaimSpec `json:"volumeClaimSpec"` } // RepoAzure represents a pgBackRest repository that is created using Azure storage diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go index 2a9f982caf..33edac4ebf 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go @@ -467,20 +467,8 @@ type PostgresInstanceSetSpec struct { // Defines a PersistentVolumeClaim for PostgreSQL data. // More info: https://kubernetes.io/docs/concepts/storage/persistent-volumes // --- - // +kubebuilder:validation:Required - // - // NOTE(validation): Every PVC must have at least one accessMode. NOTE(KEP-4153) - // TODO(k8s-1.28): fieldPath=`.accessModes`,reason="FieldValueRequired" - // - https://releases.k8s.io/v1.25.0/pkg/apis/core/validation/validation.go#L2098-L2100 - // - https://releases.k8s.io/v1.31.0/pkg/apis/core/validation/validation.go#L2292-L2294 - // +kubebuilder:validation:XValidation:rule=`has(self.accessModes) && size(self.accessModes) > 0`,message=`missing accessModes` - // - // NOTE(validation): Every PVC must have a positive storage request. NOTE(KEP-4153) - // TODO(k8s-1.28): fieldPath=`.resources.requests.storage`,reason="FieldValueRequired" - // - https://releases.k8s.io/v1.25.0/pkg/apis/core/validation/validation.go#L2126-L2133 - // - https://releases.k8s.io/v1.31.0/pkg/apis/core/validation/validation.go#L2318-L2325 - // +kubebuilder:validation:XValidation:rule=`has(self.resources) && has(self.resources.requests) && has(self.resources.requests.storage)`,message=`missing storage request` - DataVolumeClaimSpec corev1.PersistentVolumeClaimSpec `json:"dataVolumeClaimSpec"` + // +required + DataVolumeClaimSpec VolumeClaimSpec `json:"dataVolumeClaimSpec"` // Priority class name for the PostgreSQL pod. Changing this value causes // PostgreSQL to restart. @@ -521,20 +509,8 @@ type PostgresInstanceSetSpec struct { // Defines a separate PersistentVolumeClaim for PostgreSQL's write-ahead log. // More info: https://www.postgresql.org/docs/current/wal.html // --- - // +kubebuilder:validation:Optional - // - // NOTE(validation): Every PVC must have at least one accessMode. NOTE(KEP-4153) - // TODO(k8s-1.28): fieldPath=`.accessModes`,reason="FieldValueRequired" - // - https://releases.k8s.io/v1.25.0/pkg/apis/core/validation/validation.go#L2098-L2100 - // - https://releases.k8s.io/v1.31.0/pkg/apis/core/validation/validation.go#L2292-L2294 - // +kubebuilder:validation:XValidation:rule=`has(self.accessModes) && size(self.accessModes) > 0`,message=`missing accessModes` - // - // NOTE(validation): Every PVC must have a positive storage request. NOTE(KEP-4153) - // TODO(k8s-1.28): fieldPath=`.resources.requests.storage`,reason="FieldValueRequired" - // - https://releases.k8s.io/v1.25.0/pkg/apis/core/validation/validation.go#L2126-L2133 - // - https://releases.k8s.io/v1.31.0/pkg/apis/core/validation/validation.go#L2318-L2325 - // +kubebuilder:validation:XValidation:rule=`has(self.resources) && has(self.resources.requests) && has(self.resources.requests.storage)`,message=`missing storage request` - WALVolumeClaimSpec *corev1.PersistentVolumeClaimSpec `json:"walVolumeClaimSpec,omitempty"` + // +optional + WALVolumeClaimSpec *VolumeClaimSpec `json:"walVolumeClaimSpec,omitempty"` // The list of tablespaces volumes to mount for this postgrescluster // This field requires enabling TablespaceVolumes feature gate @@ -563,20 +539,8 @@ type TablespaceVolume struct { // Defines a PersistentVolumeClaim for a tablespace. // More info: https://kubernetes.io/docs/concepts/storage/persistent-volumes // --- - // +kubebuilder:validation:Required - // - // NOTE(validation): Every PVC must have at least one accessMode. NOTE(KEP-4153) - // TODO(k8s-1.28): fieldPath=`.accessModes`,reason="FieldValueRequired" - // - https://releases.k8s.io/v1.25.0/pkg/apis/core/validation/validation.go#L2098-L2100 - // - https://releases.k8s.io/v1.31.0/pkg/apis/core/validation/validation.go#L2292-L2294 - // +kubebuilder:validation:XValidation:rule=`has(self.accessModes) && size(self.accessModes) > 0`,message=`missing accessModes` - // - // NOTE(validation): Every PVC must have a positive storage request. NOTE(KEP-4153) - // TODO(k8s-1.28): fieldPath=`.resources.requests.storage`,reason="FieldValueRequired" - // - https://releases.k8s.io/v1.25.0/pkg/apis/core/validation/validation.go#L2126-L2133 - // - https://releases.k8s.io/v1.31.0/pkg/apis/core/validation/validation.go#L2318-L2325 - // +kubebuilder:validation:XValidation:rule=`has(self.resources) && has(self.resources.requests) && has(self.resources.requests.storage)`,message=`missing storage request` - DataVolumeClaimSpec corev1.PersistentVolumeClaimSpec `json:"dataVolumeClaimSpec"` + // +required + DataVolumeClaimSpec VolumeClaimSpec `json:"dataVolumeClaimSpec"` } // InstanceSidecars defines the configuration for instance sidecar containers diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types.go index 4a7236aa9c..281370a40d 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types.go @@ -110,6 +110,35 @@ func (d *Duration) UnmarshalJSON(data []byte) error { return err } +// --- +// NOTE(validation): Every PVC must have at least one accessMode. NOTE(KEP-5073) +// TODO(k8s-1.28): fieldPath=`.accessModes`,reason="FieldValueRequired" +// - https://releases.k8s.io/v1.25.0/pkg/apis/core/validation/validation.go#L2098-L2100 +// - https://releases.k8s.io/v1.32.0/pkg/apis/core/validation/validation.go#L2303-L2305 +// +kubebuilder:validation:XValidation:rule=`0 < size(self.accessModes)`,message=`missing accessModes` +// +// NOTE(validation): Every PVC must have a positive storage request. NOTE(KEP-5073) +// TODO(k8s-1.28): fieldPath=`.resources.requests.storage`,reason="FieldValueRequired" +// TODO(k8s-1.29): `&& 0 < quantity(self.resources.requests.storage).sign()` +// - https://releases.k8s.io/v1.25.0/pkg/apis/core/validation/validation.go#L2126-L2133 +// - https://releases.k8s.io/v1.32.0/pkg/apis/core/validation/validation.go#L2329-L2336 +// +kubebuilder:validation:XValidation:rule=`has(self.resources.requests.storage)`,message=`missing storage request` +// +// +structType=atomic +type VolumeClaimSpec corev1.PersistentVolumeClaimSpec + +// DeepCopyInto copies the receiver into out. Both must be non-nil. +func (spec *VolumeClaimSpec) DeepCopyInto(out *VolumeClaimSpec) { + (*corev1.PersistentVolumeClaimSpec)(spec).DeepCopyInto((*corev1.PersistentVolumeClaimSpec)(out)) +} + +// AsPersistentVolumeClaimSpec returns a copy of spec as a [corev1.PersistentVolumeClaimSpec]. +func (spec *VolumeClaimSpec) AsPersistentVolumeClaimSpec() corev1.PersistentVolumeClaimSpec { + var out corev1.PersistentVolumeClaimSpec + spec.DeepCopyInto((*VolumeClaimSpec)(&out)) + return out +} + // SchemalessObject is a map compatible with JSON object. // // Use with the following markers: @@ -121,7 +150,6 @@ type SchemalessObject map[string]any // DeepCopy creates a new SchemalessObject by copying the receiver. func (in SchemalessObject) DeepCopy() SchemalessObject { return runtime.DeepCopyJSON(in) - } type ServiceSpec struct { diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types_test.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types_test.go index 9d21093535..e4101b672d 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types_test.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types_test.go @@ -10,6 +10,8 @@ import ( "time" "gotest.tools/v3/assert" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" "k8s.io/kube-openapi/pkg/validation/strfmt" "sigs.k8s.io/yaml" ) @@ -199,3 +201,32 @@ func TestSchemalessObjectDeepCopy(t *testing.T) { assert.Assert(t, !reflect.DeepEqual(one, change)) } } + +func TestVolumeClaimSpecYAML(t *testing.T) { + t.Parallel() + + var zero VolumeClaimSpec + out, err := yaml.Marshal(zero) + assert.NilError(t, err) + assert.DeepEqual(t, string(out), "resources: {}\n") + + var parsed VolumeClaimSpec + assert.NilError(t, yaml.Unmarshal([]byte(`{ + accessModes: [ReadWriteMany], + resources: { requests: { storage: 1Gi } }, + storageClassName: zork, + }`), &parsed)) + + zork := "zork" + assert.DeepEqual(t, parsed, VolumeClaimSpec{ + StorageClassName: &zork, + AccessModes: []corev1.PersistentVolumeAccessMode{ + corev1.ReadWriteMany, + }, + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("1Gi"), + }, + }, + }) +} diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/standalone_pgadmin_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/standalone_pgadmin_types.go index 251c213d12..9042245b2f 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/standalone_pgadmin_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/standalone_pgadmin_types.go @@ -58,8 +58,9 @@ type PGAdminSpec struct { // Defines a PersistentVolumeClaim for pgAdmin data. // More info: https://kubernetes.io/docs/concepts/storage/persistent-volumes - // +kubebuilder:validation:Required - DataVolumeClaimSpec corev1.PersistentVolumeClaimSpec `json:"dataVolumeClaimSpec"` + // --- + // +required + DataVolumeClaimSpec VolumeClaimSpec `json:"dataVolumeClaimSpec"` // The image name to use for pgAdmin instance. // +optional diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go index 677a1a1fe9..b139390346 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go @@ -2200,8 +2200,7 @@ func (in *PostgresInstanceSetSpec) DeepCopyInto(out *PostgresInstanceSetSpec) { } if in.WALVolumeClaimSpec != nil { in, out := &in.WALVolumeClaimSpec, &out.WALVolumeClaimSpec - *out = new(corev1.PersistentVolumeClaimSpec) - (*in).DeepCopyInto(*out) + *out = (*in).DeepCopy() } if in.TablespaceVolumes != nil { in, out := &in.TablespaceVolumes, &out.TablespaceVolumes @@ -2652,6 +2651,16 @@ func (in *UserInterfaceSpec) DeepCopy() *UserInterfaceSpec { return out } +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VolumeClaimSpec. +func (in *VolumeClaimSpec) DeepCopy() *VolumeClaimSpec { + if in == nil { + return nil + } + out := new(VolumeClaimSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VolumeSnapshots) DeepCopyInto(out *VolumeSnapshots) { *out = *in