| Message ID | 20221019164246.5487-1-michal.orzel@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | automation: test.yaml: Introduce templates to reduce the overhead | expand |
H Michal, > -----Original Message----- > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of > Michal Orzel > Sent: Thursday, October 20, 2022 12:43 AM > To: xen-devel@lists.xenproject.org > Cc: Michal Orzel <michal.orzel@amd.com>; Doug Goldstein > <cardoe@cardoe.com>; Stefano Stabellini <sstabellini@kernel.org> > Subject: [PATCH] automation: test.yaml: Introduce templates to reduce the > overhead > > At the moment, we define lots of test jobs in test.yaml, that make use > of the same configuration sections like variables, tags, artifacts. > Introduce templates (hidden jobs whose names start with a dot) to > reduce the overhead and simplify the file (more than 100 lines saved). > This way, the actual jobs can only specify sections that are unique > to them. > > Most of the test jobs specify the same set of prerequisite jobs under needs > property with just one additional being unique to the job itself. Introduce > YAML anchors for that purpose, because when using extends, the needs > property > is not being merged (the parent property overwrites the child one). > > Signed-off-by: Michal Orzel <michal.orzel@amd.com> > --- > This patch is based on the CI next branch where we already have several > patches (already acked) to be merged into staging after the release: > https://gitlab.com/xen-project/people/sstabellini/xen/-/tree/next > > Tested pipeline: > https://gitlab.com/xen-project/people/morzel/xen-orzelmichal/- > /pipelines/671114820 Looks good to me. Reviewed-by: Jiamei.xie@arm.com
On Wed, 19 Oct 2022, Michal Orzel wrote: > At the moment, we define lots of test jobs in test.yaml, that make use > of the same configuration sections like variables, tags, artifacts. > Introduce templates (hidden jobs whose names start with a dot) to > reduce the overhead and simplify the file (more than 100 lines saved). > This way, the actual jobs can only specify sections that are unique > to them. > > Most of the test jobs specify the same set of prerequisite jobs under needs > property with just one additional being unique to the job itself. Introduce > YAML anchors for that purpose, because when using extends, the needs property > is not being merged (the parent property overwrites the child one). I like the patch. Replying here on top because the diff below is not very helpful. When you say that "extends" overwrites the properties, do you mean that "needs" in qemu-smoke-dom0-arm64-gcc overwrites "needs" in .qemu-arm64, when qemu-smoke-dom0-arm64-gcc includes .qemu-arm64? If there is no way to solve the overwrite problem then it is OK to use YAML achors but is it possible to define the anchors outside of .qemu-arm64/.qemu-arm32 ? It would make things a lot clearer in the code. Maybe under a top level "definitions" key? The point is that .qemu-arm64 and .qemu-arm32 should use the anchor rather than define the anchor. I wouldn't call it qemu-arm64-needs because it has things like alpine-3.12-arm64-rootfs-export and kernel-5.19-arm64-export that are not required by qemu-system-aarch64-6.0.0-arm64-export. If anything qemu-system-aarch64-6.0.0-arm64-export needs CONTAINER: debian:unstable-arm64v8. So I would call the anchor something like "arm64-test-needs". Same comment for the arm32 anchor. > Signed-off-by: Michal Orzel <michal.orzel@amd.com> > --- > This patch is based on the CI next branch where we already have several > patches (already acked) to be merged into staging after the release: > https://gitlab.com/xen-project/people/sstabellini/xen/-/tree/next > > Tested pipeline: > https://gitlab.com/xen-project/people/morzel/xen-orzelmichal/-/pipelines/671114820 > --- > automation/gitlab-ci/test.yaml | 266 ++++++++++----------------------- > 1 file changed, 80 insertions(+), 186 deletions(-) > > diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml > index 92e0a1f7c510..fc0884b12082 100644 > --- a/automation/gitlab-ci/test.yaml > +++ b/automation/gitlab-ci/test.yaml > @@ -7,32 +7,12 @@ > - /^coverity-tested\/.*/ > - /^stable-.*/ > > -# Test jobs > -build-each-commit-gcc: > - extends: .test-jobs-common > - variables: > - CONTAINER: debian:stretch > - XEN_TARGET_ARCH: x86_64 > - CC: gcc > - script: > - - BASE=${BASE_SHA:-${CI_COMMIT_BEFORE_SHA}} TIP=${TIP_SHA:-${CI_COMMIT_SHA}} ./automation/gitlab-ci/build-each-commit.sh 2>&1 | tee ../build-each-commit-gcc.log > - - mv ../build-each-commit-gcc.log . > - artifacts: > - paths: > - - '*.log' > - when: always > - needs: [] > - tags: > - - x86_64 > - > -qemu-smoke-dom0-arm64-gcc: > +.qemu-arm64: > extends: .test-jobs-common > variables: > CONTAINER: debian:unstable-arm64v8 > - script: > - - ./automation/scripts/qemu-smoke-dom0-arm64.sh 2>&1 | tee qemu-smoke-arm64.log > - needs: > - - alpine-3.12-gcc-arm64 > + LOGFILE: qemu-smoke-arm64.log > + needs: &qemu-arm64-needs > - alpine-3.12-arm64-rootfs-export > - kernel-5.19-arm64-export > - qemu-system-aarch64-6.0.0-arm64-export LOGFILE should be listed among the artifacts (and maybe we can remove *.log if it has become redundant?) > @@ -44,17 +24,13 @@ qemu-smoke-dom0-arm64-gcc: > tags: > - arm64 > > -qemu-smoke-dom0-arm64-gcc-debug: > +.qemu-arm32: > extends: .test-jobs-common > variables: > CONTAINER: debian:unstable-arm64v8 > - script: > - - ./automation/scripts/qemu-smoke-dom0-arm64.sh 2>&1 | tee qemu-smoke-arm64.log > - needs: > - - alpine-3.12-gcc-debug-arm64 > - - alpine-3.12-arm64-rootfs-export > - - kernel-5.19-arm64-export > - - qemu-system-aarch64-6.0.0-arm64-export > + LOGFILE: qemu-smoke-arm32.log > + needs: &qemu-arm32-needs > + - qemu-system-aarch64-6.0.0-arm32-export > artifacts: > paths: > - smoke.serial > @@ -63,16 +39,11 @@ qemu-smoke-dom0-arm64-gcc-debug: > tags: > - arm64 > > -qemu-alpine-x86_64-gcc: > +.qemu-x86-64: > extends: .test-jobs-common > variables: > CONTAINER: debian:stretch > - script: > - - ./automation/scripts/qemu-alpine-x86_64.sh 2>&1 | tee qemu-smoke-x86_64.log > - needs: > - - alpine-3.12-gcc > - - alpine-3.12-rootfs-export > - - kernel-5.10.74-export > + LOGFILE: qemu-smoke-x86-64.log > artifacts: > paths: > - smoke.serial > @@ -81,214 +52,137 @@ qemu-alpine-x86_64-gcc: > tags: > - x86_64 > > -qemu-smoke-dom0less-arm64-gcc: > +# Test jobs > +build-each-commit-gcc: > extends: .test-jobs-common > variables: > - CONTAINER: debian:unstable-arm64v8 > + CONTAINER: debian:stretch > + XEN_TARGET_ARCH: x86_64 > + CC: gcc > script: > - - ./automation/scripts/qemu-smoke-dom0less-arm64.sh 2>&1 | tee qemu-smoke-arm64.log > - needs: > - - alpine-3.12-gcc-arm64 > - - alpine-3.12-arm64-rootfs-export > - - kernel-5.19-arm64-export > - - qemu-system-aarch64-6.0.0-arm64-export > + - BASE=${BASE_SHA:-${CI_COMMIT_BEFORE_SHA}} TIP=${TIP_SHA:-${CI_COMMIT_SHA}} ./automation/gitlab-ci/build-each-commit.sh 2>&1 | tee ../build-each-commit-gcc.log > + - mv ../build-each-commit-gcc.log . > artifacts: > paths: > - - smoke.serial > - '*.log' > when: always > + needs: [] > tags: > - - arm64 > + - x86_64 > + > +qemu-smoke-dom0-arm64-gcc: > + extends: .qemu-arm64 > + script: > + - ./automation/scripts/qemu-smoke-dom0-arm64.sh 2>&1 | tee ${LOGFILE} > + needs: > + - *qemu-arm64-needs > + - alpine-3.12-gcc-arm64 > + > +qemu-smoke-dom0-arm64-gcc-debug: > + extends: .qemu-arm64 > + script: > + - ./automation/scripts/qemu-smoke-dom0-arm64.sh 2>&1 | tee ${LOGFILE} > + needs: > + - *qemu-arm64-needs > + - alpine-3.12-gcc-debug-arm64 > + > +qemu-smoke-dom0less-arm64-gcc: > + extends: .qemu-arm64 > + script: > + - ./automation/scripts/qemu-smoke-dom0less-arm64.sh 2>&1 | tee ${LOGFILE} > + needs: > + - *qemu-arm64-needs > + - alpine-3.12-gcc-arm64 > > qemu-smoke-dom0less-arm64-gcc-debug: > - extends: .test-jobs-common > - variables: > - CONTAINER: debian:unstable-arm64v8 > + extends: .qemu-arm64 > script: > - - ./automation/scripts/qemu-smoke-dom0less-arm64.sh 2>&1 | tee qemu-smoke-arm64.log > + - ./automation/scripts/qemu-smoke-dom0less-arm64.sh 2>&1 | tee ${LOGFILE} > needs: > + - *qemu-arm64-needs > - alpine-3.12-gcc-debug-arm64 > - - alpine-3.12-arm64-rootfs-export > - - kernel-5.19-arm64-export > - - qemu-system-aarch64-6.0.0-arm64-export > - artifacts: > - paths: > - - smoke.serial > - - '*.log' > - when: always > - tags: > - - arm64 > > qemu-smoke-dom0less-arm64-gcc-staticmem: > - extends: .test-jobs-common > - variables: > - CONTAINER: debian:unstable-arm64v8 > + extends: .qemu-arm64 > script: > - - ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-mem 2>&1 | tee qemu-smoke-arm64.log > + - ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-mem 2>&1 | tee ${LOGFILE} > needs: > + - *qemu-arm64-needs > - alpine-3.12-gcc-arm64-staticmem > - - alpine-3.12-arm64-rootfs-export > - - kernel-5.19-arm64-export > - - qemu-system-aarch64-6.0.0-arm64-export > - artifacts: > - paths: > - - smoke.serial > - - '*.log' > - when: always > - tags: > - - arm64 > > qemu-smoke-dom0less-arm64-gcc-debug-staticmem: > - extends: .test-jobs-common > - variables: > - CONTAINER: debian:unstable-arm64v8 > + extends: .qemu-arm64 > script: > - - ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-mem 2>&1 | tee qemu-smoke-arm64.log > + - ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-mem 2>&1 | tee ${LOGFILE} > needs: > + - *qemu-arm64-needs > - alpine-3.12-gcc-debug-arm64-staticmem > - - alpine-3.12-arm64-rootfs-export > - - kernel-5.19-arm64-export > - - qemu-system-aarch64-6.0.0-arm64-export > - artifacts: > - paths: > - - smoke.serial > - - '*.log' > - when: always > - tags: > - - arm64 > > qemu-smoke-dom0less-arm64-gcc-boot-cpupools: > - extends: .test-jobs-common > - variables: > - CONTAINER: debian:unstable-arm64v8 > + extends: .qemu-arm64 > script: > - - ./automation/scripts/qemu-smoke-dom0less-arm64.sh boot-cpupools 2>&1 | tee qemu-smoke-arm64.log > + - ./automation/scripts/qemu-smoke-dom0less-arm64.sh boot-cpupools 2>&1 | tee ${LOGFILE} > needs: > + - *qemu-arm64-needs > - alpine-3.12-gcc-arm64-boot-cpupools > - - alpine-3.12-arm64-rootfs-export > - - kernel-5.19-arm64-export > - - qemu-system-aarch64-6.0.0-arm64-export > - artifacts: > - paths: > - - smoke.serial > - - '*.log' > - when: always > - tags: > - - arm64 > > qemu-smoke-dom0less-arm64-gcc-debug-boot-cpupools: > - extends: .test-jobs-common > - variables: > - CONTAINER: debian:unstable-arm64v8 > + extends: .qemu-arm64 > script: > - - ./automation/scripts/qemu-smoke-dom0less-arm64.sh boot-cpupools 2>&1 | tee qemu-smoke-arm64.log > + - ./automation/scripts/qemu-smoke-dom0less-arm64.sh boot-cpupools 2>&1 | tee ${LOGFILE} > needs: > + - *qemu-arm64-needs > - alpine-3.12-gcc-debug-arm64-boot-cpupools > - - alpine-3.12-arm64-rootfs-export > - - kernel-5.19-arm64-export > - - qemu-system-aarch64-6.0.0-arm64-export > - artifacts: > - paths: > - - smoke.serial > - - '*.log' > - when: always > - tags: > - - arm64 > > qemu-smoke-dom0-arm32-gcc: > - extends: .test-jobs-common > - variables: > - CONTAINER: debian:unstable-arm64v8 > + extends: .qemu-arm32 > script: > - - ./automation/scripts/qemu-smoke-dom0-arm32.sh 2>&1 | tee qemu-smoke-arm32.log > + - ./automation/scripts/qemu-smoke-dom0-arm32.sh 2>&1 | tee ${LOGFILE} > needs: > + - *qemu-arm32-needs > - debian-unstable-gcc-arm32 > - - qemu-system-aarch64-6.0.0-arm32-export > - artifacts: > - paths: > - - smoke.serial > - - '*.log' > - when: always > - tags: > - - arm64 > > qemu-smoke-dom0-arm32-gcc-debug: > - extends: .test-jobs-common > - variables: > - CONTAINER: debian:unstable-arm64v8 > + extends: .qemu-arm32 > script: > - - ./automation/scripts/qemu-smoke-dom0-arm32.sh 2>&1 | tee qemu-smoke-arm32.log > + - ./automation/scripts/qemu-smoke-dom0-arm32.sh 2>&1 | tee ${LOGFILE} > needs: > + - *qemu-arm32-needs > - debian-unstable-gcc-arm32-debug > - - qemu-system-aarch64-6.0.0-arm32-export > - artifacts: > - paths: > - - smoke.serial > - - '*.log' > - when: always > - tags: > - - arm64 > + > +qemu-alpine-x86_64-gcc: > + extends: .qemu-x86-64 > + script: > + - ./automation/scripts/qemu-alpine-x86_64.sh 2>&1 | tee ${LOGFILE} > + needs: > + - alpine-3.12-gcc > + - alpine-3.12-rootfs-export > + - kernel-5.10.74-export > > qemu-smoke-x86-64-gcc: > - extends: .test-jobs-common > - variables: > - CONTAINER: debian:stretch > + extends: .qemu-x86-64 > script: > - - ./automation/scripts/qemu-smoke-x86-64.sh pv 2>&1 | tee qemu-smoke-x86-64.log > - artifacts: > - paths: > - - smoke.serial > - - '*.log' > - when: always > + - ./automation/scripts/qemu-smoke-x86-64.sh pv 2>&1 | tee ${LOGFILE} > needs: > - debian-stretch-gcc-debug > - tags: > - - x86_64 > > qemu-smoke-x86-64-clang: > - extends: .test-jobs-common > - variables: > - CONTAINER: debian:stretch > + extends: .qemu-x86-64 > script: > - - ./automation/scripts/qemu-smoke-x86-64.sh pv 2>&1 | tee qemu-smoke-x86-64.log > - artifacts: > - paths: > - - smoke.serial > - - '*.log' > - when: always > + - ./automation/scripts/qemu-smoke-x86-64.sh pv 2>&1 | tee ${LOGFILE} > needs: > - debian-unstable-clang-debug > - tags: > - - x86_64 > > qemu-smoke-x86-64-gcc-pvh: > - extends: .test-jobs-common > - variables: > - CONTAINER: debian:stretch > + extends: .qemu-x86-64 > script: > - - ./automation/scripts/qemu-smoke-x86-64.sh pvh 2>&1 | tee qemu-smoke-x86-64.log > - artifacts: > - paths: > - - smoke.serial > - - '*.log' > - when: always > + - ./automation/scripts/qemu-smoke-x86-64.sh pvh 2>&1 | tee ${LOGFILE} > needs: > - debian-stretch-gcc-debug > - tags: > - - x86_64 > > qemu-smoke-x86-64-clang-pvh: > - extends: .test-jobs-common > - variables: > - CONTAINER: debian:stretch > + extends: .qemu-x86-64 > script: > - - ./automation/scripts/qemu-smoke-x86-64.sh pvh 2>&1 | tee qemu-smoke-x86-64.log > - artifacts: > - paths: > - - smoke.serial > - - '*.log' > - when: always > + - ./automation/scripts/qemu-smoke-x86-64.sh pvh 2>&1 | tee ${LOGFILE} > needs: > - debian-unstable-clang-debug > - tags: > - - x86_64 > -- > 2.25.1 >
Hi Stefano, On 21/10/2022 23:42, Stefano Stabellini wrote: > > > On Wed, 19 Oct 2022, Michal Orzel wrote: >> At the moment, we define lots of test jobs in test.yaml, that make use >> of the same configuration sections like variables, tags, artifacts. >> Introduce templates (hidden jobs whose names start with a dot) to >> reduce the overhead and simplify the file (more than 100 lines saved). >> This way, the actual jobs can only specify sections that are unique >> to them. >> >> Most of the test jobs specify the same set of prerequisite jobs under needs >> property with just one additional being unique to the job itself. Introduce >> YAML anchors for that purpose, because when using extends, the needs property >> is not being merged (the parent property overwrites the child one). > > I like the patch. Replying here on top because the diff below is not > very helpful. > > When you say that "extends" overwrites the properties, do you mean that > "needs" in qemu-smoke-dom0-arm64-gcc overwrites "needs" in .qemu-arm64, > when qemu-smoke-dom0-arm64-gcc includes .qemu-arm64? Yes, exactly. The behavior depends on the property. For example, the variables section is merged but needs end up being overwritten. This is because the extends does not merge the keys (variables uses key: value, whereas needs does not). > > > If there is no way to solve the overwrite problem then it is OK to use > YAML achors but is it possible to define the anchors outside of > .qemu-arm64/.qemu-arm32 ? It would make things a lot clearer in the > code. Maybe under a top level "definitions" key? The point is that > .qemu-arm64 and .qemu-arm32 should use the anchor rather than define the > anchor. It is possible to define anchors outside qemu-arm64/arm32. I decided to define them in these jobs because for me it looked cleaner (less lines of code). But I'm ok to carve them out if that is what you prefer. This would require dropping the needs property from the extend jobs, as they cannot make use of the anchors (overwrite issue), and using the anchors from real jobs (just like I did in this patch). So we would have: .arm64-test-needs: &arm64-test-needs - alpine-3.12-arm64-rootfs-export - kernel-5.19-arm64-export - qemu-system-aarch64-6.0.0-arm64-export .qemu-arm64: extends: .test-jobs-common variables: CONTAINER: debian:unstable-arm64v8 LOGFILE: qemu-smoke-arm64.log artifacts: paths: - smoke.serial - '*.log' when: always tags: - arm64 qemu-smoke-dom0-arm64-gcc: extends: .qemu-arm64 script: - ./automation/scripts/qemu-smoke-dom0-arm64.sh 2>&1 | tee ${LOGFILE} needs: - *arm64-test-needs - alpine-3.12-gcc-arm64 > > I wouldn't call it qemu-arm64-needs because it has things > like alpine-3.12-arm64-rootfs-export and kernel-5.19-arm64-export that > are not required by qemu-system-aarch64-6.0.0-arm64-export. If anything > qemu-system-aarch64-6.0.0-arm64-export needs CONTAINER: > debian:unstable-arm64v8. > > So I would call the anchor something like "arm64-test-needs". Same > comment for the arm32 anchor. Ok, this naming sounds good to me. > > >> Signed-off-by: Michal Orzel <michal.orzel@amd.com> >> --- >> This patch is based on the CI next branch where we already have several >> patches (already acked) to be merged into staging after the release: >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.com%2Fxen-project%2Fpeople%2Fsstabellini%2Fxen%2F-%2Ftree%2Fnext&data=05%7C01%7Cmichal.orzel%40amd.com%7Ca83af11b062b431b4f0908dab3ad2162%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638019853419768862%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=TZxie442G%2Bm6SP%2FemyPuv8dwCDXAv1Wxwe22yGQZaB4%3D&reserved=0 >> >> Tested pipeline: >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.com%2Fxen-project%2Fpeople%2Fmorzel%2Fxen-orzelmichal%2F-%2Fpipelines%2F671114820&data=05%7C01%7Cmichal.orzel%40amd.com%7Ca83af11b062b431b4f0908dab3ad2162%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638019853419768862%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=tMwGAZUKyvDp%2BxmVdxUD1kg3uMagWdO2P1DjF5O3b2M%3D&reserved=0 >> --- >> automation/gitlab-ci/test.yaml | 266 ++++++++++----------------------- >> 1 file changed, 80 insertions(+), 186 deletions(-) >> >> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml >> index 92e0a1f7c510..fc0884b12082 100644 >> --- a/automation/gitlab-ci/test.yaml >> +++ b/automation/gitlab-ci/test.yaml >> @@ -7,32 +7,12 @@ >> - /^coverity-tested\/.*/ >> - /^stable-.*/ >> >> -# Test jobs >> -build-each-commit-gcc: >> - extends: .test-jobs-common >> - variables: >> - CONTAINER: debian:stretch >> - XEN_TARGET_ARCH: x86_64 >> - CC: gcc >> - script: >> - - BASE=${BASE_SHA:-${CI_COMMIT_BEFORE_SHA}} TIP=${TIP_SHA:-${CI_COMMIT_SHA}} ./automation/gitlab-ci/build-each-commit.sh 2>&1 | tee ../build-each-commit-gcc.log >> - - mv ../build-each-commit-gcc.log . >> - artifacts: >> - paths: >> - - '*.log' >> - when: always >> - needs: [] >> - tags: >> - - x86_64 >> - >> -qemu-smoke-dom0-arm64-gcc: >> +.qemu-arm64: >> extends: .test-jobs-common >> variables: >> CONTAINER: debian:unstable-arm64v8 >> - script: >> - - ./automation/scripts/qemu-smoke-dom0-arm64.sh 2>&1 | tee qemu-smoke-arm64.log >> - needs: >> - - alpine-3.12-gcc-arm64 >> + LOGFILE: qemu-smoke-arm64.log >> + needs: &qemu-arm64-needs >> - alpine-3.12-arm64-rootfs-export >> - kernel-5.19-arm64-export >> - qemu-system-aarch64-6.0.0-arm64-export > > LOGFILE should be listed among the artifacts (and maybe we can remove > *.log if it has become redundant?) *.log is not redundant because we have 4 logs to be stored, e.g.: - smoke.serial - config.log - build.log - qemu-smoke-arm64.log aka LOGFILE So we can either have this: artifacts: paths: - smoke.serial - '*.log' or this: artifacts: paths: - smoke.serial - ${LOGFILE} - '*.log' I would prefer the former (just as it is now) but if you prefer the latter, it is ok. ~Michal
On Mon, 24 Oct 2022, Michal Orzel wrote: > Hi Stefano, > > On 21/10/2022 23:42, Stefano Stabellini wrote: > > > > > > On Wed, 19 Oct 2022, Michal Orzel wrote: > >> At the moment, we define lots of test jobs in test.yaml, that make use > >> of the same configuration sections like variables, tags, artifacts. > >> Introduce templates (hidden jobs whose names start with a dot) to > >> reduce the overhead and simplify the file (more than 100 lines saved). > >> This way, the actual jobs can only specify sections that are unique > >> to them. > >> > >> Most of the test jobs specify the same set of prerequisite jobs under needs > >> property with just one additional being unique to the job itself. Introduce > >> YAML anchors for that purpose, because when using extends, the needs property > >> is not being merged (the parent property overwrites the child one). > > > > I like the patch. Replying here on top because the diff below is not > > very helpful. > > > > When you say that "extends" overwrites the properties, do you mean that > > "needs" in qemu-smoke-dom0-arm64-gcc overwrites "needs" in .qemu-arm64, > > when qemu-smoke-dom0-arm64-gcc includes .qemu-arm64? > Yes, exactly. The behavior depends on the property. For example, the variables > section is merged but needs end up being overwritten. This is because the extends > does not merge the keys (variables uses key: value, whereas needs does not). > > > > > > > If there is no way to solve the overwrite problem then it is OK to use > > YAML achors but is it possible to define the anchors outside of > > .qemu-arm64/.qemu-arm32 ? It would make things a lot clearer in the > > code. Maybe under a top level "definitions" key? The point is that > > .qemu-arm64 and .qemu-arm32 should use the anchor rather than define the > > anchor. > It is possible to define anchors outside qemu-arm64/arm32. I decided to > define them in these jobs because for me it looked cleaner (less lines of code). > But I'm ok to carve them out if that is what you prefer. This would > require dropping the needs property from the extend jobs, as they cannot make use > of the anchors (overwrite issue), and using the anchors from real jobs (just like I did in this patch). Yes that makes sense > So we would have: > > .arm64-test-needs: &arm64-test-needs > - alpine-3.12-arm64-rootfs-export > - kernel-5.19-arm64-export > - qemu-system-aarch64-6.0.0-arm64-export > > .qemu-arm64: > extends: .test-jobs-common > variables: > CONTAINER: debian:unstable-arm64v8 > LOGFILE: qemu-smoke-arm64.log > artifacts: > paths: > - smoke.serial > - '*.log' > when: always > tags: > - arm64 > > qemu-smoke-dom0-arm64-gcc: > extends: .qemu-arm64 > script: > - ./automation/scripts/qemu-smoke-dom0-arm64.sh 2>&1 | tee ${LOGFILE} > needs: > - *arm64-test-needs > - alpine-3.12-gcc-arm64 This looks better, thanks! > > > > I wouldn't call it qemu-arm64-needs because it has things > > like alpine-3.12-arm64-rootfs-export and kernel-5.19-arm64-export that > > are not required by qemu-system-aarch64-6.0.0-arm64-export. If anything > > qemu-system-aarch64-6.0.0-arm64-export needs CONTAINER: > > debian:unstable-arm64v8. > > > > So I would call the anchor something like "arm64-test-needs". Same > > comment for the arm32 anchor. > Ok, this naming sounds good to me. > > > > > > >> Signed-off-by: Michal Orzel <michal.orzel@amd.com> > >> --- > >> This patch is based on the CI next branch where we already have several > >> patches (already acked) to be merged into staging after the release: > >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.com%2Fxen-project%2Fpeople%2Fsstabellini%2Fxen%2F-%2Ftree%2Fnext&data=05%7C01%7Cmichal.orzel%40amd.com%7Ca83af11b062b431b4f0908dab3ad2162%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638019853419768862%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=TZxie442G%2Bm6SP%2FemyPuv8dwCDXAv1Wxwe22yGQZaB4%3D&reserved=0 > >> > >> Tested pipeline: > >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.com%2Fxen-project%2Fpeople%2Fmorzel%2Fxen-orzelmichal%2F-%2Fpipelines%2F671114820&data=05%7C01%7Cmichal.orzel%40amd.com%7Ca83af11b062b431b4f0908dab3ad2162%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638019853419768862%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=tMwGAZUKyvDp%2BxmVdxUD1kg3uMagWdO2P1DjF5O3b2M%3D&reserved=0 > >> --- > >> automation/gitlab-ci/test.yaml | 266 ++++++++++----------------------- > >> 1 file changed, 80 insertions(+), 186 deletions(-) > >> > >> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml > >> index 92e0a1f7c510..fc0884b12082 100644 > >> --- a/automation/gitlab-ci/test.yaml > >> +++ b/automation/gitlab-ci/test.yaml > >> @@ -7,32 +7,12 @@ > >> - /^coverity-tested\/.*/ > >> - /^stable-.*/ > >> > >> -# Test jobs > >> -build-each-commit-gcc: > >> - extends: .test-jobs-common > >> - variables: > >> - CONTAINER: debian:stretch > >> - XEN_TARGET_ARCH: x86_64 > >> - CC: gcc > >> - script: > >> - - BASE=${BASE_SHA:-${CI_COMMIT_BEFORE_SHA}} TIP=${TIP_SHA:-${CI_COMMIT_SHA}} ./automation/gitlab-ci/build-each-commit.sh 2>&1 | tee ../build-each-commit-gcc.log > >> - - mv ../build-each-commit-gcc.log . > >> - artifacts: > >> - paths: > >> - - '*.log' > >> - when: always > >> - needs: [] > >> - tags: > >> - - x86_64 > >> - > >> -qemu-smoke-dom0-arm64-gcc: > >> +.qemu-arm64: > >> extends: .test-jobs-common > >> variables: > >> CONTAINER: debian:unstable-arm64v8 > >> - script: > >> - - ./automation/scripts/qemu-smoke-dom0-arm64.sh 2>&1 | tee qemu-smoke-arm64.log > >> - needs: > >> - - alpine-3.12-gcc-arm64 > >> + LOGFILE: qemu-smoke-arm64.log > >> + needs: &qemu-arm64-needs > >> - alpine-3.12-arm64-rootfs-export > >> - kernel-5.19-arm64-export > >> - qemu-system-aarch64-6.0.0-arm64-export > > > > LOGFILE should be listed among the artifacts (and maybe we can remove > > *.log if it has become redundant?) > *.log is not redundant because we have 4 logs to be stored, e.g.: > - smoke.serial > - config.log > - build.log > - qemu-smoke-arm64.log aka LOGFILE > > So we can either have this: > artifacts: > paths: > - smoke.serial > - '*.log' > > or this: > artifacts: > paths: > - smoke.serial > - ${LOGFILE} > - '*.log' > > I would prefer the former (just as it is now) but if you prefer the latter, it is ok. I agree with the former
diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml index 92e0a1f7c510..fc0884b12082 100644 --- a/automation/gitlab-ci/test.yaml +++ b/automation/gitlab-ci/test.yaml @@ -7,32 +7,12 @@ - /^coverity-tested\/.*/ - /^stable-.*/ -# Test jobs -build-each-commit-gcc: - extends: .test-jobs-common - variables: - CONTAINER: debian:stretch - XEN_TARGET_ARCH: x86_64 - CC: gcc - script: - - BASE=${BASE_SHA:-${CI_COMMIT_BEFORE_SHA}} TIP=${TIP_SHA:-${CI_COMMIT_SHA}} ./automation/gitlab-ci/build-each-commit.sh 2>&1 | tee ../build-each-commit-gcc.log - - mv ../build-each-commit-gcc.log . - artifacts: - paths: - - '*.log' - when: always - needs: [] - tags: - - x86_64 - -qemu-smoke-dom0-arm64-gcc: +.qemu-arm64: extends: .test-jobs-common variables: CONTAINER: debian:unstable-arm64v8 - script: - - ./automation/scripts/qemu-smoke-dom0-arm64.sh 2>&1 | tee qemu-smoke-arm64.log - needs: - - alpine-3.12-gcc-arm64 + LOGFILE: qemu-smoke-arm64.log + needs: &qemu-arm64-needs - alpine-3.12-arm64-rootfs-export - kernel-5.19-arm64-export - qemu-system-aarch64-6.0.0-arm64-export @@ -44,17 +24,13 @@ qemu-smoke-dom0-arm64-gcc: tags: - arm64 -qemu-smoke-dom0-arm64-gcc-debug: +.qemu-arm32: extends: .test-jobs-common variables: CONTAINER: debian:unstable-arm64v8 - script: - - ./automation/scripts/qemu-smoke-dom0-arm64.sh 2>&1 | tee qemu-smoke-arm64.log - needs: - - alpine-3.12-gcc-debug-arm64 - - alpine-3.12-arm64-rootfs-export - - kernel-5.19-arm64-export - - qemu-system-aarch64-6.0.0-arm64-export + LOGFILE: qemu-smoke-arm32.log + needs: &qemu-arm32-needs + - qemu-system-aarch64-6.0.0-arm32-export artifacts: paths: - smoke.serial @@ -63,16 +39,11 @@ qemu-smoke-dom0-arm64-gcc-debug: tags: - arm64 -qemu-alpine-x86_64-gcc: +.qemu-x86-64: extends: .test-jobs-common variables: CONTAINER: debian:stretch - script: - - ./automation/scripts/qemu-alpine-x86_64.sh 2>&1 | tee qemu-smoke-x86_64.log - needs: - - alpine-3.12-gcc - - alpine-3.12-rootfs-export - - kernel-5.10.74-export + LOGFILE: qemu-smoke-x86-64.log artifacts: paths: - smoke.serial @@ -81,214 +52,137 @@ qemu-alpine-x86_64-gcc: tags: - x86_64 -qemu-smoke-dom0less-arm64-gcc: +# Test jobs +build-each-commit-gcc: extends: .test-jobs-common variables: - CONTAINER: debian:unstable-arm64v8 + CONTAINER: debian:stretch + XEN_TARGET_ARCH: x86_64 + CC: gcc script: - - ./automation/scripts/qemu-smoke-dom0less-arm64.sh 2>&1 | tee qemu-smoke-arm64.log - needs: - - alpine-3.12-gcc-arm64 - - alpine-3.12-arm64-rootfs-export - - kernel-5.19-arm64-export - - qemu-system-aarch64-6.0.0-arm64-export + - BASE=${BASE_SHA:-${CI_COMMIT_BEFORE_SHA}} TIP=${TIP_SHA:-${CI_COMMIT_SHA}} ./automation/gitlab-ci/build-each-commit.sh 2>&1 | tee ../build-each-commit-gcc.log + - mv ../build-each-commit-gcc.log . artifacts: paths: - - smoke.serial - '*.log' when: always + needs: [] tags: - - arm64 + - x86_64 + +qemu-smoke-dom0-arm64-gcc: + extends: .qemu-arm64 + script: + - ./automation/scripts/qemu-smoke-dom0-arm64.sh 2>&1 | tee ${LOGFILE} + needs: + - *qemu-arm64-needs + - alpine-3.12-gcc-arm64 + +qemu-smoke-dom0-arm64-gcc-debug: + extends: .qemu-arm64 + script: + - ./automation/scripts/qemu-smoke-dom0-arm64.sh 2>&1 | tee ${LOGFILE} + needs: + - *qemu-arm64-needs + - alpine-3.12-gcc-debug-arm64 + +qemu-smoke-dom0less-arm64-gcc: + extends: .qemu-arm64 + script: + - ./automation/scripts/qemu-smoke-dom0less-arm64.sh 2>&1 | tee ${LOGFILE} + needs: + - *qemu-arm64-needs + - alpine-3.12-gcc-arm64 qemu-smoke-dom0less-arm64-gcc-debug: - extends: .test-jobs-common - variables: - CONTAINER: debian:unstable-arm64v8 + extends: .qemu-arm64 script: - - ./automation/scripts/qemu-smoke-dom0less-arm64.sh 2>&1 | tee qemu-smoke-arm64.log + - ./automation/scripts/qemu-smoke-dom0less-arm64.sh 2>&1 | tee ${LOGFILE} needs: + - *qemu-arm64-needs - alpine-3.12-gcc-debug-arm64 - - alpine-3.12-arm64-rootfs-export - - kernel-5.19-arm64-export - - qemu-system-aarch64-6.0.0-arm64-export - artifacts: - paths: - - smoke.serial - - '*.log' - when: always - tags: - - arm64 qemu-smoke-dom0less-arm64-gcc-staticmem: - extends: .test-jobs-common - variables: - CONTAINER: debian:unstable-arm64v8 + extends: .qemu-arm64 script: - - ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-mem 2>&1 | tee qemu-smoke-arm64.log + - ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-mem 2>&1 | tee ${LOGFILE} needs: + - *qemu-arm64-needs - alpine-3.12-gcc-arm64-staticmem - - alpine-3.12-arm64-rootfs-export - - kernel-5.19-arm64-export - - qemu-system-aarch64-6.0.0-arm64-export - artifacts: - paths: - - smoke.serial - - '*.log' - when: always - tags: - - arm64 qemu-smoke-dom0less-arm64-gcc-debug-staticmem: - extends: .test-jobs-common - variables: - CONTAINER: debian:unstable-arm64v8 + extends: .qemu-arm64 script: - - ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-mem 2>&1 | tee qemu-smoke-arm64.log + - ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-mem 2>&1 | tee ${LOGFILE} needs: + - *qemu-arm64-needs - alpine-3.12-gcc-debug-arm64-staticmem - - alpine-3.12-arm64-rootfs-export - - kernel-5.19-arm64-export - - qemu-system-aarch64-6.0.0-arm64-export - artifacts: - paths: - - smoke.serial - - '*.log' - when: always - tags: - - arm64 qemu-smoke-dom0less-arm64-gcc-boot-cpupools: - extends: .test-jobs-common - variables: - CONTAINER: debian:unstable-arm64v8 + extends: .qemu-arm64 script: - - ./automation/scripts/qemu-smoke-dom0less-arm64.sh boot-cpupools 2>&1 | tee qemu-smoke-arm64.log + - ./automation/scripts/qemu-smoke-dom0less-arm64.sh boot-cpupools 2>&1 | tee ${LOGFILE} needs: + - *qemu-arm64-needs - alpine-3.12-gcc-arm64-boot-cpupools - - alpine-3.12-arm64-rootfs-export - - kernel-5.19-arm64-export - - qemu-system-aarch64-6.0.0-arm64-export - artifacts: - paths: - - smoke.serial - - '*.log' - when: always - tags: - - arm64 qemu-smoke-dom0less-arm64-gcc-debug-boot-cpupools: - extends: .test-jobs-common - variables: - CONTAINER: debian:unstable-arm64v8 + extends: .qemu-arm64 script: - - ./automation/scripts/qemu-smoke-dom0less-arm64.sh boot-cpupools 2>&1 | tee qemu-smoke-arm64.log + - ./automation/scripts/qemu-smoke-dom0less-arm64.sh boot-cpupools 2>&1 | tee ${LOGFILE} needs: + - *qemu-arm64-needs - alpine-3.12-gcc-debug-arm64-boot-cpupools - - alpine-3.12-arm64-rootfs-export - - kernel-5.19-arm64-export - - qemu-system-aarch64-6.0.0-arm64-export - artifacts: - paths: - - smoke.serial - - '*.log' - when: always - tags: - - arm64 qemu-smoke-dom0-arm32-gcc: - extends: .test-jobs-common - variables: - CONTAINER: debian:unstable-arm64v8 + extends: .qemu-arm32 script: - - ./automation/scripts/qemu-smoke-dom0-arm32.sh 2>&1 | tee qemu-smoke-arm32.log + - ./automation/scripts/qemu-smoke-dom0-arm32.sh 2>&1 | tee ${LOGFILE} needs: + - *qemu-arm32-needs - debian-unstable-gcc-arm32 - - qemu-system-aarch64-6.0.0-arm32-export - artifacts: - paths: - - smoke.serial - - '*.log' - when: always - tags: - - arm64 qemu-smoke-dom0-arm32-gcc-debug: - extends: .test-jobs-common - variables: - CONTAINER: debian:unstable-arm64v8 + extends: .qemu-arm32 script: - - ./automation/scripts/qemu-smoke-dom0-arm32.sh 2>&1 | tee qemu-smoke-arm32.log + - ./automation/scripts/qemu-smoke-dom0-arm32.sh 2>&1 | tee ${LOGFILE} needs: + - *qemu-arm32-needs - debian-unstable-gcc-arm32-debug - - qemu-system-aarch64-6.0.0-arm32-export - artifacts: - paths: - - smoke.serial - - '*.log' - when: always - tags: - - arm64 + +qemu-alpine-x86_64-gcc: + extends: .qemu-x86-64 + script: + - ./automation/scripts/qemu-alpine-x86_64.sh 2>&1 | tee ${LOGFILE} + needs: + - alpine-3.12-gcc + - alpine-3.12-rootfs-export + - kernel-5.10.74-export qemu-smoke-x86-64-gcc: - extends: .test-jobs-common - variables: - CONTAINER: debian:stretch + extends: .qemu-x86-64 script: - - ./automation/scripts/qemu-smoke-x86-64.sh pv 2>&1 | tee qemu-smoke-x86-64.log - artifacts: - paths: - - smoke.serial - - '*.log' - when: always + - ./automation/scripts/qemu-smoke-x86-64.sh pv 2>&1 | tee ${LOGFILE} needs: - debian-stretch-gcc-debug - tags: - - x86_64 qemu-smoke-x86-64-clang: - extends: .test-jobs-common - variables: - CONTAINER: debian:stretch + extends: .qemu-x86-64 script: - - ./automation/scripts/qemu-smoke-x86-64.sh pv 2>&1 | tee qemu-smoke-x86-64.log - artifacts: - paths: - - smoke.serial - - '*.log' - when: always + - ./automation/scripts/qemu-smoke-x86-64.sh pv 2>&1 | tee ${LOGFILE} needs: - debian-unstable-clang-debug - tags: - - x86_64 qemu-smoke-x86-64-gcc-pvh: - extends: .test-jobs-common - variables: - CONTAINER: debian:stretch + extends: .qemu-x86-64 script: - - ./automation/scripts/qemu-smoke-x86-64.sh pvh 2>&1 | tee qemu-smoke-x86-64.log - artifacts: - paths: - - smoke.serial - - '*.log' - when: always + - ./automation/scripts/qemu-smoke-x86-64.sh pvh 2>&1 | tee ${LOGFILE} needs: - debian-stretch-gcc-debug - tags: - - x86_64 qemu-smoke-x86-64-clang-pvh: - extends: .test-jobs-common - variables: - CONTAINER: debian:stretch + extends: .qemu-x86-64 script: - - ./automation/scripts/qemu-smoke-x86-64.sh pvh 2>&1 | tee qemu-smoke-x86-64.log - artifacts: - paths: - - smoke.serial - - '*.log' - when: always + - ./automation/scripts/qemu-smoke-x86-64.sh pvh 2>&1 | tee ${LOGFILE} needs: - debian-unstable-clang-debug - tags: - - x86_64
At the moment, we define lots of test jobs in test.yaml, that make use of the same configuration sections like variables, tags, artifacts. Introduce templates (hidden jobs whose names start with a dot) to reduce the overhead and simplify the file (more than 100 lines saved). This way, the actual jobs can only specify sections that are unique to them. Most of the test jobs specify the same set of prerequisite jobs under needs property with just one additional being unique to the job itself. Introduce YAML anchors for that purpose, because when using extends, the needs property is not being merged (the parent property overwrites the child one). Signed-off-by: Michal Orzel <michal.orzel@amd.com> --- This patch is based on the CI next branch where we already have several patches (already acked) to be merged into staging after the release: https://gitlab.com/xen-project/people/sstabellini/xen/-/tree/next Tested pipeline: https://gitlab.com/xen-project/people/morzel/xen-orzelmichal/-/pipelines/671114820 --- automation/gitlab-ci/test.yaml | 266 ++++++++++----------------------- 1 file changed, 80 insertions(+), 186 deletions(-)