* [Tarantool-patches] [PATCH v1] packages: tagged commit force push to live repo @ 2020-10-08 23:13 Alexander V. Tikhonov 2020-10-14 2:42 ` Alexander Turenko 0 siblings, 1 reply; 4+ messages in thread From: Alexander V. Tikhonov @ 2020-10-08 23:13 UTC (permalink / raw) To: Kirill Yukhin, Leonid Vasiliev; +Cc: tarantool-patches, Alexander Turenko 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 --- 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. 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 source: deploy_prepare -- 2.25.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Tarantool-patches] [PATCH v1] packages: tagged commit force push to live repo 2020-10-08 23:13 [Tarantool-patches] [PATCH v1] packages: tagged commit force push to live repo Alexander V. Tikhonov @ 2020-10-14 2:42 ` Alexander Turenko 2020-10-14 21:06 ` Alexander V. Tikhonov 0 siblings, 1 reply; 4+ messages in thread From: Alexander Turenko @ 2020-10-14 2:42 UTC (permalink / raw) To: Alexander V. Tikhonov; +Cc: tarantool-patches 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 | ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Tarantool-patches] [PATCH v1] packages: tagged commit force push to live repo 2020-10-14 2:42 ` Alexander Turenko @ 2020-10-14 21:06 ` Alexander V. Tikhonov 2020-10-14 23:32 ` Alexander Turenko 0 siblings, 1 reply; 4+ messages in thread From: Alexander V. Tikhonov @ 2020-10-14 21:06 UTC (permalink / raw) To: Alexander Turenko; +Cc: tarantool-patches 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 | ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Tarantool-patches] [PATCH v1] packages: tagged commit force push to live repo 2020-10-14 21:06 ` Alexander V. Tikhonov @ 2020-10-14 23:32 ` Alexander Turenko 0 siblings, 0 replies; 4+ messages in thread From: Alexander Turenko @ 2020-10-14 23:32 UTC (permalink / raw) To: Alexander V. Tikhonov; +Cc: tarantool-patches On Thu, Oct 15, 2020 at 12:06:15AM +0300, Alexander V. Tikhonov wrote: > 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. Thank you for the kindly words. WBR, Alexander Turenko. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-10-14 23:31 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-08 23:13 [Tarantool-patches] [PATCH v1] packages: tagged commit force push to live repo Alexander V. Tikhonov 2020-10-14 2:42 ` Alexander Turenko 2020-10-14 21:06 ` Alexander V. Tikhonov 2020-10-14 23:32 ` Alexander Turenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox