Specify Command field for internal containers (creds-init, …)

[vdemeester]

Changes

The entrypoint package will try to talk to the remotes for
containers that do not specify Command (aka Entrypoint in
image-spec).

• This should not be required for internal containers as we are the
  one maintaining it. It should remove so registry call that aren't
  required.
• It fails in some development mode (when using
  KO_DOCKER_REPO=ko.local), as the registry might not
  exists (ko.local) or not available from the
  cluster (localhost:5000, …)

This is a partial fix for #591 😅

Signed-off-by: Vincent Demeester vincent@sbr.pm

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

• Includes tests (if functionality changed/added)
• Includes docs (if user facing)
• Commit messages follow commit message best practices

See the contribution guide
for more details.

Release Notes

release-note

[tekton-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vdemeester

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [vdemeester]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[vdemeester]

/test pull-tekton-pipeline-integration-tests

[vdemeester]

/test pull-tekton-pipeline-integration-tests

[dlorenc]

/test pull-tekton-pipeline-integration-tests

[vdemeester]

@imjasonh updated 👼

[imjasonh]

Can you add docs to CONTRIBUTING or elsewhere that a recent version of ko is required? Just until we have a real ko release, then we can say "ko release >0.1 is required"

Otherwise lgtm

[bobcatfish]

i think there's a typo here, won't work seems like extra text?

[bobcatfish]

I looked at the failures a bit and I think the path to the executable for the "bash" image might be wrong:

I0313 18:08:19.629]     - args:
I0313 18:08:19.629]       - -wait_file
I0313 18:08:19.629]       - ""
I0313 18:08:19.629]       - -post_file
I0313 18:08:19.629]       - /builder/tools/0
I0313 18:08:19.629]       - -entrypoint
I0313 18:08:19.629]       - /ko-app/nop
I0313 18:08:19.629]       - --
I0313 18:08:19.630]       - -args
I0313 18:08:19.630]       - mkdir -p /workspace/workspace
I0313 18:08:19.630]       command:
I0313 18:08:19.630]       - /builder/tools/entrypoint
I0313 18:08:19.630]       env:
I0313 18:08:19.630]       - name: HOME
I0313 18:08:19.630]         value: /builder/home
I0313 18:08:19.631]       image: gcr.io/tekton-prow-3/kpipeline-e2e-img/bash-e

Instead of /ko-app/nop i think maybe it should be something like /ko-app/bash?

It looks like the variable and flag we use to control the name of this image included "noop" from the beginning but im not sure why (accident maybe?), seems like that is causing some confusion!

[bobcatfish]

helping
😇

[bobcatfish]

Specify Command field for internal containers (creds-init, …)

one more thought here, and maybe this is something you are already planning to do @vdemeester : what do you think about excluding the internal containers from passing through the logic that tries to determine the entrypoint? (i.e. only apply that logic to containers specified by the user)

[vdemeester]

one more thought here, and maybe this is something you are already planning to do @vdemeester : what do you think about excluding the internal containers from passing through the logic that tries to determine the entrypoint? (i.e. only apply that logic to containers specified by the user)

Right, I thought about that. This means we would have to add a special case in the entrypoint package and act differently for a given list of image. I choose the path of less special case (but actually more changes 😅). To be honest I would be ok with both solutions.

Instead of /ko-app/nop i think maybe it should be something like /ko-app/bash?

Yeah i thought I messed some 😅

It looks like the variable and flag we use to control the name of this image included "noop" from the beginning but im not sure why (accident maybe?), seems like that is causing some confusion!

Not sure either but it definitely cause confusion 😹

[bobcatfish]

This means we would have to add a special case in the entrypoint package and act differently for a given list of image

If you decide to go this route, id make the decision about what to pass to entrypoint before calling it - only call entrypoint with the container specs you want to redirect :D

[bobcatfish]

Let's go ahead with this for now and if we feel like coming back to the logic and improving it later we can :D

/lgtm
/meow space

[tekton-robot]

@bobcatfish:

In response to this:

Let's go ahead with this for now and if we feel like coming back to the logic and improving it later we can :D

/lgtm
/meow space

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.

[bobcatfish]

oh hahaha nice, tide thinks this is a totally new PR....

/lgtm
/meow space

[tekton-robot]

@bobcatfish:

In response to this:

oh hahaha nice, tide thinks this is a totally new PR....

/lgtm
/meow space

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.