Multi-tag support for push operation and convention plugins #867

bmuschko · 2019-10-06T14:47:37Z

This change takes a step-wise approach for the work started in #835.

This is a breaking change and will be delivered with the next major version. The property for defining a single tag has been removed without a deprecation cycle. Unify other custom tasks to simplify and unify API.

In its current implementation there's a one to one map from built tags to pushed tags. A later PR might introduce logic to make the push operation more selective about which tags to push.
The API of the push task didn't align with the build task. Separating the repository from the tag led to misunderstandings from end users. Unifying both APIs makes a much easier to hand over the tags property values. The same goes for other tasks.
Removed the method DockerPullImage.getImageId(). The returned value didn't really represent the image ID and therefore wasn't accurate. In fact you'd have to represent a list of image IDs if done right. You can simply ask for the Provider value of the tag property if required.

This is a breaking change. The property for defining a single tag has been removed without a deprecation cycle. Unify other custom tasks to simplify and unify API.

Just use one String to represent both values. In most cases, users are use to that notation anyway.

orzeh

My only concern with this feature is that we break an assumption that each task is an atomic interaction with Docker API. If we have 3 tags and pushing the second one fails, the build ends up in inconsistent state, meaning you don't know which tags were pushed successfully. I guess we have to provide some hook for users to compensate in case of an error. But probably we can add this later if someone asks for this.

orzeh · 2019-10-06T21:07:10Z

src/main/groovy/com/bmuschko/gradle/docker/tasks/image/DockerPushImage.groovy

-            logger.quiet "Pushing image with name '${imageName.get()}'."
-        }
+            if(registryCredentials) {
+                AuthConfig authConfig = new AuthConfig()


AuthConfig can be created only once outside each loop?

Going to look into it.

orzeh · 2019-10-06T21:08:08Z

src/main/groovy/com/bmuschko/gradle/docker/tasks/image/DockerPushImage.groovy

@@ -47,49 +41,45 @@ class DockerPushImage extends AbstractDockerRemoteApiTask implements RegistryCre

    @Override
    void runRemoteCommand() {
-        PushImageCmd pushImageCmd = dockerClient.pushImageCmd(imageName.get())
+        tags.get().each { currentTag ->
+            logger.quiet "Pushing image with name '${currentTag}'."


Maybe we should change name to tag.

Good idea. Will change that. Technically, tag is the wrong term here as well as we are talking about <repository>/<image-name>:<tag>. I don't think there's really a blanket term in Docker that would cover all of this.

I would argue just to say Pulling image ${currentImage} (goes inline with comments I made above).

bmuschko · 2019-10-07T00:27:29Z

My only concern with this feature is that we break an assumption that each task is an atomic interaction with Docker API. If we have 3 tags and pushing the second one fails, the build ends up in inconsistent state, meaning you don't know which tags were pushed successfully. I guess we have to provide some hook for users to compensate in case of an error. But probably we can add this later if someone asks for this.

Docker Java doesn't offer an API for providing multiple tags at the moment. I am not sure how we'd make this atomic. Let's say the first push worked but the second push failed, I am not sure how we'd properly handle the situation. I am not aware of a call that can delete a remote Docker image so there's no real rollback or atomic commit functionality.

I am not sure how a user-facing callback could help. There's nothing a user can do to delete a remote image either. Maaaaybe write out a custom error message but I am hoping that Docker Java provides a useful message here.

Let's me know how you think this could work as atomic operation and we can discuss options.

orzeh · 2019-10-07T07:34:07Z

Docker Java doesn't offer an API for providing multiple tags at the moment. I am not sure how we'd make this atomic. Let's say the first push worked but the second push failed, I am not sure how we'd properly handle the situation. I am not aware of a call that can delete a remote Docker image so there's no real rollback or atomic commit functionality.

Probably that's the reason why this option is not built into Docker itself :)

I guess we don't have to worry about it that much, but maybe we should just add one sentence to the User Guide so that people are aware.

cdancy · 2019-10-07T16:26:40Z

src/docs/samples/code/application-plugin/basic/groovy/build.gradle

@@ -12,7 +12,7 @@ docker {
        baseImage = 'dockerfile/java:openjdk-7-jre'
        maintainer = 'Benjamin Muschko "benjamin.muschko@gmail.com"'
        ports = [9090, 5701]
-        tag = 'jettyapp:1.115'
+        tags = ['jettyapp:1.115']


Maybe provide multiple tags here so that users will know how things work? Possibly the same comment for other doc-related changes.

cdancy · 2019-10-07T16:30:31Z

src/main/groovy/com/bmuschko/gradle/docker/tasks/image/DockerCommitImage.groovy

-     */
-    @Input
-    @Optional
-    final Property<String> repository = project.objects.property(String)


If we're removing this property a "better" name might be to just use image as that technically encompasses the entire URL. It's at least what is used within the docker docs all over the place.

cdancy · 2019-10-11T13:22:32Z

LGTM and I like the new functionality. Have at least one place we'll be able to take advantage of this internally for which we have custom code to do this (though admittedly not as clean :) )

bmuschko added 2 commits October 6, 2019 08:22

API should allow for pushing multiple tags

33838b8

This is a breaking change. The property for defining a single tag has been removed without a deprecation cycle. Unify other custom tasks to simplify and unify API.

Just assign the Provider without resolving value

11c409d

bmuschko added the enhancement label Oct 6, 2019

bmuschko added this to the v6.0.0 milestone Oct 6, 2019

bmuschko requested review from cdancy and orzeh October 6, 2019 14:47

bmuschko self-assigned this Oct 6, 2019

Do not separate repository from tag

a8e3ac7

Just use one String to represent both values. In most cases, users are use to that notation anyway.

orzeh approved these changes Oct 6, 2019

View reviewed changes

cdancy reviewed Oct 7, 2019

View reviewed changes

bmuschko added 4 commits October 10, 2019 17:14

Create instance only once

bc37b34

Use the term "image" to represent repository, image name and tag

b969dbf

Use multiple tags in examples

f51c777

Exercise pushing multiple tags to registries

ceb41f5

cdancy approved these changes Oct 11, 2019

View reviewed changes

bmuschko merged commit 8963279 into master Oct 14, 2019

bmuschko deleted the bm/multi-tag-support branch October 14, 2019 00:33

bmuschko mentioned this pull request Oct 14, 2019

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

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multi-tag support for push operation and convention plugins #867

Multi-tag support for push operation and convention plugins #867

bmuschko commented Oct 6, 2019 •

edited

Loading

orzeh left a comment

orzeh Oct 6, 2019

bmuschko Oct 7, 2019

bmuschko Oct 11, 2019

orzeh Oct 6, 2019

bmuschko Oct 7, 2019

cdancy Oct 7, 2019

bmuschko Oct 11, 2019

bmuschko commented Oct 7, 2019

orzeh commented Oct 7, 2019

cdancy Oct 7, 2019

bmuschko Oct 11, 2019

cdancy Oct 7, 2019

bmuschko Oct 11, 2019

cdancy commented Oct 11, 2019

Multi-tag support for push operation and convention plugins #867

Multi-tag support for push operation and convention plugins #867

Conversation

bmuschko commented Oct 6, 2019 • edited Loading

orzeh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmuschko commented Oct 7, 2019

orzeh commented Oct 7, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdancy commented Oct 11, 2019

bmuschko commented Oct 6, 2019 •

edited

Loading