diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 29b6e0a6..114d5306 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -1474,15 +1474,25 @@ def handle_import_cg(goptions, session, args): help="Attempt to hardlink instead of uploading") parser.add_option("--test", action="store_true", help="Don't actually import") parser.add_option("--token", action="store", default=None, help="Build reservation token") + parser.add_option("--draft", action="store_true", default=False, help="Import as a draft") + parser.add_option("--build-id", action="store", type="int", default=None, + help="Reserved build id") (options, args) = parser.parse_args(args) if len(args) < 2: parser.error("Please specify metadata files directory") activate_session(session, goptions) + + # get the metadata metadata = koji.load_json(args[0]) + if options.build_id: + metadata['build']['build_id'] = options.build_id + if options.draft: + metadata['build']['draft'] = True + + # prepare output list for upload if 'output' not in metadata: error("Metadata contains no output") localdir = args[1] - to_upload = [] for info in metadata['output']: if info.get('metadata_only', False): @@ -1495,10 +1505,9 @@ def handle_import_cg(goptions, session, args): if options.test: return - # get upload path - # XXX - need a better way + # upload our outputs + # TODO - need a better way to select upload path serverdir = unique_path('cli-import') - for localpath, info in to_upload: relpath = os.path.join(serverdir, info.get('relpath', '')) if _running_in_bg() or options.noprogress: @@ -1516,6 +1525,58 @@ def handle_import_cg(goptions, session, args): session.CGImport(metadata, serverdir, options.token) +def handle_reserve_cg(goptions, session, args): + "[admin] Reserve a build entry for later import" + usage = "usage: %prog reserve-cg [options] |" + parser = OptionParser(usage=get_usage_str(usage)) + parser.add_option("--draft", action="store_true", default=False, help="Reserve a draft entry") + parser.add_option("--epoch", type="int", help="Specify epoch for nvr argument") + parser.add_option("--metadata", action="store_true", default=False, + help="Treat argument as a metadata file") + (options, args) = parser.parse_args(args) + if len(args) != 2: + parser.error("The command takes exactly two arguments") + activate_session(session, goptions) + + cg = args[0] + + def read_nvr(): + try: + data = koji.parse_NVR(args[1]) + except Exception: + return None + # fix epoch + if options.epoch is not None: + data['epoch'] = options.epoch + elif data['epoch'] == '': + data['epoch'] = None + else: + data['epoch'] = int(data['epoch']) + return data + + def read_metadata(): + if options.epoch is not None: + parser.error('The --epoch option cannot be used with --metadata') + metadata = koji.load_json(args[1]) + data = metadata['build'] + fields = ('name', 'version', 'release', 'epoch', 'draft') + data = koji.util.dslice(data, fields, strict=False) + return data + + if options.metadata: + data = read_metadata() + else: + data = read_nvr() + if not data: + data = read_metadata() + + if options.draft: + data['draft'] = True + + result = session.CGInitBuild(cg, data) + print('Reserved build %(build_id)s with token %(token)r' % result) + + def handle_import_comps(goptions, session, args): "Import group/package information from a comps file" usage = "usage: %prog import-comps [options] " diff --git a/docs/source/content_generator_metadata.rst b/docs/source/content_generator_metadata.rst index 190209c3..a79de5de 100644 --- a/docs/source/content_generator_metadata.rst +++ b/docs/source/content_generator_metadata.rst @@ -46,6 +46,7 @@ The build map contains the following entries: is optional. - build_id: Reserved build ID. This field is optional. - task_id: In case there is a corresponding task in koji (int/None) +- draft: True if the build should be imported as a draft. Optional. - extra: A map of extra metadata associated with the build, which must include at least one of: diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py index d645e71d..c3268545 100644 --- a/kojihub/kojihub.py +++ b/kojihub/kojihub.py @@ -6794,7 +6794,6 @@ def cg_init_build(cg, data): other values will be ignored anyway (owner, state, ...) :return: dict with build_id and token """ - reject_draft(data) assert_cg(cg) cg_id = lookup_name('content_generator', cg, strict=True)['id'] data['owner'] = context.session.user_id @@ -6994,6 +6993,7 @@ class CG_Importer(object): 'release': self.buildinfo['release'], 'btypes': list(self.typeinfo), 'source': self.buildinfo.get('source'), + 'draft': self.buildinfo.get('draft'), 'metadata_only': self.metadata_only, 'cg_list': [self.cg], # TODO: provide more data @@ -7058,9 +7058,19 @@ class CG_Importer(object): raise koji.GenericError('Build is owned by task %(task_id)s' % buildinfo) if buildinfo['state'] != koji.BUILD_STATES['BUILDING']: raise koji.GenericError('Build ID %s is not in BUILDING state' % build_id) - if buildinfo['name'] != metadata['build']['name'] or \ - buildinfo['version'] != metadata['build']['version'] or \ - buildinfo['release'] != metadata['build']['release']: + if buildinfo['draft'] != metadata['build'].get('draft', False): + raise koji.GenericError("Draft field does not match reservation (build id = %s)" + % build_id) + if (buildinfo['name'] != metadata['build']['name'] or + buildinfo['version'] != metadata['build']['version']): + raise koji.GenericError("Build (%i) NVR is different" % build_id) + # release is complicated by draft field + if buildinfo['draft']: + # metadata should have the target release, convert it before we check + release = koji.gen_draft_release(metadata['build']['release'], build_id) + else: + release = metadata['build']['release'] + if buildinfo['release'] != release: raise koji.GenericError("Build (%i) NVR is different" % build_id) if ('epoch' in metadata['build'] and buildinfo['epoch'] != metadata['build']['epoch']): @@ -7070,16 +7080,16 @@ class CG_Importer(object): elif token is not None: raise koji.GenericError('Reservation token given, but no build_id ' 'in metadata') + else: # no build reservation buildinfo = get_build(metadata['build'], strict=False) if buildinfo: if (koji.BUILD_STATES[buildinfo['state']] not in ('CANCELED', 'FAILED')): - raise koji.GenericError("Build already exists: %r" % buildinfo) + raise koji.GenericError("Build already exists: %(nvr)s (id=%(id)s)" + % buildinfo) # note: the checks in recycle_build will also apply when we call new_build later - - if buildinfo: - reject_draft(buildinfo) + # our state check is stricter than the one in recycle_build # gather needed data buildinfo = dslice(metadata['build'], ['name', 'version', 'release', 'extra', 'source']) @@ -7087,6 +7097,7 @@ class CG_Importer(object): buildinfo['build_id'] = metadata['build']['build_id'] # epoch is not in the metadata spec, but we allow it to be specified buildinfo['epoch'] = metadata['build'].get('epoch', None) + buildinfo['draft'] = metadata['build'].get('draft', False) buildinfo['start_time'] = convert_timestamp(float(metadata['build']['start_time'])) buildinfo['completion_time'] = convert_timestamp(float(metadata['build']['end_time'])) owner = metadata['build'].get('owner', None) diff --git a/tests/test_cli/data/list-commands-admin.txt b/tests/test_cli/data/list-commands-admin.txt index 3122eb63..66776194 100644 --- a/tests/test_cli/data/list-commands-admin.txt +++ b/tests/test_cli/data/list-commands-admin.txt @@ -55,6 +55,7 @@ admin commands: remove-tag Remove a tag remove-tag-inheritance Remove a tag inheritance link remove-target Remove a build target + reserve-cg Reserve a build entry for later import restart-hosts Restart enabled hosts revoke-cg-access Remove a user from a content generator revoke-permission Revoke a permission from a user diff --git a/tests/test_cli/data/list-commands.txt b/tests/test_cli/data/list-commands.txt index 38041dd3..a2057720 100644 --- a/tests/test_cli/data/list-commands.txt +++ b/tests/test_cli/data/list-commands.txt @@ -55,6 +55,7 @@ admin commands: remove-tag Remove a tag remove-tag-inheritance Remove a tag inheritance link remove-target Remove a build target + reserve-cg Reserve a build entry for later import restart-hosts Restart enabled hosts revoke-cg-access Remove a user from a content generator revoke-permission Revoke a permission from a user diff --git a/tests/test_cli/test_import_cg.py b/tests/test_cli/test_import_cg.py index d384dcb6..b229e10f 100644 --- a/tests/test_cli/test_import_cg.py +++ b/tests/test_cli/test_import_cg.py @@ -1,35 +1,30 @@ from __future__ import absolute_import +import json import os -import unittest +import shutil +import tempfile try: from unittest import mock - from unittest.mock import call except ImportError: import mock - from mock import call -import six - -from koji_cli.commands import handle_import_cg +import koji +from koji_cli.commands import handle_import_cg, _progress_callback from . import utils class TestImportCG(utils.CliTestCase): - def mock_os_path_exists(self, filepath): - if filepath in self.custom_os_path_exists: - return self.custom_os_path_exists[filepath] - return self.os_path_exists(filepath) def setUp(self): self.maxDiff = None self.options = mock.MagicMock() self.session = mock.MagicMock() - self.custom_os_path_exists = {} - self.os_path_exists = os.path.exists + self.workdir = tempfile.mkdtemp() + self.outdir = self.workdir + '/output' self.unique_path_mock = mock.patch('koji_cli.commands.unique_path').start() self.running_in_bg = mock.patch('koji_cli.commands._running_in_bg').start() self.running_in_bg.return_value = False - self.linked_upload_mock = mock.patch('koji_cli.commands.linked_upload').start() + self.linked_upload = mock.patch('koji_cli.commands.linked_upload').start() self.activate_session_mock = mock.patch('koji_cli.commands.activate_session').start() self.error_format = """Usage: %s import-cg [options] (Specify the --help global option for a list of other help options) @@ -39,143 +34,210 @@ class TestImportCG(utils.CliTestCase): def tearDown(self): mock.patch.stopall() + shutil.rmtree(self.workdir) - @mock.patch('sys.stdout', new_callable=six.StringIO) - @mock.patch('koji_cli.commands._progress_callback') - @mock.patch('koji.json') - def test_handle_import_cg(self, json_mock, progress_callback_mock, stdout): + def make_metadata(self, metadata=None, defaults=True, write_outputs=True): + """Fill in metadata and write to workdir""" + if metadata is None: + metadata = {} + + if defaults: + build = metadata.setdefault('build', {}) + build.setdefault('name', 'mypackage') + build.setdefault('version', '1') + build.setdefault('release', '2') + + if 'output' not in metadata: + metadata['output'] = [ + {'relpath': 'relative/path', 'filename': 'file-1'}, + {'relpath': 'relative/path', 'filename': 'file-2'}, + ] + + # write metdata + fn = os.path.join(self.workdir, 'metadata.json') + with open(fn, 'wt') as fd: + json.dump(metadata, fd, indent=4) + + if write_outputs: + self.write_outputs(metadata) + + return metadata, fn + + def write_outputs(self, metadata): + # write outputs + for info in metadata.get('output', []): + fdir = os.path.join(self.outdir, info.get('relpath', '')) + koji.ensuredir(fdir) + fn = os.path.join(fdir, info['filename']) + with open(fn, 'wt') as fd: + fd.write('fnord\n') + + def test_handle_import_cg(self): """Test handle_import_cg function""" - arguments = ['fake-metafile', '/path/to/files/'] - expected = '' - fake_srv_path = '/path/to/server/cli-import' - metadata = { - 'output': [ - {'relpath': '/real/path', 'filename': 'file-1'}, - {'relpath': '/real/path', 'filename': 'file-2'} - ] - } - - # - # we just need to change original os.path.exists behavior, if the input - # is matched return the value we expected. - self.custom_os_path_exists = dict(('%(relpath)s/%(filename)s' % v, True) - for v in metadata['output']) - - # setup and start os.path.exists patch - os_path_exists_patch = mock.patch('os.path.exists', - new=self.mock_os_path_exists) - os_path_exists_patch.start() - - def gen_value(fmt, callback): - calls, expect = [], '' - for item in metadata['output']: - filepath = "%(relpath)s/%(filename)s" % item - calls.append(call(filepath, - item['relpath'], - callback=callback)) - expect += fmt % filepath + "\n" - return calls, expect - - json_mock.load.return_value = metadata - self.unique_path_mock.return_value = fake_srv_path + metadata, fn = self.make_metadata() + arguments = [fn, self.outdir] # Case 1, running in fg, progress on - with mock.patch(utils.get_builtin_open()): - handle_import_cg(self.options, self.session, arguments) + handle_import_cg(self.options, self.session, arguments) - calls, expected = gen_value("Uploading %s\n", progress_callback_mock) - self.assert_console_message(stdout, expected) - self.linked_upload_mock.assert_not_called() - self.session.uploadWrapper.assert_has_calls(calls) - self.session.CGImport.assert_called_with(metadata, fake_srv_path, None) + self.assertEqual(len(self.session.uploadWrapper.mock_calls), len(metadata['output'])) + kwargs = self.session.uploadWrapper.call_args.kwargs + self.assertEqual(kwargs['callback'], _progress_callback) + self.session.CGImport.assert_called_once() + args = self.session.CGImport.call_args.args + self.assertEqual(args[0], metadata) + self.linked_upload.assert_not_called() - # Case 2, running in fg, progress off - with mock.patch(utils.get_builtin_open()): - handle_import_cg(self.options, self.session, arguments + ['--noprogress']) + def test_handle_import_cg_nodir(self): + """Test handle_import_cg function""" - calls, expected = gen_value("Uploading %s", None) - self.assert_console_message(stdout, expected) - self.linked_upload_mock.assert_not_called() - self.session.uploadWrapper.assert_has_calls(calls) - self.session.CGImport.assert_called_with(metadata, fake_srv_path, None) + metadata, fn = self.make_metadata(write_outputs=False) + arguments = [fn] - # reset mocks - self.linked_upload_mock.reset_mock() - self.session.uploadWrapper.reset_mock() - self.session.CGImport.reset_mock() - - # Case 3, --test option - with mock.patch(utils.get_builtin_open()): - handle_import_cg(self.options, self.session, arguments + ['--test']) - - self.linked_upload_mock.assert_not_called() - self.session.uploadWrapper.assert_not_called() - self.session.CGImport.assert_not_called() - - calls = [call("%(relpath)s/%(filename)s" % item, item['relpath']) - for item in metadata['output']] - - # Case 4, --link option - with mock.patch(utils.get_builtin_open()): - handle_import_cg(self.options, self.session, arguments + ['--link']) - - self.linked_upload_mock.assert_has_calls(calls) - self.session.uploadWrapper.assert_not_called() - self.session.CGImport.assert_called_with(metadata, fake_srv_path, None) - - # make sure there is no message on output - self.assert_console_message(stdout, '') - - # stop os.path.exists patch - os_path_exists_patch.stop() - - def test_handle_import_argument_test(self): - """Test handle_import_cg function without arguments""" - arguments = ['fake-metafile', '/path/to/files/'] - - # Case 1. empty argument self.assert_system_exit( handle_import_cg, - self.options, self.session, [], - stderr=self.format_error_message("Please specify metadata files directory"), - activate_session=None) + self.options, self.session, arguments, + stderr=self.format_error_message('Please specify metadata files directory'), + stdout='', + activate_session=None + ) - # Case 2. JSON module does not exist - # dropped - it is now part of stdlib + self.session.uploadWrapper.assert_not_called() + self.session.CGImport.assert_not_called() + self.linked_upload.assert_not_called() - metadata = { - 'output': [ - {'metadata_only': True}, - {'relpath': '/real/path', 'filename': 'filename'} - ] - } + def test_handle_import_cg_output(self): + """Test handle_import_cg function""" - # - # we just need to change original os.path.exists behavior, if the input - # is matched return the value we expected. - self.custom_os_path_exists['/real/path/filename'] = False + metadata, fn = self.make_metadata({}, defaults=False) + arguments = [fn, self.outdir] - with mock.patch(utils.get_builtin_open()): - with mock.patch('os.path.exists', new=self.mock_os_path_exists): - with mock.patch('koji.json') as json_mock: + self.assert_system_exit( + handle_import_cg, + self.options, self.session, arguments, + stderr='Metadata contains no output\n', + stdout='', + activate_session=None, + exit_code=1 + ) - # Case 3. metafile doesn't have output section - json_mock.load.return_value = {} - self.assert_system_exit( - handle_import_cg, - self.options, self.session, arguments, - stderr="Metadata contains no output\n", - exit_code=1) + self.session.uploadWrapper.assert_not_called() + self.session.CGImport.assert_not_called() + self.linked_upload.assert_not_called() - # Case 4. path not exist - file_path = '%(relpath)s/%(filename)s' % metadata['output'][1] - json_mock.load.return_value = metadata - self.assert_system_exit( - handle_import_cg, - self.options, self.session, arguments, - stderr=self.format_error_message("No such file: %s" % file_path), - exit_code=2) + def test_handle_import_cg_nofiles(self): + """Test handle_import_cg function""" + + metadata, fn = self.make_metadata(write_outputs=False) + arguments = [fn, self.outdir] + + expect = "No such file: %s" % os.path.join(self.outdir, 'relative/path/file-1') + self.assert_system_exit( + handle_import_cg, + self.options, self.session, arguments, + stderr=self.format_error_message(expect), + stdout='', + activate_session=None, + exit_code=2 + ) + + self.session.uploadWrapper.assert_not_called() + self.linked_upload.assert_not_called() + self.session.CGImport.assert_not_called() + + def test_handle_import_cg_test(self): + """Test handle_import_cg function""" + + metadata, fn = self.make_metadata() + arguments = [fn, self.outdir, '--test'] + + handle_import_cg(self.options, self.session, arguments) + + self.session.uploadWrapper.assert_not_called() + self.linked_upload.assert_not_called() + self.session.CGImport.assert_not_called() + + def test_handle_import_cg_metaonly(self): + """Test handle_import_cg function""" + + metadata, fn = self.make_metadata(write_outputs=False) + for info in metadata['output']: + info['metadata_only'] = True + metadata, fn = self.make_metadata(metadata, defaults=False, write_outputs=False) + arguments = [fn, self.outdir] + + handle_import_cg(self.options, self.session, arguments) + + self.session.uploadWrapper.assert_not_called() + self.linked_upload.assert_not_called() + self.session.CGImport.assert_called_once() + args = self.session.CGImport.call_args.args + self.assertEqual(args[0], metadata) + + def test_handle_import_cg_draft(self): + """Test handle_import_cg function""" + + metadata, fn = self.make_metadata() + arguments = [fn, self.outdir, '--draft'] + # metadata from the call should have draft flag added + expect = metadata.copy() + expect['build']['draft'] = True + + # Case 1, running in fg, progress on + handle_import_cg(self.options, self.session, arguments) + + self.assertEqual(len(self.session.uploadWrapper.mock_calls), len(metadata['output'])) + self.session.CGImport.assert_called_once() + args = self.session.CGImport.call_args.args + self.assertEqual(args[0], metadata) + + def test_handle_import_cg_reserve(self): + """Test handle_import_cg function""" + + metadata, fn = self.make_metadata() + arguments = [fn, self.outdir, '--build-id', '12345'] + # metadata from the call should have draft flag added + expect = metadata.copy() + expect['build']['build_id'] = 12345 + + # Case 1, running in fg, progress on + handle_import_cg(self.options, self.session, arguments) + + self.assertEqual(len(self.session.uploadWrapper.mock_calls), len(metadata['output'])) + self.session.CGImport.assert_called_once() + args = self.session.CGImport.call_args.args + self.assertEqual(args[0], expect) + + def test_handle_import_cg_linked(self): + """Test handle_import_cg function""" + + metadata, fn = self.make_metadata() + arguments = [fn, self.outdir, '--link'] + + handle_import_cg(self.options, self.session, arguments) + + self.session.uploadWrapper.assert_not_called() + self.assertEqual(len(self.linked_upload.mock_calls), len(metadata['output'])) + self.session.CGImport.assert_called_once() + args = self.session.CGImport.call_args.args + self.assertEqual(args[0], metadata) + + def test_handle_import_cg_noprogress(self): + """Test handle_import_cg function""" + + metadata, fn = self.make_metadata() + arguments = [fn, self.outdir, '--noprogress'] + + handle_import_cg(self.options, self.session, arguments) + + self.assertEqual(len(self.session.uploadWrapper.mock_calls), len(metadata['output'])) + kwargs = self.session.uploadWrapper.call_args.kwargs + self.assertEqual(kwargs['callback'], None) + self.session.CGImport.assert_called_once() + args = self.session.CGImport.call_args.args + self.assertEqual(args[0], metadata) + self.linked_upload.assert_not_called() def test_handle_import_cg_help(self): """Test handle_import_cg help message""" @@ -185,13 +247,14 @@ class TestImportCG(utils.CliTestCase): (Specify the --help global option for a list of other help options) Options: - -h, --help show this help message and exit - --noprogress Do not display progress of the upload - --link Attempt to hardlink instead of uploading - --test Don't actually import - --token=TOKEN Build reservation token + -h, --help show this help message and exit + --noprogress Do not display progress of the upload + --link Attempt to hardlink instead of uploading + --test Don't actually import + --token=TOKEN Build reservation token + --draft Import as a draft + --build-id=BUILD_ID Reserved build id """ % self.progname) -if __name__ == '__main__': - unittest.main() +# the end