PR#427: Allow Jinja2 in raw_config_wrapper.conf and pass the `compose` variable there. - odcs

odcs

#427 Allow Jinja2 in raw_config_wrapper.conf and pass the `compose` variable there.

Merged 3 years ago by lsedlar. Opened 3 years ago by jkaluza.

jkaluza/odcs raw-config-template into master

Pass `Compose.json()` to raw_config template instead of `Compose`.

Jan Kaluza • 3 years ago

0342cb7

Allow Jinja2 in raw_config_wrapper.conf and pass the `compose` variable there.

Jan Kaluza • 3 years ago

ec7150f

client/contrib/odcs

file modified

		`@@ -274,6 +274,7 @@`
		`"--compose-type",`
		`"--koji-event",`
		`"--target-dir",`
		`+ "--build",`
		`)`

docs/api.rst

file modified

		`@@ -149,6 +149,11 @@`
		`.. note::`
		If ``target_dir`` is set to non-default value, then the ``result_repofile`` might be an empty string, because ODCS might not have enough data to generate the URL.

		`+ .. _scratch_modules:`
		`+`
		+ scratch_modules - ``(white-space separated list of strings)``
		`+ List of scratch module builds defined as N:S:V:C format which will be included in the compose.`
		`+`
		`.. _sigkeys:`

		sigkeys - ``(white-space separated list of strings)``

docs/raw_config.rst

file modified

+21 -1

		`@@ -61,14 +61,31 @@`
		the real ``raw_config`` configuration file.

		Any further options are then used to override the real ``raw_config``
		`- configuration. For example to set particular Koji profile:`
		`+ configuration.`
		`+`
		+ This config file is pre-processed using Jinja2 templates. The ``compose``
		`+ Jinja2 variable can be used in this config file. This variable contains`
		+ :ref:`compose_json` representing the current Compose.
		`+`
		`+ Configuration file example:`


		`.. sourcecode:: none`

		`from raw_config import *`
		`+`
		`+ # Override the koji_profile`
		`koji_profile = 'odcs_stg'`

		`+ # Allow overriding pkgset_koji_builds from ODCS client.`
		`+ {%- if compose["builds"] %}`
		`+ pkgset_koji_builds = [`
		`+ {%- for build in compose["builds"].split(" ") %}`
		`+ '{{ build }}',`
		`+ {%- endfor %}`
		`+ ]`
		`+ {%- endif %}`
		`+`

		Adding the ``raw_config_urls`` record
		`-------------------------------------`
		`@@ -91,6 +108,9 @@`
		- ``pungi_timeout`` - [optional] - If set, defines the timeout in seconds in
		`which the compose must be finished, otherwise the compose is marked as`
		``failed``.
		+ - ``raw_config_wrapper`` - [optional] - If set, defines the full path to
		+ custom ``raw_config_wrapper.conf`` file which is used by this Raw config
		`+ compose.`

		`For example:`

server/odcs/server/backend.py

file modified

+1 -1

		`@@ -884,7 +884,7 @@`
		`reuse_compose(compose, compose_to_reuse)`
		`else:`
		`if compose.source_type == PungiSourceType.RAW_CONFIG:`
		`- pungi_cfg = RawPungiConfig(compose.source)`
		`+ pungi_cfg = RawPungiConfig(compose)`
		`else:`
		`# Generate PungiConfig and run Pungi`
		`if compose.multilib_arches:`

server/odcs/server/pungi.py

file modified

+23 -4

		`@@ -70,9 +70,10 @@`


		`class RawPungiConfig(BasePungiConfig):`
		`- def __init__(self, compose_source):`
		`+ def __init__(self, compose):`
		`super(RawPungiConfig, self).__init__()`
		`- source_name, source_hash = compose_source.split("#")`
		`+ self.compose = compose`
		`+ source_name, source_hash = compose.source.split("#")`

		`url_data = copy.deepcopy(conf.raw_config_urls[source_name])`
		`# Do not override commit hash by hash from ODCS client if it is`
		`@@ -81,11 +82,28 @@`
		`url_data["commit"] = source_hash`

		`self.pungi_timeout = url_data.get("pungi_timeout", conf.pungi_timeout)`
		`+ self.raw_config_wrapper_conf_path = url_data.get(`
		`+ "raw_config_wrapper", conf.raw_config_wrapper_conf_path`
		`+ )`
		`self.pungi_cfg = url_data`
		`self.pungi_koji_args = conf.raw_config_pungi_koji_args.get(`
		`source_name, conf.pungi_koji_args`
		`)`

		`+ def apply_raw_config_wrapper_overrides(self, cfg_path):`
		`+ try:`
		`+ with open(cfg_path) as fd:`
		`+ raw_config_wrapper = jinja2.Template(fd.read())`
		`+ with open(cfg_path, "w") as fd:`
		`+ fd.write(raw_config_wrapper.render(compose=self.compose.json()))`
		`+ except Exception as e:`
		`+ log.exception(`
		`+ "Failed to render raw_config_wrapper template {!r}: {}".format(`
		`+ cfg_path, str(e)`
		`+ )`
		`+ )`
		`+ raise`
		`+`
		`def write_config_files(self, topdir):`
		`"""Write raw config files`

		`@@ -96,11 +114,12 @@`
		`# 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:`
		`+ if self.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")`
		`+ self.raw_config_wrapper_conf_path, os.path.join(topdir, "pungi.conf")`
		`)`
		`+ self.apply_raw_config_wrapper_overrides(os.path.join(topdir, "pungi.conf"))`
		`else:`
		`main_cfg_path = os.path.join(topdir, "pungi.conf")`

server/tests/data/custom_raw_config_wrapper.conf

file added

+11

		`@@ -0,0 +1,11 @@`
		`+ from raw_config import *`
		`+ koji_profile = 'odcs_stg'`
		`+ createrepo_deltas = False`
		`+`
		`+ {%- if compose["builds"] %}`
		`+ pkgset_koji_builds = [`
		`+ {%- for build in compose["builds"].split(" ") %}`
		`+ '{{ build }}',`
		`+ {%- endfor %}`
		`+ ]`
		`+ {%- endif %}`

server/tests/test_pungi.py

file modified

+71 -6

		`@@ -572,7 +572,8 @@`
		`}`
		`}`
		`with patch.object(conf, "raw_config_urls", new=fake_raw_config_urls):`
		`- pungi = Pungi(1, RawPungiConfig("pungi.conf#hash"))`
		`+ self.compose.source = "pungi.conf#hash"`
		`+ pungi = Pungi(1, RawPungiConfig(self.compose))`
		`pungi.run(self.compose)`

		`self.makedirs.assert_called_with(AnyStringWith("test_composes/odcs-1/"))`
		`@@ -620,7 +621,8 @@`
		`}`
		`}`
		`with patch.object(conf, "raw_config_urls", new=fake_raw_config_urls):`
		`- pungi = Pungi(1, RawPungiConfig("pungi.conf#hash"))`
		`+ self.compose.source = "pungi.conf#hash"`
		`+ pungi = Pungi(1, RawPungiConfig(self.compose))`
		`pungi.run(compose)`
		`pungi.run(compose)`

		`@@ -645,7 +647,8 @@`
		`}`
		`}`
		`with patch.object(conf, "raw_config_urls", new=fake_raw_config_urls):`
		`- pungi = Pungi(1, RawPungiConfig("pungi.conf#hash"))`
		`+ self.compose.source = "pungi.conf#hash"`
		`+ pungi = Pungi(1, RawPungiConfig(self.compose))`
		`pungi.run(self.compose)`

		`execute_cmd.assert_called_once()`
		`@@ -671,7 +674,8 @@`
		`with patch.object(`
		`conf, "pungi_config_validate", new="pungi-config-validate"`
		`):`
		`- pungi = Pungi(1, RawPungiConfig("pungi.conf#hash"))`
		`+ self.compose.source = "pungi.conf#hash"`
		`+ pungi = Pungi(1, RawPungiConfig(self.compose))`
		`pungi.run(self.compose)`

		`self.assertEqual(`
		`@@ -701,7 +705,8 @@`
		`}`
		`}`
		`with patch.object(conf, "raw_config_urls", new=fake_raw_config_urls):`
		`- pungi = Pungi(1, RawPungiConfig("pungi.conf#hash"))`
		`+ self.compose.source = "pungi.conf#hash"`
		`+ pungi = Pungi(1, RawPungiConfig(self.compose))`
		`pungi.run(self.compose)`

		`execute_cmd.assert_called_once_with(`
		`@@ -730,7 +735,8 @@`
		`}`
		`}`
		`with patch.object(conf, "raw_config_urls", new=fake_raw_config_urls):`
		`- pungi = Pungi(1, RawPungiConfig("pungi.conf#hash"))`
		`+ self.compose.source = "pungi.conf#hash"`
		`+ pungi = Pungi(1, RawPungiConfig(self.compose))`
		`pungi.run(self.compose)`

		`execute_cmd.assert_called_once_with(`
		`@@ -749,6 +755,65 @@`
		`)`


		`+ class TestRawPungiConfig(ModelsBaseTest):`
		`+ def setUp(self):`
		`+ super(TestRawPungiConfig, self).setUp()`
		`+ self.compose = Compose.create(`
		`+ db.session,`
		`+ "me",`
		`+ PungiSourceType.RAW_CONFIG,`
		`+ "test.conf#hash",`
		`+ COMPOSE_RESULTS["repository"],`
		`+ 3600,`
		`+ )`
		`+ db.session.commit()`
		`+`
		`+ def mocked_clone_repo(url, dest, branch="master", commit=None):`
		`+ makedirs(dest)`
		`+ with open(os.path.join(dest, "test.conf"), "w") as fd:`
		`+ lines = [`
		`+ 'release_name = "fake pungi conf 1"',`
		`+ 'release_short = "compose-1"',`
		`+ 'release_version = "10"',`
		`+ ]`
		`+ fd.writelines(lines)`
		`+`
		`+ self.patch_clone_repo = patch("odcs.server.pungi.clone_repo")`
		`+ self.clone_repo = self.patch_clone_repo.start()`
		`+ self.clone_repo.side_effect = mocked_clone_repo`
		`+`
		`+ def tearDown(self):`
		`+ super(TestRawPungiConfig, self).tearDown()`
		`+ self.patch_clone_repo.stop()`
		`+`
		`+ def test_raw_config_custom_wrapper(self):`
		`+ custom_raw_config_wrapper = os.path.join(`
		`+ test_dir, "data", "custom_raw_config_wrapper.conf"`
		`+ )`
		`+ fake_raw_config_urls = {`
		`+ "test.conf": {`
		`+ "url": "http://localhost/test.git",`
		`+ "config_filename": "test.conf",`
		`+ "raw_config_wrapper": custom_raw_config_wrapper,`
		`+ }`
		`+ }`
		`+ self.compose.builds = "foo-1-1 bar-1-1"`
		`+`
		`+ with patch.object(conf, "raw_config_urls", new=fake_raw_config_urls):`
		`+ td = tempfile.mkdtemp()`
		`+ try:`
		`+ cfg = RawPungiConfig(self.compose)`
		`+ cfg.write_config_files(td)`
		`+ pungi_conf = PyConfigParser()`
		`+ pungi_conf.load_from_file(os.path.join(td, "pungi.conf"))`
		`+ self.assertEqual(pungi_conf["release_short"], "compose-1")`
		`+ self.assertEqual(`
		`+ pungi_conf["pkgset_koji_builds"], ["foo-1-1", "bar-1-1"]`
		`+ )`
		`+ finally:`
		`+ shutil.rmtree(td)`
		`+`
		`+`
		`class TestPungiLogs(ModelsBaseTest):`
		`def setUp(self):`
		`super(TestPungiLogs, self).setUp()`

jkaluza commented 3 years ago

This should allow configuring ODCS in a way that particular raw_config compose
configuration variables can be overriden.

For example, the pkgset_koji_builds can be set according to compose.builds
which is set in the ODCS REST API when requesting the compose.

rebased onto f30396a72e7e863942625a74fdea28e16e978305

3 years ago

rebased onto ec7150f

3 years ago

lsedlar commented on line 38 of docs/raw_config.rst 3 years ago

There's a trailing space on this line.

lsedlar commented on line 9 of docs/raw_config.rst 3 years ago

Typo: can be used in this config file.

lsedlar commented 3 years ago

Generally looks good to me. This is a nice improvement.

One worry I have is whether the documentation should be more explicit on stability promises. What if the model changes and some fields are removed. That could break existing configuration templates. Do we want to guarantee stability there?

jkaluza commented 3 years ago

@lsedlar, that's good point. Maybe we can pass result of Compose.json() there and say in the documentation that the Compose JSON representation is passed to template as compose. This Compose JSON representation is part of API.

lsedlar commented 3 years ago

I like that. That structure is already stable. The only downside is that some values are serialized into a string there, which might be painful in the template.

1 new commit added