diff --git a/doozerlib/distgit.py b/doozerlib/distgit.py index 9caa44b77..55fd0e592 100644 --- a/doozerlib/distgit.py +++ b/doozerlib/distgit.py @@ -17,7 +17,6 @@ import aiofiles import bashlex -from kobo.rpmlib import parse_nvr import requests import yaml from dockerfile_parse import DockerfileParser @@ -2260,7 +2259,7 @@ def _merge_source(self): def _run_modifications(self): """ - Interprets and applies content.source.modify steps in the image metadata. + Interprets and applies content.source.modifications steps in the image metadata. """ dg_path = self.dg_path df_path = dg_path.joinpath('Dockerfile') diff --git a/doozerlib/metadata.py b/doozerlib/metadata.py index b01d8c778..1fe3c52a9 100644 --- a/doozerlib/metadata.py +++ b/doozerlib/metadata.py @@ -1,12 +1,13 @@ -from typing import Dict, Optional, List, Tuple, NamedTuple +from typing import Dict, Optional, List, Tuple, Any, Union import urllib.parse import yaml -from collections import OrderedDict, namedtuple +from collections import OrderedDict import pathlib import json import traceback import datetime import time +import sys import re from enum import Enum @@ -18,11 +19,11 @@ from .distgit import ImageDistGitRepo, RPMDistGitRepo from . import exectools from . import logutil -from .brew import BuildStates -from .util import isolate_el_version_in_brew_tag +from .brew import BuildStates, KojiWrapperOpts +from .util import isolate_el_version_in_brew_tag, isolate_git_commit_in_release from .model import Model, Missing -from doozerlib.assembly import assembly_metadata_config, assembly_basis_event +from doozerlib.assembly import assembly_metadata_config, assembly_basis_event, AssemblyTypes class CgitAtomFeedEntry(NamedTuple): @@ -136,7 +137,50 @@ def __init__(self, meta_type: str, runtime, data_obj: Dict, commitish: Optional[ # The first target is the primary target, against which tito will direct build. # Others are secondary targets. We will use Brew API to build against secondary # targets with the same distgit commit as the primary target. - self.targets = self.determine_targets() + self.targets: List[str] = self.determine_targets() + + if self.runtime.assembly_type != AssemblyTypes.STREAM and self.config.content.source.git.branch.target and not commitish: + # Ok, so we are a release assembly like 'art1999'. We inherit from + # 4.7.22 which was composed primarily out of "assembly.stream" builds + # but maybe one or two pinned "assembly.4.7.22" builds. + # An artists has been asked to create art1999 and bump a single RPM in the + # ose-etcd image. To do that, the artist + # adds the dependency to releases.yml for the ose-etcd distgit_key in the + # art1999 assembly and then triggers a "rebuild" job of the image. + # What upstream git commit do you expect to be built? Why the source from 4.7.22, + # of course! The customer asked for an RPM bump, not to take on any of the hundreds of + # code changes that may have take place since 4.7.22 in the 4.7 branch. + # So how do we arrive at that? Well, it is in the brew metadata of the latest build from + # the 4.7.22 assembly. + # Oh, but what if the customer DOES want a different commit? Well, the artist should + # update the release.yml for art1999 to include that commit or explicitly specify a branch. + # How do we determine whether they have done that? Looking for any explicit overrides + # in the our assembly's metadata. + # Let's do it! + assembly_overrides = assembly_metadata_config(runtime.get_releases_config(), runtime.assembly, meta_type, self.distgit_key, Model({})) + # Nice! By passing Model({}) instead of the metadata from our image yml file, we should only get fields actually defined in + # releases.yml. + if assembly_overrides.content.source.git.branch.target: + # Yep.. there is an override in releases.yml. The good news is that we are done. + # The rest of doozer code is equipped to clone that upstream commit + # and rebase using it. + pass + else: + # Ooof.. it is not defined in the assembly, so we need to find it dynamically. + build_obj = self.get_latest_build(default=None, el_target=self.determine_rhel_targets()[0]) + if build_obj: + self.commitish = isolate_git_commit_in_release(build_obj['nvr']) + self.logger.warning(f'Pinning upstream source to commit of last assembly selected build ({build_obj["id"]}) -> commit {self.commitish} ') + else: + # If this is part of a unit test, don't make the caller's life more difficult thatn it already is; skip the exception. + if 'unittest' not in sys.modules.keys(): + raise IOError(f'Expected to find pre-existing build for {self.distgit_key} in order to pin upstream source commit') + + # If you've read this far, you may be wondering, why are we not trying to find the SOURCE_GIT_URL from the last built image? + # Good question! Because it should be the value found in our assembly-modified image metadata! + # The git commit starts as a branch in standard ocp-build-data metadata and its + # commit hash is only discovered at runtime. The source git URL is literal. If it does change somewhere in the assembly + # definitions, that's fine. This assembly should find it when looking up the content.source.git.url from the metadata. def determine_targets(self) -> List[str]: """ Determine Brew targets for building this component @@ -321,7 +365,9 @@ def fetch_cgit_file(self, filename, commit_hash: Optional[str] = None, branch: O check_f=lambda req: req.code == 200) return req.read() - def get_latest_build(self, default=-1, assembly=None, extra_pattern='*', build_state: BuildStates = BuildStates.COMPLETE, el_target=None, honor_is=True, complete_before_event: Optional[int] = None): + def get_latest_build(self, default: Optional[Any] = -1, assembly: Optional[str] = None, extra_pattern: str = '*', + build_state: BuildStates = BuildStates.COMPLETE, + el_target: Optional[Union[str, int]] = None, honor_is: bool = True, complete_before_event: Optional[int] = None): """ :param default: A value to return if no latest is found (if not specified, an exception will be thrown) :param assembly: A non-default assembly name to search relative to. If not specified, runtime.assembly @@ -345,7 +391,7 @@ def get_latest_build(self, default=-1, assembly=None, extra_pattern='*', build_s component_name = self.get_component_name() builds = [] - with self.runtime.pooled_koji_client_session() as koji_api: + with self.runtime.pooled_koji_client_session(caching=True) as koji_api: package_info = koji_api.getPackage(component_name) # e.g. {'id': 66873, 'name': 'atomic-openshift-descheduler-container'} if not package_info: raise IOError(f'No brew package is defined for {component_name}') diff --git a/doozerlib/rpmcfg.py b/doozerlib/rpmcfg.py index ab0427c3a..e856650d9 100644 --- a/doozerlib/rpmcfg.py +++ b/doozerlib/rpmcfg.py @@ -1,20 +1,16 @@ import glob import io import os -import re -import shutil import threading -import traceback -from typing import List, Optional +from typing import List, Dict, Optional, Union import rpm -from doozerlib import brew, constants, util +from doozerlib import brew, util from doozerlib.exceptions import DoozerFatalError from doozerlib.source_modifications import SourceModifierFactory from . import exectools -from .brew import watch_tasks from .metadata import Metadata from .model import Missing from .pushd import Dir diff --git a/doozerlib/runtime.py b/doozerlib/runtime.py index d40b3cb3c..b05ae2209 100644 --- a/doozerlib/runtime.py +++ b/doozerlib/runtime.py @@ -729,12 +729,15 @@ def shared_build_status_detector(self) -> 'BuildStatusDetector': yield self._build_status_detector @contextmanager - def pooled_koji_client_session(self): + def pooled_koji_client_session(self, caching: bool = False): """ Context manager which offers a koji client session from a limited pool. You hold a lock on this session until you return. It is not recommended to call other methods that acquire their own pooled sessions, because that may lead to deadlock if the pool is exhausted. Honors doozer --brew-event. + :param caching: Set to True in order for your instance to place calls/results into + the global KojiWrapper cache. This is equivalent to passing + KojiWrapperOpts(caching=True) in each call within the session context. """ session = None session_id = None @@ -759,8 +762,10 @@ def pooled_koji_client_session(self): # Arriving here, we have a session to use. try: + session.force_instance_caching = caching yield session finally: + session.force_instance_caching = False # Put it back into the pool with self.mutex: self.session_pool_available[session_id] = session diff --git a/doozerlib/util.py b/doozerlib/util.py index 01d114a86..40565e70e 100644 --- a/doozerlib/util.py +++ b/doozerlib/util.py @@ -2,6 +2,7 @@ import os import pathlib import re +import urllib.parse from collections import deque from contextlib import contextmanager from datetime import datetime @@ -357,6 +358,20 @@ def get_docker_config_json(config_dir): raise FileNotFoundError("Can not find the registry config file in {}".format(config_dir)) +def isolate_git_commit_in_release(release: str) -> Optional[str]: + """ + Given a release field, determines whether is contains + .git. information. If it does, it returns the value + of . If it is not found, None is returned. + """ + match = re.match(r'.*\.git\.([a-f0-9]+)(?:\.+|$)', release) + + if match: + return match.group(1) + + return None + + def isolate_pflag_in_release(release: str) -> Optional[str]: """ Given a release field, determines whether is contains diff --git a/tests/cli/test_rpms_build.py b/tests/cli/test_rpms_build.py index 8ab832472..c40fce248 100644 --- a/tests/cli/test_rpms_build.py +++ b/tests/cli/test_rpms_build.py @@ -1,5 +1,6 @@ import asyncio import logging +import io from unittest import TestCase from mock import AsyncMock, MagicMock, Mock, patch @@ -10,10 +11,22 @@ class TestRPMsBuildCli(TestCase): + + def _make_runtime(self, assembly=None): + runtime = MagicMock() + runtime.group_config.public_upstreams = [{"private": "https://github.com/openshift-priv", "public": "https://github.com/openshift"}] + runtime.brew_logs_dir = "/path/to/brew-logs" + runtime.assembly = assembly + runtime.local = False + runtime.assert_mutation_is_permitted = MagicMock() + stream = io.StringIO() + logging.basicConfig(level=logging.INFO, stream=stream) + runtime.logger = logging.getLogger() + return runtime + @patch("doozerlib.cli.rpms_build.RPMBuilder") def test_rpms_build_success(self, MockedRPMBuilder: Mock): - runtime = MagicMock(local=False) - runtime.assert_mutation_is_permitted = MagicMock() + runtime = self._make_runtime() version = "v1.2.3" release = "202104070000.yuxzhu.test.p?" embargoed = True @@ -36,7 +49,6 @@ def test_rpms_build_success(self, MockedRPMBuilder: Mock): }) rpm = rpmcfg.RPMMetadata(runtime, data_obj, clone_source=False) rpm.distgit_repo = MagicMock(branch="rhaos-4.4-rhel-8") - rpm.logger = MagicMock(spec=logging.Logger) rpms.append(rpm) data_obj = gitdata.DataObj("bar", "/path/to/ocp-build-data/rpms/bar.yml", { @@ -51,7 +63,6 @@ def test_rpms_build_success(self, MockedRPMBuilder: Mock): }) rpm = rpmcfg.RPMMetadata(runtime, data_obj, clone_source=False) rpm.distgit_repo = MagicMock(branch="rhaos-4.4-rhel-8") - rpm.logger = MagicMock(spec=logging.Logger) rpms.append(rpm) runtime.rpm_metas.return_value = rpms @@ -64,8 +75,7 @@ def test_rpms_build_success(self, MockedRPMBuilder: Mock): @patch("doozerlib.cli.rpms_build.RPMBuilder") def test_rpms_build_failure(self, MockedRPMBuilder: Mock): - runtime = MagicMock(local=False) - runtime.assert_mutation_is_permitted = MagicMock() + runtime = self._make_runtime() version = "v1.2.3" release = "202104070000.yuxzhu.test.p?" embargoed = True @@ -87,7 +97,6 @@ def test_rpms_build_failure(self, MockedRPMBuilder: Mock): "targets": ["rhaos-4.4-rhel-8-candidate", "rhaos-4.4-rhel-7-candidate"], }) rpm = rpmcfg.RPMMetadata(runtime, data_obj, clone_source=False) - rpm.logger = MagicMock(spec=logging.Logger) rpm.distgit_repo = MagicMock(branch="rhaos-4.4-rhel-8") rpms.append(rpm) @@ -102,7 +111,6 @@ def test_rpms_build_failure(self, MockedRPMBuilder: Mock): "targets": ["rhaos-4.4-rhel-8-candidate"], }) rpm = rpmcfg.RPMMetadata(runtime, data_obj, clone_source=False) - rpm.logger = MagicMock(spec=logging.Logger) rpm.distgit_repo = MagicMock(branch="rhaos-4.4-rhel-8") rpms.append(rpm) diff --git a/tests/test_rpm_builder.py b/tests/test_rpm_builder.py index bd8a78ca1..9d08cd348 100644 --- a/tests/test_rpm_builder.py +++ b/tests/test_rpm_builder.py @@ -1,6 +1,7 @@ import asyncio import logging import unittest +import io from pathlib import Path import mock @@ -17,9 +18,12 @@ def setUp(self) -> None: def _make_runtime(self, assembly=None): runtime = mock.MagicMock() - runtime.group_config.public_upstreams = [{"private": "https://github.com/openshift-priv", "puiblic": "https://github.com/openshift"}] + runtime.group_config.public_upstreams = [{"private": "https://github.com/openshift-priv", "public": "https://github.com/openshift"}] runtime.brew_logs_dir = "/path/to/brew-logs" runtime.assembly = assembly + stream = io.StringIO() + logging.basicConfig(level=logging.INFO, stream=stream) + runtime.logger = logging.getLogger() return runtime def _make_rpm_meta(self, runtime, source_sha, distgit_sha):