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

Can't specify multiple tags in JavaApplication and SpringBootApplication plugins #834

Closed
tanelso2 opened this issue Jun 21, 2019 · 9 comments
Assignees
Milestone

Comments

@tanelso2
Copy link

Expected Behavior

When using the JavaApplication plugin, it would be nice to be able to specify multiple image tags.

Current Behavior

The property is called tag, and can only hold one thing.

Context

We like using the JavaApplication plugin to abstract out most of the building of Docker images. It makes it especially easy to port things to Kubernetes for us. However, if we want multiple tags on a Docker image, we have to add a whole separate DockerTagImage task. This works, but it isn't ideal. We would like to have a simpler and cleaner method of doing that. Changing the tag property to a tags property seems like the most straight-forward way of accomplishing this.

Your Environment

Plugin version 4.10.0

@orzeh orzeh self-assigned this Jun 21, 2019
@orzeh
Copy link
Collaborator

orzeh commented Jun 21, 2019

@tanelso2 this was already asked before in #507 and there was even a PR #505 but we didn't concluded that.

I believe this could be an useful feature, however it solves only half of a problem since you can't push those tags. This was also discussed in #461 and #638. But maybe adding this is better then nothing. @cdancy @bmuschko any thoughts?

Regarding your PR - thanks for the efforts! I'll put me comments there.

@cdancy
Copy link
Collaborator

cdancy commented Jun 21, 2019

@orzeh I've always liked the idea as we do something similar here with multiple tags for different reasons. Looking over those older ISSUE's/PR's the main concern from @bmuschko , and rightfully so, was that multiple tasks take in and/or use the "tag" option. With that in mind we should amend all existing tasks which use tags to take in a list of them and proceed accordingly.

@cdancy cdancy added this to the v5.0.0 milestone Jun 21, 2019
@cdancy cdancy self-assigned this Jun 21, 2019
@bmuschko
Copy link
Owner

@orzeh I'd rather not merge the pull request as it will be confusing to users if you can build an image with multiple tags but then the push operation only pushes one. BTW: Which one? ;-)

I think we should request that Docker Java adds a way to push multiple tags at once before we proceed. The alternative is to dynamically create multiple push tasks in the plugin based on the number of provided tags. It would mean more maintenance cost for this project.

@tanelso2
Copy link
Author

@bmuschko, isn't part of the point of having a Gradle plugin the ability to autogenerate tasks?

Most of the time the user will just ./gradlew publish, and it doesn't matter to them how many tasks actually get run, just as long as what they expect gets done.

@bmuschko
Copy link
Owner

@tanelso2 That wasn't really my point. You can do pretty much anything in a plugin. My concern was about the maintenance cost of the code going forward for the plugin developers, not the consumers.

@bmuschko
Copy link
Owner

As a side note: Pushing multiple tagged images at once with a single command isn't supported yet. So if we wanted to implement support in the plugin then we'd have to create multiple push tasks.

@orzeh
Copy link
Collaborator

orzeh commented Jun 24, 2019

As a side note: Pushing multiple tagged images at once with a single command isn't supported yet. So if we wanted to implement support in the plugin then we'd have to create multiple push tasks.

@bmuschko because of the above I think we should implement this logic in our plugin. We already had many requests for this feature and the maintenance cost shouldn't be high.

@bmuschko
Copy link
Owner

A couple of things to consider when we say "maintenance cost shouldn't be to high":

  1. We should abstract the convention plugins first so that most of the code is reusable. We got the code in a place now that it should be possible to avoid too much duplication.
  2. Dynamically created task name can't be used to create task dependencies on them after the fact by end users. Simple reason: They will be created very late in the build lifecycle as you need to evaluate the extension first. Almost no way around afterEvaluate which I'd like to avoid introducing.
  3. We should keep an aggregation task that depends on the dynamically created push tasks.

@bmuschko
Copy link
Owner

Addressed in #867.

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

No branches or pull requests

4 participants