ART-3278: Request a rebuild if its dependency is newer (#501)

* ART-3278: Request a rebuild if its dependency is newer

Currently if image A is a dependent of B and B is changing, a rebuild of A will be requested as well. However, if the rebuild of B is successful but rebuild of A is failed, Doozer will not request a rebuild of A again.

* Add None check

doozerlib/cli/scan_sources.py

@@ -1,20 +1,21 @@
-from pathlib import Path
+from datetime import datetime, timezone
 
 import click
 import yaml
 
-from doozerlib import brew, exectools, rhcos
+from doozerlib import brew, exectools, rhcos, util
 from doozerlib.cli import cli, pass_runtime
 from doozerlib.cli import release_gen_payload as rgp
 from doozerlib.metadata import RebuildHint, RebuildHintCode
+from doozerlib.runtime import Runtime
 
 
 @cli.command("config:scan-sources", short_help="Determine if any rpms / images need to be rebuilt.")
 @click.option("--ci-kubeconfig", metavar='KC_PATH', required=False,
               help="File containing kubeconfig for looking at release-controller imagestreams")
 @click.option("--yaml", "as_yaml", default=False, is_flag=True, help='Print results in a yaml block')
 @pass_runtime
-def config_scan_source_changes(runtime, ci_kubeconfig, as_yaml):
+def config_scan_source_changes(runtime: Runtime, ci_kubeconfig, as_yaml):
     """
     Determine if any rpms / images need to be rebuilt.
 
@@ -109,30 +110,56 @@ def add_image_meta_change(meta, rebuild_hint: RebuildHint):
         newest_image_event_ts = 0
         for image_meta in runtime.image_metas():
             info = image_meta.get_latest_build(default=None)
-            if info is not None:
-
-                # If no upstream change has been detected, check configurations
-                # like image meta, repos, and streams to see if they have changed
-                # We detect config changes by comparing their digest changes.
-                # The config digest of the previous build is stored at .oit/config_digest on distgit repo.
-                try:
-                    source_url = info['source']  # git://pkgs.devel.redhat.com/containers/atomic-openshift-descheduler#6fc9c31e5d9437ac19e3c4b45231be8392cdacac
-                    source_commit = source_url.split('#')[1]  # isolate the commit hash
-                    # Look at the digest that created THIS build. What is in head does not matter.
-                    prev_digest = image_meta.fetch_cgit_file('.oit/config_digest', commit_hash=source_commit).decode('utf-8')
-                    current_digest = image_meta.calculate_config_digest(runtime.group_config, runtime.streams)
-                    if current_digest.strip() != prev_digest.strip():
-                        runtime.logger.info('%s config_digest %s is differing from %s', dgk, prev_digest, current_digest)
-                        add_image_meta_change(image_meta, RebuildHint(RebuildHintCode.CONFIG_CHANGE, 'Metadata configuration change'))
-                except exectools.RetryException:
-                    runtime.logger.info('%s config_digest cannot be retrieved; request a build', dgk)
-                    add_image_meta_change(image_meta, RebuildHint(RebuildHintCode.CONFIG_CHANGE, 'Unable to retrieve config_digest'))
-
-                create_event_ts = koji_api.getEvent(info['creation_event_id'])['ts']
-                if oldest_image_event_ts is None or create_event_ts < oldest_image_event_ts:
-                    oldest_image_event_ts = create_event_ts
-                if create_event_ts > newest_image_event_ts:
-                    newest_image_event_ts = create_event_ts
+            if info is None:
+                continue
+            create_event_ts = koji_api.getEvent(info['creation_event_id'])['ts']
+            if oldest_image_event_ts is None or create_event_ts < oldest_image_event_ts:
+                oldest_image_event_ts = create_event_ts
+            if create_event_ts > newest_image_event_ts:
+                newest_image_event_ts = create_event_ts
+
+            if image_meta in changing_image_metas:
+                continue  # A rebuild is already requested.
+
+            # Request a rebuild if A is a dependent of B but the latest build of A is older than B.
+            rebase_time = util.isolate_timestamp_in_release(info["release"])
+            if not rebase_time:  # no timestamp string in NVR?
+                continue
+            rebase_time = datetime.strptime(rebase_time, "%Y%m%d%H%M%S").replace(tzinfo=timezone.utc)
+            for dep_key in image_meta.dependencies:
+                dep = runtime.image_map.get(dep_key)
+                if not dep:
+                    runtime.logger.warning("Image %s has unknown dependency %s. Is it excluded?", image_meta.distgit_key, dep_key)
+                    continue
+                dep_info = dep.get_latest_build(default=None)
+                if not dep_info:
+                    continue
+                dep_rebase_time = util.isolate_timestamp_in_release(dep_info["release"])
+                if not dep_rebase_time:  # no timestamp string in NVR?
+                    continue
+                dep_rebase_time = datetime.strptime(dep_rebase_time, "%Y%m%d%H%M%S").replace(tzinfo=timezone.utc)
+                if dep_rebase_time > rebase_time:
+                    add_image_meta_change(image_meta, RebuildHint(RebuildHintCode.DEPENDENCY_NEWER, 'Dependency has a newer build'))
+
+            if image_meta in changing_image_metas:
+                continue  # A rebuild is already requested.
+
+            # If no upstream change has been detected, check configurations
+            # like image meta, repos, and streams to see if they have changed
+            # We detect config changes by comparing their digest changes.
+            # The config digest of the previous build is stored at .oit/config_digest on distgit repo.
+            try:
+                source_url = info['source']  # git://pkgs.devel.redhat.com/containers/atomic-openshift-descheduler#6fc9c31e5d9437ac19e3c4b45231be8392cdacac
+                source_commit = source_url.split('#')[1]  # isolate the commit hash
+                # Look at the digest that created THIS build. What is in head does not matter.
+                prev_digest = image_meta.fetch_cgit_file('.oit/config_digest', commit_hash=source_commit).decode('utf-8')
+                current_digest = image_meta.calculate_config_digest(runtime.group_config, runtime.streams)
+                if current_digest.strip() != prev_digest.strip():
+                    runtime.logger.info('%s config_digest %s is differing from %s', dgk, prev_digest, current_digest)
+                    add_image_meta_change(image_meta, RebuildHint(RebuildHintCode.CONFIG_CHANGE, 'Metadata configuration change'))
+            except exectools.RetryException:
+                runtime.logger.info('%s config_digest cannot be retrieved; request a build', dgk)
+                add_image_meta_change(image_meta, RebuildHint(RebuildHintCode.CONFIG_CHANGE, 'Unable to retrieve config_digest'))
 
     runtime.logger.debug(f'Will be assessing tagging changes between newest_image_event_ts:{newest_image_event_ts} and oldest_image_event_ts:{oldest_image_event_ts}')
     change_results = runtime.parallel_exec(

doozerlib/image.py

@@ -3,16 +3,17 @@
 import hashlib
 import json
 import random
-from typing import Any, Dict, Optional, Tuple, Union, List
+from typing import Any, Dict, List, Optional, Set, Tuple, Union
+
 from kobo.rpmlib import parse_nvr
 
-from doozerlib import brew, exectools
+from doozerlib import brew, coverity, exectools
 from doozerlib.distgit import pull_image
 from doozerlib.metadata import Metadata, RebuildHint, RebuildHintCode
 from doozerlib.model import Missing, Model
 from doozerlib.pushd import Dir
-from doozerlib import coverity
-from doozerlib.util import brew_arch_for_go_arch, isolate_el_version_in_release, go_arch_for_brew_arch
+from doozerlib.util import (brew_arch_for_go_arch, go_arch_for_brew_arch,
+                            isolate_el_version_in_release)
 
 
 class ImageMetadata(Metadata):
@@ -24,9 +25,12 @@ def __init__(self, runtime, data_obj: Dict, commitish: Optional[str] = None, clo
         self.image_name_short = self.image_name.split('/')[-1]
         self.parent = None
         self.children = []  # list of ImageMetadata which use this image as a parent.
+        self.dependencies: Set[str] = set()
         dependents = self.config.get('dependents', [])
         for d in dependents:
-            self.children.append(self.runtime.late_resolve_image(d, add=True))
+            dependent: ImageMetadata = self.runtime.late_resolve_image(d, add=True)
+            dependent.dependencies.add(self.distgit_key)
+            self.children.append(dependent)
         if clone_source:
             runtime.resolve_source(self)
 

doozerlib/metadata.py

@@ -66,6 +66,7 @@ class RebuildHintCode(Enum):
     BUILD_ROOT_CHANGING = (True, 12)
     PACKAGE_CHANGE = (True, 13)
     ARCHES_CHANGE = (True, 14)
+    DEPENDENCY_NEWER = (True, 15)
 
 
 class RebuildHint(NamedTuple):

doozerlib/runtime.py

@@ -606,9 +606,10 @@ def filter_disabled(n, d):
 
             if mode in ['images', 'both']:
                 for i in image_data.values():
-                    metadata = ImageMetadata(self, i, self.upstream_commitish_overrides.get(i.key), clone_source=clone_source, prevent_cloning=prevent_cloning)
-                    self.image_map[metadata.distgit_key] = metadata
-                    self.component_map[metadata.get_component_name()] = metadata
+                    if i.key not in self.image_map:
+                        metadata = ImageMetadata(self, i, self.upstream_commitish_overrides.get(i.key), clone_source=clone_source, prevent_cloning=prevent_cloning)
+                        self.image_map[metadata.distgit_key] = metadata
+                        self.component_map[metadata.get_component_name()] = metadata
                 if not self.image_map:
                     self.logger.warning("No image metadata directories found for given options within: {}".format(self.group_dir))
 
@@ -1021,6 +1022,7 @@ def late_resolve_image(self, distgit_name, add=False):
         meta = ImageMetadata(self, data_obj, self.upstream_commitish_overrides.get(data_obj.key))
         if add:
             self.image_map[distgit_name] = meta
+        self.component_map[meta.get_component_name()] = meta
         return meta
 
     def resolve_brew_image_url(self, image_name_and_version):

doozerlib/util.py

@@ -620,3 +620,21 @@ def strip_epoch(nvr: str):
     returns NVR exactly as-is.
     """
     return nvr.split(':')[0]
+
+
+def isolate_timestamp_in_release(release: str) -> Optional[str]:
+    """
+    Given a release field, determines whether is contains
+    a timestamp. If it does, it returns the timestamp.
+    If it is not found, None is returned.
+    """
+    match = re.search(r"(\d{4})(\d{2})(\d{2})(\d{2})(\d{2})", release)  # yyyyMMddHHmm
+    if match:
+        year = int(match.group(1))
+        month = int(match.group(2))
+        day = int(match.group(3))
+        hour = int(match.group(4))
+        minute = int(match.group(5))
+        if year >= 2000 and month >= 1 and month <= 12 and day >= 1 and day <= 31 and hour <= 23 and minute <= 59:
+            return match.group(0)
+    return None

tests/test_util.py

@@ -135,6 +135,35 @@ def test_find_latest_builds(self):
         actual = util.find_latest_builds(builds, None)
         self.assertEqual([13, 23, 33], [b["id"] for b in actual])
 
+    def test_isolate_timestamp_in_release(self):
+        actual = util.isolate_timestamp_in_release("foo-4.7.0-202107021813.p0.git.01c9f3f.el8")
+        expected = "202107021813"
+        self.assertEqual(actual, expected)
+
+        actual = util.isolate_timestamp_in_release("foo-container-v4.7.0-202107021907.p0.git.8b4b094")
+        expected = "202107021907"
+        self.assertEqual(actual, expected)
+
+        actual = util.isolate_timestamp_in_release("foo-container-v4.7.0-202107021907.p0.git.8b4b094")
+        expected = "202107021907"
+        self.assertEqual(actual, expected)
+
+        actual = util.isolate_timestamp_in_release("foo-container-v4.8.0-202106152230.p0.git.25122f5.assembly.stream")
+        expected = "202106152230"
+        self.assertEqual(actual, expected)
+
+        actual = util.isolate_timestamp_in_release("foo-container-v4.7.0-1.p0.git.8b4b094")
+        expected = None
+        self.assertEqual(actual, expected)
+
+        actual = util.isolate_timestamp_in_release("foo-container-v4.7.0-202199999999.p0.git.8b4b094")
+        expected = None
+        self.assertEqual(actual, expected)
+
+        actual = util.isolate_timestamp_in_release("")
+        expected = None
+        self.assertEqual(actual, expected)
+
 
 if __name__ == "__main__":
     unittest.main()