-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
wip: add experimental kicbase image ubuntu20:04 #7884
Conversation
Can one of the admins verify this patch? |
Hi @alonyb. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
please move all your changes to a new folder called /hack/images/kic-base-experimental then we will experiment with this new image, and meanwhile keep our current image, and over time we will switch to it, after we have done enough testing. |
Travis tests have failedHey @alonyb, TravisBuddy Request Identifier: 9214c6c0-868c-11ea-afe8-f571977cac28 |
Codecov Report
@@ Coverage Diff @@
## master #7884 +/- ##
==========================================
- Coverage 35.87% 35.50% -0.37%
==========================================
Files 148 148
Lines 9201 9330 +129
==========================================
+ Hits 3301 3313 +12
- Misses 5504 5620 +116
- Partials 396 397 +1
|
I like to see integration test results on this PR, preferably on your machine or your fork. without touching minikube test code. |
We should split this in several steps, instead of doing all-at-once (even if that is how we test it...)
We can time this with the upgrade to Buildroot 2020.02
This one would be good to include in the next release
EDIT: it drops the default port, and only uses the tunneled random port that is exposed at runtime |
Aside from the ubuntu upgrade, it will be interesting to see how the layers affect the size... Currently it is those "middle" layers that are building the size (ubuntu itself is only 73 MB)
Cleaning up the apt cache for each step should help a bit... |
Do you mind helping me understand the tangible difference of forking these files? I worry because a lot of thought was put into these files, and they do see a fair bit of maintenance.
It's worth mentioning that kind has since also moved on to Ubuntu 20.04. Thanks! |
This needs a squashing of commits, and a rebase towards upstream (kindbase)... Should also sync Ubuntu 20.04 a bit with the upgraded files for Buildroot 2020.02 ? I think the SSH comments was about the combination of "expose 22" and "chpasswd" The buildroot-based image is currently 529MB, compared with 974MB for (old) kicbase
Docker layers have the same issue as git commits, if you add big files and then remove them it ends up keeping them around in the "history" which means a bigger download for the end user. If you instead clean up and remove things in the same commit layer, only the delta gets added. This leads to a smaller git repository or a smaller docker image, which means faster pull/load |
There doesn't seem to be any actual changes done to the entrypoint, besides whitespace: --- ../kind/images/base/files/usr/local/bin/entrypoint 2020-05-22 15:57:25.315972920 +0200
+++ hack/images/kic-base-experimental/files/entrypoint 2020-05-22 15:56:02.498304340 +0200
@@ -14,12 +14,14 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+# using entrypoint created by kind https://github.com/kubernetes-sigs/kind/blob/master/images/base/files/usr/local/bin/entrypoint
+
set -o errexit
set -o nounset
set -o pipefail
fix_mount() {
- echo 'INFO: ensuring we can execute /bin/mount even with userns-remap'
+ echo 'INFO: ensuring we can execute /bin/mount even with userns-remap'
# necessary only when userns-remap is enabled on the host, but harmless
# The binary /bin/mount should be owned by root and have the setuid bit
chown root:root /bin/mount
@@ -193,4 +195,4 @@
enable_network_magic
# we want the command (expected to be systemd) to be PID1, so exec to it
-exec "$@"
+exec "$@"
\ No newline at end of file But since everything is now forked and pinned, we now miss all upstream improvements... i.e. we don't get the new packages from ubuntu and we don't get the changes from kind entrypoint |
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 don't think we should fork the kind base (yet?) and I don't think we should pin the packages
gnupg=${GNUPG_VERSION} libglib2.0-0=${LIBGLIB2_VERSION} libseccomp2=${LIBSECCOMP2_VERSION} ca-certificates=${CA_CERTIFICATES_VERSION} \ | ||
curl=${CURL_VERSION} rsync=${RSYNC_VERSION} \ | ||
&& rm -rf /var/lib/apt/lists/* \ | ||
&& sh -c "echo 'deb http://download.opensuse.org/repositories/devel:/kubic:/libcontainers:/stable/xUbuntu_19.10/ /' > /etc/apt/sources.list.d/devel:kubic:libcontainers:stable.list" && \ |
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 still uses 19.10, rather than 20.04
Based on this discussion, for now, I think we should just upgrade the underlying kicbase. |
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: alonyb The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
based on coversations in office hours, minikube maintainers reached out to kind to ask for a base image that does not have nightly build containerd but they were not open for this request on the logic that kind is meant for testing lastest source code and not meant for local kuberentes experience. and also based on the cloud shell bug we ended up sed-ing the entrypoint by line number in the dockerfile this is an un-maintable approach, and we will need to start from scratch and mantain a fork of the entrypoint in the source code. therefore I re-open this PR |
Codecov Report
@@ Coverage Diff @@
## master #7884 +/- ##
==========================================
- Coverage 35.87% 34.05% -1.83%
==========================================
Files 148 153 +5
Lines 9201 9840 +639
==========================================
+ Hits 3301 3351 +50
- Misses 5504 6086 +582
- Partials 396 403 +7
|
Arguably that change should not go into the public image, for debugging a specific issue ? And we don't have to re-open this PR, if all we want to do is fork the kind base (#7788). |
/check-cla |
This PR is to #7788
After this PR: