odcs

Clone

#266 Store raw_config composes in the same directory layout as normal ones.
Merged 5 years ago by jkaluza. Opened 5 years ago by jkaluza.
jkaluza/odcs remove-raw-config  into  master

file modified
+1 -1 
@@ -685,7 +685,7 @@ 
 

              koji_tag_cache.reuse_cached(compose)
 

              old_compose = koji_tag_cache.cache_dir
 

  

-         pungi = Pungi(pungi_cfg, koji_event, old_compose)
 

+         pungi = Pungi(compose.id, pungi_cfg, koji_event, old_compose)
 

          pungi.run(compose)
 

  

          _write_repo_file(compose)

file modified
+19 -3 
@@ -289,7 +289,8 @@ 
 

  

  

  class Pungi(object):
 

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

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

+         self.compose_id = compose_id
 

          self.pungi_cfg = pungi_cfg
 

          self.koji_event = koji_event
 

          self.old_compose = old_compose
 
@@ -345,7 +346,7 @@ 
 

  

          return koji_session
 

  

-     def get_pungi_cmd(self, conf_topdir, targetdir):
 

+     def get_pungi_cmd(self, conf_topdir, targetdir, compose_dir=None):
 

          """
 

          Returns list with pungi command line arguments needed to generate
 

          the compose.
 
@@ -353,6 +354,7 @@ 
 

              configuration files.
 

          :param str targetdir: Target directory in which the compose should be
 

              generated.
 

+         :param str compose_dir: If defined, overrides the Pungi compose_dir.
 

          :rtype: list
 

          :return: List of pungi command line arguments.
 

          """
 
@@ -362,6 +364,9 @@ 
 

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

          ]
 

  

+         if compose_dir:
 

+             pungi_cmd.append("--compose-dir=%s" % compose_dir)
 

+ 

          if isinstance(self.pungi_cfg, RawPungiConfig):
 

              pungi_cmd += self.pungi_cfg.pungi_koji_args
 

          elif isinstance(self.pungi_cfg, PungiConfig):
 
@@ -375,6 +380,16 @@ 
 

              pungi_cmd += ["--old-composes", self.old_compose]
 

          return pungi_cmd
 

  

+     def _prepare_compose_dir(self, conf_topdir, targetdir):
 

+         """
 

+         Creates the compose directory and returns the full path to it.
 

+         """
 

+         compose_id = "odcs-%s-1-%s.n.0" % (
 

+             self.compose_id, time.strftime("%Y%m%d", time.localtime()))
 

+         compose_dir = os.path.join(targetdir, compose_id)
 

+         makedirs(compose_dir)
 

+         return compose_dir
 

+ 

      def run_locally(self):
 

          """
 

          Runs local Pungi compose.
 
@@ -383,7 +398,8 @@ 
 

          try:
 

              td = tempfile.mkdtemp()
 

              self._write_cfgs(td)
 

-             pungi_cmd = self.get_pungi_cmd(td, conf.target_dir)
 

+             compose_dir = self._prepare_compose_dir(td, conf.target_dir)
 

+             pungi_cmd = self.get_pungi_cmd(td, conf.target_dir, compose_dir)
 

              odcs.server.utils.execute_cmd(pungi_cmd, cwd=td,
 

                                            timeout=conf.pungi_timeout)
 

          finally:

file modified
+17 -6 
@@ -337,23 +337,31 @@ 
 

          self.clone_repo = self.patch_clone_repo.start()
 

          self.clone_repo.side_effect = mocked_clone_repo
 

  

+         self.patch_makedirs = patch("odcs.server.pungi.makedirs")
 

+         self.makedirs = self.patch_makedirs.start()
 

+ 

          self.compose = MagicMock()
 

  

      def tearDown(self):
 

          super(TestPungi, self).tearDown()
 

  

          self.patch_clone_repo.stop()
 

+         self.patch_makedirs.stop()
 

  

      @patch("odcs.server.utils.execute_cmd")
 

      def test_pungi_run(self, execute_cmd):
 

          pungi_cfg = PungiConfig("MBS-512", "1", PungiSourceType.MODULE,
 

                                  "testmodule:master:1:1")
 

-         pungi = Pungi(pungi_cfg)
 

+         pungi = Pungi(1, pungi_cfg)
 

          pungi.run(self.compose)
 

  

+         self.makedirs.assert_called_once_with(
 

+             AnyStringWith("test_composes/odcs-1-1-"))
 

+ 

          execute_cmd.assert_called_once_with(
 

              ['pungi-koji', AnyStringWith('pungi.conf'),
 

-              AnyStringWith('--target-dir'), '--nightly'],
 

+              AnyStringWith('--target-dir'), AnyStringWith('--compose-dir='),
 

+              '--nightly'],
 

              cwd=AnyStringWith('/tmp/'), timeout=3600)
 

  

      @patch("odcs.server.utils.execute_cmd")
 
@@ -372,9 +380,12 @@ 
 

              }
 

          }
 

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

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

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

              pungi.run(self.compose)
 

  

+         self.makedirs.assert_called_once_with(
 

+             AnyStringWith("test_composes/odcs-1-1-"))
 

+ 

          execute_cmd.assert_called_once()
 

          self.clone_repo.assert_called_once_with(
 

              'http://localhost/test.git', AnyStringWith("/raw_config_repo"),
 
@@ -397,7 +408,7 @@ 
 

              }
 

          }
 

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

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

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

              pungi.run(self.compose)
 

  

          execute_cmd.assert_called_once()
 
@@ -521,7 +532,7 @@ 
 

  

          pungi_cfg = PungiConfig("MBS-512", "1", PungiSourceType.MODULE,
 

                                  "testmodule:master:1:1")
 

-         pungi = Pungi(pungi_cfg)
 

+         pungi = Pungi(1, pungi_cfg)
 

          pungi.run(self.compose)
 

  

          conf_topdir = os.path.join(conf.target_dir, "odcs/unique_path")
 
@@ -557,7 +568,7 @@ 
 

          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 = Pungi(1, RawPungiConfig('pungi.conf#hash'))
 

                  pungi.run(self.compose)
 

  

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

Before this commit, the directory in which the compose was generated
was defined by Pungi. Pungi defines it according to name of a product
for which the compose is generated, the release number, compose type,
and lot of other things.

This used to work correctly for composes generated from ODCS templates,
because their name/release/type is predefined and always the same and
therefore the compose directory was also well known.

With raw_config composes, we no longer know the product's name, release,
or other things. The directory name is therefore not predictable
which makes it imposible to point to a resulting compose or remove
it once it expires.

In this commit, the compose directory path is generated and created
in ODCS (not in Pungi anymore) and is passed to Pungi using the
--compose-dir command line argument. We generate the compose directory
also for raw_config composes and therefore they are stored in directory
with predictable name on filesystem.

This allows to remove these directories when compose expires.

Why do we need the date here? If the point is to allow compose to be regenerated, then we likely something more granular, like timestamp, no?

Does this need to be the full path to the dir? Or just its name?

In theory we can use any directory after this PR, but this "pattern" I use here is what Pungi currently uses and therefore what ODCS currently uses. I want it to stay the same, because there is no benefit in changing this and in case somebody is using this directories in their workflow, it would break for no benefit for us.

To answer your question. We create always just one directory like that for single ODCS compose. Even just "odcs-%s" % self.compose_id would be enough, because given compose_id can be generated just once - it either succeeds or fails.

When regenerating the compose, it gets brand new ODCS compose id.

It needs to be full-path. It is passed to os.path.abspath in pungi-koji which effectively generates full-path from it even if you use just directory name:

https://pagure.io/pungi/blob/master/f/bin/pungi-koji#_194

It needs to be full-path. It is passed to os.path.abspath in pungi-koji which effectively generates full-path from it even if you use just directory name:
https://pagure.io/pungi/blob/master/f/bin/pungi-koji#_194

The code in pungi appears to not take the target dir into account, which is where we're generating the compose dir. Should we just return the full path from _prepare_compose_dir to be safe?

It needs to be full-path. It is passed to os.path.abspath in pungi-koji which effectively generates full-path from it even if you use just directory name:
https://pagure.io/pungi/blob/master/f/bin/pungi-koji#_194

The code in pungi appears to not take the target dir into account, which is where we're generating the compose dir. Should we just return the full path from _prepare_compose_dir to be safe?

We return full-path in _prepare_compose_dir:

+         compose_dir = os.path.join(targetdir, compose_id)

rebased onto 1ae475d
5 years ago

You are right @lucarval, I should return compose_dir there and not compose_id. Fixed now.

:+1:

Pull-Request has been merged by jkaluza
5 years ago
Changes Summary 3
+1 -1
file changed
server/odcs/server/backend.py
+19 -3
file changed
server/odcs/server/pungi.py
+17 -6
file changed
server/tests/test_pungi.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.