Refactor the prow/cmd Dockerfiles to use shared base Dockerfiles #4900
Conversation
…nctionality prow/cmd/*/Dockerfile basically has two classes of images: 1. alpine:3.6 + ca-certs 1. ^^ with git This hoists those into common Dockerfiles under prow/cmd/alpine and prow/cmd/git respectively.
|
Hi @mattmoor. 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 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. I understand the commands that are listed here. |
|
/ok-to-test |
|
The |
|
Done. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cjwagner, mattmoor The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
| @@ -12,10 +12,8 @@ | |||
| # See the License for the specific language governing permissions and | |||
| # limitations under the License. | |||
|
|
|||
| FROM alpine:3.6 | |||
| FROM gcr.io/k8s-prow/alpine | |||
There was a problem hiding this comment.
These don't have any sort of version tag on them. How will we bump to 3.7 when it's time?
There was a problem hiding this comment.
I'm happy to put in a 3.6, but wasn't sure what you'd want for git?
There was a problem hiding this comment.
We can version the images separately from both alpine and git. I'm mainly afraid of confusions that come about when you rely on :latest.
There was a problem hiding this comment.
There is a parallel here to what's called out in "Building images" here.
I was talking to someone about this this morning, and frankly it's a weakness docker_build does not share because the base image intermediate artifact is expressed declaratively, but via an explicit def/use.
In the case of that post, you'd express FROM as:
docker_build(
...
base = "//livegrep/base",
...
)This is even easier with lang_image because it is containerization that "just works" (once you get Bazel to work), in their case they mention C++ and Go:
cc_image( # same signature as cc_binary
...
base = "//livegrep/base", # doesn't have to be distroless!
...
)
go_image( # same signature as go_binary
...
base = "//livegrep/base", # doesn't have to be distroless!
...
)Once we get these base images published, my next PR was going to be to give you guys go_image with these base images, after which I can give you rules_k8s for the starter cluster.
There was a problem hiding this comment.
Neat. Does it statically compile the go binary? Alpine doesn't have glibc and that can cause issues.
There was a problem hiding this comment.
No, but now that I see go_binary has added support for static linking I can add support for that. By default we use gcr.io/distroless/base which has glibc, ca-certs and a handful of other generally necessary things (e.g. tzdata, a /tmp directory, ...).
Perhaps I will add a :foo.static target that does what go_image does, but includes the static binary version of the image (this is how rules_go does this).
There was a problem hiding this comment.
Ugh, nevermind. Nothing is easy. (╯°□°)╯︵ ┻━┻
There was a problem hiding this comment.
Alright, I'm just going to add build --features=static and run --features=static to .bazelrc :)
There was a problem hiding this comment.
Alright, I sent a PR to make Bazel build static Go binaries in this repo (needs an update to rules_go). That should unblock putting these binaries onto the Alpine base images.
|
Can we ensure the new base images are pullable? I am getting |
|
I don't have push access, what's the process? |
|
I can push the new images. We should add version tags first though as @spxtr suggested. Should we start our own version tags instead of using alpine's tag (i.e. start at 0.1)? |
|
I'll put together a PR |
|
@Kargakis I pushed the images. Both have tag |
... for common functionality
prow/cmd/*/Dockerfile basically has two classes of images:
This hoists those into common Dockerfiles under prow/cmd/alpine and prow/cmd/git respectively.