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

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

[Alexander V. Tikhonov]

Found the issue on regular testing hosts:
  https://gitlab.com/tarantool/tarantool/-/jobs/577884238#L7

  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 issue caused job used the script
runner to run docker in it as additional command. It caused the
the gitlab-runner default workspace cleanup to be outside the
docker (in opposite to the jobs when docker runner used, the
cleanup runs inside the docker and no fails exists ever).
Cleanup failed to remove files created by root inside the docker
container before and which were shared to global host.

To fix the issue need to run docker images 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 script
runner jobs with additional calls to dockers can't be control for
the branches of developers, to avoid of it need to make local
cleanup instead of default to the working paths for each job with
script 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 (...)

To fix it was added the command to clean all available git submodules:
  git submodule foreach git clean -ffdx

Closes #5036
---

Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-5036-gitlab-cleanup-full-ci
Issue: https://github.com/tarantool/tarantool/issues/5036

 .gitlab-ci.yml | 76 ++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 67 insertions(+), 9 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 7705631dd..701008767 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -5,6 +5,15 @@ 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"
+
+.shell_cleanup_variables_template:
+  variables: &shell_cleanup_variable
+    GIT_CLEAN_FLAGS: none
+
+.shell_before_script_template: &shell_cleanup_script
+  before_script:
+    - docker run -w /source -v ${PWD}:/source -i packpack/packpack:el-7 /bin/bash -c "${GIT_CLEAN_COMMAND}"
 
 # Jobs templates
 
@@ -55,6 +64,7 @@ variables:
   stage: test
   tags:
     - deploy
+  <<: *shell_cleanup_script
   script:
     - ${GITLAB_MAKE} package
 
@@ -63,6 +73,7 @@ variables:
   stage: test
   tags:
     - deploy_test
+  <<: *shell_cleanup_script
   script:
     - ${GITLAB_MAKE} package
 
@@ -71,6 +82,7 @@ variables:
   stage: test
   tags:
     - deploy
+  <<: *shell_cleanup_script
   script:
     - ${GITLAB_MAKE} deploy
 
@@ -79,9 +91,17 @@ variables:
   stage: test
   tags:
     - deploy_test
+  <<: *shell_cleanup_script
   script:
     - ${GITLAB_MAKE} deploy
 
+.osx_template: &osx_definition
+  stage: test
+  before_script:
+    - ${GIT_CLEAN_COMMAND}
+  script:
+    - ${GITLAB_MAKE} test_osx
+
 .vbox_template: &vbox_definition
   stage: test
   before_script:
@@ -152,28 +172,27 @@ release_asan_clang8:
 
 osx_14_release:
   <<: *release_only_definition
-  stage: test
   tags:
     - osx_14
-  script:
-    - ${GITLAB_MAKE} test_osx
+  variables:
+    <<: *shell_cleanup_variable
+  <<: *osx_definition
 
 osx_15_release:
-  stage: test
   tags:
     - osx_15
-  script:
-    - ${GITLAB_MAKE} test_osx
+  variables:
+    <<: *shell_cleanup_variable
+  <<: *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
+    <<: *shell_cleanup_variable
+  <<: *osx_definition
 
 freebsd_12_release:
   <<: *vbox_definition
@@ -299,6 +318,9 @@ remove_images_sh9:
 
 sources:
   <<: *pack_definition
+  variables:
+    <<: *shell_cleanup_variable
+  <<: *shell_cleanup_script
   script:
     - ${GITLAB_MAKE} source
 
@@ -307,95 +329,113 @@ centos_6:
   variables:
     OS: 'el'
     DIST: '6'
+    <<: *shell_cleanup_variable
 
 centos_7:
   <<: *pack_test_definition
   variables:
     OS: 'el'
     DIST: '7'
+    <<: *shell_cleanup_variable
 
 centos_8:
   <<: *pack_test_definition
   variables:
     OS: 'el'
     DIST: '8'
+    <<: *shell_cleanup_variable
 
 fedora_28:
   <<: *pack_test_definition
   variables:
     OS: 'fedora'
     DIST: '28'
+    <<: *shell_cleanup_variable
 
 fedora_29:
   <<: *pack_test_definition
   variables:
     OS: 'fedora'
     DIST: '29'
+    <<: *shell_cleanup_variable
 
 fedora_30:
   <<: *pack_test_definition
   variables:
     OS: 'fedora'
     DIST: '30'
+    <<: *shell_cleanup_variable
 
 fedora_31:
   <<: *pack_test_definition
   variables:
     OS: 'fedora'
     DIST: '31'
+    <<: *shell_cleanup_variable
 
 ubuntu_14_04:
   <<: *pack_definition
   variables:
     OS: 'ubuntu'
     DIST: 'trusty'
+    <<: *shell_cleanup_variable
 
 ubuntu_16_04:
   <<: *pack_definition
   variables:
     OS: 'ubuntu'
     DIST: 'xenial'
+    <<: *shell_cleanup_variable
 
 ubuntu_18_04:
   <<: *pack_definition
   variables:
     OS: 'ubuntu'
     DIST: 'bionic'
+    <<: *shell_cleanup_variable
 
 ubuntu_19_10:
   <<: *pack_definition
   variables:
     OS: 'ubuntu'
     DIST: 'eoan'
+    <<: *shell_cleanup_variable
 
 ubuntu_20_04:
   <<: *pack_definition
   variables:
     OS: 'ubuntu'
     DIST: 'focal'
+    <<: *shell_cleanup_variable
 
 debian_8:
   <<: *pack_definition
   variables:
     OS: 'debian'
     DIST: 'jessie'
+    <<: *shell_cleanup_variable
 
 debian_9:
   <<: *pack_definition
   variables:
     OS: 'debian'
     DIST: 'stretch'
+    <<: *shell_cleanup_variable
 
 debian_10:
   <<: *pack_definition
   variables:
     OS: 'debian'
     DIST: 'buster'
+    <<: *shell_cleanup_variable
 
 # Deploy
 
 sources_deploy:
   <<: *deploy_definition
+  variables:
+    <<: *shell_cleanup_variable
+  <<: *shell_cleanup_script
   script:
     - ${GITLAB_MAKE} source_deploy
 
@@ -404,90 +444,105 @@ centos_6_deploy:
   variables:
     OS: 'el'
     DIST: '6'
+    <<: *shell_cleanup_variable
 
 centos_7_deploy:
   <<: *deploy_test_definition
   variables:
     OS: 'el'
     DIST: '7'
+    <<: *shell_cleanup_variable
 
 centos_8_deploy:
   <<: *deploy_test_definition
   variables:
     OS: 'el'
     DIST: '8'
+    <<: *shell_cleanup_variable
 
 fedora_28_deploy:
   <<: *deploy_test_definition
   variables:
     OS: 'fedora'
     DIST: '28'
+    <<: *shell_cleanup_variable
 
 fedora_29_deploy:
   <<: *deploy_test_definition
   variables:
     OS: 'fedora'
     DIST: '29'
+    <<: *shell_cleanup_variable
 
 fedora_30_deploy:
   <<: *deploy_test_definition
   variables:
     OS: 'fedora'
     DIST: '30'
+    <<: *shell_cleanup_variable
 
 fedora_31_deploy:
   <<: *deploy_test_definition
   variables:
     OS: 'fedora'
     DIST: '31'
+    <<: *shell_cleanup_variable
 
 ubuntu_14_04_deploy:
   <<: *deploy_definition
   variables:
     OS: 'ubuntu'
     DIST: 'trusty'
+    <<: *shell_cleanup_variable
 
 ubuntu_16_04_deploy:
   <<: *deploy_definition
   variables:
     OS: 'ubuntu'
     DIST: 'xenial'
+    <<: *shell_cleanup_variable
 
 ubuntu_18_04_deploy:
   <<: *deploy_definition
   variables:
     OS: 'ubuntu'
     DIST: 'bionic'
+    <<: *shell_cleanup_variable
 
 ubuntu_19_10_deploy:
   <<: *deploy_definition
   variables:
     OS: 'ubuntu'
     DIST: 'eoan'
+    <<: *shell_cleanup_variable
 
 ubuntu_20_04_deploy:
   <<: *deploy_definition
   variables:
     OS: 'ubuntu'
     DIST: 'focal'
+    <<: *shell_cleanup_variable
 
 debian_8_deploy:
   <<: *deploy_definition
   variables:
     OS: 'debian'
     DIST: 'jessie'
+    <<: *shell_cleanup_variable
 
 debian_9_deploy:
   <<: *deploy_definition
   variables:
     OS: 'debian'
     DIST: 'stretch'
+    <<: *shell_cleanup_variable
 
 debian_10_deploy:
   <<: *deploy_definition
   variables:
     OS: 'debian'
     DIST: 'buster'
+    <<: *shell_cleanup_variable
 
 # Static builds
 
@@ -501,5 +556,8 @@ static_docker_build:
   stage: test
   tags:
     - deploy_test
+  variables:
+    <<: *shell_cleanup_variable
+  <<: *shell_cleanup_script
   script:
     - ${GITLAB_MAKE} test_static_docker_build
-- 
2.17.1

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

[Sergey Bronnikov]

LGTM

On 15:13 Thu 11 Jun , Alexander V. Tikhonov wrote:
> Hi Sergey, thanks for the review, please check my comments bellow.
> 
> On Thu, Jun 11, 2020 at 01:50:57PM +0300, Sergey Bronnikov wrote:
> > 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/
> > 
> 
> Sure, changed.
> 
> > >   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)?
> > 
> 
> Right, this command was taken from gitlab-ci original documentation
>   https://docs.gitlab.com/ee/raketasks/cleanup.html
> 
> I thought that it could be the mistake and rechecked it manually and
> found that options '-fdx' and '-ffdx' have different results: '-ffdx'
> options removed more files than '-fdx' before it.
> 
> > > 
> > > 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.
> > 
> 
> Right, we can set bash command w/o full path, but seems that we can't
> move the bash call inside the environment, I've tried different variants
> and it didn't work.
> 
> > > +
> > > +.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
> > > 

-- 
sergeyb@

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

[Alexander V. Tikhonov]

Hi Sergey, thanks for the review, please check my comments bellow.

On Thu, Jun 11, 2020 at 01:50:57PM +0300, Sergey Bronnikov wrote:
> 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/
> 

Sure, changed.

> >   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)?
> 

Right, this command was taken from gitlab-ci original documentation
  https://docs.gitlab.com/ee/raketasks/cleanup.html

I thought that it could be the mistake and rechecked it manually and
found that options '-fdx' and '-ffdx' have different results: '-ffdx'
options removed more files than '-fdx' before it.

> > 
> > 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.
> 

Right, we can set bash command w/o full path, but seems that we can't
move the bash call inside the environment, I've tried different variants
and it didn't work.

> > +
> > +.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
> > 

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

[Sergey Bronnikov]

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
> 

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

[Alexander V. Tikhonov]

Found the issue on regular testing hosts:
  https://gitlab.com/tarantool/tarantool/-/jobs/577884238#L7

  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

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}"
+
+.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