diff --git a/.golangci.yaml b/.golangci.yaml index 113efd7982..a1de0813b4 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -170,10 +170,6 @@ linters: text: >- deprecated: Use `RequeueAfter` instead - - linters: [unparam] - path: internal/controller/postgrescluster/autogrow.go - text: volumeType always receives \"pgData\" - # https://golangci-lint.run/usage/formatters formatters: enable: diff --git a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml index d3e8d3db8b..facd00f95e 100644 --- a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml +++ b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml @@ -18632,6 +18632,11 @@ spec: type: string description: Desired Size of the pgData volume type: object + desiredPGWALVolume: + additionalProperties: + type: string + description: Desired Size of the pgWAL volume + type: object name: type: string readyReplicas: @@ -37557,6 +37562,11 @@ spec: type: string description: Desired Size of the pgData volume type: object + desiredPGWALVolume: + additionalProperties: + type: string + description: Desired Size of the pgWAL volume + type: object name: type: string readyReplicas: diff --git a/internal/controller/postgrescluster/autogrow.go b/internal/controller/postgrescluster/autogrow.go index c95b50a9df..bb0d5b5610 100644 --- a/internal/controller/postgrescluster/autogrow.go +++ b/internal/controller/postgrescluster/autogrow.go @@ -57,7 +57,12 @@ func (r *Reconciler) storeDesiredRequest( // determine if the appropriate volume limit is set limitSet := limitIsSet(cluster, volumeType, host) - if limitSet && current.Value() > previous.Value() { + // if nil, volume does not exist + if limitSet == nil { + return "" + } + + if *limitSet && current.Value() > previous.Value() { r.Recorder.Eventf(cluster, corev1.EventTypeNormal, "VolumeAutoGrow", "%s volume expansion to %v requested for %s/%s.", volumeType, current.String(), cluster.Name, host) @@ -74,34 +79,48 @@ func (r *Reconciler) storeDesiredRequest( } // limitIsSet determines if the limit is set for a given volume type and returns -// a corresponding boolean value -func limitIsSet(cluster *v1beta1.PostgresCluster, volumeType, instanceSetName string) bool { - - var limitSet bool +// either a corresponding boolean value or nil if the volume is not defined. +func limitIsSet(cluster *v1beta1.PostgresCluster, volumeType, instanceSetName string) *bool { switch { - // Cycle through the instance sets to ensure the correct limit is identified. + // For pgData and pgWAL volumes, cycle through the instance sets to ensure + // the correct limit is identified. case volumeType == "pgData": for _, specInstance := range cluster.Spec.InstanceSets { if specInstance.Name == instanceSetName { - limitSet = !specInstance.DataVolumeClaimSpec.Resources.Limits.Storage().IsZero() + limitSet := !specInstance.DataVolumeClaimSpec.Resources.Limits.Storage().IsZero() + return &limitSet + } + } + case volumeType == "pgWAL": + for _, specInstance := range cluster.Spec.InstanceSets { + // return nil if volume is not defined + if specInstance.Name == instanceSetName && specInstance.WALVolumeClaimSpec == nil { + return nil + } + if specInstance.Name == instanceSetName && specInstance.WALVolumeClaimSpec != nil { + limitSet := !specInstance.WALVolumeClaimSpec.Resources.Limits.Storage().IsZero() + return &limitSet } } - // VolumeType for the repository host volumes should be in the form 'repoN' // where N is 1-4. As above, cycle through any defined repositories and ensure // the correct limit is identified. case strings.HasPrefix(volumeType, "repo"): for _, specRepo := range cluster.Spec.Backups.PGBackRest.Repos { + // return nil if volume is not defined + if specRepo.Name == volumeType && specRepo.Volume == nil { + return nil + } if specRepo.Name == volumeType && specRepo.Volume != nil { - limitSet = !specRepo.Volume.VolumeClaimSpec.Resources.Limits.Storage().IsZero() + limitSet := !specRepo.Volume.VolumeClaimSpec.Resources.Limits.Storage().IsZero() + return &limitSet } } } - // TODO: Add case for pgWAL - return limitSet + return nil } @@ -195,6 +214,24 @@ func getDesiredVolumeSize(cluster *v1beta1.PostgresCluster, } } + case volumeType == "pgWAL": + for i := range cluster.Status.InstanceSets { + if instanceSpecName == cluster.Status.InstanceSets[i].Name { + for _, dpv := range cluster.Status.InstanceSets[i].DesiredPGWALVolume { + if dpv != "" { + desiredRequest, err := resource.ParseQuantity(dpv) + if err == nil { + if desiredRequest.Value() > volumeRequestSize.Value() { + *volumeRequestSize = desiredRequest + } + } else { + return dpv, err + } + } + } + } + } + // VolumeType for the repository host volumes should be in the form 'repoN' // where N is 1-4. As above, cycle through any defined repositories and ensure // the correct limit is identified. @@ -218,6 +255,5 @@ func getDesiredVolumeSize(cluster *v1beta1.PostgresCluster, } } } - // TODO: Add case for pgWAL return "", nil } diff --git a/internal/controller/postgrescluster/autogrow_test.go b/internal/controller/postgrescluster/autogrow_test.go index 51fbfaab9e..3e75df6326 100644 --- a/internal/controller/postgrescluster/autogrow_test.go +++ b/internal/controller/postgrescluster/autogrow_test.go @@ -51,91 +51,180 @@ func TestStoreDesiredRequest(t *testing.T) { Limits: map[corev1.ResourceName]resource.Quantity{ corev1.ResourceStorage: resource.MustParse("1Gi"), }}}, + WALVolumeClaimSpec: &v1beta1.VolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + Resources: corev1.VolumeResourceRequirements{ + Limits: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceStorage: resource.MustParse("1Gi"), + }}}, }, { - Name: "blue", + Name: "blue", + Replicas: initialize.Int32(1), + WALVolumeClaimSpec: &v1beta1.VolumeClaimSpec{}, + }, { + Name: "green", Replicas: initialize.Int32(1), - }}}} - - t.Run("BadRequestNoBackup", func(t *testing.T) { - recorder := events.NewRecorder(t, runtime.Scheme) - reconciler := &Reconciler{Recorder: recorder} - ctx, logs := setupLogCapture(ctx) - - value := reconciler.storeDesiredRequest(ctx, &cluster, "pgData", "red", "woot", "") - - assert.Equal(t, value, "") - assert.Equal(t, len(recorder.Events), 0) - assert.Equal(t, len(*logs), 1) - assert.Assert(t, cmp.Contains((*logs)[0], "Unable to parse pgData volume request from status")) - }) - - t.Run("BadRequestWithBackup", func(t *testing.T) { - recorder := events.NewRecorder(t, runtime.Scheme) - reconciler := &Reconciler{Recorder: recorder} - ctx, logs := setupLogCapture(ctx) - - value := reconciler.storeDesiredRequest(ctx, &cluster, "pgData", "red", "foo", "1Gi") - - assert.Equal(t, value, "1Gi") - assert.Equal(t, len(recorder.Events), 0) - assert.Equal(t, len(*logs), 1) - assert.Assert(t, cmp.Contains((*logs)[0], "Unable to parse pgData volume request from status (foo) for rhino/red")) - }) - - t.Run("NoLimitNoEvent", func(t *testing.T) { - recorder := events.NewRecorder(t, runtime.Scheme) - reconciler := &Reconciler{Recorder: recorder} - ctx, logs := setupLogCapture(ctx) - - value := reconciler.storeDesiredRequest(ctx, &cluster, "pgData", "blue", "1Gi", "") - - assert.Equal(t, value, "1Gi") - assert.Equal(t, len(*logs), 0) - assert.Equal(t, len(recorder.Events), 0) - }) - - t.Run("BadBackupRequest", func(t *testing.T) { - recorder := events.NewRecorder(t, runtime.Scheme) - reconciler := &Reconciler{Recorder: recorder} - ctx, logs := setupLogCapture(ctx) - - value := reconciler.storeDesiredRequest(ctx, &cluster, "pgData", "red", "2Gi", "bar") - - assert.Equal(t, value, "2Gi") - assert.Equal(t, len(*logs), 1) - assert.Assert(t, cmp.Contains((*logs)[0], "Unable to parse pgData volume request from status backup (bar) for rhino/red")) - assert.Equal(t, len(recorder.Events), 1) - assert.Equal(t, recorder.Events[0].Regarding.Name, cluster.Name) - assert.Equal(t, recorder.Events[0].Reason, "VolumeAutoGrow") - assert.Equal(t, recorder.Events[0].Note, "pgData volume expansion to 2Gi requested for rhino/red.") - }) - - t.Run("ValueUpdateWithEvent", func(t *testing.T) { - recorder := events.NewRecorder(t, runtime.Scheme) - reconciler := &Reconciler{Recorder: recorder} - ctx, logs := setupLogCapture(ctx) - - value := reconciler.storeDesiredRequest(ctx, &cluster, "pgData", "red", "1Gi", "") + }}, + Backups: v1beta1.Backups{ + PGBackRest: v1beta1.PGBackRestArchive{ + Repos: []v1beta1.PGBackRestRepo{{ + Name: "repo1", + Volume: &v1beta1.RepoPVC{ + VolumeClaimSpec: v1beta1.VolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + Resources: corev1.VolumeResourceRequirements{ + Limits: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceStorage: resource.MustParse("1Gi"), + }}}, + }, + }, { + Name: "repo2", + Volume: &v1beta1.RepoPVC{}, + }, { + Name: "repo3", + }}}, + }, + }, + } - assert.Equal(t, value, "1Gi") - assert.Equal(t, len(*logs), 0) - assert.Equal(t, len(recorder.Events), 1) - assert.Equal(t, recorder.Events[0].Regarding.Name, cluster.Name) - assert.Equal(t, recorder.Events[0].Reason, "VolumeAutoGrow") - assert.Equal(t, recorder.Events[0].Note, "pgData volume expansion to 1Gi requested for rhino/red.") - }) + testCases := []struct { + tcName string + Voltype string + host string + desiredRequest string + desiredRequestBackup string + expectedValue string + expectedNumLogs int + expectedLog string + expectedNumEvents int + expectedEvent string + }{{ + tcName: "PGData-BadRequestNoBackup", + Voltype: "pgData", host: "red", + desiredRequest: "woot", desiredRequestBackup: "", expectedValue: "", + expectedNumEvents: 0, expectedNumLogs: 1, + expectedLog: "Unable to parse pgData volume request from status (woot) for rhino/red", + }, { + tcName: "PGData-BadRequestWithBackup", + Voltype: "pgData", host: "red", + desiredRequest: "foo", desiredRequestBackup: "1Gi", expectedValue: "1Gi", + expectedNumEvents: 0, expectedNumLogs: 1, + expectedLog: "Unable to parse pgData volume request from status (foo) for rhino/red", + }, { + tcName: "PGData-NoLimitNoEvent", + Voltype: "pgData", host: "blue", + desiredRequest: "1Gi", desiredRequestBackup: "", expectedValue: "1Gi", + expectedNumEvents: 0, expectedNumLogs: 0, + }, { + tcName: "PGData-BadBackupRequest", + Voltype: "pgData", host: "red", + desiredRequest: "2Gi", desiredRequestBackup: "bar", expectedValue: "2Gi", + expectedNumEvents: 1, expectedEvent: "pgData volume expansion to 2Gi requested for rhino/red.", + expectedNumLogs: 1, expectedLog: "Unable to parse pgData volume request from status backup (bar) for rhino/red", + }, { + tcName: "PGData-ValueUpdateWithEvent", + Voltype: "pgData", host: "red", + desiredRequest: "1Gi", desiredRequestBackup: "", expectedValue: "1Gi", + expectedNumEvents: 1, expectedEvent: "pgData volume expansion to 1Gi requested for rhino/red.", + expectedNumLogs: 0, + }, { + tcName: "PGWAL-BadRequestNoBackup", + Voltype: "pgWAL", host: "red", + desiredRequest: "woot", desiredRequestBackup: "", expectedValue: "", + expectedNumEvents: 0, expectedNumLogs: 1, + expectedLog: "Unable to parse pgWAL volume request from status (woot) for rhino/red", + }, { + tcName: "PGWAL-BadRequestWithBackup", + Voltype: "pgWAL", host: "red", + desiredRequest: "foo", desiredRequestBackup: "1Gi", expectedValue: "1Gi", + expectedNumEvents: 0, expectedNumLogs: 1, + expectedLog: "Unable to parse pgWAL volume request from status (foo) for rhino/red", + }, { + tcName: "PGWAL-NoLimitNoEvent", + Voltype: "pgWAL", host: "blue", + desiredRequest: "1Gi", desiredRequestBackup: "", expectedValue: "1Gi", + expectedNumEvents: 0, expectedNumLogs: 0, + }, { + tcName: "PGWAL-NoVolumeDefined", + Voltype: "pgWAL", host: "green", + desiredRequest: "1Gi", desiredRequestBackup: "", expectedValue: "", + expectedNumEvents: 0, expectedNumLogs: 0, + }, { + tcName: "PGWAL-BadBackupRequest", + Voltype: "pgWAL", host: "red", + desiredRequest: "2Gi", desiredRequestBackup: "bar", expectedValue: "2Gi", + expectedNumEvents: 1, expectedEvent: "pgWAL volume expansion to 2Gi requested for rhino/red.", + expectedNumLogs: 1, expectedLog: "Unable to parse pgWAL volume request from status backup (bar) for rhino/red", + }, { + tcName: "PGWAL-ValueUpdateWithEvent", + Voltype: "pgWAL", host: "red", + desiredRequest: "1Gi", desiredRequestBackup: "", expectedValue: "1Gi", + expectedNumEvents: 1, expectedEvent: "pgWAL volume expansion to 1Gi requested for rhino/red.", + expectedNumLogs: 0, + }, { + tcName: "Repo-BadRequestNoBackup", + Voltype: "repo1", host: "repo-host", + desiredRequest: "woot", desiredRequestBackup: "", expectedValue: "", + expectedNumEvents: 0, expectedNumLogs: 1, + expectedLog: "Unable to parse repo1 volume request from status (woot) for rhino/repo-host", + }, { + tcName: "Repo-BadRequestWithBackup", + Voltype: "repo1", host: "repo-host", + desiredRequest: "foo", desiredRequestBackup: "1Gi", expectedValue: "1Gi", + expectedNumEvents: 0, expectedNumLogs: 1, + expectedLog: "Unable to parse repo1 volume request from status (foo) for rhino/repo-host", + }, { + tcName: "Repo-NoLimitNoEvent", + Voltype: "repo2", host: "repo-host", + desiredRequest: "1Gi", desiredRequestBackup: "", expectedValue: "1Gi", + expectedNumEvents: 0, expectedNumLogs: 0, + }, { + tcName: "Repo-NoRepoDefined", + Voltype: "repo3", host: "repo-host", + desiredRequest: "1Gi", desiredRequestBackup: "", expectedValue: "", + expectedNumEvents: 0, expectedNumLogs: 0, + }, { + tcName: "Repo-BadBackupRequest", + Voltype: "repo1", host: "repo-host", + desiredRequest: "2Gi", desiredRequestBackup: "bar", expectedValue: "2Gi", + expectedNumEvents: 1, expectedEvent: "repo1 volume expansion to 2Gi requested for rhino/repo-host.", + expectedNumLogs: 1, expectedLog: "Unable to parse repo1 volume request from status backup (bar) for rhino/repo-host", + }, { + tcName: "Repo-ValueUpdateWithEvent", + Voltype: "repo1", host: "repo-host", + desiredRequest: "1Gi", desiredRequestBackup: "", expectedValue: "1Gi", + expectedNumEvents: 1, expectedEvent: "repo1 volume expansion to 1Gi requested for rhino/repo-host.", + expectedNumLogs: 0, + }} - t.Run("NoLimitNoEvent", func(t *testing.T) { - recorder := events.NewRecorder(t, runtime.Scheme) - reconciler := &Reconciler{Recorder: recorder} - ctx, logs := setupLogCapture(ctx) + for _, tc := range testCases { + t.Run(tc.tcName, func(t *testing.T) { + recorder := events.NewRecorder(t, runtime.Scheme) + reconciler := &Reconciler{Recorder: recorder} + ctx, logs := setupLogCapture(ctx) - value := reconciler.storeDesiredRequest(ctx, &cluster, "pgData", "blue", "1Gi", "") + value := reconciler.storeDesiredRequest( + ctx, + &cluster, + tc.Voltype, + tc.host, + tc.desiredRequest, + tc.desiredRequestBackup, + ) + assert.Equal(t, value, tc.expectedValue) + assert.Equal(t, len(recorder.Events), tc.expectedNumEvents) + if tc.expectedNumEvents == 1 { + assert.Equal(t, recorder.Events[0].Regarding.Name, cluster.Name) + assert.Equal(t, recorder.Events[0].Reason, "VolumeAutoGrow") + assert.Equal(t, recorder.Events[0].Note, tc.expectedEvent) + } + assert.Equal(t, len(*logs), tc.expectedNumLogs) + if tc.expectedNumLogs == 1 { + assert.Assert(t, cmp.Contains((*logs)[0], tc.expectedLog)) + } + }) - assert.Equal(t, value, "1Gi") - assert.Equal(t, len(*logs), 0) - assert.Equal(t, len(recorder.Events), 0) - }) + } } func TestLimitIsSet(t *testing.T) { @@ -155,9 +244,19 @@ func TestLimitIsSet(t *testing.T) { Limits: map[corev1.ResourceName]resource.Quantity{ corev1.ResourceStorage: resource.MustParse("1Gi"), }}}, + WALVolumeClaimSpec: &v1beta1.VolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + Resources: corev1.VolumeResourceRequirements{ + Limits: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceStorage: resource.MustParse("2Gi"), + }}}, }, { Name: "blue", Replicas: initialize.Int32(1), + }, { + Name: "green", + Replicas: initialize.Int32(1), + WALVolumeClaimSpec: &v1beta1.VolumeClaimSpec{}, }}, Backups: v1beta1.Backups{ PGBackRest: v1beta1.PGBackRestArchive{ @@ -173,52 +272,86 @@ func TestLimitIsSet(t *testing.T) { }, }}}, { - Name: "repo2", - }}}}, + Name: "repo2", + Volume: &v1beta1.RepoPVC{}, + }, { + Name: "repo3", + }, + }}}, }} testCases := []struct { tcName string Voltype string instanceName string - expected bool + expected *bool }{{ - tcName: "Limit is set for instance PGDATA volume", + tcName: "PGDATA Limit is set for defined volume", Voltype: "pgData", instanceName: "red", - expected: true, + expected: initialize.Pointer(true), }, { - tcName: "Limit is not set for instance PGDATA volume", + tcName: "PGDATA Limit is not set for defined volume", Voltype: "pgData", instanceName: "blue", - expected: false, + expected: initialize.Pointer(false), }, { - tcName: "Check PGDATA volume for non-existent instance", + tcName: "PGDATA Check volume for non-existent instance", Voltype: "pgData", instanceName: "orange", - expected: false, + expected: nil, + }, { + tcName: "PGWAL Limit is set for defined volume", + Voltype: "pgWAL", + instanceName: "red", + expected: initialize.Pointer(true), + }, { + tcName: "PGWAL WAL volume defined but limit is not set", + Voltype: "pgWAL", + instanceName: "green", + expected: initialize.Pointer(false), + }, { + tcName: "PGWAL Instance has no pg_wal volume defined", + Voltype: "pgWAL", + instanceName: "blue", + expected: nil, + }, { + tcName: "PGWAL Check volume for non-existent instance", + Voltype: "pgWAL", + instanceName: "orange", + expected: nil, }, { - tcName: "Limit is not set for repo1 volume", + tcName: "REPO Limit set for defined volume", Voltype: "repo1", instanceName: "", - expected: true, + expected: initialize.Pointer(true), }, { - tcName: "Limit is not set for repo2 volume", + tcName: "REPO Limit is not set for defined volume", Voltype: "repo2", instanceName: "", - expected: false, + expected: initialize.Pointer(false), }, { - tcName: "Check non-existent repo volume", + tcName: "REPO volume not defined for repo", Voltype: "repo3", instanceName: "", - expected: false, + expected: nil, + }, { + tcName: "REPO Check volume for non-existent repo", + Voltype: "repo4", + instanceName: "", + expected: nil, }} for _, tc := range testCases { t.Run(tc.tcName, func(t *testing.T) { limitSet := limitIsSet(&cluster, tc.Voltype, tc.instanceName) - assert.Check(t, limitSet == tc.expected) + if tc.expected == nil { + assert.Assert(t, limitSet == nil) + } else { + assert.Assert(t, limitSet != nil) + assert.Check(t, *limitSet == *tc.expected) + } }) } } @@ -508,7 +641,8 @@ resources: // NB: The code in 'setVolumeSize' is identical no matter the volume type. // Since the different statuses are pulled out by the embedded 'getDesiredVolumeSize' - // function, we can just try a couple scenarios to validate the behavior. + // function, we can just try a couple scenarios to validate the behavior + // for the repo and pg_wal volumes. t.Run("StatusWithLimitGrowToLimit-RepoHost", func(t *testing.T) { recorder := events.NewRecorder(t, runtime.Scheme) reconciler := &Reconciler{Recorder: recorder} @@ -542,10 +676,42 @@ resources: assert.Equal(t, recorder.Events[0].Note, "repo1 volume(s) for elephant/repo-host are at size limit (2Gi).") }) + t.Run("StatusWithLimitGrowToLimit-pgWAL", func(t *testing.T) { + recorder := events.NewRecorder(t, runtime.Scheme) + reconciler := &Reconciler{Recorder: recorder} + ctx, logs := setupLogCapture(ctx) + + spec := pvcSpec("2Gi", "3Gi") + + cluster.Status = v1beta1.PostgresClusterStatus{ + InstanceSets: []v1beta1.PostgresInstanceSetStatus{{ + Name: "another-instance", + DesiredPGWALVolume: map[string]string{"elephant-another-instance-abcd-0": "3Gi"}, + }}} + + reconciler.setVolumeSize(ctx, &cluster, spec, "pgWAL", "another-instance") + + assert.Assert(t, cmp.MarshalMatches(spec, ` +accessModes: +- ReadWriteOnce +resources: + limits: + storage: 3Gi + requests: + storage: 3Gi +`)) + + assert.Equal(t, len(*logs), 0) + assert.Equal(t, len(recorder.Events), 1) + assert.Equal(t, recorder.Events[0].Regarding.Name, cluster.Name) + assert.Equal(t, recorder.Events[0].Reason, "VolumeLimitReached") + assert.Equal(t, recorder.Events[0].Note, "pgWAL volume(s) for elephant/another-instance are at size limit (3Gi).") + }) + }) } -func TestDetermineDesiredVolumeRequest(t *testing.T) { +func TestGetDesiredVolumeSize(t *testing.T) { t.Parallel() cluster := v1beta1.PostgresCluster{ @@ -568,6 +734,10 @@ func TestDetermineDesiredVolumeRequest(t *testing.T) { InstanceSets: []v1beta1.PostgresInstanceSetStatus{{ Name: "some-instance", DesiredPGDataVolume: desiredMap, + DesiredPGWALVolume: desiredMap, + }, { + Name: "another-instance", + DesiredPGDataVolume: desiredMap, }}, PGBackRest: &v1beta1.PGBackRestStatus{ Repos: []v1beta1.RepoStatus{{ @@ -622,6 +792,50 @@ func TestDetermineDesiredVolumeRequest(t *testing.T) { expected: "1Gi", expectedError: "quantities must match the regular expression", expectedDPV: "batman", + }, { + tcName: "pgwal-Larger size requested", + sizeFromStatus: "3Gi", + pvcRequestSize: "2Gi", + volType: "pgWAL", + host: "some-instance", + expected: "3Gi", + }, { + tcName: "pgwal-PVC is desired size", + sizeFromStatus: "2Gi", + pvcRequestSize: "2Gi", + volType: "pgWAL", + host: "some-instance", + expected: "2Gi", + }, { + tcName: "pgwal-Original larger than status request", + sizeFromStatus: "1Gi", + pvcRequestSize: "2Gi", + volType: "pgWAL", + host: "some-instance", + expected: "2Gi", + }, { + tcName: "pgwal-Instance doesn't exist", + sizeFromStatus: "2Gi", + pvcRequestSize: "1Gi", + volType: "pgWAL", + host: "not-an-instance", + expected: "1Gi", + }, { + tcName: "pgwal-Bad Value", + sizeFromStatus: "batman", + pvcRequestSize: "1Gi", + volType: "pgWAL", + host: "some-instance", + expected: "1Gi", + expectedError: "quantities must match the regular expression", + expectedDPV: "batman", + }, { + tcName: "pgwal-No value set for instance", + sizeFromStatus: "batman", + pvcRequestSize: "5Gi", + volType: "pgWAL", + host: "another-instance", + expected: "5Gi", }, { tcName: "repo1-Larger size requested", sizeFromStatus: "3Gi", diff --git a/internal/controller/postgrescluster/instance.go b/internal/controller/postgrescluster/instance.go index 835bfad8d2..1f5d171e81 100644 --- a/internal/controller/postgrescluster/instance.go +++ b/internal/controller/postgrescluster/instance.go @@ -317,11 +317,17 @@ func (r *Reconciler) observeInstances( // Save desired volume size values in case the status is removed. // This may happen in cases where the Pod is restarted, the cluster // is shutdown, etc. Only save values for instances defined in the spec. - previousDesiredRequests := make(map[string]string) + previousPGDataDesiredRequests := make(map[string]string) + previousPGWALDesiredRequests := make(map[string]string) if autogrow { for _, statusIS := range cluster.Status.InstanceSets { if statusIS.DesiredPGDataVolume != nil { - maps.Copy(previousDesiredRequests, statusIS.DesiredPGDataVolume) + maps.Copy(previousPGDataDesiredRequests, statusIS.DesiredPGDataVolume) + } + } + for _, statusIS := range cluster.Status.InstanceSets { + if statusIS.DesiredPGWALVolume != nil { + maps.Copy(previousPGWALDesiredRequests, statusIS.DesiredPGWALVolume) } } } @@ -331,6 +337,7 @@ func (r *Reconciler) observeInstances( for _, name := range sets.List(observed.setNames) { status := v1beta1.PostgresInstanceSetStatus{Name: name} status.DesiredPGDataVolume = make(map[string]string) + status.DesiredPGWALVolume = make(map[string]string) for _, instance := range observed.bySet[name] { //nolint:gosec // This slice is always small. @@ -343,15 +350,19 @@ func (r *Reconciler) observeInstances( status.UpdatedReplicas++ } if autogrow { - // Store desired pgData volume size for each instance Pod. - // The 'suggested-pgdata-pvc-size' annotation value is stored in the PostgresCluster - // status so that 1) it is available to the function 'reconcilePostgresDataVolume' - // and 2) so that the value persists after Pod restart and cluster shutdown events. + // Store desired pgData and pgWAL volume sizes for each instance Pod. + // The 'suggested-pgdata-pvc-size' and 'suggested-pgwal-pvc-size' annotation + // values are stored in the PostgresCluster status so that 1) they are available + // to the 'reconcilePostgresDataVolume' and 'reconcilePostgresWALVolume' functions + // and 2) so that the values persist after Pod restart and cluster shutdown events. for _, pod := range instance.Pods { // don't set an empty status if pod.Annotations["suggested-pgdata-pvc-size"] != "" { status.DesiredPGDataVolume[instance.Name] = pod.Annotations["suggested-pgdata-pvc-size"] } + if pod.Annotations["suggested-pgwal-pvc-size"] != "" { + status.DesiredPGWALVolume[instance.Name] = pod.Annotations["suggested-pgwal-pvc-size"] + } } } } @@ -363,7 +374,10 @@ func (r *Reconciler) observeInstances( if autogrow { for _, instance := range observed.bySet[name] { status.DesiredPGDataVolume[instance.Name] = r.storeDesiredRequest(ctx, cluster, "pgData", - name, status.DesiredPGDataVolume[instance.Name], previousDesiredRequests[instance.Name]) + name, status.DesiredPGDataVolume[instance.Name], previousPGDataDesiredRequests[instance.Name]) + + status.DesiredPGWALVolume[instance.Name] = r.storeDesiredRequest(ctx, cluster, "pgWAL", + name, status.DesiredPGWALVolume[instance.Name], previousPGWALDesiredRequests[instance.Name]) } } diff --git a/internal/controller/postgrescluster/pgbackrest_test.go b/internal/controller/postgrescluster/pgbackrest_test.go index fe7f6633c9..4591f1770f 100644 --- a/internal/controller/postgrescluster/pgbackrest_test.go +++ b/internal/controller/postgrescluster/pgbackrest_test.go @@ -29,6 +29,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/rand" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/manager" @@ -4430,8 +4431,9 @@ func TestGetRepoHostVolumeRequests(t *testing.T) { require.ParallelCapacity(t, 1) reconciler := &Reconciler{ - Client: tClient, - Owner: client.FieldOwner(t.Name()), + Client: tClient, + Owner: client.FieldOwner(t.Name()), + Recorder: new(record.FakeRecorder), } testCases := []struct { @@ -4505,11 +4507,41 @@ func TestGetRepoHostVolumeRequests(t *testing.T) { t.Cleanup(func() { assert.Check(t, tClient.Delete(ctx, repoHost)) }) } + // A limit is expected otherwise an empty string ("") is returned + testRepoPVC := func() *v1beta1.RepoPVC { + return &v1beta1.RepoPVC{ + VolumeClaimSpec: v1beta1.VolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + Resources: corev1.VolumeResourceRequirements{ + Limits: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceStorage: resource.MustParse("1Gi"), + }, + }}} + } + cluster := &v1beta1.PostgresCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "orange", Namespace: namespace, }, + Spec: v1beta1.PostgresClusterSpec{ + Backups: v1beta1.Backups{ + PGBackRest: v1beta1.PGBackRestArchive{ + Repos: []v1beta1.PGBackRestRepo{ + { + Name: "repo1", + Volume: testRepoPVC(), + }, { + Name: "repo2", + Volume: testRepoPVC(), + }, { + Name: "repo3", + Volume: testRepoPVC(), + }, { + Name: "repo4", + Volume: testRepoPVC(), + }, + }}}}, Status: v1beta1.PostgresClusterStatus{ PGBackRest: &v1beta1.PGBackRestStatus{}, }, diff --git a/internal/controller/postgrescluster/postgres.go b/internal/controller/postgrescluster/postgres.go index a10364b2d6..d1bcb86993 100644 --- a/internal/controller/postgrescluster/postgres.go +++ b/internal/controller/postgrescluster/postgres.go @@ -951,6 +951,12 @@ func (r *Reconciler) reconcilePostgresWALVolume( pvc.Spec = instanceSpec.WALVolumeClaimSpec.AsPersistentVolumeClaimSpec() + r.setVolumeSize(ctx, cluster, &pvc.Spec, "pgWAL", instanceSpec.Name) + + // Clear any set limit before applying PVC. This is needed to allow the limit + // value to change later. + pvc.Spec.Resources.Limits = nil + if err == nil { err = r.handlePersistentVolumeClaimError(cluster, errors.WithStack(r.apply(ctx, pvc))) diff --git a/internal/controller/postgrescluster/watches.go b/internal/controller/postgrescluster/watches.go index 2e063af280..06c421b033 100644 --- a/internal/controller/postgrescluster/watches.go +++ b/internal/controller/postgrescluster/watches.go @@ -64,6 +64,7 @@ func (*Reconciler) watchPods() handler.Funcs { // auto-grow volume resize annotations volumeAnnotations := []string{ "suggested-pgdata-pvc-size", + "suggested-pgwal-pvc-size", "suggested-repo1-pvc-size", "suggested-repo2-pvc-size", "suggested-repo3-pvc-size", diff --git a/internal/postgres/config.go b/internal/postgres/config.go index e7e7c163de..5513f88cef 100644 --- a/internal/postgres/config.go +++ b/internal/postgres/config.go @@ -330,6 +330,11 @@ while read -r -t 5 -u "${fd}" ||:; do # manage autogrow annotation for the pgData volume manageAutogrowAnnotation "pgdata" + + # manage autogrow annotation for the pgWAL volume, if it exists + if [[ -d /pgwal ]]; then + manageAutogrowAnnotation "pgwal" + fi done `, naming.CertMountPath, diff --git a/internal/postgres/reconcile.go b/internal/postgres/reconcile.go index bd191549eb..cf8b02e4b8 100644 --- a/internal/postgres/reconcile.go +++ b/internal/postgres/reconcile.go @@ -179,8 +179,7 @@ func InstancePod(ctx context.Context, Image: container.Image, ImagePullPolicy: container.ImagePullPolicy, SecurityContext: initialize.RestrictedSecurityContext(), - - VolumeMounts: []corev1.VolumeMount{certVolumeMount, dataVolumeMount}, + VolumeMounts: []corev1.VolumeMount{certVolumeMount, dataVolumeMount}, } if inInstanceSpec.Sidecars != nil && @@ -254,6 +253,7 @@ func InstancePod(ctx context.Context, container.VolumeMounts = append(container.VolumeMounts, walVolumeMount) startup.VolumeMounts = append(startup.VolumeMounts, walVolumeMount) + reloader.VolumeMounts = append(reloader.VolumeMounts, walVolumeMount) outInstancePod.Spec.Volumes = append(outInstancePod.Spec.Volumes, walVolume) } diff --git a/internal/postgres/reconcile_test.go b/internal/postgres/reconcile_test.go index 9ec45a96b1..5950ddafc9 100644 --- a/internal/postgres/reconcile_test.go +++ b/internal/postgres/reconcile_test.go @@ -210,6 +210,11 @@ containers: # manage autogrow annotation for the pgData volume manageAutogrowAnnotation "pgdata" + + # manage autogrow annotation for the pgWAL volume, if it exists + if [[ -d /pgwal ]]; then + manageAutogrowAnnotation "pgwal" + fi done }; export -f monitor; exec -a "$0" bash -ceu monitor - replication-cert-copy diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1/postgrescluster_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1/postgrescluster_types.go index e9778b93bb..f1873bdc7e 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1/postgrescluster_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1/postgrescluster_types.go @@ -592,6 +592,10 @@ type PostgresInstanceSetStatus struct { // Desired Size of the pgData volume // +optional DesiredPGDataVolume map[string]string `json:"desiredPGDataVolume,omitempty"` + + // Desired Size of the pgWAL volume + // +optional + DesiredPGWALVolume map[string]string `json:"desiredPGWALVolume,omitempty"` } // PostgresProxySpec is a union of the supported PostgreSQL proxies. diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1/zz_generated.deepcopy.go b/pkg/apis/postgres-operator.crunchydata.com/v1/zz_generated.deepcopy.go index b0916d8cce..20e65aee22 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1/zz_generated.deepcopy.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1/zz_generated.deepcopy.go @@ -570,6 +570,13 @@ func (in *PostgresInstanceSetStatus) DeepCopyInto(out *PostgresInstanceSetStatus (*out)[key] = val } } + if in.DesiredPGWALVolume != nil { + in, out := &in.DesiredPGWALVolume, &out.DesiredPGWALVolume + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PostgresInstanceSetStatus. 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 f053345c88..397b6d4f76 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go @@ -605,6 +605,10 @@ type PostgresInstanceSetStatus struct { // Desired Size of the pgData volume // +optional DesiredPGDataVolume map[string]string `json:"desiredPGDataVolume,omitempty"` + + // Desired Size of the pgWAL volume + // +optional + DesiredPGWALVolume map[string]string `json:"desiredPGWALVolume,omitempty"` } // PostgresProxySpec is a union of the supported PostgreSQL proxies. 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 8670e751a6..e75af532a2 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 @@ -2499,6 +2499,13 @@ func (in *PostgresInstanceSetStatus) DeepCopyInto(out *PostgresInstanceSetStatus (*out)[key] = val } } + if in.DesiredPGWALVolume != nil { + in, out := &in.DesiredPGWALVolume, &out.DesiredPGWALVolume + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PostgresInstanceSetStatus.