[Tarantool-patches] [PATCH v7] gitlab-ci: implement packing into MCS S3

Alexander Tikhonov avtikhon at tarantool.org
Fri Jan 31 07:59:33 MSK 2020




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


-- 
Alexander Tikhonov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20200131/2345d346/attachment.html>


More information about the Tarantool-patches mailing list