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

Add publishing docker image by gradle in release workflow #159

Merged
merged 4 commits into from
Jul 28, 2020

Conversation

xfiderek
Copy link
Contributor

Summary

  • Refactored publishing docker image in release workflow
  • Previously images were being published by github action
  • Now they're published using gradle

Other Information
Workflow tested on mock repository in both possible cases (hotfix/non hotfix), but note that during tests different image (simple dockerfile) was being published on different repository (with different credentials).

./gradlew dockerTagsPush -P version=${{ steps.bump_dry.outputs.tag }}

- name: "4-2. Publish non-hotfix docker image"
if: ${{ !contains(github.event.head_commit.message, '#hotfix') }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens in case of #patch in commit message?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually also - what happens in case of hotfix itself?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe versioning logic should be documented somewhere (not for this pr) as it looks like it is not easy to deduce, at least for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens in case of #patch in commit message?

There was not any specific behaviour for #patch commit before my changes as far as I can see

Copy link
Contributor Author

@xfiderek xfiderek Jul 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually also - what happens in case of hotfix itself?

In hotfix we skip publishing docker image with tag 'latest', since hotfix (according to my conversation with @endrjuskr ) refers to changes in one of previous versions

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely something we have to concretely specify somewhere. Because the two latest hotfixes changed the latest release..

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hotfixe is something that applies to older version that we still support. For instance v1 and v2. In our cases it was more like patch or minor, so that is why we released latest.

- name: "3. Determine docker tags"
run: echo ::set-env name=DOCKER_TAGS::$(echo latest, ${{ steps.bump_dry.outputs.tag }})
- name: "3. Login to docker hub"
run: "echo '${{ secrets.DOCKER_HUB_TOKEN }}' | docker login --username ${{ secrets.DOCKER_HUB_USER }} --password-stdin"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you do docker login --username ${{ secrets.DOCKER_HUB_USER }} --password ${{ secrets.DOCKER_HUB_TOKEN }}" ? In such case you will leave pasword in logs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we have to pass the secret with env like here? https://docs.github.com/en/actions/configuring-and-managing-workflows/creating-and-storing-encrypted-secrets#example-using-bash

yes, probably you are right. I will change it soon

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you do docker login --username ${{ secrets.DOCKER_HUB_USER }} --password ${{ secrets.DOCKER_HUB_TOKEN }}" ? In such case you will leave pasword in logs

According to docker login documentation using --password-stdin avoids logging credentials, but correct me if I misunderstood it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is when you cat a file and then pipe it. In our case we echo it, so it will be visible. In a solution that I proposed it will as well because it inject it to command

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- name: "3. Determine docker tags"
run: echo ::set-env name=DOCKER_TAGS::$(echo latest, ${{ steps.bump_dry.outputs.tag }})
- name: "3. Login to docker hub"
run: "echo '${{ secrets.DOCKER_HUB_TOKEN }}' | docker login --username ${{ secrets.DOCKER_HUB_USER }} --password-stdin"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you do docker login --username ${{ secrets.DOCKER_HUB_USER }} --password ${{ secrets.DOCKER_HUB_TOKEN }}" ? In such case you will leave pasword in logs

build.gradle Outdated
@@ -16,7 +16,8 @@ task build {
// docker & docker run
docker {
name "unit8/darts"
tags version, "latest"
//it will also automatically create image with tag 'latest'
tag version, "unit8/darts:${version}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will it create three tags? why we need tag version ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's explicit say latest, rather than assume it is automatic

Copy link
Contributor Author

@xfiderek xfiderek Jul 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will it create three tags? why we need tag version ?

It will create two tags - 'latest' and 'x.y.z' (where 'x.y.z' is release version)

version is variable that is passed to gradle after resolving current release tag by github action. So if release tag is 0.3.0 version will be replaced by 0.3.0, and gradle will build docker image with two tags, 'latest' and '0.3.0' ('latest' tag is always attached automatically). Maybe there is better way to parse current version to gradle. If so, I am open to any further suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so why we need this - "unit8/darts:${version}" ? Is it a formatter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also think that it is a bit unclear, but I will do my best to explain it. Basically "tags" is deprecated and now it is replaced by "tag". So, we can add any number of following lines:
tag x, y
Now, x is internal name, some sort of alias, whereas y is image:tag that corresponds to image name and demanded tag, so for example we can specify following line:
tag unicorn, unit8/darts:0.3.0
Purpose of this key, value correspondence is to refer to created tags by keys. For example, we can then call:
./gradlew dockerPushUnicorn
And image with tag 0.3.0 (unit8/darts:0.3.0) will be pushed. It can be useful if we have multiple tags and want to push only subset of them. I really recommend reading Task section in plugin's README.

- name: "4-2. Publish non-hotfix docker image"
if: ${{ !contains(github.event.head_commit.message, '#hotfix') }}
run: |
./gradlew dockerPush -P version=${{ steps.bump_dry.outputs.tag }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference between dockerPush and dockerTagsPush? It is not intuitive what each method does

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference between dockerPush and dockerTagsPush? It is not intuitive what each method does

As you can see above in gradle.build file in docker plugin we specify only tag version, but I've mentioned before that gradle automatically creates also tag 'latest'. dockerPush pushes image with all created tags, wheareas dockerTagsPush publish image only with specified tags, so dockerTagsPush will push image only with one tag - version, while dockerPush will push image with both version and 'latest' tags. I've disscussed value of version variable in previous comments.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use always dockerTagsPush? to make it more explicit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use always dockerTagsPush? to make it more explicit?

We can create two tags, one for latest and one for current version and use a dockerPush${version} and optionally dockerPushLatest (See here). In such case only thing that we should agree on is how to pass version variable to gradle. I see two possibilities:

  1. Pass version to gradle as property from CLI in workflow (current implementation)
  2. Set version as enviromental variable in workflow and use System.env.version in gradle to retrieve it.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend passing it as property. Reading something from env in gradle is not visible at first and might confuse people

@xfiderek xfiderek merged commit 217e595 into develop Jul 28, 2020
@xfiderek xfiderek deleted the feature/gradle_docker_pub branch July 28, 2020 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants