diff --git a/pkg/controller/storage/capability/capability_controller.go b/pkg/controller/storage/capability/capability_controller.go index 27cc7b2ef..df0910ebd 100644 --- a/pkg/controller/storage/capability/capability_controller.go +++ b/pkg/controller/storage/capability/capability_controller.go @@ -21,13 +21,15 @@ package capability import ( "context" "fmt" + "reflect" "strconv" "time" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" storagev1 "k8s.io/api/storage/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" storageinformersv1 "k8s.io/client-go/informers/storage/v1" @@ -89,7 +91,8 @@ func NewController( }) csiDriverInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ - AddFunc: controller.enqueueStorageClassByCSI, + AddFunc: controller.enqueueStorageClassByCSI, + DeleteFunc: controller.enqueueStorageClassByCSI, }) return controller @@ -202,23 +205,25 @@ func (c *StorageCapabilityController) syncHandler(key string) error { // Get StorageClass storageClass, err := c.storageClassLister.Get(name) + if err != nil { + return err + } // Cloning and volumeSnapshot support only available for CSI drivers. isCSIStorage := c.hasCSIDriver(storageClass) - // Annotate storageClass storageClassUpdated := storageClass.DeepCopy() - err = c.addStorageClassSnapshotAnnotation(storageClassUpdated, isCSIStorage) - if err != nil { - return err + if isCSIStorage { + c.updateSnapshotAnnotation(storageClassUpdated, isCSIStorage) + c.updateCloneVolumeAnnotation(storageClassUpdated, isCSIStorage) + } else { + c.removeAnnotations(storageClassUpdated) } - err = c.addCloneVolumeAnnotation(storageClassUpdated, isCSIStorage) - if err != nil { - return err - } - _, err = c.storageClassClient.Update(context.Background(), storageClassUpdated, metav1.UpdateOptions{}) - if err != nil { - return err + if !reflect.DeepEqual(storageClass, storageClassUpdated) { + _, err = c.storageClassClient.Update(context.Background(), storageClassUpdated, metav1.UpdateOptions{}) + if err != nil { + return err + } } return nil } @@ -234,25 +239,27 @@ func (c *StorageCapabilityController) hasCSIDriver(storageClass *storagev1.Stora return false } -func (c *StorageCapabilityController) addStorageClassSnapshotAnnotation(storageClass *storagev1.StorageClass, snapshotAllow bool) error { +func (c *StorageCapabilityController) updateSnapshotAnnotation(storageClass *storagev1.StorageClass, snapshotAllow bool) { if storageClass.Annotations == nil { storageClass.Annotations = make(map[string]string) } - _, err := strconv.ParseBool(storageClass.Annotations[annotationAllowSnapshot]) - // err != nil means annotationAllowSnapshot is not illegal, include empty - if err != nil { + if _, err := strconv.ParseBool(storageClass.Annotations[annotationAllowSnapshot]); err != nil { storageClass.Annotations[annotationAllowSnapshot] = strconv.FormatBool(snapshotAllow) } - return nil + return } -func (c *StorageCapabilityController) addCloneVolumeAnnotation(storageClass *storagev1.StorageClass, cloneAllow bool) error { +func (c *StorageCapabilityController) updateCloneVolumeAnnotation(storageClass *storagev1.StorageClass, cloneAllow bool) { if storageClass.Annotations == nil { storageClass.Annotations = make(map[string]string) } - _, err := strconv.ParseBool(storageClass.Annotations[annotationAllowClone]) - if err != nil { + if _, err := strconv.ParseBool(storageClass.Annotations[annotationAllowClone]); err != nil { storageClass.Annotations[annotationAllowClone] = strconv.FormatBool(cloneAllow) } - return nil + return +} + +func (c *StorageCapabilityController) removeAnnotations(storageClass *storagev1.StorageClass) { + delete(storageClass.Annotations, annotationAllowClone) + delete(storageClass.Annotations, annotationAllowSnapshot) } diff --git a/pkg/controller/storage/capability/capability_controller_test.go b/pkg/controller/storage/capability/capability_controller_test.go index 22486e5e5..986cbad27 100644 --- a/pkg/controller/storage/capability/capability_controller_test.go +++ b/pkg/controller/storage/capability/capability_controller_test.go @@ -235,14 +235,13 @@ func TestCreateStorageClass(t *testing.T) { func TestStorageClassHadAnnotation(t *testing.T) { fixture := newFixture(t, true) storageClass := newStorageClass("csi-example", "csi.example.com") - storageClass.Annotations = map[string]string{annotationAllowSnapshot: "false", annotationAllowClone: "false"} + storageClass.Annotations = make(map[string]string) storageClassUpdate := storageClass.DeepCopy() - csiDriver := newCSIDriver("csi.example.com") + storageClass.Annotations = map[string]string{annotationAllowSnapshot: "false", annotationAllowClone: "false"} // Object exist fixture.storageObjects = append(fixture.storageObjects, storageClass) fixture.storageClassLister = append(fixture.storageClassLister, storageClass) - fixture.csiDriverLister = append(fixture.csiDriverLister, csiDriver) // Action expected fixture.expectUpdateStorageClassAction(storageClassUpdate) @@ -270,3 +269,22 @@ func TestStorageClassHadOneAnnotation(t *testing.T) { // Run test fixture.run(getKey(storageClass, t)) } + +func TestStorageClassHadNoCSIDriver(t *testing.T) { + fixture := newFixture(t, true) + storageClass := newStorageClass("csi-example", "csi.example.com") + storageClass.Annotations = map[string]string{} + storageClassUpdate := storageClass.DeepCopy() + storageClass.Annotations = map[string]string{annotationAllowSnapshot: "false"} + storageClass.Annotations = map[string]string{annotationAllowClone: "false"} + + // Object exist + fixture.storageObjects = append(fixture.storageObjects, storageClass) + fixture.storageClassLister = append(fixture.storageClassLister, storageClass) + + // Action expected + fixture.expectUpdateStorageClassAction(storageClassUpdate) + + // Run test + fixture.run(getKey(storageClass, t)) +}