From a54f7a19e99c01b8e205b0d524ee2d86d491dbf5 Mon Sep 17 00:00:00 2001 From: Merlin Mathesius Date: Apr 08 2019 17:17:20 +0000 Subject: [PATCH 1/4] Make Koji upload methods more generic so they can be reused. Signed-off-by: Merlin Mathesius --- diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py index 94138b8..aa87d1b 100644 --- a/pyrpkg/__init__.py +++ b/pyrpkg/__init__.py @@ -2423,7 +2423,7 @@ class Commands(object): self.check_repo(is_dirty=False, all_pushed=False) return self.construct_build_url() - def koji_upload(self, file, path, callback=None): + def koji_upload(self, file, path, callback=None, name=None): """Upload a file to koji file is the file you wish to upload @@ -2432,6 +2432,9 @@ class Commands(object): callback is the progress callback to use, if any + name is an alternate upload file name to use on the server if specified, + otherwise basename of local file will be used + Returns nothing or raises """ @@ -2442,7 +2445,7 @@ class Commands(object): raise rpkgError('No active %s session.' % os.path.basename(self.build_client)) # This should have a try and catch koji errors - self.kojisession.uploadWrapper(file, path, callback=callback) + self.kojisession.uploadWrapper(file, path, name=name, callback=callback) def install(self, arch=None, short=False, builddir=None, nocheck=False, buildrootdir=None, define=None): diff --git a/pyrpkg/cli.py b/pyrpkg/cli.py index 1f068a1..1a37004 100644 --- a/pyrpkg/cli.py +++ b/pyrpkg/cli.py @@ -1550,7 +1550,14 @@ see API KEY section of copr-cli(1) man page. def usage(self): self.parser.print_help() - def _upload_srpm_for_build(self): + def _upload_file_for_build(self, file, name=None): + """Upload a file (srpm or module) for building. + :param str file: specify which file to upload. + :param str name: specify alternate basename for file on upload server + :return: a unique path of directory inside server into which the file + has been uploaded. + :rtype: str + """ # Figure out if we want a verbose output or not callback = None if not self.args.q: @@ -1560,16 +1567,18 @@ see API KEY section of copr-cli(1) man page. time.time(), ''.join([random.choice(string.ascii_letters) for i in range(8)]) ) + if not name: + name = os.path.basename(file) # Should have a try here, not sure what errors we'll get yet though if self.args.dry_run: - self.log.info('DRY-RUN: self.cmd.koji_upload(%s, %s, callback=%s)', - self.args.srpm, uniquepath, callback) + self.log.info('DRY-RUN: self.cmd.koji_upload(%s, %s, name=%s, callback=%s)', + file, uniquepath, name, callback) else: - self.cmd.koji_upload(self.args.srpm, uniquepath, callback=callback) + self.cmd.koji_upload(file, uniquepath, name=name, callback=callback) if not self.args.q: # print an extra blank line due to callback oddity print('') - return '%s/%s' % (uniquepath, os.path.basename(self.args.srpm)) + return '%s/%s' % (uniquepath, name) def _handle_srpm_option(self): """Generate SRPM according to --srpm option value and upload it @@ -1585,7 +1594,7 @@ see API KEY section of copr-cli(1) man page. self.log.debug('Generating an srpm') self.srpm() self.args.srpm = '%s.src.rpm' % self.cmd.nvr - return self._upload_srpm_for_build() + return self._upload_file_for_build(self.args.srpm) def _watch_build_tasks(self, task_ids): """Watch build tasks diff --git a/tests/test_cli.py b/tests/test_cli.py index 4d2aa3d..03a482c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -3124,7 +3124,8 @@ class TestBuildPackage(CliTestCase): else: self.assertEqual(expected_srpm_file, srpm_file) six.assertRegex(self, unique_path, r'^cli-build/\d+\.\d+\.[a-zA-Z]+$') - self.assertEqual({'callback': koji_cli.lib._progress_callback}, kwargs) + self.assertEqual({'name': os.path.basename(srpm_file), + 'callback': koji_cli.lib._progress_callback}, kwargs) def test_srpm_option_with_srpm_file(self): self.assert_option_srpm_use('/path/to/docpkg-0.1-1.fc28.src.rpm') From f979bde1ff8e8f989f1156fc6fbba481d5944ad1 Mon Sep 17 00:00:00 2001 From: Merlin Mathesius Date: Apr 08 2019 17:17:20 +0000 Subject: [PATCH 2/4] Refactor fake Koji credential handling from TestBuildPackage class into new FakeKojiCreds class so it can be shared with TestModulesCli class. Signed-off-by: Merlin Mathesius --- diff --git a/tests/test_cli.py b/tests/test_cli.py index 03a482c..c2c07c3 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1936,11 +1936,52 @@ class TestPatch(CliTestCase): os.path.join(cli.cmd.path, patch_file)) +class FakeKojiCreds(object): + + @classmethod + def setUpClass(cls): + super(FakeKojiCreds, cls).setUpClass() + + fake_koji_config = dict( + authtype='kerberos', + server='http://localhost/kojihub', + weburl='http://localhost/koji', + topurl='http://kojipkgs.localhost/', + cert='', + ) + cls.read_config_p = patch('koji.read_config', + return_value=fake_koji_config) + cls.mock_read_config = cls.read_config_p.start() + cls.load_krb_user_p = patch('pyrpkg.Commands._load_krb_user') + cls.mock_load_krb_user = cls.load_krb_user_p.start() + + cls.has_krb_creds_p = patch('pyrpkg.Commands._has_krb_creds', + return_value=True) + cls.mock_has_krb_creds = cls.has_krb_creds_p.start() + + @classmethod + def tearDownClass(cls): + cls.has_krb_creds_p.stop() + cls.load_krb_user_p.stop() + cls.read_config_p.stop() + super(FakeKojiCreds, cls).tearDownClass() + + def setUp(self): + super(FakeKojiCreds, self).setUp() + + self.ClientSession_p = patch('koji.ClientSession') + self.mock_ClientSession = self.ClientSession_p.start() + + def tearDown(self): + self.ClientSession_p.stop() + super(FakeKojiCreds, self).tearDown() + + @unittest.skipUnless( openidc_client, 'Skip if rpkg is rebuilt for an environment where Kerberos authentication' 'is used and python-openidc-client is not available.') -class TestModulesCli(CliTestCase): +class TestModulesCli(FakeKojiCreds, CliTestCase): """Test module commands""" create_repo_per_test = False @@ -1995,6 +2036,17 @@ class TestModulesCli(CliTestCase): 'version': '20171010145511' } + @classmethod + def setUpClass(cls): + super(TestModulesCli, cls).setUpClass() + + @classmethod + def tearDownClass(cls): + super(TestModulesCli, cls).tearDownClass() + + def setUp(self): + super(TestModulesCli, self).setUp() + def tearDown(self): super(TestModulesCli, self).tearDown() @@ -2931,7 +2983,7 @@ class TestOptionNameAndNamespace(CliTestCase): self.assertEqual('modules', cli.cmd.ns) -class TestBuildPackage(CliTestCase): +class TestBuildPackage(FakeKojiCreds, CliTestCase): """Test build package, common build, scratch build and chain build""" create_repo_per_test = False @@ -2942,36 +2994,13 @@ class TestBuildPackage(CliTestCase): super(TestBuildPackage, cls).setUpClass() cls.checkout_branch(git.Repo(cls.cloned_repo_path), 'rhel-7') - fake_koji_config = dict( - authtype='kerberos', - server='http://localhost/kojihub', - weburl='http://localhost/koji', - topurl='http://kojipkgs.localhost/', - cert='', - ) - cls.read_config_p = patch('koji.read_config', - return_value=fake_koji_config) - cls.mock_read_config = cls.read_config_p.start() - cls.load_krb_user_p = patch('pyrpkg.Commands._load_krb_user') - cls.mock_load_krb_user = cls.load_krb_user_p.start() - - cls.has_krb_creds_p = patch('pyrpkg.Commands._has_krb_creds', - return_value=True) - cls.mock_has_krb_creds = cls.has_krb_creds_p.start() - @classmethod def tearDownClass(cls): - cls.has_krb_creds_p.stop() - cls.load_krb_user_p.stop() - cls.read_config_p.stop() super(TestBuildPackage, cls).tearDownClass() def setUp(self): super(TestBuildPackage, self).setUp() - self.ClientSession_p = patch('koji.ClientSession') - self.mock_ClientSession = self.ClientSession_p.start() - session = self.mock_ClientSession.return_value session.getBuildTarget.return_value = { 'id': 1, @@ -3006,7 +3035,6 @@ class TestBuildPackage(CliTestCase): f.write(self.gating_yaml_content) def tearDown(self): - self.ClientSession_p.stop() super(TestBuildPackage, self).tearDown() # Some tests might make changes in the repository for their test From 49cd908427669fa6a9477fd105bd10d80a8acf2a Mon Sep 17 00:00:00 2001 From: Merlin Mathesius Date: Apr 08 2019 18:39:27 +0000 Subject: [PATCH 3/4] Updates to support scratch module builds - Add scratch module build sub-commands and updates for handling custom modulemd file and SRPMs. - Add tests for scratch module builds and adjust existing tests for new 'scratch' option. - Allow custom SRPMs with local module builds for consistency with module scratch builds. Signed-off-by: Merlin Mathesius --- diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py index aa87d1b..e0b6f23 100644 --- a/pyrpkg/__init__.py +++ b/pyrpkg/__init__.py @@ -3360,7 +3360,8 @@ class Commands(object): def module_local_build(self, file_path, stream, local_builds_nsvs=None, verbose=False, debug=False, skip_tests=False, mbs_config=None, mbs_config_section=None, default_streams=None, - offline=False, base_module_repositories=None): + offline=False, base_module_repositories=None, + srpms=None): """ A wrapper for `mbs-manager build_module_locally`. @@ -3387,6 +3388,9 @@ class Commands(object): :param list base_module_repositories: a list of full paths to local .repo files defining the repositories for base module when building with offline set to True. + :param srpms: a list, contains strings with paths to SRPMs to override + components from distgit when building module. + :type srpms: list[str] :return: None """ command = ['mbs-manager'] @@ -3419,6 +3423,10 @@ class Commands(object): for name_stream in default_streams: command.extend(['-s', name_stream]) + if srpms: + for srpm in srpms: + command.extend(['--srpm', srpm]) + env = {} if mbs_config: env['MBS_CONFIG_FILE'] = mbs_config @@ -3614,7 +3622,8 @@ class Commands(object): def module_submit_build(self, scm_url, branch, auth_method, buildrequires=None, requires=None, optional=None, oidc_id_provider=None, oidc_client_id=None, - oidc_client_secret=None, oidc_scopes=None): + oidc_client_secret=None, oidc_scopes=None, + scratch=False, modulemd=None, srpms=[]): """ Submit a module build to the MBS @@ -3637,11 +3646,18 @@ class Commands(object): MBS is using OIDC for authentication. Based on the OIDC setup, this could be None. :kwarg oidc_scopes: a list of OIDC scopes when MBS is using OIDC for authentication. + :param bool scratch: optionally perform a scratch module build, + default is `False`. + :param str modulemd: a string, path of the module's modulemd yaml file + to use for a scratch build. + :param srpms: a list of koji upload links for SRPMs to use for a scratch + build. + :type srpms: list[str] :return: a list of module build IDs that are being built from this request. :rtype: list[int] """ - body = {'scmurl': scm_url, 'branch': branch} + body = {'scmurl': scm_url, 'branch': branch, 'scratch': scratch} optional = optional if optional else [] optional_dict = {} for option in optional: @@ -3660,6 +3676,15 @@ class Commands(object): body[key][dep_name].append(dep_stream) body.update(optional_dict) + + if modulemd: + with open(modulemd, 'rb') as mmd_file: + mmd_content = mmd_file.read().decode('utf-8') + body['modulemd'] = mmd_content + body['module_name'] = str(os.path.splitext(os.path.basename(modulemd))[0]) + if srpms: + body['srpms'] = srpms + url = self.module_get_url(build_id=None, action='POST') resp = self.module_send_authorized_request( 'POST', url, body, auth_method, oidc_id_provider, oidc_client_id, diff --git a/pyrpkg/cli.py b/pyrpkg/cli.py index 1a37004..1f4bcdb 100644 --- a/pyrpkg/cli.py +++ b/pyrpkg/cli.py @@ -20,6 +20,7 @@ import logging import os import random import re +import rpm import string import sys import time @@ -434,6 +435,7 @@ class cliClient(object): # Add a common parsers self.register_build_common() self.register_container_build_common() + self.register_module_build_common() self.register_rpm_common() # Other targets @@ -461,6 +463,7 @@ class cliClient(object): self.register_mockbuild() self.register_mock_config() self.register_module_build() + self.register_module_scratch_build() self.register_module_build_cancel() self.register_module_build_info() self.register_module_local_build() @@ -1101,34 +1104,58 @@ defined, packages will be built sequentially.""" % {'name': self.name}) mock_config_parser.add_argument('--arch', help='Override local arch') mock_config_parser.set_defaults(command=self.mock_config) - def register_module_build(self): - sub_help = 'Build a module using MBS' - self.module_build_parser = self.subparsers.add_parser( - 'module-build', help=sub_help, description=sub_help) - self.module_build_parser.add_argument( + def register_module_build_common(self): + """Create a common module build parser to use in other commands""" + + parser = ArgumentParser( + 'module_build_common', add_help=False, allow_abbrev=False) + self.module_build_parser_common = parser + parser.add_argument( 'scm_url', nargs='?', help='The module\'s SCM URL. This defaults to the current repo.') - self.module_build_parser.add_argument( + parser.add_argument( 'branch', nargs='?', help=('The module\'s SCM branch. This defaults to the current ' 'checked-out branch.')) - self.module_build_parser.add_argument( + parser.add_argument( '--watch', '-w', help='Watch the module build', action='store_true') - self.module_build_parser.add_argument( + parser.add_argument( '--buildrequires', action='append', metavar='name:stream', dest='buildrequires', type=utils.validate_module_dep_override, help='Buildrequires to override in the form of "name:stream"') - self.module_build_parser.add_argument( + parser.add_argument( '--requires', action='append', metavar='name:stream', dest='requires', type=utils.validate_module_dep_override, help='Requires to override in the form of "name:stream"') - self.module_build_parser.add_argument( + parser.add_argument( '--optional', action='append', metavar='key=value', dest='optional', type=utils.validate_module_build_optional, help='MBS optional arguments in the form of "key=value"') + parser.add_argument( + '--file', nargs='?', dest='file_path', + help='The modulemd yaml file for module scratch build.') + parser.add_argument( + '--srpm', action='append', dest='srpms', + help='Include one or more srpms for module scratch build.') + + def register_module_build(self): + sub_help = 'Build a module using MBS' + self.module_build_parser = self.subparsers.add_parser( + 'module-build', parents=[self.module_build_parser_common], + help=sub_help, description=sub_help) + self.module_build_parser.add_argument( + '--scratch', action='store_true', default=False, + help='Perform a scratch build') self.module_build_parser.set_defaults(command=self.module_build) + def register_module_scratch_build(self): + sub_help = 'Build a scratch module using MBS' + self.module_build_parser = self.subparsers.add_parser( + 'module-scratch-build', parents=[self.module_build_parser_common], + help=sub_help, description=sub_help) + self.module_build_parser.set_defaults(command=self.module_scratch_build) + def register_module_build_cancel(self): sub_help = 'Cancel an MBS module build' self.module_build_cancel_parser = self.subparsers.add_parser( @@ -1156,6 +1183,9 @@ defined, packages will be built sequentially.""" % {'name': self.name}) help=('The module\'s modulemd yaml file. If not specified, a yaml file' ' with the same basename as the name of the repository will be used.')) self.module_build_local_parser.add_argument( + '--srpm', action='append', dest='srpms', + help='Include one or more srpms for module build.') + self.module_build_local_parser.add_argument( '--stream', nargs='?', dest='stream', help=('The module\'s stream/SCM branch. This defaults to the current ' 'checked-out branch.')) @@ -1580,6 +1610,24 @@ see API KEY section of copr-cli(1) man page. print('') return '%s/%s' % (uniquepath, name) + def _get_rpm_package_name(self, rpm_file): + """Returns rpm package name by examining rpm file + :param str rpm_file: specify path to RPM file. + :return: the name of the package in the specified file according to + the contained RPM header information, or None if file does not + contain RPM data. + :rtype: str + """ + ts = rpm.TransactionSet() + ts.setVSFlags(rpm._RPMVSF_NOSIGNATURES) + fdno = os.open(rpm_file, os.O_RDONLY) + try: + hdr = ts.hdrFromFdno(fdno) + except rpm.error: + return None + os.close(fdno) + return hdr[rpm.RPMTAG_NAME].decode('utf-8') + def _handle_srpm_option(self): """Generate SRPM according to --srpm option value and upload it @@ -2054,43 +2102,74 @@ see API KEY section of copr-cli(1) man page. """Builds a module using MBS""" self.set_module_api_url() self.module_validate_config() - scm_url, branch = self.cmd.module_get_scm_info( - self.args.scm_url, self.args.branch) + + modmd_path = None + srpm_links = [] + if self.args.file_path or self.args.srpms: + if not self.args.scratch: + raise rpkgError('--file and --srpms may only be used with ' + 'scratch module builds.') + if self.args.file_path: + modmd_path = os.path.abspath(self.args.file_path) + if not os.path.isfile(modmd_path): + raise IOError("Module metadata yaml file %s not found!" % modmd_path) + if self.args.srpms: + for srpm in self.args.srpms: + srpm_path = os.path.abspath(srpm) + if not os.path.isfile(srpm_path): + raise IOError("SRPM file %s not found!" % srpm_path) + srpm_name = "%s.src.rpm" % self._get_rpm_package_name(srpm_path) + srpm_links.append( + self._upload_file_for_build(os.path.abspath(srpm_path), + name=srpm_name)) + + scm_url = None + branch = None + try: + scm_url, branch = self.cmd.module_get_scm_info( + self.args.scm_url, self.args.branch) + except rpkgError as e: + # if a scratch build modulemd was specified on the command line, + # it's OK if the SCM info can't be determined + if not modmd_path: + raise(e) buildrequires = self.args.buildrequires or [] branch_search = None - try: - # The regexes to parse the base module stream override from the dist-git branch name. - # For example, if you had a branch called `10-fedora-29.0.1` and you wanted to - # parse `f29.0.1` from that as the stream to use for the base module, you could - # have a regex of `(f)(?:edora)(?:\-)(\d+\.\d+\.\d+)$`. Then after combining - # the capture groups, you'd get the desired result of `f29.0.1`. - branch_bm_regexes = self.config.get( - self.config_section, 'base_module_stream_regex_from_branch').strip().split('\n') - # The base module (e.g. platform) to apply the buildrequire override to if the parsing - # of the branch name is successfull - bm = self.config.get(self.config_section, 'base_module') - except configparser.NoOptionError: - pass - else: - # Check if any of the regexes match, and break after the first match - for regex in branch_bm_regexes: - branch_search = re.search(regex, branch) - if branch_search: - break - - if branch_search: - # Concatenate all the groups that are not None together to get the desired stream. - # This approach is taken in case there are sections to ignore. - # For instance, if we need to parse `f27.0.0` from `fedora-27.0.0`. - bm_stream = ''.join(group for group in branch_search.groups() if group) - # Don't override the base module buildrequire if the user - # manually already overrode it with the `--buildrequire` argument - if not any(br[0] == bm for br in buildrequires): - buildrequires.append((bm, bm_stream)) - if not self.args.q: - print('Added the buildrequire override "{0}" based on the module branch name' - .format('{0}:{1}'.format(bm, bm_stream))) + if branch: + try: + # The regexes to parse the base module stream override from the dist-git branch + # name. For example, if you had a branch called `10-fedora-29.0.1` and you wanted to + # parse `f29.0.1` from that as the stream to use for the base module, you could + # have a regex of `(f)(?:edora)(?:\-)(\d+\.\d+\.\d+)$`. Then after combining + # the capture groups, you'd get the desired result of `f29.0.1`. + branch_bm_regexes = self.config.get( + self.config_section, 'base_module_stream_regex_from_branch').strip().split('\n') + # The base module (e.g. platform) to apply the buildrequire override to if the + # parsing of the branch name is successfull + bm = self.config.get(self.config_section, 'base_module') + except configparser.NoOptionError: + pass + else: + # Check if any of the regexes match, and break after the first match + for regex in branch_bm_regexes: + branch_search = re.search(regex, branch) + if branch_search: + break + + if branch_search: + # Concatenate all the groups that are not None together to get the desired stream. + # This approach is taken in case there are sections to ignore. + # For instance, if we need to parse `f27.0.0` from `fedora-27.0.0`. + bm_stream = ''.join(group for group in branch_search.groups() if group) + # Don't override the base module buildrequire if the user + # manually already overrode it with the `--buildrequire` argument + if not any(br[0] == bm for br in buildrequires): + buildrequires.append((bm, bm_stream)) + if not self.args.q: + print('Added the buildrequire override "{0}" based on ' + 'the module branch name' + .format('{0}:{1}'.format(bm, bm_stream))) auth_method, oidc_id_provider, oidc_client_id, oidc_client_secret, \ oidc_scopes = self.module_get_auth_config() @@ -2100,7 +2179,8 @@ see API KEY section of copr-cli(1) man page. build_ids = self._cmd.module_submit_build( scm_url, branch, auth_method, buildrequires, self.args.requires, self.args.optional, oidc_id_provider, oidc_client_id, - oidc_client_secret, oidc_scopes) + oidc_client_secret, oidc_scopes, + self.args.scratch, modmd_path, srpm_links) if self.args.watch: try: self.module_watch_build(build_ids) @@ -2153,6 +2233,11 @@ see API KEY section of copr-cli(1) man page. else: file_path = self.args.file_path + if self.args.srpms: + for srpm in self.args.srpms: + if not os.path.isfile(srpm): + raise IOError("SRPM file %s not found!" % srpm) + if not os.path.isfile(file_path): raise IOError("Module metadata yaml file %s not found!" % file_path) @@ -2170,7 +2255,8 @@ see API KEY section of copr-cli(1) man page. mbs_config=mbs_config, mbs_config_section=mbs_config_section, default_streams=self.args.default_streams, offline=self.args.offline, - base_module_repositories=self.args.base_module_repositories) + base_module_repositories=self.args.base_module_repositories, + srpms=self.args.srpms) def module_get_auth_config(self): """Get the authentication configuration for the MBS @@ -2262,6 +2348,11 @@ see API KEY section of copr-cli(1) man page. self.args.limit, finished=(not self.args.unfinished)) + def module_scratch_build(self): + # A module scratch build is just a module build with --scratch + self.args.scratch = True + return self.module_build() + def module_validate_config(self): """Validates the configuration needed for MBS commands diff --git a/tests/test_cli.py b/tests/test_cli.py index c2c07c3..0773d22 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -5,6 +5,7 @@ import glob import hashlib import logging import os +import re import shutil import subprocess import sys @@ -17,7 +18,7 @@ import git import koji_cli.lib import pyrpkg.cli import utils -from mock import Mock, PropertyMock, call, mock_open, patch +from mock import ANY, Mock, PropertyMock, call, mock_open, patch from pyrpkg import Commands, Modulemd, rpkgError from utils import CommandTestCase, FakeThreadPool @@ -56,6 +57,12 @@ else: # The SafeConfigParser class has been renamed to ConfigParser in Python 3.2. ConfigParser = configparser.ConfigParser +KOJI_UNIQUE_PATH_REGEX = r'^cli-build/\d+\.\d+\.[a-zA-Z]+$' + + +def mock_get_rpm_package_name(self, rpm_file): + return(os.path.basename(rpm_file)[:-len('.src.rpm')]) + class CliTestCase(CommandTestCase): @@ -2091,7 +2098,9 @@ class TestModulesCli(FakeKojiCreds, CliTestCase): exp_json = { 'scmurl': ('git://pkgs.fedoraproject.org/modules/testmodule?' '#79d87a5a'), - 'branch': 'master'} + 'branch': 'master', + 'scratch': False, + } mock_oidc_req.assert_called_once_with( exp_url, http_method='POST', @@ -2143,7 +2152,9 @@ class TestModulesCli(FakeKojiCreds, CliTestCase): exp_json = { 'scmurl': ('git://pkgs.fedoraproject.org/modules/testmodule?' '#79d87a5a'), - 'branch': 'master'} + 'branch': 'master', + 'scratch': False, + } mock_oidc_req.assert_called_once_with( exp_url, http_method='POST', @@ -2222,6 +2233,7 @@ class TestModulesCli(FakeKojiCreds, CliTestCase): 'scmurl': ('git://pkgs.fedoraproject.org/modules/testmodule?' '#79d87a5a'), 'branch': '10-LP-f27.0.1', + 'scratch': False, 'buildrequire_overrides': {'platform': ['f27.0.1']} } exp_url = ('https://mbs.fedoraproject.org/module-build-service/2/' @@ -2263,6 +2275,7 @@ class TestModulesCli(FakeKojiCreds, CliTestCase): 'scmurl': ('git://pkgs.fedoraproject.org/modules/testmodule?' '#79d87a5a'), 'branch': '10-fedora-27.0.1', + 'scratch': False, 'buildrequire_overrides': {'platform': ['f29']} } exp_url = ('https://mbs.fedoraproject.org/module-build-service/2/' @@ -2333,8 +2346,210 @@ class TestModulesCli(FakeKojiCreds, CliTestCase): 'https://mbs.fedoraproject.org/module-build-service/1/about/', timeout=60 ) - # Can't verify the calls since the SCM commit hash always changes - mock_oidc_req.assert_called_once() + exp_url = ('https://mbs.fedoraproject.org/module-build-service/2/' + 'module-builds/') + exp_json = { + 'scmurl': ANY, + 'branch': 'master', + 'scratch': False, + } + mock_oidc_req.assert_called_once_with( + exp_url, + http_method='POST', + json=exp_json, + scopes=self.scopes, + timeout=120) + + @patch('sys.stdout', new=StringIO()) + @patch('requests.get') + @patch('openidc_client.OpenIDCClient.send_request') + def test_module_build_scratch(self, mock_oidc_req, mock_get): + """ + Test a scratch module build with default parameters + """ + cli_cmd = [ + 'rpkg', + '--path', + self.cloned_repo_path, + 'module-scratch-build', + ] + mock_get.return_value.ok = True + mock_get.return_value.json.return_value = { + 'auth_method': 'oidc', + 'api_version': 2 + } + mock_oidc_req.return_value.json.return_value = [{'id': 1094}] + + with patch('sys.argv', new=cli_cmd): + cli = self.new_cli() + cli.module_scratch_build() + + output = sys.stdout.getvalue().strip() + expected_output = ( + 'Submitting the module build...', + 'The build 1094 was submitted to the MBS', + 'Build URLs:', + cli.cmd.module_get_url(1094, verbose=False), + ) + self.assertEqual(output, '\n'.join(expected_output)) + mock_get.assert_called_once_with( + 'https://mbs.fedoraproject.org/module-build-service/1/about/', + timeout=60 + ) + exp_url = ('https://mbs.fedoraproject.org/module-build-service/2/' + 'module-builds/') + exp_json = { + 'scmurl': ANY, + 'branch': 'master', + 'scratch': True, + } + mock_oidc_req.assert_called_once_with( + exp_url, + http_method='POST', + json=exp_json, + scopes=self.scopes, + timeout=120) + + @patch('sys.stdout', new=StringIO()) + @patch('requests.get') + @patch('openidc_client.OpenIDCClient.send_request') + def test_module_build_with_scratch_option(self, mock_oidc_req, mock_get): + """ + Test a scratch module build with default parameters using scratch option + """ + cli_cmd = [ + 'rpkg', + '--path', + self.cloned_repo_path, + 'module-build', + '--scratch' + ] + mock_get.return_value.ok = True + mock_get.return_value.json.return_value = { + 'auth_method': 'oidc', + 'api_version': 2 + } + mock_oidc_req.return_value.json.return_value = [{'id': 1094}] + + with patch('sys.argv', new=cli_cmd): + cli = self.new_cli() + cli.module_build() + + output = sys.stdout.getvalue().strip() + expected_output = ( + 'Submitting the module build...', + 'The build 1094 was submitted to the MBS', + 'Build URLs:', + cli.cmd.module_get_url(1094, verbose=False), + ) + self.assertEqual(output, '\n'.join(expected_output)) + mock_get.assert_called_once_with( + 'https://mbs.fedoraproject.org/module-build-service/1/about/', + timeout=60 + ) + exp_url = ('https://mbs.fedoraproject.org/module-build-service/2/' + 'module-builds/') + exp_json = { + 'scmurl': ANY, + 'branch': 'master', + 'scratch': True, + } + mock_oidc_req.assert_called_once_with( + exp_url, + http_method='POST', + json=exp_json, + scopes=self.scopes, + timeout=120) + + @patch('sys.stdout', new=StringIO()) + @patch('pyrpkg.cli.cliClient._get_rpm_package_name', new=mock_get_rpm_package_name) + @patch('requests.get') + @patch('openidc_client.OpenIDCClient.send_request') + def test_module_build_scratch_with_modulemd_and_srpms(self, mock_oidc_req, mock_get): + """ + Test a scratch module build with provided modulemd and srpms + """ + + file_path = os.path.join(self.cloned_repo_path, "modulemd.yaml") + pkg1_path = os.path.join(self.cloned_repo_path, "package1.src.rpm") + pkg2_path = os.path.join(self.cloned_repo_path, "package2.src.rpm") + + cli_cmd = [ + 'rpkg', + '--path', + self.cloned_repo_path, + 'module-scratch-build', + '--srpm', + pkg1_path, + '--file', + file_path, + '--srpm', + pkg2_path, + ] + + mock_get.return_value.ok = True + mock_get.return_value.json.return_value = { + 'auth_method': 'oidc', + 'api_version': 2 + } + mock_oidc_req.return_value.json.return_value = [{'id': 1094}] + + with patch('sys.argv', new=cli_cmd): + cli = self.new_cli() + # we create empty files for the purpose of this test so we don't + # raise an exception + open(file_path, 'a').close() + open(pkg1_path, 'a').close() + open(pkg2_path, 'a').close() + cli.module_scratch_build() + + output = sys.stdout.getvalue().strip() + expected_output = ( + 'Submitting the module build...', + 'The build 1094 was submitted to the MBS', + 'Build URLs:', + cli.cmd.module_get_url(1094, verbose=False), + ) + self.assertEqual(output, '\n'.join(expected_output)) + mock_get.assert_called_once_with( + 'https://mbs.fedoraproject.org/module-build-service/1/about/', + timeout=60 + ) + exp_url = ('https://mbs.fedoraproject.org/module-build-service/2/' + 'module-builds/') + exp_json = { + 'scmurl': ANY, + 'branch': 'master', + 'scratch': True, + 'modulemd': '', + 'module_name': 'modulemd', + 'srpms': ANY, + } + mock_oidc_req.assert_called_once_with( + exp_url, + http_method='POST', + json=exp_json, + scopes=self.scopes, + timeout=120) + + args, kwargs = mock_oidc_req.call_args + + # verify that OIDC request srpms argument contains a list of links + # exactly corresponding to the provided SRPM paths + srpm_paths = [pkg1_path, pkg2_path] + req_srpms = kwargs['json']['srpms'] + self.assertEqual(len(req_srpms), len(srpm_paths)) + for srpm_path in srpm_paths: + filename = os.path.basename(srpm_path) + srpm_link_regex = '{0}/{1}$'.format(KOJI_UNIQUE_PATH_REGEX.rstrip('$'), filename) + p = re.compile(srpm_link_regex) + srpm_link_found = False + for srpm_link in req_srpms: + if p.match(srpm_link): + srpm_link_found = True + break + self.assertTrue(srpm_link_found, + "Did not find srpm {0} amongst {1}".format(filename, req_srpms)) @patch('sys.stdout', new=StringIO()) @patch('requests.get') @@ -2750,6 +2965,45 @@ State: failed file_path, '--stream', 'test'], env={}) @patch.object(Commands, '_run_command') + def test_module_build_local_with_modulemd_and_srpms(self, mock_run): + """ + Test submitting a local module build with provided modulemd and srpms + """ + + file_path = os.path.join(self.cloned_repo_path, 'modulemd.yaml') + pkg1_path = os.path.join(self.cloned_repo_path, "package1.src.rpm") + pkg2_path = os.path.join(self.cloned_repo_path, "package2.src.rpm") + + cli_cmd = [ + 'rpkg', + '--path', + self.cloned_repo_path, + 'module-build-local', + '--file', + file_path, + '--srpm', + pkg1_path, + '--srpm', + pkg2_path, + '--stream', + 'test' + ] + mock_proc = Mock() + mock_proc.returncode = 0 + mock_run.return_value = mock_proc + with patch('sys.argv', new=cli_cmd): + cli = self.new_cli() + # we create empty files for the purpose of this test so we don't raise an exception + open(file_path, 'a').close() + open(pkg1_path, 'a').close() + open(pkg2_path, 'a').close() + cli.module_build_local() + + mock_run.assert_called_once_with(['mbs-manager', 'build_module_locally', + '--file', file_path, '--stream', 'test', + '--srpm', pkg1_path, '--srpm', pkg2_path], env={}) + + @patch.object(Commands, '_run_command') def test_module_build_local_with_skiptests(self, mock_run): """ Test submitting a local module build with skiptests @@ -2987,7 +3241,6 @@ class TestBuildPackage(FakeKojiCreds, CliTestCase): """Test build package, common build, scratch build and chain build""" create_repo_per_test = False - UNIQUE_PATH_REGEX = r'^cli-build/\d+\.\d+\.[a-zA-Z]+$' @classmethod def setUpClass(cls): @@ -3095,7 +3348,7 @@ class TestBuildPackage(FakeKojiCreds, CliTestCase): else: filename = os.path.basename(cli_cmd[i + 1]) match_regex = '{0}/{1}$'.format( - self.UNIQUE_PATH_REGEX.rstrip('$'), filename) + KOJI_UNIQUE_PATH_REGEX.rstrip('$'), filename) six.assertRegex(self, url, match_regex) else: expected_url = '{0}#{1}'.format( From 1fc5ef52f8288e501ea12c890de378c576aba80f Mon Sep 17 00:00:00 2001 From: Merlin Mathesius Date: Apr 08 2019 18:39:27 +0000 Subject: [PATCH 4/4] Improvements for scratch module builds: - Allow module-build-watch to exit when a scratch build completes. - Add scratch status to module-build-info output. Signed-off-by: Merlin Mathesius --- diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py index e0b6f23..7f2b181 100644 --- a/pyrpkg/__init__.py +++ b/pyrpkg/__init__.py @@ -3211,6 +3211,7 @@ class Commands(object): print('Name: {0}'.format(data['name'])) print('Stream: {0}'.format(data['stream'])) print('Version: {0}'.format(data['version'])) + print('Scratch: {0}'.format(data.get('scratch', False))) print('Koji Tag: {0}'.format(data['koji_tag'])) print('Owner: {0}'.format(data['owner'])) print('State: {0}'.format(data['state_name'])) @@ -3779,7 +3780,9 @@ class Commands(object): yield module_builds - all_builds_finish = all(item['state_name'] in ('ready', 'failed') + all_builds_finish = all((item['state_name'] in ('ready', 'failed') + or (item['state_name'] == 'done' + and item.get('scratch', False))) for item in module_builds) if all_builds_finish: break diff --git a/tests/test_cli.py b/tests/test_cli.py index 0773d22..85874ad 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -2692,6 +2692,7 @@ class TestModulesCli(FakeKojiCreds, CliTestCase): Name: python3-ecosystem Stream: master Version: 20171010145511 +Scratch: False Koji Tag: module-14050f52e62d955b Owner: torsava State: failed