HELP-85556 - fix PVC scaling ignoring namespaces

changelog/20251217_fix_persistent_volume_claim_resize.md

@@ -0,0 +1,6 @@
+---
+kind: fix
+date: 2025-12-17
+---
+
+* **Persistent Volume Claim resize fix**: Fixed an issue where the Operator ignored namespaces when listing PVCs, causing conflicts with resizing PVCs of the same name. Now, PVCs are filtered by both name and namespace for accurate resizing.

controllers/operator/create/create.go

@@ -123,25 +123,26 @@ func HandlePVCResize(ctx context.Context, memberClient kubernetesClient.Client,
 	// and we are not in the middle of a resize (that means pvcPhase is pvc.PhaseNoAction) for this statefulset.
 	// This means we want to start one
 	if increaseStorageOfAtLeastOnePVC {
-		err := enterprisests.AddPVCAnnotation(desiredSts)
-		if err != nil {
+		if err := enterprisests.AddPVCAnnotation(desiredSts); err != nil {
 			return workflow.Failed(xerrors.Errorf("can't add pvc annotation, err: %s", err))
 		}
+
 		log.Infof("Detected PVC size expansion; patching all pvcs and increasing the size for sts: %s", desiredSts.Name)
-		if err := resizePVCsStorage(memberClient, desiredSts); err != nil {
+		if err := resizePVCsStorage(ctx, memberClient, desiredSts, log); err != nil {
 			return workflow.Failed(xerrors.Errorf("can't resize pvc, err: %s", err))
 		}
 
 		finishedResizing, err := hasFinishedResizing(ctx, memberClient, desiredSts)
 		if err != nil {
 			return workflow.Failed(err)
 		}
+
 		if finishedResizing {
 			log.Info("PVCs finished resizing")
 			log.Info("Deleting StatefulSet and orphan pods")
 			// Cascade delete the StatefulSet
 			deletePolicy := metav1.DeletePropagationOrphan
-			if err := memberClient.Delete(context.TODO(), desiredSts, client.PropagationPolicy(deletePolicy)); err != nil && !apiErrors.IsNotFound(err) {
+			if err := memberClient.Delete(ctx, desiredSts, client.PropagationPolicy(deletePolicy)); err != nil && !apiErrors.IsNotFound(err) {
 				return workflow.Failed(xerrors.Errorf("error deleting sts, err: %s", err))
 			}
 
@@ -182,7 +183,7 @@ func checkStatefulsetIsDeleted(ctx context.Context, memberClient kubernetesClien
 
 func hasFinishedResizing(ctx context.Context, memberClient kubernetesClient.Client, desiredSts *appsv1.StatefulSet) (bool, error) {
 	pvcList := corev1.PersistentVolumeClaimList{}
-	if err := memberClient.List(ctx, &pvcList); err != nil {
+	if err := memberClient.List(ctx, &pvcList, client.InNamespace(desiredSts.Namespace)); err != nil {
 		return false, err
 	}
 
@@ -198,20 +199,23 @@ func hasFinishedResizing(ctx context.Context, memberClient kubernetesClient.Clie
 }
 
 // resizePVCsStorage takes the sts we want to create and update all matching pvc with the new storage
-func resizePVCsStorage(client kubernetesClient.Client, statefulSetToCreate *appsv1.StatefulSet) error {
-	pvcList := corev1.PersistentVolumeClaimList{}
-
+func resizePVCsStorage(ctx context.Context, kubeClient kubernetesClient.Client, statefulSetToCreate *appsv1.StatefulSet, log *zap.SugaredLogger) error {
 	// this is to ensure that requests to a potentially not allowed resource is not blocking the operator until the end
-	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
+	ctx, cancel := context.WithTimeout(ctx, 2*time.Minute)
 	defer cancel()
 
-	if err := client.List(ctx, &pvcList); err != nil {
+	pvcList := corev1.PersistentVolumeClaimList{}
+	if err := kubeClient.List(ctx, &pvcList, client.InNamespace(statefulSetToCreate.Namespace)); err != nil {
 		return err
 	}
+
 	for _, existingPVC := range pvcList.Items {
 		if template, _ := getMatchingPVCTemplateFromSTS(statefulSetToCreate, &existingPVC); template != nil {
-			existingPVC.Spec.Resources.Requests[corev1.ResourceStorage] = *template.Spec.Resources.Requests.Storage()
-			if err := client.Update(ctx, &existingPVC); err != nil {
+			currentSize := existingPVC.Spec.Resources.Requests[corev1.ResourceStorage]
+			targetSize := *template.Spec.Resources.Requests.Storage()
+			log.Infof("Resizing PVC %s/%s from %s to %s", existingPVC.GetNamespace(), existingPVC.GetName(), currentSize.String(), targetSize.String())
+			existingPVC.Spec.Resources.Requests[corev1.ResourceStorage] = targetSize
+			if err := kubeClient.Update(ctx, &existingPVC); err != nil {
 				return err
 			}
 		}

controllers/operator/create/create_test.go

@@ -949,45 +949,86 @@ func createMongosSts(ctx context.Context, t *testing.T, mdb *mdbv1.MongoDB, log
 func TestResizePVCsStorage(t *testing.T) {
 	fakeClient, _ := mock.NewDefaultFakeClient()
 
-	initialSts := createStatefulSet("20Gi", "20Gi", "20Gi")
+	const testStsName = "test"
+	const testStsNamespace = "mongodb-test"
+	initialSts := createStatefulSet(testStsName, testStsNamespace, "20Gi", "20Gi", "20Gi")
 
 	// Create the StatefulSet that we want to resize the PVC to
 	err := fakeClient.CreateStatefulSet(context.TODO(), *initialSts)
 	assert.NoError(t, err)
 
 	for _, template := range initialSts.Spec.VolumeClaimTemplates {
 		for i := range *initialSts.Spec.Replicas {
-			pvc := createPVCFromTemplate(template, initialSts.Name, i)
+			pvc := createPVCFromTemplate(template, initialSts.Name, initialSts.Namespace, i)
 			err = fakeClient.Create(context.TODO(), pvc)
 			assert.NoError(t, err)
 		}
 	}
 
-	err = resizePVCsStorage(fakeClient, createStatefulSet("30Gi", "30Gi", "20Gi"))
+	// PVCs from different STS (same name, but different namespace) should be ignored and not resized
+	// Previously, we had not taken into account namespace when listing PVCs https://jira.mongodb.org/browse/HELP-85556
+	const otherTestStsNamespace = "mongodb-test-2"
+	otherSts := createStatefulSet(testStsName, otherTestStsNamespace, "25Gi", "20Gi", "15Gi")
+
+	// Create the StatefulSet that we want to resize the PVC to
+	err = fakeClient.CreateStatefulSet(context.TODO(), *otherSts)
+	assert.NoError(t, err)
+
+	for _, template := range otherSts.Spec.VolumeClaimTemplates {
+		for i := range *otherSts.Spec.Replicas {
+			pvc := createPVCFromTemplate(template, otherSts.Name, otherSts.Namespace, i)
+			err = fakeClient.Create(context.TODO(), pvc)
+			assert.NoError(t, err)
+		}
+	}
+
+	// We are resizing only initialSts PVCs here and otherSts PVCs should remain unchanged
+	err = resizePVCsStorage(context.TODO(), fakeClient, createStatefulSet(testStsName, testStsNamespace, "30Gi", "30Gi", "20Gi"), zap.S())
 	assert.NoError(t, err)
 
 	pvcList := corev1.PersistentVolumeClaimList{}
 	err = fakeClient.List(context.TODO(), &pvcList)
 	assert.NoError(t, err)
 
+	expectedPVCSizesPerNamespace := map[string]map[string]string{
+		// PVCs from the STS namespace that was resized should have the new sizes
+		testStsNamespace: {
+			"data":    "30Gi",
+			"journal": "30Gi",
+			"logs":    "20Gi",
+		},
+		// PVCs from other namespace should remain unchanged
+		otherTestStsNamespace: {
+			"data":    "25Gi",
+			"journal": "20Gi",
+			"logs":    "15Gi",
+		},
+	}
+
 	for _, pvc := range pvcList.Items {
+		pvcSizes, ok := expectedPVCSizesPerNamespace[pvc.Namespace]
+		if !ok {
+			t.Fatalf("unexpected namespace %s for pvc %s", pvc.Namespace, pvc.Name)
+		}
+
 		if strings.HasPrefix(pvc.Name, "data") {
-			assert.Equal(t, pvc.Spec.Resources.Requests.Storage().String(), "30Gi")
+			assert.Equal(t, pvc.Spec.Resources.Requests.Storage().String(), pvcSizes["data"])
 		} else if strings.HasPrefix(pvc.Name, "journal") {
-			assert.Equal(t, pvc.Spec.Resources.Requests.Storage().String(), "30Gi")
+			assert.Equal(t, pvc.Spec.Resources.Requests.Storage().String(), pvcSizes["journal"])
 		} else if strings.HasPrefix(pvc.Name, "logs") {
-			assert.Equal(t, pvc.Spec.Resources.Requests.Storage().String(), "20Gi")
+			assert.Equal(t, pvc.Spec.Resources.Requests.Storage().String(), pvcSizes["logs"])
 		} else {
 			t.Fatal("no pvc was compared while we should have at least detected and compared one")
 		}
 	}
 }
 
 // Helper function to create a StatefulSet
-func createStatefulSet(size1, size2, size3 string) *appsv1.StatefulSet {
+func createStatefulSet(name, namespace, size1, size2, size3 string) *appsv1.StatefulSet {
 	return &appsv1.StatefulSet{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "test",
+			Name:      name,
+			Namespace: namespace,
 		},
 		Spec: appsv1.StatefulSetSpec{
 			Replicas: ptr.To(int32(3)),
@@ -1033,12 +1074,12 @@ func createStatefulSet(size1, size2, size3 string) *appsv1.StatefulSet {
 	}
 }
 
-func createPVCFromTemplate(pvcTemplate corev1.PersistentVolumeClaim, stsName string, ordinal int32) *corev1.PersistentVolumeClaim {
+func createPVCFromTemplate(pvcTemplate corev1.PersistentVolumeClaim, stsName string, namespace string, ordinal int32) *corev1.PersistentVolumeClaim {
 	pvcName := fmt.Sprintf("%s-%s-%d", pvcTemplate.Name, stsName, ordinal)
 	return &corev1.PersistentVolumeClaim{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      pvcName,
-			Namespace: "default",
+			Namespace: namespace,
 		},
 		Spec: pvcTemplate.Spec,
 	}
@@ -1147,9 +1188,11 @@ func TestResourceStorageHasChanged(t *testing.T) {
 }
 
 func TestHasFinishedResizing(t *testing.T) {
-	stsName := "test"
-	desiredSts := &appsv1.StatefulSet{
-		ObjectMeta: metav1.ObjectMeta{Name: stsName},
+	sts := &appsv1.StatefulSet{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test",
+			Namespace: "mongodb-test",
+		},
 		Spec: appsv1.StatefulSetSpec{
 			VolumeClaimTemplates: []corev1.PersistentVolumeClaim{
 				{
@@ -1184,40 +1227,44 @@ func TestHasFinishedResizing(t *testing.T) {
 	{
 		fakeClient, _ := mock.NewDefaultFakeClient()
 		// Scenario 1: All PVCs have finished resizing
-		pvc1 := createPVCWithCapacity("data-"+stsName+"-0", "20Gi")
-		pvc2 := createPVCWithCapacity("logs-"+stsName+"-0", "30Gi")
-		notPartOfSts := createPVCWithCapacity("random-sts-0", "30Gi")
+		pvc1 := createPVCWithCapacity("data-"+sts.Name+"-0", sts.Namespace, "20Gi")
+		pvc2 := createPVCWithCapacity("logs-"+sts.Name+"-0", sts.Namespace, "30Gi")
+		// PVCs in different namespace, but same name, should be ignored and not taken into account when checking resizing status
+		pvc1InDifferentNamespace := createPVCWithCapacity("data-"+sts.Name+"-0", "mongodb-test-2", "15Gi")
+		pvc2InDifferentNamespace := createPVCWithCapacity("logs-"+sts.Name+"-0", "mongodb-test-2", "10Gi")
 		err := fakeClient.Create(ctx, pvc1)
 		assert.NoError(t, err)
 		err = fakeClient.Create(ctx, pvc2)
 		assert.NoError(t, err)
-		err = fakeClient.Create(ctx, notPartOfSts)
+		err = fakeClient.Create(ctx, pvc1InDifferentNamespace)
+		assert.NoError(t, err)
+		err = fakeClient.Create(ctx, pvc2InDifferentNamespace)
 		assert.NoError(t, err)
 
-		finished, err := hasFinishedResizing(ctx, fakeClient, desiredSts)
+		finished, err := hasFinishedResizing(ctx, fakeClient, sts)
 		assert.NoError(t, err)
 		assert.True(t, finished, "PVCs should be finished resizing")
 	}
 
 	{
 		// Scenario 2: Some PVCs are still resizing
 		fakeClient, _ := mock.NewDefaultFakeClient()
-		pvc2Incomplete := createPVCWithCapacity("logs-"+stsName+"-0", "10Gi")
+		pvc2Incomplete := createPVCWithCapacity("logs-"+sts.Name+"-0", sts.Namespace, "10Gi")
 		err := fakeClient.Create(ctx, pvc2Incomplete)
 		assert.NoError(t, err)
 
-		finished, err := hasFinishedResizing(ctx, fakeClient, desiredSts)
+		finished, err := hasFinishedResizing(ctx, fakeClient, sts)
 		assert.NoError(t, err)
 		assert.False(t, finished, "PVCs should not be finished resizing")
 	}
 }
 
 // Helper function to create a PVC with a specific capacity and status
-func createPVCWithCapacity(name string, capacity string) *corev1.PersistentVolumeClaim {
+func createPVCWithCapacity(name string, namespace string, capacity string) *corev1.PersistentVolumeClaim {
 	return &corev1.PersistentVolumeClaim{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
-			Namespace: "default",
+			Namespace: namespace,
 		},
 		Spec: corev1.PersistentVolumeClaimSpec{
 			Resources: corev1.VolumeResourceRequirements{