#251 Add support for hybrid composes - composes with both modular and normal RPMs.
Merged 5 months ago by jkaluza. Opened 6 months ago by jkaluza.
jkaluza/odcs modular-tag  into  master

file modified
+2

@@ -151,6 +151,8 @@ 

  - `builds` - List of NVRs defining the Koji builds to include in a compose. Only valid for `tag` and `build` source types. For `tag` source type, the NVRs will be considered

  for inclusion in a compose on top of Koji tag defined by `source`. For `build` source type, only the Koji builds defined by the NVRs will be considered for inclusion. The `packages` still need to be set to include particular packages from the Koji builds in a compose.

  - `lookaside_repos` - List of base URLs of RPM repositories which should be considered when choosing packages for a compose.

+ - `module_defaults_url` - List with URL to git repository with Module defaults data and the branch name or commit hash. For example ["https://pagure.io/releng/fedora-module-defaults.git", "master"]. This is used only when creating modular compose including non-modular RPMs.

+ - `modular_koji_tags` - List of Koji tags in which the modular Koji Content Generator builds are tagged. Such builds will be included in a compose.

  

  The `new_compose` method returns `dict` object describing the compose, for example:

  

file modified
+14

@@ -102,6 +102,18 @@ 

  create_parser.add_argument(

      'builds', metavar='build', nargs='*',

      help='Builds to be included in the compose.')

+ create_parser.add_argument(

+     '--module-defaults-url', default="",

+     metavar='module_defaults_url',

+     help="URL to git repository with module defaults.")

+ create_parser.add_argument(

+     '--module-defaults-commit', default="",

+     metavar='module_defaults_commit',

+     help="Git commit/branch from which to take the module defaults.")

+ create_parser.add_argument(

+     '--modular-tag', default=[], action='append',

+     metavar="modular_koji_tags",

+     help="Koji tag with module builds.")

  

  

  wait_parser = subparsers.add_parser(

@@ -188,6 +200,8 @@ 

              flags=args.flag,

              arches=args.arch,

              builds=args.builds,

+             module_defaults_url=module_defaults_url,

+             module_defaults_commit=module_defaults_commit,

          )

      elif args.command == "wait":

          result = {"id": int(args.compose_id)}

file modified
+8 -1

@@ -212,7 +212,8 @@ 

      def new_compose(self, source, source_type,

                      seconds_to_live=None, packages=[], flags=[],

                      sigkeys=None, results=None, arches=None,

-                     builds=None):

+                     builds=None, modular_koji_tags=None,

+                     module_defaults_url=None, module_defaults_commit=None):

          """Request a new compose

  

          :param str source: from where to grab and make new compose, different

@@ -250,6 +251,12 @@ 

              request_data['source']['builds'] = builds

          if sigkeys:

              request_data['source']['sigkeys'] = sigkeys

+         if modular_koji_tags:

+             request_data['source']['modular_koji_tags'] = modular_koji_tags

+         if module_defaults_url:

+             request_data['source']['module_defaults_url'] = module_defaults_url

+         if module_defaults_commit:

+             request_data['source']['module_defaults_commit'] = module_defaults_commit

          if seconds_to_live is not None:

              request_data['seconds-to-live'] = seconds_to_live

          if flags:

file modified
+19

@@ -35,11 +35,30 @@ 

  }

  {%- elif config.pkgset_source == 'koji' %}

  pkgset_source = 'koji'

+ 

  {%- if config.koji_tag %}

  pkgset_koji_tag = '{{ config.koji_tag }}'

  {%- else %}

  pkgset_koji_tag = ""

  {%- endif %}

+ 

+ {%- if config.koji_module_tags %}

+ pkgset_koji_module_tag = [

+ {%- for tag in config.koji_module_tags %}

+     '{{ tag }}',

+ {%- endfor %}

+ ]

+ {%- endif %}

+ 

+ {%- if config.module_defaults_url %}

+ module_defaults_dir = {

+     "scm": "git",

+     "repo": '{{ config.module_defaults_url[0] }}',

+     "branch": '{{ config.module_defaults_url[1] }}',

+     "dir": ".",

+ }

+ {%- endif %}

+ 

  pkgset_koji_inherit = {{ config.pkgset_koji_inherit }}

  pkgset_koji_builds = [

  {%- for build in config.builds %}

@@ -170,7 +170,7 @@ 

              # not exploitable.

              if last_dict_key in ["packages"]:

                  continue

-             allowed_chars = [' ', '-', '/', '_', '.', ':', '#', '+', '$']

+             allowed_chars = [' ', '-', '/', '_', '.', ':', '#', '+', '?', '$']

              if not all(c.isalnum() or c in allowed_chars for c in v):

                  raise ValueError(

                      "Only alphanumerical characters and %r characters "

file modified
+19 -1

@@ -488,6 +488,22 @@ 

                        old_compose)

              continue

  

+         modular_koji_tags = set(compose.modular_koji_tags.split(" ")) \

+             if compose.modular_koji_tags else set()

+         old_modular_koji_tags = set(old_compose.modular_koji_tags.split(" ")) \

+             if old_compose.modular_koji_tags else set()

+         if modular_koji_tags != old_modular_koji_tags:

+             log.debug("%r: Cannot reuse %r - modular_koji_tags not same", compose,

+                       old_compose)

+             continue

+ 

+         module_defaults_url = compose.module_defaults_url

+         old_module_defaults_url = old_compose.module_defaults_url

+         if module_defaults_url != old_module_defaults_url:

+             log.debug("%r: Cannot reuse %r - module_defaults_url not same", compose,

+                       old_compose)

+             continue

+ 

          # In case of compose renewal, the compose.koji_event will be actually

          # lower than the "old_compose"'s one - the `compose` might have been for

          # example submitted 1 year ago, so koji_event will be one year old.

@@ -652,7 +668,9 @@ 

                                      multilib_arches=multilib_arches,

                                      multilib_method=compose.multilib_method,

                                      builds=builds, flags=compose.flags,

-                                     lookaside_repos=compose.lookaside_repos)

+                                     lookaside_repos=compose.lookaside_repos,

+                                     modular_koji_tags=compose.modular_koji_tags,

+                                     module_defaults_url=compose.module_defaults_url)

              if compose.flags & COMPOSE_FLAGS["no_deps"]:

                  pungi_cfg.gather_method = "nodeps"

              if compose.flags & COMPOSE_FLAGS["no_inheritance"]:

@@ -0,0 +1,24 @@ 

+ """Add modular_koji_tags and module_defaults_url columns.

+ 

+ Revision ID: e186faabdafe

+ Revises: b2725d046624

+ Create Date: 2019-01-28 08:15:35.059106

+ 

+ """

+ 

+ # revision identifiers, used by Alembic.

+ revision = 'e186faabdafe'

+ down_revision = 'b2725d046624'

+ 

+ from alembic import op

+ import sqlalchemy as sa

+ 

+ 

+ def upgrade():

+     op.add_column('composes', sa.Column('modular_koji_tags', sa.String(), nullable=True))

+     op.add_column('composes', sa.Column('module_defaults_url', sa.String(), nullable=True))

+ 

+ 

+ def downgrade():

+     op.drop_column('composes', 'module_defaults_url')

+     op.drop_column('composes', 'modular_koji_tags')

file modified
+12 -1

@@ -103,6 +103,10 @@ 

      source_type = db.Column(db.Integer, nullable=False)

      # White-space separated list of koji_tags or modules

      source = db.Column(db.String, nullable=False)

+     # White-space separated list of modular Koji tags.

+     modular_koji_tags = db.Column(db.String)

+     # URL on which the module defaults can be found.

+     module_defaults_url = db.Column(db.String)

      # Koji event id at which the compose has been generated

      koji_event = db.Column(db.Integer)

      # White-space separated list sigkeys to define the key using which

@@ -143,7 +147,8 @@ 

      def create(cls, session, owner, source_type, source, results,

                 seconds_to_live, packages=None, flags=0, sigkeys=None,

                 arches=None, multilib_arches=None, multilib_method=None,

-                builds=None, lookaside_repos=None):

+                builds=None, lookaside_repos=None, modular_koji_tags=None,

+                module_defaults_url=None):

          now = datetime.utcnow()

          compose = cls(

              owner=owner,

@@ -161,6 +166,8 @@ 

              multilib_method=multilib_method if multilib_method else 0,

              builds=builds,

              lookaside_repos=lookaside_repos,

+             modular_koji_tags=modular_koji_tags,

+             module_defaults_url=module_defaults_url,

          )

          session.add(compose)

          return compose

@@ -193,6 +200,8 @@ 

              multilib_method=compose.multilib_method,

              sigkeys=compose.sigkeys,

              lookaside_repos=compose.lookaside_repos,

+             modular_koji_tags=compose.modular_koji_tags,

+             module_defaults_url=compose.module_defaults_url,

          )

          session.add(compose)

          return compose

@@ -321,6 +330,8 @@ 

              'multilib_arches': self.multilib_arches,

              'multilib_method': self.multilib_method,

              'lookaside_repos': self.lookaside_repos,

+             'modular_koji_tags': self.modular_koji_tags,

+             'module_defaults_url': self.module_defaults_url,

          }

  

      @staticmethod

file modified
+8 -2

@@ -113,7 +113,8 @@ 

      def __init__(self, release_name, release_version, source_type, source,

                   packages=None, arches=None, sigkeys=None, results=0,

                   multilib_arches=None, multilib_method=0, builds=None,

-                  flags=0, lookaside_repos=None):

+                  flags=0, lookaside_repos=None, modular_koji_tags=None,

+                  module_defaults_url=None):

          self.release_name = release_name

          self.release_version = release_version

          self.bootable = False

@@ -148,9 +149,14 @@ 

              self.bootable = True

  

          if source_type == PungiSourceType.KOJI_TAG:

+             self.koji_module_tags = modular_koji_tags.split(" ") if modular_koji_tags else []

+             self.module_defaults_url = module_defaults_url.split(" ") if module_defaults_url else []

              self.koji_tag = source

              self.gather_source = "comps"

-             self.gather_method = "deps"

+             if self.koji_module_tags:

+                 self.gather_method = "hybrid"

+             else:

+                 self.gather_method = "deps"

          elif source_type == PungiSourceType.MODULE:

              self.koji_tag = None

              self.gather_source = "module"

file modified
+24 -1

@@ -308,6 +308,27 @@ 

                      raise ValueError("Unknown multilib method \"%s\"" % name)

                  multilib_method |= MULTILIB_METHODS[name]

  

+         modular_koji_tags = None

+         if "modular_koji_tags" in source_data:

+             modular_koji_tags = ' '.join(source_data["modular_koji_tags"])

+ 

+         module_defaults_url = None

+         if "module_defaults_url" in source_data:

+             module_defaults_url = source_data["module_defaults_url"]

+ 

+         module_defaults_commit = None

+         if "module_defaults_commit" in source_data:

+             module_defaults_commit = source_data["module_defaults_commit"]

+ 

+         module_defaults = None

+         # The "^" operator is logical XOR.

+         if bool(module_defaults_url) ^ bool(module_defaults_commit):

+             raise ValueError(

+                 'The "module_defaults_url" and "module_defaults_commit" '

+                 'must be used together.')

+         elif module_defaults_url and module_defaults_commit:

+             module_defaults = "%s %s" % (module_defaults_url, module_defaults_commit)

+ 

          raise_if_input_not_allowed(

              source_types=source_type, sources=source, results=results,

              flags=flags, arches=arches)

@@ -317,7 +338,9 @@ 

              results, seconds_to_live,

              packages, flags, sigkeys, arches, multilib_arches=multilib_arches,

              multilib_method=multilib_method, builds=builds,

-             lookaside_repos=lookaside_repos)

+             lookaside_repos=lookaside_repos,

+             modular_koji_tags=modular_koji_tags,

+             module_defaults_url=module_defaults)

          db.session.add(compose)

          # Flush is needed, because we use `before_commit` SQLAlchemy event to

          # send message and before_commit can be called before flush and

@@ -390,6 +390,8 @@ 

          attrs["multilib_arches"] = "x86_64 i686"

          attrs["multilib_method"] = 1

          attrs["lookaside_repos"] = "foo bar"

+         attrs["modular_koji_tags"] = "f26-modules"

+         attrs["module_defaults_url"] = "git://localhost/x.git#branch"

          for attr, value in attrs.items():

              c = Compose.create(

                  db.session, "me", PungiSourceType.REPO, os.path.join(thisdir, "repo"),

file modified
+3 -1

@@ -69,7 +69,9 @@ 

                           'arches': 'x86_64',

                           'multilib_arches': '',

                           'multilib_method': 0,

-                          'lookaside_repos': None}

+                          'lookaside_repos': None,

+                          'modular_koji_tags': None,

+                          'module_defaults_url': None}

          self.assertEqual(c.json(), expected_json)

  

      def test_create_copy(self):

@@ -211,6 +211,29 @@ 

              self.assertEqual(cfg["additional_packages"],

                               [(u'^Temporary$', {u'*': [u'*']})])

  

+     def test_get_pungi_conf_modular_koji_tags(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.KOJI_TAG, "f26",

+                 modular_koji_tags="f26-modules",

+                 module_defaults_url="git://localhost.tld/x.git master")

+ 

+             template = pungi_cfg.get_pungi_config()

+             cfg = self._load_pungi_cfg(template)

+             self.assertEqual(set(cfg["pkgset_koji_module_tag"]),

+                              set(["f26-modules"]))

+             self.assertEqual(cfg["gather_method"], "hybrid")

+             self.assertEqual(cfg["module_defaults_dir"], {

+                 'branch': 'master',

+                 'dir': '.',

+                 'repo': 'git://localhost.tld/x.git',

+                 'scm': 'git'})

+ 

      def test_get_pungi_conf_source_type_build(self):

          _, mock_path = tempfile.mkstemp()

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

file modified
+57 -2

@@ -305,7 +305,9 @@ 

                           'arches': 'x86_64',

                           'multilib_arches': '',

                           'multilib_method': 0,

-                          'lookaside_repos': ''}

+                          'lookaside_repos': '',

+                          'modular_koji_tags': None,

+                          'module_defaults_url': None}

          self.assertEqual(data, expected_json)

  

          db.session.expire_all()

@@ -487,6 +489,57 @@ 

          self.assertEqual(

              data['message'], 'Unknown multilib method "foo"')

  

+     def test_submit_build_modular_koji_tags(self):

+         with self.test_request_context(user='dev'):

+             flask.g.oidc_scopes = [

+                 '{0}{1}'.format(conf.oidc_base_namespace, 'new-compose')

+             ]

+ 

+             rv = self.client.post('/api/1/composes/', data=json.dumps(

+                 {'source': {'type': 'tag', 'source': 'f26',

+                             'modular_koji_tags': ['f26-modules']}}))

+             data = json.loads(rv.get_data(as_text=True))

+ 

+         self.assertEqual(data['modular_koji_tags'], "f26-modules")

+ 

+         db.session.expire_all()

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

+         self.assertEqual(c.state, COMPOSE_STATES["wait"])

+ 

+     def test_submit_build_module_defaults_url(self):

+         with self.test_request_context(user='dev'):

+             flask.g.oidc_scopes = [

+                 '{0}{1}'.format(conf.oidc_base_namespace, 'new-compose')

+             ]

+ 

+             rv = self.client.post('/api/1/composes/', data=json.dumps(

+                 {'source': {'type': 'tag', 'source': 'f26',

+                             'module_defaults_url': 'git://localhost.tld/x.git',

+                             'module_defaults_commit': 'master'}}))

+             data = json.loads(rv.get_data(as_text=True))

+ 

+         self.assertEqual(data['module_defaults_url'], 'git://localhost.tld/x.git master')

+ 

+         db.session.expire_all()

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

+         self.assertEqual(c.state, COMPOSE_STATES["wait"])

+ 

+     def test_submit_build_module_defaults_url_no_branch(self):

+         with self.test_request_context(user='dev'):

+             flask.g.oidc_scopes = [

+                 '{0}{1}'.format(conf.oidc_base_namespace, 'new-compose')

+             ]

+ 

+             rv = self.client.post('/api/1/composes/', data=json.dumps(

+                 {'source': {'type': 'tag', 'source': 'f26',

+                             'module_defaults_url': 'git://localhost.tld/x.git'}}))

+             data = json.loads(rv.get_data(as_text=True))

+             self.assertEqual(data['status'], 400)

+             self.assertEqual(data['error'], 'Bad Request')

+             self.assertEqual(data['message'],

+                              'The "module_defaults_url" and "module_defaults_commit" '

+                              'must be used together.')

+ 

      def test_submit_build_duplicate_sources(self):

          with self.test_request_context(user='dev'):

              flask.g.oidc_scopes = [

@@ -1011,7 +1064,9 @@ 

                           'arches': 'x86_64',

                           'multilib_arches': '',

                           'multilib_method': 0,

-                          'lookaside_repos': ''}

+                          'lookaside_repos': '',

+                          'modular_koji_tags': None,

+                          'module_defaults_url': None}

          self.assertEqual(data, expected_json)

  

          db.session.expire_all()

This adds modular_koji_tags list to API which can list the Koji tags
in which the modular Content Generator Koji builds created by MBS
are tagged. Such builds are included in a compose together with RPMs
from classic Koji tag defined in compose source. The "hybrid" Pungi
compose gather method is used to achieve that internally.

This adds module_defaults_url string to API which can be set to git
repository with module defaults which are added to the resulting
compose metadata or which are used when resolving RPMs for compose
when using Pungi hybrid compose gather method.

Be consistent with using ?# ?

This seems a bit fragile to me. ? may or may not be there. Can we use urlparse?

@lucarval, I'm not sure why I just didn't use list or dict there instead of string. I will rewrite the module_defaults_url to use something like {"url": "git://foo.tld/x.git", "commit": "deadbeaf"}, or ["git://foo.tld/x.git", "deadbeaf"].

rebased onto 97d0ea50b273d662e77bfac0104230c5b7114d5e

6 months ago

@lucarval, ^ done, please review again :).

Does this need changing since this is now supposed to be a list with two values?

Also, since we're calling them out separately, it might be a good idea to just have url and branch being completely separate parameters.

rebased onto 48b1e58cb0028ed0037d78c9dd49957c3fe99f5f

6 months ago

rebased onto 88276144cdb2967a51c24f07455919ea5d2b7803

6 months ago

rebased onto 33718cdd31924f6a387b32e50d0049ac0a2cf97c

6 months ago

Shouldn't this be module_defaults_commit ?

This one maps module_defaultsto module_defaults_url, just want to double check that's intended. IOW, why not user _url in dict key?

rebased onto 5a449e835b63609d5626ff8bccaf39d3fb6a3524

6 months ago

rebased onto 16e68a8

5 months ago

Pull-Request has been merged by jkaluza

5 months ago