Allow components to be reused from module builds even if the buildrequires commit hashes changed for the changed-and-after rebuild strategy

This behavior was not documented, and it was confusing to users since module builds
in a stream should always have a compatible API.

module_build_service/migrations/versions/1817e62719f9_remove_ref_build_context.py

@@ -0,0 +1,24 @@
+"""Remove the ref_build_context column
+
+Revision ID: 1817e62719f9
+Revises: d5188b4a7bf1
+Create Date: 2019-08-02 12:31:00.707314
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "1817e62719f9"
+down_revision = "d5188b4a7bf1"
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    with op.batch_alter_table("module_builds", schema=None) as batch_op:
+        batch_op.drop_column("ref_build_context")
+
+
+def downgrade():
+    with op.batch_alter_table("module_builds", schema=None) as batch_op:
+        batch_op.add_column(sa.Column("ref_build_context", sa.VARCHAR(), nullable=True))

module_build_service/models.py

@@ -85,7 +85,7 @@ INVERSE_BUILD_STATES = {v: k for k, v in BUILD_STATES.items()}
 FAILED_STATES = (BUILD_STATES["failed"], BUILD_STATES["garbage"])
 
 
-Contexts = namedtuple("Contexts", "ref_build_context build_context runtime_context context")
+Contexts = namedtuple("Contexts", "build_context runtime_context context")
 
 
 def _utc_datetime_to_iso(datetime_object):
@@ -255,7 +255,6 @@ class ModuleBuild(MBSBase):
     name = db.Column(db.String, nullable=False)
     stream = db.Column(db.String, nullable=False)
     version = db.Column(db.String, nullable=False)
-    ref_build_context = db.Column(db.String)
     build_context = db.Column(db.String)
     runtime_context = db.Column(db.String)
     context = db.Column(db.String, nullable=False, server_default=DEFAULT_MODULE_CONTEXT)
@@ -626,17 +625,15 @@ class ModuleBuild(MBSBase):
     @classmethod
     def contexts_from_mmd(cls, mmd_str):
         """
-        Returns tuple (ref_build_context, build_context, runtime_context, context)
+        Returns tuple (build_context, runtime_context, context)
         with hashes:
-            - ref_build_context - Hash of commit hashes of expanded buildrequires.
             - build_context - Hash of stream names of expanded buildrequires.
             - runtime_context - Hash of stream names of expanded runtime requires.
             - context - Hash of combined hashes of build_context and runtime_context.
 
         :param str mmd_str: String with Modulemd metadata.
         :rtype: Contexts
-        :return: Named tuple with build_context, strem_build_context, runtime_context and
-                 context hashes.
+        :return: Named tuple with build_context, runtime_context and context hashes.
         """
         from module_build_service.utils.general import load_mmd
 
@@ -651,28 +648,11 @@ class ModuleBuild(MBSBase):
         runtime_context = cls.calculate_runtime_context(mmd_deps)
 
         return Contexts(
-            cls.calculate_ref_build_context(mbs_xmd_buildrequires),
             build_context,
             runtime_context,
             cls.calculate_module_context(build_context, runtime_context)
         )
 
-    @staticmethod
-    def calculate_ref_build_context(mbs_xmd_buildrequires):
-        """
-        Returns the hash of commit hashes of expanded buildrequires.
-        :param mbs_xmd_buildrequires: xmd["mbs"]["buildrequires"] from Modulemd
-        :rtype: str
-        :return: ref_build_context hash
-        """
-        # Get the buildrequires from the XMD section, because it contains
-        # all the buildrequires as we resolved them using dependency resolver.
-        mmd_formatted_buildrequires = {
-            dep: info["ref"] for dep, info in mbs_xmd_buildrequires.items()
-        }
-        property_json = json.dumps(OrderedDict(sorted(mmd_formatted_buildrequires.items())))
-        return hashlib.sha1(property_json.encode("utf-8")).hexdigest()
-
     @staticmethod
     def calculate_build_context(mbs_xmd_buildrequires):
         """
@@ -983,7 +963,6 @@ class ModuleBuild(MBSBase):
             "base_module_buildrequires": [br.short_json(True, False) for br in self.buildrequires],
             "build_context": self.build_context,
             "modulemd": self.modulemd,
-            "ref_build_context": self.ref_build_context,
             "reused_module_id": self.reused_module_id,
             "runtime_context": self.runtime_context,
             "state_trace": [

module_build_service/utils/reuse.py

@@ -117,9 +117,7 @@ def get_reusable_module(db_session, module):
             previous_module_build = previous_module_build.filter(
                 models.ModuleBuild.rebuild_strategy.in_(["all", "changed-and-after"])
             )
-            previous_module_build = previous_module_build.filter_by(
-                ref_build_context=module.ref_build_context
-            )
+
         previous_module_build = previous_module_build.first()
 
         if previous_module_build:

module_build_service/utils/submit.py

@@ -1019,12 +1019,8 @@ def submit_module_build(db_session, username, mmd, params):
                 scratch=params.get("scratch"),
                 srpms=params.get("srpms"),
             )
-            (
-                module.ref_build_context,
-                module.build_context,
-                module.runtime_context,
-                module.context,
-            ) = module.contexts_from_mmd(module.modulemd)
+            module.build_context, module.runtime_context, module.context = \
+                module.contexts_from_mmd(module.modulemd)
             module.context += context_suffix
 
         all_modules_skipped = False

tests/__init__.py

@@ -210,7 +210,6 @@ def _populate_data(db_session, data_size=10, contexts=False, scratch=False):
                 unique_hash = hashlib.sha1(nsvc.encode('utf-8')).hexdigest()
                 build_one.build_context = unique_hash
                 build_one.runtime_context = unique_hash
-                build_one.ref_build_context = unique_hash
                 combined_hashes = "{0}:{1}".format(unique_hash, unique_hash)
                 build_one.context = hashlib.sha1(combined_hashes.encode("utf-8")).hexdigest()[:8]
 

tests/conftest.py

@@ -107,7 +107,6 @@ def reuse_component_init_data(db_session):
         stream="master",
         version='20170109091357',
         state=BUILD_STATES["ready"],
-        ref_build_context="ac4de1c346dcf09ce77d38cd4e75094ec1c08eb0",
         runtime_context="ac4de1c346dcf09ce77d38cd4e75094ec1c08eb0",
         build_context="ac4de1c346dcf09ce77d38cd4e75094ec1c08eb1",
         context="78e4a6fd",
@@ -208,7 +207,6 @@ def reuse_component_init_data(db_session):
         stream="master",
         version='20170219191323',
         state=BUILD_STATES["build"],
-        ref_build_context="ac4de1c346dcf09ce77d38cd4e75094ec1c08eb0",
         runtime_context="ac4de1c346dcf09ce77d38cd4e75094ec1c08eb0",
         build_context="ac4de1c346dcf09ce77d38cd4e75094ec1c08eb1",
         context="c40c156c",

tests/test_build/test_build.py

@@ -1144,7 +1144,6 @@ class TestBuild(BaseTestBuild):
         build_one.stream = "master"
         build_one.version = "2820180205135154"
         build_one.build_context = "return_runtime_context"
-        build_one.ref_build_context = "return_runtime_context"
         build_one.runtime_context = "9c690d0e"
         build_one.context = "9c690d0e"
         build_one.state = models.BUILD_STATES["failed"]
@@ -1283,7 +1282,6 @@ class TestBuild(BaseTestBuild):
         build_one.stream = "master"
         build_one.version = "2820180205135154"
         build_one.build_context = "return_runtime_context"
-        build_one.ref_build_context = "return_runtime_context"
         build_one.runtime_context = "9c690d0e"
         build_one.state = models.BUILD_STATES["failed"]
         # this is not calculated by real but just a value to

tests/test_models/test_models.py

@@ -67,13 +67,8 @@ class TestModels:
         build = ModuleBuild.get_by_id(db_session, 1)
         db_session.commit()
         build.modulemd = read_staged_data("testmodule_dependencies")
-        (
-            build.ref_build_context,
-            build.build_context,
-            build.runtime_context,
-            build.context,
-        ) = ModuleBuild.contexts_from_mmd(build.modulemd)
-        assert build.ref_build_context == "f6e2aeec7576196241b9afa0b6b22acf2b6873d7"
+        build.build_context, build.runtime_context, build.context = \
+            ModuleBuild.contexts_from_mmd(build.modulemd)
         assert build.build_context == "089df24993c037e10174f3fa7342ab4dc191a4d4"
         assert build.runtime_context == "bbc84c7b817ab3dd54916c0bcd6c6bdf512f7f9c"
         assert build.context == "3ee22b28"

tests/test_utils/test_utils.py

@@ -187,40 +187,6 @@ class TestUtilsComponentReuse:
             db_session, second_module_build, "tangerine")
         assert bool(tangerine is None) != bool(set_current_arch == set_database_arch)
 
-    @pytest.mark.parametrize("rebuild_strategy", models.ModuleBuild.rebuild_strategies.keys())
-    def test_get_reusable_component_different_buildrequires_hash(
-        self, rebuild_strategy, db_session
-    ):
-        first_module_build = models.ModuleBuild.get_by_id(db_session, 2)
-        first_module_build.rebuild_strategy = rebuild_strategy
-        db_session.commit()
-
-        second_module_build = models.ModuleBuild.get_by_id(db_session, 3)
-        mmd = second_module_build.mmd()
-        xmd = mmd.get_xmd()
-        xmd["mbs"]["buildrequires"]["platform"]["ref"] = "da39a3ee5e6b4b0d3255bfef95601890afd80709"
-        mmd.set_xmd(xmd)
-        second_module_build.modulemd = mmd_to_str(mmd)
-        second_module_build.ref_build_context = "37c6c57bedf4305ef41249c1794760b5cb8fad17"
-        second_module_build.rebuild_strategy = rebuild_strategy
-        db_session.commit()
-
-        plc_rv = module_build_service.utils.get_reusable_component(
-            db_session, second_module_build, "perl-List-Compare")
-        pt_rv = module_build_service.utils.get_reusable_component(
-            db_session, second_module_build, "perl-Tangerine")
-        tangerine_rv = module_build_service.utils.get_reusable_component(
-            db_session, second_module_build, "tangerine")
-
-        if rebuild_strategy == "only-changed":
-            assert plc_rv is not None
-            assert pt_rv is not None
-            assert tangerine_rv is not None
-        else:
-            assert plc_rv is None
-            assert pt_rv is None
-            assert tangerine_rv is None
-
     @pytest.mark.parametrize("rebuild_strategy", models.ModuleBuild.rebuild_strategies.keys())
     def test_get_reusable_component_different_buildrequires_stream(
         self, rebuild_strategy, db_session
@@ -273,7 +239,8 @@ class TestUtilsComponentReuse:
         }
         mmd.set_xmd(xmd)
         second_module_build.modulemd = mmd_to_str(mmd)
-        second_module_build.ref_build_context = "37c6c57bedf4305ef41249c1794760b5cb8fad17"
+        second_module_build.build_context = models.ModuleBuild.calculate_build_context(
+            xmd["mbs"]["buildrequires"])
         db_session.commit()
 
         plc_rv = module_build_service.utils.get_reusable_component(