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

build: do not attempt to push unnamed service images #10814

Merged
merged 1 commit into from
Jul 19, 2023

Conversation

milas
Copy link
Contributor

@milas milas commented Jul 17, 2023

What I did
When building, if images are being pushed, ensure that only named images (i.e. services with a populated image field) are attempted to be pushed.

Services without image get an auto-generated name, which will be a "Docker library" reference since they're in the format $project-$service, which is implicitly the same as docker.io/library/$project-$service. A push for that is never desirable / will always fail.

The key here is that we cannot overwrite the <svc>.image field when doing builds, as we need to be able to check for its presence to determine whether a push makes sense.

Related issue

(not mandatory) A picture of a cute animal, if possible in relation to what you did
otters on a water slide

@milas milas requested a review from a team July 17, 2023 14:40
@milas milas self-assigned this Jul 17, 2023
@milas milas requested review from nicksieger, ndeloof, StefanScherer, ulyssessouza, glours and laurazard and removed request for a team July 17, 2023 14:40
@codecov
Copy link

codecov bot commented Jul 17, 2023

Codecov Report

Patch coverage: 81.48% and project coverage change: +0.19 🎉

Comparison is base (5a072b1) 59.42% compared to head (636c13f) 59.62%.

Additional details and impacted files
@@            Coverage Diff             @@
##               v2   #10814      +/-   ##
==========================================
+ Coverage   59.42%   59.62%   +0.19%     
==========================================
  Files         115      115              
  Lines        9896     9899       +3     
==========================================
+ Hits         5881     5902      +21     
+ Misses       3426     3407      -19     
- Partials      589      590       +1     
Impacted Files Coverage Δ
cmd/compose/config.go 33.54% <0.00%> (+0.61%) ⬆️
pkg/api/api.go 42.30% <ø> (-1.09%) ⬇️
pkg/compose/pull.go 75.62% <33.33%> (-1.11%) ⬇️
pkg/compose/build.go 74.43% <100.00%> (+0.89%) ⬆️
pkg/compose/build_classic.go 52.56% <100.00%> (+0.30%) ⬆️
pkg/compose/viz.go 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@glours glours left a comment

Choose a reason for hiding this comment

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

LGTM

When building, if images are being pushed, ensure that only
named images (i.e. services with a populated `image` field)
are attempted to be pushed.

Services without `image` get an auto-generated name, which
will be a "Docker library" reference since they're in the
format `$project-$service`, which is implicitly the same as
`docker.io/library/$project-$service`. A push for that is
never desirable / will always fail.

The key here is that we cannot overwrite the `<svc>.image`
field when doing builds, as we need to be able to check for
its presence to determine whether a push makes sense.

Fixes docker#10813.

Signed-off-by: Milas Bowman <[email protected]>
@glours glours merged commit 7d88eda into docker:v2 Jul 19, 2023
25 checks passed
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.

3 participants