Clear volume auto-grow statuses when limit not set

Issue: PGO-2654

Author: tjmoore4

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
 
 }
 

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)
 		})
 	}
 }

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)
 				}
 			}
 		}