-
Notifications
You must be signed in to change notification settings - Fork 485
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: make annotations work with push flag #2098
Conversation
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine. I'd just like to see the line where --push
get translated so I can better understand how things fit together.
for _, e := range opts.Exports { | ||
for k, v := range annotations { | ||
e.Attrs[k.String()] = v | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So here --push
hasn't been translated to an export entry. Can you just link me the line where this happens for learning purposes?
for _, o := range outputs { | ||
for k, v := range annotations { | ||
o.Attrs[k.String()] = v | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's delayed here (so --push
is a true alias for --output=type=registry
) and that's why this works.
Please ignore my previous (deleted) comment, which is nonsensical. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like a better approach would be to remove ExportPush/ExportLoad
from controller (and then likely new Annotations
as well). But controller API is not backwards compatible so not a blocker.
related to #2020
When implementing annotations in our build-push-action (docker/build-push-action#992), I found out they were not working: https://github.com/docker/build-push-action/actions/runs/6642453793/job/18047273130#step:7:24
Annotations work with
--output
but it doesn't when using--push
. This moves the parsing annotations logic to the controller so we can set annotations after--push
has been converted to an export entry.