Skip to content

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

Merged
glours merged 1 commit intodocker:v2from
milas:fix-build-push
Jul 19, 2023
Merged

build: do not attempt to push unnamed service images#10814
glours merged 1 commit intodocker:v2from
milas:fix-build-push

Conversation

@milas
Copy link
Copy Markdown
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 StefanScherer, glours, laurazard, ndeloof, nicksieger and ulyssessouza and removed request for a team July 17, 2023 14:40
@codecov
Copy link
Copy Markdown

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
Copy Markdown
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
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.

3 participants