Merge ~seb128/utah:without-desktop-rename into utah:master

[Sebastien Bacher]

Alternative to https://code.launchpad.net/~seb128/utah/+git/utah/+merge/435880 without doing different variants since tests rely on the st_variant matching the filename. The only exception is desktop-legacy on >= lunar. We already have a ubiquity_image variable which we can use to tell old images vs new

[Heather Ellsworth (hellsworth)]

Thanks Seb for working on this! To support your work, I've tested this PR but a few changes were needed to make all of the tests pass. With the following changes, here is my local test log showing all passing tests: https://pastebin.ubuntu.com/p/mW8xNfj3sv/

The only remaining strange thing is that all of the utah*.deb files are put in two places: just outside the root utah directory (as is usual), and then also inside the utah directory but it's not clear to me why they are duplicated. I've just been ignoring the debs inside the utah directory, when building/installing/testing.

Here are the test changes previously mentioned:

1) The change to test_run_utah_tests.py fixes an issue in python 3.11 where 'U' is deprecated.

diff --git a/tests/test_run_utah_tests.py b/tests/test_run_utah_tests.py
index 4c756b40..419c1e68 100644
--- a/tests/test_run_utah_tests.py
+++ b/tests/test_run_utah_tests.py
@@ -29,7 +29,7 @@ server_script_path = os.path.join(os.path.dirname(__file__),
 server_script = imp.load_module('run_utah_tests',
                                 open(server_script_path),
                                 server_script_path,
- ('.py', 'U', 1))
+ ('.py', 'r', 1))

2) A change to iso_static_validation.py to fix the cdimage url used for comparison. Without this change, pointing to a lunar desktop iso generates a file not found error (because self.st_variant was not getting set to equal st_variant):
ERROR: Failed to fetch URL 'http://cdimage.ubuntu.com/daily-legacy/pending/lunar-desktop-amd64.manifest': HTTP Error 404: Not Found

diff --git a/utah/isotest/iso_static_validation.py b/utah/isotest/iso_static_validation.py
index 9123813c..e220a7b9 100755
--- a/utah/isotest/iso_static_validation.py
+++ b/utah/isotest/iso_static_validation.py
@@ -176,7 +176,7 @@ class TestValidateISO(unittest.TestCase):
         self.iso = ISO(image=self.iso_location, logger=logging)
         self.st_release = self.iso.series
         self.st_variant = self.iso.installtype
- if self.st_variant == "desktop" and self.st_release > "kinetic":
+ if self.st_variant == "desktop" and self.st_release < "lunar":
             self.st_variant = "desktop-legacy"
         self.st_arch = self.iso.arch

3) Another change to iso_static_validation.py to remove the .git folder from the secure boot test before doing the sparse checkout, otherwise the test fails because the git checkout fails to reinitialize the git repo.

@@ -563,6 +563,9 @@ class TestValidateISO(unittest.TestCase):
             # so we just pull to ensure it is up to date
             git_keys.pull()
         else:
+ # Remove .git folder before the sparse checkout
+ keys_dot_git = os.path.join(repo_dir, '.git')
+ shutil.rmtree(keys_dot_git)
             git_keys.sparse_checkout(paths=[keys_repo_path])
         cmd = ["make", "-C", keys]
         output = subprocess.Popen(cmd, stdout=subprocess.PIPE,

[Paride Legovini (paride)]

Thanks Heather! Unfortunately that still doesn't seem to be working with legacy images.

I prepared yet another version of the fix, hopefully covering all the cases of interest.
Here is the MP:

https://code.launchpad.net/~paride/utah/+git/utah-1/+merge/437355

[Heather Ellsworth (hellsworth)]

Ah dang, I only tested with the latest lunar desktop iso so my apologies for not catching the mistakes. I can go test MP 437355.

[Sebastien Bacher (seb128)]

Thanks Heather, I've reviewed Paride's merge request but side notes there

1. indeed seems like a pytho3.11 fix which isn't really related to this changeset but needed

2. I think the suggested change isn't correct either because self.st_variant is used to build the iso name and 'desktop-legacy' is part of the name only in Lunar. Paride's variant of change doesn't have that issue though so we can probably consider it resolved if we merge his

3. seems like an improvement worth suggesting but that's an existing problem and not related to the new iso right?

perhaps you could do a merge request with the fix from 3.?

[Paride Legovini (paride)]

I am not sure I understand when the 3. issue happens: I had a look at the code and looks like we *want* that directory to be a git repo? @Heather maybe it ended up somehow corrupted on your system?

Unmerged commits

fb4596f... by Sebastien Bacher on 2023-01-17

Update the ISO tests for the new desktop installer

diff --git a/utah/isotest/data/file_list_udi b/utah/isotest/data/file_list_udi
 new file mode 100644
index 0000000..926c5c3
--- /dev/null
+++ b/utah/isotest/data/file_list_udi
@@ -0,0 +1,18 @@
+boot
+casper
+.disk
+dists
+EFI
+install
+md5sum.txt
+pool
+preseed
+ubuntu
+casper
+casper/install-sources.yaml
+casper/minimal.squashfs
+casper/minimal.standard.squashfs
+casper/vmlinuz
+casper/initrd
+casper/filesystem.manifest
+casper/filesystem.size
diff --git a/utah/isotest/iso_static_validation.py b/utah/isotest/iso_static_validation.py
index 4480d33..9123813 100755
--- a/utah/isotest/iso_static_validation.py
+++ b/utah/isotest/iso_static_validation.py
@@ -124,9 +124,16 @@ else:
         st_variant = "legacy-server"
     st_arch = iso.arch
 
+# Checking the squashfs content to tell legacy vs desktop
+if st_variant == "desktop" and st_release > "kinetic":
+    files = set(iso.listfiles(returnlist=True))
+    if ('install/filesystem.squashfs' in files
+        or './install/filesystem.squashfs' in files):
+         st_variant = "desktop-legacy"
 
 #Set a flag for ubiquity based images
-if st_variant == 'desktop' or st_variant == 'dvd':
+if st_variant == 'desktop-legacy' or st_variant == 'dvd' or \
+    (st_variant == 'desktop' and st_release < 'lunar'):
     ubiquity_image = True
 else:
     ubiquity_image = False
@@ -138,7 +145,7 @@ else:
     subiquity_image = False
 
 #Set a flag for UEFI&SecureBoot
-if (st_variant in ('desktop', 'server', 'live-server', 'dvd')
+if (st_variant in ('desktop', 'desktop-legacy', 'server', 'live-server', 'dvd')
         and st_arch == 'amd64'):
     uefi_image = True
 else:
@@ -169,6 +176,8 @@ class TestValidateISO(unittest.TestCase):
         self.iso = ISO(image=self.iso_location, logger=logging)
         self.st_release = self.iso.series
         self.st_variant = self.iso.installtype
+        if self.st_variant == "desktop" and self.st_release > "kinetic":
+            self.st_variant = "desktop-legacy"
         self.st_arch = self.iso.arch
 
         if self.st_variant == "server" and self.st_release == "focal":
@@ -190,8 +199,10 @@ class TestValidateISO(unittest.TestCase):
         else:
             self.url = DEFAULT_URL
 
-        if self.st_variant == 'desktop':
+        if self.st_variant =='desktop':
             self.url = os.path.join(self.url, 'daily-live')
+        elif self.st_variant =='desktop-legacy':
+            self.url = os.path.join(self.url, 'daily-legacy')
         elif self.st_variant == 'dvd':
             self.url = os.path.join(self.url, 'dvd')
         elif self.st_variant == 'alternate':
@@ -295,8 +306,8 @@ class TestValidateISO(unittest.TestCase):
                           '{!r} not found in:\n{}'
                           .format(fname, stdout))
 
-    @unittest.skipUnless(ubiquity_image,
-                         "manifest only for ubiquity based images")
+    @unittest.skipUnless(ubiquity_image or st_variant == 'desktop',
+                         "manifest only for ubiquity and desktop images")
     def test_manifest(self):
         """Test if the ISO manifest matches the one in the server."""
         manifest_url = os.path.join(
@@ -341,7 +352,7 @@ class TestValidateISO(unittest.TestCase):
             logging.debug('Checking buildids are the same in iso and repo')
             self.assertIn(id_server, build_info_line)
 
-    @unittest.skipUnless(st_variant == 'desktop' and
+    @unittest.skipUnless(ubiquity_image == True and
                          st_arch != 'powerpc',
                          "Skipping images that aren't ubiquity-based.")
     @unittest.skipIf(st_release != 'trusty', "Skipping images without wubi.")
@@ -358,8 +369,8 @@ class TestValidateISO(unittest.TestCase):
         self.assertEqual(stderr, '')
         self.assertIn(" PE32 executable (GUI)", stdout)
 
-    @unittest.skipUnless(st_variant == 'desktop' and
-                         st_arch != 'powerpc', "Skipping for non desktop iso")
+    @unittest.skipUnless(ubiquity_image == True and
+                         st_arch != 'powerpc', "Skipping for non desktop legacy iso")
     def test_files_ubiquity(self):
         """Test if the relevant files are present in the ISO for desktop."""
         (stdout, stderr) = self.iso.listfiles()
@@ -376,9 +387,10 @@ class TestValidateISO(unittest.TestCase):
             else:
                 self.assertIn(path, stdout)
 
-    # Test is skipped for [s]ubiquity based images
+    # Test is skipped for [s]ubiquity based and desktop images
     @unittest.skipIf(ubiquity_image, "Skipping for ubiquity images")
     @unittest.skipIf(subiquity_image, "Skipping for subiquity images")
+    @unittest.skipIf(st_variant == 'desktop', "Skipping for desktop images")
     def test_files_di(self):
         """Test if the relevant files are present for d-i based images ISO.
 
@@ -429,6 +441,19 @@ class TestValidateISO(unittest.TestCase):
             path = list_server.rstrip()
             self.assertIn(path, stdout)
 
+    @unittest.skipUnless(st_variant == 'desktop' and not ubiquity_image, "Skipping for non desktop iso")
+    def test_files_udi(self):
+        """Test if the relevant files are present for u-d-i isos."""
+        (stdout, stderr) = self.iso.listfiles()
+        logging.debug('Check for error in extracting file list from the iso')
+        self.assertEqual(stderr, '')
+        file_list_path = self.get_latest_file_list("file_list_udi")
+        file_list = open(file_list_path)
+        logging.debug('Check if relevant files are present in the iso')
+        for list_server in file_list:
+            path = list_server.rstrip()
+            self.assertIn(path, stdout)
+
     @unittest.skipIf(st_arch in ['powerpc', 'ppc64el'],
                          "vmlinuz is not present only for powerpc images")
     def test_vmlinuz(self):
@@ -456,7 +481,7 @@ class TestValidateISO(unittest.TestCase):
         self.assertEqual(stderr, '')
         self.assertIn(kernel_string, stdout)
 
-    @unittest.skipUnless(ubiquity_image or
+    @unittest.skipUnless(ubiquity_image or st_variant == 'desktop' or
                          (st_variant == 'server' and st_release > 'precise'),
                          "Skipping for d-i images without squashfs.")
     def test_filesystem_squashfs(self):
@@ -468,6 +493,8 @@ class TestValidateISO(unittest.TestCase):
         logging.debug('Extracting the filesystem.squashfs')
         if ubiquity_image:
             squashfs_name = 'casper/filesystem.squashfs'
+        elif st_variant == 'desktop':
+            squashfs_name = 'casper/minimal.squashfs'
         else:
             squashfs_name = 'install/filesystem.squashfs'
 
@@ -484,7 +511,7 @@ class TestValidateISO(unittest.TestCase):
         self.assertIn("Squashfs filesystem", stdout)
 
         # Skipping vmlinuz image extraction on server images
-        if not ubiquity_image:
+        if st_variant == 'server':
             return
 
         #unsquashfs extracts only in pwd
@@ -502,7 +529,7 @@ class TestValidateISO(unittest.TestCase):
 
         logging.debug('Check that vmlinuz could be extracted')
         self.assertEqual(stderr, '')
-        self.assertIn("created 1 directories", stdout)
+        self.assertIn("created 1 directory", stdout)
 
     @unittest.skipUnless(uefi_image,
                          "Test EFI files on ubuntu amd64 images")