Skip to content

Commit e64c4b0

Browse files
phiscolitaocdljsilvela
authored
fix: allow omitting storage size if in pvcTemplate (cloudnative-pg#914)
This patch allows omitting the `spec.storage.size` parameter if and only if a request size is specified through the provided template. Signed-off-by: Philippe Scorsolini <p.scorsolini@gmail.com> Signed-off-by: Tao Li <tao.li@enterprisedb.com> Signed-off-by: Jaime Silvela <jaime.silvela@enterprisedb.com> Co-authored-by: Tao Li <tao.li@enterprisedb.com> Co-authored-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
1 parent 8d3867e commit e64c4b0

File tree

7 files changed

+92
-18
lines changed

7 files changed

+92
-18
lines changed

‎api/v1/cluster_types.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -1069,7 +1069,7 @@ type StorageConfiguration struct {
10691069
// Size of the storage. Required if not already specified in the PVC template.
10701070
// Changes to this field are automatically reapplied to the created PVCs.
10711071
// Size cannot be decreased.
1072-
Size string `json:"size"`
1072+
Size string `json:"size,omitempty"`
10731073

10741074
// Resize existent PVCs, defaults to true
10751075
// +optional

‎api/v1/cluster_webhook.go

+18-3
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,7 @@ func (r *Cluster) Validate() (allErrs field.ErrorList) {
280280
r.validatePrimaryUpdateStrategy,
281281
r.validateMinSyncReplicas,
282282
r.validateMaxSyncReplicas,
283+
r.validateStorageSize,
283284
r.validateWalStorageSize,
284285
r.validateName,
285286
r.validateBootstrapPgBaseBackupSource,
@@ -1090,6 +1091,10 @@ func (r *Cluster) validateMinSyncReplicas() field.ErrorList {
10901091
return result
10911092
}
10921093

1094+
func (r *Cluster) validateStorageSize() field.ErrorList {
1095+
return validateStorageConfigurationSize("Storage", r.Spec.StorageConfiguration)
1096+
}
1097+
10931098
func (r *Cluster) validateWalStorageSize() field.ErrorList {
10941099
var result field.ErrorList
10951100

@@ -1103,13 +1108,23 @@ func (r *Cluster) validateWalStorageSize() field.ErrorList {
11031108
func validateStorageConfigurationSize(structPath string, storageConfiguration StorageConfiguration) field.ErrorList {
11041109
var result field.ErrorList
11051110

1106-
if _, err := resource.ParseQuantity(storageConfiguration.Size); err != nil {
1111+
if storageConfiguration.Size != "" {
1112+
if _, err := resource.ParseQuantity(storageConfiguration.Size); err != nil {
1113+
result = append(result, field.Invalid(
1114+
field.NewPath("spec", structPath, "size"),
1115+
storageConfiguration.Size,
1116+
"Size value isn't valid"))
1117+
}
1118+
}
1119+
1120+
if storageConfiguration.Size == "" &&
1121+
(storageConfiguration.PersistentVolumeClaimTemplate == nil ||
1122+
storageConfiguration.PersistentVolumeClaimTemplate.Resources.Requests.Storage().IsZero()) {
11071123
result = append(result, field.Invalid(
11081124
field.NewPath("spec", structPath, "size"),
11091125
storageConfiguration.Size,
1110-
"Size value isn't valid"))
1126+
"Size not configured. Please add it, or a storage request in the pvcTemplate."))
11111127
}
1112-
11131128
return result
11141129
}
11151130

‎config/crd/bases/postgresql.cnpg.io_clusters.yaml

-4
Original file line numberDiff line numberDiff line change
@@ -2305,8 +2305,6 @@ spec:
23052305
not specified, generated PVCs will be satisfied by the default
23062306
storage class
23072307
type: string
2308-
required:
2309-
- size
23102308
type: object
23112309
superuserSecret:
23122310
description: The secret containing the superuser password. If not
@@ -2514,8 +2512,6 @@ spec:
25142512
not specified, generated PVCs will be satisfied by the default
25152513
storage class
25162514
type: string
2517-
required:
2518-
- size
25192515
type: object
25202516
required:
25212517
- instances

‎controllers/cluster_controller.go

+4
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,10 @@ func (r *ClusterReconciler) ReconcilePVCs(ctx context.Context, cluster *apiv1.Cl
585585
return nil
586586
}
587587

588+
// Size is empty would due to size is defined through request and not changed yet
589+
if cluster.Spec.StorageConfiguration.Size == "" {
590+
return nil
591+
}
588592
quantity, err := resource.ParseQuantity(cluster.Spec.StorageConfiguration.Size)
589593
if err != nil {
590594
return fmt.Errorf("while parsing PVC size %v: %w", cluster.Spec.StorageConfiguration.Size, err)

‎docs/src/api_reference.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -947,7 +947,7 @@ StorageConfiguration is the configuration of the storage of the PostgreSQL insta
947947
Name | Description | Type
948948
------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------
949949
`storageClass ` | StorageClass to use for database data (`PGDATA`). Applied after evaluating the PVC template, if available. If not specified, generated PVCs will be satisfied by the default storage class | *string
950-
`size ` | Size of the storage. Required if not already specified in the PVC template. Changes to this field are automatically reapplied to the created PVCs. Size cannot be decreased. - *mandatory* | string
950+
`size ` | Size of the storage. Required if not already specified in the PVC template. Changes to this field are automatically reapplied to the created PVCs. Size cannot be decreased. | string
951951
`resizeInUseVolumes` | Resize existent PVCs, defaults to true | *bool
952952
`pvcTemplate ` | Template to be used to generate the Persistent Volume Claim | [*corev1.PersistentVolumeClaimSpec](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#persistentvolumeclaim-v1-core)
953953

‎pkg/specs/pvcs.go

+15-9
Original file line numberDiff line numberDiff line change
@@ -109,16 +109,18 @@ func CreatePVC(
109109
result.Spec.StorageClassName = storageConfiguration.StorageClass
110110
}
111111

112-
// Insert the storage requirement
113-
parsedSize, err := resource.ParseQuantity(storageConfiguration.Size)
114-
if err != nil {
115-
return nil, ErrorInvalidSize
116-
}
112+
if storageConfiguration.Size != "" {
113+
// Insert the storage requirement
114+
parsedSize, err := resource.ParseQuantity(storageConfiguration.Size)
115+
if err != nil {
116+
return nil, ErrorInvalidSize
117+
}
117118

118-
result.Spec.Resources = corev1.ResourceRequirements{
119-
Requests: corev1.ResourceList{
120-
"storage": parsedSize,
121-
},
119+
result.Spec.Resources = corev1.ResourceRequirements{
120+
Requests: corev1.ResourceList{
121+
"storage": parsedSize,
122+
},
123+
}
122124
}
123125

124126
if len(result.Spec.AccessModes) == 0 {
@@ -127,6 +129,10 @@ func CreatePVC(
127129
}
128130
}
129131

132+
if result.Spec.Resources.Requests.Storage().IsZero() {
133+
return nil, ErrorInvalidSize
134+
}
135+
130136
return result, nil
131137
}
132138

‎pkg/specs/pvcs_test.go

+53
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121

2222
batchv1 "k8s.io/api/batch/v1"
2323
corev1 "k8s.io/api/core/v1"
24+
"k8s.io/apimachinery/pkg/api/resource"
2425
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2526

2627
apiv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
@@ -117,3 +118,55 @@ var _ = Describe("PVC detection", func() {
117118
Expect(pvcUsage.InstanceNames).Should(ConsistOf(clusterName+"-1", clusterName+"-2", clusterName+"-4"))
118119
})
119120
})
121+
122+
var _ = Describe("PVC Creation", func() {
123+
storageClass := "default"
124+
It("handles size properly only with size specified", func() {
125+
pvc, err := CreatePVC(
126+
apiv1.StorageConfiguration{
127+
Size: "1Gi",
128+
StorageClass: &storageClass,
129+
},
130+
apiv1.Cluster{},
131+
0,
132+
utils.PVCRolePgData,
133+
)
134+
Expect(err).NotTo(HaveOccurred())
135+
Expect(pvc.Spec.Resources.Requests.Storage().String()).To(Equal("1Gi"))
136+
})
137+
It("handles size properly with only template specified", func() {
138+
pvc, err := CreatePVC(
139+
apiv1.StorageConfiguration{
140+
StorageClass: &storageClass,
141+
PersistentVolumeClaimTemplate: &corev1.PersistentVolumeClaimSpec{
142+
Resources: corev1.ResourceRequirements{
143+
Requests: corev1.ResourceList{"storage": resource.MustParse("1Gi")},
144+
},
145+
},
146+
},
147+
apiv1.Cluster{},
148+
0,
149+
utils.PVCRolePgData,
150+
)
151+
Expect(err).NotTo(HaveOccurred())
152+
Expect(pvc.Spec.Resources.Requests.Storage().String()).To(Equal("1Gi"))
153+
})
154+
It("handles size properly with both template and size specified, size taking precedence", func() {
155+
pvc, err := CreatePVC(
156+
apiv1.StorageConfiguration{
157+
Size: "2Gi",
158+
StorageClass: &storageClass,
159+
PersistentVolumeClaimTemplate: &corev1.PersistentVolumeClaimSpec{
160+
Resources: corev1.ResourceRequirements{
161+
Requests: corev1.ResourceList{"storage": resource.MustParse("1Gi")},
162+
},
163+
},
164+
},
165+
apiv1.Cluster{},
166+
0,
167+
utils.PVCRolePgData,
168+
)
169+
Expect(err).NotTo(HaveOccurred())
170+
Expect(pvc.Spec.Resources.Requests.Storage().String()).To(Equal("2Gi"))
171+
})
172+
})

0 commit comments

Comments
 (0)