Test-infra: prow/podspec: Hardcoded-path to commands

[philhug]

Currently the following commands are hard-coded in podspec.go.
This is problematic as go_image puts the image into a different place:
e.g. /app/prow/cmd/initupload/initupload

So, we need to either change go_image to make them go to root or drop the Command entirely.
Unfortunately for the entrypoint container, cp is invoked and the locaion of entrypoint needs to be known.
Ideas?

clonerefs:

Command:      []string{"/clonerefs"},

initupload:

Command: []string{"/initupload"},

place-tools:

Command:      []string{"/bin/cp"},
Args:         []string{"/entrypoint", entrypointLocation},

sidecar:

Command: []string{"/sidecar"},

[philhug]

/area prow

[philhug]

/assign stevekuznetsov

[stevekuznetsov]

What is go_image and why is it not honoring the Dockerfile that is being used to build these images?

[philhug]

@stevekuznetsov bazel is not using Dockerfile to build the docker images.
Instead it uses the bazel docker rules: See here

The result of this bazel build is what ends up on gcr.io/k8s-prow

[philhug]

see also #7912
to test, simply run bazel run //prow:release and you'll have the docker images loaded in your local docker daemon.

[BenTheElder]

as an aside: dumping binaries into / is not the greatest imho. you wouldn't do that on a "real" linux system either.

That said we can configure go_image to use any directory (including / which the container image rules default to, the language ones add the binary to /app by default) with data_path and directory fields.

https://github.com/bazelbuild/rules_docker#container_layer

[BenTheElder]

just need to set directory = "/" on any image we have these hardcoded paths for, but really callilng something like $(which foo) or similar would be better so the images can be more flexible, imho...

[philhug]

is there consensus on this? I can create a PR to move the binary within Dockerfile or modify bazel to put the binary into the / directory.

[BenTheElder]

@philhug let's just set the directory fields in the bazel builds for now.

[philhug]

Unfortunately the directory = "/" approach didn't work.
Directory doesn't have any impact at all.
Is this due to outdated bazel rules?

The go binary always ends up in /app.

/app/
/app/prow
/app/prow/cmd
/app/prow/cmd/clonerefs
/app/prow/cmd/clonerefs/clonerefs.runfiles
/app/prow/cmd/clonerefs/clonerefs.runfiles/__main__
/app/prow/cmd/clonerefs/clonerefs.runfiles/__main__/prow
/app/prow/cmd/clonerefs/clonerefs.runfiles/__main__/prow/cmd
/app/prow/cmd/clonerefs/clonerefs.runfiles/__main__/prow/cmd/clonerefs
/app/prow/cmd/clonerefs/clonerefs.runfiles/__main__/prow/cmd/clonerefs/linux_amd64_pure_stripped
/app/prow/cmd/clonerefs/clonerefs.runfiles/__main__/prow/cmd/clonerefs/linux_amd64_pure_stripped/clonerefs
/app/prow/cmd/clonerefs/clonerefs.runfiles/__main__/external
/app/prow/cmd/clonerefs/clonerefs

[philhug]

/reopen

[philhug]

I updated rules_docker and rules_go to latest master, but same effect.

@BenTheElder do you have any ideas?

[philhug]

/assign @BenTheElder

[BenTheElder]

we may not even want to use go_image, really we just want to take an existing image, add the binar{y,ies} and set the entrypoint, which we can do with the other rules. we mostly tend to use our own base image anyhow, so we're not benefiting from the distroless defaults.

[BenTheElder]

/priority backlog

[BenTheElder]

/cc @cjwagner
are we just using the dockerfiles for testing these? we should really fix this and start pushing these with prow bumps

[cjwagner]

Yes pod utilities must be built with make and the Dockerfile right now.

[BenTheElder]

We switched to container_image, in theory this should be fixed now, but we'll need to try actually using them in some job to verify for sure :upside_down_face:

[fejta]

The reason this didn't work but also didn't fail is because:

• go_image accepts **kwargs so you won't get a compile-time error
• go_image doesn't plumb the directory parameter into its app_layer call (just args, data, tags, visibility):

https://github.com/bazelbuild/rules_docker/blob/26e5ff9a6dc292452de518e2393afe6b80f2d390/go/image.bzl#L91-L100

Fixing this in rules_go is the better solution here

[fejta]

/reopen

[k8s-ci-robot]

@fejta: you can't re-open an issue/PR unless you authored it or you are assigned to it.

In response to this:

/reopen

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.

[BenTheElder]

@fejta we do not want the runfiles to be in the image, using container_image guarantees we _just_ take the binary and place it in an image at an expected location. The other functionality of go_image doesn't make sense here, except maybe using their base image, but we don't even do that.

[BenTheElder]

@fejta if we do this we should consider using symlinks = {'/binary: '/prow/cmd/binary/binary'} instead of directory. then the entrypoint magic will still WAI from bazel's end but the decorated entrypoints of /binary should still work.
This would require updating the $lang_rules as well though. I did take a quick stab at this before using container_image.

[stevekuznetsov]

/unassign
/assign @BenTheElder

[BenTheElder]

current pattern seems to work fine. image layers are handy :man_shrugging: