Snapshot overlay view #532
Conversation
Codecov Report
@@ Coverage Diff @@
## master #532 +/- ##
==========================================
- Coverage 62.12% 61.59% -0.54%
==========================================
Files 3 3
Lines 404 440 +36
==========================================
+ Hits 251 271 +20
- Misses 93 102 +9
- Partials 60 67 +7
Continue to review full report at Codecov.
|
snapshot/overlay/overlay_test.go
Outdated
| t.Errorf("expected source %q but received %q", expected, m.Source) | ||
| } | ||
| if m.Options[0] != "ro" { | ||
| t.Errorf("expected mount option rw but received %q", m.Options[0]) |
There was a problem hiding this comment.
nit: typo - "expected mount option ro but received.."
There was a problem hiding this comment.
nice catch, thanks! updated
| @@ -174,7 +185,7 @@ func (o *Snapshotter) Walk(fn func(snapshot.Info) error) error { | |||
| }) | |||
| } | |||
|
|
|||
| func (o *Snapshotter) newActiveDir(key string) (*activeDir, error) { | |||
| func (o *Snapshotter) newActiveDir(key string, readonly bool) (*activeDir, error) { | |||
There was a problem hiding this comment.
This extra argument is only used in two places and is hard-coded in each. Can this logic be handled in the caller instead of adding a bool arg?
There was a problem hiding this comment.
This okay here, since we've split the callers into Prepare and View.
| @@ -236,6 +254,13 @@ func (a *activeDir) setParent(name string) error { | |||
| } | |||
|
|
|||
| func (a *activeDir) commit(name string, c *cache) error { | |||
| if _, err := os.Stat(filepath.Join(a.path, "fs")); err != nil { | |||
| if os.IsNotExist(err) { | |||
| return errors.New("cannot commit view") | |||
There was a problem hiding this comment.
My anticipation is that this could be replaced with an error defined in the snapshot package as I think this would be an error any snapshot implementation would return (trying to commit a view, an error described in the function interface definition)
41a6c1b to
4edd969
Compare
snapshot/overlay/overlay.go
Outdated
| @@ -114,7 +114,18 @@ func (o *Snapshotter) Prepare(key, parent string) ([]containerd.Mount, error) { | |||
| } | |||
|
|
|||
| func (o *Snapshotter) View(key, parent string) ([]containerd.Mount, error) { | |||
| panic("not implemented") | |||
| if parent == "" { | |||
There was a problem hiding this comment.
Do we not allow viewing an empty directory?
We should define that boundary condition.
There was a problem hiding this comment.
I don't think it makes much sense to view an empty directory. The callers will need to check but I think it makes more sense for the caller to handle it than have the snapshot drivers create an empty directory just to create a ro bind mount to it.
There was a problem hiding this comment.
The definition of the empty string is the empty directory. I see no reason to create this special case and arbitrary restriction.
|
LGTM |
Allow creating actives without an upper directory for capturing changes. Actives without the upper directory will always be mounted read only. Read only actives must have a parent. Signed-off-by: Derek McGowan <derek@mcgstyle.net> (github: dmcgowan)
Signed-off-by: Derek McGowan <derek@mcgstyle.net> (github: dmcgowan)
Signed-off-by: Derek McGowan <derek@mcgstyle.net> (github: dmcgowan)
4edd969 to
eeb8855
Compare
|
Rebased, removed unnecessary restriction on View having a parent |
Signed-off-by: Derek McGowan <derek@mcgstyle.net> (github: dmcgowan)
Implements view function for overlay. This creates an active directory in overlay with no upper or work directory. Mounts without these directories will be treated as readonly in overlay.
For reference
https://www.kernel.org/doc/Documentation/filesystems/overlayfs.txt