Re: [Tarantool-patches] [PATCH v1] Correct cleanup gitlab-ci

[Sergey Bronnikov <sergeyb@tarantool.org>]

Hi,

thanks for the patch!

On 08:55 Thu 11 Jun , Alexander V. Tikhonov wrote:
> Found the issue on regular testing hosts:
>   https://gitlab.com/tarantool/tarantool/-/jobs/577884238#L7

Could you move all web links in a text to a single place below a text
and refer to them by number? It would be easier to read in such case.
For example:

bla-bla [1]

1. https://google.com/

>   Fetching changes...
>   00:04
>    Reinitialized existing Git repository in
>      /home/gitlab-runner/builds/zzyC6hh5/0/tarantool/tarantool/.git/
>    Checking out 8ff7f32c as ...
>    warning: failed to remove CMakeFiles/Makefile.cmake
> 
> Found the job that saved the directories with root permissions
> https://gitlab.com/tarantool/tarantool/-/jobs/577768553
> 
> The issue appeared because the job that saved directories with root
> permissions used the 'shell' runner to run docker container inside.
> It caused the gitlab-runner to run the default workspace cleanup
> outside the docker container. In opposite to it, when 'docker' runner
> is used, the cleanup routine runs inside the docker container and no
> fails ever exist, because the root permissions are used in the docker
> container and this is the same root permissions for the host. As the
> result using 'shell' runner, cleanup routine failed to remove files
> created by root inside the docker container and which were shared to
> global host with the same permissions, because gitlab-runner runs the
> 'shell' runner by regular 'gitlab-runner' user, but not by root.
> 
> To fix the issue need to run docker containers using the gitlab-runner
> only in RO mode with Out-Of-Source builds in it. Either use the
> 'docker' runners when docker containers are needed. Anyway 'shell'
> runner jobs with additional calls to docker containers can't be
> control for the branches of developers, to avoid of it need to make
> local cleanup routine instead of default to the working paths for each
> job with 'shell' runners use.
> 
> Decided to setup gitlab-runner configuration as described in:
> https://docs.gitlab.com/ce/ci/yaml/README.html#git-clean-flags
> 
> Also got the issue with left data from previous builds at the
> submodule pathes, like here:
>   https://gitlab.com/tarantool/tarantool/-/jobs/574199256#L3141
> 
>    Undefined symbols for architecture x86_64:
>      "_u_isprint_66", referenced from:
>          _yaml_emitter_is_printable in libyaml_static.a(emitter.c.o)
>    ld: symbol(s) not found for architecture x86_64
>    clang: error: linker command failed with exit code 1 (...)
> 
> Also got issues with left files from previously tested branches,
> like here:
>   https://gitlab.com/tarantool/tarantool/-/jobs/590573606#L3718
> 
>  [087] small/rlist.test
>  [087] TAP13 parse failed (Missing plan in the TAP source).
>  [087]
>  [087] No result file (small/rlist.result) found.
>  [087] Run the test with --update-result option to write the new result file.
>  [087] [ fail ]
> 
>  [087] small/static.test
>  [087] TAP13 parse failed (Missing plan in the TAP source).
>  [087]
>  [087] No result file (small/static.result) found.
>  [087] Run the test with --update-result option to write the new result file.
>  [087] [ fail ]
> 
> To fix it was added the command to clean all available git submodules:
>   git submodule foreach git clean -ffdx

2. Duplicate '-f' option (here and in a patch)?

> 
> Closes #5036
> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-5036-gitlab-cleanup-1.10-full-ci
> Issue: https://github.com/tarantool/tarantool/issues/5036
> 
>  .gitlab-ci.yml | 37 ++++++++++++++++++++++++++++---------
>  1 file changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index cf445c638..592163b08 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -5,6 +5,16 @@ stages:
>  
>  variables:
>    GITLAB_MAKE: "make -f .gitlab.mk"
> +  GIT_CLEAN_COMMAND: "git clean -ffdx -e packpack && git submodule foreach git clean -ffdx && git submodule foreach git status"
> +  GIT_CLEAN_FLAGS: none
> +
> +.shell_before_script_template: &shell_cleanup_script
> +  before_script:
> +    - /bin/bash -c "${GIT_CLEAN_COMMAND}"

3. Why do you execute docker by specifying binary name and execute bash
with specifying full path to a binary? If "/bin" is absent in a PATH
then we can add to it. If full path is mandatory then let's move
'/bin/bash' to a variables.

> +
> +.docker_before_script_template: &docker_cleanup_script
> +  before_script:
> +    - docker run -w /source -v ${PWD}:/source -i packpack/packpack:el-7 /bin/bash -c "${GIT_CLEAN_COMMAND}"
>  
>  # Jobs templates
>  
> @@ -43,18 +53,21 @@ variables:
>    stage: test
>    tags:
>      - docker_test
> +  <<: *shell_cleanup_script
>  
>  .docker_test_clang8_template: &docker_test_clang8_definition
>    image: "${CI_REGISTRY}/${CI_PROJECT_PATH}/testing/debian-buster:latest"
>    stage: test
>    tags:
>      - docker_test
> +  <<: *shell_cleanup_script
>  
>  .pack_template: &pack_definition
>    <<: *pack_only_definition
>    stage: test
>    tags:
>      - deploy
> +  <<: *docker_cleanup_script
>    script:
>      - ${GITLAB_MAKE} package
>  
> @@ -63,6 +76,7 @@ variables:
>    stage: test
>    tags:
>      - deploy_test
> +  <<: *docker_cleanup_script
>    script:
>      - ${GITLAB_MAKE} package
>  
> @@ -71,6 +85,7 @@ variables:
>    stage: test
>    tags:
>      - deploy
> +  <<: *docker_cleanup_script
>    script:
>      - ${GITLAB_MAKE} deploy
>  
> @@ -79,12 +94,20 @@ variables:
>    stage: test
>    tags:
>      - deploy_test
> +  <<: *docker_cleanup_script
>    script:
>      - ${GITLAB_MAKE} deploy
>  
> +.osx_template: &osx_definition
> +  stage: test
> +  <<: *shell_cleanup_script
> +  script:
> +    - ${GITLAB_MAKE} test_osx
> +
>  .vbox_template: &vbox_definition
>    stage: test
>    before_script:
> +    - /bin/bash -c "${GIT_CLEAN_COMMAND}"
>      - ${GITLAB_MAKE} vms_start
>    after_script:
>      - ${GITLAB_MAKE} vms_shutdown
> @@ -98,6 +121,7 @@ variables:
>      paths:
>        - "*_result.txt"
>        - "*_t_version.txt"
> +  <<: *shell_cleanup_script
>    script:
>      - ${GITLAB_MAKE} perf_run
>  
> @@ -152,28 +176,22 @@ release_asan_clang8:
>  
>  osx_14_release:
>    <<: *release_only_definition
> -  stage: test
>    tags:
>      - osx_14
> -  script:
> -    - ${GITLAB_MAKE} test_osx
> +  <<: *osx_definition
>  
>  osx_15_release:
> -  stage: test
>    tags:
>      - osx_15
> -  script:
> -    - ${GITLAB_MAKE} test_osx
> +  <<: *osx_definition
>  
>  osx_15_release_lto:
>    <<: *release_only_definition
> -  stage: test
>    tags:
>      - osx_15
>    variables:
>      EXTRA_ENV: 'export CMAKE_EXTRA_PARAMS=-DENABLE_LTO=ON ;'
> -  script:
> -    - ${GITLAB_MAKE} test_osx
> +  <<: *osx_definition
>  
>  freebsd_12_release:
>    <<: *vbox_definition
> @@ -485,5 +503,6 @@ static_docker_build:
>    stage: test
>    tags:
>      - deploy_test
> +  <<: *docker_cleanup_script
>    script:
>      - ${GITLAB_MAKE} test_static_docker_build
> -- 
> 2.17.1
>