From 6654cd61e0f612c2a35552ed9fb38b97a10c44ea Mon Sep 17 00:00:00 2001
From: Cedric Bosdonnat <cbosdonnat@suse.com>
Date: Thu, 11 Feb 2021 16:41:14 +0100
Subject: [PATCH] Open suse 3002.2 xen grub (#316)

* virt: convert xen pv XML generation test to pytest

* virt: better look for grub.xen when generating xen pv definition

openSUSE 15.3 and SLES 15 SP3 have removed the compatibility symlink for
/usr/share/grub2/x86_64-xen/grub.xen to
/usr/lib/grub2/x86_64-xen/grub.xen. virt._gen_xml() need to check which
is present and put in Xen PV VMs XML definition.
---
 changelog/59484.fixed                         |  1 +
 salt/modules/virt.py                          |  9 +-
 .../pytests/unit/modules/virt/test_domain.py  | 82 ++++++++++++++++++-
 tests/unit/modules/test_virt.py               | 48 -----------
 4 files changed, 90 insertions(+), 50 deletions(-)
 create mode 100644 changelog/59484.fixed

diff --git a/changelog/59484.fixed b/changelog/59484.fixed
new file mode 100644
index 0000000000..b685510ad9
--- /dev/null
+++ b/changelog/59484.fixed
@@ -0,0 +1 @@
+Detect and fix grub.xen path
diff --git a/salt/modules/virt.py b/salt/modules/virt.py
index a47003c08c..209c859fd5 100644
--- a/salt/modules/virt.py
+++ b/salt/modules/virt.py
@@ -889,7 +889,14 @@ def _gen_xml(
         # Compute the Xen PV boot method
         if __grains__["os_family"] == "Suse":
             if not boot or not boot.get("kernel", None):
-                context["boot"]["kernel"] = "/usr/lib/grub2/x86_64-xen/grub.xen"
+                paths = [
+                    path
+                    for path in ["/usr/share", "/usr/lib"]
+                    if os.path.exists(path + "/grub2/x86_64-xen/grub.xen")
+                ]
+                if not paths:
+                    raise CommandExecutionError("grub-x86_64-xen needs to be installed")
+                context["boot"]["kernel"] = paths[0] + "/grub2/x86_64-xen/grub.xen"
                 context["boot_dev"] = []
 
     default_port = 23023
diff --git a/tests/pytests/unit/modules/virt/test_domain.py b/tests/pytests/unit/modules/virt/test_domain.py
index 9938499d37..4765fc1bd7 100644
--- a/tests/pytests/unit/modules/virt/test_domain.py
+++ b/tests/pytests/unit/modules/virt/test_domain.py
@@ -5,7 +5,7 @@ import salt.modules.virt as virt
 import salt.syspaths
 import salt.utils.xmlutil as xmlutil
 from salt._compat import ElementTree as ET
-from salt.exceptions import SaltInvocationError
+from salt.exceptions import CommandExecutionError, SaltInvocationError
 from tests.support.mock import MagicMock, patch
 
 from .conftest import loader_modules_config
@@ -17,6 +17,86 @@ def configure_loader_modules():
     return loader_modules_config()
 
 
+@pytest.mark.parametrize(
+    "loader",
+    [
+        "/usr/lib/grub2/x86_64-xen/grub.xen",
+        "/usr/share/grub2/x86_64-xen/grub.xen",
+        None,
+    ],
+)
+def test_gen_xml_for_xen_default_profile(loader):
+    """
+    Test virt._gen_xml(), XEN PV default profile case
+    """
+    diskp = virt._disk_profile(
+        virt.libvirt.openAuth.return_value, "default", "xen", [], "hello"
+    )
+    nicp = virt._nic_profile("default", "xen")
+    with patch.dict(
+        virt.__grains__, {"os_family": "Suse"}  # pylint: disable=no-member
+    ):
+        os_mock = MagicMock(spec=virt.os)
+
+        def fake_exists(path):
+            return loader and path == loader
+
+        os_mock.path.exists = MagicMock(side_effect=fake_exists)
+
+        with patch.dict(virt.__dict__, {"os": os_mock}):
+            if loader:
+                xml_data = virt._gen_xml(
+                    virt.libvirt.openAuth.return_value,
+                    "hello",
+                    1,
+                    512,
+                    diskp,
+                    nicp,
+                    "xen",
+                    "xen",
+                    "x86_64",
+                    boot=None,
+                )
+                root = ET.fromstring(xml_data)
+                assert root.attrib["type"] == "xen"
+                assert root.find("vcpu").text == "1"
+                assert root.find("memory").text == str(512 * 1024)
+                assert root.find("memory").attrib["unit"] == "KiB"
+                assert root.find(".//kernel").text == loader
+
+                disks = root.findall(".//disk")
+                assert len(disks) == 1
+                disk = disks[0]
+                root_dir = salt.config.DEFAULT_MINION_OPTS.get("root_dir")
+                assert disk.find("source").attrib["file"].startswith(root_dir)
+                assert "hello_system" in disk.find("source").attrib["file"]
+                assert disk.find("target").attrib["dev"] == "xvda"
+                assert disk.find("target").attrib["bus"] == "xen"
+                assert disk.find("driver").attrib["name"] == "qemu"
+                assert disk.find("driver").attrib["type"] == "qcow2"
+
+                interfaces = root.findall(".//interface")
+                assert len(interfaces) == 1
+                iface = interfaces[0]
+                assert iface.attrib["type"] == "bridge"
+                assert iface.find("source").attrib["bridge"] == "br0"
+                assert iface.find("model") is None
+            else:
+                with pytest.raises(CommandExecutionError):
+                    xml_data = virt._gen_xml(
+                        virt.libvirt.openAuth.return_value,
+                        "hello",
+                        1,
+                        512,
+                        diskp,
+                        nicp,
+                        "xen",
+                        "xen",
+                        "x86_64",
+                        boot=None,
+                    )
+
+
 def test_update_xen_disk_volumes(make_mock_vm, make_mock_storage_pool):
     xml_def = """
         <domain type='xen'>
diff --git a/tests/unit/modules/test_virt.py b/tests/unit/modules/test_virt.py
index 41a15eaa3d..e8282ea7a1 100644
--- a/tests/unit/modules/test_virt.py
+++ b/tests/unit/modules/test_virt.py
@@ -1125,54 +1125,6 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         self.assertEqual(iface.find("source").attrib["bridge"], "DEFAULT")
         self.assertEqual(iface.find("model").attrib["type"], "e1000")
 
-    def test_gen_xml_for_xen_default_profile(self):
-        """
-        Test virt._gen_xml(), XEN PV default profile case
-        """
-        diskp = virt._disk_profile(self.mock_conn, "default", "xen", [], "hello")
-        nicp = virt._nic_profile("default", "xen")
-        with patch.dict(
-            virt.__grains__, {"os_family": "Suse"}  # pylint: disable=no-member
-        ):
-            xml_data = virt._gen_xml(
-                self.mock_conn,
-                "hello",
-                1,
-                512,
-                diskp,
-                nicp,
-                "xen",
-                "xen",
-                "x86_64",
-                boot=None,
-            )
-            root = ET.fromstring(xml_data)
-            self.assertEqual(root.attrib["type"], "xen")
-            self.assertEqual(root.find("vcpu").text, "1")
-            self.assertEqual(root.find("memory").text, str(512 * 1024))
-            self.assertEqual(root.find("memory").attrib["unit"], "KiB")
-            self.assertEqual(
-                root.find(".//kernel").text, "/usr/lib/grub2/x86_64-xen/grub.xen"
-            )
-
-            disks = root.findall(".//disk")
-            self.assertEqual(len(disks), 1)
-            disk = disks[0]
-            root_dir = salt.config.DEFAULT_MINION_OPTS.get("root_dir")
-            self.assertTrue(disk.find("source").attrib["file"].startswith(root_dir))
-            self.assertTrue("hello_system" in disk.find("source").attrib["file"])
-            self.assertEqual(disk.find("target").attrib["dev"], "xvda")
-            self.assertEqual(disk.find("target").attrib["bus"], "xen")
-            self.assertEqual(disk.find("driver").attrib["name"], "qemu")
-            self.assertEqual(disk.find("driver").attrib["type"], "qcow2")
-
-            interfaces = root.findall(".//interface")
-            self.assertEqual(len(interfaces), 1)
-            iface = interfaces[0]
-            self.assertEqual(iface.attrib["type"], "bridge")
-            self.assertEqual(iface.find("source").attrib["bridge"], "br0")
-            self.assertIsNone(iface.find("model"))
-
     def test_gen_xml_for_esxi_custom_profile(self):
         """
         Test virt._gen_xml(), ESXi/vmware custom profile case
-- 
2.30.0


