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

Sergey Bronnikov sergeyb at tarantool.org
Thu Jun 11 19:08:02 MSK 2020


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@


More information about the Tarantool-patches mailing list