Adopt wider tagging model

[winem]

This PR tags built images with the major st2 version and major.minor st2 version and pushs those.

This was tested with ST2_VERSION variations like 3.3, 3.3.0, 3.4dev, 3.4.0dev

So, although I don't expect all the mentioned cases above being relevant, we're covered for any ST2_VERSIONs like that.

One of the challenges with the make limitations was that you can't have two lines like ifeq ($(RELEASE_VERSION), true) for the same target. That's why I added dedicated sections at the end of each target handling the new tags.

I'll keep a last set of make build and make push tests running while going to bed now and mark this PR as draft until tomorrow. I'll remove the draft flag if these tests are successfull, too.

And I'll create another PR to add the latest tag as soon as this one got merged into the master.

close #31
closes #35
closes StackStorm/st2#5139

[winem]

Ready for review.

[arm4b]

Thanks. The major/minor version parsing logic looks pretty clean 👍

Let's imagine we have the following Docker tags present in Docker hub:

• 3.4 - dynamic, refers to 3.4.1
• 3.4.0 - immutable
• 3.4.1 - immutable

In this situation, triggering the Docker build + deploy for 3.4.0 shouldn't update the 3 or 3.4 tags.
However triggering the Docker build + deploy for 3.4.1 (or new release) should update those references.
So the script needs to fetch the Docker Hub API, find the latest version present, sort and take that into account.

Otherwise we could push some older/outdated Docker images under dynamic tag (like 3.4) by mistake.

I suggest to look at scripts we had before and adopt it for the new model:

• https://github.com/StackStorm/st2-docker/blob/2c2032595a03394b56fbf9cc16114d823a540471/bin/common.sh
• https://github.com/StackStorm/st2-docker/blob/2c2032595a03394b56fbf9cc16114d823a540471/bin/deploy.sh
• https://github.com/StackStorm/st2-docker/blob/2c2032595a03394b56fbf9cc16114d823a540471/bin/gatekeeper.sh

Let's also include the VERSIONING.md in the PR to make it super clear for the users.

[winem]

Hi,

thanks for the review & feedback. I probably won't have much time to proceed on this topic this weekend as I have to prepare a talk and a hackathon but I'll proceed latest end of next week.

My current idea is to have a shell script similar to the ones you linked from the st2-docker repo which takes the version of the current build as argument and compares the provided version with the ones available at Dockerhub. The response of the script will be processed by the Makefile to determine whether the additional tags need to be set or not.

Let me know if you see any issues with the concept or have better ideas.

[arm4b]

@winem Sounds good, I like the logic you described more comparing to what we had before. Thanks for the work in this area!

[winem]

The script to parse and compare the releases takes a bit more time than expected but it'll also be suitable for the other issue where we want to start using the latest tag.

@armab, can it happen, that we release a new version (i.e. a hotfix) for an older release? Let me describe what I mean:
Latest release at Dockerhub: 3.4
Latest 3.3-patch release at DH: 3.3.4
New release 3.4.1 comes with new features and let's say an important security fix (-> the image will get the tags latest, 3 and 3.4) - no questions so far
Now let's say we port that super important security fix back to 3.3 as well. So we build 3.3.5 with that fix and the image should only have the tags 3.3 and 3.3.5 but neither the latest nor 3 tag.

Is this a realistic case or something that we generally avoid and don't do in the st2 project? I hope it's clear what I mean.

Just asking not to miss a test case. :)

[arm4b]

Between all these versions:

3.3.1
3.3.2
3.3 - refers to 3.3.2
3.4.0
3.4.1
3.4 - refers to 3.4.1
latest - refers to v3.4.1

the latest should always refer to latest stable version, eg v3.4.1, even if we release a security patch for the older v3.3.3 at some point. And yes, if that happens (security patch for and older version) , - 3.3.3 should update 3.3 tag as well.
I guess that's what you described too.

https://github.com/StackStorm/st2-docker/blob/2c2032595a03394b56fbf9cc16114d823a540471/VERSIONING.md

[arm4b]

Look at https://github.com/StackStorm/st2-dockerfiles/tree/v3.2 or https://github.com/StackStorm/st2-dockerfiles/tree/v3.3 branches.
We could rebuild the images for those branches at any time for existing older version (example https://github.com/StackStorm/st2-dockerfiles/blob/v3.2/Makefile#L1) or if we decide to release new patch version there.

It's just a matter of restarting a CircleCI build in that branch (https://app.circleci.com/jobs/github/StackStorm/st2-dockerfiles/628) which will deploy to Docker Hub.

[winem]

Thanks, all clear now!

[winem]

All tests passed so far. I'll just add some extra error handling in case the determine_needed_tags.sh fails before removing the Draft flag from this PR and call it ready for review.

[arm4b]

Sure, feel free to adjust the CircleCI config to install curl and jq.

[winem]

Sorry, I forgot that I can do it by myself. Will do it and let you know when it's done. Thanks!

[winem]

I pushed some improvements on the error handling and additional validation of the variables. The CI run is still expected to fail as I did not push the new circleci config yet. I'll do that once I saw that the errors are handled as expected now.

[arm4b]

Thanks!

Interesting that even after the latest changes CircleCI build (https://app.circleci.com/pipelines/github/StackStorm/st2-dockerfiles/474/workflows/5dd3a84c-67f4-43f8-8eb1-5ead13c2cf4c/jobs/992) still reports syntax errors while I guess it should stop at dependencies check:

Build Docker Images:

#!/bin/sh -eo pipefail
make build

./determine_needed_tags.sh: line 47: [: curl: binary operator expected
./determine_needed_tags.sh: line 78: curl: command not found
./determine_needed_tags.sh: line 80: [: -eq: unary operator expected
./determine_needed_tags.sh: line 84: [: -ne: unary operator expected
./determine_needed_tags.sh: line 90: curl: command not found
./determine_needed_tags.sh: line 91: jq: command not found
./determine_needed_tags.sh: line 95: available_releases: bad array subscript
./determine_needed_tags.sh: line 106: [: 3.2.0: unary operator expected
./determine_needed_tags.sh: line 111: [: 3: unary operator expected
./determine_needed_tags.sh: line 157: [: 3: unary operator expected

[winem]

Hi, this was caused by the variable expansion of ${missing_packages} in bash and I fixed it.

But I would love to hear your input regarding a design question.

Now, the issue is that for curl and jq are missing, so determine_needed_tags.sh fails with exit 1 and returns Requirement(s) not satisfied: curl jq which becomes the value of the TAG_UPDATE_FLAG in the makefile. See: https://app.circleci.com/pipelines/github/StackStorm/st2-dockerfiles/478/workflows/2c234ed4-f0c7-4ad8-aca3-9f2202e18321/jobs/996

The code I pushed just now does not cause the make command to fail but prints an error that no tags were set. Is this ok or do we want the CI run to fail, too?

I do have a few options in mind to let the make command fail, too, but this requires some refactoring of the makefile. I could get it done this week in the evenings to have it ready before the code freeze for 3.4.

[arm4b]

I think would be best to stop the entire process at early stage once we understood that script failed instead of waiting for the entire Docker build.

[winem]

Now all failures in the script should be handled and return a message with the "Error:"-Prefix and the makefile checks the output for the Error-Prefix and fails before building any images.

Another idea was to introduce another return value like -1 in case of an error but then I wouldn't know how to show the error message in the CI logs. If you have an idea to show the output from the script in the CI log please let me know.

I just pushed a Makefile with 3.3.0 and will change it back to 3.4dev before merging this branch. That's just because the determine tags script is skipped if we build a *dev version and not a release.

[arm4b]

Depending on "Error" message as a failure indicator adds some knowledge overhead in requirements that could be broken in the future. What if 3rd Maintainer comes, edit the determine_needed_tags.sh script and doesn't add "Error" message for the failure case.

We can at least reinforce it with determine_needed_tags.sh || echo Error to fit your solution.

[winem]

So, this one failed as expected because curl and jq were not installed: https://app.circleci.com/pipelines/github/StackStorm/st2-dockerfiles/482/workflows/f8a8c4b3-a6f5-493a-a17d-70f88f686d80/jobs/1000

This one for 3.3.0 was successful and tagged the images as expected:
https://app.circleci.com/pipelines/github/StackStorm/st2-dockerfiles/485/workflows/48cac0b0-436b-4d7f-8c83-e34766361f43/jobs/1003

And this one for 3.4dev was successful as well. No additional tags were set because it's a dev-build. So from my side, this should be fine now.

[arm4b]

Gave it a few more testing cycles and it works really well now.
Thanks for the diligence and extra work catching all the corner cases 👍

It was really important to make sure we don't accidentally overwrite the wrong Docker tag in prod/release.