#220 Refactor PungiConfig and allow setting per raw_config pungi arguments
Merged 8 months ago by jkaluza. Opened 8 months ago by cqi.

file modified
+20

@@ -146,6 +146,21 @@ 

      #   }

      # }

  

+     # Command line arguments used to construct pungi-koji command.

+     PUNGI_KOJI_ARGS = ['--nightly']

+ 

+     # Command line argument for raw_config source type, which overwrite

+     # arguments listed PUNGI_KOJI_ARGS.

+     # If a particular raw config should have specific pungi-koji arguments, add

+     # a key/value into this option, where key should exist in the

+     # RAW_CONFIG_URLS, and value lists each arguments.

+     # If no argument is required, just omit it, or add a key/value just as

+     # mentioned, but keep value as a empty list.

+     # RAW_CONFIG_PUNGI_KOJI_ARGS = {

+     #     'my_raw_config': ['arg1', ...],

+     #     'another_raw_config': ['arg1', ...],

+     # }

+ 

  

  class DevConfiguration(BaseConfiguration):

      DEBUG = True

@@ -166,6 +181,11 @@ 

      AUTH_BACKEND = 'noauth'

      AUTH_OPENIDC_USERINFO_URI = 'https://iddev.fedorainfracloud.org/openidc/UserInfo'

  

+     KOJI_PROFILE = 'stg'

+ 

+     PUNGI_RUNROOT_KOJI_CONF_PATH = path.join(confdir, 'runroot_koji.conf')

+     RAW_CONFIG_WRAPPER_CONF_PATH = path.join(confdir, 'raw_config_wrapper.conf')

+ 

  

  class TestConfiguration(BaseConfiguration):

      LOG_BACKEND = 'console'

@@ -31,7 +31,7 @@ 

  from datetime import datetime, timedelta

  from odcs.server import log, conf, app, db

  from odcs.server.models import Compose, COMPOSE_STATES, COMPOSE_FLAGS

- from odcs.server.pungi import Pungi, PungiConfig, PungiSourceType, PungiLogs

+ from odcs.server.pungi import Pungi, PungiConfig, PungiSourceType, PungiLogs, RawPungiConfig

  from odcs.server.pulp import Pulp

  from odcs.server.cache import KojiTagCache

  from concurrent.futures import ThreadPoolExecutor

@@ -520,13 +520,7 @@ 

          reuse_compose(compose, compose_to_reuse)

      else:

          if compose.source_type == PungiSourceType.RAW_CONFIG:

-             source_name, source_hash = compose.source.split("#")

-             url_data = conf.raw_config_urls[source_name]

-             # Do not override commit hash by hash from ODCS client if it is

-             # hardcoded in the config file.

-             if "commit" not in url_data:

-                 url_data["commit"] = source_hash

-             pungi_cfg = url_data

+             pungi_cfg = RawPungiConfig(compose.source)

          else:

              # Generate PungiConfig and run Pungi

              pungi_cfg = PungiConfig(compose.name, "1", compose.source_type,

@@ -335,6 +335,18 @@ 

                      'file. This file holds Pungi configuration which should '

                      'import real pungi configuration from raw_config.conf '

                      'in order to override some variables.'},

+         'pungi_koji_args': {

+             'type': list,

+             'default': [],

+             'desc': 'Command line arguments used to construct pungi-koji '

+                     'command.'

+         },

+         'raw_config_pungi_koji_args': {

+             'type': dict,

+             'default': {},

+             'desc': 'Command line argument for raw_config source type, which '

+                     'overwrite arguments listed PUNGI_KOJI_ARGS.'

+         }

      }

  

      def __init__(self, conf_section_obj):

file modified
+105 -62

@@ -38,7 +38,76 @@ 

  from odcs.server.utils import makedirs, clone_repo, copytree

  

  

- class PungiConfig(object):

+ class BasePungiConfig(object):

+ 

+     def _write(self, path, cfg):

+         """

+         Writes configuration string `cfg` to file defined by `path`.

+ 

+         :param str path: Full path to file to write to.

+         :param str cfg: Configuration to write.

+         """

+         with open(path, "w") as f:

+             log.info("Writing %s configuration to %s.",

+                      os.path.basename(path), path)

+             f.write(cfg)

+ 

+     def write_config_files(self, topdir):

+         """Write configuration into files"""

+         raise NotImplementedError('Concrete config object must implement.')

+ 

+ 

+ class RawPungiConfig(BasePungiConfig):

+ 

+     def __init__(self, compose_source):

+         source_name, source_hash = compose_source.split("#")

+ 

+         url_data = conf.raw_config_urls[source_name]

+         # Do not override commit hash by hash from ODCS client if it is

+         # hardcoded in the config file.

+         if "commit" not in url_data:

+             url_data["commit"] = source_hash

+ 

+         self.pungi_cfg = url_data

+         self.pungi_koji_args = conf.raw_config_pungi_koji_args.get(

+             source_name, [])

+ 

+     def write_config_files(self, topdir):

+         """Write raw config files

+ 

+         :param str topdir: Directory to write the files to.

+         """

+         # In case the raw_config wrapper config is set, download the

+         # original pungi.conf as "raw_config.conf" and use

+         # the raw_config wrapper as real "pungi.conf".

+         # The reason is that wrapper config can import raw_config

+         # and override some variables.

+         if conf.raw_config_wrapper_conf_path:

+             main_cfg_path = os.path.join(topdir, "raw_config.conf")

+             shutil.copy2(conf.raw_config_wrapper_conf_path,

+                          os.path.join(topdir, "pungi.conf"))

+         else:

+             main_cfg_path = os.path.join(topdir, "pungi.conf")

+ 

+         # Clone the git repo with raw_config pungi config files.

+         repo_dir = os.path.join(topdir, "raw_config_repo")

+         clone_repo(self.pungi_cfg["url"], repo_dir,

+                    commit=self.pungi_cfg["commit"])

+ 

+         # If the 'path' is defined, copy only the files form the 'path'

+         # to topdir.

+         if "path" in self.pungi_cfg:

+             repo_dir = os.path.join(repo_dir, self.pungi_cfg["path"])

+ 

+         copytree(repo_dir, topdir)

+ 

+         # Create the "pungi.conf" from config_filename.

+         config_path = os.path.join(topdir, self.pungi_cfg["config_filename"])

+         if config_path != main_cfg_path:

+             shutil.copy2(config_path, main_cfg_path)

+ 

+ 

+ class PungiConfig(BasePungiConfig):

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

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

          self.release_name = release_name

@@ -140,75 +209,40 @@ 

                  "Failed to render pungi conf template {!r}: {}".format(conf.pungi_conf_path,

                                                                         str(e)))

  

- 

- class Pungi(object):

-     def __init__(self, pungi_cfg, koji_event=None, old_compose=None):

-         self.pungi_cfg = pungi_cfg

-         self.koji_event = koji_event

-         self.old_compose = old_compose

- 

-     def _write_cfg(self, path, cfg):

-         """

-         Writes configuration string `cfg` to file defined by `path`.

-         :param str path: Full path to file to write to.

-         :param str cfg: Configuration to write.

-         """

-         with open(path, "w") as f:

-             log.info("Writing %s configuration to %s.", os.path.basename(path), path)

-             f.write(cfg)

- 

-     def _write_cfgs(self, topdir):

+     def write_config_files(self, topdir):

          """

          Writes "pungi.conf", "variants.xml" and "comps.xml" defined in

          `self.pungi_cfg` to `topdir` directory.

+ 

          :param str topdir: Directory to write the files to.

          """

-         if type(self.pungi_cfg) == PungiConfig:

-             main_cfg = self.pungi_cfg.get_pungi_config()

-             variants_cfg = self.pungi_cfg.get_variants_config()

-             comps_cfg = self.pungi_cfg.get_comps_config()

-             log.debug("Main Pungi config:")

-             log.debug("%s", main_cfg)

-             log.debug("Variants.xml:")

-             log.debug("%s", variants_cfg)

-             log.debug("Comps.xml:")

-             log.debug("%s", comps_cfg)

- 

-             self._write_cfg(os.path.join(topdir, "pungi.conf"), main_cfg)

-             self._write_cfg(os.path.join(topdir, "variants.xml"), variants_cfg)

-             self._write_cfg(os.path.join(topdir, "comps.xml"), comps_cfg)

-         elif type(self.pungi_cfg) == dict:

-             # In case the raw_config wrapper config is set, download the

-             # original pungi.conf as "raw_config.conf" and use

-             # the raw_config wrapper as real "pungi.conf".

-             # The reason is that wrapper config can import raw_config

-             # and override some variables.

-             if conf.raw_config_wrapper_conf_path:

-                 main_cfg_path = os.path.join(topdir, "raw_config.conf")

-                 shutil.copy2(conf.raw_config_wrapper_conf_path,

-                              os.path.join(topdir, "pungi.conf"))

-             else:

-                 main_cfg_path = os.path.join(topdir, "pungi.conf")

+         main_cfg = self.get_pungi_config()

+         variants_cfg = self.get_variants_config()

+         comps_cfg = self.get_comps_config()

+         log.debug("Main Pungi config:")

+         log.debug("%s", main_cfg)

+         log.debug("Variants.xml:")

+         log.debug("%s", variants_cfg)

+         log.debug("Comps.xml:")

+         log.debug("%s", comps_cfg)

  

-             # Clone the git repo with raw_config pungi config files.

-             repo_dir = os.path.join(topdir, "raw_config_repo")

-             clone_repo(self.pungi_cfg["url"], repo_dir,

-                        commit=self.pungi_cfg["commit"])

+         self._write(os.path.join(topdir, "pungi.conf"), main_cfg)

+         self._write(os.path.join(topdir, "variants.xml"), variants_cfg)

+         self._write(os.path.join(topdir, "comps.xml"), comps_cfg)

  

-             # If the 'path' is defined, copy only the files form the 'path'

-             # to topdir.

-             if "path" in self.pungi_cfg:

-                 repo_dir = os.path.join(repo_dir, self.pungi_cfg["path"])

  

-             copytree(repo_dir, topdir)

+ class Pungi(object):

+     def __init__(self, pungi_cfg, koji_event=None, old_compose=None):

+         self.pungi_cfg = pungi_cfg

+         self.koji_event = koji_event

+         self.old_compose = old_compose

  

-             # Create the "pungi.conf" from config_filename.

-             config_path = os.path.join(topdir, self.pungi_cfg["config_filename"])

-             if config_path != main_cfg_path:

-                 shutil.copy2(config_path, main_cfg_path)

-         else:

-             raise ValueError("Unexpected pungi_conf type: %r" % self.pungi_cfg)

+     def _write_cfgs(self, topdir):

+         """Wrtie pungi config

  

+         :param str topdir: Directory to write the files to.

+         """

+         self.pungi_cfg.write_config_files(topdir)

          if conf.pungi_runroot_koji_conf_path:

              shutil.copy2(conf.pungi_runroot_koji_conf_path,

                           os.path.join(topdir, "odcs_koji.conf"))

@@ -266,8 +300,17 @@ 

          :return: List of pungi command line arguments.

          """

          pungi_cmd = [

-             conf.pungi_koji, "--config=%s" % os.path.join(conf_topdir, "pungi.conf"),

-             "--target-dir=%s" % targetdir, "--nightly"]

+             conf.pungi_koji,

+             "--config=%s" % os.path.join(conf_topdir, "pungi.conf"),

+             "--target-dir=%s" % targetdir,

+         ]

+ 

+         if isinstance(self.pungi_cfg, RawPungiConfig):

+             pungi_cmd += self.pungi_cfg.pungi_koji_args

+         elif isinstance(self.pungi_cfg, PungiConfig):

+             pungi_cmd += conf.pungi_koji_args

+         else:

+             raise RuntimeError('Unknown pungi config type to handle.')

  

          if self.koji_event:

              pungi_cmd += ["--koji-event", str(self.koji_event)]

file modified
+15 -7

@@ -26,7 +26,7 @@ 

  from mock import patch, MagicMock

  from productmd.rpms import Rpms

  

- from odcs.server import db

+ from odcs.server import conf, db

  from odcs.server.models import Compose

  from odcs.common.types import COMPOSE_FLAGS, COMPOSE_RESULTS, COMPOSE_STATES

  from odcs.server.mbs import ModuleLookupError

@@ -544,12 +544,20 @@ 

              COMPOSE_RESULTS["repository"], 60)

          c.id = 1

  

-         generate_pungi_compose(c)

-         self.assertEqual(

-             self.pungi_config,

-             {'url': 'git://localhost/test.git',

-              'commit': 'hash',

-              'config_filename': 'pungi.conf'})

+         fake_raw_config_urls = {

+             'pungi_cfg': {

+                 'url': 'git://localhost/test.git',

+                 'config_filename': 'pungi.conf'

+             }

+         }

+         with patch.object(conf, 'raw_config_urls', new=fake_raw_config_urls):

+             generate_pungi_compose(c)

+ 

+         self.assertEqual(self.pungi_config.pungi_cfg, {

+             'url': 'git://localhost/test.git',

+             'config_filename': 'pungi.conf',

+             'commit': 'hash'

+         })

  

  

  class TestValidatePungiCompose(ModelsBaseTest):

file modified
+31 -20

@@ -30,7 +30,7 @@ 

  from kobo.conf import PyConfigParser

  

  from odcs.server.pungi import (

-     Pungi, PungiConfig, PungiSourceType, PungiLogs)

+     Pungi, PungiConfig, PungiSourceType, PungiLogs, RawPungiConfig)

  import odcs.server.pungi

  from odcs.server import conf, db

  from odcs.server.models import Compose

@@ -202,13 +202,15 @@ 

                  self.assertTrue("fake pungi conf 1" in data)

          execute_cmd.side_effect = mocked_execute_cmd

  

-         pungi_cfg = {

-             "url": "http://localhost/test.git",

-             "config_filename": "pungi.conf",

-             "commit": "hash",

+         fake_raw_config_urls = {

+             'pungi.conf': {

+                 "url": "http://localhost/test.git",

+                 "config_filename": "pungi.conf",

+             }

          }

-         pungi = Pungi(pungi_cfg)

-         pungi.run(self.compose)

+         with patch.object(conf, 'raw_config_urls', new=fake_raw_config_urls):

+             pungi = Pungi(RawPungiConfig('pungi.conf#hash'))

+             pungi.run(self.compose)

  

          execute_cmd.assert_called_once()

          self.clone_repo.assert_called_once_with(

@@ -224,14 +226,16 @@ 

                  self.assertTrue("fake pungi conf 2" in data)

          execute_cmd.side_effect = mocked_execute_cmd

  

-         pungi_cfg = {

-             "url": "http://localhost/test.git",

-             "config_filename": "pungi.conf",

-             "commit": "hash",

-             "path": "another",

+         fake_raw_config_urls = {

+             'pungi.conf': {

+                 "url": "http://localhost/test.git",

+                 "config_filename": "pungi.conf",

+                 "path": "another",

+             }

          }

-         pungi = Pungi(pungi_cfg)

-         pungi.run(self.compose)

+         with patch.object(conf, 'raw_config_urls', new=fake_raw_config_urls):

+             pungi = Pungi(RawPungiConfig('pungi.conf#hash'))

+             pungi.run(self.compose)

  

          execute_cmd.assert_called_once()

          self.clone_repo.assert_called_once_with(

@@ -371,13 +375,20 @@ 

      def test_pungi_run_runroot_raw_config(self):

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

  

-         pungi_cfg = {

-             "url": "http://localhost/test.git",

-             "config_filename": "pungi.conf",

-             "commit": "hash",

+         fake_raw_config_urls = {

+             'pungi.conf': {

+                 "url": "http://localhost/test.git",

+                 "config_filename": "pungi.conf",

+             }

          }

-         pungi = Pungi(pungi_cfg)

-         pungi.run(self.compose)

+         fake_raw_config_pungi_koji_args = {

+             'pungi.conf': ['--nightly']

+         }

+         with patch.object(conf, 'raw_config_urls', new=fake_raw_config_urls):

+             with patch.object(conf, 'raw_config_pungi_koji_args',

+                               new=fake_raw_config_pungi_koji_args):

+                 pungi = Pungi(RawPungiConfig('pungi.conf#hash'))

+                 pungi.run(self.compose)

  

          conf_topdir = os.path.join(conf.target_dir, "odcs/unique_path")

          self.koji_session.uploadWrapper.assert_has_calls(

Raw pungi config and non raw pungi config are deteremined twice, and in
the second time, raw pungi config is determined by check if passed in
value has dict type. While I'm reading through the code and try to
understand the logic, I found I'm confused easily by that.

Instead, RawPungiConfig class is introduced to represent the raw config,
and both RawPungiConfig and original PungiConfig implement method from
BasePungiConfig to write options to config file, which could be simplify
code a bit.

Signed-off-by: Chenxiong Qi cqi@redhat.com

Add another commit to allow setting per raw_config pungi arguments, which is based on the PungiConfig refactor.

rebased onto 31cd745

8 months ago

1 new commit added

  • Allow setting per raw_config pungi args
8 months ago

Pull-Request has been merged by jkaluza

8 months ago