#235 Add support for building composes including particular Koji builds.
Merged 9 months ago by jkaluza. Opened 9 months ago by jkaluza.
jkaluza/odcs extra-builds  into  master

file modified
+3

@@ -125,6 +125,7 @@ 

  | module        | White-space separated NAME:STREAM or NAME:STREAM:VERSION of modules to include in compose. |

  | pulp          | White-space separated list of content-sets. Repositories defined by these content-sets will be included in a compose. |

  | raw_config    | String in `name#commit` hash format. The `name` must match one of the raw config locations defined in ODCS server config as `raw_config_urls`. The `commit` is commit hash defining the version of raw config to use. This config is then used as input config for Pungi. |

+ | build         | Source should be omitted in the request. The list of Koji builds included in a compose is defined by `builds` attribute. |

  

  There are also additional optional attributes you can pass to `new_compose(...)` method:

  

@@ -144,6 +145,8 @@ 

      - `runtime` - Packages whose name ends with "-devel" or "-static" suffix will be considered as multilib. 

      - `devel` - Packages that install some shared object file "*.so.*" will be considered as multilib. 

      - `all` - All pakages will be considered as multilib.

+ - `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.

The packages still need to be set to include particular packages from the Koji builds in a compose.

This only applies to tag source type, right?

No, even for Koji builds, you might want to include just certain packages built as part of that Koji build.

We might want to allow unset or empty packages and include all the RPMs in a compose, but this is not part of this PR.

  

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

  

file modified
+6 -2

@@ -77,10 +77,10 @@ 

  create_parser.set_defaults(command='create')

  create_parser.add_argument(

      'source_type', default=None,

-     choices=['tag', 'module', 'raw_config', 'pulp'],

+     choices=['tag', 'module', 'raw_config', 'pulp', 'build'],

      help="Type for the source. Must be 'tag' or 'module'")

  create_parser.add_argument(

-     'source', default=None,

+     'source', default="",

Why this change? Usually None is a better default value than "" in most cases.

That's because pungi expects empty string and not None, but I admit I can do that change in ODCS server-side and keep the client side the same. I will rework this bit.

      help="Source for the compose. May be a koji tag or a "

      "whitespace separated list of modules.")

  create_parser.add_argument(

@@ -99,6 +99,9 @@ 

  create_parser.add_argument(

      'packages', metavar='package', nargs='*',

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

+ create_parser.add_argument(

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

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

  

  

  wait_parser = subparsers.add_parser(

@@ -184,6 +187,7 @@ 

              sigkeys=args.sigkey,

              flags=args.flag,

              arches=args.arch,

+             builds=args.builds,

          )

      elif args.command == "wait":

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

file modified
+4 -1

@@ -211,7 +211,8 @@ 

  

      def new_compose(self, source, source_type,

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

-                     sigkeys=None, results=None, arches=None):

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

+                     builds=None):

          """Request a new compose

  

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

@@ -245,6 +246,8 @@ 

          }

          if packages:

              request_data['source']['packages'] = packages

+         if builds:

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

          if sigkeys:

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

          if seconds_to_live is not None:

@@ -28,6 +28,7 @@ 

      REPO = 3

      PULP = 4

      RAW_CONFIG = 5

+     BUILD = 6

  

  

  PUNGI_SOURCE_TYPE_NAMES = {

@@ -41,6 +42,9 @@ 

      # This allows to submit raw pungi config from predefined URLs in ODCS

      # server-side configuration.

      "raw_config": PungiSourceType.RAW_CONFIG,

+     # Generates compose using exactly defined set of Koji builds without

+     # pulling in RPMs from any Koji tag.

+     "build": PungiSourceType.BUILD,

  }

  

  INVERSE_PUNGI_SOURCE_TYPE_NAMES = {

file modified
+7

@@ -37,8 +37,15 @@ 

  pkgset_source = 'koji'

  {%- if config.koji_tag %}

  pkgset_koji_tag = '{{ config.koji_tag }}'

+ {%- else %}

+ pkgset_koji_tag = ""

  {%- endif %}

  pkgset_koji_inherit = {{ config.pkgset_koji_inherit }}

+ pkgset_koji_builds = [

+ {%- for build in config.builds %}

+     '{{ build }}',

+ {%- endfor %}

+ ]

  {%- endif %}

  

  filter_system_release_packages = False

@@ -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
+14 -1

@@ -381,6 +381,15 @@ 

                        old_compose)

              continue

  

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

+             if compose.builds else set()

+         old_builds = set(old_compose.builds.split(" ")) \

+             if old_compose.builds else set()

+         if builds != old_builds:

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

+                       old_compose)

+             continue

+ 

          source = set(compose.source.split(" "))

          old_source = set(old_compose.source.split(" "))

          if source != old_source:

@@ -567,6 +576,9 @@ 

      packages = compose.packages

      if packages:

          packages = packages.split(" ")

+     builds = compose.builds

+     if builds:

+         builds = builds.split(" ")

  

      # Resolve the general data in the compose.

      resolve_compose(compose)

@@ -592,7 +604,8 @@ 

                                      results=compose.results,

                                      arches=compose.arches.split(" "),

                                      multilib_arches=multilib_arches,

-                                     multilib_method=compose.multilib_method)

+                                     multilib_method=compose.multilib_method,

+                                     builds=builds)

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

                  pungi_cfg.gather_method = "nodeps"

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

file modified
+1 -1

@@ -199,7 +199,7 @@ 

              'desc': 'Number of concurrent Pungi processes.'},

          'allowed_source_types': {

              'type': list,

-             'default': ["tag", "module"],

+             'default': ["tag", "module", "build"],

              'desc': 'Allowed source types.'},

          'allowed_flags': {

              'type': list,

@@ -0,0 +1,22 @@ 

+ """Add Compose.builds.

+ 

+ Revision ID: 4514febd31fa

+ Revises: d1da07e15c54

+ Create Date: 2018-10-25 13:28:01.798873

+ 

+ """

+ 

+ # revision identifiers, used by Alembic.

+ revision = '4514febd31fa'

+ down_revision = 'd1da07e15c54'

+ 

+ from alembic import op

+ import sqlalchemy as sa

+ 

+ 

+ def upgrade():

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

+ 

+ 

+ def downgrade():

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

file modified
+8 -2

@@ -116,6 +116,8 @@ 

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

      # White-space separated list of packages

      packages = db.Column(db.String)

+     # White-space separated list of builds (NVR) to include in a compose.

+     builds = db.Column(db.String)

      # COMPOSE_FLAGS

      flags = db.Column(db.Integer)

      time_to_expire = db.Column(db.DateTime, nullable=False, index=True)

@@ -138,7 +140,8 @@ 

      @classmethod

      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):

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

+                builds=None):

          now = datetime.utcnow()

          compose = cls(

              owner=owner,

@@ -153,7 +156,8 @@ 

              flags=flags,

              arches=arches if arches else " ".join(conf.arches),

              multilib_arches=multilib_arches if multilib_arches else "",

-             multilib_method=multilib_method if multilib_method else 0

+             multilib_method=multilib_method if multilib_method else 0,

+             builds=builds,

          )

          session.add(compose)

          return compose

@@ -178,6 +182,7 @@ 

              time_submitted=now,

              time_to_expire=now + timedelta(seconds=seconds_to_live),

              packages=compose.packages,

+             builds=compose.builds,

              flags=compose.flags,

              koji_event=compose.koji_event,

              arches=compose.arches,

@@ -307,6 +312,7 @@ 

              'koji_event': self.koji_event,

              'koji_task_id': self.koji_task_id,

              'packages': self.packages,

+             'builds': self.builds,

              'arches': self.arches,

              'multilib_arches': self.multilib_arches,

              'multilib_method': self.multilib_method,

file modified
+6 -1

@@ -110,7 +110,7 @@ 

  class PungiConfig(BasePungiConfig):

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

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

-                  multilib_arches=None, multilib_method=0):

+                  multilib_arches=None, multilib_method=0, builds=None):

          self.release_name = release_name

          self.release_version = release_version

          self.bootable = False

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

          else:

              self.arches = conf.arches

          self.packages = packages or []

+         self.builds = builds or []

  

          # Store results as list of strings, so it can be used by jinja2

          # templates.

@@ -158,6 +159,10 @@ 

              self.gather_source = "comps"

              self.gather_method = "deps"

              self.koji_tag = None

+         elif source_type == PungiSourceType.BUILD:

+             self.gather_source = "comps"

+             self.gather_method = "deps"

+             self.koji_tag = None

          else:

              raise ValueError("Unknown source_type %r" % source_type)

  

file modified
+12 -5

@@ -200,7 +200,7 @@ 

              log.error(err)

              raise ValueError(err)

  

-         needed_keys = ["type", "source"]

+         needed_keys = ["type"]

          for key in needed_keys:

              if key not in source_data:

                  err = "Missing %s in source configuration, received: %s" % (key, str(source_data))

@@ -215,9 +215,12 @@ 

  

          source_type = PUNGI_SOURCE_TYPE_NAMES[source_type]

  

-         # Use list(set()) here to remove duplicate sources.

-         source = list(set(source_data["source"].split(" ")))

-         if not source:

+         source = []

+         if "source" in source_data:

+             # Use list(set()) here to remove duplicate sources.

+             source = list(set(source_data["source"].split(" ")))

+ 

+         if not source and source_type != PungiSourceType.BUILD:

              err = "No source provided for %s" % source_type

              log.error(err)

              raise ValueError(err)

@@ -259,6 +262,10 @@ 

          if "packages" in source_data:

              packages = ' '.join(source_data["packages"])

  

+         builds = None

+         if "builds" in source_data:

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

+ 

          if not packages and source_type == PungiSourceType.KOJI_TAG:

              raise ValueError(

                  '"packages" must be defined for "tag" source_type.')

@@ -308,7 +315,7 @@ 

              db.session, self._get_compose_owner(), source_type, source,

              results, seconds_to_live,

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

-             multilib_method=multilib_method)

+             multilib_method=multilib_method, builds=builds)

          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

file modified
+26 -2

@@ -304,7 +304,8 @@ 

      def test_get_reusable_compose_attrs_not_the_same(self):

          old_c = Compose.create(

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

-             COMPOSE_RESULTS["repository"], 3600, packages="ed", sigkeys="123")

+             COMPOSE_RESULTS["repository"], 3600, packages="ed", sigkeys="123",

+             builds="foo-1-1")

          old_c.state = COMPOSE_STATES["done"]

          resolve_compose(old_c)

          db.session.add(old_c)

@@ -312,6 +313,7 @@ 

  

          attrs = {}

          attrs["packages"] = "ed foo"

+         attrs["builds"] = "foo-1-1 bar-1-1"

          attrs["sigkeys"] = "321"

          attrs["koji_event"] = 123456

          attrs["source"] = "123"

@@ -321,7 +323,8 @@ 

          for attr, value in attrs.items():

              c = Compose.create(

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

-                 COMPOSE_RESULTS["repository"], 3600, packages="ed", sigkeys="123")

+                 COMPOSE_RESULTS["repository"], 3600, packages="ed", sigkeys="123",

+                 builds="foo-1-1")

              setattr(c, attr, value)

  

              # Do not resolve compose for non-existing source and in case we

@@ -716,6 +719,27 @@ 

          self.assertEqual(self.pungi_config.gather_method, "deps")

          self.assertEqual(self.pungi_config.pkgset_koji_inherit, False)

  

+     def test_generate_pungi_compose_builds(self):

+         c = Compose.create(

+             db.session, "me", PungiSourceType.KOJI_TAG, "f26",

+             COMPOSE_RESULTS["repository"], 60, builds='foo-1-1 bar-1-1',

+             flags=COMPOSE_FLAGS["no_inheritance"])

+         c.id = 1

+ 

+         generate_pungi_compose(c)

+         self.assertEqual(self.pungi_config.builds, ["foo-1-1", "bar-1-1"])

+ 

+     def test_generate_pungi_compose_source_type_build(self):

+         c = Compose.create(

+             db.session, "me", PungiSourceType.BUILD, "x",

+             COMPOSE_RESULTS["repository"], 60, builds='foo-1-1 bar-1-1',

+             flags=COMPOSE_FLAGS["no_inheritance"])

+         c.id = 1

+ 

+         generate_pungi_compose(c)

+         self.assertEqual(self.pungi_config.koji_tag, None)

+         self.assertEqual(self.pungi_config.builds, ["foo-1-1", "bar-1-1"])

+ 

      @patch.object(odcs.server.config.Config, 'raw_config_urls',

                    new={

                        "pungi_cfg": {

@@ -65,6 +65,7 @@ 

                           'koji_event': None,

                           'koji_task_id': None,

                           'packages': None,

+                          'builds': None,

                           'arches': 'x86_64',

                           'multilib_arches': '',

                           'multilib_method': 0}

@@ -174,6 +174,37 @@ 

                  for method in arch_method_dict.values():

                      self.assertEqual(set(method), set(['runtime', 'devel']))

  

+     def test_get_pungi_conf_pkgset_koji_builds(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", builds=["foo-1-1", "bar-1-1"])

+ 

+             template = pungi_cfg.get_pungi_config()

+             cfg = self._load_pungi_cfg(template)

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

+                              set(["foo-1-1", "bar-1-1"]))

+ 

+     def test_get_pungi_conf_source_type_build(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.BUILD,

+                                     "x", builds=["foo-1-1", "bar-1-1"])

+ 

+             template = pungi_cfg.get_pungi_config()

+             cfg = self._load_pungi_cfg(template)

+             self.assertEqual(cfg["pkgset_koji_tag"], '')

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

+                              set(["foo-1-1", "bar-1-1"]))

+ 

  

  class TestPungi(unittest.TestCase):

  

@@ -300,6 +300,7 @@ 

                           'koji_event': None,

                           'koji_task_id': None,

                           'packages': None,

+                          'builds': None,

                           'arches': 'x86_64',

                           'multilib_arches': '',

                           'multilib_method': 0}

@@ -498,6 +499,43 @@ 

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

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

  

+     def test_submit_build_extra_builds(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', 'packages': ['ed'],

+                             'builds': ['foo-1-1', 'bar-1-1']}}))

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

+ 

+         self.assertEqual(data['builds'], 'foo-1-1 bar-1-1')

+ 

+         db.session.expire_all()

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

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

+         self.assertEqual(c.builds, 'foo-1-1 bar-1-1')

+ 

+     def test_submit_build_source_type_build(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': 'build', 'packages': ['ed'],

+                             'builds': ['foo-1-1', 'bar-1-1']}}))

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

+ 

+         self.assertEqual(data['builds'], 'foo-1-1 bar-1-1')

+ 

+         db.session.expire_all()

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

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

+         self.assertEqual(c.source_type, PungiSourceType.BUILD)

+         self.assertEqual(c.builds, 'foo-1-1 bar-1-1')

+ 

      def test_submit_build_resurrection_removed(self):

          self.c1.state = COMPOSE_STATES["removed"]

          self.c1.reused_id = 1

@@ -964,6 +1002,7 @@ 

                           'koji_event': None,

                           'koji_task_id': None,

                           'packages': None,

+                          'builds': None,

                           'arches': 'x86_64',

                           'multilib_arches': '',

                           'multilib_method': 0}

Pungi now supports including Koji builds defined by particular NVR
using the pkgset_koji_builds option. This PR adds support for this
new feature in ODCS:

  • For tag source_type, additional builds list in REST API can be used
    to define extra builds which will be considered for inclusion in a compose
    on top of the builds comming from the Koji tag.
  • New build source_type is introduced which can be used to build a compose
    including just Koji builds from builds list without any Koji tag.

The packages still need to be set to include particular packages from the Koji builds in a compose.

This only applies to tag source type, right?

Why this change? Usually None is a better default value than "" in most cases.

No, even for Koji builds, you might want to include just certain packages built as part of that Koji build.

We might want to allow unset or empty packages and include all the RPMs in a compose, but this is not part of this PR.

That's because pungi expects empty string and not None, but I admit I can do that change in ODCS server-side and keep the client side the same. I will rework this bit.

Pull-Request has been merged by jkaluza

9 months ago