From c6e4e18a23506bdbcaf91cde57dd69a606fa0580 Mon Sep 17 00:00:00 2001 From: Aleksa Svitlica Date: Mon, 23 Jan 2023 16:10:41 -0500 Subject: [PATCH] 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 | 23 +++++- britney2/policies/cloud.py | 153 +++++++++++++++++++++---------------- tests/test_cloud.py | 99 +++++++++++++++++------- 3 files changed, 183 insertions(+), 92 deletions(-) diff --git a/britney.conf b/britney.conf index d1c7176..cd15063 100644 --- a/britney.conf +++ b/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 = +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 diff --git a/britney2/policies/cloud.py b/britney2/policies/cloud.py index f97453a..ebbd591 100644 --- a/britney2/policies/cloud.py +++ b/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 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 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"] - 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" - } + DEFAULT_EMAILS = ["cpc@canonical.com"] 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 - 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) + self.package_set = self._retrieve_cloud_package_set_for_series(self.options.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._send_emails_if_needed(item.package, source_data_srcdist.version, self.series, self.pocket) + self._run_cloud_tests(item.package, self.options.series, self.source, self.source_type) - self._cleanup_work_directory() - return PolicyVerdict.PASS + if len(self.failures) > 0 or len(self.errors) > 0: + 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): - """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): + def _run_cloud_tests(self, package, series, source, source_type): """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) - if urn is None: - return + urn = self._retrieve_urn(series) + install_flag = self._determine_install_flag(source_type) - 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), - "--install-archive-package", "{}/{}".format(package, pocket), + "--instance-prefix", "britney-{}-{}-{}".format(package, series, source), + 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 - pocket = self.pocket + series = self.options.series + 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): - shutil.rmtree(self.WORK_DIR) + if os.path.exists(self.work_dir): + shutil.rmtree(self.work_dir) diff --git a/tests/test_cloud.py b/tests/test_cloud.py index 1182295..ac356ad 100644 --- a/tests/test_cloud.py +++ b/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 tests.test_sourceppa import FakeOptions +from britney2.policies.cloud import CloudPolicy, ERR_MESSAGE, MissingURNException 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.pocket = "proposed" + self.policy.options.series = "jammy" + 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.pocket = "proposed" + self.policy.options.series = "jammy" + 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.pocket = "proposed" + self.policy.options.series = "jammy" + 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") - path2 = pathlib.PurePath(CloudPolicy.WORK_DIR, "Test-OtherTests-20230101010101.xml") + path = pathlib.PurePath(self.policy.work_dir, "TEST-FakeTests-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.pocket = "proposed" + self.policy.options.series = "jammy" + 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) - path = pathlib.PurePath(CloudPolicy.WORK_DIR, "TEST-FakeTests-20230101010101.xml") + os.makedirs(self.policy.work_dir, exist_ok=True) + path = pathlib.PurePath(self.policy.work_dir, "TEST-FakeTests-20230101010101.xml") root = ET.Element("testsuite", attrib={"name": "FakeTests-1234567890"})