-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Add tags plugin to handle image tagging and deployment of tagged app images #1225
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
Conversation
|
Do images get tagged on deploy atm? Would be nice to do that automatically - heroku-style at least, could be optional. |
|
Tagging is not automatic at the moment. I wanted to provide the method to do it first before automating it. I am also not sure what makes sense as far as naming and retention. Automatic tagging could definitely eat up disk space quickly. |
|
Ah in that you'd have things to rollback to etc. Are there new pluginhooks for when an image is tagged? |
|
No new pluginhooks here |
|
GIVE ME PLUGINHOOKS |
|
plz |
|
Can you be more specific about what functionality you'd like? Example use-case? |
|
I'd like to be able to:
Basically it. |
|
Done. |
|
@josegonzalez thoughts on where to communicate to plugin devs that they need to get the image name + tag from get_running_image_tag() when developing non *-build plugins that require the image name? |
|
Uh what? I'm not sure I understand your question. |
|
Yeah sorry I was poorly multi-tasking when I wrote that. This PR exposes to the user control over the Does that make more sense? |
|
That sounds like a breaking change, and we should make a 0.4.0 release to handle it imo. |
|
Totally agree with you there. We should probably call it out in the release notes for 0.4.0 and update the plugin dev docs to call it out specifically. I'll do the latter now and update this PR. |
|
Anything else we want to do for 0.4.0? We may as well do that now-ish. |
|
what about buildkit and plugn? |
48996f9 to
8be5c87
Compare
|
@josegonzalez this needs another code review pass given its age |
docs/development/pluginhooks.md
Outdated
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.
Do all our built-in hooks follow this pattern?
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.
Just ones that use an image but depending on your thoughts on how $IMAGE is set we can revert this and make a new function call to get the app image name and tag.
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.
Yeah I'd rather it be clear as to how variables are set than placing magic in random places like this ;)
dokku
Outdated
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.
The output after this is duplicated in release_and_deploy I think.
c697116 to
9c37069
Compare
plugins/tag/commands
Outdated
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.
@michaelshobbs $ make you holler! 💸
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.
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.
points! well played
docs/process-management.md
Outdated
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.
Should this go in the deployment doc instead?
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.
yes. moving.
allow tagging and deployment of tagged app images
|
Image tagging == crushed. |
|
@michaelshobbs A few questions now that I'm using this (ffs I should have tested this before):
|
|
To close the loop here
If you want to push to a docker registry then use a plugin that implements the |
|
To setup a mobius loop:
Thanks @michaelshobbs for setting me straight :) |
as with most "simple" things, this was not as straightforward as we've been fairly lax about how $IMAGE is set.
$IMAGEtoverify_app_name()verify_app_name()to accept$TAG(image tag)get_running_image_tag()release/deploycalls torelease_and_deploy()$TAGto appropriate pluginhook callspre-deploydocker-args-deploydocker-args-runpre-releasepre-release-buildsteppre-release-dockerfilepost-releasepost-release-buildsteppost-release-dockerfile