Re: [libvirt PATCH v2 1/1] cirrus: Generate jobs dynamically
On Tue, 2020-06-30 at 16:01 +0200, Erik Skultety wrote:
On Mon, Jun 29, 2020 at 08:51:51PM +0200, Andrea Bolognani wrote:
> +# Note that the $PATH environment variable has to be treated with
> +# special care, because we can't just override it at the GitLab CI job
> +# definition level or we risk breaking it completely.
Informative, thanks :). I just wish I didn't have to google what semantics the
various types of parameter expansion in Bash had (obviously not your fault).
I can assure you that I had to Google that while writing the code as
well ;)
> .cirrus_build_job_template: &cirrus_build_job_definition
> stage: builds
> image: registry.gitlab.com/libvirt/libvirt-ci/cirrus-run:master
> script:
> - - cirrus-run ci/cirrus/$NAME.yml.j2
> + - source ci/cirrus/libvirt-$NAME.vars
> + - sed -e "s|[@]CI_REPOSITORY_URL@|$CI_REPOSITORY_URL|g"
> + -e "s|[@]CI_COMMIT_REF_NAME@|$CI_COMMIT_REF_NAME|g"
> + -e "s|[@]CI_COMMIT_SHA@|$CI_COMMIT_SHA|g"
> + -e "s|[@]CIRRUS_VM_INSTANCE_TYPE@|$CIRRUS_VM_INSTANCE_TYPE|g"
> + -e
"s|[@]CIRRUS_VM_IMAGE_SELECTOR@|$CIRRUS_VM_IMAGE_SELECTOR|g"
> + -e "s|[@]CIRRUS_VM_IMAGE_NAME@|$CIRRUS_VM_IMAGE_NAME|g"
> + -e "s|[@]INSTALL_COMMAND@|$INSTALL_COMMAND|g"
> + -e "s|[@]PATH@|$PATH_EXTRA${PATH_EXTRA:+:}\$PATH|g"
> + -e "s|[@]PKG_CONFIG_PATH@|$PKG_CONFIG_PATH|g"
> + -e "s|[@]PKGS@|$PKGS|g"
> + -e "s|[@]MAKE@|$MAKE|g"
> + -e "s|[@]PYTHON@|$PYTHON|g"
> + <ci/cirrus/build.yml >ci/cirrus/$NAME.yml
> + - cat ci/cirrus/$NAME.yml
was ^this 'cat' just for debugging purposes or was that intended to be part of
the job output in production?
Initially it was for debugging purposes, but I didn't remove it
before posting because I felt it could be useful to keep it there:
unlike the other GitLab CI jobs, in this case there's an additional
level of indirection and thus it's a bit harder to figure out what's
going on by just looking at the bits that are in the repository.
I could drop it, though.
> +for host in $HOSTS
> +do
> + $LCITOOL dockerfile "$host" libvirt --variables
>"$host.vars"
> +done
Overall, I prefer this to v1 because of the further level of abstraction this
introduces. However, the final format we're producing here is a YAML template
rather than a Dockerfile, so introducing just an option for the "dockerfile"
subcommand rather than a dedicated "cirrus-ci-file" (or similar) subcommand
doesn't feel completely right, especially from the long run perspective. At
some point we'd like to generate the whole gitlab-ci.yml file, so that one
would very likely get a dedicated subcommand as well - it should, as it's just
another "plugin" generator. If we expand beyond libvirt, say QEMU, we may need
to add a different CI provider plugin as well.
Right. The name was something that Dan also argued against in the
libvirt-ci MR[1], and I'm not opposed to changing it. In that case,
however, I would go for a more generic "variables" or something like
that instead, because there's really nothing specific to Cirrus CI or
any one CI service in the variables: they only describe the build
environment the CI job will eventually run in.
So even if we start introducing sub-commands specific to various CI
services, a generic way to get just the variables themselves can be
useful and, among other things, can serve as a way to integrate with
CI services or local environments that we don't have dedicated
support for.
[1] https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/30
--
Andrea Bolognani / Red Hat / Virtualization