From 31cd74560cec67d638332a391f6526723cd2b298 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Aug 20 2018 02:22:08 +0000 Subject: [PATCH 1/2] Refactor PungiConfig 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 --- diff --git a/server/odcs/server/backend.py b/server/odcs/server/backend.py index 4d9b396..59cd315 100644 --- a/server/odcs/server/backend.py +++ b/server/odcs/server/backend.py @@ -31,7 +31,7 @@ import productmd.common 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 @@ def generate_pungi_compose(compose): 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, diff --git a/server/odcs/server/pungi.py b/server/odcs/server/pungi.py index ede5427..778ce5b 100644 --- a/server/odcs/server/pungi.py +++ b/server/odcs/server/pungi.py @@ -38,7 +38,74 @@ from odcs.common.types import PungiSourceType, COMPOSE_RESULTS 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 + + 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 +207,40 @@ class PungiConfig(object): "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")) diff --git a/server/tests/test_backend.py b/server/tests/test_backend.py index 45c2ae6..08f4949 100644 --- a/server/tests/test_backend.py +++ b/server/tests/test_backend.py @@ -26,7 +26,7 @@ import shutil 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 @@ class TestGeneratePungiCompose(ModelsBaseTest): 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): diff --git a/server/tests/test_pungi.py b/server/tests/test_pungi.py index 5fe1a1f..9a7c156 100644 --- a/server/tests/test_pungi.py +++ b/server/tests/test_pungi.py @@ -30,7 +30,7 @@ from mock import patch, MagicMock, call, mock_open 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 @@ class TestPungi(unittest.TestCase): 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 @@ class TestPungi(unittest.TestCase): 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,15 @@ class TestPungiRunroot(unittest.TestCase): 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) + with patch.object(conf, 'raw_config_urls', new=fake_raw_config_urls): + 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( From 04d4e6d0a8142e8a62647cc25536d6c0a14d3d13 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Aug 20 2018 07:07:59 +0000 Subject: [PATCH 2/2] Allow setting per raw_config pungi args This patch allows to specify pungi-koji command line arguments in config file for raw_config and common pungi config separately. Signed-off-by: Chenxiong Qi --- diff --git a/server/conf/config.py b/server/conf/config.py index 45f4292..e1f49c4 100644 --- a/server/conf/config.py +++ b/server/conf/config.py @@ -146,6 +146,21 @@ class BaseConfiguration(object): # } # } + # 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 @@ class DevConfiguration(BaseConfiguration): 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' diff --git a/server/odcs/server/config.py b/server/odcs/server/config.py index 3a282ac..54cf5cf 100644 --- a/server/odcs/server/config.py +++ b/server/odcs/server/config.py @@ -335,6 +335,18 @@ class Config(object): '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): diff --git a/server/odcs/server/pungi.py b/server/odcs/server/pungi.py index 778ce5b..caae104 100644 --- a/server/odcs/server/pungi.py +++ b/server/odcs/server/pungi.py @@ -69,6 +69,8 @@ class RawPungiConfig(BasePungiConfig): 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 @@ -298,8 +300,17 @@ class Pungi(object): :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)] diff --git a/server/tests/test_pungi.py b/server/tests/test_pungi.py index 9a7c156..533e46a 100644 --- a/server/tests/test_pungi.py +++ b/server/tests/test_pungi.py @@ -381,9 +381,14 @@ class TestPungiRunroot(unittest.TestCase): "config_filename": "pungi.conf", } } + fake_raw_config_pungi_koji_args = { + 'pungi.conf': ['--nightly'] + } with patch.object(conf, 'raw_config_urls', new=fake_raw_config_urls): - pungi = Pungi(RawPungiConfig('pungi.conf#hash')) - pungi.run(self.compose) + 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(