-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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] Run in a container #558
[WIP] Run in a container #558
Conversation
I've tested this on Docker for Mac:
The build worked, but deploy hits
I suppose I could fix it by running in a pod, but I'm not so sure if it makes much sense, as the user experience is kind of convoluted. My primary use-case was to make it easy to run in Docker-enabled CI, without having to download the binary. I was able to use
This broadly satisfies my needs, although I've noticed that removing
I'll try this image in CI now and report back. |
I've tried using this in CircleCI with a following config: # .circleci/config.yml
version: 2
jobs:
build:
docker:
- image: errordeveloper/skaffold:1c894265d81c618c782500cde2d788fb77f3b416
steps:
- checkout
- run: skaffold build
workflows:
version: 2
build_and_test:
jobs:
- build I got the same error, I didn't expect anything different. The config is quite nice an simple. I think I could actually download the binary, but that won't make any difference. I'll try to stab-out I've opened #559 to discuss the general issue of |
With a dummy # .circleci/config.yml
version: 2
jobs:
build:
docker:
- image: errordeveloper/skaffold:66cc263ef18f107adce245b8fc622a8ea46385f2
steps:
- checkout
- setup_remote_docker: {docker_layer_caching: true}
- run: skaffold build
workflows:
version: 2
build_and_test:
jobs:
- build I still need to add registry auth, but so far I think it's rather nice and simple config, it demonstrates how easy it is to use Skaffold in a Docker-native CI. In Travis CI I'd have to call Let's figure out how to fix #559 properly and then review how to build the container image. Also #550 would be handy at some point. |
Sorry for the delayed response - I just got back from vacation after KubeCon :)
root SGTM
I'm inclined to use distroless, but no strong opinions here.
we can follow the example and set GCB as a profile if users want I'm also inclined to either build the integration test image on top of this one or have them be the same. There's currently nothing extra in the integration test image and theoretically they should really be the same. The real issue is shipping support for all the builders and deployers. The image is a bit large now because it needs to include the binaries for each builder and deployer (and its actually incorrect right now since it doesn't include bazel). I think anything we publish needs to have all of them, otherwise we'll have to maintain many. |
Yes, that'd be a good idea. I just didn't know whether the way the other Dockerfile is done is important in any way or not, so I've not decided to do any refactoring there yet.
I see, good point, I didn't realise this. I suppose it'd have to come before we refactor the integration image. |
66cc263
to
a8166a5
Compare
a8166a5
to
8ed88d4
Compare
@r2d4 I've just rebased this and update it to include latest versions of all builder. |
RUN curl --silent --location "https://github.com/GoogleCloudPlatform/docker-credential-gcr/releases/download/v${DOCKER_CREDENTIAL_GCR_VERSION}/docker-credential-gcr_linux_amd64-${DOCKER_CREDENTIAL_GCR_VERSION}.tar.gz" \ | ||
| tar xz ./docker-credential-gcr \ | ||
&& mv docker-credential-gcr usr/local/bin/docker-credential-gcr | ||
# TODO: docker-credential-gcr configure-docker |
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 this setup step required? Do we know what does it actually do?
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.
docker-credential-gcr configure-docker
sets up the "gcr.io" repository patterns to use the docker-credential-gcr credential helper for the docker deamon.
Skaffold needs the gcr credential helper to be able to push to gcr.io repos (https://github.com/GoogleContainerTools/skaffold/blob/master/pkg/skaffold/docker/auth.go) but as far as I understand we don't depend on the docker daemon to push so this line can probably go away. @r2d4?
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.
no, we follow the same auth flow as the CLI to push - read the .docker/config to figure out what credential helpers we need to call
|
||
RUN ln -s /lib /lib64 | ||
|
||
ENV KUBECTL_VERSION v1.10.6 |
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.
It seems plausible to support latest 1.10, do folks agree?
RUN curl --silent --location "https://dl.k8s.io/${KUBECTL_VERSION}/bin/linux/amd64/kubectl" --output usr/local/bin/kubectl \ | ||
&& chmod +x usr/local/bin/kubectl | ||
|
||
ENV DOCKER_VERSION 18.03.0 |
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.
One constraint I have here, is that it has to support multi-stage build, and I'm happy to include the latest version available (perhaps LTS one), but I don't know if we have other constrains or not...
8ed88d4
to
70a742e
Compare
Codecov Report
@@ Coverage Diff @@
## master #558 +/- ##
=======================================
Coverage 38.27% 38.27%
=======================================
Files 56 56
Lines 2576 2576
=======================================
Hits 986 986
Misses 1476 1476
Partials 114 114 Continue to review full report at Codecov.
|
Also, here is how integration image could be built on top of this
|
|
||
ENV BAZEL_VERSION 0.16.1 | ||
RUN curl --silent --location "https://github.com/bazelbuild/bazel/releases/download/${BAZEL_VERSION}/bazel-${BAZEL_VERSION}-linux-x86_64" --output usr/local/bin/bazel \ | ||
&& chmod +x usr/local/bin/bazel |
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.
Looks like it needs glibc... 😿
dfc0b8b9e9f8:/go# ldd /usr/local/bin/bazel
/lib64/ld-linux-x86-64.so.2 (0x7f2384c48000)
librt.so.1 => /lib64/ld-linux-x86-64.so.2 (0x7f2384c48000)
libdl.so.2 => /lib64/ld-linux-x86-64.so.2 (0x7f2384c48000)
libpthread.so.0 => /lib64/ld-linux-x86-64.so.2 (0x7f2384c48000)
libm.so.6 => /lib64/ld-linux-x86-64.so.2 (0x7f2384c48000)
libstdc++.so.6 => /usr/lib/libstdc++.so.6 (0x7f23848f6000)
libgcc_s.so.1 => /usr/lib/libgcc_s.so.1 (0x7f23846e4000)
libc.so.6 => /lib64/ld-linux-x86-64.so.2 (0x7f2384c48000)
Error relocating /usr/local/bin/bazel: __realpath_chk: symbol not found
Error relocating /usr/local/bin/bazel: __memcpy_chk: symbol not found
Error relocating /usr/local/bin/bazel: __sprintf_chk: symbol not found
dfc0b8b9e9f8:/go# bazel
Error relocating /usr/local/bin/bazel: __realpath_chk: symbol not found
Error relocating /usr/local/bin/bazel: __memcpy_chk: symbol not found
Error relocating /usr/local/bin/bazel: __sprintf_chk: symbol not found
dfc0b8b9e9f8:/go#
We have libc6-compat
already, and I tried using instructions from mongodb-js/mongodb-prebuilt#35 without any luck. Will have another go later on.
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.
Also filed bazelbuild/bazel#5891.
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.
For now, I'm gonna compile bazel, as it's relatively easy to do. However, it'd be good to compare how much do we win (in terms of image size) from having alpine vs e.g. ubuntu.
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.
Also filed bazelbuild/bazel#5909.
As discuss on thew call today, I'll look at the existing Dockerfile and see if that works for the use-case. |
@errordeveloper so, I'd rename this PR to "make skaffold Docker image smaller", what do you think? I seems that the current gcr.io/k8s-skaffold/skaffold image should be good enough for CI, just a bit on the larger side due to the ubuntu baseimage? |
Yes, perhaps! But I am tempted to close this for now really.
…On Mon, 8 Oct 2018, 3:03 pm Balint Pato, ***@***.***> wrote:
@errordeveloper <https://github.com/errordeveloper> so, I'd rename this
PR to "make skaffold Docker image smaller", what do you think? I seems that
the current gcr.io/k8s-skaffold/skaffold image should be good enough for
CI, just a bit on the larger side due to the ubuntu baseimage?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#558 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAPWS7-WdpIBfMbXPI_B8VRvDQCAcOH7ks5ui1stgaJpZM4UBED7>
.
|
@errordeveloper Are you ok if I close it, then? |
Yes, please.
…On Tue, 9 Oct 2018, 2:58 pm David Gageot, ***@***.***> wrote:
@errordeveloper <https://github.com/errordeveloper> Are you ok if I close
it, then?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#558 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAPWS3Pcm3vV8h23bLsue1tXbizlkeNtks5ujKubgaJpZM4UBED7>
.
|
I've had a look at
deploy/skaffold/Dockerfile
, and it uses Ubuntu base image and I couldn't actually build it locally. Also, it seems like that container image is meant for integration tests.I'd like to see an official slim container that I can use in CI. I'm not sure what's the preferred way to go about it, so I did the simplest thing first – multi-stage build based on Alpine. Please take a look and let me know.
I suppose we will need to consider: