From d41c206f54b8c5f027ed0bac9f13efeae3b8065a Mon Sep 17 00:00:00 2001 From: TJ Moore Date: Mon, 25 Aug 2025 13:33:54 -0400 Subject: [PATCH 1/3] Add repo host volume tests to TestStoreDesiredRequest Update test and refactor to support different volume types. --- .../postgrescluster/autogrow_test.go | 193 +++++++++++------- 1 file changed, 115 insertions(+), 78 deletions(-) diff --git a/internal/controller/postgrescluster/autogrow_test.go b/internal/controller/postgrescluster/autogrow_test.go index 51fbfaab9e..4a37a27ae8 100644 --- a/internal/controller/postgrescluster/autogrow_test.go +++ b/internal/controller/postgrescluster/autogrow_test.go @@ -54,88 +54,125 @@ func TestStoreDesiredRequest(t *testing.T) { }, { Name: "blue", 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", + }}}, + }, + }, + } - 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: "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-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) { From 1e391ed8eddb305e4efaf6beb65944059fca2028 Mon Sep 17 00:00:00 2001 From: TJ Moore Date: Thu, 28 Aug 2025 17:59:30 -0400 Subject: [PATCH 2/3] Add Disk Auto Grow for pgWAL volumes This update adds the ability to automatically grow pg_wal PVCs. The AutoGrowVolumes feature gate must be enabled and the relevant volumes must include a Limit value. Once enabled, this feature tracks the current disk utilization and, when utilization reaches 75%, the disk request is updated to 150% of the observed value. At this point and beyond, the requested value will be tracked by CPK. The volume request can grow up to the configured limit value. Note: This change now treats limit values as authoritative regardless of the feature gate setting. However, the implementation also now allows limits to be updated after being set. Issue: PGO-1428 --- .golangci.yaml | 4 - ...ator.crunchydata.com_postgresclusters.yaml | 10 + .../controller/postgrescluster/autogrow.go | 60 ++++- .../postgrescluster/autogrow_test.go | 217 ++++++++++++++++-- .../controller/postgrescluster/instance.go | 28 ++- .../controller/postgrescluster/postgres.go | 6 + .../controller/postgrescluster/watches.go | 1 + internal/postgres/config.go | 5 + internal/postgres/reconcile.go | 4 +- internal/postgres/reconcile_test.go | 5 + .../v1/postgrescluster_types.go | 4 + .../v1/zz_generated.deepcopy.go | 7 + .../v1beta1/postgrescluster_types.go | 4 + .../v1beta1/zz_generated.deepcopy.go | 7 + 14 files changed, 317 insertions(+), 45 deletions(-) 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 4a37a27ae8..3e75df6326 100644 --- a/internal/controller/postgrescluster/autogrow_test.go +++ b/internal/controller/postgrescluster/autogrow_test.go @@ -51,8 +51,18 @@ 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), }}, Backups: v1beta1.Backups{ @@ -68,7 +78,10 @@ func TestStoreDesiredRequest(t *testing.T) { }}}, }, }, { - Name: "repo2", + Name: "repo2", + Volume: &v1beta1.RepoPVC{}, + }, { + Name: "repo3", }}}, }, }, @@ -114,6 +127,40 @@ func TestStoreDesiredRequest(t *testing.T) { 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", @@ -131,6 +178,11 @@ func TestStoreDesiredRequest(t *testing.T) { 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", @@ -192,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{ @@ -210,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: "Limit is not set for repo1 volume", + tcName: "PGWAL Check volume for non-existent instance", + Voltype: "pgWAL", + instanceName: "orange", + expected: nil, + }, { + 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) + } }) } } @@ -545,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} @@ -579,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{ @@ -605,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{{ @@ -659,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/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. From 6831c78a6a2bd2143dbe7dadc7b1abb62ab424cf Mon Sep 17 00:00:00 2001 From: TJ Moore Date: Tue, 2 Sep 2025 17:04:56 -0400 Subject: [PATCH 3/3] Update TestGetRepoHostVolumeRequests Autogrow statuses for volumes that no longer exist are now cleared. This commit updates the underlying PostgresCluster used for testing to conform to the new behavior. The test previously used partially constructed objects that wouldn't exist in a real cluster. --- .../postgrescluster/pgbackrest_test.go | 36 +++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) 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{}, },