k8s metadata cache: delete shouldn't fail on NotFound errors #216
Conversation
|
PTAL @rootfs @mickymiek |
pkg/util/k8scmcache.go
Outdated
| @@ -172,6 +172,9 @@ func (k8scm *K8sCMCache) Get(identifier string, data interface{}) error { | |||
| func (k8scm *K8sCMCache) Delete(identifier string) error { | |||
| err := k8scm.Client.CoreV1().ConfigMaps(k8scm.Namespace).Delete(identifier, nil) | |||
| if err != nil { | |||
| if apierrs.IsNotFound(err) { | |||
| klog.V(4).Infof("k8s-cm-cache: assuming metadata configmap %s already deleted", identifier) | |||
There was a problem hiding this comment.
can we return nil to avoid printing log at line 180 klog.V(4).Infof("k8s-cm-cache: successfully deleted metadata configmap %s", identifier)
@gman0 I have one question.
if user manually deletes this entry from the config map by this changes we may end up in having stale volume at backend
There was a problem hiding this comment.
Absolutely, that's why it says it's only assuming
There was a problem hiding this comment.
And yes, I'll fix that
pkg/util/nodecache.go
Outdated
| @@ -152,7 +152,9 @@ func (nc *NodeCache) Delete(identifier string) error { | |||
| file := path.Join(nc.BasePath, cacheDir, identifier+".json") | |||
| err := os.Remove(file) | |||
| if err != nil { | |||
| if err != os.ErrNotExist { | |||
| if err == os.ErrNotExist { | |||
| klog.V(4).Infof("node-cache: assuming metadata storage file %s already deleted", file) | |||
gman0
force-pushed
the
cachepersister-idempotency-fix
branch
2 times, most recently
from
daeda2c to
144df2a
Compare
|
It should now return earlier ; also the log messages should be now a bit more descriptive. |
pkg/util/k8scmcache.go
Outdated
| @@ -128,7 +128,7 @@ func (k8scm *K8sCMCache) Create(identifier string, data interface{}) error { | |||
| } | |||
| dataJSON, err := json.Marshal(data) | |||
| if err != nil { | |||
| return errors.Wrap(err, "k8s-cm-cache: marshal error") | |||
| return errors.Wrapf(err, "k8s-cm-cache: JSON marshaling failed for configmap %s: %v", identifier, err) | |||
There was a problem hiding this comment.
Wrapf already wrap err with string i think we can skip adding err at last
simply it will become
errors.Wrapf(err, "k8s-cm-cache: JSON marshaling failed for configmap %s:",identifier)
gman0
force-pushed
the
cachepersister-idempotency-fix
branch
from
144df2a to
0235b9c
Compare
k8s metadata cache: delete shouldn't fail on NotFound errors
Syncing latest changes from devel for ceph-csi
In order to maintain idempotency when deleting k8s metadata entries, we may assume that non-existent entries are already deleted.