Merge pull request #116089 from cvvz/fix-116028

fix: should not set default storageclass if annotation "volume.beta.kubernetes.io/storage-class" is set
kubernetes · Mar 2, 2023 · 5157158 · 5157158
2 parents 1646ed8 + 4d6be42
commit 5157158
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 1 deletion.
diff --git a/pkg/controller/volume/persistentvolume/pv_controller.go b/pkg/controller/volume/persistentvolume/pv_controller.go
@@ -937,7 +937,7 @@ func (ctrl *PersistentVolumeController) updateVolumePhaseWithEvent(volume *v1.Pe
 // Ignores claims that already have a storage class.
 // TODO: if resync is ever changed to a larger period, we might need to change how we set the default class on existing unbound claims
 func (ctrl *PersistentVolumeController) assignDefaultStorageClass(claim *v1.PersistentVolumeClaim) (bool, error) {
-	if claim.Spec.StorageClassName != nil {
+	if storagehelpers.GetPersistentVolumeClaimClass(claim) != "" {
 		return false, nil
 	}
 

diff --git a/pkg/controller/volume/persistentvolume/pv_controller_test.go b/pkg/controller/volume/persistentvolume/pv_controller_test.go
@@ -862,6 +862,24 @@ func TestRetroactiveStorageClassAssignment(t *testing.T) {
 				},
 			},
 		},
+		{
+			storageClasses: []*storagev1.StorageClass{
+				makeDefaultStorageClass(classGold, &modeImmediate),
+				makeStorageClass(classCopper, &modeImmediate),
+			},
+			tests: []controllerTest{
+				{
+					name:            "15-7 - pvc storage class is not changed if claim is not bound but already set annotation \"volume.beta.kubernetes.io/storage-class\"",
+					initialVolumes:  novolumes,
+					expectedVolumes: novolumes,
+					initialClaims:   newClaimArray("claim15-7", "uid15-7", "1Gi", "", v1.ClaimPending, nil, v1.BetaStorageClassAnnotation),
+					expectedClaims:  newClaimArray("claim15-7", "uid15-7", "1Gi", "", v1.ClaimPending, nil, v1.BetaStorageClassAnnotation),
+					expectedEvents:  noevents,
+					errors:          noerrors,
+					test:            testSyncClaim,
+				},
+			},
+		},
 	}
 	for _, test := range tests {
 		runSyncTests(t, test.tests, test.storageClasses, nil)