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

Add a tag-only flag to publisher. #332

Merged
merged 1 commit into from
May 17, 2021

Conversation

chhsia0
Copy link
Contributor

@chhsia0 chhsia0 commented Mar 30, 2021

The --tag-only flag resolves images into tag-only references. This flag is useful when digests are not preserved when images are repopulated. It can only be used when a single tag is specified, and the tag is not latest.

@imjasonh
Copy link
Member

I'm not sure I understand the use case, in what cases are references by digest not preferred?

Images by tag are problematic because the tag can change and you can get unexpected/inconsistent behavior across replicas/nodes.

@jonjohnsonjr
Copy link
Collaborator

I can imagine a couple situations where this could be useful, e.g. when writing to the docker daemon, but not when pushing to the registry. Agree with Jason, I'd like to understand the problem a little more.

@codecov-io
Copy link

Codecov Report

Merging #332 (ab03af6) into main (0b96f41) will increase coverage by 0.14%.
The diff coverage is 53.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #332      +/-   ##
==========================================
+ Coverage   38.24%   38.39%   +0.14%     
==========================================
  Files          33       33              
  Lines        1527     1542      +15     
==========================================
+ Hits          584      592       +8     
- Misses        849      853       +4     
- Partials       94       97       +3     
Impacted Files Coverage Δ
pkg/commands/resolver.go 11.60% <0.00%> (-0.07%) ⬇️
pkg/publish/default.go 61.84% <45.45%> (-2.78%) ⬇️
pkg/publish/options.go 28.12% <100.00%> (+7.43%) ⬆️

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 0b96f41...ab03af6. Read the comment docs.

@chhsia0
Copy link
Contributor Author

chhsia0 commented Mar 30, 2021

Sure I could explain more. Our use case is to use ko to generate manifests that supports air-gapped environments, where we repopulate necessary images to a local registry or containerd from tarballs (similar to docker load). However, such repopulation does not preserve image digests, so we have to use tags to refer to a certain image version in the manifests so they can be deployed in both a normal and an air-gapped environment.

@chhsia0
Copy link
Contributor Author

chhsia0 commented Mar 30, 2021

In general I agree that image digests are preferred, just that it doesn't work in this particular use case.

I also put a constraint for this tag to disallow people from generating image references that uses the (commonly) moving latest tag.

@jonjohnsonjr
Copy link
Collaborator

I also put a constraint for this tag to disallow people from generating image references that uses the (commonly) moving latest tag.

I'm not sure if we want to be that opinionated about latest. I generally agree with you, but it feels like an artificial constraint. That being said, it is easier to relax this if people complain than it would be to introduce this constraint and break people who rely on it, so I don't have a strong opinion.

I'm ok to merge unless @imjasonh has objections

@imjasonh
Copy link
Member

Sgtm

@jonjohnsonjr jonjohnsonjr merged commit bc92184 into ko-build:main May 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants