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

Feature: Add 'tag' input to allow creating tags based on upstream versions #18

Closed
LongLiveCHIEF opened this issue Nov 25, 2020 · 16 comments · Fixed by #24
Closed

Feature: Add 'tag' input to allow creating tags based on upstream versions #18

LongLiveCHIEF opened this issue Nov 25, 2020 · 16 comments · Fixed by #24

Comments

@LongLiveCHIEF
Copy link

I maintain several repositories where the docker repo is separate from the application repo. I wrote an action a while back HackerHappyHour/tagging-strategy that had a lot of the same intentions of this action, and I'd like to archive it in favor of this action.

The only feature my action has that isn't yet implemented here is the ability to (optionally) supply a tag value, instead of getting the tag from the repository.

This allows upstream repo's to dispatch on tagging/releases, and and for the docker repo to build/publish tags when that happens.

Example where we do this: github.com/OctoPrint/Octoprint triggers github.com/OctoPrint/octoprint-docker.

This goes especially well with #15, which was the other feature missing from this action, but seems like @hugopeixoto beat me to it!

I'd be willing to contribute this feature, just need approved suggestion on the input name. I'm thinking:

inputs:
  # other inputs
  input_tag:
    description: 'if provided will use the supplied value. Not compatible with all inputs (see README.md for compatibility)'
    required: false

Then for docs adding a 4th column in the inputs table in the README, specifying if compatible with input_tag, along with notes like

using input_tag is for scenarios when using external events to trigger builds is required such as repository_dispatch or workflow_dispatch

@hugopeixoto
Copy link

Even though I wrote #15, I ended up not using this action because of the exact same reason you mention. I was thinking about implementing this as well :P
This is the github action where I was planning on using it: https://github.com/hedgedoc/container/blob/master/.github/workflows/release.yml

I wasn't sure which name I'd give the input. For my use case, I was thinking of adding version, but maybe git_tag would be a good name? It clears up the ambiguity between docker tags and git tags.

@crazy-max
Copy link
Member

@hugopeixoto @LongLiveCHIEF Yeah that LGTM! Feel free to open a PR. I will soon merge #15 ;)
About the input name I would prefer something more conform with the current inputs implemented like tags-input that could be the same type as images.

@LongLiveCHIEF
Copy link
Author

The other thing I'm thinking is that since this introduces the concept of external events, we could have a problem with latest tag.

From reading docs, it seems that currently latest tag is implicitly generated when a push or push tag event occurs, or if you have supplied a tag-match.

With this scenario, where you supply the tag instead of getting it from an event, we'll need to make sure the user has the ability to disable latest from being generated while still using tag-semver.

About the input name I would prefer something more conform with the current inputs implemented like tags-input that could be the same type as images.

I think it's better to use a singular string type input here rather than a list, as the recommendation would be to use a matrix to supply multiple input tags, since it's very likely each input tag has different build conditions.

For example let's say you supplied a an emergency security patch, which might be a scenario where you would build multiple major versions:

tags-input: 2.1.1,3.0.1

Now, when you get to building the image, both of those would generate the respective list of tags, but would be passed to the same build command... which isn't usually what you want since major versions are often passed different build contexts.

However, using a matrix:

job:
  release:
    strategy:
      matrix:
        input-tags: ['2.1.1', '3.0.1'
    steps:
      -
        uses: docker-meta
        with:
          tag-input: ${{ matrix.input-tags }}

This will allow the build and subsequent tests to pass/fail on their own merits for each major tag.

Then again, I guess we could support a csv/list and then the user can decide when it's better to use a matrix with a single-value-list vs a list of input-tags.

ok yeah.... disregard everything I just said... i'm going with that.

@crazy-max
Copy link
Member

@LongLiveCHIEF

I think it's better to use a singular string type input here rather than a list

Why not a list then? If you only want tag then put one tag, if someone else want more tags let's do that. I don't see any blocking case about a list.

since it's very likely each input tag has different build conditions.

Maybe in your case but let say a user want 2.1.1,2.1 that looks quiet reasonable to me.

@crazy-max
Copy link
Member

crazy-max commented Nov 25, 2020

@LongLiveCHIEF Sorry didn't see the end of your post ahah

@LongLiveCHIEF
Copy link
Author

@crazy-max I think you missed the last 2 lines of my comment 🤣

@hugopeixoto the action I wrote supports both variants and upstream triggers, but the downside is that it was designed to be used by a matrix, because i didn't know buildx could support multiple tags at the time.

Depending on your needs, you might be able to use that action until I get this feature added here.

@crazy-max
Copy link
Member

crazy-max commented Nov 25, 2020

@LongLiveCHIEF

didn't know buildx could support multiple tags at the time.

Standard docker build support multi tags too. Do you have a link to a workflow?

@LongLiveCHIEF
Copy link
Author

https://github.com/OctoPrint/octoprint-docker/blob/master/.github/workflows/octoprint-release.yml

That's what I'm doing currently, and it was built before the docker official actions were supported. It used your buildx action and my aforementioned tagging-strategy action.

Earlier today i started migrating to the new docker actions, which should improve that workflow quite a bit if I can get this feature added over the next few days.

@LongLiveCHIEF
Copy link
Author

...and here is the work in progress so far: https://github.com/OctoPrint/octoprint-docker/blob/update-usage-set-env-in-workflows/.github/workflows/octoprint-release.yml

Note: it's broken because I'm not done and there are places where the refactor is only half done, but you can kinda see where I'm going with it.

@crazy-max
Copy link
Member

@LongLiveCHIEF

Argh that's what I thought, your tags are tampered (different digest for each because it's a whole new build through your matrix strategy):

image

So the following tags are not linked between each other by their manifest. You should only have one digest between them like this:

image

In your case each image for each tag is written to disk on the registry.

To demonstrate this behavior you can pull octoprint/octoprint:1.5.0 and octoprint/octoprint:1.5 and it will download the image twice. You're lucky Docker doesn't have a size limit retention on their registry ahah. I strongly suggest to change this on your repo. This will be more effective for the end user.

@LongLiveCHIEF
Copy link
Author

well yeah, that's why i'm doing what I'm doing. 😃

@LongLiveCHIEF
Copy link
Author

LongLiveCHIEF commented Nov 25, 2020

well yeah, that's why i'm doing what I'm doing.

And by that I mean migrating to use multiple tags in a single build.

@crazy-max
Copy link
Member

@LongLiveCHIEF

well yeah, that's why i'm doing what I'm doing. 😃

Oh ok that's a limitation of your action, got it!

@LongLiveCHIEF
Copy link
Author

I have started to work on this, but will probably wait to see how the discussion in #19 pans out before submitting a PR, since if we decide that change would be better, then it would be much easier to write this feature after that change is made.

@LongLiveCHIEF
Copy link
Author

@hugopeixoto while we're finishing discussions here, I've created a pre-release version of my tagging-strategy action that supports multiple tag outputs and should do everything you want. https://github.com/marketplace/actions/tagging-strategy?version=2.0.0-rc2

@crazy-max
Copy link
Member

@LongLiveCHIEF #24

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants