sts-sponsors team mailing list archive

Thread
Date

[Merge] ~ack/maas:provisioningserver-utils-arch into maas:master

To: mp+439920@xxxxxxxxxxxxxxxxxx
From: Alberto Donato <mp+439920@xxxxxxxxxxxxxxxxxx>
Date: Wed, 29 Mar 2023 12:46:22 -0000
Reply-to: mp+439920@xxxxxxxxxxxxxxxxxx
Sender: noreply@xxxxxxxxxxxxx

Alberto Donato has proposed merging ~ack/maas:provisioningserver-utils-arch into maas:master.

Commit message:
move arch-related utils to provisioningserver.utils.arch

This also removes duplicated arches definitions in virsh driver



Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~ack/maas/+git/maas/+merge/439920
-- 
Your team MAAS Committers is subscribed to branch maas:master.

diff --git a/src/maasserver/testing/commissioning.py b/src/maasserver/testing/commissioning.py
index 5f712db..0965788 100644
--- a/src/maasserver/testing/commissioning.py
+++ b/src/maasserver/testing/commissioning.py
@@ -5,7 +5,7 @@ import random
 from typing import Dict, List, Optional
 
 from maasserver.testing.factory import factory
-from provisioningserver.utils import kernel_to_debian_architecture
+from provisioningserver.utils.arch import kernel_to_debian_architecture
 from provisioningserver.utils.network import (
     annotate_with_default_monitored_interfaces,
     get_default_monitored_interfaces,
diff --git a/src/metadataserver/builtin_scripts/hooks.py b/src/metadataserver/builtin_scripts/hooks.py
index b269747..81aeb6b 100644
--- a/src/metadataserver/builtin_scripts/hooks.py
+++ b/src/metadataserver/builtin_scripts/hooks.py
@@ -52,7 +52,7 @@ from provisioningserver.refresh.node_info_scripts import (
     LIST_MODALIASES_OUTPUT_NAME,
     NODE_INFO_SCRIPTS,
 )
-from provisioningserver.utils import kernel_to_debian_architecture
+from provisioningserver.utils.arch import kernel_to_debian_architecture
 from provisioningserver.utils.ipaddr import is_ipoib_mac
 from provisioningserver.utils.lxd import parse_lxd_cpuinfo, parse_lxd_networks
 
diff --git a/src/provisioningserver/drivers/hardware/tests/test_virsh.py b/src/provisioningserver/drivers/hardware/tests/test_virsh.py
index 04e0c7d..9b80ed8 100644
--- a/src/provisioningserver/drivers/hardware/tests/test_virsh.py
+++ b/src/provisioningserver/drivers/hardware/tests/test_virsh.py
@@ -20,6 +20,7 @@ from maastesting.factory import factory
 from maastesting.matchers import MockCalledOnceWith, MockCallsMatch
 from maastesting.testcase import MAASTestCase, MAASTwistedRunTest
 from provisioningserver.drivers.hardware import virsh
+from provisioningserver.utils.arch import KERNEL_TO_DEBIAN_ARCHITECTURES
 from provisioningserver.utils.shell import get_env_with_locale
 from provisioningserver.utils.twisted import asynchronous
 
@@ -223,15 +224,15 @@ class TestVirshSSH(MAASTestCase):
         arch = factory.make_name("arch")
         output = SAMPLE_DUMPXML % arch
         conn = self.configure_virshssh(output)
-        expected = conn.get_arch("")
+        expected = conn.get_arch("machine")
         self.assertEqual(arch, expected)
 
     def test_get_arch_returns_valid_fixed(self):
-        arch = random.choice(list(virsh.ARCH_FIX))
-        fixed_arch = virsh.ARCH_FIX[arch]
+        arch = random.choice(list(KERNEL_TO_DEBIAN_ARCHITECTURES))
+        fixed_arch = KERNEL_TO_DEBIAN_ARCHITECTURES[arch]
         output = SAMPLE_DUMPXML % arch
         conn = self.configure_virshssh(output)
-        expected = conn.get_arch("")
+        expected = conn.get_arch("machine")
         self.assertEqual(fixed_arch, expected)
 
     def test_resets_locale(self):
diff --git a/src/provisioningserver/drivers/hardware/virsh.py b/src/provisioningserver/drivers/hardware/virsh.py
index a4817dd..12c1dfe 100644
--- a/src/provisioningserver/drivers/hardware/virsh.py
+++ b/src/provisioningserver/drivers/hardware/virsh.py
@@ -9,6 +9,7 @@ import pexpect
 
 from provisioningserver.logger import get_maas_logger
 from provisioningserver.rpc.utils import commission_node, create_node
+from provisioningserver.utils.arch import kernel_to_debian_architecture
 from provisioningserver.utils.shell import get_env_with_locale
 from provisioningserver.utils.twisted import synchronous
 
@@ -18,17 +19,6 @@ XPATH_ARCH = "/domain/os/type/@arch"
 XPATH_BOOT = "/domain/os/boot"
 XPATH_OS = "/domain/os"
 
-# Virsh stores the architecture with a different
-# label then MAAS. This maps virsh architecture to
-# MAAS architecture.
-ARCH_FIX = {
-    "x86_64": "amd64",
-    "ppc64": "ppc64el",
-    "ppc64le": "ppc64el",
-    "i686": "i386",
-    "aarch64": "arm64",
-}
-
 
 class VirshVMState:
     OFF = "shut off"
@@ -192,9 +182,11 @@ class VirshSSH(pexpect.spawn):
         evaluator = etree.XPathEvaluator(doc)
         arch = evaluator(XPATH_ARCH)[0]
 
-        # Fix architectures that need to be referenced by a different
-        # name, that MAAS understands.
-        return ARCH_FIX.get(arch, arch)
+        # Report arch as debian architecture name
+        try:
+            return kernel_to_debian_architecture(arch)
+        except KeyError:
+            return arch
 
     def configure_pxe_boot(self, machine):
         """Given the specified machine, reads the XML dump and determines
diff --git a/src/provisioningserver/drivers/pod/lxd.py b/src/provisioningserver/drivers/pod/lxd.py
index 571ef0b..2a19d49 100644
--- a/src/provisioningserver/drivers/pod/lxd.py
+++ b/src/provisioningserver/drivers/pod/lxd.py
@@ -48,7 +48,7 @@ from provisioningserver.refresh.node_info_scripts import (
     RUN_MACHINE_RESOURCES,
 )
 from provisioningserver.rpc.exceptions import PodInvalidResources
-from provisioningserver.utils import (
+from provisioningserver.utils.arch import (
     debian_to_kernel_architecture,
     kernel_to_debian_architecture,
 )
diff --git a/src/provisioningserver/drivers/pod/tests/test_lxd.py b/src/provisioningserver/drivers/pod/tests/test_lxd.py
index baf0c4a..7a82865 100644
--- a/src/provisioningserver/drivers/pod/tests/test_lxd.py
+++ b/src/provisioningserver/drivers/pod/tests/test_lxd.py
@@ -41,7 +41,7 @@ from provisioningserver.testing.certificates import (
     get_sample_cert,
     SampleCertificateFixture,
 )
-from provisioningserver.utils import (
+from provisioningserver.utils.arch import (
     debian_to_kernel_architecture,
     kernel_to_debian_architecture,
 )
diff --git a/src/provisioningserver/drivers/pod/tests/test_virsh.py b/src/provisioningserver/drivers/pod/tests/test_virsh.py
index de293bd..bd1f362 100644
--- a/src/provisioningserver/drivers/pod/tests/test_virsh.py
+++ b/src/provisioningserver/drivers/pod/tests/test_virsh.py
@@ -41,7 +41,7 @@ from provisioningserver.drivers.pod.virsh import (
 )
 from provisioningserver.enum import LIBVIRT_NETWORK, MACVLAN_MODE_CHOICES
 from provisioningserver.rpc.exceptions import PodInvalidResources
-from provisioningserver.utils import (
+from provisioningserver.utils.arch import (
     debian_to_kernel_architecture,
     kernel_to_debian_architecture,
     KERNEL_TO_DEBIAN_ARCHITECTURES,
diff --git a/src/provisioningserver/drivers/pod/virsh.py b/src/provisioningserver/drivers/pod/virsh.py
index 6a89b08..c96a37b 100644
--- a/src/provisioningserver/drivers/pod/virsh.py
+++ b/src/provisioningserver/drivers/pod/virsh.py
@@ -42,10 +42,10 @@ from provisioningserver.path import get_path
 from provisioningserver.prometheus.metrics import PROMETHEUS_METRICS
 from provisioningserver.rpc.exceptions import PodInvalidResources
 from provisioningserver.rpc.utils import commission_node, create_node
-from provisioningserver.utils import (
+from provisioningserver.utils import shell
+from provisioningserver.utils.arch import (
     debian_to_kernel_architecture,
     kernel_to_debian_architecture,
-    shell,
 )
 from provisioningserver.utils.network import generate_mac_address
 from provisioningserver.utils.shell import get_env_with_locale
diff --git a/src/provisioningserver/utils/__init__.py b/src/provisioningserver/utils/__init__.py
index a0800e2..62d7392 100644
--- a/src/provisioningserver/utils/__init__.py
+++ b/src/provisioningserver/utils/__init__.py
@@ -165,30 +165,3 @@ def is_instance_or_subclass(test, *query):
         return issubclass(test, query_tuple)
     except TypeError:
         return False
-
-
-# Architectures as defined by:
-# https://github.com/lxc/lxd/blob/master/shared/osarch/architectures.go
-# https://www.debian.org/releases/oldstable/i386/ch02s01.html.en
-DEBIAN_TO_KERNEL_ARCHITECTURES = {
-    "i386/generic": "i686",
-    "amd64/generic": "x86_64",
-    "arm64/generic": "aarch64",
-    "ppc64el/generic": "ppc64le",
-    "s390x/generic": "s390x",
-    "mips/generic": "mips",
-    "mips64el/generic": "mips64",
-}
-KERNEL_TO_DEBIAN_ARCHITECTURES = {
-    v: k for k, v in DEBIAN_TO_KERNEL_ARCHITECTURES.items()
-}
-
-
-def kernel_to_debian_architecture(kernel_arch):
-    """Map a kernel architecture to Debian architecture."""
-    return KERNEL_TO_DEBIAN_ARCHITECTURES[kernel_arch]
-
-
-def debian_to_kernel_architecture(debian_arch):
-    """Map a Debian architecture to kernel architecture."""
-    return DEBIAN_TO_KERNEL_ARCHITECTURES[debian_arch]
diff --git a/src/provisioningserver/utils/arch.py b/src/provisioningserver/utils/arch.py
index a195ea2..9998201 100644
--- a/src/provisioningserver/utils/arch.py
+++ b/src/provisioningserver/utils/arch.py
@@ -6,6 +6,22 @@
 from functools import lru_cache
 import os
 
+# Architectures as defined by:
+# https://github.com/lxc/lxd/blob/master/shared/osarch/architectures.go
+# https://www.debian.org/releases/oldstable/i386/ch02s01.html.en
+DEBIAN_TO_KERNEL_ARCHITECTURES = {
+    "i386/generic": "i686",
+    "amd64/generic": "x86_64",
+    "arm64/generic": "aarch64",
+    "ppc64el/generic": "ppc64le",
+    "s390x/generic": "s390x",
+    "mips/generic": "mips",
+    "mips64el/generic": "mips64",
+}
+KERNEL_TO_DEBIAN_ARCHITECTURES = {
+    v: k for k, v in DEBIAN_TO_KERNEL_ARCHITECTURES.items()
+}
+
 
 @lru_cache(maxsize=1)
 def get_architecture():
@@ -18,3 +34,13 @@ def get_architecture():
         apt_pkg.init()
         arch = apt_pkg.get_architectures()[0]
     return arch
+
+
+def kernel_to_debian_architecture(kernel_arch):
+    """Map a kernel architecture to Debian architecture."""
+    return KERNEL_TO_DEBIAN_ARCHITECTURES[kernel_arch]
+
+
+def debian_to_kernel_architecture(debian_arch):
+    """Map a Debian architecture to kernel architecture."""
+    return DEBIAN_TO_KERNEL_ARCHITECTURES[debian_arch]
diff --git a/src/provisioningserver/utils/pytest_tests/test_arch.py b/src/provisioningserver/utils/pytest_tests/test_arch.py
new file mode 100644
index 0000000..f03a716
--- /dev/null
+++ b/src/provisioningserver/utils/pytest_tests/test_arch.py
@@ -0,0 +1,55 @@
+import random
+
+import pytest
+
+from provisioningserver.utils.arch import (
+    debian_to_kernel_architecture,
+    get_architecture,
+    kernel_to_debian_architecture,
+)
+
+
+@pytest.fixture
+def clear_arch_cache():
+    get_architecture.cache_clear()
+    yield
+    get_architecture.cache_clear()
+
+
+@pytest.mark.usefixtures("clear_arch_cache")
+class TestGetArchitecture:
+    def test_get_architecture_from_deb(self, mocker):
+        arch = random.choice(["i386", "amd64", "arm64", "ppc64el"])
+        mocker.patch(
+            "apt_pkg.get_architectures", return_value=[arch, "otherarch"]
+        )
+        assert get_architecture() == arch
+
+    def test_get_architecture_from_snap_env(
+        self, mocker, monkeypatch, factory
+    ):
+        arch = factory.make_name("arch")
+        mock_get_architectures = mocker.patch("apt_pkg.get_architectures")
+        monkeypatch.setenv("SNAP_ARCH", arch)
+        assert get_architecture() == arch
+        mock_get_architectures.assert_not_called()
+
+
+@pytest.mark.parametrize(
+    "kernel_arch,debian_arch",
+    [
+        ("i686", "i386/generic"),
+        ("x86_64", "amd64/generic"),
+        ("aarch64", "arm64/generic"),
+        ("ppc64le", "ppc64el/generic"),
+        ("s390x", "s390x/generic"),
+        ("mips", "mips/generic"),
+        ("mips64", "mips64el/generic"),
+    ],
+)
+class TestKernelAndDebianArchitectures:
+    def test_kernel_to_debian_architecture(self, kernel_arch, debian_arch):
+        assert kernel_to_debian_architecture(kernel_arch) == debian_arch
+
+    def test_debian_to_kernel_architecture(self, kernel_arch, debian_arch):
+        assert debian_to_kernel_architecture(debian_arch) == kernel_arch
diff --git a/src/provisioningserver/utils/tests/test_arch.py b/src/provisioningserver/utils/tests/test_arch.py
deleted file mode 100644
index c0fcc45..0000000
--- a/src/provisioningserver/utils/tests/test_arch.py
+++ /dev/null
@@ -1,32 +0,0 @@
-import os
-import random
-from unittest.mock import patch
-
-from maastesting.factory import factory
-from maastesting.testcase import MAASTestCase
-from provisioningserver.utils.arch import get_architecture
-
-
-class TestGetArchitecture(MAASTestCase):
-    def setUp(self):
-        super().setUp()
-        get_architecture.cache_clear()
-
-    def tearDown(self):
-        super().tearDown()
-        get_architecture.cache_clear()
-
-    @patch("apt_pkg.get_architectures")
-    def test_get_architecture_from_deb(self, mock_get_architectures):
-        arch = random.choice(["i386", "amd64", "arm64", "ppc64el"])
-        mock_get_architectures.return_value = [arch, "otherarch"]
-        ret_arch = get_architecture()
-        self.assertEqual(arch, ret_arch)
-
-    @patch("apt_pkg.get_architectures")
-    def test_get_architecture_from_snap_env(self, mock_get_architectures):
-        arch = factory.make_name("arch")
-        self.patch(os, "environ", {"SNAP_ARCH": arch})
-        ret_arch = get_architecture()
-        self.assertEqual(arch, ret_arch)
-        mock_get_architectures.assert_not_called()
diff --git a/src/provisioningserver/utils/tests/test_utils.py b/src/provisioningserver/utils/tests/test_utils.py
index df16d05..c36c519 100644
--- a/src/provisioningserver/utils/tests/test_utils.py
+++ b/src/provisioningserver/utils/tests/test_utils.py
@@ -20,10 +20,8 @@ import provisioningserver.utils
 from provisioningserver.utils import (
     CircularDependency,
     classify,
-    debian_to_kernel_architecture,
     flatten,
     is_instance_or_subclass,
-    kernel_to_debian_architecture,
     locate_config,
     locate_template,
     sorttop,
@@ -288,47 +286,3 @@ class TestIsInstanceOrSubclass(MAASTestCase):
         self.assertTrue(
             is_instance_or_subclass(self.bar, *[Baz, [Bar, [Foo]]])
         )
-
-
-class TestKernelToDebianArchitecture(MAASTestCase):
-    """Tests for `kernel_to_debian_architecture`."""
-
-    scenarios = (
-        ("i386", {"kernel": "i686", "debian": "i386/generic"}),
-        ("x86_64", {"kernel": "x86_64", "debian": "amd64/generic"}),
-        ("aarch64", {"kernel": "aarch64", "debian": "arm64/generic"}),
-        ("ppc64le", {"kernel": "ppc64le", "debian": "ppc64el/generic"}),
-        ("s390x", {"kernel": "s390x", "debian": "s390x/generic"}),
-        ("mips", {"kernel": "mips", "debian": "mips/generic"}),
-        ("mips64", {"kernel": "mips64", "debian": "mips64el/generic"}),
-    )
-
-    def test_kernel_to_debian_architecture(self):
-        self.assertEqual(
-            self.debian, kernel_to_debian_architecture(self.kernel)
-        )
-
-
-class TestDebianToKernalArchitecture(MAASTestCase):
-    """Tests for `debian_to_kernal_architecture`."""
-
-    scenarios = (
-        ("i386/generic", {"kernel": "i686", "debian": "i386/generic"}),
-        ("amd64/generic", {"kernel": "x86_64", "debian": "amd64/generic"}),
-        ("arm64/generic", {"kernel": "aarch64", "debian": "arm64/generic"}),
-        (
-            "ppc64el/generic",
-            {"kernel": "ppc64le", "debian": "ppc64el/generic"},
-        ),
-        ("s390x/generic", {"kernel": "s390x", "debian": "s390x/generic"}),
-        ("mips/generic", {"kernel": "mips", "debian": "mips/generic"}),
-        (
-            "mips64el/generic",
-            {"kernel": "mips64", "debian": "mips64el/generic"},
-        ),
-    )
-
-    def test_kernel_to_debian_architecture(self):
-        self.assertEqual(
-            self.kernel, debian_to_kernel_architecture(self.debian)
-        )

Follow ups

[Merge] ~ack/maas:provisioningserver-utils-arch into maas:master
From: MAAS Lander, 2023-03-29
[Merge] ~ack/maas:provisioningserver-utils-arch into maas:master
From: Alberto Donato, 2023-03-29
Re: [UNITTESTS] -b provisioningserver-utils-arch lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas - TESTS PASS
From: MAAS Lander, 2023-03-29
Re: [Merge] ~ack/maas:provisioningserver-utils-arch into maas:master
From: Igor Brovtsin, 2023-03-29
Re: [UNITTESTS] -b provisioningserver-utils-arch lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas - TESTS FAILED
From: MAAS Lander, 2023-03-29
Re: [Merge] ~ack/maas:provisioningserver-utils-arch into maas:master
From: Igor Brovtsin, 2023-03-29
Re: [UNITTESTS] -b provisioningserver-utils-arch lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas - TESTS PASS
From: MAAS Lander, 2023-03-29