Skip to content

fix: fetch images by platform tag when using --platform#7393

Merged
gsquared94 merged 3 commits intoGoogleContainerTools:mainfrom
tejal29:fix_retagging_pl
May 12, 2022
Merged

fix: fetch images by platform tag when using --platform#7393
gsquared94 merged 3 commits intoGoogleContainerTools:mainfrom
tejal29:fix_retagging_pl

Conversation

@tejal29
Copy link
Copy Markdown
Contributor

@tejal29 tejal29 commented May 11, 2022

Fix fetching digest by platform.

This is carry over of PR #7301 from an external contributor. They opened it against the v1 branch.
Initial commit belongs to #3559 with rebase on main after v2 merge.
Second commit is tests fixed.

@tejal29 tejal29 added this to the v1.39.0 milestone May 11, 2022
@tejal29 tejal29 requested a review from a team as a code owner May 11, 2022 19:22
@tejal29 tejal29 requested a review from MarlonGamez May 11, 2022 19:22
@tejal29 tejal29 changed the title Fix retagging pl fix: fetch images by platform tag when using --platform May 11, 2022
@codecov
Copy link
Copy Markdown

codecov bot commented May 11, 2022

Codecov Report

Merging #7393 (27cb6f5) into main (290280e) will decrease coverage by 3.85%.
The diff coverage is 55.51%.

@@            Coverage Diff             @@
##             main    #7393      +/-   ##
==========================================
- Coverage   70.48%   66.62%   -3.86%     
==========================================
  Files         515      566      +51     
  Lines       23150    27120    +3970     
==========================================
+ Hits        16317    18069    +1752     
- Misses       5776     7755    +1979     
- Partials     1057     1296     +239     
Impacted Files Coverage Δ
cmd/skaffold/app/cmd/credits/export.go 0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/deploy.go 50.00% <0.00%> (-3.85%) ⬇️
cmd/skaffold/app/cmd/test.go 44.44% <0.00%> (ø)
cmd/skaffold/skaffold.go 0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/render.go 36.66% <12.50%> (-4.72%) ⬇️
cmd/skaffold/app/cmd/inspect_tests.go 62.50% <14.28%> (-1.14%) ⬇️
cmd/skaffold/app/cmd/lsp.go 28.12% <28.12%> (ø)
cmd/skaffold/app/cmd/fix.go 54.09% <38.88%> (-22.38%) ⬇️
cmd/skaffold/app/cmd/verify.go 41.17% <41.17%> (ø)
cmd/skaffold/app/cmd/lint.go 42.85% <42.85%> (ø)
... and 290 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Copy link
Copy Markdown
Contributor

@gsquared94 gsquared94 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for taking care of this @tejal29

@gsquared94 gsquared94 merged commit f08a832 into GoogleContainerTools:main May 12, 2022
gsquared94 pushed a commit that referenced this pull request May 13, 2022
* fix: Do not ignore supplied platform when retagging an image

* fix tests

* fix lint'

Co-authored-by: Bar Perach <barp@google.com>
gsquared94 pushed a commit that referenced this pull request May 13, 2022
* fix: Do not ignore supplied platform when retagging an image

* fix tests

* fix lint'

Co-authored-by: Bar Perach <barp@google.com>
// Retrieve the digest for that tag.
// TODO(dgageot): I don't think GCB can push to an insecure registry.
return docker.RemoteDigest(defaultToTag, b.cfg)
return docker.RemoteDigest(defaultToTag, b.cfg, util.ConvertToV1Platform(platforms.Platforms[0]))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hi @tejal29. This line is causing a panic when there are no platforms specified. I'm hoping you can add some context to the open issue #7587 or suggest a fix? Thanks!

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