[Tarantool-patches] [PATCH v1] packages: tagged commit force push to live repo
Alexander V. Tikhonov
avtikhon at tarantool.org
Thu Oct 15 00:06:15 MSK 2020
Hi Alexander, thanks a lot for the deep investigation. I've checked your
solution and it looks much better than my, so I've completely used your
fix instead of mine.
On Wed, Oct 14, 2020 at 05:42:58AM +0300, Alexander Turenko wrote:
> 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 |
More information about the Tarantool-patches
mailing list