pungi

Clone

#1187 buildinstall: Copy files in thread
Merged 5 years ago by lsedlar. Opened 5 years ago by lsedlar.
lsedlar/pungi buildinstall-copy  into  master

file modified
-3 
@@ -384,9 +384,6 @@ 
 

      essentials_phase.start()
 

      essentials_phase.stop()
 

  

-     if not buildinstall_phase.skip():
 

-         buildinstall_phase.copy_files()
 

- 

      productimg_phase.start()
 

      productimg_phase.stop()

file modified
+36 -34 
@@ -143,6 +143,9 @@ 
 

          release = self.compose.conf["release_version"]
 

          disc_type = self.compose.conf['disc_types'].get('dvd', 'dvd')
 

  

+         # Prepare kickstart file for final images.
 

+         self.pool.kickstart_file = get_kickstart_file(self.compose)
 

+ 

          for arch in self.compose.get_arches():
 

              commands = []
 

  
@@ -199,40 +202,6 @@ 
 

          return (super(BuildinstallPhase, self).skip()
 

                  or (variant.uid if self.used_lorax else None, arch) in self.pool.finished_tasks)
 

  

-     def copy_files(self):
 

-         disc_type = self.compose.conf['disc_types'].get('dvd', 'dvd')
 

- 

-         # copy buildinstall files to the 'os' dir
 

-         kickstart_file = get_kickstart_file(self.compose)
 

-         for arch in self.compose.get_arches():
 

-             for variant in self.compose.get_variants(arch=arch, types=["self", "variant"]):
 

-                 if variant.is_empty:
 

-                     continue
 

-                 if not self.succeeded(variant, arch):
 

-                     self.compose.log_debug(
 

-                         'Buildinstall: skipping copying files for %s.%s due to failed runroot task'
 

-                         % (variant.uid, arch))
 

-                     continue
 

- 

-                 buildinstall_dir = self.compose.paths.work.buildinstall_dir(arch)
 

- 

-                 # Lorax runs per-variant, so we need to tweak the source path
 

-                 # to include variant.
 

-                 if self.used_lorax:
 

-                     buildinstall_dir = os.path.join(buildinstall_dir, variant.uid)
 

- 

-                 if not os.path.isdir(buildinstall_dir) or not os.listdir(buildinstall_dir):
 

-                     continue
 

- 

-                 os_tree = self.compose.paths.compose.os_tree(arch, variant)
 

-                 # TODO: label is not used
 

-                 label = ""
 

-                 volid = get_volid(self.compose, arch, variant, disc_type=disc_type)
 

-                 can_fail = self.compose.can_fail(variant, arch, 'buildinstall')
 

-                 with failable(self.compose, can_fail, variant, arch, 'buildinstall'):
 

-                     tweak_buildinstall(self.compose, buildinstall_dir, os_tree, arch, variant.uid, label, volid, kickstart_file)
 

-                     link_boot_iso(self.compose, arch, variant, can_fail)
 

- 

  

  def get_kickstart_file(compose):
 

      scm_dict = compose.conf.get("buildinstall_kickstart")
 
@@ -494,4 +463,37 @@ 
 

              f.write("\n".join(rpms))
 

  

          self.pool.finished_tasks.add((variant.uid if variant else None, arch))
 

+ 

+         self.copy_files(compose, variant, arch)
 

+ 

          self.pool.log_info("[DONE ] %s" % msg)
 

+ 

+     def copy_files(self, compose, variant, arch):
 

+         disc_type = compose.conf['disc_types'].get('dvd', 'dvd')
 

+ 

+         buildinstall_dir = compose.paths.work.buildinstall_dir(arch)
 

+ 

+         # Lorax runs per-variant, so we need to tweak the source path
 

+         # to include variant.
 

+         if variant:
 

+             buildinstall_dir = os.path.join(buildinstall_dir, variant.uid)
 

+ 

+         # Find all relevant variants if lorax is not used.
 

+         variants = [variant] if variant else compose.get_variants(arch=arch, types=["self", "variant"])
 

+         for var in variants:
 

+             os_tree = compose.paths.compose.os_tree(arch, var)
 

+             # TODO: label is not used
 

+             label = ""
 

+             volid = get_volid(compose, arch, var, disc_type=disc_type)
 

+             can_fail = compose.can_fail(var, arch, 'buildinstall')
 

+             tweak_buildinstall(
 

+                 compose,
 

+                 buildinstall_dir,
 

+                 os_tree,
 

+                 arch,
 

+                 var.uid,
 

+                 label,
 

+                 volid,
 

+                 self.pool.kickstart_file,
 

+             )
 

+             link_boot_iso(compose, arch, var, can_fail)

file modified
+90 -184 
@@ -15,7 +15,7 @@ 
 

  

  from pungi.phases.buildinstall import (BuildinstallPhase, BuildinstallThread, link_boot_iso,
 

                                         BOOT_CONFIGS, tweak_configs)
 

- from tests.helpers import DummyCompose, PungiTestCase, touch, boom
 

+ from tests.helpers import DummyCompose, PungiTestCase, touch
 

  

  

  class BuildInstallCompose(DummyCompose):
 
@@ -555,179 +555,9 @@ 
 

                         log_dir=self.topdir + '/logs/amd64/buildinstall-Client-logs')])
 

  

  

- class TestCopyFiles(PungiTestCase):
 

- 

-     @mock.patch('pungi.phases.buildinstall.link_boot_iso')
 

-     @mock.patch('pungi.phases.buildinstall.tweak_buildinstall')
 

-     @mock.patch('pungi.phases.buildinstall.get_volid')
 

-     @mock.patch('os.listdir')
 

-     @mock.patch('os.path.isdir')
 

-     @mock.patch('pungi.phases.buildinstall.get_kickstart_file')
 

-     def test_copy_files_buildinstall(self, get_kickstart_file, isdir, listdir,
 

-                                      get_volid, tweak_buildinstall, link_boot_iso):
 

-         compose = BuildInstallCompose(self.topdir, {
 

-             'buildinstall_method': 'buildinstall'
 

-         })
 

- 

-         get_volid.side_effect = (
 

-             lambda compose, arch, variant, disc_type: "%s.%s" % (variant.uid, arch)
 

-         )
 

-         get_kickstart_file.return_value = 'kickstart'
 

- 

-         phase = BuildinstallPhase(compose)
 

-         phase.pool.finished_tasks = set([(None, 'x86_64'), (None, 'amd64')])
 

-         phase.copy_files()
 

- 

-         self.assertItemsEqual(
 

-             get_volid.mock_calls,
 

-             [mock.call(compose, 'x86_64', compose.variants['Server'], disc_type='dvd'),
 

-              mock.call(compose, 'amd64', compose.variants['Client'], disc_type='dvd'),
 

-              mock.call(compose, 'amd64', compose.variants['Server'], disc_type='dvd')])
 

-         self.assertItemsEqual(
 

-             tweak_buildinstall.mock_calls,
 

-             [mock.call(compose,
 

-                        self.topdir + '/work/x86_64/buildinstall',
 

-                        self.topdir + '/compose/Server/x86_64/os',
 

-                        'x86_64', 'Server', '', 'Server.x86_64', 'kickstart'),
 

-              mock.call(compose,
 

-                        self.topdir + '/work/amd64/buildinstall',
 

-                        self.topdir + '/compose/Server/amd64/os',
 

-                        'amd64', 'Server', '', 'Server.amd64', 'kickstart'),
 

-              mock.call(compose,
 

-                        self.topdir + '/work/amd64/buildinstall',
 

-                        self.topdir + '/compose/Client/amd64/os',
 

-                        'amd64', 'Client', '', 'Client.amd64', 'kickstart')])
 

-         self.assertItemsEqual(
 

-             link_boot_iso.mock_calls,
 

-             [mock.call(compose, 'x86_64', compose.variants['Server'], False),
 

-              mock.call(compose, 'amd64', compose.variants['Client'], False),
 

-              mock.call(compose, 'amd64', compose.variants['Server'], False)])
 

- 

-     @mock.patch('pungi.phases.buildinstall.link_boot_iso')
 

-     @mock.patch('pungi.phases.buildinstall.tweak_buildinstall')
 

-     @mock.patch('pungi.phases.buildinstall.get_volid')
 

-     def test_copy_files_buildinstall_failed_task(self, get_volid, tweak_buildinstall, link_boot_iso):
 

-         compose = BuildInstallCompose(self.topdir, {
 

-             'buildinstall_method': 'buildinstall'
 

-         })
 

- 

-         phase = BuildinstallPhase(compose)
 

-         phase.pool.finished_tasks = set()
 

-         phase.copy_files()
 

- 

-         self.assertItemsEqual(get_volid.mock_calls, [])
 

-         self.assertItemsEqual(tweak_buildinstall.mock_calls, [])
 

-         self.assertItemsEqual(link_boot_iso.mock_calls, [])
 

- 

-     @mock.patch('pungi.phases.buildinstall.link_boot_iso')
 

-     @mock.patch('pungi.phases.buildinstall.tweak_buildinstall')
 

-     @mock.patch('pungi.phases.buildinstall.get_volid')
 

-     @mock.patch('os.listdir')
 

-     @mock.patch('os.path.isdir')
 

-     @mock.patch('pungi.phases.buildinstall.get_kickstart_file')
 

-     def test_copy_files_lorax(self, get_kickstart_file, isdir, listdir,
 

-                               get_volid, tweak_buildinstall, link_boot_iso):
 

-         compose = BuildInstallCompose(self.topdir, {
 

-             'buildinstall_method': 'lorax'
 

-         })
 

- 

-         get_volid.side_effect = (
 

-             lambda compose, arch, variant, disc_type: "%s.%s" % (variant.uid, arch)
 

-         )
 

-         get_kickstart_file.return_value = 'kickstart'
 

- 

-         phase = BuildinstallPhase(compose)
 

-         phase.pool.finished_tasks = set([('Server', 'x86_64'), ('Server', 'amd64'), ('Client', 'amd64')])
 

-         phase.copy_files()
 

- 

-         self.assertItemsEqual(
 

-             get_volid.mock_calls,
 

-             [mock.call(compose, 'x86_64', compose.variants['Server'], disc_type='dvd'),
 

-              mock.call(compose, 'amd64', compose.variants['Client'], disc_type='dvd'),
 

-              mock.call(compose, 'amd64', compose.variants['Server'], disc_type='dvd')])
 

-         self.assertItemsEqual(
 

-             tweak_buildinstall.mock_calls,
 

-             [mock.call(compose,
 

-                        self.topdir + '/work/x86_64/buildinstall/Server',
 

-                        self.topdir + '/compose/Server/x86_64/os',
 

-                        'x86_64', 'Server', '', 'Server.x86_64', 'kickstart'),
 

-              mock.call(compose,
 

-                        self.topdir + '/work/amd64/buildinstall/Server',
 

-                        self.topdir + '/compose/Server/amd64/os',
 

-                        'amd64', 'Server', '', 'Server.amd64', 'kickstart'),
 

-              mock.call(compose,
 

-                        self.topdir + '/work/amd64/buildinstall/Client',
 

-                        self.topdir + '/compose/Client/amd64/os',
 

-                        'amd64', 'Client', '', 'Client.amd64', 'kickstart')])
 

-         self.assertItemsEqual(
 

-             link_boot_iso.mock_calls,
 

-             [mock.call(compose, 'x86_64', compose.variants['Server'], False),
 

-              mock.call(compose, 'amd64', compose.variants['Client'], False),
 

-              mock.call(compose, 'amd64', compose.variants['Server'], False)])
 

- 

-     @mock.patch('pungi.phases.buildinstall.link_boot_iso')
 

-     @mock.patch('pungi.phases.buildinstall.tweak_buildinstall')
 

-     @mock.patch('pungi.phases.buildinstall.get_volid')
 

-     def test_copy_files_lorax_failed_task(self, get_volid, tweak_buildinstall, link_boot_iso):
 

-         compose = BuildInstallCompose(self.topdir, {
 

-             'buildinstall_method': 'lorax'
 

-         })
 

- 

-         phase = BuildinstallPhase(compose)
 

-         phase.pool.finished_tasks = set()
 

-         phase.copy_files()
 

- 

-         self.assertItemsEqual(get_volid.mock_calls, [])
 

-         self.assertItemsEqual(tweak_buildinstall.mock_calls, [])
 

-         self.assertItemsEqual(link_boot_iso.mock_calls, [])
 

- 

-     @mock.patch('pungi.phases.buildinstall.link_boot_iso')
 

-     @mock.patch('pungi.phases.buildinstall.tweak_buildinstall')
 

-     @mock.patch('pungi.phases.buildinstall.get_volid')
 

-     @mock.patch('os.listdir')
 

-     @mock.patch('os.path.isdir')
 

-     @mock.patch('pungi.phases.buildinstall.get_kickstart_file')
 

-     def test_copy_fail(self, get_kickstart_file, isdir, listdir,
 

-                        get_volid, tweak_buildinstall, link_boot_iso):
 

-         compose = BuildInstallCompose(self.topdir, {
 

-             'buildinstall_method': 'lorax',
 

-             'failable_deliverables': [
 

-                 ('^.+$', {'*': ['buildinstall']})
 

-             ],
 

-         })
 

- 

-         get_volid.side_effect = (
 

-             lambda compose, arch, variant, disc_type: "%s.%s" % (variant.uid, arch)
 

-         )
 

-         get_kickstart_file.return_value = 'kickstart'
 

-         tweak_buildinstall.side_effect = boom
 

- 

-         phase = BuildinstallPhase(compose)
 

-         phase.pool.finished_tasks = set([('Server', 'x86_64'), ('Server', 'amd64'), ('Client', 'amd64')])
 

-         phase.copy_files()
 

- 

-         self.assertItemsEqual(
 

-             get_volid.mock_calls,
 

-             [mock.call(compose, 'x86_64', compose.variants['Server'], disc_type='dvd'),
 

-              mock.call(compose, 'amd64', compose.variants['Client'], disc_type='dvd'),
 

-              mock.call(compose, 'amd64', compose.variants['Server'], disc_type='dvd')])
 

-         self.assertItemsEqual(
 

-             tweak_buildinstall.mock_calls,
 

-             [mock.call(compose,
 

-                        self.topdir + '/work/x86_64/buildinstall/Server',
 

-                        self.topdir + '/compose/Server/x86_64/os',
 

-                        'x86_64', 'Server', '', 'Server.x86_64', 'kickstart'),
 

-              mock.call(compose,
 

-                        self.topdir + '/work/amd64/buildinstall/Server',
 

-                        self.topdir + '/compose/Server/amd64/os',
 

-                        'amd64', 'Server', '', 'Server.amd64', 'kickstart'),
 

-              mock.call(compose,
 

-                        self.topdir + '/work/amd64/buildinstall/Client',
 

-                        self.topdir + '/compose/Client/amd64/os',
 

-                        'amd64', 'Client', '', 'Client.amd64', 'kickstart')])
 

-         self.assertItemsEqual(link_boot_iso.mock_calls, [])
 

- 

- 

+ @mock.patch(
 

+     'pungi.phases.buildinstall.get_volid', new=lambda *args, **kwargs: "dummy-volid"
 

+ )
 

  class BuildinstallThreadTestCase(PungiTestCase):
 

  

      def setUp(self):
 
@@ -735,10 +565,14 @@ 
 

          self.pool = mock.Mock(finished_tasks=set())
 

          self.cmd = mock.Mock()
 

  

+     @mock.patch('pungi.phases.buildinstall.link_boot_iso')
 

+     @mock.patch('pungi.phases.buildinstall.tweak_buildinstall')
 

      @mock.patch('pungi.wrappers.kojiwrapper.KojiWrapper')
 

      @mock.patch('pungi.wrappers.kojiwrapper.get_buildroot_rpms')
 

      @mock.patch('pungi.phases.buildinstall.run')
 

-     def test_buildinstall_thread_with_lorax_in_runroot(self, run, get_buildroot_rpms, KojiWrapperMock):
 

+     def test_buildinstall_thread_with_lorax_in_runroot(
 

+         self, run, get_buildroot_rpms, KojiWrapperMock, mock_tweak, mock_link
 

+     ):
 

          compose = BuildInstallCompose(self.topdir, {
 

              'buildinstall_method': 'lorax',
 

              'runroot': True,
 
@@ -763,13 +597,14 @@ 
 

          with mock.patch('time.sleep'):
 

              t.process((compose, 'x86_64', compose.variants['Server'], self.cmd), 0)
 

  

+         destdir = os.path.join(self.topdir, "work/x86_64/buildinstall/Server")
 

          self.assertItemsEqual(
 

              get_runroot_cmd.mock_calls,
 

              [mock.call(
 

                  'rrt', 'x86_64', self.cmd, channel=None,
 

                  use_shell=True, task_id=True,
 

                  packages=['lorax'], mounts=[self.topdir], weight=123,
 

-                 destdir=os.path.join(self.topdir, "work/x86_64/buildinstall/Server"),
 

+                 destdir=destdir,
 

              )])
 

          self.assertItemsEqual(
 

              run_runroot_cmd.mock_calls,
 
@@ -780,10 +615,34 @@ 
 

          self.assertItemsEqual(rpms, ['bash', 'zsh'])
 

          self.assertItemsEqual(self.pool.finished_tasks, [('Server', 'x86_64')])
 

  

+         self.assertEqual(
 

+             mock_tweak.call_args_list,
 

+             [
 

+                 mock.call(
 

+                     compose,
 

+                     destdir,
 

+                     os.path.join(self.topdir, "compose/Server/x86_64/os"),
 

+                     "x86_64",
 

+                     "Server",
 

+                     "",
 

+                     "dummy-volid",
 

+                     self.pool.kickstart_file,
 

+                 )
 

+             ],
 

+         )
 

+         self.assertEqual(
 

+             mock_link.call_args_list,
 

+             [mock.call(compose, "x86_64", compose.variants["Server"], False)],
 

+         )
 

+ 

+     @mock.patch('pungi.phases.buildinstall.link_boot_iso')
 

+     @mock.patch('pungi.phases.buildinstall.tweak_buildinstall')
 

      @mock.patch('pungi.wrappers.kojiwrapper.KojiWrapper')
 

      @mock.patch('pungi.wrappers.kojiwrapper.get_buildroot_rpms')
 

      @mock.patch('pungi.phases.buildinstall.run')
 

-     def test_buildinstall_thread_with_buildinstall_in_runroot(self, run, get_buildroot_rpms, KojiWrapperMock):
 

+     def test_buildinstall_thread_with_buildinstall_in_runroot(
 

+         self, run, get_buildroot_rpms, KojiWrapperMock, mock_tweak, mock_link
 

+     ):
 

          compose = BuildInstallCompose(self.topdir, {
 

              'buildinstall_method': 'buildinstall',
 

              'runroot': True,
 
@@ -805,24 +664,48 @@ 
 

          t = BuildinstallThread(self.pool)
 

  

          with mock.patch('time.sleep'):
 

-             t.process((compose, 'x86_64', None, self.cmd), 0)
 

+             t.process((compose, "amd64", None, self.cmd), 0)
 

  

+         destdir = os.path.join(self.topdir, "work/amd64/buildinstall")
 

          self.assertItemsEqual(
 

              get_runroot_cmd.mock_calls,
 

              [mock.call(
 

-                 'rrt', 'x86_64', self.cmd, channel=None,
 

+                 "rrt", "amd64", self.cmd, channel=None,
 

                  use_shell=True, task_id=True,
 

                  packages=['anaconda'], mounts=[self.topdir], weight=None,
 

-                 destdir=os.path.join(self.topdir, "work/x86_64/buildinstall"),
 

+                 destdir=destdir,
 

              )])
 

          self.assertItemsEqual(
 

              run_runroot_cmd.mock_calls,
 

              [mock.call(get_runroot_cmd.return_value,
 

-                        log_file=self.topdir + '/logs/x86_64/buildinstall.x86_64.log')])
 

-         with open(self.topdir + '/logs/x86_64/buildinstall-RPMs.x86_64.log') as f:
 

+                        log_file=self.topdir + "/logs/amd64/buildinstall.amd64.log")])
 

+         with open(self.topdir + "/logs/amd64/buildinstall-RPMs.amd64.log") as f:
 

              rpms = f.read().strip().split('\n')
 

          self.assertItemsEqual(rpms, ['bash', 'zsh'])
 

-         self.assertItemsEqual(self.pool.finished_tasks, [(None, 'x86_64')])
 

+         self.assertItemsEqual(self.pool.finished_tasks, [(None, 'amd64')])
 

+         self.assertItemsEqual(
 

+             mock_tweak.call_args_list,
 

+             [
 

+                 mock.call(
 

+                     compose,
 

+                     destdir,
 

+                     os.path.join(self.topdir, "compose", var, "amd64/os"),
 

+                     "amd64",
 

+                     var,
 

+                     "",
 

+                     "dummy-volid",
 

+                     self.pool.kickstart_file,
 

+                 )
 

+                 for var in ["Client", "Server"]
 

+             ],
 

+         )
 

+         self.assertItemsEqual(
 

+             mock_link.call_args_list,
 

+             [
 

+                 mock.call(compose, "amd64", compose.variants["Client"], False),
 

+                 mock.call(compose, "amd64", compose.variants["Server"], False),
 

+             ],
 

+         )
 

  

      @mock.patch('pungi.wrappers.kojiwrapper.KojiWrapper')
 

      @mock.patch('pungi.wrappers.kojiwrapper.get_buildroot_rpms')
 
@@ -922,12 +805,15 @@ 
 

          self.assertTrue(os.path.exists(dummy_file))
 

          self.assertItemsEqual(self.pool.finished_tasks, [])
 

  

+     @mock.patch('pungi.phases.buildinstall.link_boot_iso')
 

+     @mock.patch('pungi.phases.buildinstall.tweak_buildinstall')
 

      @mock.patch('pungi.wrappers.kojiwrapper.KojiWrapper')
 

      @mock.patch('pungi.wrappers.kojiwrapper.get_buildroot_rpms')
 

      @mock.patch('pungi.phases.buildinstall.run')
 

      @mock.patch('pungi.phases.buildinstall.copy_all')
 

      def test_buildinstall_thread_with_lorax_custom_buildinstall_topdir(
 

-             self, copy_all, run, get_buildroot_rpms, KojiWrapperMock):
 

+             self, copy_all, run, get_buildroot_rpms, KojiWrapperMock, mock_tweak, mock_link
 

+     ):
 

          compose = BuildInstallCompose(self.topdir, {
 

              'buildinstall_method': 'lorax',
 

              'runroot': True,
 
@@ -980,6 +866,26 @@ 
 

                         os.path.join(self.topdir, 'logs/x86_64/buildinstall-Server-logs'))]
 

          )
 

  

+         self.assertEqual(
 

+             mock_tweak.call_args_list,
 

+             [
 

+                 mock.call(
 

+                     compose,
 

+                     os.path.join(self.topdir, "work/x86_64/buildinstall/Server"),
 

+                     os.path.join(self.topdir, "compose/Server/x86_64/os"),
 

+                     "x86_64",
 

+                     "Server",
 

+                     "",
 

+                     "dummy-volid",
 

+                     self.pool.kickstart_file,
 

+                 )
 

+             ],
 

+         )
 

+         self.assertEqual(
 

+             mock_link.call_args_list,
 

+             [mock.call(compose, "x86_64", compose.variants["Server"], False)],
 

+         )
 

+ 

  

  class TestSymlinkIso(PungiTestCase):

Instead of running the copy from the main script explicitly, make it part of the thread.

This should make things very slightly faster, and the code is much simpler.

Fixes: https://pagure.io/pungi/issue/959

rebased onto b818e4247eb0c24f398642d6f42d605557eec1c9
5 years ago

rebased onto a6de8646d070e58851cf349446fab6770a698496
5 years ago

I tested this on stage and the results looked similar to what original code produced.

I haven't seen any issue with this.

rebased onto 2dd3000
5 years ago

Pull-Request has been merged by lsedlar
5 years ago
Changes Summary 3
+0 -3
file changed
bin/pungi-koji
+36 -34
file changed
pungi/phases/buildinstall.py
+90 -184
file changed
tests/test_buildinstall.py
Powered by Pagure 5.14.1
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.