[Tarantool-patches] [PATCH v1] Make S3 paths the same to Tarantool documented

Alexander Turenko alexander.turenko at tarantool.org
Tue Feb 18 20:08:50 MSK 2020


> Make S3 paths the same to Tarantool documented

Please, add a prefix: gitlab-ci, infra or so. Before we use 'gitlab-ci'.

'S3 paths' does not say anything about change you doing. Is it about
tarballs? Or packages? Or some other part of infrastructure?

> Changed the Vanilla paths of the packages in S3 to
> the Tarantool documented on its web site, to give
> the ability to Tarantool users to continue use it
> w/o changes.

Here you introduce the term 'the Vanilla paths' (which is used as
'Vanila' below), but it is neither commonly known nor explained. In fact
you meant base URL of main CentOS/Fedora repository. Since we providing
our own repository it obviously will have its own base URL.

> 
> Also moved the RPM sources packages to the standalone
> path.
> 
> Follows up #3380

Maybe like so:

 | gitlab-ci: adjust base URL of RPM/Deb repositories
 |
 | Our S3 based repositories now reflect packagecloud.io repositories
 | structure.
 |
 | It will allow us to migrate from packagecloud.io w/o much complicating
 | redirection rules on a web server serving download.tarantool.org.
 |
 | Deploy source packages (*.src.rpm) into separate 'SRPM' repository
 | like packagecloud.io does.
 |
 | <everything else you change>
 | 
 | Follows up #3380

See, I didn't introduced any new term, but described why and how the
behaviour of the script was changed.

> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-3380-rpm-packages-paths-full-ci
> Issue: https://github.com/tarantool/tarantool/issues/3380
> 
>  tools/update_repo.sh | 73 +++++++++++++++++++++++++++++++-------------
>  1 file changed, 51 insertions(+), 22 deletions(-)
> 
> diff --git a/tools/update_repo.sh b/tools/update_repo.sh
> index 65a977187..ec6146514 100755
> --- a/tools/update_repo.sh
> +++ b/tools/update_repo.sh
> @@ -405,42 +405,68 @@ EOF
>  }
>  
>  # The 'pack_rpm' function especialy created for RPM packages. It works
> -# with RPM packing OS like Centos, Fedora. It is based on globaly known
> +# with RPM packing OS like CentOS, Fedora. It is based on globaly known

If you want to fix typos, then fix also 'globaly'.

>  # tool 'createrepo' from:
> -#     https://linux.die.net/man/8/createrepo
> +#   https://linux.die.net/man/8/createrepo
>  # This tool works with single distribution of the given OS.
> -# Result of the routine is the rpm package for YUM repository with
> -# file structure equal to the Centos/Fedora:
> -#     http://mirror.centos.org/centos/7/os/x86_64/Packages/
> -#     http://mirrors.kernel.org/fedora/releases/30/Everything/x86_64/os/Packages/t/
> +#
> +# Vanilla RPM packages structure for YUM repositories looks like:
> +#   CentOS:
> +#     Binaries: http://mirror.centos.org/centos/7/os/x86_64/Packages/
> +#     Sources: http://vault.centos.org/8.1.1911/BaseOS/Source/SPackages/
> +#  Fedora:
> +#     Binaries: http://mirrors.kernel.org/fedora/releases/30/Everything/x86_64/os/Packages/t/
> +#     Sources: http://mirrors.kernel.org/fedora/releases/30/Everything/source/tree/Packages/t/
> +# Decided to have back port capability at Tarantool links:
> +#   CentOS:
> +#     Tarantool: https://www.tarantool.io/en/download/os-installation/rhel-centos/
> +#     Changed Vanila -> MCS S3:
> +#       centos/7/os/x86_64/Packages -> centos/7/x86_64/Packages
> +#       8.1.1911/BaseOS/Source/SPackages -> centos/8.1.1911/SRPMS/Packages
> +#   Fedora:
> +#     Tarantool: https://www.tarantool.io/en/download/os-installation/fedora/
> +#     Changed Vanila -> MCS S3:
> +#       fedora/releases/30/Everything/x86_64/os/Packages -> fedora/30/x86_64/Packages
> +#       fedora/releases/30/Everything/source/tree/Packages -> fedora/30/SRPMS/Packages

If we agree to don't bother anybody to imagine what 'Vanilla/Vanila' is
(and what is meant under 'MCS S3', which is just file storage), then
let's just show base URLs.

However it is okay to give links to intructions, which will allow to
verify that the URLs are right.

>  function pack_rpm {
> -    if ! ls $repo/*.rpm >/dev/null ; then
> -        echo "ERROR: Current '$repo' path doesn't have RPM packages in path"
> -        usage
> -        exit 1
> -    fi
> +    pack_subdir=$1
> +    pack_patterns=$2
> +
> +    for pack_pattern in $pack_patterns ; do
> +        if ! ls $repo/$pack_pattern >/dev/null ; then
> +            if [ "$pack_pattern" == "*.noarch.rpm" ]; then
> +                pack_patterns=$(echo "$pack_patterns" | sed "s#$pack_pattern##g")
> +                continue
> +            fi
> +            echo "ERROR: Current '$repo' path doesn't have $pack_pattern packages in path"
> +            usage
> +            exit 1
> +        fi
> +    done

This logic is flawed (say, it will not work with vshard). Okay, let's
give an error if there are no files, but it should not require files to
be present for a certain architecture.

I would collect files using globs. Then check whether we found at least
one. Then use this list of files. A kind of this:

 | # Collect RPM files matching provided glob.
 | rpms=''
 | for glog in ${globs}; do
 |     for file in ${repo}/${glob}; do
 |         rpms="${rpms} ${file}"
 |     done
 | done
 |
 | # Verify that at least one RPM file is found.
 | if [ -z "${rpms}" ]; then
 |     <...>
 | fi

But feel free to pick up any way, I'll not push you for mine one.

>  
>      # prepare the workspace
>      prepare_ws ${os}_${option_dist}
>  
>      # copy the needed package binaries to the workspace
> -    cp $repo/*.rpm $ws/.
> +    for pack_pattern in $pack_patterns ; do
> +        cp $repo/$pack_pattern $ws/.
> +    done

After the change above it can be simplified to:

 | cp ${rpms} "$ws/"

Note: no quotes around ${rpms} to use file glob expansion.

>  
>      pushd $ws
>  
>      # set the paths
> -    if [ "$os" == "el" ]; then
> -        repopath=$option_dist/os/x86_64
> -        rpmpath=Packages
> -    elif [ "$os" == "fedora" ]; then
> -        repopath=releases/$option_dist/Everything/x86_64/os
> -        rpmpath=Packages/$proddir
> +    repopath=$option_dist/$pack_subdir
> +    rpmpath=Packages
> +    if [ "$os" == "fedora" ]; then
> +        rpmpath=$rpmpath/$proddir
>      fi

I don't think that we need different layout for CentOS and Fedora.
Packagecloud.io has the same flat layout for both.

>      packpath=$repopath/$rpmpath
>  
>      # prepare local repository with packages
>      $mk_dir $packpath
> -    mv *.rpm $packpath/.
> +    for pack_pattern in $pack_patterns ; do
> +        mv $pack_pattern $packpath/.
> +    done

Same as above.

>      cd $repopath
>  
>      # copy the current metadata files from S3
> @@ -547,8 +573,10 @@ EOF
>      gpg --detach-sign --armor repodata/repomd.xml
>  
>      # copy the packages to S3
> -    for file in $rpmpath/*.rpm ; do
> -        $aws_cp_public $file "$bucket_path/$repopath/$file"
> +    for pack_pattern in $pack_patterns ; do
> +        for file in $rpmpath/$pack_pattern ; do
> +            $aws_cp_public $file "$bucket_path/$repopath/$file"
> +        done

Same as above.

>      done
>  
>      # update the metadata at the S3
> @@ -563,7 +591,8 @@ EOF
>  if [ "$os" == "ubuntu" -o "$os" == "debian" ]; then
>      pack_deb
>  elif [ "$os" == "el" -o "$os" == "fedora" ]; then
> -    pack_rpm
> +    pack_rpm x86_64 "*.x86_64.rpm *.noarch.rpm"
> +    pack_rpm SRPMS "*.src.rpm"

It deserves a comment.

>  else
>      echo "USAGE: given OS '$os' is not supported, use any single from the list: $alloss"
>      usage
> -- 
> 2.17.1
> 

And while we're here: I would extract explicit keyid to an environment
variable (it is a least in 'SignWith:').

I would also use `gpg -u <keyid> <...>` when signing, because it is
possible that there is more then one key for a user/machine.


More information about the Tarantool-patches mailing list