<HTML><BODY><br><br><br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">
Четверг, 30 января 2020, 18:49 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:<br><br><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15803993760665546273_BODY">CCed Oleg.<br><br>
Oleg, please look at the commit message: whether it is understandable<br>
w/o much context? If not, please, work together with Alexander on it.<br><br>
I almost didn't look into the script (because I guess I'll no give much<br>
value here and because Igor already did).<br><br>
I have several small comments, but I don't against the patch at whole.<br><br>
WBR, Alexander Turenko.<br></div></div></div></div></blockquote>Oleg, I've added the answers, please proceed with check.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15803993760665546273_BODY"><br>
> The changes introduce new Gitlab-CI rules for creating packages on<br>
> branches with "-full-ci" suffix and their subsequent deployment to the<br>
> 'live' repository for master and release branches. Packages for tagged<br>
> commits are also delivered to the corresponding 'release' repository.<br>
> <br>
> The PackageCloud storage is replaced with the new self-hosted one<br><br>
Packagecloud rules are not removed (and this is what we planned to do at<br>
the moment), so the word 'replaced' looks confusing here.<br></div></div></div></div></blockquote><p>Corrected, as:<br></p><p> Packages creation activities relocating from the PackageCloud storage<br> to the new self-hosted MCS S3 where all old packages have been synced.</p><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15803993760665546273_BODY"><br>
> (based on S3 object storage) where all old packages have been synced.<br>
> The new builds will be pushed only to S3 based repos. Benefits of the<br><br>
This is not so, we don't plan to disable packagecloud repositories right<br>
now.</div></div></div></div></blockquote>Removed the comment.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15803993760665546273_BODY"><br><br>
> introduced approach are the following:<br>
> * As far as all contents of self-hosted repos are fully controlled<br>
> theirs layout is the same as the ones provided by the corresponding<br>
> distro<br>
> * Repo metadata rebuild is excess considering the known repo layout<br>
> * Old packages are not pruned since they do not affect the repo<br>
> metadata rebuilding time<br>
> <br>
> For these purposes the standalone script for pushing DEB and RPM<br>
> packages to self-hosted repositories is introduced. The script<br>
> implements the following flow:<br>
> * creates new metafiles for the new packages<br>
> * copies new packages to S3 storage<br>
> * fetches relevant metafiles from the repo<br>
> * merges the new metadata with the fetched one<br>
> * pushes the updated metadata to S3 storage<br><br>
Is some blocking mechanism implemented? What will going on if two CI<br>
jobs will fetch metadata, update them locally and them push back? Is it<br>
depends on whether those CI jobs are run on the same machine or<br>
different ones?<br></div></div></div></div></blockquote>Using lock files with the owner pid in it, the any next process checks the lock<br>file existence and if so gives it 60 seconds timeout to finish it's job after which<br>it forces to kill it to avoid of hanged processes.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15803993760665546273_BODY"><br>
> <br>
> There are distro dependent parts in the script:<br>
> * For RPM packages it updates metadata separately per each repo<br>
> considering 'createrepo' util behaviour<br>
> * For DEB packages it updates metadata simultaniously for all repos<br>
> considering 'reprepro' util behaviour<br><br>
What 'repo' means here? We have 1.10 repo. But it contains centos/7,<br>
debian/jessie and other repos. I don't understand this paragraph, to be<br>
honest.<br><br>
Aside of that, we always update only one repository (say, 1.10 x<br>
centos/7) at the moment. 'Simultaneous update' looks even more<br>
confusing.<br></div></div></div></div></blockquote><p>This part just says that RPM packages updates for each distribution separately,<br>because meta files exist for each distribution, while DEB packages have only<br>single metafile for the all distributions within single OS.</p><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15803993760665546273_BODY"><br>
> <br>
> Closes #3380<br>
> <br>
> @TarantoolBot<br>
> Title: Update download instructions on the website<br>
> <br>
> Need to update download instructions on the website, due to the new<br>
> repository based on MCS S3.<br><br>
First, it is stub, not a request to the documentation team. What actions<br>
you are requested here?<br><br>
Second, since we decided to keep current repositories structure<br>
(separate 1.10/2.1/... repos and no version in a package name) no<br>
actions are required actually.<br></div></div></div></div></blockquote>Removed.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15803993760665546273_BODY"><br>
> ---<br>
> <br>
> Github: <a href="https://github.com/tarantool/tarantool/tree/avtikhon/gh-3380-push-packages-s3-full-ci" target="_blank">https://github.com/tarantool/tarantool/tree/avtikhon/gh-3380-push-packages-s3-full-ci</a><br>
> Issue: <a href="https://github.com/tarantool/tarantool/issues/3380" target="_blank">https://github.com/tarantool/tarantool/issues/3380</a><br>
> <br>
> v6: <a href="https://lists.tarantool.org/pipermail/tarantool-patches/2020-January/013763.html" target="_blank">https://lists.tarantool.org/pipermail/tarantool-patches/2020-January/013763.html</a><br>
> v5: <a href="https://lists.tarantool.org/pipermail/tarantool-patches/2020-January/013636.html" target="_blank">https://lists.tarantool.org/pipermail/tarantool-patches/2020-January/013636.html</a><br>
> v4: <a href="https://lists.tarantool.org/pipermail/tarantool-patches/2020-January/013568.html" target="_blank">https://lists.tarantool.org/pipermail/tarantool-patches/2020-January/013568.html</a><br>
> v3: <a href="https://lists.tarantool.org/pipermail/tarantool-patches/2019-December/013060.html" target="_blank">https://lists.tarantool.org/pipermail/tarantool-patches/2019-December/013060.html</a><br>
> v2: <a href="https://lists.tarantool.org/pipermail/tarantool-patches/2019-November/012352.html" target="_blank">https://lists.tarantool.org/pipermail/tarantool-patches/2019-November/012352.html</a><br>
> v1: <a href="https://lists.tarantool.org/pipermail/tarantool-patches/2019-October/012021.html" target="_blank">https://lists.tarantool.org/pipermail/tarantool-patches/2019-October/012021.html</a><br><br>
> diff --git a/.gitlab.mk b/.gitlab.mk<br>
> index 48a92e518..243f83f2c 100644<br>
> --- a/.gitlab.mk<br>
> +++ b/.gitlab.mk<br>
> @@ -98,14 +98,38 @@ vms_test_%:<br>
> vms_shutdown:<br>
> VBoxManage controlvm ${VMS_NAME} poweroff<br>
> <br>
> -# ########################<br>
> -# Build RPM / Deb packages<br>
> -# ########################<br>
> +# ###########################<br>
> +# Sources tarballs & packages<br>
> +# ###########################<br><br>
But it is about packages, not tarballs. It seems you copied the comment<br>
from .travis.yml, but it is not appropriate here.<br></div></div></div></div></blockquote>I've meant the sources in real, but if it is not appropriate I'm removing it.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15803993760665546273_BODY"><br>
> +<br>
> +# Push alpha and beta versions to <major>x bucket (say, 2x),<br>
> +# stable to <major>.<minor> bucket (say, 2.2).<br>
> +GIT_DESCRIBE=$(shell git describe HEAD)<br>
> +MAJOR_VERSION=$(word 1,$(subst ., ,$(GIT_DESCRIBE)))<br>
> +MINOR_VERSION=$(word 2,$(subst ., ,$(GIT_DESCRIBE)))<br>
> +BUCKET=$(MAJOR_VERSION)_$(MINOR_VERSION)<br>
> +ifeq ($(MINOR_VERSION),0)<br>
> +BUCKET=$(MAJOR_VERSION)x<br>
> +endif<br>
> +ifeq ($(MINOR_VERSION),1)<br>
> +BUCKET=$(MAJOR_VERSION)x<br>
> +endif<br><br>
Let's push to 2.1, we'll add 2x for compatibility on using redirects on<br>
download.tarantool.org.<br></div></div></div></div></blockquote>Ok, removed redirections to 2x bucket.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15803993760665546273_BODY"><br>
> <br>
> package: git_submodule_update<br>
> git clone <a href="https://github.com/packpack/packpack.git" target="_blank">https://github.com/packpack/packpack.git</a> packpack<br>
> PACKPACK_EXTRA_DOCKER_RUN_PARAMS='--network=host' ./packpack/packpack<br>
> <br>
> +deploy: package<br>
> + for key in ${GPG_SECRET_KEY} ${GPG_PUBLIC_KEY} ; do \<br>
> + echo $${key} | base64 -d | gpg --batch --import || true ; done<br><br>
I guess we need just secret key to signing.<br></div></div></div></div></blockquote>To use the secret key for signing it has to be imported into the user's environment.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15803993760665546273_BODY"><br>
> + ./tools/update_repo.sh -o=${OS} -d=${DIST} \<br>
> + -b="s3://tarantool_repo/live/${BUCKET}" build<br><br>
I would hide name of an S3 bucket under a CI variable. It is more about<br>
our infrastructure and it would be good to show less in the repo.<br></div></div></div></div></blockquote>S3 bucket name is opened and visible in the logs, there is no need to hide it and<br> to pay the attention for it, due to name is not secret at all, while the additional<br> secret values setup will be the dead code or additional manual steps.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15803993760665546273_BODY"><br>
> + for tag in $$(git tag) ; do \<br>
> + git describe --long $${tag} ; \<br>
> + done | grep "^$$(git describe --long)$$" >/dev/null && \<br><br>
Simpler way:<br><br>
| git name-rev --name-only --tags --no-undefined HEAD 2>/dev/null</div></div></div></div></blockquote>Ok, changed.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15803993760665546273_BODY"><br><br>
See <a href="https://stackoverflow.com/a/11489642/1598057" target="_blank">https://stackoverflow.com/a/11489642/1598057</a><br><br>
> + ./tools/update_repo.sh -o=${OS} -d=${DIST} \<br>
> + -b="s3://tarantool_repo/release/${BUCKET}" build<br><br>
> +function get_os_dists {<br>
> + os=$1<br>
> + alldists=<br>
> +<br>
> + if [ "$os" == "ubuntu" ]; then<br>
> + alldists='trusty xenial bionic cosmic disco eoan'<br>
> + elif [ "$os" == "debian" ]; then<br>
> + alldists='jessie stretch buster'<br>
> + elif [ "$os" == "el" ]; then<br>
> + alldists='6 7 8'<br>
> + elif [ "$os" == "fedora" ]; then<br>
> + alldists='27 28 29 30 31'<br>
> + fi<br><br>
We have no Fedora 27 in CI. Should it be here? I don't mind, just found<br>
the tiny inconsistency.<br></div></div></div></div></blockquote>Just wanted to have the same availability matrix of the OS/DISTS that<br>packagecloud repository has.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15803993760665546273_BODY"><br>
> +<br>
> + echo "$alldists"<br>
> +}<br>
> +<br>
> +function prepare_ws {<br>
> + # temporary lock the publication to the repository<br>
> + ws_suffix=$1<br>
> + ws=${ws_prefix}_${ws_suffix}<br>
> + ws_lockfile=${ws}.lock<br>
> + if [ -f $ws_lockfile ]; then<br>
> + old_proc=$(cat $ws_lockfile)<br>
> + fi<br>
> + lockfile -l 60 $ws_lockfile<br>
> + chmod u+w $ws_lockfile && echo $$ >$ws_lockfile && chmod u-w $ws_lockfile<br>
> + if [ "$old_proc" != "" -a "$old_proc" != "0" ]; then<br>
> + kill -9 $old_proc >/dev/null || true<br>
> + fi<br><br>
So the current script can be killed by another instance of the script?<br>
This means that the lock does not work.</div></div></div></div></blockquote>No, it means that the common path used by the previous process should be free from it<br>in 60 seconds timeout. Also discussed in QA chat and proved that it works.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15803993760665546273_BODY"><br><br>
> +<br>
> + # create temporary workspace with repository copy<br>
> + $rm_dir $ws<br>
> + $mk_dir $ws<br><br>
Am I understand right: we don't create a copy of remote repository?</div></div></div></div></blockquote>Right, it is the copy of the newly created repository meta files.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15803993760665546273_BODY"><br></div></div></div></div></blockquote>
<br>
<br>-- <br>Alexander Tikhonov<br></BODY></HTML>