drivers/overlay: Notify the user if the backing filesystem is not supported with OverlayFS #1374
Conversation
aarontomlin
force-pushed
the
atomlin-overlayfs-give-up-when-back-end-fs-is-unsupported
branch
from
2185716 to
1f72cb4
Compare
Done. |
drivers/overlay/overlay.go
Outdated
| if fsName, ok := graphdriver.FsNames[fsMagic]; ok { | ||
| if fsName, ok := graphdriver.FsNames[fsMagic]; !ok { | ||
| return nil, errors.Wrapf(graphdriver.ErrIncompatibleFS, "filesystem type %#x reported for %s is not supported with 'overlay'", fsMagic, filepath.Dir(home)) | ||
| } else { |
There was a problem hiding this comment.
Just remove the else { } block
and only have
backingFs=fsName
Should fix the lint complaint.
drivers/overlay/overlay.go
Outdated
| @@ -309,7 +310,9 @@ func Init(home string, options graphdriver.Options) (graphdriver.Driver, error) | |||
| if err != nil { | |||
| return nil, err | |||
| } | |||
| if fsName, ok := graphdriver.FsNames[fsMagic]; ok { | |||
| if fsName, ok := graphdriver.FsNames[fsMagic]; !ok { | |||
| return nil, errors.Wrapf(graphdriver.ErrIncompatibleFS, "filesystem type %#x reported for %s is not supported with 'overlay'", fsMagic, filepath.Dir(home)) | |||
There was a problem hiding this comment.
Could you also use
fmt.Erorrf("filesystem type %#x reported for %s is not supported with 'overlay': %w", fsMagic, filepath.Dir(home), graphdriver.ErrIncompatibleFS)
aarontomlin
force-pushed
the
atomlin-overlayfs-give-up-when-back-end-fs-is-unsupported
branch
2 times, most recently
from
2f32b4b to
6e4927e
Compare
aarontomlin
requested review from
rhatdan and
giuseppe
and removed request for
rhatdan and
giuseppe
|
LGTM |
| } | ||
| backingFs = fsName |
There was a problem hiding this comment.
Please move this to an else block, since fsName does not exists outside of the scope.
There was a problem hiding this comment.
I modified the code so lint will be happy.
aarontomlin
force-pushed
the
atomlin-overlayfs-give-up-when-back-end-fs-is-unsupported
branch
from
e9a4ec1 to
722edb1
Compare
…ported with OverlayFS If the backing filesystem is not supported with OverlayFS as a container storage driver/or graph driver then do not continue. In this context, report the incompatibility by notifying the user of an unsupported configuration. The filesystem type/or ID that is returned by the statfs(2) system call is reported too, to assist further troubleshooting efforts. It can be used to deduce the actual filesystem used for the container storage graph directory. Signed-off-by: Aaron Tomlin <[email protected]>
aarontomlin
force-pushed
the
atomlin-overlayfs-give-up-when-back-end-fs-is-unsupported
branch
from
722edb1 to
d9da335
Compare
so I disagree with my assessment from some months ago. I think it is better if we revert, or do not make it a failure as it is hiding valid use cases: #1546 |
The overlay driver previously raised an error when encountering an unsupported filesystem. This commit changes the error message to a debug log, allowing the overlay driver to continue its operation even with unsupported filesystems, without causing a failure. [NO NEW TESTS NEEDED] Introduced-by: containers#1374 Closes: containers#1546 Signed-off-by: Giuseppe Scrivano <[email protected]>
I believe the failure condition is reasonable, however I recommend adding a flag at a minimum to ignore the check and let users own the tool failing by using some |
|
no there is no need to fail just because we don't know the file system. At most, we can fail/raise warning if we know something won't work correctly |
If the backing filesystem is not supported with OverlayFS as a container storage driver/or graph driver then do not continue. In this context, report the incompatibility by notifying the user of an unsupported configuration. The filesystem type/or ID that is returned by the statfs(2) system call is reported too, to assist further troubleshooting efforts. It can be used to deduce the actual filesystem used for the container storage graph directory.
Signed-off-by: Aaron Tomlin [email protected]