Re: [PATCH v2 5/5] ci: add support for GitLab CI

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Oct 27, 2023 at 11:19:04AM +0100, Phillip Wood wrote:
> On 27/10/2023 10:25, Patrick Steinhardt wrote:
> > We already support Azure Pipelines and GitHub Workflows in the Git
> > project, but until now we do not have support for GitLab CI. While it is
> > arguably not in the interest of the Git project to maintain a ton of
> > different CI platforms, GitLab has recently ramped up its efforts and
> > tries to contribute to the Git project more regularly.
> 
> I agree we don't want to support too many CI platforms but I think adding
> support for GitLab is good as it helps to stop us being too tied to GitHub
> Actions (which should make it easier if we ever need to transition to a
> different platform in the future) and provides an alternative for
> contributors who want to use a different platform.
> 
> > Part of a problem we hit at GitLab rather frequently is that our own,
> > custom CI setup we have is so different to the setup that the Git
> > project has. More esoteric jobs like "linux-TEST-vars" that also set a
> > couple of environment variables do not exist in GitLab's custom CI
> > setup, and maintaining them to keep up with what Git does feels like
> > wasted time. The result is that we regularly send patch series upstream
> > that fail to compile or pass tests in GitHub Workflows. We would thus
> > like to integrate the GitLab CI configuration into the Git project to
> > help us send better patch series upstream and thus reduce overhead for
> > the maintainer.
> > 
> > The integration does not necessarily have to be a first-class citizen,
> > which would in practice only add to the fallout that pipeline failures
> > have for the maintainer. That being said, we are happy to maintain this
> > alternative CI setup for the Git project and will make test results
> > available as part of our own mirror of the Git project at [1].
> 
> Having someone committed to on-going maintenance is great.
> 
> > This commit introduces the integration into our regular CI scripts so
> > that most of the setup continues to be shared across all of the CI
> > solutions.
> > 
> > [1]: https://gitlab.com/gitlab-org/git
> > 
> > Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> > ---
> >   .gitlab-ci.yml                    | 51 +++++++++++++++++++++++++++++++
> >   ci/install-docker-dependencies.sh |  9 +++++-
> >   ci/lib.sh                         | 49 +++++++++++++++++++++++++++++
> >   ci/print-test-failures.sh         |  6 ++++
> >   4 files changed, 114 insertions(+), 1 deletion(-)
> >   create mode 100644 .gitlab-ci.yml
> > 
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > new file mode 100644
> > index 00000000000..43d3a961fa0
> > --- /dev/null
> > +++ b/.gitlab-ci.yml
> > @@ -0,0 +1,51 @@
> > +default:
> > +  timeout: 2h
> > +
> > +workflow:
> > +  rules:
> > +    - if: $CI_PIPELINE_SOURCE == "merge_request_event"
> > +    - if: $CI_COMMIT_TAG
> > +    - if: $CI_COMMIT_REF_PROTECTED == "true"
> > +
> > +test:
> > +  image: $image
> > +  before_script:
> > +    - ./ci/install-docker-dependencies.sh
> > +  script:
> > +    - useradd builder --home-dir "${CI_PROJECT_DIR}"
> > +    - chown -R builder "${CI_PROJECT_DIR}"
> > +    - sudo --preserve-env --set-home --user=builder ./ci/run-build-and-tests.sh
> 
> It's really good that you're running the tests as an unprivileged user. This
> is something we used to do when we were using Travis that got lost in the
> transition to Azure Pipelines which means some tests that rely on httpd are
> now skipped as they refuse to run as root. ci/run-docker-build.sh is
> currently bit-rotting, I wonder if it is possible to update it so that we
> can run the dockerized tests in the same way on all CI platforms.
> 
> > +  after_script:
> > +    - |
> > +      if test "$CI_JOB_STATUS" != 'success'
> > +      then
> > +        sudo --preserve-env --set-home --user=builder ./ci/print-test-failures.sh
> > +      fi
> > +  parallel:
> > +    matrix:
> > +      - jobname: linux-sha256
> > +        image: ubuntu:latest
> > +        CC: clang
> > +      - jobname: linux-gcc
> > +        image: ubuntu:20.04
> > +        CC: gcc
> > +        CC_PACKAGE: gcc-8
> > +      - jobname: linux-TEST-vars
> > +        image: ubuntu:20.04
> > +        CC: gcc
> > +        CC_PACKAGE: gcc-8
> > +      - jobname: linux-gcc-default
> > +        image: ubuntu:latest
> > +        CC: gcc
> > +      - jobname: linux-leaks
> > +        image: ubuntu:latest
> > +        CC: gcc
> > +      - jobname: linux-asan-ubsan
> > +        image: ubuntu:latest
> > +        CC: clang
> > +      - jobname: linux-musl
> > +        image: alpine:latest
> > +  artifacts:
> > +    paths:
> > +      - t/failed-test-artifacts
> > +    when: on_failure
> 
> This file is pleasingly small.
> 
> > diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
> > index d0bc19d3bb3..1cd92db1876 100755
> > --- a/ci/install-docker-dependencies.sh
> > +++ b/ci/install-docker-dependencies.sh
> > @@ -7,6 +7,9 @@
> >   begin_group "Install dependencies"
> > +# Required so that apt doesn't wait for user input on certain packages.
> > +export DEBIAN_FRONTEND=noninteractive
> > +
> >   case "$jobname" in
> >   linux32)
> >   	linux32 --32bit i386 sh -c '
> > @@ -16,9 +19,13 @@ linux32)
> >   	'
> >   	;;
> >   linux-musl)
> > -	apk add --update build-base curl-dev openssl-dev expat-dev gettext \
> > +	apk add --update git shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
> >   		pcre2-dev python3 musl-libintl perl-utils ncurses >/dev/null
> 
> It would be helpful to explain the new dependencies in the commit message. I
> can see why you're adding sudo, but how were we getting away without
> installing the other packages for GitHub Actions?

True, that part is missing.

- Both sudo and shadow are now required because of `useradd` that we use
  to set up the unprivileged build.

- Git has been required all along, I think. `save_good_tree ()` is used
  in our CI scripts, and Toon (fellow GitLabber from my team) has
  noticed that the CI job warned about missing Git. The warning was
  mostly benign as it seems, but still, doesn't hurt to fix it while at
  it.

I'll have a look at whether I can add another patch on top that adjusts
`ci/run-docker-build.sh` to do rootless builds, which would also make it
more obvious why we now need to install sudo and shadow. And I'll make
sure to document why we now need to have Git around.

> >   	;;
> > +linux-*)
> > +	apt update -q &&
> > +	apt install -q -y sudo git make language-pack-is libsvn-perl apache2 libssl-dev libcurl4-openssl-dev libexpat-dev tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl ${CC_PACKAGE:-${CC:-gcc}}
> > +	;;
> >   pedantic)
> >   	dnf -yq update >/dev/null &&
> >   	dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null
> > diff --git a/ci/lib.sh b/ci/lib.sh
> > index 9ffdf743903..f518df7e2cb 100755
> > --- a/ci/lib.sh
> > +++ b/ci/lib.sh
> > @@ -14,6 +14,22 @@ then
> >   		need_to_end_group=
> >   		echo '::endgroup::' >&2
> >   	}
> > +elif test true = "$GITLAB_CI"
> > +then
> > +	begin_group () {
> > +		need_to_end_group=t
> > +		echo -e "\e[0Ksection_start:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K$1"
> > +		trap "end_group '$1'" EXIT
> > +		set -x
> > +	}
> > +
> > +	end_group () {
> > +		test -n "$need_to_end_group" || return 0
> > +		set +x
> > +		need_to_end_group=
> > +		echo -e "\e[0Ksection_end:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K"
> > +		trap - EXIT
> > +	}
> >   else
> >   	begin_group () { :; }
> >   	end_group () { :; }
> > @@ -203,6 +219,39 @@ then
> >   	MAKEFLAGS="$MAKEFLAGS --jobs=10"
> >   	test windows != "$CI_OS_NAME" ||
> >   	GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
> > +elif test true = "$GITLAB_CI"
> > +then
> > +	CI_TYPE=gitlab-ci
> > +	CI_BRANCH="$CI_COMMIT_REF_NAME"
> > +	CI_COMMIT="$CI_COMMIT_SHA"
> > +	case "$CI_JOB_IMAGE" in
> > +	macos-*)
> > +		CI_OS_NAME=osx;;
> > +	alpine:*|ubuntu:*)
> > +		CI_OS_NAME=linux;;
> > +	*)
> > +		echo "Could not identify OS image" >&2
> > +		env >&2
> > +		exit 1
> > +		;;
> > +	esac
> > +	CI_REPO_SLUG="$CI_PROJECT_PATH"
> > +	CI_JOB_ID="$CI_JOB_ID"
> 
> I guess making this explicit is helpful, otherwise someone may wonder why
> CI_JOB_ID is not being set.
> 
> > +	CC="${CC_PACKAGE:-${CC:-gcc}}"
> > +	DONT_SKIP_TAGS=t
> > +	handle_failed_tests () {
> > +		create_failed_test_artifacts
> > +	}
> > +
> > +	cache_dir="$HOME/none"
> > +
> > +	runs_on_pool=$(echo "$CI_JOB_IMAGE" | tr : -)
> > +
> > +	export GIT_PROVE_OPTS="--timer --jobs $(nproc)"
> > +	export GIT_TEST_OPTS="--verbose-log -x"
> > +	MAKEFLAGS="$MAKEFLAGS --jobs=$(nproc)"
> > +	test windows != "$CI_OS_NAME" ||
> > +	GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
> 
> This last paragraph feels like it should be common to all the CI providers.
> There are some small differences but if we're going to support several
> providers it would be nice to set the common options centrally. I'm pretty
> sure the --jobs=10 we use for the GitHub and Azure is not optimal, using
> $(nproc) is nice so long as it is supported on all the images we use.

True enough, I can have a go at it. I'm rather cautious around updating
the build infra as I cannot easily verify that things continue to work
for GitHub. But this part here might be a good candidate for deduping.

> I had a quick glance through the previous patches and they all look like
> nice cleanups that make our ci support less dependent on a single provider.
> This series looks like a nice addition to our CI support.
> 
> Best Wishes
> 
> Phillip

Thanks!

Patrick

> 
> >   else
> >   	echo "Could not identify CI type" >&2
> >   	env >&2
> > diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh
> > index 57277eefcd0..c33ad4e3a22 100755
> > --- a/ci/print-test-failures.sh
> > +++ b/ci/print-test-failures.sh
> > @@ -51,6 +51,12 @@ do
> >   			tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
> >   			continue
> >   			;;
> > +		gitlab-ci)
> > +			mkdir -p failed-test-artifacts
> > +			cp "${TEST_EXIT%.exit}.out" failed-test-artifacts/
> > +			tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
> > +			continue
> > +			;;
> >   		*)
> >   			echo "Unhandled CI type: $CI_TYPE" >&2
> >   			exit 1
> 

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux