From 125df6771bdc2600d80aaefea6ff709288fdf65a Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 10 2016 16:06:30 +0000 Subject: [PATCH 1/3] use correct temporary dirs in openRemoteFile and mergerepos --- diff --git a/builder/kojid b/builder/kojid index 240b5ef..c225713 100755 --- a/builder/kojid +++ b/builder/kojid @@ -650,6 +650,7 @@ class BuildRoot(object): repomdpath = os.path.join(repodir, self.br_arch, 'repodata', 'repomd.xml') opts = dict([(k, getattr(self.options, k)) for k in 'topurl','topdir']) + opts['tempdir'] = self.options.workdir fo = koji.openRemoteFile(repomdpath, **opts) try: repodata = repoMDObject.RepoMD('ourrepo', fo) @@ -904,6 +905,7 @@ class BuildTask(BaseTaskHandler): self.logger.debug("Reading SRPM") relpath = "work/%s" % srpm opts = dict([(k, getattr(self.options, k)) for k in 'topurl','topdir']) + opts['tempdir'] = self.workdir fo = koji.openRemoteFile(relpath, **opts) h = koji.get_rpm_header(fo) fo.close() @@ -3150,6 +3152,7 @@ class OzImageTask(BaseTaskHandler): kspath = os.path.join(scmsrcdir, os.path.basename(ksfile)) else: tops = dict([(k, getattr(self.options, k)) for k in 'topurl','topdir']) + tops['tempdir'] = self.workdir ks_src = koji.openRemoteFile(ksfile, **tops) kspath = os.path.join(self.workdir, os.path.basename(ksfile)) ks_dest = open(kspath, 'w') @@ -3956,6 +3959,7 @@ class BuildIndirectionImageTask(OzImageTask): final_path = os.path.join(scmsrcdir, os.path.basename(filepath)) else: tops = dict([(k, getattr(self.options, k)) for k in 'topurl','topdir']) + tops['tempdir'] = self.workdir remote_fileobj = koji.openRemoteFile(filepath, **tops) final_path = os.path.join(self.workdir, os.path.basename(filepath)) final_fileobj = open(final_path, 'w') diff --git a/builder/mergerepos b/builder/mergerepos index 1f486d6..d47c748 100755 --- a/builder/mergerepos +++ b/builder/mergerepos @@ -73,6 +73,8 @@ def parse_args(args): help="A file containing a list of srpm names to exclude from the merged repo") parser.add_option("-o", "--outputdir", default=None, help="Location to create the repository") + parser.add_option("--tempdir", default=None, + help="Location for temporary files") (opts, argsleft) = parser.parse_args(args) if len(opts.repos) < 1: @@ -109,9 +111,10 @@ def make_const_func(value): class RepoMerge(object): - def __init__(self, repolist, arches, groupfile, blocked, outputdir): + def __init__(self, repolist, arches, groupfile, blocked, outputdir, tempdir=None): self.repolist = repolist self.outputdir = outputdir + self.tempdir = tempdir self.mdconf = createrepo.MetaDataConfig() # explicitly request sha1 for backward compatibility with older yum self.mdconf.sumtype = 'sha1' @@ -125,7 +128,7 @@ class RepoMerge(object): self.yumbase.preconf.debuglevel = 2 else: self.yumbase._getConfig('/dev/null', init_plugins=False, debuglevel=2) - self.yumbase.conf.cachedir = tempfile.mkdtemp() + self.yumbase.conf.cachedir = tempfile.mkdtemp(dir=self.tempdir) self.yumbase.conf.cache = 0 self.archlist = arches self.mdconf.groupfile = groupfile @@ -287,7 +290,8 @@ def main(args): else: blocked = {} - merge = RepoMerge(opts.repos, opts.arches, opts.groupfile, blocked, opts.outputdir) + merge = RepoMerge(opts.repos, opts.arches, opts.groupfile, blocked, + opts.outputdir, opts.tempdir) try: merge.merge_repos() diff --git a/koji/__init__.py b/koji/__init__.py index b453017..067ffb5 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -1438,7 +1438,7 @@ def format_exc_plus(): rv += "\n" return rv -def openRemoteFile(relpath, topurl=None, topdir=None): +def openRemoteFile(relpath, topurl=None, topdir=None, tempdir=None): """Open a file on the main server (read-only) This is done either via a mounted filesystem (nfs) or http, depending @@ -1446,7 +1446,7 @@ def openRemoteFile(relpath, topurl=None, topdir=None): if topurl: url = "%s/%s" % (topurl, relpath) src = urllib2.urlopen(url) - fo = tempfile.TemporaryFile() + fo = tempfile.TemporaryFile(dir=tempdir) shutil.copyfileobj(src, fo) src.close() fo.seek(0) From 0c27f1b1b4172f8e0a3ac93723857067d5e19443 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 10 2016 16:06:30 +0000 Subject: [PATCH 2/3] add unit tests for koji.openRemoteFile --- diff --git a/tests/test_utils.py b/tests/test_utils.py index 9f5b604..68bec02 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -76,3 +76,62 @@ class MiscFunctionTestCase(unittest.TestCase): exists.assert_called_once_with(dst) islink.assert_called_once_with(dst) move.assert_not_called() + + + @mock.patch('urllib2.urlopen') + @mock.patch('tempfile.TemporaryFile') + @mock.patch('shutil.copyfileobj') + @mock.patch('__builtin__.open') + def test_openRemoteFile(self, m_open, m_copyfileobj, m_TemporaryFile, + m_urlopen): + """Test openRemoteFile function""" + + mocks = [m_open, m_copyfileobj, m_TemporaryFile, m_urlopen] + + topurl = 'http://example.com/koji' + path = 'relative/file/path' + url = 'http://example.com/koji/relative/file/path' + + #using topurl, no tempfile + fo = koji.openRemoteFile(path, topurl) + m_urlopen.assert_called_once_with(url) + m_urlopen.return_value.close.assert_called_once() + m_TemporaryFile.assert_called_once_with(dir=None) + m_copyfileobj.assert_called_once() + m_open.assert_not_called() + assert fo is m_TemporaryFile.return_value + + for m in mocks: + m.reset_mock() + + #using topurl + tempfile + tempdir = '/tmp/koji/1234' + fo = koji.openRemoteFile(path, topurl, tempdir=tempdir) + m_urlopen.assert_called_once_with(url) + m_urlopen.return_value.close.assert_called_once() + m_TemporaryFile.assert_called_once_with(dir=tempdir) + m_copyfileobj.assert_called_once() + m_open.assert_not_called() + assert fo is m_TemporaryFile.return_value + + for m in mocks: + m.reset_mock() + + #using topdir + topdir = '/mnt/mykojidir' + filename = '/mnt/mykojidir/relative/file/path' + fo = koji.openRemoteFile(path, topdir=topdir) + m_urlopen.assert_not_called() + m_TemporaryFile.assert_not_called() + m_copyfileobj.assert_not_called() + m_open.assert_called_once_with(filename) + assert fo is m_open.return_value + + for m in mocks: + m.reset_mock() + + # using neither + with self.assertRaises(koji.GenericError): + koji.openRemoteFile(path) + for m in mocks: + m.assert_not_called() From 896b4f91c237fb61d175f21dea8a637ac562dfab Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 10 2016 16:06:30 +0000 Subject: [PATCH 3/3] pass workdir to mergerepos command --- diff --git a/builder/kojid b/builder/kojid index c225713..05438d2 100755 --- a/builder/kojid +++ b/builder/kojid @@ -4801,6 +4801,7 @@ class CreaterepoTask(BaseTaskHandler): cmd = ['/usr/bin/mergerepo_c', '--koji'] else: cmd = ['/usr/libexec/kojid/mergerepos'] + cmd.extend(['--tempdir', self.workdir]) cmd.extend(['-a', arch, '-b', blocklist, '-o', self.outdir]) if os.path.isfile(groupdata): cmd.extend(['-g', groupdata])