Re: [libvirt PATCH 2/2] ci: refresh with latest lcitool manifest
On Thu, Oct 06, 2022 at 09:42:26AM +0200, Peter Krempa wrote:
On Tue, Oct 04, 2022 at 08:51:50 -0400, Daniel P. Berrangé wrote:
> This refresh switches the CI for contributors to be triggered by merge
> requests. Pushing to a branch in a fork will no longer run CI pipelines,
> in order to avoid consuming CI minutes. To regain the original behaviour
> contributors can opt-in to a pipeline on push
>
> git push <remote> -o ci.variable=RUN_PIPELINE=1
This should be also documented in ci/README.rst as this commit message
will become gradually harder to find.
It is present in comments at the top of ci/gitlab.yml, along with
info about some other variables that exist.
I welcome if pipelines are run automatically and I don't have to
fiddle
with the web UI or try to figure out which custom approach the project
picked to run pipelines especially if I can't be bothered to set up the
test env locally e.g. for a one-off contribution.
Users were always able to opt-out of running pipelines if they wish to
just store code in their fork by using '-o ci.skip=true' which is a
gitlab option thus same for all projects.
That was fine (for users, but not GitLab's $$$ burn) when CI was
unlimited, but we need to be more respectful of users CI quota now
it is finite and very easy to quickly exhaust.
> This variable can also be set globally on the repository, though
this is
> not recommended.
Also mention how to do this for anyone interested to preserve the old
behaviour by default.
As above, I really recommend against doing this, unles you want to
burn through your CI minutes quickly and find you can't run anymore
for the rest of the month. If you really really really want to take
this risk, then it is just Settings -> CI -> Variables in the repo
in question
> Upstream repo pushes to branches will run CI.
>
> The use of containers has changed in this update, with only the upstream
> repo creating containers, in order to avoid consuming contributors'
> limited storage quotas. A fork with existing container images may delete
> them. Containers will be rebuilt upstream when pushing commits with CI
> changes to the default branch. Any other scenario with CI changes will
> simply install build pre-requisite packages in a throaway environment,
> using the ci/buildenv/ scripts. These scripts may also be used on a
> contributor's local machines.
>
> With pipelines triggered by merge requests, it is also now possible to
> workaround the inability of contributors to run pipelines if they have
> run out of CI quota. A project member can trigger a pipeline from the
> merge request, which will run in context of upstream, however, note
> this should only be done after reviewing the code for any malicious
> CI changes.
This is good to know. Is there a warning or notification that the
pipeline then runs in the context of the project? If not we should
document it at least so that project members are aware not to run
pipelines without thinking about it.
If you look at the pipelines page:
https://gitlab.com/libvirt/libvirt-glib/-/merge_requests/38/pipelines
The 'merge request' label there indicates it was triggered by a
merge request. The 'fork' label further indicates that it ran
in context of the repo fork, instead of upstream. IOW, the lack
of a 'fork' label means it is upstream.
While I'm not a fan of changes to the pipeline running, I'll
be able to
live with them once you document them (separately). I welcome the
removal of building containers in the forks.
Reviewed-by: Peter Krempa <pkrempa(a)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 :|