Re: [Tarantool-patches] [PATCH v1] packages: tagged commit force push to live repo

[Alexander Turenko <alexander.turenko@tarantool.org>]

In brief:

The commit fixes the problem with pushing to the release repositories
(no x.y.z.0 package + red CI), but introduces the problem with pushing
to the live repositories (no x.y.(z-1).rrr package + red CI). I would
say, it decreases urgency of the problem. However:

1. It is not good to have red CI due to infrastructure problems in the
   day of a release.
2. Races still affect a set of packages we deploy, that's disgusting.
3. One more commit in `git log` that I unable to undestand without
   diving into a lot of subtle details.

There is proposal how to overcome all problems and I don't see downsides
or complexities. I already described it voicely and written in the
issue. It is as simple as:

 | [ -z "${CI_COMMIT_TAG:-}" ] && \
 |     git tag -d "$(git tag --points-at HEAD)" 2>/dev/null || true

I would let Kirill to decide how much known to be problematic partial
fixes we should create before actually fix the root problem (as shown
above).

Let's dive into details.

----

`git tag --points-at` is supported since git 1.7.10 (8 years ago).

Aside of `git tag --points-at` there is also `git describe
--exact-match` since git 1.5.5 (10 years ago). It's commit message have
suggestion how to do the same on older versions ([1]).

My previous suggestion about `git name-rev` was just mistake. I'm sorry.
Now I found that it does not guarantee that it'll point to a tag that
points to a given commit. It may be some other tag:

 | $ git name-rev --name-only --tags --no-undefined c2d8c03ee
 | 2.6.0~2
 |
 | $ git tag --points-at c2d8c03ee | cat
 | 2.5.1
 |
 | $ git describe --exact-match c2d8c03ee
 | 2.5.1

Okay, if you're bother about the next CI jobs, `git fetch` will fetch
tags by default: I verified it. You may add explicit --tags (-t). You
may also want to add --prune-tags in addition to --prune to get rid of
stale tags if we'll step into the undesired situation when a tag is
changed somehow.

See details below.

[1]: https://github.com/git/git/commit/2c33f7575452f53382dcf77fdc88a2ea5d46f09d

WBR, Alexander Turenko.

> packages: tagged commit force push to live repo

There are no commits with such prefix, but there are 24 commits made
during this year with 'gitlab-ci' prefix, which fit good here.

>
> If the commit tagged, then push packages to 'live' repository with force
> push '-f -s' flags to be sure that if occasionaly the commit before current
> commit pushed packages with the current branch naming then the current push
> won't fail on it and will push the packages to the release repository after.
> 
> Closes #3745

We discussed it as the temporary workaround for the problem. Let's be
realistic, will you return back here, if we'll close the issue?

> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-3745-force-push-live
> Issue: https://github.com/tarantool/tarantool/issues/3745
> 
>  .gitlab.mk | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/.gitlab.mk b/.gitlab.mk
> index e1a83ac07..808288f23 100644
> --- a/.gitlab.mk
> +++ b/.gitlab.mk
> @@ -120,11 +120,18 @@ package: deploy_prepare
>  
>  deploy: package
>  	echo ${GPG_SECRET_KEY} | base64 -d | gpg --batch --import || true
> -	./tools/update_repo.sh -o=${OS} -d=${DIST} \
> -		-b="${LIVE_REPO_S3_DIR}/${BUCKET}" build
> +	# If the commit tagged, then push packages to 'live' repository with force
> +	# push '-f -s' flags to be sure that if occasionaly the commit before current
> +	# commit pushed packages with the current branch naming then the current push
> +	# won't fail on it and will push the packages to the release repository after.

-s (skip error) is to pass over a clash, -f (force) is to replace a
package in the case. Those options are kinda opposite, right?

I just checked the code: -s does not change anything when -f is present.

So it looks for me as a good way to create a confusion for no reason at
all.

>  	if [ "${CI_COMMIT_TAG}" != "" ]; then \
> +		./tools/update_repo.sh -o=${OS} -d=${DIST} \
> +			-b="${LIVE_REPO_S3_DIR}/${BUCKET}" -f -s build ; \
>  		./tools/update_repo.sh -o=${OS} -d=${DIST} \
>  			-b="${RELEASE_REPO_S3_DIR}/${BUCKET}" build ; \
> +	else \
> +		./tools/update_repo.sh -o=${OS} -d=${DIST} \
> +			-b="${LIVE_REPO_S3_DIR}/${BUCKET}" build ; \
>  	fi

How it works now (before your patch):

 |               | branch push job | tag job                   |
 | ------------- | --------------- | ------------------------- |
 | x.y.(z-1).rrr | push live       | not possible              |
 | x.y.z.0       | push live [^1]  | push live && release [^1] |

[^1]: Those jobs will obviously clash. Everything depends on the order:
      as they will be scheduled. Possible variants:

      Scenario 1:

       | (branch push job)  push live x.y.z.0
       | (tag job)          fail on push live x.y.z.0
       |                    => no release x.y.z.0
       |                    => red CI

      Scenario 2:

       | (tag job)          push live x.y.z.0
       |                    push release x.y.z.0
       | (branch push job)  fail on push live x.y.z.0
       |                    => red CI

--------

As it would work after the full resolution that I proposed (drop a
current tag if "${CI_COMMIT_TAG}" is empty or not defined):

 |               | branch push job | tag job                   |
 | ------------- | --------------- | ------------------------- |
 | x.y.(z-1).rrr | push live       | not possible              |
 | x.y.z.0       | not possible    | push live && push release |

--------

The alternative would be detection of a tagged commit on a branch job
and skip the job when we found this situation. It'll give the following
behaviour:

 |               | branch push job | tag job                   |
 | ------------- | --------------- | ------------------------- |
 | x.y.(z-1).rrr | push live       | not possible              |
 | x.y.z.0       | skip [^2]       | push live && push release |

[^2]: In fact this x.y.z.0 build is some x.y.(z-1).rrr, but it was not
      lucky and was scheduled after arriving of the x.y.z tag to the
      mirror. So we'll miss live pushes and, what is weird, randomly. We
      can get x.y.(z-1).rrr pushed for one distro, but missed for
      another.

This variant is not the best, but at least it will push all packages to
the release repositories and will not lead to any extra fails in CI with
package name clashes.

--------

Now let's look at the behaviour you propose:

                 | branch push job | tag job                         |
 | ------------- | --------------- | ------------------------------- |
 | x.y.(z-1).rrr | push live       | not possible                    |
 | x.y.z.0       | push live [^3]  | push-force live && push release |

[^3]: So, we there are two possible situations:

      Scenario 1:

       | (branch push job)  push live x.y.z.0
       | (tab job)          push-force live x.y.z.0
       |                    push release x.y.z.0
       |                    => missed x.y.(z-1).rrr, but
       |                       everything else is okay

      Scenario 2:

       | (tag job)          push-force live x.y.z.0
       |                    push release x.y.z.0
       | (branch push job)  fail on push live x.y.z.0
       |                    => red CI
       |                    => missed x.y.(z-1).rrr

      I doubt GitLab CI provides any implicit guarantees about ordering
      of jobs. Everything is highly depend on timings, so unlikely
      someone may say that the scenario 2 is not possible or have a
      little probability.

--------

Regarding `git tag --points-at` vs `git describe --exact-match` vs `git
name-rev <...>`.

I made some tests:

 | #!/bin/sh
 | 
 | set -e
 | 
 | revisions="" comments=""
 | revisions="${revisions} e7203e389" comment_0="current master"
 | revisions="${revisions} d5ec7e948" comment_1="just after 2.6.0"
 | revisions="${revisions} 47aa4e01e" comment_2="on 2.6.0"
 | revisions="${revisions} 7bfcf57e4" comment_3="just before 2.6.0 (after 2.5.1)"
 | revisions="${revisions} c2d8c03ee" comment_4="on 2.5.1"
 | revisions="${revisions} 807c7fa58" comment_5="just before 2.5.1"
 | revisions="${revisions} d1647590e" comment_6="2.5.0.82 (between 2.5.0 and 2.5.1)"
 | revisions="${revisions} ad13b6d57" comment_7="just after 2.5.0"
 | revisions="${revisions} fef6505c7" comment_8="on 2.5.0"
 | revisions="${revisions} ca79e1cfb" comment_9="just before 2.5.0"
 | 
 | i=0
 | for rev in ${revisions}; do
 |     # Get a tag.
 |     #
 |     # tag=''
 |     # version="$(git describe --long --always "${rev}")"
 |     # case "${version}" in
 |     # *.*.*-0-g*)
 |     #     tag="${version%%-*}"
 |     #     ;;
 |     # esac
 | 
 |     # Get a tag (bashism)
 |     #
 |     # tag=''
 |     # version="$(git describe --long --always "${rev}")"
 |     # [[ $version =~ [0-9]+\.[0-9]+\.[0-9]+-0-g ]] && tag="${version%%-*}"
 | 
 |     comment_ref="comment_${i}"
 |     printf '| %-5s | %-5s | %-9s | %-34s |\n' \
 |         "$(git tag --points-at "${rev}" 2>/dev/null)" \
 |         "$(git describe --exact-match "${rev}" 2>/dev/null)" \
 |         "$(git name-rev --name-only --tags --no-undefined "${rev}" 2>/dev/null)" \
 |         "${!comment_ref}"
 |     let i=i+1
 | done

It gives the following output:

 |       |       |           | current master                     |
 |       |       |           | just after 2.6.0                   |
 | 2.6.0 | 2.6.0 | 2.6.0^0   | on 2.6.0                           |
 |       |       | 2.6.0~1   | just before 2.6.0 (after 2.5.1)    |
 | 2.5.1 | 2.5.1 | 2.6.0~2   | on 2.5.1                           |
 |       |       | 2.6.0~3   | just before 2.5.1                  |
 |       |       | 2.6.0~199 | 2.5.0.82 (between 2.5.0 and 2.5.1) |
 |       |       | 2.6.0~280 | just after 2.5.0                   |
 | 2.5.0 | 2.5.0 | 2.5.0^0   | on 2.5.0                           |
 |       |       | 2.5.0~1   | just before 2.5.0                  |