[PATCH 0/3] ci: Fixes for integration jobs
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
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@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
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@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@redhat.com>
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@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
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@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
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@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 :|
Signed-off-by: Andrea Bolognani <abologna@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
On Mon, Jan 08, 2024 at 11:43:23 +0100, Andrea Bolognani wrote:
Signed-off-by: Andrea Bolognani <abologna@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@redhat.com>
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@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
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@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
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@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/JMSSR... -- Andrea Bolognani / Red Hat / Virtualization
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@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
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@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@redhat.com>
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@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
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@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 :|
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
participants (3)
-
Andrea Bolognani -
Daniel P. Berrangé -
Peter Krempa