Skip to content

Commit 59b9ccf

Browse files
MaciejKarasnammn
andauthored
HELP-85556 - fix PVC scaling ignoring namespaces (#651)
# Summary We are ignoring namespaces when listing PVCs to resize and if customer has multiple resources named exactly the same, but in different namespaces, we will try to resize them all. This fix adds namespace filtering in `resizePVCsStorage` + more accurate logging when actual resize is requested. Additionally fixed issues with missing context propagation. These fixes are commented in the PR. ## Proof of Work Updated unit tests that verify `resizePVCsStorage` logic. ## Checklist - [x] Have you linked a jira ticket and/or is the ticket in the title? - [x] Have you checked whether your jira ticket required DOCSP changes? - [x] Have you added changelog file? - use `skip-changelog` label if not needed - refer to [Changelog files and Release Notes](https://github.com/mongodb/mongodb-kubernetes/blob/master/CONTRIBUTING.md#changelog-files-and-release-notes) section in CONTRIBUTING.md for more details --------- Co-authored-by: Nam Nguyen <nam.nguyen@mongodb.com>
1 parent 2352446 commit 59b9ccf

File tree

3 files changed

+91
-34
lines changed

3 files changed

+91
-34
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
kind: fix
3+
date: 2025-12-17
4+
---
5+
6+
* **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

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -123,25 +123,26 @@ func HandlePVCResize(ctx context.Context, memberClient kubernetesClient.Client,
123123
// and we are not in the middle of a resize (that means pvcPhase is pvc.PhaseNoAction) for this statefulset.
124124
// This means we want to start one
125125
if increaseStorageOfAtLeastOnePVC {
126-
err := enterprisests.AddPVCAnnotation(desiredSts)
127-
if err != nil {
126+
if err := enterprisests.AddPVCAnnotation(desiredSts); err != nil {
128127
return workflow.Failed(xerrors.Errorf("can't add pvc annotation, err: %s", err))
129128
}
129+
130130
log.Infof("Detected PVC size expansion; patching all pvcs and increasing the size for sts: %s", desiredSts.Name)
131-
if err := resizePVCsStorage(memberClient, desiredSts); err != nil {
131+
if err := resizePVCsStorage(ctx, memberClient, desiredSts, log); err != nil {
132132
return workflow.Failed(xerrors.Errorf("can't resize pvc, err: %s", err))
133133
}
134134

135135
finishedResizing, err := hasFinishedResizing(ctx, memberClient, desiredSts)
136136
if err != nil {
137137
return workflow.Failed(err)
138138
}
139+
139140
if finishedResizing {
140141
log.Info("PVCs finished resizing")
141142
log.Info("Deleting StatefulSet and orphan pods")
142143
// Cascade delete the StatefulSet
143144
deletePolicy := metav1.DeletePropagationOrphan
144-
if err := memberClient.Delete(context.TODO(), desiredSts, client.PropagationPolicy(deletePolicy)); err != nil && !apiErrors.IsNotFound(err) {
145+
if err := memberClient.Delete(ctx, desiredSts, client.PropagationPolicy(deletePolicy)); err != nil && !apiErrors.IsNotFound(err) {
145146
return workflow.Failed(xerrors.Errorf("error deleting sts, err: %s", err))
146147
}
147148

@@ -182,7 +183,7 @@ func checkStatefulsetIsDeleted(ctx context.Context, memberClient kubernetesClien
182183

183184
func hasFinishedResizing(ctx context.Context, memberClient kubernetesClient.Client, desiredSts *appsv1.StatefulSet) (bool, error) {
184185
pvcList := corev1.PersistentVolumeClaimList{}
185-
if err := memberClient.List(ctx, &pvcList); err != nil {
186+
if err := memberClient.List(ctx, &pvcList, client.InNamespace(desiredSts.Namespace)); err != nil {
186187
return false, err
187188
}
188189

@@ -198,20 +199,23 @@ func hasFinishedResizing(ctx context.Context, memberClient kubernetesClient.Clie
198199
}
199200

200201
// resizePVCsStorage takes the sts we want to create and update all matching pvc with the new storage
201-
func resizePVCsStorage(client kubernetesClient.Client, statefulSetToCreate *appsv1.StatefulSet) error {
202-
pvcList := corev1.PersistentVolumeClaimList{}
203-
202+
func resizePVCsStorage(ctx context.Context, kubeClient kubernetesClient.Client, statefulSetToCreate *appsv1.StatefulSet, log *zap.SugaredLogger) error {
204203
// this is to ensure that requests to a potentially not allowed resource is not blocking the operator until the end
205-
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
204+
ctx, cancel := context.WithTimeout(ctx, 2*time.Minute)
206205
defer cancel()
207206

208-
if err := client.List(ctx, &pvcList); err != nil {
207+
pvcList := corev1.PersistentVolumeClaimList{}
208+
if err := kubeClient.List(ctx, &pvcList, client.InNamespace(statefulSetToCreate.Namespace)); err != nil {
209209
return err
210210
}
211+
211212
for _, existingPVC := range pvcList.Items {
212213
if template, _ := getMatchingPVCTemplateFromSTS(statefulSetToCreate, &existingPVC); template != nil {
213-
existingPVC.Spec.Resources.Requests[corev1.ResourceStorage] = *template.Spec.Resources.Requests.Storage()
214-
if err := client.Update(ctx, &existingPVC); err != nil {
214+
currentSize := existingPVC.Spec.Resources.Requests[corev1.ResourceStorage]
215+
targetSize := *template.Spec.Resources.Requests.Storage()
216+
log.Infof("Resizing PVC %s/%s from %s to %s", existingPVC.GetNamespace(), existingPVC.GetName(), currentSize.String(), targetSize.String())
217+
existingPVC.Spec.Resources.Requests[corev1.ResourceStorage] = targetSize
218+
if err := kubeClient.Update(ctx, &existingPVC); err != nil {
215219
return err
216220
}
217221
}

controllers/operator/create/create_test.go

Lines changed: 69 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -949,45 +949,86 @@ func createMongosSts(ctx context.Context, t *testing.T, mdb *mdbv1.MongoDB, log
949949
func TestResizePVCsStorage(t *testing.T) {
950950
fakeClient, _ := mock.NewDefaultFakeClient()
951951

952-
initialSts := createStatefulSet("20Gi", "20Gi", "20Gi")
952+
const testStsName = "test"
953+
const testStsNamespace = "mongodb-test"
954+
initialSts := createStatefulSet(testStsName, testStsNamespace, "20Gi", "20Gi", "20Gi")
953955

954956
// Create the StatefulSet that we want to resize the PVC to
955957
err := fakeClient.CreateStatefulSet(context.TODO(), *initialSts)
956958
assert.NoError(t, err)
957959

958960
for _, template := range initialSts.Spec.VolumeClaimTemplates {
959961
for i := range *initialSts.Spec.Replicas {
960-
pvc := createPVCFromTemplate(template, initialSts.Name, i)
962+
pvc := createPVCFromTemplate(template, initialSts.Name, initialSts.Namespace, i)
961963
err = fakeClient.Create(context.TODO(), pvc)
962964
assert.NoError(t, err)
963965
}
964966
}
965967

966-
err = resizePVCsStorage(fakeClient, createStatefulSet("30Gi", "30Gi", "20Gi"))
968+
// PVCs from different STS (same name, but different namespace) should be ignored and not resized
969+
// Previously, we had not taken into account namespace when listing PVCs https://jira.mongodb.org/browse/HELP-85556
970+
const otherTestStsNamespace = "mongodb-test-2"
971+
otherSts := createStatefulSet(testStsName, otherTestStsNamespace, "25Gi", "20Gi", "15Gi")
972+
973+
// Create the StatefulSet that we want to resize the PVC to
974+
err = fakeClient.CreateStatefulSet(context.TODO(), *otherSts)
975+
assert.NoError(t, err)
976+
977+
for _, template := range otherSts.Spec.VolumeClaimTemplates {
978+
for i := range *otherSts.Spec.Replicas {
979+
pvc := createPVCFromTemplate(template, otherSts.Name, otherSts.Namespace, i)
980+
err = fakeClient.Create(context.TODO(), pvc)
981+
assert.NoError(t, err)
982+
}
983+
}
984+
985+
// We are resizing only initialSts PVCs here and otherSts PVCs should remain unchanged
986+
err = resizePVCsStorage(context.TODO(), fakeClient, createStatefulSet(testStsName, testStsNamespace, "30Gi", "30Gi", "20Gi"), zap.S())
967987
assert.NoError(t, err)
968988

969989
pvcList := corev1.PersistentVolumeClaimList{}
970990
err = fakeClient.List(context.TODO(), &pvcList)
971991
assert.NoError(t, err)
972992

993+
expectedPVCSizesPerNamespace := map[string]map[string]string{
994+
// PVCs from the STS namespace that was resized should have the new sizes
995+
testStsNamespace: {
996+
"data": "30Gi",
997+
"journal": "30Gi",
998+
"logs": "20Gi",
999+
},
1000+
// PVCs from other namespace should remain unchanged
1001+
otherTestStsNamespace: {
1002+
"data": "25Gi",
1003+
"journal": "20Gi",
1004+
"logs": "15Gi",
1005+
},
1006+
}
1007+
9731008
for _, pvc := range pvcList.Items {
1009+
pvcSizes, ok := expectedPVCSizesPerNamespace[pvc.Namespace]
1010+
if !ok {
1011+
t.Fatalf("unexpected namespace %s for pvc %s", pvc.Namespace, pvc.Name)
1012+
}
1013+
9741014
if strings.HasPrefix(pvc.Name, "data") {
975-
assert.Equal(t, pvc.Spec.Resources.Requests.Storage().String(), "30Gi")
1015+
assert.Equal(t, pvc.Spec.Resources.Requests.Storage().String(), pvcSizes["data"])
9761016
} else if strings.HasPrefix(pvc.Name, "journal") {
977-
assert.Equal(t, pvc.Spec.Resources.Requests.Storage().String(), "30Gi")
1017+
assert.Equal(t, pvc.Spec.Resources.Requests.Storage().String(), pvcSizes["journal"])
9781018
} else if strings.HasPrefix(pvc.Name, "logs") {
979-
assert.Equal(t, pvc.Spec.Resources.Requests.Storage().String(), "20Gi")
1019+
assert.Equal(t, pvc.Spec.Resources.Requests.Storage().String(), pvcSizes["logs"])
9801020
} else {
9811021
t.Fatal("no pvc was compared while we should have at least detected and compared one")
9821022
}
9831023
}
9841024
}
9851025

9861026
// Helper function to create a StatefulSet
987-
func createStatefulSet(size1, size2, size3 string) *appsv1.StatefulSet {
1027+
func createStatefulSet(name, namespace, size1, size2, size3 string) *appsv1.StatefulSet {
9881028
return &appsv1.StatefulSet{
9891029
ObjectMeta: metav1.ObjectMeta{
990-
Name: "test",
1030+
Name: name,
1031+
Namespace: namespace,
9911032
},
9921033
Spec: appsv1.StatefulSetSpec{
9931034
Replicas: ptr.To(int32(3)),
@@ -1033,12 +1074,12 @@ func createStatefulSet(size1, size2, size3 string) *appsv1.StatefulSet {
10331074
}
10341075
}
10351076

1036-
func createPVCFromTemplate(pvcTemplate corev1.PersistentVolumeClaim, stsName string, ordinal int32) *corev1.PersistentVolumeClaim {
1077+
func createPVCFromTemplate(pvcTemplate corev1.PersistentVolumeClaim, stsName string, namespace string, ordinal int32) *corev1.PersistentVolumeClaim {
10371078
pvcName := fmt.Sprintf("%s-%s-%d", pvcTemplate.Name, stsName, ordinal)
10381079
return &corev1.PersistentVolumeClaim{
10391080
ObjectMeta: metav1.ObjectMeta{
10401081
Name: pvcName,
1041-
Namespace: "default",
1082+
Namespace: namespace,
10421083
},
10431084
Spec: pvcTemplate.Spec,
10441085
}
@@ -1147,9 +1188,11 @@ func TestResourceStorageHasChanged(t *testing.T) {
11471188
}
11481189

11491190
func TestHasFinishedResizing(t *testing.T) {
1150-
stsName := "test"
1151-
desiredSts := &appsv1.StatefulSet{
1152-
ObjectMeta: metav1.ObjectMeta{Name: stsName},
1191+
sts := &appsv1.StatefulSet{
1192+
ObjectMeta: metav1.ObjectMeta{
1193+
Name: "test",
1194+
Namespace: "mongodb-test",
1195+
},
11531196
Spec: appsv1.StatefulSetSpec{
11541197
VolumeClaimTemplates: []corev1.PersistentVolumeClaim{
11551198
{
@@ -1184,40 +1227,44 @@ func TestHasFinishedResizing(t *testing.T) {
11841227
{
11851228
fakeClient, _ := mock.NewDefaultFakeClient()
11861229
// Scenario 1: All PVCs have finished resizing
1187-
pvc1 := createPVCWithCapacity("data-"+stsName+"-0", "20Gi")
1188-
pvc2 := createPVCWithCapacity("logs-"+stsName+"-0", "30Gi")
1189-
notPartOfSts := createPVCWithCapacity("random-sts-0", "30Gi")
1230+
pvc1 := createPVCWithCapacity("data-"+sts.Name+"-0", sts.Namespace, "20Gi")
1231+
pvc2 := createPVCWithCapacity("logs-"+sts.Name+"-0", sts.Namespace, "30Gi")
1232+
// PVCs in different namespace, but same name, should be ignored and not taken into account when checking resizing status
1233+
pvc1InDifferentNamespace := createPVCWithCapacity("data-"+sts.Name+"-0", "mongodb-test-2", "15Gi")
1234+
pvc2InDifferentNamespace := createPVCWithCapacity("logs-"+sts.Name+"-0", "mongodb-test-2", "10Gi")
11901235
err := fakeClient.Create(ctx, pvc1)
11911236
assert.NoError(t, err)
11921237
err = fakeClient.Create(ctx, pvc2)
11931238
assert.NoError(t, err)
1194-
err = fakeClient.Create(ctx, notPartOfSts)
1239+
err = fakeClient.Create(ctx, pvc1InDifferentNamespace)
1240+
assert.NoError(t, err)
1241+
err = fakeClient.Create(ctx, pvc2InDifferentNamespace)
11951242
assert.NoError(t, err)
11961243

1197-
finished, err := hasFinishedResizing(ctx, fakeClient, desiredSts)
1244+
finished, err := hasFinishedResizing(ctx, fakeClient, sts)
11981245
assert.NoError(t, err)
11991246
assert.True(t, finished, "PVCs should be finished resizing")
12001247
}
12011248

12021249
{
12031250
// Scenario 2: Some PVCs are still resizing
12041251
fakeClient, _ := mock.NewDefaultFakeClient()
1205-
pvc2Incomplete := createPVCWithCapacity("logs-"+stsName+"-0", "10Gi")
1252+
pvc2Incomplete := createPVCWithCapacity("logs-"+sts.Name+"-0", sts.Namespace, "10Gi")
12061253
err := fakeClient.Create(ctx, pvc2Incomplete)
12071254
assert.NoError(t, err)
12081255

1209-
finished, err := hasFinishedResizing(ctx, fakeClient, desiredSts)
1256+
finished, err := hasFinishedResizing(ctx, fakeClient, sts)
12101257
assert.NoError(t, err)
12111258
assert.False(t, finished, "PVCs should not be finished resizing")
12121259
}
12131260
}
12141261

12151262
// Helper function to create a PVC with a specific capacity and status
1216-
func createPVCWithCapacity(name string, capacity string) *corev1.PersistentVolumeClaim {
1263+
func createPVCWithCapacity(name string, namespace string, capacity string) *corev1.PersistentVolumeClaim {
12171264
return &corev1.PersistentVolumeClaim{
12181265
ObjectMeta: metav1.ObjectMeta{
12191266
Name: name,
1220-
Namespace: "default",
1267+
Namespace: namespace,
12211268
},
12221269
Spec: corev1.PersistentVolumeClaimSpec{
12231270
Resources: corev1.VolumeResourceRequirements{

0 commit comments

Comments
 (0)