diff --git a/internal/controller/postgrescluster/autogrow.go b/internal/controller/postgrescluster/autogrow.go index 9572e0e4b..6abe380c0 100644 --- a/internal/controller/postgrescluster/autogrow.go +++ b/internal/controller/postgrescluster/autogrow.go @@ -57,17 +57,12 @@ func (r *Reconciler) storeDesiredRequest( // determine if the appropriate volume limit is set limitSet := limitIsSet(cluster, volumeType, host) - // if nil, volume does not exist - if limitSet == nil { + // if the limit is not set or the volume does not exist do not return the new value + if !limitSet { return "" } - // if the volume exists but the limit is not set, do not return the new value - if !*limitSet { - return desiredRequestBackup - } - - if *limitSet && current.Value() > previous.Value() { + 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) @@ -85,7 +80,7 @@ func (r *Reconciler) storeDesiredRequest( // limitIsSet determines if the limit is set for a given volume type and returns // either a corresponding boolean value or nil if the volume is not defined. -func limitIsSet(cluster *v1beta1.PostgresCluster, volumeType, instanceSetName string) *bool { +func limitIsSet(cluster *v1beta1.PostgresCluster, volumeType, instanceSetName string) bool { switch { @@ -94,19 +89,17 @@ func limitIsSet(cluster *v1beta1.PostgresCluster, volumeType, instanceSetName st case volumeType == "pgData": for _, specInstance := range cluster.Spec.InstanceSets { if specInstance.Name == instanceSetName { - limitSet := !specInstance.DataVolumeClaimSpec.Resources.Limits.Storage().IsZero() - return &limitSet + return !specInstance.DataVolumeClaimSpec.Resources.Limits.Storage().IsZero() } } 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 + return false } if specInstance.Name == instanceSetName && specInstance.WALVolumeClaimSpec != nil { - limitSet := !specInstance.WALVolumeClaimSpec.Resources.Limits.Storage().IsZero() - return &limitSet + return !specInstance.WALVolumeClaimSpec.Resources.Limits.Storage().IsZero() } } // VolumeType for the repository host volumes should be in the form 'repoN' @@ -116,16 +109,15 @@ func limitIsSet(cluster *v1beta1.PostgresCluster, volumeType, instanceSetName st for _, specRepo := range cluster.Spec.Backups.PGBackRest.Repos { // return nil if volume is not defined if specRepo.Name == volumeType && specRepo.Volume == nil { - return nil + return false } if specRepo.Name == volumeType && specRepo.Volume != nil { - limitSet := !specRepo.Volume.VolumeClaimSpec.Resources.Limits.Storage().IsZero() - return &limitSet + return !specRepo.Volume.VolumeClaimSpec.Resources.Limits.Storage().IsZero() } } } - return nil + return false } diff --git a/internal/controller/postgrescluster/autogrow_test.go b/internal/controller/postgrescluster/autogrow_test.go index a97e32f23..e276e60a1 100644 --- a/internal/controller/postgrescluster/autogrow_test.go +++ b/internal/controller/postgrescluster/autogrow_test.go @@ -291,74 +291,69 @@ func TestLimitIsSet(t *testing.T) { tcName string Voltype string instanceName string - expected *bool + expected bool }{{ tcName: "PGDATA Limit is set for defined volume", Voltype: "pgData", instanceName: "red", - expected: initialize.Pointer(true), + expected: true, }, { tcName: "PGDATA Limit is not set for defined volume", Voltype: "pgData", instanceName: "blue", - expected: initialize.Pointer(false), + expected: false, }, { tcName: "PGDATA Check volume for non-existent instance", Voltype: "pgData", instanceName: "orange", - expected: nil, + expected: false, }, { tcName: "PGWAL Limit is set for defined volume", Voltype: "pgWAL", instanceName: "red", - expected: initialize.Pointer(true), + expected: true, }, { tcName: "PGWAL WAL volume defined but limit is not set", Voltype: "pgWAL", instanceName: "green", - expected: initialize.Pointer(false), + expected: false, }, { tcName: "PGWAL Instance has no pg_wal volume defined", Voltype: "pgWAL", instanceName: "blue", - expected: nil, + expected: false, }, { tcName: "PGWAL Check volume for non-existent instance", Voltype: "pgWAL", instanceName: "orange", - expected: nil, + expected: false, }, { tcName: "REPO Limit set for defined volume", Voltype: "repo1", instanceName: "", - expected: initialize.Pointer(true), + expected: true, }, { tcName: "REPO Limit is not set for defined volume", Voltype: "repo2", instanceName: "", - expected: initialize.Pointer(false), + expected: false, }, { tcName: "REPO volume not defined for repo", Voltype: "repo3", instanceName: "", - expected: nil, + expected: false, }, { tcName: "REPO Check volume for non-existent repo", Voltype: "repo4", instanceName: "", - expected: nil, + expected: false, }} for _, tc := range testCases { t.Run(tc.tcName, func(t *testing.T) { limitSet := limitIsSet(&cluster, tc.Voltype, tc.instanceName) - if tc.expected == nil { - assert.Assert(t, limitSet == nil) - } else { - assert.Assert(t, limitSet != nil) - assert.Check(t, *limitSet == *tc.expected) - } + assert.Check(t, limitSet == tc.expected) }) } } diff --git a/internal/controller/postgrescluster/instance.go b/internal/controller/postgrescluster/instance.go index f07dfc6a3..8a699a680 100644 --- a/internal/controller/postgrescluster/instance.go +++ b/internal/controller/postgrescluster/instance.go @@ -373,7 +373,8 @@ func (r *Reconciler) observeInstances( // checked to ensure that the value from the annotations can be parsed to a valid // value. Otherwise the previous value, if available, will be used. If a limit is // not defined for the given volume and an empty string has been returned, nothing - // will be stored in the status. + // will be stored in the status. In the event that the value is empty, any existing + // request value will be removed. if autogrow { for _, instance := range observed.bySet[name] { if pgDataRequest := r.storeDesiredRequest( @@ -382,6 +383,8 @@ func (r *Reconciler) observeInstances( previousPGDataDesiredRequests[instance.Name], ); pgDataRequest != "" { status.DesiredPGDataVolume[instance.Name] = pgDataRequest + } else { + delete(status.DesiredPGDataVolume, instance.Name) } if pgWALRequest := r.storeDesiredRequest( @@ -390,6 +393,8 @@ func (r *Reconciler) observeInstances( previousPGWALDesiredRequests[instance.Name], ); pgWALRequest != "" { status.DesiredPGWALVolume[instance.Name] = pgWALRequest + } else { + delete(status.DesiredPGWALVolume, instance.Name) } } }