cache: fix null pointer access when cachemanager.Disable is true #536
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #536 +/- ##
==========================================
- Coverage 33.56% 33.56% -0.01%
==========================================
Files 65 65
Lines 8175 8176 +1
==========================================
Hits 2744 2744
- Misses 5116 5117 +1
Partials 315 315
|
|
Why not check if the cacheMgr is nil? |
| @@ -37,6 +37,7 @@ type Manager struct { | |||
| } | |||
|
|
|||
| type Opt struct { | |||
| Disabled bool | |||
There was a problem hiding this comment.
Good catch! Maybe also update these config.toml:
- misc/snapshotter/config.toml
- tests/e2e/k8s/snapshotter-cri.yaml
- tests/e2e/k8s/snapshotter-kubeconf.yaml
There was a problem hiding this comment.
cache_manager.disable is still set to be false for those three files. I'm keeping the configuration item for compatibility though it doesn't take effect. Not sure whether cache_manager.disable will be used to control auto garbage collect in future or not.
There was a problem hiding this comment.
After a second thought, I think we can support recyclebin to keep deleted files for a while. Those configuration item can be used for it.
There was a problem hiding this comment.
Introducing recyclebin seems to add complexity and doesn't feel necessary, it is better to let kubelet & containerd control the GC policy.
With the evolution of nydus-snapshotter, |
Filesystem.cacheMgr is null when cacheManager.Disable is configured to be true, thus cause null pointer access. Fix it by always create the cacheMgr object no matter cacheManager.Disable is true or false. Signed-off-by: Jiang Liu <[email protected]>
Filesystem.cacheMgr is null when cacheManager.Disable is configured to be true, thus cause null pointer access. Fix it by always create the cacheMgr object no matter cacheManager.Disable is true or false.