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 digest source 'tag' to use tag without digest #5436

Merged
merged 2 commits into from
Mar 4, 2021
Merged

Add digest source 'tag' to use tag without digest #5436

merged 2 commits into from
Mar 4, 2021

Conversation

fengye87
Copy link
Contributor

Related: #3649

Description

A new option skaffold render --digest-source=tag was added, which will render the manifests with built tags. The difference between 'none' and 'tag' is that the former one will keep whatever artifacts are in the original manifests, while the latter will update them with proper tags, without sha256 digests.

This could be very helpful when rendering one-for-all release manifests in multi-arch deployment scenarios, although extra cares should be taken in this case to not move any released tags.

User facing changes (remove if N/A)

User gets a new option skaffold render --digest-source=tag

@fengye87 fengye87 requested a review from a team as a code owner February 22, 2021 12:59
@google-cla
Copy link

google-cla bot commented Feb 22, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@codecov
Copy link

codecov bot commented Feb 23, 2021

Codecov Report

Merging #5436 (5b75087) into master (cac0c53) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5436      +/-   ##
==========================================
+ Coverage   71.39%   71.42%   +0.02%     
==========================================
  Files         397      397              
  Lines       14540    14553      +13     
==========================================
+ Hits        10381    10394      +13     
- Misses       3388     3389       +1     
+ Partials      771      770       -1     
Impacted Files Coverage Δ
pkg/skaffold/runner/runner.go 0.00% <ø> (ø)
cmd/skaffold/app/cmd/render.go 46.42% <100.00%> (ø)
pkg/skaffold/runner/build_deploy.go 69.23% <100.00%> (+0.34%) ⬆️
pkg/skaffold/docker/parse.go 82.16% <0.00%> (-0.81%) ⬇️
pkg/skaffold/util/config.go 77.77% <0.00%> (ø)
pkg/skaffold/instrumentation/types.go 0.00% <0.00%> (ø)
pkg/skaffold/schema/defaults/defaults.go 87.57% <0.00%> (+0.15%) ⬆️
pkg/skaffold/instrumentation/export.go 76.92% <0.00%> (+0.17%) ⬆️
cmd/skaffold/app/cmd/flags.go 87.93% <0.00%> (+0.43%) ⬆️
pkg/skaffold/test/structure/structure.go 87.50% <0.00%> (+0.70%) ⬆️
... and 2 more

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 cac0c53...5b75087. Read the comment docs.

@tejal29 tejal29 added the kokoro:run runs the kokoro jobs on a PR label Feb 25, 2021
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Feb 25, 2021
Copy link
Contributor

@MarlonGamez MarlonGamez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit: @tejal29 clarified this for me offline, ignore

This may be a dumb question but, what is the difference between using 'remote' vs 'tag'? Since they both go down the same code path now

pkg/skaffold/runner/build_deploy.go Outdated Show resolved Hide resolved
Copy link
Contributor

@MarlonGamez MarlonGamez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just one nit to update the comment

Co-authored-by: Marlon Gamez <marlongamez@google.com>
@fengye87
Copy link
Contributor Author

fengye87 commented Mar 1, 2021

@tejal29 I've updated the comment as @MarlonGamez suggested, could you start another kokoro run please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants