CloudPolicy: Move hardcoded values to config

Changes made
- Multiple hardcoded fields moved to config
- Series is now retrieved from options
- Pocket is now called source and retrieved from config
- Adds source type config which can be either archive or ppa
- Returns REJECTED_PERMANENTLY policy verdict when test failures or
  errors occur. Adds verdict info the the excuse.

britney.conf

@@ -124,4 +124,25 @@ SRUREGRESSIONEMAIL_ENABLE = no
 PIUPARTS_ENABLE = no
 
 # run cloud tests on packages
-CLOUD_ENABLE = yes
+CLOUD_ENABLE            = no
+# Where the package can be downloaded from in the cloud. Either an archive or ppa.
+# For PPAs it should be defined in the format <ppa-url>=<fingerprint>
+CLOUD_SOURCE            = proposed
+# Can be either 'archive' or 'ppa'
+CLOUD_SOURCE_TYPE       = archive
+# A directory to store Cloud test results and logs. Is created at the start of
+# each policy run and deleted after test results are parsed.
+CLOUD_WORK_DIR          = cloud_tests
+# Who to notify regarding test failures
+CLOUD_FAILURE_EMAILS    = cpc@canonical.com
+# Who to notify regarding test errors
+CLOUD_ERROR_EMAILS      = cpc@canonical.com
+# A set of Azure specific settings
+CLOUD_AZURE_LOCATION    = westeurope
+CLOUD_AZURE_VM_SIZE     = Standard_D2s_v5
+
+CLOUD_AZURE_LUNAR_URN   = Canonical:0001-com-ubuntu-server-lunar-daily:23_04-daily-gen2:23.04.202301030
+CLOUD_AZURE_KINETIC_URN = Canonical:0001-com-ubuntu-server-kinetic:22_10:22.10.202301040
+CLOUD_AZURE_JAMMY_URN   = Canonical:0001-com-ubuntu-server-jammy:22_04-lts-gen2:22.04.202212140
+CLOUD_AZURE_FOCAL_URN   = Canonical:0001-com-ubuntu-server-focal:20_04-lts-gen2:20.04.202212140
+CLOUD_AZURE_BIONIC_URN  = Canonical:UbuntuServer:18_04-lts-gen2:18.04.202212090

britney2/policies/cloud.py

@@ -12,10 +12,13 @@ from britney2 import SuiteClass
 from britney2.policies.policy import BasePolicy
 from britney2.policies import PolicyVerdict
 
+class MissingURNException(Exception):
+    pass
+
 FAIL_MESSAGE = """From: Ubuntu Release Team <noreply+proposed-migration@ubuntu.com>
 To: {recipients}
 X-Proposed-Migration: notice
-Subject: [proposed-migration] {package} {version} in {series}-{pocket} failed Cloud tests.
+Subject: [proposed-migration] {package} {version} in {series}, {source} failed Cloud tests.
 
 Hi,
 
@@ -33,7 +36,7 @@ Regards, Ubuntu Release Team.
 ERR_MESSAGE = """From: Ubuntu Release Team <noreply+proposed-migration@ubuntu.com>
 To: {recipients}
 X-Proposed-Migration: notice
-Subject: [proposed-migration] {package} {version} in {series}-{pocket} had errors running Cloud Tests.
+Subject: [proposed-migration] {package} {version} in {series}, {source} had errors running Cloud Tests.
 
 Hi,
 
@@ -46,16 +49,8 @@ If you have any questions about this email, please ask them in #ubuntu-release c
 Regards, Ubuntu Release Team.
 """
 class CloudPolicy(BasePolicy):
-    WORK_DIR = "cloud_tests"
     PACKAGE_SET_FILE = "cloud_package_set"
-    EMAILS = ["cpc@canonical.com"]
+    DEFAULT_EMAILS = ["cpc@canonical.com"]
-    SERIES_TO_URN = {
-        "lunar": "Canonical:0001-com-ubuntu-server-lunar-daily:23_04-daily-gen2:23.04.202301030",
-        "kinetic": "Canonical:0001-com-ubuntu-server-kinetic:22_10:22.10.202301040",
-        "jammy": "Canonical:0001-com-ubuntu-server-jammy:22_04-lts-gen2:22.04.202212140",
-        "focal": "Canonical:0001-com-ubuntu-server-focal:20_04-lts-gen2:20.04.202212140",
-        "bionic": "Canonical:UbuntuServer:18_04-lts-gen2:18.04.202212090"
-    }
 
     def __init__(self, options, suite_info, dry_run=False):
         super().__init__(
@@ -69,17 +64,20 @@ class CloudPolicy(BasePolicy):
         self.logger.info(
             "Cloud Policy: will send emails to: %s", self.email_host
         )
+        self.work_dir = getattr(self.options, "cloud_work_dir", "cloud_tests")
+        self.failure_emails = getattr(self.options, "cloud_failure_emails", self.DEFAULT_EMAILS)
+        self.error_emails = getattr(self.options, "cloud_error_emails", self.DEFAULT_EMAILS)
+
+        self.source = getattr(self.options, "cloud_source")
+        self.source_type = getattr(self.options, "cloud_source_type")
+
         self.failures = {}
         self.errors = {}
 
     def initialise(self, britney):
         super().initialise(britney)
 
-        primary_suite = self.suite_info.primary_source_suite
+        self.package_set = self._retrieve_cloud_package_set_for_series(self.options.series)
-        series, pocket = self._retrieve_series_and_pocket_from_path(primary_suite.path)
-        self.series = series
-        self.pocket = pocket
-        self.package_set = self._retrieve_cloud_package_set_for_series(self.series)
 
     def apply_src_policy_impl(self, policy_info, item, source_data_tdist, source_data_srcdist, excuse):
         if item.package not in self.package_set:
@@ -87,7 +85,7 @@ class CloudPolicy(BasePolicy):
 
         if self.dry_run:
             self.logger.info(
-                "Cloud Policy: Dry run would test {} in {}-{}".format(item.package , self.series, self.pocket)
+                "Cloud Policy: Dry run would test {} in {}, {}".format(item.package , self.options.series, self.source)
             )
             return PolicyVerdict.PASS
 
@@ -95,11 +93,19 @@ class CloudPolicy(BasePolicy):
         self.failures = {}
         self.errors = {}
 
-        self._run_cloud_tests(item.package, self.series, self.pocket)
+        self._run_cloud_tests(item.package, self.options.series, self.source, self.source_type)
-        self._send_emails_if_needed(item.package, source_data_srcdist.version, self.series, self.pocket)
 
-        self._cleanup_work_directory()
+        if len(self.failures) > 0 or len(self.errors) > 0:
-        return PolicyVerdict.PASS
+            self._send_emails_if_needed(item.package, source_data_srcdist.version, self.options.series, self.source)
+
+            self._cleanup_work_directory()
+            verdict = PolicyVerdict.REJECTED_PERMANENTLY
+            info = self._generate_verdict_info(self.failures, self.errors)
+            excuse.add_verdict_info(verdict, info)
+            return verdict
+        else:
+            self._cleanup_work_directory()
+            return PolicyVerdict.PASS
 
     def _retrieve_cloud_package_set_for_series(self, series):
         """Retrieves a set of packages for the given series in which cloud
@@ -118,44 +124,27 @@ class CloudPolicy(BasePolicy):
 
         return package_set
 
-    def _retrieve_series_and_pocket_from_path(self, suite_path):
+    def _run_cloud_tests(self, package, series, source, source_type):
-        """Given the suite path return a tuple of series and pocket.
-        With input 'data/jammy-proposed' the expected output is a tuple of
-        (jammy, proposed)
-
-        :param suite_path The path attribute of the suite
-        """
-        result = (None, None)
-        match = re.search("([a-z]*)-([a-z]*)$", suite_path)
-
-        if(match):
-            result = match.groups()
-        else:
-            raise RuntimeError(
-                "Could not determine series and pocket from the path: %s" %suite_path
-            )
-        return result
-
-    def _run_cloud_tests(self, package, series, pocket):
         """Runs any cloud tests for the given package.
-        Returns a list of failed tests or an empty list if everything passed.
+        Nothing is returned but test failures and errors are stored in instance variables.
 
         :param package The name of the package to test
         :param series The Ubuntu codename for the series (e.g. jammy)
-        :param pocket The name of the pocket the package should be installed from (e.g. proposed)
+        :param source Where the package should be installed from (e.g. proposed or a PPA)
+        :param source_type Either 'archive' or 'ppa'
         """
-        self._run_azure_tests(package, series, pocket)
+        self._run_azure_tests(package, series, source, source_type)
 
-    def _send_emails_if_needed(self, package, version, series, pocket):
+    def _send_emails_if_needed(self, package, version, series, source):
         """Sends email(s) if there are test failures and/or errors
 
         :param package The name of the package that was tested
         :param version The version number of the package
         :param series The Ubuntu codename for the series (e.g. jammy)
-        :param pocket The name of the pocket the package should be installed from (e.g. proposed)
+        :param source Where the package should be installed from (e.g. proposed or a PPA)
         """
         if len(self.failures) > 0:
-            emails = self.EMAILS
+            emails = self.failure_emails
             message = self._format_email_message(
                 FAIL_MESSAGE, emails, package, version, self.failures
             )
@@ -163,51 +152,64 @@ class CloudPolicy(BasePolicy):
             self._send_email(emails, message)
 
         if len(self.errors) > 0:
-            emails = self.EMAILS
+            emails = self.error_emails
             message = self._format_email_message(
                 ERR_MESSAGE, emails, package, version, self.errors
             )
             self.logger.info("Cloud Policy: Sending error email for {}, to {}".format(package, emails))
             self._send_email(emails, message)
 
-    def _run_azure_tests(self, package, series, pocket):
+    def _run_azure_tests(self, package, series, source, source_type):
         """Runs Azure's required package tests.
 
         :param package The name of the package to test
         :param series The Ubuntu codename for the series (e.g. jammy)
-        :param pocket The name of the pocket the package should be installed from (e.g. proposed)
+        :param source Where the package should be installed from (e.g. proposed or a PPA)
+        :param source_type Either 'archive' or 'ppa'
         """
-        urn = self.SERIES_TO_URN.get(series, None)
+        urn = self._retrieve_urn(series)
-        if urn is None:
+        install_flag = self._determine_install_flag(source_type)
-            return
 
-        self.logger.info("Cloud Policy: Running Azure tests for: {} in {}-{}".format(package, series, pocket))
+        self.logger.info("Cloud Policy: Running Azure tests for: {} in {}, {}".format(package, series, source))
         subprocess.run(
             [
                 "/snap/bin/cloud-test-framework",
-                "--instance-prefix", "britney-{}-{}-{}".format(package, series, pocket),
+                "--instance-prefix", "britney-{}-{}-{}".format(package, series, source),
-                "--install-archive-package", "{}/{}".format(package, pocket),
+                install_flag, "{}/{}".format(package, source),
                 "azure_gen2",
                 "--location", "westeurope",
                 "--vm-size", "Standard_D2s_v5",
                 "--urn", urn,
                 "run-test", "package-install-with-reboot",
             ],
-            cwd=self.WORK_DIR
+            cwd=self.work_dir
         )
 
         results_file_paths = self._find_results_files(r"TEST-NetworkTests-[0-9]*.xml")
         self._parse_xunit_test_results("Azure", results_file_paths)
 
+    def _retrieve_urn(self, series):
+        """Retrieves an URN from the configuration options based on series.
+        An URN identifies a unique image in Azure.
+
+        :param series The ubuntu codename for the series (e.g. jammy)
+        """
+        urn = getattr(self.options, "cloud_azure_{}_urn".format(series), None)
+
+        if urn is None:
+            raise MissingURNException("No URN configured for {}".format(series))
+
+        return urn
+
     def _find_results_files(self, file_regex):
         """Find any test results files that match the given regex pattern.
 
         :param file_regex A regex pattern to use for matching the name of the results file.
         """
         file_paths = []
-        for file in os.listdir(self.WORK_DIR):
+        for file in os.listdir(self.work_dir):
             if re.fullmatch(file_regex, file):
-                file_paths.append(PurePath(self.WORK_DIR, file))
+                file_paths.append(PurePath(self.work_dir, file))
 
         return file_paths
 
@@ -215,7 +217,7 @@ class CloudPolicy(BasePolicy):
         """Parses and stores any failure or error test results.
 
         :param cloud The name of the cloud, use for storing the results.
-        :results_file_paths List of paths to results files
+        :param results_file_paths List of paths to results files
         """
         for file_path in results_file_paths:
             with open(file_path) as file:
@@ -250,11 +252,11 @@ class CloudPolicy(BasePolicy):
 
     def _format_email_message(self, template, emails, package, version, test_results):
         """Insert given parameters into the email template."""
-        series = self.series
+        series = self.options.series
-        pocket = self.pocket
+        source = self.source
         results = json.dumps(test_results, indent=4)
         recipients = ", ".join(emails)
-        message= template.format(**locals())
+        message = template.format(**locals())
 
         return message
 
@@ -272,14 +274,35 @@ class CloudPolicy(BasePolicy):
             self.logger.error("Cloud Policy: Failed to send mail! Is SMTP server running?")
             self.logger.error(err)
 
+    def _generate_verdict_info(self, failures, errors):
+        info = ""
+
+        if len(failures) > 0:
+            fail_clouds = ",".join(list(failures.keys()))
+            info += "Cloud testing failed for {}.".format(fail_clouds)
+
+        if len(errors) > 0:
+            error_clouds = ",".join(list(errors.keys()))
+            info += " Cloud testing had errors for {}.".format(error_clouds)
+
+        return info
+
+    def _determine_install_flag(self, source_type):
+        if source_type == "archive":
+            return "--install-archive-package"
+        elif source_type == "ppa":
+            return "--install-ppa-package"
+        else:
+            raise RuntimeError("Cloud Policy: Unexpected source type, {}".format(source_type))
+
     def _setup_work_directory(self):
         """Create a directory for tests to be run in."""
         self._cleanup_work_directory()
 
-        os.makedirs(self.WORK_DIR)
+        os.makedirs(self.work_dir)
 
     def _cleanup_work_directory(self):
         """Delete the the directory used for running tests."""
-        if os.path.exists(self.WORK_DIR):
+        if os.path.exists(self.work_dir):
-            shutil.rmtree(self.WORK_DIR)
+            shutil.rmtree(self.work_dir)
 

tests/test_cloud.py

@@ -17,8 +17,7 @@ import xml.etree.ElementTree as ET
 PROJECT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 sys.path.insert(0, PROJECT_DIR)
 
-from britney2.policies.cloud import CloudPolicy, ERR_MESSAGE
+from britney2.policies.cloud import CloudPolicy, ERR_MESSAGE, MissingURNException
-from tests.test_sourceppa import FakeOptions
 
 class FakeItem:
     package = "chromium-browser"
@@ -27,6 +26,15 @@ class FakeItem:
 class FakeSourceData:
     version = "55.0"
 
+class FakeOptions:
+    distribution = "testbuntu"
+    series = "zazzy"
+    unstable = "/tmp"
+    verbose = False
+    cloud_source = "zazzy-proposed"
+    cloud_source_type = "archive"
+    cloud_azure_zazzy_urn = "fake-urn-value"
+
 class T(unittest.TestCase):
     def setUp(self):
         self.policy = CloudPolicy(FakeOptions, {})
@@ -35,38 +43,29 @@ class T(unittest.TestCase):
     def tearDown(self):
         self.policy._cleanup_work_directory()
 
-    def test_retrieve_series_and_pocket_from_path(self):
-        """Retrieves the series and pocket from the suite path.
-        Ensure an exception is raised if the regex fails to match.
-        """
-        result = self.policy._retrieve_series_and_pocket_from_path("data/jammy-proposed")
-        self.assertTupleEqual(result, ("jammy", "proposed"))
-
-        self.assertRaises(
-            RuntimeError, self.policy._retrieve_series_and_pocket_from_path, "data/badpath"
-        )
-
     @patch("britney2.policies.cloud.CloudPolicy._run_cloud_tests")
     def test_run_cloud_tests_called_for_package_in_manifest(self, mock_run):
         """Cloud tests should run for a package in the cloud package set.
         """
         self.policy.package_set = set(["chromium-browser"])
-        self.policy.series = "jammy"
+        self.policy.options.series = "jammy"
-        self.policy.pocket = "proposed"
+        self.policy.source = "jammy-proposed"
 
         self.policy.apply_src_policy_impl(
             None, FakeItem, None, FakeSourceData, None
         )
 
-        mock_run.assert_called_once_with("chromium-browser", "jammy", "proposed")
+        mock_run.assert_called_once_with(
+            "chromium-browser", "jammy", "jammy-proposed", "archive"
+        )
 
     @patch("britney2.policies.cloud.CloudPolicy._run_cloud_tests")
     def test_run_cloud_tests_not_called_for_package_not_in_manifest(self, mock_run):
         """Cloud tests should not run for packages not in the cloud package set"""
 
         self.policy.package_set = set(["vim"])
-        self.policy.series = "jammy"
+        self.policy.options.series = "jammy"
-        self.policy.pocket = "proposed"
+        self.policy.source = "jammy-proposed"
 
         self.policy.apply_src_policy_impl(
             None, FakeItem, None, FakeSourceData, None
@@ -79,8 +78,8 @@ class T(unittest.TestCase):
     def test_no_tests_run_during_dry_run(self, mock_run, smtp):
         self.policy = CloudPolicy(FakeOptions, {}, dry_run=True)
         self.policy.package_set = set(["chromium-browser"])
-        self.policy.series = "jammy"
+        self.policy.options.series = "jammy"
-        self.policy.pocket = "proposed"
+        self.policy.source = "jammy-proposed"
 
         self.policy.apply_src_policy_impl(
             None, FakeItem, None, FakeSourceData, None
@@ -91,8 +90,8 @@ class T(unittest.TestCase):
 
     def test_finding_results_file(self):
         """Ensure result file output from Cloud Test Framework can be found"""
-        path = pathlib.PurePath(CloudPolicy.WORK_DIR, "TEST-FakeTests-20230101010101.xml")
+        path = pathlib.PurePath(self.policy.work_dir, "TEST-FakeTests-20230101010101.xml")
-        path2 = pathlib.PurePath(CloudPolicy.WORK_DIR, "Test-OtherTests-20230101010101.xml")
+        path2 = pathlib.PurePath(self.policy.work_dir, "Test-OtherTests-20230101010101.xml")
         with open(path, "a"): pass
         with open(path2, "a"): pass
 
@@ -128,14 +127,62 @@ class T(unittest.TestCase):
                 "failing_test2": "Error reason 2"
             }
         }
-        self.policy.series = "jammy"
+        self.policy.options.series = "jammy"
-        self.policy.pocket = "proposed"
+        self.policy.source = "jammy-proposed"
         message = self.policy._format_email_message(ERR_MESSAGE, ["work@canonical.com"], "vim", "9.0", failures)
 
         self.assertIn("To: work@canonical.com", message)
         self.assertIn("vim 9.0", message)
         self.assertIn("Error reason 2", message)
 
+    def test_urn_retrieval(self):
+        """Test that URN retrieval throws the expected error when not configured."""
+        self.assertRaises(
+            MissingURNException, self.policy._retrieve_urn, "jammy"
+        )
+
+        urn = self.policy._retrieve_urn("zazzy")
+        self.assertEqual(urn, "fake-urn-value")
+
+    def test_generation_of_verdict_info(self):
+        """Test that the verdict info correctly states which clouds had failures and/or errors"""
+        failures = {
+            "cloud1": {
+                "test_name1": "message1",
+                "test_name2": "message2"
+            },
+            "cloud2": {
+                "test_name3": "message3"
+            }
+        }
+
+        errors = {
+            "cloud1": {
+                "test_name4": "message4",
+            },
+            "cloud3": {
+                "test_name5": "message5"
+            }
+        }
+
+        info = self.policy._generate_verdict_info(failures, errors)
+
+        expected_failure_info = "Cloud testing failed for cloud1,cloud2."
+        expected_error_info = "Cloud testing had errors for cloud1,cloud3."
+
+        self.assertIn(expected_failure_info, info)
+        self.assertIn(expected_error_info, info)
+
+    def test_determine_install_flag(self):
+        """Ensure the correct flag is determined and errors are raised for unknown source types"""
+        install_flag = self.policy._determine_install_flag("archive")
+        self.assertEqual(install_flag, "--install-archive-package")
+
+        install_flag = self.policy._determine_install_flag("ppa")
+        self.assertEqual(install_flag, "--install-ppa-package")
+
+        self.assertRaises(RuntimeError, self.policy._determine_install_flag, "something")
+
     def _create_fake_test_result_file(self, num_pass=1, num_err=0, num_fail=0):
         """Helper function to generate an xunit test result file.
 
@@ -145,8 +192,8 @@ class T(unittest.TestCase):
 
         Returns the path to the created file.
         """
-        os.makedirs(CloudPolicy.WORK_DIR, exist_ok=True)
+        os.makedirs(self.policy.work_dir, exist_ok=True)
-        path = pathlib.PurePath(CloudPolicy.WORK_DIR, "TEST-FakeTests-20230101010101.xml")
+        path = pathlib.PurePath(self.policy.work_dir, "TEST-FakeTests-20230101010101.xml")
 
         root = ET.Element("testsuite", attrib={"name": "FakeTests-1234567890"})