#256 Allow creating composes with -devel modules.
Merged 5 months ago by jkaluza. Opened 5 months ago by jkaluza.
jkaluza/odcs devel-modules-resolve  into  master

file modified
+8

@@ -140,3 +140,11 @@ 

  koji_profile = '{{ config.koji_profile }}'

  

  repoclosure_strictness = [('.*', {'*': 'off'})]

+ 

+ {%- if config.include_devel_modules %}

+ include_devel_modules = { "Temporary": [

+ {%- for ns in config.include_devel_modules %}

+         '{{ ns }}',

+ {%- endfor%}

+ ]}

+ {%- endif %}

file modified
+23

@@ -62,6 +62,19 @@ 

          }

          modules = self.get_modules(**params)

  

+         # True if the module is "-devel" module.

+         devel_module = False

+         if not modules["meta"]["total"]:

+             # In case this is "-devel" module, it won't be included in MBS,

+             # but it exists as CG build in Koji. It therefore can be used

+             # to generate the compose, but to actually find the MBS build,

+             # we need to remove the "-devel" suffix from the NSVC.

+             n = nsvc.split(":")[0]

You could do this to simplify the str join later on:

n, svc = nsvc.split(':', 1)
if n.endswith("-devel"):
  params["nsvc"] = n[:-len("-devel")] + ':' + svc

This of course, assumes that nsvc will always have at least name and version.

Thanks. Yeah, there will always be name and version. That's the minimum module description we accept in odcs.

+             if n.endswith("-devel"):

+                 params["nsvc"] = n[:-len("-devel")] + params["nsvc"][len(n):]

+                 modules = self.get_modules(**params)

+                 devel_module = True

+ 

          if not modules["meta"]["total"]:

              raise ModuleLookupError(

                  "Failed to find module %s in the MBS." % nsvc)

@@ -77,6 +90,16 @@ 

          for module in modules["items"]:

              if ret and ret[0]["version"] != module["version"]:

                  break

+             if devel_module:

+                 # Add -devel to module metadata in case we are composing devel

+                 # module.

+                 module["name"] += "-devel"

+                 # Devel module always depend on the non-devel version

+                 mmd = Modulemd.Module.new_from_string(module['modulemd'])

+                 mmd.upgrade()

+                 for dep in mmd.get_dependencies():

+                     dep.add_requires_single(mmd.get_name(), mmd.get_stream())

+                 module["modulemd"] = unicode(mmd.dumps())

              ret.append(module)

          return ret

  

@@ -124,6 +124,7 @@ 

          self.koji_profile = conf.koji_profile

          self.pkgset_koji_inherit = True

          self.lookaside_repos = lookaside_repos.split(" ") if lookaside_repos else []

+         self.include_devel_modules = []

          if arches:

              self.arches = arches

          else:

@@ -162,6 +163,8 @@ 

              self.gather_source = "module"

              self.gather_method = "nodeps"

  

+             self._sort_out_devel_modules()

+ 

              if self.packages:

                  raise ValueError("Exact packages cannot be set for MODULE "

                                   "source type.")

@@ -176,6 +179,31 @@ 

  

          self.check_deps = bool(flags & COMPOSE_FLAGS["check_deps"])

  

+     def _sort_out_devel_modules(self):

+         """

+         Helper method filtering out "-devel" modules from `self.source`

+         and adding them to `include_devel_modules` list.

+         """

+         source_list = self.source.split(" ")

+         new_source = []

+         for nsvc in source_list:

+             n, s, v, c = nsvc.split(":")

+ 

+             # It does not have -devel suffix, so it is not -devel module.

+             if not n.endswith("-devel"):

+                 new_source.append(nsvc)

+                 continue

+ 

+             # If it is -devel module, there must exist the non-devel

+             # counterpart.

+             non_devel_nsvc = ":".join([n[:-len("-devel")], s, v, c])

+             if not non_devel_nsvc in source_list:

+                 new_source.append(nsvc)

+                 continue

+ 

+             self.include_devel_modules.append(":".join([n, s]))

Out of curiosity, why just use NS instead of NSVC?

@lucarval, that's how Pungi defines it. The idea is that you have the non-devel module fully specified in the self.source (this is later stored in variants.xml input file for Pungi) and you just tell Pungi that for the module with this "name:stream", you also want to include -devel counterpart in a compose.

I think it is like that to just keep the Pungi configuration simpler, so RCM people don't have to change the include_devel_modules everytime new module is released (because NSVC would change on every new module build).

+         self.source = " ".join(new_source)

+ 

      @property

      def source_type_str(self):

          return INVERSE_PUNGI_SOURCE_TYPE_NAMES[self.source_type]

@@ -78,6 +78,44 @@ 

                                     "moduleD:f26:20170806000000:00000000"]))

  

      @mock_mbs()

+     def test_resolve_compose_module_devel(self):

+         c = Compose.create(

+             db.session, "me", PungiSourceType.MODULE,

+             "moduleA:f26 moduleA-devel:f26",

+             COMPOSE_RESULTS["repository"], 3600)

+         db.session.commit()

+ 

+         resolve_compose(c)

+         db.session.commit()

+ 

+         c = db.session.query(Compose).filter(Compose.id == 1).one()

+         self.assertEqual(c.source,

+                          ' '.join(["moduleA-devel:f26:20170809000000:00000000",

+                                    "moduleA:f26:20170809000000:00000000",

+                                    "moduleB:f26:20170808000000:00000000",

+                                    "moduleC:f26:20170807000000:00000000",

+                                    "moduleD:f26:20170806000000:00000000"]))

+ 

+     @mock_mbs()

+     def test_resolve_compose_module_devel_deps_resolving(self):

+         c = Compose.create(

+             db.session, "me", PungiSourceType.MODULE,

+             "moduleA-devel:f26",

+             COMPOSE_RESULTS["repository"], 3600)

+         db.session.commit()

+ 

+         resolve_compose(c)

+         db.session.commit()

+ 

+         c = db.session.query(Compose).filter(Compose.id == 1).one()

+         self.assertEqual(c.source,

+                          ' '.join(["moduleA-devel:f26:20170809000000:00000000",

+                                    "moduleA:f26:20170809000000:00000000",

+                                    "moduleB:f26:20170808000000:00000000",

+                                    "moduleC:f26:20170807000000:00000000",

+                                    "moduleD:f26:20170806000000:00000000"]))

+ 

+     @mock_mbs()

      def test_resolve_compose_module_multiple_contexts_no_deps(self):

          c = Compose.create(

              db.session, "me", PungiSourceType.MODULE,

file modified
+25 -7

@@ -56,7 +56,7 @@ 

  

      def test_pungi_config_module(self):

          pungi_cfg = PungiConfig("MBS-512", "1", PungiSourceType.MODULE,

-                                 "testmodule-master")

+                                 "testmodule:master:1:1")

          pungi_cfg.get_pungi_config()

          variants = pungi_cfg.get_variants_config()

          comps = pungi_cfg.get_comps_config()

@@ -86,7 +86,7 @@ 

  

          with patch("odcs.server.pungi.conf.pungi_conf_path", mock_path):

              pungi_cfg = PungiConfig("MBS-512", "1", PungiSourceType.MODULE,

-                                     "testmodule-master")

+                                     "testmodule:master:1:1")

              template = pungi_cfg.get_pungi_config()

              cfg = self._load_pungi_cfg(template)

              self.assertEqual(cfg["release_name"], "MBS-512")

@@ -98,7 +98,7 @@ 

      @patch("odcs.server.pungi.log")

      def test_get_pungi_conf_exception(self, log):

          pungi_cfg = PungiConfig("MBS-512", "1", PungiSourceType.MODULE,

-                                 "testmodule-master")

+                                 "testmodule:master:1:1")

          _, mock_path = tempfile.mkstemp(suffix='-pungi.conf')

          with open(mock_path, 'w') as f:

              # write an invalid jinja2 template file

@@ -116,7 +116,7 @@ 

  

          with patch("odcs.server.pungi.conf.pungi_conf_path", mock_path):

              pungi_cfg = PungiConfig("MBS-512", "1", PungiSourceType.MODULE,

-                                     "testmodule-master",

+                                     "testmodule:master:1:1",

                                      results=COMPOSE_RESULTS["iso"])

              template = pungi_cfg.get_pungi_config()

              cfg = self._load_pungi_cfg(template)

@@ -130,7 +130,7 @@ 

  

          with patch("odcs.server.pungi.conf.pungi_conf_path", mock_path):

              pungi_cfg = PungiConfig("MBS-512", "1", PungiSourceType.MODULE,

-                                     "testmodule-master",

+                                     "testmodule:master:1:1",

                                      results=COMPOSE_RESULTS["boot.iso"])

              template = pungi_cfg.get_pungi_config()

              cfg = self._load_pungi_cfg(template)

@@ -301,6 +301,24 @@ 

                  cfg["gather_lookaside_repos"],

                  [(u'^.*$', {u'*': [u'foo', u'bar']})])

  

+     def test_get_pungi_conf_include_devel_modules(self):

+         _, mock_path = tempfile.mkstemp()

+         template_path = os.path.abspath(os.path.join(test_dir,

+                                                      "../conf/pungi.conf"))

+         shutil.copy2(template_path, mock_path)

+ 

+         with patch("odcs.server.pungi.conf.pungi_conf_path", mock_path):

+             pungi_cfg = PungiConfig(

+                 "MBS-512", "1", PungiSourceType.MODULE,

+                 "foo:1:1:1 foo-devel:1:1:1 bar-devel:1:1:1")

+ 

+             template = pungi_cfg.get_pungi_config()

+             cfg = self._load_pungi_cfg(template)

+             self.assertEqual(

+                 cfg["include_devel_modules"],

+                 {"Temporary": ["foo-devel:1"]})

+             self.assertEqual(pungi_cfg.source, "foo:1:1:1 bar-devel:1:1:1")

+ 

  

  class TestPungi(unittest.TestCase):

  

@@ -329,7 +347,7 @@ 

      @patch("odcs.server.utils.execute_cmd")

      def test_pungi_run(self, execute_cmd):

          pungi_cfg = PungiConfig("MBS-512", "1", PungiSourceType.MODULE,

-                                 "testmodule-master")

+                                 "testmodule:master:1:1")

          pungi = Pungi(pungi_cfg)

          pungi.run(self.compose)

  

@@ -502,7 +520,7 @@ 

          self.koji_session.getTaskInfo.return_value = {"state": koji.TASK_STATES["CLOSED"]}

  

          pungi_cfg = PungiConfig("MBS-512", "1", PungiSourceType.MODULE,

-                                 "testmodule-master")

+                                 "testmodule:master:1:1")

          pungi = Pungi(pungi_cfg)

          pungi.run(self.compose)

  

The -devel modules do not exist in the MBS database, but they exist
only as CG build in Koji. Therefore the current code fails, because
it only queries MBS to get the modules.

This is also made worse by a fact that Pungi actually cannot handle
-devel CG module builds because of known issues. Therefore we cannot
simply pass the module NSVCs down to Koji, but we need to handle
them in a special way.

This commit detects the situation when -devel NSVC cannot be found
in the MBS and in this case it tries to find the non-devel version.
This is needed to be able to resolve uncomplete NSVC (for example
just name:stream) to full NSVC.

The -devel modules are then passed down to Pungi using the
include_devel_modules option, while non-devel modules are stored
in variant.xml as usually.

This all is only temporary implementation to make things work until
Pungi can handle normal -devel modules and use the metadata from
the CG builds.

You could do this to simplify the str join later on:

n, svc = nsvc.split(':', 1)
if n.endswith("-devel"):
  params["nsvc"] = n[:-len("-devel")] + ':' + svc

This of course, assumes that nsvc will always have at least name and version.

Out of curiosity, why just use NS instead of NSVC?

@lucarval, that's how Pungi defines it. The idea is that you have the non-devel module fully specified in the self.source (this is later stored in variants.xml input file for Pungi) and you just tell Pungi that for the module with this "name:stream", you also want to include -devel counterpart in a compose.

I think it is like that to just keep the Pungi configuration simpler, so RCM people don't have to change the include_devel_modules everytime new module is released (because NSVC would change on every new module build).

Thanks. Yeah, there will always be name and version. That's the minimum module description we accept in odcs.

:+1:

I'll leave it up to you to decide whether or not to take the code pattern suggestion. Code looks fine either way :)

Pull-Request has been merged by jkaluza

5 months ago