4:43 a.m.
Andrea Bolognani (3):
ci: Fix upstream-qemu job definitions
ci: Move upstream-qemu job to Fedora 39
ci: Add notes for integration jobs
ci/integration.yml | 58 +++++++++++++++++++++++++++++++---------------
1 file changed, 39 insertions(+), 19 deletions(-)
--
2.43.0
4:43 a.m.
New subject: [PATCH 1/3] ci: Fix upstream-qemu job definitions
These are jobs are supposed to be running tests using a QEMU
binary built from the latest upstream sources, but right now
they're just doing the same thing as the other jobs for the
target. Use the correct job templates.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
ci/integration.yml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ci/integration.yml b/ci/integration.yml
index 8b66a8c688..c234696d10 100644
--- a/ci/integration.yml
+++ b/ci/integration.yml
@@ -132,7 +132,7 @@ fedora-38-tests-local-env:
fedora-38-upstream-qemu-tests-prebuilt-env:
extends:
- - .integration_tests_prebuilt_env
+ - .integration_tests_upstream_qemu_prebuilt_env
- .fedora-38-upstream-qemu-tests
needs:
- x86_64-fedora-38-prebuilt-env
@@ -147,7 +147,7 @@ fedora-38-upstream-qemu-tests-prebuilt-env:
fedora-38-upstream-qemu-tests-local-env:
extends:
- - .integration_tests_local_env
+ - .integration_tests_upstream_qemu_local_env
- .fedora-38-upstream-qemu-tests
needs:
- x86_64-fedora-38-local-env
--
2.43.0
2:07 p.m.
New subject: [PATCH 1/3] ci: Fix upstream-qemu job definitions
On Mon, Jan 08, 2024 at 11:43:22 +0100, Andrea Bolognani wrote:
These are jobs are supposed to be running tests using a QEMU
binary built from the latest upstream sources, but right now
they're just doing the same thing as the other jobs for the
target. Use the correct job templates.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
ci/integration.yml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Looks reasonable, but IMO the CI stuff got out of hand complexity wise,
so I'm not as confident in my review as usually.
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
2:41 a.m.
New subject: [PATCH 1/3] ci: Fix upstream-qemu job definitions
On Mon, Jan 08, 2024 at 09:07:33PM +0100, Peter Krempa wrote:
On Mon, Jan 08, 2024 at 11:43:22 +0100, Andrea Bolognani wrote:
> These are jobs are supposed to be running tests using a QEMU
> binary built from the latest upstream sources, but right now
> they're just doing the same thing as the other jobs for the
> target. Use the correct job templates.
>
> Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
> ---
> ci/integration.yml | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Looks reasonable, but IMO the CI stuff got out of hand complexity wise,
so I'm not as confident in my review as usually.
Yeah, I'm not too confident myself, and unlike regular CI jobs I
thought there was no way to test changes to the integration part
before pushing. I see now that the LIBVIRT_CI_INTEGRATION variable
exists though, so I'll give that a try and see whether I can get a
successful run out of it before pushing.
--
Andrea Bolognani / Red Hat / Virtualization
3:02 a.m.
New subject: [PATCH 1/3] ci: Fix upstream-qemu job definitions
On Tue, Jan 09, 2024 at 12:41:25AM -0800, Andrea Bolognani wrote:
On Mon, Jan 08, 2024 at 09:07:33PM +0100, Peter Krempa wrote:
> On Mon, Jan 08, 2024 at 11:43:22 +0100, Andrea Bolognani wrote:
> > These are jobs are supposed to be running tests using a QEMU
> > binary built from the latest upstream sources, but right now
> > they're just doing the same thing as the other jobs for the
> > target. Use the correct job templates.
> >
> > Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
> > ---
> > ci/integration.yml | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Looks reasonable, but IMO the CI stuff got out of hand complexity wise,
> so I'm not as confident in my review as usually.
Yeah, I'm not too confident myself, and unlike regular CI jobs I
thought there was no way to test changes to the integration part
before pushing. I see now that the LIBVIRT_CI_INTEGRATION variable
exists though, so I'll give that a try and see whether I can get a
successful run out of it before pushing.
Never mind, the jobs got stuck because a suitable runner can't be
found. Can't say that I'm surprised, we don't want to give anyone
access to those scarce resources... Anyway, at least the jobs were
created according to my expectations, so that's good to know. I'm
going to push the series now. Thanks for the review!
--
Andrea Bolognani / Red Hat / Virtualization
3:26 a.m.
New subject: [PATCH 1/3] ci: Fix upstream-qemu job definitions
On Tue, Jan 09, 2024 at 01:02:38AM -0800, Andrea Bolognani wrote:
On Tue, Jan 09, 2024 at 12:41:25AM -0800, Andrea Bolognani wrote:
> On Mon, Jan 08, 2024 at 09:07:33PM +0100, Peter Krempa wrote:
> > On Mon, Jan 08, 2024 at 11:43:22 +0100, Andrea Bolognani wrote:
> > > These are jobs are supposed to be running tests using a QEMU
> > > binary built from the latest upstream sources, but right now
> > > they're just doing the same thing as the other jobs for the
> > > target. Use the correct job templates.
> > >
> > > Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
> > > ---
> > > ci/integration.yml | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > Looks reasonable, but IMO the CI stuff got out of hand complexity wise,
> > so I'm not as confident in my review as usually.
>
> Yeah, I'm not too confident myself, and unlike regular CI jobs I
> thought there was no way to test changes to the integration part
> before pushing. I see now that the LIBVIRT_CI_INTEGRATION variable
> exists though, so I'll give that a try and see whether I can get a
> successful run out of it before pushing.
Never mind, the jobs got stuck because a suitable runner can't be
found. Can't say that I'm surprised, we don't want to give anyone
access to those scarce resources... Anyway, at least the jobs were
created according to my expectations, so that's good to know. I'm
going to push the series now. Thanks for the review!
If you create a pipeline in your fork, it would not be able to accesss
the runners, but if you create a merge request against upstream, it
would have access to the runners, since you're a project member.
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 :|
4:43 a.m.
New subject: [PATCH 2/3] ci: Move upstream-qemu job to Fedora 39
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
ci/integration.yml | 42 +++++++++++++++++++++---------------------
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/ci/integration.yml b/ci/integration.yml
index c234696d10..ff1bb5d81d 100644
--- a/ci/integration.yml
+++ b/ci/integration.yml
@@ -121,47 +121,47 @@ fedora-38-tests-local-env:
artifacts: true
-.fedora-38-upstream-qemu-tests:
+.fedora-39-tests:
variables:
# needed by libvirt-gitlab-executor
- DISTRO: fedora-38
+ DISTRO: fedora-39
# can be overridden in forks to set a different runner tag
LIBVIRT_CI_INTEGRATION_RUNNER_TAG: redhat-vm-host
tags:
- $LIBVIRT_CI_INTEGRATION_RUNNER_TAG
-fedora-38-upstream-qemu-tests-prebuilt-env:
+fedora-39-tests-prebuilt-env:
extends:
- - .integration_tests_upstream_qemu_prebuilt_env
- - .fedora-38-upstream-qemu-tests
+ - .integration_tests_prebuilt_env
+ - .fedora-39-tests
needs:
- - x86_64-fedora-38-prebuilt-env
+ - x86_64-fedora-39-prebuilt-env
- project: libvirt/libvirt-perl
- job: x86_64-fedora-38-prebuilt-env
+ job: x86_64-fedora-39-prebuilt-env
ref: master
artifacts: true
- project: libvirt/libvirt-python
- job: x86_64-fedora-38-prebuilt-env
+ job: x86_64-fedora-39-prebuilt-env
ref: master
artifacts: true
-fedora-38-upstream-qemu-tests-local-env:
+fedora-39-tests-local-env:
extends:
- - .integration_tests_upstream_qemu_local_env
- - .fedora-38-upstream-qemu-tests
+ - .integration_tests_local_env
+ - .fedora-39-tests
needs:
- - x86_64-fedora-38-local-env
+ - x86_64-fedora-39-local-env
- project: libvirt/libvirt-perl
- job: x86_64-fedora-38-prebuilt-env
+ job: x86_64-fedora-39-prebuilt-env
ref: master
artifacts: true
- project: libvirt/libvirt-python
- job: x86_64-fedora-38-prebuilt-env
+ job: x86_64-fedora-39-prebuilt-env
ref: master
artifacts: true
-.fedora-39-tests:
+.fedora-39-upstream-qemu-tests:
variables:
# needed by libvirt-gitlab-executor
DISTRO: fedora-39
@@ -170,10 +170,10 @@ fedora-38-upstream-qemu-tests-local-env:
tags:
- $LIBVIRT_CI_INTEGRATION_RUNNER_TAG
-fedora-39-tests-prebuilt-env:
+fedora-39-upstream-qemu-tests-prebuilt-env:
extends:
- - .integration_tests_prebuilt_env
- - .fedora-39-tests
+ - .integration_tests_upstream_qemu_prebuilt_env
+ - .fedora-39-upstream-qemu-tests
needs:
- x86_64-fedora-39-prebuilt-env
- project: libvirt/libvirt-perl
@@ -185,10 +185,10 @@ fedora-39-tests-prebuilt-env:
ref: master
artifacts: true
-fedora-39-tests-local-env:
+fedora-39-upstream-qemu-tests-local-env:
extends:
- - .integration_tests_local_env
- - .fedora-39-tests
+ - .integration_tests_upstream_qemu_local_env
+ - .fedora-39-upstream-qemu-tests
needs:
- x86_64-fedora-39-local-env
- project: libvirt/libvirt-perl
--
2.43.0
2:11 p.m.
New subject: [PATCH 2/3] ci: Move upstream-qemu job to Fedora 39
On Mon, Jan 08, 2024 at 11:43:23 +0100, Andrea Bolognani wrote:
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
ci/integration.yml | 42 +++++++++++++++++++++---------------------
1 file changed, 21 insertions(+), 21 deletions(-)
Reordering the entries made this very unpleasant to review.
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
2:44 a.m.
New subject: [PATCH 2/3] ci: Move upstream-qemu job to Fedora 39
On Mon, Jan 08, 2024 at 09:11:12PM +0100, Peter Krempa wrote:
On Mon, Jan 08, 2024 at 11:43:23 +0100, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
> ---
> ci/integration.yml | 42 +++++++++++++++++++++---------------------
> 1 file changed, 21 insertions(+), 21 deletions(-)
Reordering the entries made this very unpleasant to review.
Agreed. I don't know how to make it better though. Is there a way to
convince git to not try quite as hard to reuse existing lines? The
alternative would be to have two patches, one that drops the Fedora
38 job and one that adds the Fedora 39 one.
--
Andrea Bolognani / Red Hat / Virtualization
2:30 a.m.
New subject: [PATCH 2/3] ci: Move upstream-qemu job to Fedora 39
On Tue, Jan 09, 2024 at 00:44:45 -0800, Andrea Bolognani wrote:
On Mon, Jan 08, 2024 at 09:11:12PM +0100, Peter Krempa wrote:
> On Mon, Jan 08, 2024 at 11:43:23 +0100, Andrea Bolognani wrote:
> > Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
> > ---
> > ci/integration.yml | 42 +++++++++++++++++++++---------------------
> > 1 file changed, 21 insertions(+), 21 deletions(-)
>
> Reordering the entries made this very unpleasant to review.
Agreed. I don't know how to make it better though. Is there a way to
convince git to not try quite as hard to reuse existing lines? The
alternative would be to have two patches, one that drops the Fedora
38 job and one that adds the Fedora 39 one.
The job seems to have failed:
https://gitlab.com/libvirt/libvirt/-/jobs/5915956009
It looks like qemu and everything built properly but the failure is
rather opaque:
/tmp/script.: line 188: ci/jobs.sh: No such file or directory
4:13 a.m.
New subject: [PATCH 2/3] ci: Move upstream-qemu job to Fedora 39
On Fri, Jan 12, 2024 at 09:30:12AM +0100, Peter Krempa wrote:
On Tue, Jan 09, 2024 at 00:44:45 -0800, Andrea Bolognani wrote:
> On Mon, Jan 08, 2024 at 09:11:12PM +0100, Peter Krempa wrote:
> > On Mon, Jan 08, 2024 at 11:43:23 +0100, Andrea Bolognani wrote:
> > > Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
> > > ---
> > > ci/integration.yml | 42 +++++++++++++++++++++---------------------
> > > 1 file changed, 21 insertions(+), 21 deletions(-)
The job seems to have failed:
https://gitlab.com/libvirt/libvirt/-/jobs/5915956009
It looks like qemu and everything built properly but the failure is
rather opaque:
/tmp/script.: line 188: ci/jobs.sh: No such file or directory
Completely untested fix:
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/JM...
--
Andrea Bolognani / Red Hat / Virtualization
4:43 a.m.
New subject: [PATCH 3/3] ci: Add notes for integration jobs
These are special in that, when a new target is introduced, some
preparation is needed before the changes can be merged. Since
this only happens every six months or so, it's unsurprising that
we keep messing it up and forgetting some steps. Having notes
right in the file will hopefully help going forward.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
ci/integration.yml | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/ci/integration.yml b/ci/integration.yml
index ff1bb5d81d..fec9ef9256 100644
--- a/ci/integration.yml
+++ b/ci/integration.yml
@@ -1,6 +1,10 @@
include:
- 'ci/integration-template.yml'
+# XXX The integration tests use artifacts produced by the libvirt-perl
+# and libvirt-python CI jobs, so the new target needs to be introduced
+# there before it can be used here. The VM template for the target
+# also needs to be created on the runner host.
.centos-stream-8-tests:
variables:
# needed by libvirt-gitlab-executor
@@ -41,6 +45,10 @@ centos-stream-8-tests-local-env:
artifacts: true
+# XXX The integration tests use artifacts produced by the libvirt-perl
+# and libvirt-python CI jobs, so the new target needs to be introduced
+# there before it can be used here. The VM template for the target
+# also needs to be created on the runner host.
.centos-stream-9-tests:
variables:
# needed by libvirt-gitlab-executor
@@ -81,6 +89,10 @@ centos-stream-9-tests-local-env:
artifacts: true
+# XXX The integration tests use artifacts produced by the libvirt-perl
+# and libvirt-python CI jobs, so the new target needs to be introduced
+# there before it can be used here. The VM template for the target
+# also needs to be created on the runner host.
.fedora-38-tests:
variables:
# needed by libvirt-gitlab-executor
@@ -121,6 +133,10 @@ fedora-38-tests-local-env:
artifacts: true
+# XXX The integration tests use artifacts produced by the libvirt-perl
+# and libvirt-python CI jobs, so the new target needs to be introduced
+# there before it can be used here. The VM template for the target
+# also needs to be created on the runner host.
.fedora-39-tests:
variables:
# needed by libvirt-gitlab-executor
@@ -161,6 +177,10 @@ fedora-39-tests-local-env:
artifacts: true
+# XXX The integration tests use artifacts produced by the libvirt-perl
+# and libvirt-python CI jobs, so the new target needs to be introduced
+# there before it can be used here. The VM template for the target
+# also needs to be created on the runner host.
.fedora-39-upstream-qemu-tests:
variables:
# needed by libvirt-gitlab-executor
--
2.43.0
4:54 a.m.
New subject: [PATCH 3/3] ci: Add notes for integration jobs
On Mon, Jan 08, 2024 at 11:43:24 +0100, Andrea Bolognani wrote:
These are special in that, when a new target is introduced, some
preparation is needed before the changes can be merged. Since
this only happens every six months or so, it's unsurprising that
we keep messing it up and forgetting some steps. Having notes
right in the file will hopefully help going forward.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
ci/integration.yml | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/ci/integration.yml b/ci/integration.yml
index ff1bb5d81d..fec9ef9256 100644
--- a/ci/integration.yml
+++ b/ci/integration.yml
@@ -1,6 +1,10 @@
include:
- 'ci/integration-template.yml'
+# XXX The integration tests use artifacts produced by the libvirt-perl
XXX? Either use 'Important:' 'Note:' or nothing altogether.
With that:
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
4:57 a.m.
New subject: [PATCH 3/3] ci: Add notes for integration jobs
On Mon, Jan 08, 2024 at 11:54:00AM +0100, Peter Krempa wrote:
On Mon, Jan 08, 2024 at 11:43:24 +0100, Andrea Bolognani wrote:
> These are special in that, when a new target is introduced, some
> preparation is needed before the changes can be merged. Since
> this only happens every six months or so, it's unsurprising that
> we keep messing it up and forgetting some steps. Having notes
> right in the file will hopefully help going forward.
>
> Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
> ---
> ci/integration.yml | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/ci/integration.yml b/ci/integration.yml
> index ff1bb5d81d..fec9ef9256 100644
> --- a/ci/integration.yml
> +++ b/ci/integration.yml
> @@ -1,6 +1,10 @@
> include:
> - 'ci/integration-template.yml'
>
> +# XXX The integration tests use artifacts produced by the libvirt-perl
XXX? Either use 'Important:' 'Note:' or nothing altogether.
That gets automatically highlighted by vim, which helps make it stand
out. 'NOTE' gets the same effect though. Would that work for you?
--
Andrea Bolognani / Red Hat / Virtualization
6:07 a.m.
New subject: [PATCH 3/3] ci: Add notes for integration jobs
On Mon, Jan 08, 2024 at 02:57:40AM -0800, Andrea Bolognani wrote:
On Mon, Jan 08, 2024 at 11:54:00AM +0100, Peter Krempa wrote:
> On Mon, Jan 08, 2024 at 11:43:24 +0100, Andrea Bolognani wrote:
> > These are special in that, when a new target is introduced, some
> > preparation is needed before the changes can be merged. Since
> > this only happens every six months or so, it's unsurprising that
> > we keep messing it up and forgetting some steps. Having notes
> > right in the file will hopefully help going forward.
> >
> > Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
> > ---
> > ci/integration.yml | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/ci/integration.yml b/ci/integration.yml
> > index ff1bb5d81d..fec9ef9256 100644
> > --- a/ci/integration.yml
> > +++ b/ci/integration.yml
> > @@ -1,6 +1,10 @@
> > include:
> > - 'ci/integration-template.yml'
> >
> > +# XXX The integration tests use artifacts produced by the libvirt-perl
>
> XXX? Either use 'Important:' 'Note:' or nothing altogether.
That gets automatically highlighted by vim, which helps make it stand
out. 'NOTE' gets the same effect though. Would that work for you?
"NOTE" is more appropriate, because IMHO "XXX" indicates something
that
is /already/ broken.
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 :|
7:01 a.m.
New subject: [PATCH 3/3] ci: Add notes for integration jobs
On Mon, Jan 08, 2024 at 12:07:16PM +0000, Daniel P. Berrangé wrote:
On Mon, Jan 08, 2024 at 02:57:40AM -0800, Andrea Bolognani wrote:
> On Mon, Jan 08, 2024 at 11:54:00AM +0100, Peter Krempa wrote:
> > On Mon, Jan 08, 2024 at 11:43:24 +0100, Andrea Bolognani wrote:
> > > +++ b/ci/integration.yml
> > > @@ -1,6 +1,10 @@
> > > include:
> > > - 'ci/integration-template.yml'
> > >
> > > +# XXX The integration tests use artifacts produced by the libvirt-perl
> >
> > XXX? Either use 'Important:' 'Note:' or nothing altogether.
>
> That gets automatically highlighted by vim, which helps make it stand
> out. 'NOTE' gets the same effect though. Would that work for you?
"NOTE" is more appropriate, because IMHO "XXX" indicates something
that
is /already/ broken.
That'd be "FIXME" IMO, I see "XXX" as a generic "you
attention needed
here" marker. But the interpretation is not entirely clear-cut, and
I'm perfectly happy with using "NOTE" :)
Can I get ACKs for the rest of the series too, please?
--
Andrea Bolognani / Red Hat / Virtualization
346
days inactive
350
days old
15 comments
3 participants
participants (3)
-
Andrea Bolognani -
Daniel P. Berrangé -
Peter Krempa