-
Notifications
You must be signed in to change notification settings - Fork 85
Conversation
-v "${kernel_mod_dir}":"${kernel_mod_dir}" \ | ||
-v "${GOPATH}":"${GOPATH}" \ | ||
--privileged \ |
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.
debootstrap needs chroot Is there another way of doing this?
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.
Is it mounting any special device or why chroot need it ?
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.
I think debootstrap first downloads the rootfs then chroots into it then mounts file systems and then installs packages.
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.
There are fakeroot
and fakechroot
packages. I had a quick play around but they didn't allow me to run debootstrap
successfully ;(
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.
I will go through them and get back soon.
Thanks
rootfs-builder/ubuntu/rootfs_lib.sh
Outdated
elif check_program "debootstrap" ; then | ||
PKG_MANAGER="debootstrap" | ||
else | ||
die "debootstrap is not installed" |
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.
This block specifies debootstrap
quite a few times. Could you maybe add a variable to rootfs-builder/ubuntu/config.sh
and use that variable in all these places?
rootfs-builder/ubuntu/config.sh
Outdated
@@ -0,0 +1,17 @@ | |||
# | |||
# Copyright (c) 2017 Intel Corporation |
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.
This is a new file so should show 2018 but also your name/or company right @grahamwhaley ? Same comment for the other new files.
rootfs-builder/ubuntu/Dockerfile.in
Outdated
# This will install the proper golang to build Kata components | ||
@INSTALL_GO@ | ||
|
||
|
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.
Nit: multiple blank lines here and below in various places.
ARCHIVE_URL=${ARCHIVE_URL:-"http://archive.ubuntu.com/ubuntu/"} | ||
|
||
# this should be ubuntu's codename eg Xenial for 16.04 | ||
OS_NAME=${OS_NAME:-"xenial"} |
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.
This isn't really a comment specific to this PR - just a reminder to all...
I appreciate this is following the existing convention, but we do all need to think about how we'll manage these versions and code names as one day, they will become invalid.
See:
- https://github.com/clearcontainers/runtime/wiki/Handling-new-and-end-of-life-distro-versions
- Create tool to detect new and end-of-life distro versions clearcontainers/tests#857
For osbuilder
, we could create a special build target which called a shell script and checked ARCHIVE_URL
+ OS_NAME
maybe (and there are bound to be distro-specific bits to add to that). But we could then atleast call this build rule as part of the CI.
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.
I agree we should have a mechanism to detect EOL distro version.....However I won't be able to work on that issue before 17th(have my exams till then) I can hopefully work on it after that, if the issue is not already resolved by then.
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.
Hi @ydjainopensource - thanks. I added this comment as your PR just reminded me of the general issue. Please don't feel you have to work on that feature unless it's something that interests you ;)
I suspect there will need to be a bit of discussion about how best to solve this problem for all the distros we handle but it is something that has "caught us out" before so it would be good to find a general and robust solution to the problem.
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.
I loved love to work on it. Do you have any ideas as to how we should go about tackling them? I have worked only on apt based distros.
I have removed the WIP tag since I had kept it as I was looking for an alternative to running debootstrap without a privileged container. I have also changed and squashed the PR to include the suggested fixes |
Ubuntu rootfs using debootstrap with support for extra packages Fixes #32 Signed-off-by: Yash Jain <[email protected]>
Signed-off-by: Yash Jain <[email protected]>
I think the CI is failing due to some network error issue...can someone confirm? |
Looks like an Euleros repo timeout/failure?
|
Yep - we really need #56 to land... |
Now #56 is merged (cough...), I've re-started Travis so fingers crossed... /cc @jcvenegas. |
Hi @ydjainopensource - any update on this PR? It needs a rebase now. |
Sorry for the late reply. I have been busy lately and hence haven't been able to work on this. |
Support for ubuntu rootfs