| Message ID | 20210414221457.1653745-3-crosa@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Acceptance Tests: support choosing specific distro and version | expand |
Hi, On 4/14/21 7:14 PM, Cleber Rosa wrote: > Instead of having, by default, the checksum in the tests, and the > definition of tests in the framework, let's keep them together. > > A central definition for distributions is available, and it should > allow other known distros to be added more easily. > > No behavior change is expected here, and tests can still define > a distro_checksum value if for some reason they want to override > the known distribution information. > > Signed-off-by: Cleber Rosa <crosa@redhat.com> > --- > tests/acceptance/avocado_qemu/__init__.py | 34 +++++++++++++++++++++-- > tests/acceptance/boot_linux.py | 8 ------ > 2 files changed, 32 insertions(+), 10 deletions(-) > > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py > index aae1e5bbc9..97093614d9 100644 > --- a/tests/acceptance/avocado_qemu/__init__.py > +++ b/tests/acceptance/avocado_qemu/__init__.py > @@ -299,6 +299,30 @@ def ssh_command(self, command): > return stdout_lines, stderr_lines > > > +#: A collection of known distros and their respective image checksum > +KNOWN_DISTROS = { Do you plan to expand that mapping to record values other than checksums? Otherwise it could be named KNOWN_DISTROS_CHECKSUMS. > + 'fedora': { > + '31': { > + 'x86_64': > + {'checksum': 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'}, > + 'aarch64': > + {'checksum': '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'}, > + 'ppc64': > + {'checksum': '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'}, > + 's390x': > + {'checksum': '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'}, > + } > + } > + } > + > + > +def get_known_distro_checksum(distro, distro_version, arch): > + try: > + return KNOWN_DISTROS.get(distro).get(distro_version).get(arch).get('checksum') > + except AttributeError: > + return None > + > + Currently we have a few loose methods on avocado_qemu/__init__.py, and I'm about to send a series to wrap them in a mixin class. This series will introduce more loose code on the file; so would you consider moving KNOWN_DISTROS and get_known_distro_checksum() to the LinuxTest class, and possibly making the latest a class method? > class LinuxTest(Test, LinuxSSHMixIn): > """Facilitates having a cloud-image Linux based available. > > @@ -348,14 +372,20 @@ def download_boot(self): > vmimage.QEMU_IMG = qemu_img > > self.log.info('Downloading/preparing boot image') > + distro = 'fedora' > + distro_version = '31' > + known_distro_checksum = get_known_distro_checksum(distro, > + distro_version, > + self.arch) > + distro_checksum = self.distro_checksum or known_distro_checksum distro_checksum may be None. In this case vmimage.get() will silently skip the check? I suggest to log a warn message. > # Fedora 31 only provides ppc64le images > image_arch = self.arch > if image_arch == 'ppc64': > image_arch = 'ppc64le' > try: > boot = vmimage.get( > - 'fedora', arch=image_arch, version='31', > - checksum=self.distro_checksum, > + distro, arch=image_arch, version=distro_version, > + checksum=distro_checksum, > algorithm='sha256', > cache_dir=self.cache_dirs[0], > snapshot_dir=self.workdir) > diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py > index c7bc3a589e..9e618c6daa 100644 > --- a/tests/acceptance/boot_linux.py > +++ b/tests/acceptance/boot_linux.py > @@ -20,8 +20,6 @@ class BootLinuxX8664(LinuxTest): > :avocado: tags=arch:x86_64 > """ > > - distro_checksum = 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0' > - > def test_pc_i440fx_tcg(self): > """ > :avocado: tags=machine:pc > @@ -66,8 +64,6 @@ class BootLinuxAarch64(LinuxTest): > :avocado: tags=machine:gic-version=2 > """ > > - distro_checksum = '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49' > - > def add_common_args(self): > self.vm.add_args('-bios', > os.path.join(BUILD_DIR, 'pc-bios', > @@ -119,8 +115,6 @@ class BootLinuxPPC64(LinuxTest): > :avocado: tags=arch:ppc64 > """ > > - distro_checksum = '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58' > - > def test_pseries_tcg(self): > """ > :avocado: tags=machine:pseries > @@ -136,8 +130,6 @@ class BootLinuxS390X(LinuxTest): > :avocado: tags=arch:s390x > """ > > - distro_checksum = '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d' > - > @skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab') > def test_s390_ccw_virtio_tcg(self): > """
On Mon, Apr 19, 2021 at 12:25:44PM -0300, Wainer dos Santos Moschetta wrote: > Hi, > > On 4/14/21 7:14 PM, Cleber Rosa wrote: > > Instead of having, by default, the checksum in the tests, and the > > definition of tests in the framework, let's keep them together. > > > > A central definition for distributions is available, and it should > > allow other known distros to be added more easily. > > > > No behavior change is expected here, and tests can still define > > a distro_checksum value if for some reason they want to override > > the known distribution information. > > > > Signed-off-by: Cleber Rosa <crosa@redhat.com> > > --- > > tests/acceptance/avocado_qemu/__init__.py | 34 +++++++++++++++++++++-- > > tests/acceptance/boot_linux.py | 8 ------ > > 2 files changed, 32 insertions(+), 10 deletions(-) > > > > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py > > index aae1e5bbc9..97093614d9 100644 > > --- a/tests/acceptance/avocado_qemu/__init__.py > > +++ b/tests/acceptance/avocado_qemu/__init__.py > > @@ -299,6 +299,30 @@ def ssh_command(self, command): > > return stdout_lines, stderr_lines > > +#: A collection of known distros and their respective image checksum > > +KNOWN_DISTROS = { > > Do you plan to expand that mapping to record values other than checksums? > Otherwise it could be named KNOWN_DISTROS_CHECKSUMS. > Let's just say I had an intuition about it being used for other purposes. Talking to Eric Auger earlier this morning, he will expand this mapping with default kernel args distros, so that he can *add* to the common args. > > + 'fedora': { > > + '31': { > > + 'x86_64': > > + {'checksum': 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'}, > > + 'aarch64': > > + {'checksum': '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'}, > > + 'ppc64': > > + {'checksum': '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'}, > > + 's390x': > > + {'checksum': '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'}, > > + } > > + } > > + } > > + > > + > > +def get_known_distro_checksum(distro, distro_version, arch): > > + try: > > + return KNOWN_DISTROS.get(distro).get(distro_version).get(arch).get('checksum') > > + except AttributeError: > > + return None > > + > > + > > Currently we have a few loose methods on avocado_qemu/__init__.py, and I'm > about to send a series to wrap them in a mixin class. This series will > introduce more loose code on the file; so would you consider moving > KNOWN_DISTROS and get_known_distro_checksum() to the LinuxTest class, and > possibly making the latest a class method? > Some of our experience in "avocado.Test" revealed that users would: 1) find it confusing to have so many methods in the class that are not useful to them 2) would conflict with variables/attributes of their own About #2, we end up turning a lot of variables atttributes into properties so that errors would be explicit when users tried to overwrite them unknowingly. But, in the specific example of KNOWN_DISTROS and its expansion I mentioned before, it may indeed make sense to have a Test or LinuxTest method that test writers can use. It'd probably need to be a bit more generic and evolved than this current version though. Maybe wait for Eric's input based on real world use case here? > > class LinuxTest(Test, LinuxSSHMixIn): > > """Facilitates having a cloud-image Linux based available. > > @@ -348,14 +372,20 @@ def download_boot(self): > > vmimage.QEMU_IMG = qemu_img > > self.log.info('Downloading/preparing boot image') > > + distro = 'fedora' > > + distro_version = '31' > > + known_distro_checksum = get_known_distro_checksum(distro, > > + distro_version, > > + self.arch) > > + distro_checksum = self.distro_checksum or known_distro_checksum > > > distro_checksum may be None. In this case vmimage.get() will silently skip > the check? I suggest to log a warn message. > > Yes, good point. But, I think adding that warning to Avocado's vmimage.get() method itself is even better. What do you think? Thanks for the review! - Cleber.
Hi, On 4/19/21 3:35 PM, Cleber Rosa wrote: > On Mon, Apr 19, 2021 at 12:25:44PM -0300, Wainer dos Santos Moschetta wrote: >> Hi, >> >> On 4/14/21 7:14 PM, Cleber Rosa wrote: >>> Instead of having, by default, the checksum in the tests, and the >>> definition of tests in the framework, let's keep them together. >>> >>> A central definition for distributions is available, and it should >>> allow other known distros to be added more easily. >>> >>> No behavior change is expected here, and tests can still define >>> a distro_checksum value if for some reason they want to override >>> the known distribution information. >>> >>> Signed-off-by: Cleber Rosa <crosa@redhat.com> >>> --- >>> tests/acceptance/avocado_qemu/__init__.py | 34 +++++++++++++++++++++-- >>> tests/acceptance/boot_linux.py | 8 ------ >>> 2 files changed, 32 insertions(+), 10 deletions(-) >>> >>> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py >>> index aae1e5bbc9..97093614d9 100644 >>> --- a/tests/acceptance/avocado_qemu/__init__.py >>> +++ b/tests/acceptance/avocado_qemu/__init__.py >>> @@ -299,6 +299,30 @@ def ssh_command(self, command): >>> return stdout_lines, stderr_lines >>> +#: A collection of known distros and their respective image checksum >>> +KNOWN_DISTROS = { >> Do you plan to expand that mapping to record values other than checksums? >> Otherwise it could be named KNOWN_DISTROS_CHECKSUMS. >> > Let's just say I had an intuition about it being used for other > purposes. Talking to Eric Auger earlier this morning, he will expand > this mapping with default kernel args distros, so that he can *add* > to the common args. Ok > >>> + 'fedora': { >>> + '31': { >>> + 'x86_64': >>> + {'checksum': 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'}, >>> + 'aarch64': >>> + {'checksum': '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'}, >>> + 'ppc64': >>> + {'checksum': '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'}, >>> + 's390x': >>> + {'checksum': '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'}, >>> + } >>> + } >>> + } >>> + >>> + >>> +def get_known_distro_checksum(distro, distro_version, arch): >>> + try: >>> + return KNOWN_DISTROS.get(distro).get(distro_version).get(arch).get('checksum') >>> + except AttributeError: >>> + return None >>> + >>> + >> Currently we have a few loose methods on avocado_qemu/__init__.py, and I'm >> about to send a series to wrap them in a mixin class. This series will >> introduce more loose code on the file; so would you consider moving >> KNOWN_DISTROS and get_known_distro_checksum() to the LinuxTest class, and >> possibly making the latest a class method? >> > Some of our experience in "avocado.Test" revealed that users would: > > 1) find it confusing to have so many methods in the class that are not useful > to them > > 2) would conflict with variables/attributes of their own > > About #2, we end up turning a lot of variables atttributes into > properties so that errors would be explicit when users tried to > overwrite them unknowingly. > > But, in the specific example of KNOWN_DISTROS and its expansion I > mentioned before, it may indeed make sense to have a Test or LinuxTest > method that test writers can use. It'd probably need to be a bit more > generic and evolved than this current version though. > > Maybe wait for Eric's input based on real world use case here? Sure, we can wait on Eric's input. > >>> class LinuxTest(Test, LinuxSSHMixIn): >>> """Facilitates having a cloud-image Linux based available. >>> @@ -348,14 +372,20 @@ def download_boot(self): >>> vmimage.QEMU_IMG = qemu_img >>> self.log.info('Downloading/preparing boot image') >>> + distro = 'fedora' >>> + distro_version = '31' >>> + known_distro_checksum = get_known_distro_checksum(distro, >>> + distro_version, >>> + self.arch) >>> + distro_checksum = self.distro_checksum or known_distro_checksum >> >> distro_checksum may be None. In this case vmimage.get() will silently skip >> the check? I suggest to log a warn message. >> >> > Yes, good point. But, I think adding that warning to Avocado's > vmimage.get() method itself is even better. What do you think? I like the idea. IMHO avocado_qemu should be a thin layer upon Avocado, so such as kind of code should live on avocado's side. I just opened the PR: https://github.com/avocado-framework/avocado/pull/4539 - Wainer > > Thanks for the review! > - Cleber.
Hi Cleber, On 4/15/21 12:14 AM, Cleber Rosa wrote: > Instead of having, by default, the checksum in the tests, and the > definition of tests in the framework, let's keep them together. > > A central definition for distributions is available, and it should > allow other known distros to be added more easily. > > No behavior change is expected here, and tests can still define > a distro_checksum value if for some reason they want to override > the known distribution information. > > Signed-off-by: Cleber Rosa <crosa@redhat.com> > --- > tests/acceptance/avocado_qemu/__init__.py | 34 +++++++++++++++++++++-- > tests/acceptance/boot_linux.py | 8 ------ > 2 files changed, 32 insertions(+), 10 deletions(-) > > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py > index aae1e5bbc9..97093614d9 100644 > --- a/tests/acceptance/avocado_qemu/__init__.py > +++ b/tests/acceptance/avocado_qemu/__init__.py > @@ -299,6 +299,30 @@ def ssh_command(self, command): > return stdout_lines, stderr_lines > > > +#: A collection of known distros and their respective image checksum > +KNOWN_DISTROS = { > + 'fedora': { > + '31': { > + 'x86_64': > + {'checksum': 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'}, > + 'aarch64': > + {'checksum': '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'}, > + 'ppc64': > + {'checksum': '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'}, > + 's390x': > + {'checksum': '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'}, > + } > + } > + } assuming we may put other data like kernel params and maybe pxeboot URL, this may grow rapidly, Maybe we should put that in a different file? > + > + > +def get_known_distro_checksum(distro, distro_version, arch): > + try: > + return KNOWN_DISTROS.get(distro).get(distro_version).get(arch).get('checksum') > + except AttributeError: > + return None > + > + > class LinuxTest(Test, LinuxSSHMixIn): > """Facilitates having a cloud-image Linux based available. > > @@ -348,14 +372,20 @@ def download_boot(self): > vmimage.QEMU_IMG = qemu_img > > self.log.info('Downloading/preparing boot image') > + distro = 'fedora' > + distro_version = '31' > + known_distro_checksum = get_known_distro_checksum(distro, > + distro_version, > + self.arch) > + distro_checksum = self.distro_checksum or known_distro_checksum > # Fedora 31 only provides ppc64le images > image_arch = self.arch > if image_arch == 'ppc64': > image_arch = 'ppc64le' > try: > boot = vmimage.get( > - 'fedora', arch=image_arch, version='31', > - checksum=self.distro_checksum, > + distro, arch=image_arch, version=distro_version, > + checksum=distro_checksum, > algorithm='sha256', > cache_dir=self.cache_dirs[0], > snapshot_dir=self.workdir) > diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py > index c7bc3a589e..9e618c6daa 100644 > --- a/tests/acceptance/boot_linux.py > +++ b/tests/acceptance/boot_linux.py > @@ -20,8 +20,6 @@ class BootLinuxX8664(LinuxTest): > :avocado: tags=arch:x86_64 > """ > > - distro_checksum = 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0' > - > def test_pc_i440fx_tcg(self): > """ > :avocado: tags=machine:pc > @@ -66,8 +64,6 @@ class BootLinuxAarch64(LinuxTest): > :avocado: tags=machine:gic-version=2 > """ > > - distro_checksum = '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49' > - > def add_common_args(self): > self.vm.add_args('-bios', > os.path.join(BUILD_DIR, 'pc-bios', > @@ -119,8 +115,6 @@ class BootLinuxPPC64(LinuxTest): > :avocado: tags=arch:ppc64 > """ > > - distro_checksum = '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58' > - > def test_pseries_tcg(self): > """ > :avocado: tags=machine:pseries > @@ -136,8 +130,6 @@ class BootLinuxS390X(LinuxTest): > :avocado: tags=arch:s390x > """ > > - distro_checksum = '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d' > - > @skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab') > def test_s390_ccw_virtio_tcg(self): > """ > Thanks Eric
On 4/22/21 3:56 AM, Auger Eric wrote: > Hi Cleber, > > On 4/15/21 12:14 AM, Cleber Rosa wrote: >> Instead of having, by default, the checksum in the tests, and the >> definition of tests in the framework, let's keep them together. >> >> A central definition for distributions is available, and it should >> allow other known distros to be added more easily. >> >> No behavior change is expected here, and tests can still define >> a distro_checksum value if for some reason they want to override >> the known distribution information. >> >> Signed-off-by: Cleber Rosa <crosa@redhat.com> >> --- >> tests/acceptance/avocado_qemu/__init__.py | 34 +++++++++++++++++++++-- >> tests/acceptance/boot_linux.py | 8 ------ >> 2 files changed, 32 insertions(+), 10 deletions(-) >> >> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py >> index aae1e5bbc9..97093614d9 100644 >> --- a/tests/acceptance/avocado_qemu/__init__.py >> +++ b/tests/acceptance/avocado_qemu/__init__.py >> @@ -299,6 +299,30 @@ def ssh_command(self, command): >> return stdout_lines, stderr_lines >> >> >> +#: A collection of known distros and their respective image checksum >> +KNOWN_DISTROS = { >> + 'fedora': { >> + '31': { >> + 'x86_64': >> + {'checksum': 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'}, >> + 'aarch64': >> + {'checksum': '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'}, >> + 'ppc64': >> + {'checksum': '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'}, >> + 's390x': >> + {'checksum': '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'}, >> + } >> + } >> + } > assuming we may put other data like kernel params and maybe pxeboot URL, > this may grow rapidly, Maybe we should put that in a different file? Hi Eric, Sorry for the monstrous delay here. You and I are thinking alike, but I'm planning to handle this on the Avocado side. I'm writing a BluePrint[1] for what would be a revision of the Avocado asset API. In that new proposal, data files (some builtin, some external) would be used to contain information about known assets. So, for now, I think it's better to keep this AS IS. Let me know if this works for you. Thanks, - Cleber. [1] - https://github.com/avocado-framework/avocado/issues/4458
Hi Cleber, On 4/15/21 12:14 AM, Cleber Rosa wrote: > Instead of having, by default, the checksum in the tests, and the > definition of tests in the framework, let's keep them together. > > A central definition for distributions is available, and it should > allow other known distros to be added more easily. > > No behavior change is expected here, and tests can still define > a distro_checksum value if for some reason they want to override > the known distribution information. > > Signed-off-by: Cleber Rosa <crosa@redhat.com> Acked-by: Eric Auger <eric.auger@redhat.com> Thanks Eric > --- > tests/acceptance/avocado_qemu/__init__.py | 34 +++++++++++++++++++++-- > tests/acceptance/boot_linux.py | 8 ------ > 2 files changed, 32 insertions(+), 10 deletions(-) > > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py > index aae1e5bbc9..97093614d9 100644 > --- a/tests/acceptance/avocado_qemu/__init__.py > +++ b/tests/acceptance/avocado_qemu/__init__.py > @@ -299,6 +299,30 @@ def ssh_command(self, command): > return stdout_lines, stderr_lines > > > +#: A collection of known distros and their respective image checksum > +KNOWN_DISTROS = { > + 'fedora': { > + '31': { > + 'x86_64': > + {'checksum': 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'}, > + 'aarch64': > + {'checksum': '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'}, > + 'ppc64': > + {'checksum': '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'}, > + 's390x': > + {'checksum': '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'}, > + } > + } > + } > + > + > +def get_known_distro_checksum(distro, distro_version, arch): > + try: > + return KNOWN_DISTROS.get(distro).get(distro_version).get(arch).get('checksum') > + except AttributeError: > + return None > + > + > class LinuxTest(Test, LinuxSSHMixIn): > """Facilitates having a cloud-image Linux based available. > > @@ -348,14 +372,20 @@ def download_boot(self): > vmimage.QEMU_IMG = qemu_img > > self.log.info('Downloading/preparing boot image') > + distro = 'fedora' > + distro_version = '31' > + known_distro_checksum = get_known_distro_checksum(distro, > + distro_version, > + self.arch) > + distro_checksum = self.distro_checksum or known_distro_checksum > # Fedora 31 only provides ppc64le images > image_arch = self.arch > if image_arch == 'ppc64': > image_arch = 'ppc64le' > try: > boot = vmimage.get( > - 'fedora', arch=image_arch, version='31', > - checksum=self.distro_checksum, > + distro, arch=image_arch, version=distro_version, > + checksum=distro_checksum, > algorithm='sha256', > cache_dir=self.cache_dirs[0], > snapshot_dir=self.workdir) > diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py > index c7bc3a589e..9e618c6daa 100644 > --- a/tests/acceptance/boot_linux.py > +++ b/tests/acceptance/boot_linux.py > @@ -20,8 +20,6 @@ class BootLinuxX8664(LinuxTest): > :avocado: tags=arch:x86_64 > """ > > - distro_checksum = 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0' > - > def test_pc_i440fx_tcg(self): > """ > :avocado: tags=machine:pc > @@ -66,8 +64,6 @@ class BootLinuxAarch64(LinuxTest): > :avocado: tags=machine:gic-version=2 > """ > > - distro_checksum = '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49' > - > def add_common_args(self): > self.vm.add_args('-bios', > os.path.join(BUILD_DIR, 'pc-bios', > @@ -119,8 +115,6 @@ class BootLinuxPPC64(LinuxTest): > :avocado: tags=arch:ppc64 > """ > > - distro_checksum = '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58' > - > def test_pseries_tcg(self): > """ > :avocado: tags=machine:pseries > @@ -136,8 +130,6 @@ class BootLinuxS390X(LinuxTest): > :avocado: tags=arch:s390x > """ > > - distro_checksum = '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d' > - > @skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab') > def test_s390_ccw_virtio_tcg(self): > """
Hi Cleber, On 7/8/21 4:47 PM, Cleber Rosa wrote: > > On 4/22/21 3:56 AM, Auger Eric wrote: >> Hi Cleber, >> >> On 4/15/21 12:14 AM, Cleber Rosa wrote: >>> Instead of having, by default, the checksum in the tests, and the >>> definition of tests in the framework, let's keep them together. >>> >>> A central definition for distributions is available, and it should >>> allow other known distros to be added more easily. >>> >>> No behavior change is expected here, and tests can still define >>> a distro_checksum value if for some reason they want to override >>> the known distribution information. >>> >>> Signed-off-by: Cleber Rosa <crosa@redhat.com> >>> --- >>> tests/acceptance/avocado_qemu/__init__.py | 34 >>> +++++++++++++++++++++-- >>> tests/acceptance/boot_linux.py | 8 ------ >>> 2 files changed, 32 insertions(+), 10 deletions(-) >>> >>> diff --git a/tests/acceptance/avocado_qemu/__init__.py >>> b/tests/acceptance/avocado_qemu/__init__.py >>> index aae1e5bbc9..97093614d9 100644 >>> --- a/tests/acceptance/avocado_qemu/__init__.py >>> +++ b/tests/acceptance/avocado_qemu/__init__.py >>> @@ -299,6 +299,30 @@ def ssh_command(self, command): >>> return stdout_lines, stderr_lines >>> +#: A collection of known distros and their respective image >>> checksum >>> +KNOWN_DISTROS = { >>> + 'fedora': { >>> + '31': { >>> + 'x86_64': >>> + {'checksum': >>> 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'}, >>> + 'aarch64': >>> + {'checksum': >>> '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'}, >>> + 'ppc64': >>> + {'checksum': >>> '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'}, >>> + 's390x': >>> + {'checksum': >>> '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'}, >>> + } >>> + } >>> + } >> assuming we may put other data like kernel params and maybe pxeboot URL, >> this may grow rapidly, Maybe we should put that in a different file? > > > Hi Eric, > > > Sorry for the monstrous delay here. You and I are thinking alike, but > I'm planning to handle this on the Avocado side. I'm writing a > BluePrint[1] for what would be a revision of the Avocado asset API. > In that new proposal, data files (some builtin, some external) would > be used to contain information about known assets. > > > So, for now, I think it's better to keep this AS IS. Let me know if > this works for you. OK for me. This was just a suggestion anyway and the functionality is more important atm. Thanks Eric > > > Thanks, > > - Cleber. > > > [1] - https://github.com/avocado-framework/avocado/issues/4458 > >
diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py index aae1e5bbc9..97093614d9 100644 --- a/tests/acceptance/avocado_qemu/__init__.py +++ b/tests/acceptance/avocado_qemu/__init__.py @@ -299,6 +299,30 @@ def ssh_command(self, command): return stdout_lines, stderr_lines +#: A collection of known distros and their respective image checksum +KNOWN_DISTROS = { + 'fedora': { + '31': { + 'x86_64': + {'checksum': 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'}, + 'aarch64': + {'checksum': '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'}, + 'ppc64': + {'checksum': '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'}, + 's390x': + {'checksum': '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'}, + } + } + } + + +def get_known_distro_checksum(distro, distro_version, arch): + try: + return KNOWN_DISTROS.get(distro).get(distro_version).get(arch).get('checksum') + except AttributeError: + return None + + class LinuxTest(Test, LinuxSSHMixIn): """Facilitates having a cloud-image Linux based available. @@ -348,14 +372,20 @@ def download_boot(self): vmimage.QEMU_IMG = qemu_img self.log.info('Downloading/preparing boot image') + distro = 'fedora' + distro_version = '31' + known_distro_checksum = get_known_distro_checksum(distro, + distro_version, + self.arch) + distro_checksum = self.distro_checksum or known_distro_checksum # Fedora 31 only provides ppc64le images image_arch = self.arch if image_arch == 'ppc64': image_arch = 'ppc64le' try: boot = vmimage.get( - 'fedora', arch=image_arch, version='31', - checksum=self.distro_checksum, + distro, arch=image_arch, version=distro_version, + checksum=distro_checksum, algorithm='sha256', cache_dir=self.cache_dirs[0], snapshot_dir=self.workdir) diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py index c7bc3a589e..9e618c6daa 100644 --- a/tests/acceptance/boot_linux.py +++ b/tests/acceptance/boot_linux.py @@ -20,8 +20,6 @@ class BootLinuxX8664(LinuxTest): :avocado: tags=arch:x86_64 """ - distro_checksum = 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0' - def test_pc_i440fx_tcg(self): """ :avocado: tags=machine:pc @@ -66,8 +64,6 @@ class BootLinuxAarch64(LinuxTest): :avocado: tags=machine:gic-version=2 """ - distro_checksum = '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49' - def add_common_args(self): self.vm.add_args('-bios', os.path.join(BUILD_DIR, 'pc-bios', @@ -119,8 +115,6 @@ class BootLinuxPPC64(LinuxTest): :avocado: tags=arch:ppc64 """ - distro_checksum = '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58' - def test_pseries_tcg(self): """ :avocado: tags=machine:pseries @@ -136,8 +130,6 @@ class BootLinuxS390X(LinuxTest): :avocado: tags=arch:s390x """ - distro_checksum = '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d' - @skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab') def test_s390_ccw_virtio_tcg(self): """
Instead of having, by default, the checksum in the tests, and the definition of tests in the framework, let's keep them together. A central definition for distributions is available, and it should allow other known distros to be added more easily. No behavior change is expected here, and tests can still define a distro_checksum value if for some reason they want to override the known distribution information. Signed-off-by: Cleber Rosa <crosa@redhat.com> --- tests/acceptance/avocado_qemu/__init__.py | 34 +++++++++++++++++++++-- tests/acceptance/boot_linux.py | 8 ------ 2 files changed, 32 insertions(+), 10 deletions(-)