Preserve conditions in case we are retrying expansion in some cases

When marking pvc expansion for failed condition, we should try and
preserve old resizing conditions with different name.

pkg/volume/util/resize_util.go

@@ -142,7 +142,7 @@ func MarkResizeInProgressWithResizer(
 	}
 	conditions := []v1.PersistentVolumeClaimCondition{progressCondition}
 	newPVC := pvc.DeepCopy()
-	newPVC = MergeResizeConditionOnPVC(newPVC, conditions)
+	newPVC = MergeResizeConditionOnPVC(newPVC, conditions, false /* keepOldResizeConditions */)
 	newPVC = setResizer(newPVC, resizerName)
 	return PatchPVCStatus(pvc /*oldPVC*/, newPVC, kubeClient)
 }
@@ -156,7 +156,7 @@ func MarkControllerReisizeInProgress(pvc *v1.PersistentVolumeClaim, resizerName
 	}
 	conditions := []v1.PersistentVolumeClaimCondition{progressCondition}
 	newPVC := pvc.DeepCopy()
-	newPVC = MergeResizeConditionOnPVC(newPVC, conditions)
+	newPVC = MergeResizeConditionOnPVC(newPVC, conditions, false /* keepOldResizeConditions */)
 	newPVC = mergeStorageResourceStatus(newPVC, v1.PersistentVolumeClaimControllerResizeInProgress)
 	newPVC = mergeStorageAllocatedResources(newPVC, newSize)
 	newPVC = setResizer(newPVC, resizerName)
@@ -198,7 +198,7 @@ func MarkForFSResize(
 		newPVC = mergeStorageResourceStatus(newPVC, v1.PersistentVolumeClaimNodeResizePending)
 	}
 
-	newPVC = MergeResizeConditionOnPVC(newPVC, conditions)
+	newPVC = MergeResizeConditionOnPVC(newPVC, conditions, true /* keepOldResizeConditions */)
 	updatedPVC, err := PatchPVCStatus(pvc /*oldPVC*/, newPVC, kubeClient)
 	return updatedPVC, err
 }
@@ -231,7 +231,7 @@ func MarkFSResizeFinished(
 		}
 	}
 
-	newPVC = MergeResizeConditionOnPVC(newPVC, []v1.PersistentVolumeClaimCondition{})
+	newPVC = MergeResizeConditionOnPVC(newPVC, []v1.PersistentVolumeClaimCondition{}, false /* keepOldResizeConditions */)
 	updatedPVC, err := PatchPVCStatus(pvc /*oldPVC*/, newPVC, kubeClient)
 	return updatedPVC, err
 }
@@ -247,7 +247,9 @@ func MarkNodeExpansionInfeasible(pvc *v1.PersistentVolumeClaim, kubeClient clien
 		LastTransitionTime: metav1.Now(),
 		Message:            fmt.Sprintf("failed to expand pvc with %v", err),
 	}
-	newPVC = MergeResizeConditionOnPVC(newPVC, []v1.PersistentVolumeClaimCondition{errorCondition})
+	newPVC = MergeResizeConditionOnPVC(newPVC,
+		[]v1.PersistentVolumeClaimCondition{errorCondition},
+		true /* keepOldResizeConditions */)
 
 	patchBytes, err := createPVCPatch(pvc, newPVC, false /* addResourceVersionCheck */)
 	if err != nil {
@@ -270,16 +272,18 @@ func MarkNodeExpansionFailedCondition(pvc *v1.PersistentVolumeClaim, kubeClient
 		LastTransitionTime: metav1.Now(),
 		Message:            fmt.Sprintf("failed to expand pvc with %v", err),
 	}
-	newPVC = MergeResizeConditionOnPVC(newPVC, []v1.PersistentVolumeClaimCondition{errorCondition})
+	newPVC = MergeResizeConditionOnPVC(newPVC,
+		[]v1.PersistentVolumeClaimCondition{errorCondition},
+		true /* keepOldResizeConditions */)
 	patchBytes, err := createPVCPatch(pvc, newPVC, false /* addResourceVersionCheck */)
 	if err != nil {
-		return pvc, fmt.Errorf("patchPVCStatus failed to patch PVC %q: %v", pvc.Name, err)
+		return pvc, fmt.Errorf("patchPVCStatus failed to patch PVC %q: %w", pvc.Name, err)
 	}
 
 	updatedClaim, updateErr := kubeClient.CoreV1().PersistentVolumeClaims(pvc.Namespace).
 		Patch(context.TODO(), pvc.Name, types.StrategicMergePatchType, patchBytes, metav1.PatchOptions{}, "status")
 	if updateErr != nil {
-		return pvc, fmt.Errorf("patchPVCStatus failed to patch PVC %q: %v", pvc.Name, updateErr)
+		return pvc, fmt.Errorf("patchPVCStatus failed to patch PVC %q: %w", pvc.Name, updateErr)
 	}
 	return updatedClaim, nil
 }
@@ -364,7 +368,7 @@ func addResourceVersion(patchBytes []byte, resourceVersion string) ([]byte, erro
 // leaving other conditions untouched.
 func MergeResizeConditionOnPVC(
 	pvc *v1.PersistentVolumeClaim,
-	resizeConditions []v1.PersistentVolumeClaimCondition) *v1.PersistentVolumeClaim {
+	resizeConditions []v1.PersistentVolumeClaimCondition, keepOldResizeConditions bool) *v1.PersistentVolumeClaim {
 	resizeConditionMap := map[v1.PersistentVolumeClaimConditionType]*resizeProcessStatus{}
 
 	for _, condition := range resizeConditions {
@@ -387,6 +391,10 @@ func MergeResizeConditionOnPVC(
 				newConditions = append(newConditions, condition)
 			}
 			newCondition.processed = true
+		} else if keepOldResizeConditions {
+			// if keepOldResizeConditions is true, we keep the old resize conditions that were present in the
+			// existing pvc.Status.Conditions field.
+			newConditions = append(newConditions, condition)
 		}
 	}
 

pkg/volume/util/resize_util_test.go

@@ -35,6 +35,7 @@ import (
 type conditionMergeTestCase struct {
 	description             string
 	pvc                     *v1.PersistentVolumeClaim
+	keepOldResizeConditions bool
 	newConditions           []v1.PersistentVolumeClaimCondition
 	finalConditions         []v1.PersistentVolumeClaimCondition
 }
@@ -132,10 +133,34 @@ func TestMergeResizeCondition(t *testing.T) {
 				},
 			},
 		},
+		{
+			description:             "when adding new condition with existing resize conditions",
+			pvc:                     pvc.DeepCopy(),
+			keepOldResizeConditions: true,
+			newConditions: []v1.PersistentVolumeClaimCondition{
+				{
+					Type:               v1.PersistentVolumeClaimNodeResizeError,
+					Status:             v1.ConditionTrue,
+					LastTransitionTime: currentTime,
+				},
+			},
+			finalConditions: []v1.PersistentVolumeClaimCondition{
+				{
+					Type:               v1.PersistentVolumeClaimResizing,
+					Status:             v1.ConditionTrue,
+					LastTransitionTime: currentTime,
+				},
+				{
+					Type:               v1.PersistentVolumeClaimNodeResizeError,
+					Status:             v1.ConditionTrue,
+					LastTransitionTime: currentTime,
+				},
+			},
+		},
 	}
 
 	for _, testcase := range testCases {
-		updatePVC := MergeResizeConditionOnPVC(testcase.pvc, testcase.newConditions)
+		updatePVC := MergeResizeConditionOnPVC(testcase.pvc, testcase.newConditions, testcase.keepOldResizeConditions)
 
 		updateConditions := updatePVC.Status.Conditions
 		if !reflect.DeepEqual(updateConditions, testcase.finalConditions) {