#1708 Exclude specific platform streams from stream expansion
Merged 2 years ago by mikem. Opened 2 years ago by breilly.
breilly/fm-orchestrator streamexclude-1812  into  master

@@ -448,6 +448,12 @@ 

                      "considered by MBS as possible buildrequirement. When False, modules "

                      "built against any base module stream can be used as a buildrequire.",

          },

+         "base_module_stream_exclusions": {

+             "type": list,

+             "default": [],

+             "desc":

+                 "List of base module streams to exclude from stream expansion"

+         },

          "koji_cg_tag_build": {

              "type": bool,

              "default": True,

@@ -43,7 +43,11 @@ 

              raise StreamAmbigous("There are multiple streams to choose from for module %s." % name)

          else:

              builds = models.ModuleBuild.get_last_build_in_all_streams(db_session, name)

-             expanded_streams = [build.stream for build in builds]

+             if name in conf.base_module_names and conf.base_module_stream_exclusions:

+                 expanded_streams = [build.stream for build in builds

+                                     if build.stream not in conf.base_module_stream_exclusions]

+             else:

+                 expanded_streams = [build.stream for build in builds]

      else:

          expanded_streams = []

      for stream in streams:

file modified
+194 -2
@@ -3,7 +3,7 @@ 

  from __future__ import absolute_import

  

  import pytest

- 

+ from mock import patch, PropertyMock

  from module_build_service.common.errors import StreamAmbigous, ValidationError

  from module_build_service.scheduler.db_session import db_session

  from module_build_service.web.mse import (
@@ -53,6 +53,24 @@ 

          make_module_in_db("foo:2:0:c5", f29_deps, base_module=platform_f29)

          make_module_in_db("app:1:0:c6", f29_deps, base_module=platform_f29)

  

+     def _generate_eln_modules(self):

+         """

+         Generates gtk:1, gtk:2, foo:1 and foo:2 modules requiring the

+         platform:eln module.

+         """

+         platform_eln = make_module_in_db("platform:eln:0:c12")

+ 

+         eln_deps = [{

+             "requires": {"platform": ["eln"]},

+             "buildrequires": {"platform": ["eln"]}

+         }]

+ 

+         make_module_in_db("gtk:1:0:c7", eln_deps, base_module=platform_eln)

+         make_module_in_db("gtk:2:0:c8", eln_deps, base_module=platform_eln)

+         make_module_in_db("foo:1:0:c7", eln_deps, base_module=platform_eln)

+         make_module_in_db("foo:2:0:c8", eln_deps, base_module=platform_eln)

+         make_module_in_db("app:1:0:c9", eln_deps, base_module=platform_eln)

+ 

      def test_generate_expanded_mmds_context(self):

          self._generate_default_modules()

          module_build = make_module_in_db(
@@ -273,9 +291,12 @@ 

                      "platform:f29:0:c11",

                      "gtk:2:0:c4",

                      "gtk:2:0:c5",

+                     "gtk:2:0:c8",

                      "platform:f28:0:c10",

                      "gtk:1:0:c2",

                      "gtk:1:0:c3",

+                     "gtk:1:0:c7",

+                     "platform:eln:0:c12",

                  ],

              ),

              (
@@ -287,9 +308,12 @@ 

                      "platform:f28:0:c10",

                      "gtk:1:0:c2",

                      "gtk:1:0:c3",

+                     "gtk:1:0:c7",

                      "foo:1:0:c2",

                      "foo:1:0:c3",

+                     "foo:1:0:c7",

                      "platform:f29:0:c11",

+                     "platform:eln:0:c12",

                  ],

              ),

              (
@@ -313,14 +337,19 @@ 

                  [

                      "foo:1:0:c2",

                      "foo:1:0:c3",

+                     "foo:1:0:c7",

                      "foo:2:0:c4",

                      "foo:2:0:c5",

+                     "foo:2:0:c8",

                      "platform:f28:0:c10",

                      "platform:f29:0:c11",

+                     "platform:eln:0:c12",

                      "gtk:1:0:c2",

                      "gtk:1:0:c3",

+                     "gtk:1:0:c7",

                      "gtk:2:0:c4",

                      "gtk:2:0:c5",

+                     "gtk:2:0:c8",

                  ],

              ),

              (
@@ -331,17 +360,180 @@ 

                  [

                      "foo:1:0:c2",

                      "foo:1:0:c3",

+                     "foo:1:0:c7",

                      "platform:f29:0:c11",

                      "platform:f28:0:c10",

+                     "platform:eln:0:c12",

                      "gtk:1:0:c2",

                      "gtk:1:0:c3",

+                     "gtk:1:0:c7",

                  ],

              ),

          ],

      )

-     def test_get_required_modules_simple(self, module_deps, expected):

+     def test_get_required_modules_simple(

+             self, module_deps, expected

+     ):

+         module_build = make_module_in_db("app:1:0:c1", module_deps)

+         self._generate_default_modules()

+         self._generate_eln_modules()

+         nsvcs = self._get_mmds_required_by_module_recursively(module_build, db_session)

+         assert set(nsvcs) == set(expected)

+ 

+     @patch(

+         "module_build_service.common.config.Config.base_module_stream_exclusions",

+         new_callable=PropertyMock,

+         return_value=["eln"],

+     )

+     @pytest.mark.parametrize(

+         "module_deps,expected",

+         [

+             (

+                 [{"requires": {}, "buildrequires": {"platform": [], "gtk": ["1", "2"]}}],

+                 [

+                     "platform:f29:0:c11",

+                     "gtk:2:0:c4",

+                     "gtk:2:0:c5",

+                     "platform:f28:0:c10",

+                     "gtk:1:0:c2",

+                     "gtk:1:0:c3",

+                 ],

+             ),

+             (

+                 [{

+                     "requires": {},

+                     "buildrequires": {"platform": [], "gtk": ["1"], "foo": ["1"]}

+                 }],

+                 [

+                     "platform:f28:0:c10",

+                     "gtk:1:0:c2",

+                     "gtk:1:0:c3",

+                     "foo:1:0:c2",

+                     "foo:1:0:c3",

+                     "platform:f29:0:c11",

+                 ],

+             ),

+             (

+                 [{

+                     "requires": {},

+                     "buildrequires": {"gtk": ["1"], "foo": ["1"], "platform": ["f28"]}

+                 }],

+                 ["platform:f28:0:c10", "gtk:1:0:c2", "foo:1:0:c2"],

+             ),

+             (

+                 [

+                     {

+                         "requires": {},

+                         "buildrequires": {"platform": [], "gtk": ["1"], "foo": ["1"]}

+                     },

+                     {

+                         "requires": {},

+                         "buildrequires": {"platform": [], "gtk": ["2"], "foo": ["2"]},

+                     }

+                 ],

+                 [

+                     "foo:1:0:c2",

+                     "foo:1:0:c3",

+                     "foo:2:0:c4",

+                     "foo:2:0:c5",

+                     "platform:f28:0:c10",

+                     "platform:f29:0:c11",

+                     "gtk:1:0:c2",

+                     "gtk:1:0:c3",

+                     "gtk:2:0:c4",

+                     "gtk:2:0:c5",

+                 ],

+             ),

+             (

+                 [{

+                     "requires": {},

+                     "buildrequires": {"platform": [], "gtk": ["-2"], "foo": ["-2"]},

+                 }],

+                 [

+                     "foo:1:0:c2",

+                     "foo:1:0:c3",

+                     "platform:f29:0:c11",

+                     "platform:f28:0:c10",

+                     "gtk:1:0:c2",

+                     "gtk:1:0:c3",

+                 ],

+             ),

+         ],

+     )

+     def test_get_required_modules_exclusion(

+             self, mocked_base_module_stream_exclusions, module_deps, expected

+     ):

+         module_build = make_module_in_db("app:1:0:c1", module_deps)

+         self._generate_default_modules()

+         self._generate_eln_modules()

+         nsvcs = self._get_mmds_required_by_module_recursively(module_build, db_session)

+         assert set(nsvcs) == set(expected)

+ 

+     @patch(

+         "module_build_service.common.config.Config.base_module_stream_exclusions",

+         new_callable=PropertyMock,

+         return_value=["eln"],

+     )

+     @pytest.mark.parametrize(

+         "module_deps,expected",

+         [

+             (

+                 [{"requires": {}, "buildrequires": {"platform": ["eln"], "gtk": ["1", "2"]}}],

+                 [

+                     "platform:eln:0:c12",

+                     "gtk:1:0:c7",

+                     "gtk:2:0:c8",

+                 ],

+             ),

+             (

+                 [{

+                     "requires": {},

+                     "buildrequires": {"platform": ["eln"], "gtk": ["1"], "foo": ["1"]}

+                 }],

+                 [

+                     "platform:eln:0:c12",

+                     "gtk:1:0:c7",

+                     "foo:1:0:c7",

+                 ],

+             ),

+             (

+                 [

+                     {

+                         "requires": {},

+                         "buildrequires": {"platform": ["eln"], "gtk": ["1"], "foo": ["1"]}

+                     },

+                     {

+                         "requires": {},

+                         "buildrequires": {"platform": ["eln"], "gtk": ["2"], "foo": ["2"]},

+                     }

+                 ],

+                 [

+                     "foo:1:0:c7",

+                     "foo:2:0:c8",

+                     "gtk:1:0:c7",

+                     "gtk:2:0:c8",

+                     "platform:eln:0:c12",

+                 ],

+             ),

+             (

+                 [{

+                     "requires": {},

+                     "buildrequires": {"platform": ["eln"], "gtk": ["-2"], "foo": ["-2"]},

+                 }],

+                 [

+                     "platform:eln:0:c12",

+                     "foo:1:0:c7",

+                     "gtk:1:0:c7",

+                 ],

+             ),

+         ],

+     )

+     def test_get_required_modules_explicit_dep(

+             self, mocked_base_module_stream_exclusions, module_deps, expected

+     ):

          module_build = make_module_in_db("app:1:0:c1", module_deps)

          self._generate_default_modules()

+         self._generate_eln_modules()

          nsvcs = self._get_mmds_required_by_module_recursively(module_build, db_session)

          assert set(nsvcs) == set(expected)

  

Build 692cfd0a25a8d8f1c73b8175c457e9caa9f9263b FAILED!
Rebase or make new commits to rebuild.

rebased onto 065779f1e3cbdba439197c7f72a8aba9077dc2eb

2 years ago

Build 065779f1e3cbdba439197c7f72a8aba9077dc2eb FAILED!
Rebase or make new commits to rebuild.

I'm concerned about the unit test updates.

It seems like this feature ought to merit its own unit test(s), rather than just modifying test_get_required_modules_simple. Possibly this could be worked into the parameterization, but as-is this patch applies base_module_stream_exclusions =["eln"] to all variations of this existing test.

My instinct is that we'd to cover the following new cases in addition to the cases we were covering before:

  • exclusions configured -> eln avoided in expansion
  • exclusions configured but explicit eln dep -> eln still present
  • exclusions not configured -> normal behavior

rebased onto 1ccb6ec

2 years ago

@mikem good point, I've made test_get_required_modules_simple be the exclusions not configured case, and created a couple new tests for the others.

Build 1ccb6ec FAILED!
Rebase or make new commits to rebuild.

Commit e5d0b39 fixes this pull-request

Pull-Request has been merged by mikem

2 years ago