[Tarantool-patches] [PATCH v1] Correct cleanup gitlab-ci
Alexander V. Tikhonov
avtikhon at tarantool.org
Thu Jun 11 15:13:18 MSK 2020
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
> >
More information about the Tarantool-patches
mailing list