diff --git a/doozerlib/assembly.py b/doozerlib/assembly.py index d274b00c6..f2033b52a 100644 --- a/doozerlib/assembly.py +++ b/doozerlib/assembly.py @@ -66,6 +66,11 @@ class AssemblyIssueCode(Enum): # in x86_64 RHCOS have different versions FAILED_CROSS_RPM_VERSIONS_REQUIREMENT = 9 + # An rpm defined in rpm_deliveries group config is missing the ship-ok Brew tag. + # Note this issue code only occurs in pre-GA releases (ECs or RCs). + # For GA releases, IMPERMISSIBLE code will be reported. + MISSING_SHIP_OK_TAG = 10 + class AssemblyIssue: """ diff --git a/doozerlib/assembly_inspector.py b/doozerlib/assembly_inspector.py index adb6ad92d..b619f5988 100644 --- a/doozerlib/assembly_inspector.py +++ b/doozerlib/assembly_inspector.py @@ -1,9 +1,10 @@ -from typing import List, Dict, Optional +from typing import Any, List, Dict, Optional from koji import ClientSession +from doozerlib.model import Model from doozerlib.rpm_utils import parse_nvr -from doozerlib import util, Runtime +from doozerlib import brew, util, Runtime from doozerlib.image import BrewBuildImageInspector from doozerlib.rpmcfg import RPMMetadata from doozerlib.assembly import assembly_rhcos_config, AssemblyTypes, assembly_permits, AssemblyIssue, \ @@ -45,6 +46,21 @@ def __init__(self, runtime: Runtime, brew_session: ClientSession = None, lookup_ else: self._release_image_inspectors[image_meta.distgit_key] = None + # Preprocess rpm_deliveries group config + # This is mainly to support weekly kernel delivery + self._rpm_deliveries: Dict[str, Model] = {} # Dict[package_name] => per package rpm_delivery config + if self.runtime.group_config.rpm_deliveries: + for entry in self.runtime.group_config.rpm_deliveries: + packages = entry.packages + if not packages: + raise ValueError("`packages` in `group_config.rpm_deliveries` can't be empty") + if not entry.ship_ok_tag: + raise ValueError("`ship_ok_tag` in `group_config.rpm_deliveries` can't be empty") + if not entry.stop_ship_tag: + raise ValueError("`stop_ship_tag` in `group_config.rpm_deliveries` can't be empty") + for package in packages: + self._rpm_deliveries[package] = entry + def get_type(self) -> AssemblyTypes: return self.assembly_type @@ -63,12 +79,55 @@ def does_permit(self, issue: AssemblyIssue) -> bool: return True return False + def _check_installed_packages_for_rpm_delivery(self, component: str, component_description: str, rpm_packages: Dict[str, Dict[str, Any]]): + """ + If rpm_deliveries is configured, this checks installed packages in a component + follow the stop-ship/ship-ok tagging requirement. + See https://issues.redhat.com/browse/ART-6100 for more information. + + :param component: Component name, like `rhcos` or image distgit key + :param component_description: Component description, used to include more details for the component in the AssemblyIssue message + :param rpm_packages: A dict; key is rpm package name, value is brew build dict + """ + self.runtime.logger.info("Checking installed rpms in %s", component_description) + issues: List[AssemblyIssue] = [] + for package_name, rpm_build in rpm_packages.items(): + if package_name in self._rpm_deliveries: + rpm_delivery_config = self._rpm_deliveries[package_name] + self.runtime.logger.info("Getting tags for rpm build %s...", rpm_build['nvr']) + tag_names = {tag["name"] for tag in self.brew_session.listTags(brew.KojiWrapperOpts(caching=True), build=rpm_build["id"])} + # If the rpm is tagged into the stop-ship tag, it is never permissible + if rpm_delivery_config.stop_ship_tag and rpm_delivery_config.stop_ship_tag in tag_names: + issues.append(AssemblyIssue(f'{component_description} has {rpm_build["nvr"]}, which has been tagged into the stop-ship tag: {rpm_delivery_config.stop_ship_tag}', component=component, code=AssemblyIssueCode.IMPERMISSIBLE)) + continue + # For GA releases, the rpm must have an explicit "ship-ok" tag + if self.assembly_type is AssemblyTypes.STANDARD and rpm_delivery_config.ship_ok_tag and rpm_delivery_config.ship_ok_tag not in tag_names: + issues.append(AssemblyIssue(f'{component_description} has {rpm_build["nvr"]}, which is missing the ship-ok tag: {rpm_delivery_config.ship_ok_tag}', component=component, code=AssemblyIssueCode.IMPERMISSIBLE)) + # For pre-GA releases, MISSING_SHIP_OK_TAG issue will be reported if the rpm doesn't have a "ship-ok" tag + if self.assembly_type is AssemblyTypes.CANDIDATE and rpm_delivery_config.ship_ok_tag and rpm_delivery_config.ship_ok_tag not in tag_names: + issues.append(AssemblyIssue(f'{component_description} has {rpm_build["nvr"]}, which is missing the ship-ok tag: {rpm_delivery_config.ship_ok_tag}', component=component, code=AssemblyIssueCode.MISSING_SHIP_OK_TAG)) + return issues + + def check_installed_rpms_in_image(self, dg_key: str, build_inspector: BrewBuildImageInspector): + """ Analyzes an image build to check if installed packages are allowed to assemble. + """ + issues: List[AssemblyIssue] = [] + self.runtime.logger.info("Getting rpms in image build %s...", build_inspector.get_nvr()) + installed_packages = build_inspector.get_all_installed_package_build_dicts() + # If rpm_deliveries is configured, check if the image build has rpms respecting the stop-ship/ship-ok tag + issues.extend(self._check_installed_packages_for_rpm_delivery(dg_key, f'Image build {build_inspector.get_nvr()}', installed_packages)) + return issues + def check_rhcos_issues(self, rhcos_build: RHCOSBuildInspector) -> List[AssemblyIssue]: """ Analyzes an RHCOS build to check whether the installed packages are consistent with: 1. package NVRs defined at the group dependency level 2. package NVRs defined at the rhcos dependency level 3. package NVRs of any RPMs built in this assembly/group + + If rpm_deliveries is defined in group config, this will also check + whether the installed packages meet the tagging requirements like "stop-ship" and "ship-ok". + :param rhcos_build: The RHCOS build to analyze. :return: Returns a (potentially empty) list of inconsistencies in the build. """ @@ -77,6 +136,14 @@ def check_rhcos_issues(self, rhcos_build: RHCOSBuildInspector) -> List[AssemblyI issues: List[AssemblyIssue] = [] required_packages: Dict[str, str] = dict() # Dict[package_name] -> nvr # Dependency specified in 'rhcos' in assembly definition desired_packages: Dict[str, str] = dict() # Dict[package_name] -> nvr # Dependency specified at group level + installed_packages: Dict[str, Dict[str, Any]] = dict() # Dict[package_name] -> build dict # rpms installed in the rhcos image + + self.runtime.logger.info("Getting rpms in RHCOS build %s...", rhcos_build.build_id) + installed_packages = rhcos_build.get_package_build_objects() + + # If rpm_deliveries is configured, check if the RHCOS build has rpms with the stop-ship/ship-ok tag + issues.extend(self._check_installed_packages_for_rpm_delivery('rhcos', f'RHCOS build {rhcos_build.build_id} ({rhcos_build.brew_arch})', installed_packages)) + el_tag = f'el{rhcos_build.get_rhel_base_version()}' for package_entry in (self.runtime.get_group_config().dependencies or []): if el_tag in package_entry: @@ -91,7 +158,6 @@ def check_rhcos_issues(self, rhcos_build: RHCOSBuildInspector) -> List[AssemblyI required_packages[package_name] = nvr desired_packages[package_name] = nvr # Override if something else was at the group level - installed_packages = rhcos_build.get_package_build_objects() for package_name, desired_nvr in desired_packages.items(): if package_name in required_packages and package_name not in installed_packages: diff --git a/doozerlib/cli/__main__.py b/doozerlib/cli/__main__.py index a7117db67..f5e2ff9bc 100644 --- a/doozerlib/cli/__main__.py +++ b/doozerlib/cli/__main__.py @@ -1660,7 +1660,7 @@ def release_calc_previous(version, arch, graph_url, graph_content_stable, graph_ help="Arch for which the repo should be generated (if not specified, use all runtime arches).", default=None, required=False) @pass_runtime -def config_rhcos_src(runtime, version, output, brew_root, arch): +def config_rhcos_src(runtime: Runtime, version, output, brew_root, arch): runtime.initialize(clone_distgits=False) package_build_objects: Dict[str, Dict] = dict() diff --git a/doozerlib/cli/release_gen_payload.py b/doozerlib/cli/release_gen_payload.py index 1f01dbd87..1d2a4cfe1 100644 --- a/doozerlib/cli/release_gen_payload.py +++ b/doozerlib/cli/release_gen_payload.py @@ -365,6 +365,9 @@ async def generate_assembly_issues_report(self, assembly_inspector: AssemblyInsp # check that images for this assembly/group are consistent with the assembly definition. self.detect_inconsistent_images(assembly_inspector) + # check that images for this assembly/group have installed rpms that are not allowed to assemble. + self.detect_installed_rpms_issues(assembly_inspector) + # update issues found for payload images and check RPM consistency self.detect_extend_payload_entry_issues(assembly_inspector) @@ -499,6 +502,15 @@ def detect_inconsistent_images(self, assembly_inspector: AssemblyInspector): if bbii: self.assembly_issues.extend(assembly_inspector.check_group_image_consistency(bbii)) + def detect_installed_rpms_issues(self, assembly_inspector: AssemblyInspector): + """ + Create issues for image builds with installed rpms + """ + self.logger.debug("Detecting issues with installed rpms...") + for dg_key, bbii in assembly_inspector.get_group_release_images().items(): + if bbii: + self.assembly_issues.extend(assembly_inspector.check_installed_rpms_in_image(dg_key, bbii)) + def full_component_repo(self) -> str: """ Full pullspec for the component repo diff --git a/doozerlib/rhcos.py b/doozerlib/rhcos.py index 53094a90c..99ee8f205 100644 --- a/doozerlib/rhcos.py +++ b/doozerlib/rhcos.py @@ -7,6 +7,7 @@ from tenacity import retry, stop_after_attempt, wait_fixed from urllib import request from urllib.error import URLError +from doozerlib.runtime import Runtime from doozerlib.util import brew_suffix_for_arch, isolate_el_version_in_release from doozerlib import exectools from doozerlib.model import ListModel, Model @@ -231,7 +232,7 @@ def latest_container(self, container_conf: dict = None) -> Tuple[Optional[str], class RHCOSBuildInspector: - def __init__(self, runtime, pullspec_for_tag: Dict[str, str], brew_arch: str, build_id: Optional[str] = None): + def __init__(self, runtime: Runtime, pullspec_for_tag: Dict[str, str], brew_arch: str, build_id: Optional[str] = None): self.runtime = runtime self.brew_arch = brew_arch self.pullspec_for_tag = pullspec_for_tag @@ -352,20 +353,33 @@ def get_package_build_objects(self) -> Dict[str, Dict]: """ aggregate: Dict[str, Dict] = dict() + rpms = self.get_rpm_nvras() with self.runtime.pooled_koji_client_session() as koji_api: - for nvra in self.get_rpm_nvras(): + self.runtime.logger.info("Getting %s rpm(s) from Brew...", len(rpms)) + tasks = [] + with koji_api.multicall(strict=False) as m: # strict=False means don't raise the first error it encounters + for nvra in rpms: + tasks.append(m.getRPM(nvra, brew.KojiWrapperOpts(caching=True), strict=True)) + rpm_defs = [] + for task in tasks: try: - rpm_def = koji_api.getRPM(nvra, strict=True) + rpm_defs.append(task.result) except koji.GenericError as e: + nvra = task.args[0] if self.runtime.group_config.rhcos.allow_missing_brew_rpms and "No such rpm" in str(e): self.runtime.logger.warning("Failed to find RPM %s in Brew", nvra, exc_info=True) continue # if conigured, just skip RPMs brew doesn't know about raise Exception(f"Failed to find RPM {nvra} in brew: {e}") - package_build = koji_api.getBuild(rpm_def['build_id'], brew.KojiWrapperOpts(caching=True), strict=True) + self.runtime.logger.info("Getting build infos for %s rpm(s)...", len(rpm_defs)) + tasks = [] + with koji_api.multicall(strict=True) as m: + for rpm_def in rpm_defs: + tasks.append(m.getBuild(rpm_def['build_id'], brew.KojiWrapperOpts(caching=True), strict=True)) + for task in tasks: + package_build = task.result package_name = package_build['package_name'] aggregate[package_name] = package_build - return aggregate def get_primary_container_conf(self): diff --git a/requirements-dev.txt b/requirements-dev.txt index 5e22744bf..32e37be1b 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,4 +1,3 @@ -asynctest autopep8 coverage flake8 diff --git a/tests/cli/test_gen_payload.py b/tests/cli/test_gen_payload.py index af1127136..d4920c57a 100644 --- a/tests/cli/test_gen_payload.py +++ b/tests/cli/test_gen_payload.py @@ -125,6 +125,7 @@ async def test_generate_assembly_issues_report(self, cnc_mock): detect_mismatched_siblings=None, detect_non_latest_rpms=None, detect_inconsistent_images=None, + detect_installed_rpms_issues=None, detect_extend_payload_entry_issues=None, summarize_issue_permits=(True, {}), ) @@ -711,3 +712,17 @@ def contains(name: str): self.assertEqual('false', new_tag_annotations['release.openshift.io/rewrite']) self.assertEqual(os.getenv('BUILD_URL', ''), new_tag_annotations['release.openshift.io/build-url']) self.assertIn('release.openshift.io/runtime-brew-event', new_tag_annotations) + + def test_rpm_deliveries(self): + gpcli = rgp_cli.GenPayloadCli(output_dir="/tmp", runtime=MagicMock(assembly_type=AssemblyTypes.STREAM)) + ai = MagicMock(spec=AssemblyInspector) + ai.get_group_release_images.return_value = dict( + foo=Mock(spec=BrewBuildImageInspector), + bar=Mock(spec=BrewBuildImageInspector), + ) + ai.check_installed_rpms_in_image.side_effect = lambda dg_key, bi: { + "foo": [], + "bar": [Mock(AssemblyIssue, code=AssemblyIssueCode.MISSING_SHIP_OK_TAG, component="bar", msg="")] + }[dg_key] + gpcli.detect_installed_rpms_issues(ai) + self.assertEqual(len(gpcli.assembly_issues), 1) diff --git a/tests/cli/test_rpms_build.py b/tests/cli/test_rpms_build.py index afb8ac6f0..5bc8e8aef 100644 --- a/tests/cli/test_rpms_build.py +++ b/tests/cli/test_rpms_build.py @@ -1,7 +1,6 @@ -import logging import io - -import asynctest +import logging +from unittest import IsolatedAsyncioTestCase from unittest.mock import AsyncMock, MagicMock, Mock, patch from doozerlib import gitdata, rpmcfg @@ -9,7 +8,7 @@ from doozerlib.exectools import RetryException -class TestRPMsBuildCli(asynctest.TestCase): +class TestRPMsBuildCli(IsolatedAsyncioTestCase): def _make_runtime(self, assembly=None): runtime = MagicMock() diff --git a/tests/test_assembly_inspector.py b/tests/test_assembly_inspector.py new file mode 100644 index 000000000..3e76aab91 --- /dev/null +++ b/tests/test_assembly_inspector.py @@ -0,0 +1,85 @@ + +from unittest import IsolatedAsyncioTestCase +from unittest.mock import MagicMock +from doozerlib.assembly import AssemblyTypes + +from doozerlib.assembly_inspector import AssemblyInspector +from doozerlib.model import Model + + +class TestAssemblyInspector(IsolatedAsyncioTestCase): + def test_check_installed_packages_for_rpm_delivery(self): + # Test a standard release with a package tagged into my-ship-ok-tag + rt = MagicMock(mode="both", group_config=Model({ + "rpm_deliveries": [ + { + "packages": ["kernel", "kernel-rt"], + "integration_tag": "my-integration-tag", + "ship_ok_tag": "my-ship-ok-tag", + "stop_ship_tag": "my-stop-ship-tag" + } + ] + })) + brew_session = MagicMock() + brew_session.listTags.return_value = [ + {"name": "tag-a"}, + {"name": "my-integration-tag"}, + {"name": "my-ship-ok-tag"}, + ] + ai = AssemblyInspector(rt, brew_session) + ai.assembly_type = AssemblyTypes.STANDARD + rpm_packages = { + "kernel": {"nvr": "kernel-1.2.3-1", "id": 1} + } + issues = ai._check_installed_packages_for_rpm_delivery("foo", "foo-1.2.3-1", rpm_packages) + self.assertEqual(issues, []) + + # Test a standard release with a package not tagged into my-ship-ok-tag + brew_session.listTags.return_value = [ + {"name": "tag-a"}, + {"name": "my-integration-tag"}, + ] + issues = ai._check_installed_packages_for_rpm_delivery("foo", "foo-1.2.3-1", rpm_packages) + self.assertEqual(len(issues), 1) + + # Test a stream "release" with a package not tagged into my-ship-ok-tag + ai.assembly_type = AssemblyTypes.STREAM + issues = ai._check_installed_packages_for_rpm_delivery("foo", "foo-1.2.3-1", rpm_packages) + self.assertEqual(len(issues), 0) + + # Test a stream "release" with a package not tagged into my-stop-ship-tag + brew_session.listTags.return_value = [ + {"name": "tag-a"}, + {"name": "my-integration-tag"}, + {"name": "my-ship-ok-tag"}, + {"name": "my-stop-ship-tag"}, + ] + issues = ai._check_installed_packages_for_rpm_delivery("foo", "foo-1.2.3-1", rpm_packages) + self.assertEqual(len(issues), 1) + + def test_check_installed_rpms_in_image(self): + rt = MagicMock(mode="both", group_config=Model({ + "rpm_deliveries": [ + { + "packages": ["kernel", "kernel-rt"], + "integration_tag": "my-integration-tag", + "ship_ok_tag": "my-ship-ok-tag", + "stop_ship_tag": "my-stop-ship-tag" + } + ] + })) + brew_session = MagicMock() + brew_session.listTags.return_value = [ + {"name": "tag-a"}, + {"name": "my-integration-tag"}, + {"name": "my-ship-ok-tag"}, + ] + ai = AssemblyInspector(rt, brew_session) + ai.assembly_type = AssemblyTypes.STANDARD + build_inspector = MagicMock() + build_inspector.get_all_installed_package_build_dicts.return_value = { + "kernel": {"nvr": "kernel-1.2.3-1", "id": 1} + } + issues = ai.check_installed_rpms_in_image("foo", build_inspector) + self.assertEqual(issues, []) + pass diff --git a/tests/test_distgit/support.py b/tests/test_distgit/support.py index d86625f31..e52190d5c 100644 --- a/tests/test_distgit/support.py +++ b/tests/test_distgit/support.py @@ -1,4 +1,4 @@ -import asynctest +from unittest import IsolatedAsyncioTestCase from future import standard_library from doozerlib.assembly import AssemblyTypes @@ -85,7 +85,7 @@ def __init__(self): self.files = [] -class TestDistgit(asynctest.TestCase): +class TestDistgit(IsolatedAsyncioTestCase): """ Test the methods and functions used to manage and update distgit repos """ diff --git a/tests/test_exectools.py b/tests/test_exectools.py index 8fcab4b51..4ebc6e28a 100644 --- a/tests/test_exectools.py +++ b/tests/test_exectools.py @@ -7,8 +7,7 @@ import asyncio import unittest -import asynctest -from unittest import mock +from unittest import IsolatedAsyncioTestCase, mock try: from importlib import reload @@ -22,7 +21,7 @@ from doozerlib import exectools, assertion -class RetryTestCase(asynctest.TestCase): +class RetryTestCase(IsolatedAsyncioTestCase): """ Test the exectools.retry() method """ @@ -128,7 +127,7 @@ def test_cmd_assert_fail(self): self.assertEqual(len(lines), 12) -class TestGather(asynctest.TestCase): +class TestGather(IsolatedAsyncioTestCase): """ """ diff --git a/tests/test_osbs2.py b/tests/test_osbs2.py index 5cd212f15..7795d2917 100644 --- a/tests/test_osbs2.py +++ b/tests/test_osbs2.py @@ -1,8 +1,6 @@ import unittest from unittest.mock import ANY, MagicMock, patch -import asynctest - from doozerlib import constants from doozerlib.distgit import ImageDistGitRepo from doozerlib.gitdata import DataObj @@ -10,7 +8,7 @@ from doozerlib.osbs2_builder import OSBS2Builder -class TestOSBS2Builder(asynctest.TestCase): +class TestOSBS2Builder(unittest.IsolatedAsyncioTestCase): def _make_image_meta(self, runtime): data_obj = DataObj("foo", "/path/to/ocp-build-data/images/foo.yml", { diff --git a/tests/test_rhcos.py b/tests/test_rhcos.py index 6ba4b6467..598afe66a 100755 --- a/tests/test_rhcos.py +++ b/tests/test_rhcos.py @@ -19,6 +19,7 @@ class MockRuntime(object): def __init__(self, logger): self.logger = logger self.group_config = Model({}) + self.pooled_koji_client_session = MagicMock() def _urlopen_json_cm(mock_urlopen, content, rc=200): @@ -36,15 +37,8 @@ def setUp(self): self.logger = MagicMock(spec=logging.Logger) runtime = MockRuntime(self.logger) - koji_mock = Mock() - koji_mock.__enter__ = Mock() - koji_mock.__enter__.return_value = koji_mock - koji_mock.__exit__ = Mock() - - runtime.pooled_koji_client_session = Mock() - runtime.pooled_koji_client_session.return_value = koji_mock self.runtime = runtime - self.koji_mock = koji_mock + self.koji_mock = runtime.pooled_koji_client_session.return_value.__enter__.return_value self.respath = Path(os.path.dirname(__file__), 'resources') def tearDown(self): @@ -148,8 +142,9 @@ def canned_getRPM(nvra, *_, **__): def canned_getBuild(build_id, *_, **__): return pkg_build_dicts[build_id] - self.koji_mock.getRPM.side_effect = canned_getRPM - self.koji_mock.getBuild.side_effect = canned_getBuild + koji_multicall = self.koji_mock.multicall.return_value.__enter__.return_value + koji_multicall.getRPM.side_effect = lambda n, *_, **__: Mock(result=canned_getRPM(n)) + koji_multicall.getBuild.side_effect = lambda b, *_, **__: Mock(result=canned_getBuild(b)) self.assertIn("util-linux-2.32.1-24.el8.s390x", rhcos_build.get_rpm_nvras()) self.assertIn("util-linux-2.32.1-24.el8", rhcos_build.get_rpm_nvrs()) diff --git a/tests/test_rpm_builder.py b/tests/test_rpm_builder.py index ed0842db0..6d1666b26 100644 --- a/tests/test_rpm_builder.py +++ b/tests/test_rpm_builder.py @@ -2,15 +2,14 @@ import io from pathlib import Path -import asynctest -from unittest import mock +from unittest import IsolatedAsyncioTestCase, mock from doozerlib import distgit, gitdata, rpmcfg from doozerlib.exectools import RetryException from doozerlib.rpm_builder import RPMBuilder -class TestRPMBuilder(asynctest.TestCase): +class TestRPMBuilder(IsolatedAsyncioTestCase): def setUp(self) -> None: pass