From e5dbea15d34afc00b771b56750aeacfe515f4923 Mon Sep 17 00:00:00 2001 From: gman Date: Mon, 25 Feb 2019 18:05:20 +0100 Subject: [PATCH 1/3] util/cachepersister: check and return CacheEntryNotFound error in Get() --- pkg/util/cachepersister.go | 10 ++++++++-- pkg/util/k8scmcache.go | 4 ++++ pkg/util/nodecache.go | 4 ++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/pkg/util/cachepersister.go b/pkg/util/cachepersister.go index e72c28f16..ba8918587 100644 --- a/pkg/util/cachepersister.go +++ b/pkg/util/cachepersister.go @@ -23,13 +23,19 @@ import ( ) const ( - //PluginFolder defines location of plugins + // PluginFolder defines location of plugins PluginFolder = "/var/lib/kubelet/plugins" ) -// ForAllFunc stores metadata with identifier +// ForAllFunc is a unary predicate for visiting all cache entries +// matching the `pattern' in CachePersister's ForAll function. type ForAllFunc func(identifier string) error +// CacheEntryNotFound is an error type for "Not Found" cache errors +type CacheEntryNotFound struct { + error +} + // CachePersister interface implemented for store type CachePersister interface { Create(identifier string, data interface{}) error diff --git a/pkg/util/k8scmcache.go b/pkg/util/k8scmcache.go index 9ba1f9de5..10a6d7ef5 100644 --- a/pkg/util/k8scmcache.go +++ b/pkg/util/k8scmcache.go @@ -159,6 +159,10 @@ func (k8scm *K8sCMCache) Create(identifier string, data interface{}) error { func (k8scm *K8sCMCache) Get(identifier string, data interface{}) error { cm, err := k8scm.getMetadataCM(identifier) if err != nil { + if apierrs.IsNotFound(err) { + return &CacheEntryNotFound{err} + } + return err } err = json.Unmarshal([]byte(cm.Data[cmDataKey]), data) diff --git a/pkg/util/nodecache.go b/pkg/util/nodecache.go index 510ffa246..5659d4eaa 100644 --- a/pkg/util/nodecache.go +++ b/pkg/util/nodecache.go @@ -130,6 +130,10 @@ func (nc *NodeCache) Get(identifier string, data interface{}) error { // #nosec fp, err := os.Open(file) if err != nil { + if os.IsNotExist(errors.Cause(err)) { + return &CacheEntryNotFound{err} + } + return errors.Wrapf(err, "node-cache: open error for %s", file) } From ce3affcc6a16ef8e3a5aaebc689c035f28c9a6e8 Mon Sep 17 00:00:00 2001 From: gman Date: Mon, 25 Feb 2019 18:07:28 +0100 Subject: [PATCH 2/3] cephfs: DeleteVolume should assume the volume to be already deleted if metadata doesn't exist --- pkg/cephfs/controllerserver.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/pkg/cephfs/controllerserver.go b/pkg/cephfs/controllerserver.go index 235a99f3a..67a342040 100644 --- a/pkg/cephfs/controllerserver.go +++ b/pkg/cephfs/controllerserver.go @@ -97,8 +97,9 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol }, nil } -// DeleteVolume deletes the volume in backend and removes the volume metadata -// from store +// DeleteVolume deletes the volume in backend +// and removes the volume metadata from store +// nolint: gocyclo func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest) (*csi.DeleteVolumeResponse, error) { if err := cs.validateDeleteVolumeRequest(); err != nil { klog.Errorf("DeleteVolumeRequest validation failed: %v", err) @@ -108,11 +109,15 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol var ( volID = volumeID(req.GetVolumeId()) secrets = req.GetSecrets() - err error ) ce := &controllerCacheEntry{} - if err = cs.MetadataStore.Get(string(volID), ce); err != nil { + if err := cs.MetadataStore.Get(string(volID), ce); err != nil { + if err, ok := err.(*util.CacheEntryNotFound); ok { + klog.Infof("cephfs: metadata for volume %s not found, assuming the volume to be already deleted (%v)", volID, err) + return &csi.DeleteVolumeResponse{}, nil + } + return nil, status.Error(codes.Internal, err.Error()) } From d12fdfd40039ac4c1c5094fdb1a63e4db039f6eb Mon Sep 17 00:00:00 2001 From: gman Date: Mon, 25 Feb 2019 18:09:21 +0100 Subject: [PATCH 3/3] rbd: fixed metadata idempotency in DeleteVolume; DeleteSnapshot should assume the snapshot to be already deleted if metadata doesn't exist --- pkg/rbd/controllerserver.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/rbd/controllerserver.go b/pkg/rbd/controllerserver.go index b1eddf29e..69b237693 100644 --- a/pkg/rbd/controllerserver.go +++ b/pkg/rbd/controllerserver.go @@ -18,7 +18,6 @@ package rbd import ( "fmt" - "os" "os/exec" "syscall" @@ -29,7 +28,6 @@ import ( "github.com/kubernetes-csi/csi-lib-utils/protosanitizer" "github.com/kubernetes-csi/drivers/pkg/csi-common" "github.com/pborman/uuid" - "github.com/pkg/errors" "golang.org/x/net/context" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -252,9 +250,11 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol rbdVol := &rbdVolume{} if err := cs.MetadataStore.Get(volumeID, rbdVol); err != nil { - if os.IsNotExist(errors.Cause(err)) { + if err, ok := err.(*util.CacheEntryNotFound); ok { + klog.V(3).Infof("metadata for volume %s not found, assuming the volume to be already deleted (%v)", volumeID, err) return &csi.DeleteVolumeResponse{}, nil } + return nil, err } @@ -471,6 +471,11 @@ func (cs *ControllerServer) DeleteSnapshot(ctx context.Context, req *csi.DeleteS rbdSnap := &rbdSnapshot{} if err := cs.MetadataStore.Get(snapshotID, rbdSnap); err != nil { + if err, ok := err.(*util.CacheEntryNotFound); ok { + klog.V(3).Infof("metadata for snapshot %s not found, assuming the snapshot to be already deleted (%v)", snapshotID, err) + return &csi.DeleteSnapshotResponse{}, nil + } + return nil, err }