Skip to content
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

Name the app using the base of the importpath #380

Merged
merged 2 commits into from
Feb 22, 2019

Conversation

bradhoekstra
Copy link
Contributor

Currently the entrypoint is always '/ko-app', which makes it difficult to tell which ko-built app is which from the host system.

This changes it so that a container built from the importpath
github.com/knative/serving/cmd/autoscaler

has an entrypoint of
/ko-app/autoscaler

Fixes #375

pkg/ko/build/gobuild.go Outdated Show resolved Hide resolved
pkg/ko/build/gobuild.go Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Feb 20, 2019

Codecov Report

Merging #380 into master will increase coverage by 0.05%.
The diff coverage is 65.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #380      +/-   ##
==========================================
+ Coverage   51.12%   51.17%   +0.05%     
==========================================
  Files          75       75              
  Lines        3458     3478      +20     
==========================================
+ Hits         1768     1780      +12     
- Misses       1424     1428       +4     
- Partials      266      270       +4
Impacted Files Coverage Δ
pkg/ko/build/gobuild.go 56.7% <65.21%> (+0.45%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bada66e...f66e691. Read the comment docs.

@jonjohnsonjr jonjohnsonjr merged commit d3e6a44 into google:master Feb 22, 2019
bobcatfish added a commit to bobcatfish/pipeline that referenced this pull request Mar 21, 2019
We were using `stable` and `stable` was suddenly updated to install `ko`
from https://github.com/google/ko but unfortunately that version of `ko`
didn't include google/go-containerregistry#380
which was the critical change from building binaries at `/ko-app` to
building them at `/ko-app/my-pkg` <- which we built some of our
functionality around.

Now we'll try to pin the image so we aren't caught by surprise by
changes again and can decide for ourselves when we want to consume them.
(Hopefully old images don't get deleted too often.. 🙏 Or at least not
before tektoncd#267 😛)
tekton-robot pushed a commit to tektoncd/pipeline that referenced this pull request Mar 22, 2019
We were using `stable` and `stable` was suddenly updated to install `ko`
from https://github.com/google/ko but unfortunately that version of `ko`
didn't include google/go-containerregistry#380
which was the critical change from building binaries at `/ko-app` to
building them at `/ko-app/my-pkg` <- which we built some of our
functionality around.

Now we'll try to pin the image so we aren't caught by surprise by
changes again and can decide for ourselves when we want to consume them.
(Hopefully old images don't get deleted too often.. 🙏 Or at least not
before #267 😛)
vdemeester added a commit to vdemeester/tektoncd-pipeline that referenced this pull request Mar 27, 2019
In tektoncd#605, we required that `ko` was recent enough to be *guaranteed* to
have `/ko-app/{app}` (see google/go-containerregistry#380). As we now
point to `google/ko` and as we have a `ko` release (v0.1), we can
require this version for development.

Given the requirement above, we can simplify the `entrypoint` copy
container `args`.

Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
tekton-robot pushed a commit to tektoncd/pipeline that referenced this pull request Mar 27, 2019
In #605, we required that `ko` was recent enough to be *guaranteed* to
have `/ko-app/{app}` (see google/go-containerregistry#380). As we now
point to `google/ko` and as we have a `ko` release (v0.1), we can
require this version for development.

Given the requirement above, we can simplify the `entrypoint` copy
container `args`.

Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants