From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp60.i.mail.ru (smtp60.i.mail.ru [217.69.128.40]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 2AFCB42EF5C for ; Thu, 11 Jun 2020 19:09:17 +0300 (MSK) Date: Thu, 11 Jun 2020 19:08:02 +0300 From: Sergey Bronnikov Message-ID: <20200611160802.GA25241@pony.bronevichok.ru> References: <32772d0a1f20e9cbc8373a852dcfffe472d38985.1591854437.git.avtikhon@tarantool.org> <20200611105057.GA40260@pony.bronevichok.ru> <20200611121318.GA20513@hpalx> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200611121318.GA20513@hpalx> Subject: Re: [Tarantool-patches] [PATCH v1] Correct cleanup gitlab-ci List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Alexander V. Tikhonov" Cc: tarantool-patches@dev.tarantool.org 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@