From 60cd523a0f8a3e78bc392285c2b1702d60db4f42 Mon Sep 17 00:00:00 2001 From: f10atin9 Date: Tue, 28 Sep 2021 10:29:26 +0800 Subject: [PATCH] [fix] fix update logic Now controller will judge whether the storageClassClient need to send the update request. Signed-off-by: f10atin9 --- .../capability/capability_controller.go | 48 +++++++++---------- .../capability/capability_controller_test.go | 5 +- 2 files changed, 25 insertions(+), 28 deletions(-) diff --git a/pkg/controller/storage/capability/capability_controller.go b/pkg/controller/storage/capability/capability_controller.go index 4c5b209ab..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,27 +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() - if !isCSIStorage { - c.removeAnnotations(storageClassUpdated) + if isCSIStorage { + c.updateSnapshotAnnotation(storageClassUpdated, isCSIStorage) + c.updateCloneVolumeAnnotation(storageClassUpdated, isCSIStorage) } else { - err = c.updateStorageClassSnapshotAnnotation(storageClassUpdated, isCSIStorage) - if err != nil { - return err - } - err = c.updateCloneVolumeAnnotation(storageClassUpdated, isCSIStorage) - if err != nil { - return err - } + c.removeAnnotations(storageClassUpdated) } - _, 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 } @@ -238,27 +239,24 @@ func (c *StorageCapabilityController) hasCSIDriver(storageClass *storagev1.Stora return false } -func (c *StorageCapabilityController) updateStorageClassSnapshotAnnotation(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) updateCloneVolumeAnnotation(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) { diff --git a/pkg/controller/storage/capability/capability_controller_test.go b/pkg/controller/storage/capability/capability_controller_test.go index 7890d5d2f..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)