feature: support creating container by just specifying rootfs #1474
Conversation
daemon/mgr/container.go
Outdated
| } | ||
|
|
||
| func (mgr *ContainerManager) ensureRootFSMounted(rootfs string, snapData map[string]string) error { | ||
| if rootfs == "" || snapData == nil { |
There was a problem hiding this comment.
If it is better to use len(snapData) == 0? @HusterWan
Since snapData has possibility to be map[string]string{}.
Codecov Report
@@ Coverage Diff @@
## master #1474 +/- ##
==========================================
- Coverage 41.29% 41.17% -0.13%
==========================================
Files 257 258 +1
Lines 16988 17038 +50
==========================================
Hits 7016 7016
- Misses 9100 9145 +45
- Partials 872 877 +5
|
ctrd/container_types.go
Outdated
| IO *containerio.IO | ||
| Spec *specs.Spec | ||
| BaseFS string | ||
| Takeover bool |
There was a problem hiding this comment.
More description for the whole Container struct. In addition, I wish that we could use CtrdContainer instead of Container, since we have already so many Container struct in pouchd.
Currently, with the change, this struct will handle two kinds of container, the first one is the original one, and the second is the ones from Moby and taken over by Pouch. This info is quite important to be added in the comments of this struct.
Also, every field needs comments.
There was a problem hiding this comment.
CtrdContainer will cause glint error: it recommands to use name Container :)
i will add more details description.
|
ci/circleci: markdownlint-misspell-shellcheck — Your tests failed on CircleCI CI fails @HusterWan |
ctrd/container_types.go
Outdated
| BaseFS string | ||
| // Takeover specify if the containerd container is just | ||
| // taken over by pouch, not created by pouch | ||
| Takeover bool |
There was a problem hiding this comment.
Please change this field. @HusterWan
There was a problem hiding this comment.
i think we can merge this pr now, and i will create a new pr to support create container by specify rootfs, WDTY?
|
Please update this pull request soon, and we need to try merging this fast. @HusterWan |
|
Updated @allencloud |
HusterWan
force-pushed
the
master
branch
3 times, most recently
from
4a8b538 to
41464fa
Compare
HusterWan
changed the title
HusterWan
changed the title
Signed-off-by: Michael Wan <[email protected]>
|
LGTM |
pouchrobot
added
the
LGTM
4 participants
Signed-off-by: Michael Wan [email protected]
Ⅰ. Describe what this PR did
Actually, the containerd support creating a container by just specifying the container rootfs, the feature is very useful when we want to take over the container created by other container hypervisor.
in order to distinguish container that created by specifying rootfs or snapshot, we add a flag
RootFSProvided, when the flag set, the container is created by just specifying rootfs.this feature is also needed by upgrade docker to PouchContainer, we must support taking over containers created by docker
we also should consider the abnormal situations like host crashed etc. so when starting a
RootFSProvidedcontainer , we should check if therootfshas mounted, if not, we must mount it.Ⅱ. Does this pull request fix one issue?
Ⅲ. Describe how you did it
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews