[libvirt PATCH 00/20] ci: Move GitLab build recipes to a standalone script
This is a follow up to: https://listman.redhat.com/archives/libvir-list/2023-January/237201.html The effort here is to unify the way builds/tests are executed in GitLab CI vs local container executions and make another step forward in terms of reproducibility of (specifically) GitLab environments. Even though code to run all but one (coverity) jobs from GitLab via the build.sh script is added with this series, local behavior remains the same as before this series. The reason for that is that that will require more patches ridding of the Makefile which is currently used and instead integrate usage of lcitool with the ci/helper Python script which is currently the entry point for local container executions. Pipeline: https://gitlab.com/eskultety/libvirt/-/pipelines/768645158 Ubuntu is having some repo connection issues today, so the one failed ^job can be ignored Erik Skultety (20): gitlab-ci.yml: Replace all explicit calls to ninja with meson commands gitlab-ci.yml: potfile: Consolidate the meson compile calls gitlab-ci.yml: Use $HOME for rpmbuild's topdir instead of PWD ci: build.sh: Drop the commentary about CI_BUILD_SCRIPT ci: build.sh: Use 'meson setup' explicitly ci: build.sh: Always assume -Dsystem=true ci: build.sh: Drop the CI prefix from the CI_{MESON,NINJA}_ARGS vars ci: build.sh: Move off of ninja command to directly calling meson ci: build.sh: Join MESON_ARGS and MESON_OPTS ci: build.sh: Break the script functionality into helper functions ci: build.sh: Move the necessary env variables to build.sh ci: build.sh: Add support for individual GitLab jobs ci: build.sh: Wire up the individual job functions to the CLI ci: build.sh: Document CI_CONT_SRCDIR ci: build.sh: Make the build script fail ASAP with 'set -e' ci: build.sh: Update git index in local container environments on 'dist' ci: build.sh: Make the script executable gitlab-ci.yml: Add 'after_script' stage to prep for artifact collection gitlab-ci.yml: Adopt job execution via a Bash script gitlab-ci.yml: Drop the usage of script variables reference .gitlab-ci.yml | 56 ++++++++++------------- ci/Makefile | 16 ++++--- ci/build.sh | 121 +++++++++++++++++++++++++++++++++++++++++++------ ci/helper | 21 ++++++--- 4 files changed, 155 insertions(+), 59 deletions(-) mode change 100644 => 100755 ci/build.sh -- 2.39.1
This is continuation of what commit b56e2be68e3 started. If we stick to only calling meson commands directly, we can achieve much better consistency in passing arguments to meson especially if we unify the recipes run in gitlab CI and what we can currently run locally in containers using docker/podman. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- .gitlab-ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 1b72ebc493..699be460ca 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -76,7 +76,7 @@ include: script: - *script_variables - meson setup build --werror -Dsystem=true || (cat build/meson-logs/meson-log.txt && exit 1) - - DESTDIR=$(pwd)/install ninja -C build install-web + - DESTDIR=$(pwd)/install meson compile -C build install-web - mv install/usr/share/doc/libvirt/html/ website artifacts: expose_as: 'Website' @@ -110,7 +110,7 @@ website_local_env: script: - *script_variables - meson setup build --werror || (cat build/meson-logs/meson-log.txt && exit 1) - - ninja -C build libvirt-pot-dep + - meson compile -C build libvirt-pot-dep - meson test -C build --suite syntax-check --no-rebuild --print-errorlogs codestyle_prebuilt_env: @@ -153,8 +153,8 @@ potfile: - *script_variables script: - meson setup build --werror || (cat build/meson-logs/meson-log.txt && exit 1) - - ninja -C build libvirt-pot-dep - - ninja -C build libvirt-pot + - meson compile -C build libvirt-pot-dep + - meson compile -C build libvirt-pot - cp po/libvirt.pot libvirt.pot artifacts: expose_as: 'Potfile' -- 2.39.1
On Mon, Feb 06, 2023 at 02:52:58PM +0100, Erik Skultety wrote:
This is continuation of what commit b56e2be68e3 started. If we stick to only calling meson commands directly, we can achieve much better consistency in passing arguments to meson especially if we unify the recipes run in gitlab CI and what we can currently run locally in containers using docker/podman.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- .gitlab-ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
You can specify multiple targets at once for the 'compile' command. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- .gitlab-ci.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 699be460ca..e20d0b9be8 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -153,8 +153,7 @@ potfile: - *script_variables script: - meson setup build --werror || (cat build/meson-logs/meson-log.txt && exit 1) - - meson compile -C build libvirt-pot-dep - - meson compile -C build libvirt-pot + - meson compile -C build libvirt-pot-dep libvirt-pot - cp po/libvirt.pot libvirt.pot artifacts: expose_as: 'Potfile' -- 2.39.1
On Mon, Feb 06, 2023 at 02:52:59PM +0100, Erik Skultety wrote:
You can specify multiple targets at once for the 'compile' command.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- .gitlab-ci.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 699be460ca..e20d0b9be8 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -153,8 +153,7 @@ potfile: - *script_variables script: - meson setup build --werror || (cat build/meson-logs/meson-log.txt && exit 1) - - meson compile -C build libvirt-pot-dep - - meson compile -C build libvirt-pot + - meson compile -C build libvirt-pot-dep libvirt-pot
while possible i think its better to keep them separate so when things fail you see which of the two commands was the failure straight away. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- .gitlab-ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e20d0b9be8..921b04cd7b 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -30,8 +30,8 @@ include: - meson dist -C build --no-tests - if test -x /usr/bin/rpmbuild && test "$RPM" != "skip"; then - rpmbuild --clean --nodeps --define "_without_mingw 1" --define "_topdir $PWD/rpmbuild/" -ta build/meson-dist/libvirt-*.tar.xz; - mv rpmbuild/RPMS/x86_64/ libvirt-rpms/; + rpmbuild --clean --nodeps --define "_without_mingw 1" -ta build/meson-dist/libvirt-*.tar.xz; + mv "$HOME"/rpmbuild/RPMS/x86_64/ libvirt-rpms/; else meson compile -C build; meson test -C build --no-suite syntax-check --print-errorlogs; -- 2.39.1
On Mon, Feb 06, 2023 at 02:53:00PM +0100, Erik Skultety wrote:
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- .gitlab-ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
build.sh is not the place where this should be mentioned as the official entrypoint for this script locally is ci/helper which can download the right image from our upstream CI registry. Since the idea is to ultimately drop the usage of a Makefile for the local executions, this patch doesn't provide an alternative place for the comment in question as the functionality is going to be altered substantially in the future. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/build.sh | 9 --------- 1 file changed, 9 deletions(-) diff --git a/ci/build.sh b/ci/build.sh index 0f23df1fa3..3fa28eafa8 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -1,14 +1,5 @@ #!/bin/sh -# This script is used to build libvirt inside the container. -# -# You can customize it to your liking, or alternatively use a -# completely different script by passing -# -# CI_BUILD_SCRIPT=/path/to/your/build/script -# -# to make. - cd "$CI_CONT_SRCDIR" export VIR_TEST_DEBUG=1 -- 2.39.1
On Mon, Feb 06, 2023 at 02:53:01PM +0100, Erik Skultety wrote:
build.sh is not the place where this should be mentioned as the official entrypoint for this script locally is ci/helper which can download the right image from our upstream CI registry. Since the idea is to ultimately drop the usage of a Makefile for the local executions, this patch doesn't provide an alternative place for the comment in question as the functionality is going to be altered substantially in the future.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/build.sh | 9 --------- 1 file changed, 9 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Even though 'setup' is assumed when no other command is given, we're being explicit in our GitLab recipes, so do the same for the local build.sh script too. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/build.sh b/ci/build.sh index 3fa28eafa8..c7cba6ffa8 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -8,7 +8,7 @@ export VIR_TEST_DEBUG=1 # populated at build time from the Dockerfile. A typical use case would # be to pass options to trigger cross-compilation -meson build --werror $MESON_OPTS $CI_MESON_ARGS || \ +meson setup build --werror $MESON_OPTS $CI_MESON_ARGS || \ (cat build/meson-logs/meson-log.txt && exit 1) ninja -C build $CI_NINJA_ARGS -- 2.39.1
On Mon, Feb 06, 2023 at 02:53:02PM +0100, Erik Skultety wrote:
Even though 'setup' is assumed when no other command is given, we're being explicit in our GitLab recipes, so do the same for the local build.sh script too.
It is actually gouing to become mandatory soon based on the warnings shown in recent meson WARNING: Running the setup command as `meson [options]` instead of `meson setup [options]` is ambiguous and deprecated. so if there are any other places across projects with this old syntax we'll need to flush them out too.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
diff --git a/ci/build.sh b/ci/build.sh index 3fa28eafa8..c7cba6ffa8 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -8,7 +8,7 @@ export VIR_TEST_DEBUG=1 # populated at build time from the Dockerfile. A typical use case would # be to pass options to trigger cross-compilation
-meson build --werror $MESON_OPTS $CI_MESON_ARGS || \ +meson setup build --werror $MESON_OPTS $CI_MESON_ARGS || \ (cat build/meson-logs/meson-log.txt && exit 1)
ninja -C build $CI_NINJA_ARGS -- 2.39.1
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
There's no harm in always building in system mode, i.e. setting the right paths. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/build.sh b/ci/build.sh index c7cba6ffa8..f6db4d2a7f 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -8,7 +8,7 @@ export VIR_TEST_DEBUG=1 # populated at build time from the Dockerfile. A typical use case would # be to pass options to trigger cross-compilation -meson setup build --werror $MESON_OPTS $CI_MESON_ARGS || \ +meson setup build --werror -Dsystem=true $MESON_OPTS $CI_MESON_ARGS || \ (cat build/meson-logs/meson-log.txt && exit 1) ninja -C build $CI_NINJA_ARGS -- 2.39.1
On Mon, Feb 06, 2023 at 02:53:03PM +0100, Erik Skultety wrote:
There's no harm in always building in system mode, i.e. setting the right paths.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Although it is currently consistent with the other variables we define when running ci in a local container environment, it isn't consistent with the variable naming we use in GitLab recipes. Since the idea is to unite the two, we're likely going to drop a few other variables from the local env configuration anyway, hence this renaming. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/Makefile | 4 ++-- ci/build.sh | 4 ++-- ci/helper | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ci/Makefile b/ci/Makefile index 81f08d4f88..8f1be4318d 100644 --- a/ci/Makefile +++ b/ci/Makefile @@ -161,8 +161,8 @@ CI_ENGINE_ARGS = \ --user "$(CI_UID)":"$(CI_GID)" \ --workdir "$(CI_USER_HOME)" \ --env CI_CONT_SRCDIR="$(CI_CONT_SRCDIR)" \ - --env CI_MESON_ARGS="$(CI_MESON_ARGS)" \ - --env CI_NINJA_ARGS="$(CI_NINJA_ARGS)" \ + --env MESON_ARGS="$(MESON_ARGS)" \ + --env NINJA_ARGS="$(NINJA_ARGS)" \ $(CI_PODMAN_ARGS) \ $(CI_PWDB_MOUNTS) \ $(CI_HOME_MOUNTS) \ diff --git a/ci/build.sh b/ci/build.sh index f6db4d2a7f..9489c4ab2f 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -8,7 +8,7 @@ export VIR_TEST_DEBUG=1 # populated at build time from the Dockerfile. A typical use case would # be to pass options to trigger cross-compilation -meson setup build --werror -Dsystem=true $MESON_OPTS $CI_MESON_ARGS || \ +meson setup build --werror -Dsystem=true $MESON_OPTS $MESON_ARGS || \ (cat build/meson-logs/meson-log.txt && exit 1) -ninja -C build $CI_NINJA_ARGS +ninja -C build $NINJA_ARGS diff --git a/ci/helper b/ci/helper index 8b8d0f68cb..fb562d55e1 100755 --- a/ci/helper +++ b/ci/helper @@ -152,8 +152,8 @@ class Application: if self._args.action in ["build", "test"]: args.extend([ - f"CI_MESON_ARGS={self._args.meson_args}", - f"CI_NINJA_ARGS={self._args.ninja_args}", + f"MESON_ARGS={self._args.meson_args}", + f"NINJA_ARGS={self._args.ninja_args}", ]) if pty.spawn(["make"] + args) != 0: -- 2.39.1
On Mon, Feb 06, 2023 at 02:53:04PM +0100, Erik Skultety wrote:
Although it is currently consistent with the other variables we define when running ci in a local container environment, it isn't consistent with the variable naming we use in GitLab recipes. Since the idea is to unite the two, we're likely going to drop a few other variables from the local env configuration anyway, hence this renaming.
The namespacing originated with & was only relevant when we were still using the old automake system, as it let is 'include ci/Makefile' without risk of it clashing with automake variables. So yeah, fine for it to go.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/Makefile | 4 ++-- ci/build.sh | 4 ++-- ci/helper | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
This change however involves adding a couple of new environment variables as well as tuning the helper script to support local container executions properly. The overall motivation here is to move all script logic from .gitlab-ci.yml to the build.sh script so that the steps are consistent and identical when executing in local containers and GitLab. By adding the new env variables and increasing the granularity in meson commands executed in the script it gives us better options on how to port the existing code from .gitlab-ci.yml to a standalone Bash script. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/Makefile | 11 +++++++---- ci/build.sh | 3 ++- ci/helper | 21 ++++++++++++++------- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/ci/Makefile b/ci/Makefile index 8f1be4318d..217eda3cc0 100644 --- a/ci/Makefile +++ b/ci/Makefile @@ -162,7 +162,9 @@ CI_ENGINE_ARGS = \ --workdir "$(CI_USER_HOME)" \ --env CI_CONT_SRCDIR="$(CI_CONT_SRCDIR)" \ --env MESON_ARGS="$(MESON_ARGS)" \ - --env NINJA_ARGS="$(NINJA_ARGS)" \ + --env MESON_BUILD_ARGS="$(MESON_BUILD_ARGS)" \ + --env MESON_RUN_TEST=$(MESON_RUN_TEST) \ + --env MESON_TEST_ARGS="$(MESON_TEST_ARGS)" \ $(CI_PODMAN_ARGS) \ $(CI_PWDB_MOUNTS) \ $(CI_HOME_MOUNTS) \ @@ -209,7 +211,7 @@ ci-build@%: $(MAKE) -C $(CI_ROOTDIR) ci-run-command@$* CI_COMMAND="$(CI_USER_HOME)/build" ci-test@%: - $(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_NINJA_ARGS=test + $(MAKE) -C $(CI_ROOTDIR) ci-build@$* ci-help: @echo @@ -240,6 +242,7 @@ ci-help: @echo " CI_USER_LOGIN= - which user should run in the container (default is $$USER)" @echo " CI_IMAGE_PREFIX= - override to prefer a locally built image, (default is $(CI_IMAGE_PREFIX))" @echo " CI_IMAGE_TAG=:latest - optionally use in conjunction with 'CI_IMAGE_PREFIX'" - @echo " CI_MESON_ARGS= - extra arguments passed to meson" - @echo " CI_NINJA_ARGS= - extra arguments passed to ninja" + @echo " MESON_ARGS= - extra configure arguments passed to meson setup" + @echo " MESON_BUILD_ARGS= - extra build arguments passed to meson compile" + @echo " MESON_TEST_ARGS= - extra arguments passed to meson test" @echo diff --git a/ci/build.sh b/ci/build.sh index 9489c4ab2f..2a83f756d5 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -11,4 +11,5 @@ export VIR_TEST_DEBUG=1 meson setup build --werror -Dsystem=true $MESON_OPTS $MESON_ARGS || \ (cat build/meson-logs/meson-log.txt && exit 1) -ninja -C build $NINJA_ARGS +meson compile -C build $MESON_BUILD_ARGS +meson test -C build $MESON_TEST_ARGS diff --git a/ci/helper b/ci/helper index fb562d55e1..7b8f2e6826 100755 --- a/ci/helper +++ b/ci/helper @@ -48,15 +48,21 @@ class Parser: # project's build system mesonparser = argparse.ArgumentParser(add_help=False) mesonparser.add_argument( - "--meson-args", + "--meson-configure-args", default="", - help="additional arguments passed to meson " - "(eg --meson-args='-Dopt1=enabled -Dopt2=disabled')", + help="additional arguments passed to meson setup" + "(eg --meson-configure-args='-Dopt1=enabled -Dopt2=disabled')", ) mesonparser.add_argument( - "--ninja-args", + "--meson-build-args", default="", - help="additional arguments passed to ninja", + help="additional arguments passed to meson compile" + "(eg --meson-build-args='--clean --jobs N <build target>')", + ) + mesonparser.add_argument( + "--meson-test-args", + default="", + help="additional arguments passed to meson test", ) # Options that are common to actions communicating with a GitLab @@ -152,8 +158,9 @@ class Application: if self._args.action in ["build", "test"]: args.extend([ - f"MESON_ARGS={self._args.meson_args}", - f"NINJA_ARGS={self._args.ninja_args}", + f"MESON_ARGS={self._args.meson_configure_args}", + f"MESON_BUILD_ARGS={self._args.meson_build_args}", + f"MESON_TEST_ARGS={self._args.meson_test_args}", ]) if pty.spawn(["make"] + args) != 0: -- 2.39.1
It is quite confusing seeing these two in a call like this one: $ meson build $MESON_OPTS $MESON_ARGS One has to ask 'how are they different' and 'shouldn't these be merged'. In fact, these variables hold very different things and we should make it more obvious. The problem is that renaming MESON_OPTS to something more meaningful, like 'MESON_CROSS_OPTS' which is what MESON_OPTS really does would require changes to lcitool and would impact Dockerfile generation which in turn might have an impact on other projects which rely on this lcitool functionality which is risky. Instead, provide a docstring for the former tu supplement the latter and join the two variables in a single one MESON_ARGS which is then passed to meson's command line so it's a little less confusing. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/build.sh | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/ci/build.sh b/ci/build.sh index 2a83f756d5..322aff2632 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -7,8 +7,15 @@ export VIR_TEST_DEBUG=1 # $MESON_OPTS is an env that can optionally be set in the container, # populated at build time from the Dockerfile. A typical use case would # be to pass options to trigger cross-compilation +# +# $MESON_ARGS correspond to meson's setup args, i.e. configure args. It's +# populated either from a GitLab's job configuration or from command line as +# `$ helper build --meson-configure-args=-Dopt1 -Dopt2` when run in a local +# containerized environment -meson setup build --werror -Dsystem=true $MESON_OPTS $MESON_ARGS || \ +MESON_ARGS="$MESON_ARGS $MESON_OPTS" + +meson setup build --werror -Dsystem=true $MESON_ARGS || \ (cat build/meson-logs/meson-log.txt && exit 1) meson compile -C build $MESON_BUILD_ARGS -- 2.39.1
On Mon, Feb 06, 2023 at 02:53:06PM +0100, Erik Skultety wrote:
It is quite confusing seeing these two in a call like this one: $ meson build $MESON_OPTS $MESON_ARGS
One has to ask 'how are they different' and 'shouldn't these be merged'. In fact, these variables hold very different things and we should make it more obvious. The problem is that renaming MESON_OPTS to something more meaningful, like 'MESON_CROSS_OPTS' which is what MESON_OPTS really does would require changes to lcitool and would impact Dockerfile generation which in turn might have an impact on other projects which rely on this lcitool functionality which is risky.
Instead, provide a docstring for the former tu supplement the latter
s/tu/to/
and join the two variables in a single one MESON_ARGS which is then passed to meson's command line so it's a little less confusing.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/build.sh | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Resurrecting this thread now what you've pushed some of the patches. On Mon, Feb 06, 2023 at 02:53:06PM +0100, Erik Skultety wrote:
+# $MESON_ARGS correspond to meson's setup args, i.e. configure args. It's +# populated either from a GitLab's job configuration or from command line as +# `$ helper build --meson-configure-args=-Dopt1 -Dopt2` when run in a local +# containerized environment
You need quotes around the value. As is, the shell will interpret '--meson-configure-args=-Dopt1' and '-Dopt2' as separate arguments and things will not work the way you expect them to.
-meson setup build --werror -Dsystem=true $MESON_OPTS $MESON_ARGS || \ +MESON_ARGS="$MESON_ARGS $MESON_OPTS" + +meson setup build --werror -Dsystem=true $MESON_ARGS || \
This has inverted the priority of the two lists of arguments. Before the change, an option (e.g. -Dfoo=enabled) could be added to $MESON_ARGS at the job level and it would override the same option (e.g. -Dfoo=disabled) defined as part of $MESON_OPTS in the container image. Now the option in the container image will always take precedence, which is undesirable. -- Andrea Bolognani / Red Hat / Virtualization
On Mon, Aug 21, 2023 at 05:17:06AM -0700, Andrea Bolognani wrote:
Resurrecting this thread now what you've pushed some of the patches.
On Mon, Feb 06, 2023 at 02:53:06PM +0100, Erik Skultety wrote:
+# $MESON_ARGS correspond to meson's setup args, i.e. configure args. It's +# populated either from a GitLab's job configuration or from command line as +# `$ helper build --meson-configure-args=-Dopt1 -Dopt2` when run in a local +# containerized environment
You need quotes around the value. As is, the shell will interpret '--meson-configure-args=-Dopt1' and '-Dopt2' as separate arguments and things will not work the way you expect them to.
-meson setup build --werror -Dsystem=true $MESON_OPTS $MESON_ARGS || \ +MESON_ARGS="$MESON_ARGS $MESON_OPTS" + +meson setup build --werror -Dsystem=true $MESON_ARGS || \
This has inverted the priority of the two lists of arguments.
Before the change, an option (e.g. -Dfoo=enabled) could be added to $MESON_ARGS at the job level and it would override the same option (e.g. -Dfoo=disabled) defined as part of $MESON_OPTS in the container image. Now the option in the container image will always take precedence, which is undesirable.
Good points. However, I'm already close to proposing something vastly different from this, dropping most of these variables as they are to make the local executions pretty much >95% compatible to what happens in GitLab and overriding these shell variables simply isn't part of it because it only makes sense right now, but I think with the changes I have it'll only make sense in interactive container shell sessions in which case it's left to the developer to set and pass the right options IMO. Erik
On Wed, Aug 23, 2023 at 11:38:17AM +0200, Erik Skultety wrote:
On Mon, Aug 21, 2023 at 05:17:06AM -0700, Andrea Bolognani wrote:
On Mon, Feb 06, 2023 at 02:53:06PM +0100, Erik Skultety wrote:
-meson setup build --werror -Dsystem=true $MESON_OPTS $MESON_ARGS || \ +MESON_ARGS="$MESON_ARGS $MESON_OPTS" + +meson setup build --werror -Dsystem=true $MESON_ARGS || \
This has inverted the priority of the two lists of arguments.
Before the change, an option (e.g. -Dfoo=enabled) could be added to $MESON_ARGS at the job level and it would override the same option (e.g. -Dfoo=disabled) defined as part of $MESON_OPTS in the container image. Now the option in the container image will always take precedence, which is undesirable.
Good points. However, I'm already close to proposing something vastly different from this, dropping most of these variables as they are to make the local executions pretty much >95% compatible to what happens in GitLab and overriding these shell variables simply isn't part of it because it only makes sense right now, but I think with the changes I have it'll only make sense in interactive container shell sessions in which case it's left to the developer to set and pass the right options IMO.
This all sounds good, but in the meantime the feature is not working as intended. I've posted a quick fix here: https://listman.redhat.com/archives/libvir-list/2023-August/241402.html -- Andrea Bolognani / Red Hat / Virtualization
Future patches will add more functions corresponding to the behaviour we define in individual GitLab jobs in .gitlab-ci.yml. This is just a preliminary patch. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/build.sh | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/ci/build.sh b/ci/build.sh index 322aff2632..5faa96e123 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -15,8 +15,23 @@ export VIR_TEST_DEBUG=1 MESON_ARGS="$MESON_ARGS $MESON_OPTS" -meson setup build --werror -Dsystem=true $MESON_ARGS || \ -(cat build/meson-logs/meson-log.txt && exit 1) +run_meson_setup() { + meson setup build --werror -Dsystem=true $MESON_ARGS || \ + (cat build/meson-logs/meson-log.txt && exit 1) +} -meson compile -C build $MESON_BUILD_ARGS -meson test -C build $MESON_TEST_ARGS +run_build() { + meson compile -C build $MESON_BUILD_ARGS +} + +run_test() { + meson test -C build $MESON_TEST_ARGS +} + +main() { + run_meson_setup + run_build + run_test +} + +main "$@" -- 2.39.1
Originally coming from the list in our gitlab-ci.yml. The corresponding gitlab-ci.yml change will come in a future patch. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/build.sh | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/ci/build.sh b/ci/build.sh index 5faa96e123..15c157067b 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -1,9 +1,14 @@ #!/bin/sh +export CCACHE_BASEDIR="$(pwd)" +export CCACHE_DIR="$CCACHE_BASEDIR/ccache" +export CCACHE_MAXSIZE="500M" +export PATH="$CCACHE_WRAPPERSDIR:$PATH" +export VIR_TEST_VERBOSE="1" +export VIR_TEST_DEBUG="1" + cd "$CI_CONT_SRCDIR" -export VIR_TEST_DEBUG=1 - # $MESON_OPTS is an env that can optionally be set in the container, # populated at build time from the Dockerfile. A typical use case would # be to pass options to trigger cross-compilation -- 2.39.1
Introduce more helper functions corresponding to the jobs we currently run for container workloads in GitLab. Some of the variables used in the functions have to provide default values identical to the options we pass to the jobs in GitLab to match the same behaviour if not overriden by the user on the CLI when the local container execution is used. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/build.sh | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/ci/build.sh b/ci/build.sh index 15c157067b..f169dd01a1 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -29,10 +29,41 @@ run_build() { meson compile -C build $MESON_BUILD_ARGS } +run_dist() { + meson dist -C build --no-tests +} + run_test() { meson test -C build $MESON_TEST_ARGS } +run_codestyle() { + MESON_BUILD_ARGS=${MESON_BUILD_ARGS:="libvirt-pot-dep"} + MESON_TEST_ARGS=${MESON_TEST_ARGS:="--suite syntax-check \ + --no-rebuild \ + --print-errorlogs"} + run_build + run_test +} + +run_potfile() { + MESON_BUILD_ARGS=${MESON_BUILD_ARGS:="libvirt-pot-dep libvirt-pot"} + run_build +} + +run_rpmbuild() { + run_dist + rpmbuild --clean \ + --nodeps \ + --define "_without_mingw 1" \ + -ta build/meson-dist/libvirt-*.tar.xz +} + +run_website_build() { + MESON_BUILD_ARGS=${MESON_BUILD_ARGS:="install-web"} + DESTDIR=$(pwd)/install run_build +} + main() { run_meson_setup run_build -- 2.39.1
Now that we have the GitLab job helper functions in place, we can wire them up to the CLI interface. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/Makefile | 5 ++--- ci/build.sh | 39 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/ci/Makefile b/ci/Makefile index 217eda3cc0..8525b2ff88 100644 --- a/ci/Makefile +++ b/ci/Makefile @@ -208,11 +208,10 @@ ci-shell@%: $(MAKE) -C $(CI_ROOTDIR) ci-run-command@$* CI_COMMAND="/bin/bash" ci-build@%: - $(MAKE) -C $(CI_ROOTDIR) ci-run-command@$* CI_COMMAND="$(CI_USER_HOME)/build" + $(MAKE) -C $(CI_ROOTDIR) ci-run-command@$* CI_COMMAND="$(CI_USER_HOME)/build --build" ci-test@%: - $(MAKE) -C $(CI_ROOTDIR) ci-build@$* - + $(MAKE) -C $(CI_ROOTDIR) ci-run-command@$* CI_COMMAND="$(CI_USER_HOME)/build --build --test" ci-help: @echo @echo diff --git a/ci/build.sh b/ci/build.sh index f169dd01a1..aeb1bf4b05 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -65,9 +65,44 @@ run_website_build() { } main() { + ACTIONS="" + + while [ "$#" -ne 0 ] + do + case "$1" in + --build) + ACTIONS="$ACTIONS run_build" + ;; + --potfile) + ACTIONS="$ACTIONS run_potfile" + ;; + --codestyle) + ACTIONS="$ACTIONS run_codestyle" + ;; + --rpmbuild) + ACTIONS="$ACTIONS run_rpmbuild" + ;; + --test) + ACTIONS="$ACTIONS run_test" + ;; + --website) + ACTIONS="$ACTIONS run_website_build" + ;; + *) + echo "Error: Unknown action '$1'" + exit 1 + ;; + esac + shift + done + + ACTIONS=${ACTIONS:=run_build} + run_meson_setup - run_build - run_test + for action in $ACTIONS + do + $action + done } main "$@" -- 2.39.1
This variable is specific to local container execution and is a result of the hierarchy we chose for local container executions: $ ls build libvirt # build is the script, libvirt is a repo clone The variable would never be populated in GitLab environment, so we set the default to $PWD to make it harmless in such a case. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/build.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ci/build.sh b/ci/build.sh index aeb1bf4b05..4d7ef810f2 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -7,7 +7,10 @@ export PATH="$CCACHE_WRAPPERSDIR:$PATH" export VIR_TEST_VERBOSE="1" export VIR_TEST_DEBUG="1" -cd "$CI_CONT_SRCDIR" +# CI_CONT_SRCDIR will only ever be set in local container executions. For +# GitLab CI environment the variable defaults to $PWD to avoid any disruptions +# in the expected environment view. +cd "${CI_CONT_SRCDIR:=$PWD}" # $MESON_OPTS is an env that can optionally be set in the container, # populated at build time from the Dockerfile. A typical use case would -- 2.39.1
This is the default setting in GitLab container environments and it makes sense to stop executing the script with the first error encountered. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/build.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/build.sh b/ci/build.sh index 4d7ef810f2..b6596300be 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -69,6 +69,7 @@ run_website_build() { main() { ACTIONS="" + set -e while [ "$#" -ne 0 ] do -- 2.39.1
Meson dist build is unhappy with the git clone we mount into local container environments and forces updating git's index. Since this is only relevant to the dist build, only update the index then. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- Honestly I have no good explanation why dist kept complaining about uncommitted changes even though there weren't any. It probably has something to do with the fact how git clone --local works, but I don't see why - an option would be to convert to using '--no-hardlinks'. Anyhow, 'update-index --refresh' was the only thing that helped to solve the problem so that the dist tarball could be created inside a container locally followed by an RPM build. ci/build.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ci/build.sh b/ci/build.sh index b6596300be..6731db50c1 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -33,6 +33,10 @@ run_build() { } run_dist() { + # dist is unhappy in local container environment complaining about + # uncommitted changes in the repo which is often not the case - refreshing + # git's index solves the problem + git update-index --refresh meson dist -C build --no-tests } -- 2.39.1
Unless we run it as 'sh ci/build.sh' in .gitlab-ci.yml recipes and instead opt into doing 'chmod +x && ci/build.sh' it will cause meson dist build to fail with a fatal error about having uncommitted changes in the repo. Therefore, let's just make the script executable, it's the most straightforward solution. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/build.sh | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 ci/build.sh diff --git a/ci/build.sh b/ci/build.sh old mode 100644 new mode 100755 -- 2.39.1
On Mon, Feb 06, 2023 at 02:53:14PM +0100, Erik Skultety wrote:
Unless we run it as 'sh ci/build.sh' in .gitlab-ci.yml recipes and instead opt into doing 'chmod +x && ci/build.sh' it will cause meson dist build to fail with a fatal error about having uncommitted changes in the repo. Therefore, let's just make the script executable, it's the most straightforward solution.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/build.sh | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 ci/build.sh
diff --git a/ci/build.sh b/ci/build.sh old mode 100644 new mode 100755
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
This would otherwise collide with local executions where we 1) don't collect artifacts 2) are not limited by GitLab's environment and hence moving build artifacts to unusual places would only cause confusion when doing local build inspection Signed-off-by: Erik Skultety <eskultet@redhat.com> --- .gitlab-ci.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 921b04cd7b..abd7498058 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -31,11 +31,16 @@ include: - if test -x /usr/bin/rpmbuild && test "$RPM" != "skip"; then rpmbuild --clean --nodeps --define "_without_mingw 1" -ta build/meson-dist/libvirt-*.tar.xz; - mv "$HOME"/rpmbuild/RPMS/x86_64/ libvirt-rpms/; else meson compile -C build; meson test -C build --no-suite syntax-check --print-errorlogs; fi + after_script: + - test "$CI_JOB_STATUS" != "success" && exit 1; + - if test -x /usr/bin/rpmbuild && test "$RPM" != "skip"; + then + mv "$HOME"/rpmbuild/RPMS/x86_64/ libvirt-rpms/; + fi .native_build_job_prebuilt_env: extends: @@ -77,6 +82,8 @@ include: - *script_variables - meson setup build --werror -Dsystem=true || (cat build/meson-logs/meson-log.txt && exit 1) - DESTDIR=$(pwd)/install meson compile -C build install-web + after_script: + - test "$CI_JOB_STATUS" != "success" && exit 1; - mv install/usr/share/doc/libvirt/html/ website artifacts: expose_as: 'Website' @@ -154,6 +161,8 @@ potfile: script: - meson setup build --werror || (cat build/meson-logs/meson-log.txt && exit 1) - meson compile -C build libvirt-pot-dep libvirt-pot + after_script: + - test "$CI_JOB_STATUS" != "success" && exit 1; - cp po/libvirt.pot libvirt.pot artifacts: expose_as: 'Potfile' -- 2.39.1
Instead of open-coding the script steps like we've done until now, use a shell script. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- .gitlab-ci.yml | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index abd7498058..747209dca9 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -26,14 +26,11 @@ include: key: "$CI_JOB_NAME" script: - *script_variables - - meson setup build --werror $MESON_ARGS || (cat build/meson-logs/meson-log.txt && exit 1) - - meson dist -C build --no-tests - if test -x /usr/bin/rpmbuild && test "$RPM" != "skip"; then - rpmbuild --clean --nodeps --define "_without_mingw 1" -ta build/meson-dist/libvirt-*.tar.xz; + ci/build.sh --rpmbuild; else - meson compile -C build; - meson test -C build --no-suite syntax-check --print-errorlogs; + ci/build.sh --build --test; fi after_script: - test "$CI_JOB_STATUS" != "success" && exit 1; @@ -41,6 +38,8 @@ include: then mv "$HOME"/rpmbuild/RPMS/x86_64/ libvirt-rpms/; fi + variables: + MESON_TEST_ARGS: --no-suite syntax-check --print-errorlogs .native_build_job_prebuilt_env: extends: @@ -59,9 +58,14 @@ include: key: "$CI_JOB_NAME" script: - *script_variables - - meson setup build --werror $MESON_OPTS || (cat build/meson-logs/meson-log.txt && exit 1) - - meson compile -C build - - if test "$CROSS" = "i686" ; then meson test -C build --no-suite syntax-check --print-errorlogs ; fi + - ci/build.sh --build + - if test "$CROSS" = "i686" ; + then + ci/build.sh --test; + fi + variables: + MESON_TEST_ARGS: --no-suite syntax-check --print-errorlogs + .cross_build_job_prebuilt_env: extends: @@ -80,8 +84,7 @@ include: .website_job: script: - *script_variables - - meson setup build --werror -Dsystem=true || (cat build/meson-logs/meson-log.txt && exit 1) - - DESTDIR=$(pwd)/install meson compile -C build install-web + - ci/build.sh --website after_script: - test "$CI_JOB_STATUS" != "success" && exit 1; - mv install/usr/share/doc/libvirt/html/ website @@ -116,9 +119,7 @@ website_local_env: stage: sanity_checks script: - *script_variables - - meson setup build --werror || (cat build/meson-logs/meson-log.txt && exit 1) - - meson compile -C build libvirt-pot-dep - - meson test -C build --suite syntax-check --no-rebuild --print-errorlogs + - ci/build.sh --codestyle codestyle_prebuilt_env: extends: @@ -159,8 +160,7 @@ potfile: before_script: - *script_variables script: - - meson setup build --werror || (cat build/meson-logs/meson-log.txt && exit 1) - - meson compile -C build libvirt-pot-dep libvirt-pot + - ci/build.sh --potfile after_script: - test "$CI_JOB_STATUS" != "success" && exit 1; - cp po/libvirt.pot libvirt.pot -- 2.39.1
ci/build.sh is already exporting all of those and there's no need for these variables to necessarily be defined and exported by GitLab explicitly, this can be transparent to the job definition plus it keeps everything important in a single place. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- .gitlab-ci.yml | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 747209dca9..7cbccb5fe6 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -7,14 +7,6 @@ stages: - integration_tests - sanity_checks -.script_variables: &script_variables | - export CCACHE_BASEDIR="$(pwd)" - export CCACHE_DIR="$CCACHE_BASEDIR/ccache" - export CCACHE_MAXSIZE="500M" - export PATH="$CCACHE_WRAPPERSDIR:$PATH" - export VIR_TEST_VERBOSE="1" - export VIR_TEST_DEBUG="1" - include: - '/ci/gitlab.yml' - '/ci/integration.yml' @@ -25,7 +17,6 @@ include: - ccache/ key: "$CI_JOB_NAME" script: - - *script_variables - if test -x /usr/bin/rpmbuild && test "$RPM" != "skip"; then ci/build.sh --rpmbuild; @@ -57,7 +48,6 @@ include: - ccache/ key: "$CI_JOB_NAME" script: - - *script_variables - ci/build.sh --build - if test "$CROSS" = "i686" ; then @@ -83,7 +73,6 @@ include: # https://gitlab.com/libvirt/libvirt/-/jobs/artifacts/master/download?job=webs... .website_job: script: - - *script_variables - ci/build.sh --website after_script: - test "$CI_JOB_STATUS" != "success" && exit 1; @@ -118,7 +107,6 @@ website_local_env: .codestyle_job: stage: sanity_checks script: - - *script_variables - ci/build.sh --codestyle codestyle_prebuilt_env: @@ -157,8 +145,6 @@ potfile: - if: '$CI_PROJECT_NAMESPACE == $RUN_UPSTREAM_NAMESPACE && $CI_PIPELINE_SOURCE == "push" && $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH' when: on_success - when: never - before_script: - - *script_variables script: - ci/build.sh --potfile after_script: -- 2.39.1
On Mon, Feb 06, 2023 at 02:52:57PM +0100, Erik Skultety wrote:
This is a follow up to: https://listman.redhat.com/archives/libvir-list/2023-January/237201.html
The effort here is to unify the way builds/tests are executed in GitLab CI vs local container executions and make another step forward in terms of reproducibility of (specifically) GitLab environments.
Even though code to run all but one (coverity) jobs from GitLab via the build.sh script is added with this series, local behavior remains the same as before this series. The reason for that is that that will require more patches ridding of the Makefile which is currently used and instead integrate usage of lcitool with the ci/helper Python script which is currently the entry point for local container executions.
snip
.gitlab-ci.yml | 56 ++++++++++------------- ci/Makefile | 16 ++++--- ci/build.sh | 121 +++++++++++++++++++++++++++++++++++++++++++------ ci/helper | 21 ++++++--- 4 files changed, 155 insertions(+), 59 deletions(-) mode change 100644 => 100755 ci/build.sh
I'm really super unenthusiastic about this change, and also the similar change to add an ci/integration.sh. Shell scripting is something we worked hard to eliminate from libvirt. It is an awful language to do anything non-trivial with, error handling, lack of data structures, variable handling, portability are all bug generators. I know the .gitlab-ci.yml 'script' commands are technically shell script, but they are pretty trivial bits and don't have to worry about portability for dash vs bash etc, or complex control logic. The majority of it is just a simple list of commands to invoke, with the occasional conditional. The build.sh script is by contrast significantly more complex. By the very nature of taking "N" separate gitlab jobs and multiplexing them onto the one shell script, we're now having todo command line arg parsing in shell and de-multiplexing back to commands. The CI logic is now split up across even more sources - the gitlab config, the build.sh script and the meson.build files, which I think is worse for maintaining this too. I appreciate the goal of being able to run CI commands locally, but I'm not feeling this approach is a net win. I'd much rather just having developers invoke the meson command directly, which is not that hard. If we do really want to provide something that 100% mirrors the gitlab CI job commands though, I'm even more convinced now that we should be using the .gitlab-ci.yml as the canonical source. Since I last mentioned that idea, I found out that this should be something we can achieve via the gitlab runner 'exec' command. I've not quite figured out the right incantations though. I can get it to work for a simple repo, but not for the lbivirt.git yet as it seems to not find the included yml files. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Mon, Feb 06, 2023 at 03:45:12PM +0000, Daniel P. Berrangé wrote:
On Mon, Feb 06, 2023 at 02:52:57PM +0100, Erik Skultety wrote:
This is a follow up to: https://listman.redhat.com/archives/libvir-list/2023-January/237201.html
The effort here is to unify the way builds/tests are executed in GitLab CI vs local container executions and make another step forward in terms of reproducibility of (specifically) GitLab environments.
Even though code to run all but one (coverity) jobs from GitLab via the build.sh script is added with this series, local behavior remains the same as before this series. The reason for that is that that will require more patches ridding of the Makefile which is currently used and instead integrate usage of lcitool with the ci/helper Python script which is currently the entry point for local container executions.
snip
.gitlab-ci.yml | 56 ++++++++++------------- ci/Makefile | 16 ++++--- ci/build.sh | 121 +++++++++++++++++++++++++++++++++++++++++++------ ci/helper | 21 ++++++--- 4 files changed, 155 insertions(+), 59 deletions(-) mode change 100644 => 100755 ci/build.sh
I'm really super unenthusiastic about this change, and also the similar change to add an ci/integration.sh. Shell scripting is something we worked hard to eliminate from libvirt. It is an awful language to do anything non-trivial with, error handling, lack of data structures, variable handling, portability are all bug generators.
I know the .gitlab-ci.yml 'script' commands are technically shell script, but they are pretty trivial bits and don't have to worry about portability for dash vs bash etc, or complex control logic. The majority of it is just a simple list of commands to invoke, with the occasional conditional.
The build.sh script is by contrast significantly more complex. By the very nature of taking "N" separate gitlab jobs and multiplexing them onto the one shell script, we're now having todo command line arg parsing in shell and de-multiplexing back to commands. The CI logic is now split up across even more sources - the gitlab config, the build.sh script and the meson.build files, which I think is worse for maintaining this too.
True, except that even despite that, and I know what I'm talking about since I wrote the integration jobs templates, GitLab is super picky; you can't debug the syntax problems properly; certain common syntactic sugars don't work and have to be replaced by less common counterparts; using YAML for bash formatting leads to many table-flipping moments; you have to wait for Nx10 minutes before it fails with some ridiculous error you would have caught early locally but you couldn't because of the stage and artifact dependencies. So, once you have it finally in place it's unarguably nice, but the road towards that isn't really rainbow and unicorns either, it's still shell after all. Since I'm mainly working on this part of libvirt, I myself would appreciate if I could easily just run a single script instead of copy pasting commands one-by-one to a local VM to catch stupid errors quickly rather than wait for GitLab to fail.
I appreciate the goal of being able to run CI commands locally, but I'm not feeling this approach is a net win. I'd much rather just having developers invoke the meson command directly, which is not that hard.
Well, the big picture here is about making running integration tests locally less painful than it is now...plus increase the amount of automation involved. That's the major driver here - libvirt needs to be built locally in a safe environment before integration tests could be run against it without touching the host. So, with this script, while I agree with everything you said about Bash, was just one step towards getting the automation I mentioned in place. I also considered Python (yeah..why), but that would be super akward. TL;DR: if we don't do anything about how we currently maintain the build recipes (note we're maintaining 2 already), everybody will continue ignoring the integration test suite, unless we enable merge requests where the status quo would be somewhat (but not completely) eliminated. With integration tests you can't ignore the aspect of getting feedback early compared to waiting for GitLab CI.
If we do really want to provide something that 100% mirrors the gitlab CI job commands though, I'm even more convinced now that we should be using the .gitlab-ci.yml as the canonical source.
Since I last mentioned that idea, I found out that this should be something we can achieve via the gitlab runner 'exec' command.
Haven't heard of ^this one, but I wonder how we can get something meaningful out of it for the purposes I mentioned above. Erik
I've not quite figured out the right incantations though. I can get it to work for a simple repo, but not for the lbivirt.git yet as it seems to not find the included yml files.
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Mon, Feb 06, 2023 at 05:19:52PM +0100, Erik Skultety wrote:
On Mon, Feb 06, 2023 at 03:45:12PM +0000, Daniel P. Berrangé wrote:
On Mon, Feb 06, 2023 at 02:52:57PM +0100, Erik Skultety wrote:
This is a follow up to: https://listman.redhat.com/archives/libvir-list/2023-January/237201.html
The effort here is to unify the way builds/tests are executed in GitLab CI vs local container executions and make another step forward in terms of reproducibility of (specifically) GitLab environments.
Even though code to run all but one (coverity) jobs from GitLab via the build.sh script is added with this series, local behavior remains the same as before this series. The reason for that is that that will require more patches ridding of the Makefile which is currently used and instead integrate usage of lcitool with the ci/helper Python script which is currently the entry point for local container executions.
snip
.gitlab-ci.yml | 56 ++++++++++------------- ci/Makefile | 16 ++++--- ci/build.sh | 121 +++++++++++++++++++++++++++++++++++++++++++------ ci/helper | 21 ++++++--- 4 files changed, 155 insertions(+), 59 deletions(-) mode change 100644 => 100755 ci/build.sh
I'm really super unenthusiastic about this change, and also the similar change to add an ci/integration.sh. Shell scripting is something we worked hard to eliminate from libvirt. It is an awful language to do anything non-trivial with, error handling, lack of data structures, variable handling, portability are all bug generators.
I know the .gitlab-ci.yml 'script' commands are technically shell script, but they are pretty trivial bits and don't have to worry about portability for dash vs bash etc, or complex control logic. The majority of it is just a simple list of commands to invoke, with the occasional conditional.
The build.sh script is by contrast significantly more complex. By the very nature of taking "N" separate gitlab jobs and multiplexing them onto the one shell script, we're now having todo command line arg parsing in shell and de-multiplexing back to commands. The CI logic is now split up across even more sources - the gitlab config, the build.sh script and the meson.build files, which I think is worse for maintaining this too.
True, except that even despite that, and I know what I'm talking about since I wrote the integration jobs templates, GitLab is super picky; you can't debug the syntax problems properly; certain common syntactic sugars don't work and have to be replaced by less common counterparts; using YAML for bash formatting leads to many table-flipping moments; you have to wait for Nx10 minutes before it fails with some ridiculous error you would have caught early locally but you couldn't because of the stage and artifact dependencies. So, once you have it finally in place it's unarguably nice, but the road towards that isn't really rainbow and unicorns either, it's still shell after all. Since I'm mainly working on this part of libvirt, I myself would appreciate if I could easily just run a single script instead of copy pasting commands one-by-one to a local VM to catch stupid errors quickly rather than wait for GitLab to fail.
I appreciate the goal of being able to run CI commands locally, but I'm not feeling this approach is a net win. I'd much rather just having developers invoke the meson command directly, which is not that hard.
Well, the big picture here is about making running integration tests locally less painful than it is now...plus increase the amount of automation involved. That's the major driver here - libvirt needs to be built locally in a safe environment before integration tests could be run against it without touching the host. So, with this script, while I agree with everything you said about Bash, was just one step towards getting the automation I mentioned in place. I also considered Python (yeah..why), but that would be super akward.
TL;DR: if we don't do anything about how we currently maintain the build recipes (note we're maintaining 2 already), everybody will continue ignoring the integration test suite, unless we enable merge requests where the status quo would be somewhat (but not completely) eliminated. With integration tests you can't ignore the aspect of getting feedback early compared to waiting for GitLab CI.
If we do really want to provide something that 100% mirrors the gitlab CI job commands though, I'm even more convinced now that we should be using the .gitlab-ci.yml as the canonical source.
Since I last mentioned that idea, I found out that this should be something we can achieve via the gitlab runner 'exec' command.
Haven't heard of ^this one, but I wonder how we can get something meaningful out of it for the purposes I mentioned above.
Erik
So, I had a brief look at gihttps://gitlab.com/gitlab-org/gitlab-runner/-/issues/2797tlab-runner exec this morning. Long story short, it's deprecated [1] and though it was scheduled for complete removal, that plan was put on hold for now. Nevertheless, it's not getting any features that are introduced to .gitlab-ci.yml. The gist is that it doesn't understand 'include' nor 'extends' which we're using heavily across our gitlab configuration hierarchy, so it's a no-go. It also doesn't support artifacts in any form, so while it technically should be possible to save RPM builds in a volume (not sure if the bind mount is cleared on job completion or not) we could not pass them into a VM running the integration tests the same convenient way as we do in the CI. Another thing to consider is that this solution also requires to have the gitlab-runner binary installed locally, which many in the community may not be comfortable with, but that's up for a debate, but hey, you can always build it yourself. There's a plan to rework GitLab's local CI execution functionality heavily [2] before the 'exec' command is sunset, but ultimately their plan is have something working by Q3 FY24, which can be anywhere from March 2024 until September 2024, that's more than a year ahead. In your reply you mentioned the unappealing complexity of the script potentially leading to bugs. At the same time though one currently can't consume .gitlab-ci.yml recipes to run a local integration test in a VM. So, how about I get rid of the multiplexing and CLI parsing by placing each job recipe in a standalone script or even going one step further and extract the commonalities to a separate file that would be source from within each job script? Would you consider that meeting each other halfway? As for POSIX compliance, I guess this would be a soft-requirement based on whether shellcheck was run during review, does gitlab do something better? IIUC there's no pre-check and you'll only know after a job was executed that there was an incompatible statement. [1] https://docs.gitlab.com/runner/commands/#gitlab-runner-exec-deprecated [2] https://gitlab.com/gitlab-org/gitlab-runner/-/issues/2797 Regards, Erik
On Tue, Feb 07, 2023 at 10:56:59AM +0100, Erik Skultety wrote:
So, I had a brief look at https://gitlab.com/gitlab-org/gitlab-runner/-/issues/2797tlab-runner exec this morning. Long story short, it's deprecated [1] and though it was scheduled for complete removal, that plan was put on hold for now. Nevertheless, it's not getting any features that are introduced to .gitlab-ci.yml. The gist is that it doesn't understand 'include' nor 'extends' which we're using heavily across our gitlab configuration hierarchy, so it's a no-go.
Urgh, I wasn't anticipating that they used completely different code to interpret the config in the runner, than elsewhere in gitlab. No wonder they want to kill it off :-(
It also doesn't support artifacts in any form, so while it technically should be possible to save RPM builds in a volume (not sure if the bind mount is cleared on job completion or not) we could not pass them into a VM running the integration tests the same convenient way as we do in the CI.
Yep, I'm sure there are possible workarounds, but only worth it if the rest of the thing is viable, which isn't the case with include/extends support missing :-(
In your reply you mentioned the unappealing complexity of the script potentially leading to bugs. At the same time though one currently can't consume .gitlab-ci.yml recipes to run a local integration test in a VM.
So, how about I get rid of the multiplexing and CLI parsing by placing each job recipe in a standalone script or even going one step further and extract the commonalities to a separate file that would be source from within each job script? Would you consider that meeting each other halfway?
Lets consider the core CI (build + unit tests in .gitlab-ci.yml), separately from the integration test CI (ci/integration-template.yml). For the core CI, I'm just not convinced of the benefit of moving the commands out into a shell script, as the set of commands is small and straightforward. For the ingration CI though, I can see benefit because of all the command logic related to fetching and building dependancies and setup tasks. In retrospect this is a sign that we need to introduce a higher level frontend for invoking the libvirt-tck tests. We already have avocado as a frontend to replace the existing 'libvirt-tck' command line tool, but we never got rid of the latter. We should probably get rid of the existing 'libvirt-tck' cli and introduce a brand new pure python 'libvirt-tck' cli, with commands for running avocado, optionally installing deps, and all the other interesting things devs want help with. The ci/integration-template.yml could then invoke this newly re-invented 'libvirt-tck' command and eliminate most of the complexity in the integration-template.yml file. Essentially we need to look at the gitlab CI yml files as being a very thin glue layer, and anything interesting should be in the end user facing tools developers already use. Introducing a new libvirt-tck python cli is likely not a quick fix though. So if we split the ci/integration-template.yml commands out into a set of bash scripts, that'll give us a good illustration of what commands we would want in a new libvirt-tck cli impl. Then as the new libvirt-tck cli arrives, we can eliminate the bash scripts from libvirt.git I guess this would mean dropping much of this particular series, but keeping much of the corresponding integration test series.
As for POSIX compliance, I guess this would be a soft-requirement based on whether shellcheck was run during review, does gitlab do something better? IIUC there's no pre-check and you'll only know after a job was executed that there was an incompatible statement.
I think its ok if we use bash scripts as a short term solution, with a plan to move to a more supportable python impl in future. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
...
So, how about I get rid of the multiplexing and CLI parsing by placing each job recipe in a standalone script or even going one step further and extract the commonalities to a separate file that would be source from within each job script? Would you consider that meeting each other halfway?
Lets consider the core CI (build + unit tests in .gitlab-ci.yml), separately from the integration test CI (ci/integration-template.yml).
For the core CI, I'm just not convinced of the benefit of moving the commands out into a shell script, as the set of commands is small and straightforward.
For the ingration CI though, I can see benefit because of all the command logic related to fetching and building dependancies and setup tasks.
In retrospect this is a sign that we need to introduce a higher level frontend for invoking the libvirt-tck tests. We already have avocado as a frontend to replace the existing 'libvirt-tck' command line tool, but we never got rid of the latter. We should probably get rid of the existing 'libvirt-tck' cli and introduce a brand new pure python 'libvirt-tck' cli, with commands for running avocado, optionally installing deps, and all the other interesting things devs want help with. The ci/integration-template.yml could then invoke this newly re-invented 'libvirt-tck' command and eliminate most of the complexity in the integration-template.yml file.
Well, that was the plan ever since June 2021 when the concept of lavocado was proposed to the list, but it wasn't widely accepted as it appears we haven't found the common ground back then. IIRC the idea of making lavocado the new TCK re-incarnation was actually accepted, but the effort on its own didn't get traction, particularly because IMO there's not that much value spending time re-writing existing Perl tests to Python without investing time into improving the environment preparation and hence eliminating the biggest problem all these frameworks have which is that to some extent they're still touching the system libvirt on the host, potentially leaving it in an inconsitent state. And so I've been steering my attention towards making the necessary changes to lcitool and hopefully make it the virt-stack's universal environment preparation tool. As for what the new TCK cli should look like, that's a good question, FWIW I don't agree we should introduce any logic related to dependency installation into the environment, nor anything that is environment related - we've done that with Avocado-VT and we know how it ended; the pure Avocado framework also provides some of these features which I don't necessarily agree with (Ansible integration, container execution integration, etc.), but luckily these are not mandatory. I am a strong advocate of dividing the CI problem into environment preparation and test execution and not actually trying to combine these two, it's not going to end up well IMO (we have a precedent) and instead the new framework needs to do a single thing which is execute the tests and do it well and do it agnostic to the environment AND to the language in which the tests have been written - all of which Avocado is very good at. Now, while I'm very happy that the idea of reworking TCK has come to the table again, like you mention below, it's not an easy and quick task, so we need some temporary bandaids before we get there and I'm totally fine dropping much of the conversion efforts in the future, but I still think it can help us short term.
Essentially we need to look at the gitlab CI yml files as being a very thin glue layer, and anything interesting should be in the end user facing tools developers already use.
Introducing a new libvirt-tck python cli is likely not a quick fix though. So if we split the ci/integration-template.yml commands out into a set of bash scripts, that'll give us a good illustration of what commands we would want in a new libvirt-tck cli impl. Then as the new libvirt-tck cli arrives, we can eliminate the bash scripts from libvirt.git
Yes, agreed.
I guess this would mean dropping much of this particular series, but keeping much of the corresponding integration test series.
How much of this series should be dropped in your opinion, given my vision I tried to briefly outline above? I admit I got confused a bit by what your actual expectation from this series is. Erik
As for POSIX compliance, I guess this would be a soft-requirement based on whether shellcheck was run during review, does gitlab do something better? IIUC there's no pre-check and you'll only know after a job was executed that there was an incompatible statement.
I think its ok if we use bash scripts as a short term solution, with a plan to move to a more supportable python impl in future.
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Mon, Feb 06, 2023 at 03:45:12PM +0000, Daniel P. Berrangé wrote:
On Mon, Feb 06, 2023 at 02:52:57PM +0100, Erik Skultety wrote: I appreciate the goal of being able to run CI commands locally, but I'm not feeling this approach is a net win. I'd much rather just having developers invoke the meson command directly, which is not that hard.
If we do really want to provide something that 100% mirrors the gitlab CI job commands though, I'm even more convinced now that we should be using the .gitlab-ci.yml as the canonical source.
Since I last mentioned that idea, I found out that this should be something we can achieve via the gitlab runner 'exec' command.
I wonder if we've been thinking about this from the wrong angle. We've been looking at the problem of "we have a gitlab-ci.yml" and we want to be able to locally run the jobs it defines. This creates us the problem of interpreting the gitlab-ci.yml file, which is very hard. 95% of what we define in the gitlab-ci.yml file comes in via the includes of ci/gitlab.yml which is entirely auto-generated from our ci/manifest.yml file. IOW, can we rephase the problem to "we have a ci/manifest.yml" and we want to locally run the jobs it implies. We already have the code for parsing the ci/manifest.yml file, and so (mostly) know what jobs we expect to have. What we're missing is the project specific build rules which still live in the hand crafted .gitlab-ci.yml. If we could get the project specific build rules to be expressed in the ci/manifest.yml file, then we have all the info we need to be able to run jobs locally. We could then make even the root .gitlab-ci.yml be auto-generated, which would be nice. If we have the project build rules in ci/manifest.yml, we ahve lots of flexibility. We can easily trigger the builds in containers or VMs, or the local host, or whatever. 'lcitool' already has the command for runing builds in VMs, but that relies on build rules we defined separately in libvirt-ci.git and we've somewhat neglected the VM build logic since adopting containers. Essentially ci/manifest.yml would become a fully self-contained canonical representation of any info needed to run builds, and be independant of gitlab CI, or any other CI framework we might like to plug into in the future. So 'lcitool' can be the entrypoint for triggering builds that 100% match what gets run in gitlab Yes, this only addresses the core build/unittest side of things. The integration test suite is still a separate task to address. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Feb 14, 2023 at 02:31:26PM +0000, Daniel P. Berrangé wrote:
On Mon, Feb 06, 2023 at 03:45:12PM +0000, Daniel P. Berrangé wrote:
On Mon, Feb 06, 2023 at 02:52:57PM +0100, Erik Skultety wrote: I appreciate the goal of being able to run CI commands locally, but I'm not feeling this approach is a net win. I'd much rather just having developers invoke the meson command directly, which is not that hard.
If we do really want to provide something that 100% mirrors the gitlab CI job commands though, I'm even more convinced now that we should be using the .gitlab-ci.yml as the canonical source.
Since I last mentioned that idea, I found out that this should be something we can achieve via the gitlab runner 'exec' command.
I wonder if we've been thinking about this from the wrong angle.
We've been looking at the problem of "we have a gitlab-ci.yml" and we want to be able to locally run the jobs it defines. This creates us the problem of interpreting the gitlab-ci.yml file, which is very hard.
95% of what we define in the gitlab-ci.yml file comes in via the includes of ci/gitlab.yml which is entirely auto-generated from our ci/manifest.yml file.
IOW, can we rephase the problem to "we have a ci/manifest.yml" and we want to locally run the jobs it implies. We already have the code for parsing the ci/manifest.yml file, and so (mostly) know what jobs we expect to have.
What we're missing is the project specific build rules which still live in the hand crafted .gitlab-ci.yml.
If we could get the project specific build rules to be expressed in the ci/manifest.yml file, then we have all the info we need to be able to run jobs locally. We could then make even the root .gitlab-ci.yml be auto-generated, which would be nice.
Honestly ^this to me sounds too meta. The way I look at this effort is investing a significant amount of time into figuring out how build jobs which are simply too specific for every project could be described in a generic manner appropriately in YAML. I can't help it but it sounds like we'd be inventing a problem for ourselves to solve. It might very well be relevant in the future, I think we need faster results now. Like I mentioned in my previous replies, I totally share your view on Bash from the language POV, but if we make sure these scripts will avoid using any kind of CLI parsing and wannabe data structure implementation, then I believe we can have a reasonable compromise for the time being. After all it all boils down to how many people are willing to invest how much of their time into improving CI.
If we have the project build rules in ci/manifest.yml, we ahve lots of flexibility. We can easily trigger the builds in containers or VMs, or the local host, or whatever. 'lcitool' already has the command for runing builds in VMs, but that relies on build rules we defined separately in libvirt-ci.git and we've somewhat neglected the VM build logic since adopting containers.
Essentially ci/manifest.yml would become a fully self-contained canonical representation of any info needed to run builds, and be independant of gitlab CI, or any other CI framework we might like to plug into in the future. So 'lcitool' can be the entrypoint for triggering builds that 100% match what gets run in gitlab
Yes, this only addresses the core build/unittest side of things.
^This on its own would not be a problem as long as we have other tools ready to be chained in the pipeline (or locally) appropriately. Still, I'd be wary of trying to make a swiss-army knife from lcitool by generating everything especially with tools like Tekton which has become the industry standard for pipeline and job generation on K8S, something that might become relevant to libvirt in coming years too, so I'm not completely stoked about this idea proposal as I'm afraid we'll be forced to ditch some of it in the future nonetheless. Regards, Erik
On Wed, Feb 15, 2023 at 09:06:46AM +0100, Erik Skultety wrote:
On Tue, Feb 14, 2023 at 02:31:26PM +0000, Daniel P. Berrangé wrote:
On Mon, Feb 06, 2023 at 03:45:12PM +0000, Daniel P. Berrangé wrote:
On Mon, Feb 06, 2023 at 02:52:57PM +0100, Erik Skultety wrote: I appreciate the goal of being able to run CI commands locally, but I'm not feeling this approach is a net win. I'd much rather just having developers invoke the meson command directly, which is not that hard.
If we do really want to provide something that 100% mirrors the gitlab CI job commands though, I'm even more convinced now that we should be using the .gitlab-ci.yml as the canonical source.
Since I last mentioned that idea, I found out that this should be something we can achieve via the gitlab runner 'exec' command.
I wonder if we've been thinking about this from the wrong angle.
We've been looking at the problem of "we have a gitlab-ci.yml" and we want to be able to locally run the jobs it defines. This creates us the problem of interpreting the gitlab-ci.yml file, which is very hard.
95% of what we define in the gitlab-ci.yml file comes in via the includes of ci/gitlab.yml which is entirely auto-generated from our ci/manifest.yml file.
IOW, can we rephase the problem to "we have a ci/manifest.yml" and we want to locally run the jobs it implies. We already have the code for parsing the ci/manifest.yml file, and so (mostly) know what jobs we expect to have.
What we're missing is the project specific build rules which still live in the hand crafted .gitlab-ci.yml.
If we could get the project specific build rules to be expressed in the ci/manifest.yml file, then we have all the info we need to be able to run jobs locally. We could then make even the root .gitlab-ci.yml be auto-generated, which would be nice.
Honestly ^this to me sounds too meta. The way I look at this effort is investing a significant amount of time into figuring out how build jobs which are simply too specific for every project could be described in a generic manner appropriately in YAML. I can't help it but it sounds like we'd be inventing a problem for ourselves to solve. It might very well be relevant in the future, I think we need faster results now. Like I mentioned in my previous replies, I totally share your view on Bash from the language POV, but if we make sure these scripts will avoid using any kind of CLI parsing and wannabe data structure implementation, then I believe we can have a reasonable compromise for the time being. After all it all boils down to how many people are willing to invest how much of their time into improving CI.
If we have the project build rules in ci/manifest.yml, we ahve lots of flexibility. We can easily trigger the builds in containers or VMs, or the local host, or whatever. 'lcitool' already has the command for runing builds in VMs, but that relies on build rules we defined separately in libvirt-ci.git and we've somewhat neglected the VM build logic since adopting containers.
Essentially ci/manifest.yml would become a fully self-contained canonical representation of any info needed to run builds, and be independant of gitlab CI, or any other CI framework we might like to plug into in the future. So 'lcitool' can be the entrypoint for triggering builds that 100% match what gets run in gitlab
Yes, this only addresses the core build/unittest side of things.
^This on its own would not be a problem as long as we have other tools ready to be chained in the pipeline (or locally) appropriately. Still, I'd be wary of trying to make a swiss-army knife from lcitool by generating everything especially with tools like Tekton which has become the industry standard for pipeline and job generation on K8S, something that might become relevant to libvirt in coming years too, so I'm not completely stoked about this idea proposal as I'm afraid we'll be forced to ditch some of it in the future nonetheless.
Ping. I'd still like some conclusion on this whether the proposed approach (with changes like decomposition to scripts) is a complete no go and I need to drop the whole series or if there's anything we can do about it right now. Regards, Erik
participants (3)
-
Andrea Bolognani -
Daniel P. Berrangé -
Erik Skultety