-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add the EROFS rootfs support #9486
Add the EROFS rootfs support #9486
Conversation
6692c76
to
a11a914
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the patch! I really like the idea of getting rid of the gofer process when no gofer mounts exist. Before, this was not possible because rootfs was always a gofer mount. So gofer was always needed. But EROFS changes that.
Before I review further, could you rebase this on master. e77deec is a relevant change that was submitted yesterday. Specifically see changes around GoferMountConf.ShouldUseLisafs()
.
a11a914
to
95397d2
Compare
@ayushr2 I just pushed a new version. In this version, I did some refactors to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
396d0be and 78a667c look good to me. Regarding 95397d22973675bb235c0819b36b067d85d7ea30, did you consider adding a test in runsc/container/container_test.go instead? You can easily set annotations there. It is easier to maintain tests there. Separately, I think that the test should come with the functionality (hence squash the test into the feature commit).
I might take some more time to chew the third commit, in the meantime you could make a separate PR with the checkpoint restore work and we can submit that. The rootfs support work is orthogonal.
Makes sense, I will see how to add a test in runsc/container/container_test.go instead and will squash the test into the feature commit.
Sure. I will make a separate PR with the checkpoint/restore work. The third commit does require and deserve more time for review, and thanks for taking time on it! PS. Currently the rootfs support patch has a minimal dependency on the |
95397d2
to
51df621
Compare
51df621
to
35cdafd
Compare
Can you rebase? |
Sure, will do. Thanks! |
35cdafd
to
05371ac
Compare
8ad8854
to
59d61fa
Compare
86f1534
to
e699051
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! This is taking good shape.
4b4ffa6
to
11cdf38
Compare
This patch decouples GoferMountConf to two layers to allow us to configure all combinations of a gofer mount in a succinct way: - Upper layer config: none, memory, self, anon. The upper layer is always tmpfs. It describes the backend for tmpfs. - Lower layer config: none, lisafs. It describes the backend for the filesystem which actually holds the image contents. The old SelfTmpfs will be represented as "upper=self,lower=none", MemoryOverlay will be "upper=memory,lower=lisafs", SelfOverlay will be "upper=self,lower=lisafs", and so on. Thanks to @ayushr2 for the suggestion on how to better decouple this. This is a preparation for adding the EROFS rootfs support. There is no functional change intended. Signed-off-by: Tiwei Bie <tiwei.btw@antgroup.com>
11cdf38
to
ea6f72e
Compare
ea6f72e
to
cae689e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your work.
For the first time, it will be possible to create goferless runsc sandboxes.
It won't be possible without all your constructive comments, they are really helpful! :) |
cae689e
to
160b6e2
Compare
This patch refactors the overlay medium from string to a new type OverlayMedium. Now all overlay medium related methods, such as validation and extracting anon directory, are implemented on this type. This is a preparation for adding the EROFS rootfs support. There is no functional change intended. Co-authored-by: Ayush Ranjan <ayushranjan@google.com> Signed-off-by: Tiwei Bie <tiwei.btw@antgroup.com>
0126e20
to
e71f35b
Compare
Could you apply the following patch? Our internal build system is not happy with package tests inheriting dependencies implicitly.
|
This patch adds the EROFS rootfs support. No gofer process will be created for the container, when there is no need to pass through host files into a container via the gofer process. Annotations for rootfs are also introduced to provide extra information, including the mount source, mount type and overlay config. Additionally, busybox-static is added to the default image and will be used to build the EROFS rootfs images during the test. Updates google#8956 Signed-off-by: Tiwei Bie <tiwei.btw@antgroup.com>
e71f35b
to
bff1150
Compare
I see. Just fixed it. Thanks! |
This PR adds the EROFS rootfs support. No gofer process will be created for the container, when there is no need to pass through host files into a container via the gofer process. Annotations for rootfs are introduced to provide extra information, including the mount source, mount type and overlay config.
busybox-static
is also added to the default image and will be used to build the EROFS rootfs images during the test. The overlay medium is refactored fromstring
to a new typeOverlayMedium
. Now all overlay medium related methods, such as validation and extracting anon directory, are implemented on this type. TheGoferMountConf
is decoupled to two layers to allow us to configure all combinations of a gofer mount in a succinct way.cc @ayushr2