From 4e71d456cd50591aa801b650ba4f4780b18ca14d Mon Sep 17 00:00:00 2001 From: "Eric D. Helms" Date: Tue, 25 Jul 2023 09:54:12 -0400 Subject: [PATCH] Refactor Copr build to focus on chroot The Copr build functionality was on partially considering the chroot when looking at if a build exists. Further, there was a lot of copied code between release and scratch that is slimmed down here to allow the by chroot method to properly work. --- .pylintrc | 1 + obal/data/module_utils/obal.py | 58 +++++-- obal/data/modules/copr_build.py | 103 ++++++++++--- obal/data/modules/copr_build_info.py | 2 +- .../roles/build_package/tasks/copr_build.yml | 142 ++++++------------ tests/fixtures/mockbin/mockbin | 4 +- tests/test_functional.py | 10 +- 7 files changed, 191 insertions(+), 129 deletions(-) diff --git a/.pylintrc b/.pylintrc index b950d690..b14a4ac8 100644 --- a/.pylintrc +++ b/.pylintrc @@ -4,3 +4,4 @@ ignore=docs,tests disable=R0205,C0209,W1514,R1735 min-similarity-lines=10 max-line-length=120 +max-locals=20 diff --git a/obal/data/module_utils/obal.py b/obal/data/module_utils/obal.py index e41c0175..ceef5a85 100644 --- a/obal/data/module_utils/obal.py +++ b/obal/data/module_utils/obal.py @@ -10,6 +10,23 @@ from .koji_wrapper import koji, KojiCommandError # pylint:disable=import-error,no-name-in-module +def macro_lookup(command, scl=None, dist=None, macros=None): + """run a macro lookup command""" + if dist: + command += ['--define', 'dist %s' % dist] + else: + command += ['--undefine', 'dist'] + + if scl: + command += ['--define', 'scl %s' % scl] + + if macros is not None: + for (macro, value) in macros.items(): + command += ['--define', '%s %s' % (macro, value)] + + return subprocess.check_output(command, universal_newlines=True) + + def specfile_macro_lookup(specfile, macro_str, scl=None, dist=None, macros=None): """expand a given macro from a specfile""" command = [ @@ -21,19 +38,20 @@ def specfile_macro_lookup(specfile, macro_str, scl=None, dist=None, macros=None) specfile ] - if dist: - command += ['--define', '"dist %s"' % dist] - else: - command += ['--undefine', 'dist'] + return macro_lookup(command, scl=scl, dist=dist, macros=macros) - if scl: - command += ['--define', '"scl %s"' % scl] - if macros is not None: - for (macro, value) in macros.items(): - command += ['--define', '"%s %s"' % (macro, value)] +def srpm_macro_lookup(srpm, macro_str, scl=None, dist=None, macros=None): + """expand a given macro from an srpm""" + command = [ + 'rpmquery', + '--queryformat', + macro_str, + '--package', + srpm + ] - return subprocess.check_output(' '.join(command), universal_newlines=True, shell=True) + return macro_lookup(command, scl=scl, dist=dist, macros=macros) def get_changelog_evr(specfile): @@ -54,21 +72,41 @@ def get_specfile_evr(specfile): return specfile_macro_lookup(specfile, '%{evr}') +def get_srpm_evr(srpm): + """get the EVR from the source header of the srpm""" + return srpm_macro_lookup(srpm, '%{evr}') + + def get_specfile_name(specfile, scl=None): """get the name from the specfile""" return specfile_macro_lookup(specfile, '%{name}', scl=scl) +def get_srpm_name(srpm, scl=None): + """get the name from the srpm""" + return srpm_macro_lookup(srpm, '%{name}', scl=scl) + + def get_specfile_nevr(specfile, scl=None, dist=None, macros=None): """get the name, epoch, version and release from the specfile""" return specfile_macro_lookup(specfile, '%{nevr}', scl=scl, dist=dist, macros=macros) +def get_srpm_nevr(srpm, scl=None, dist=None, macros=None): + """get the name, epoch, version and release from the srpm""" + return srpm_macro_lookup(srpm, '%{nevr}', scl=scl, dist=dist, macros=macros) + + def get_specfile_nvr(specfile, scl=None, dist=None, macros=None): """get the name, version and release from the specfile""" return specfile_macro_lookup(specfile, '%{nvr}', scl=scl, dist=dist, macros=macros) +def get_srpm_nvr(srpm, scl=None, dist=None, macros=None): + """get the name, version and release from the srpm""" + return srpm_macro_lookup(srpm, '%{nvr}', scl=scl, dist=dist, macros=macros) + + def get_whitelist_status(build_command, tag, package): """ Get whitelist status of a given package within a tag. diff --git a/obal/data/modules/copr_build.py b/obal/data/modules/copr_build.py index 6b583c7c..e31dc65e 100644 --- a/obal/data/modules/copr_build.py +++ b/obal/data/modules/copr_build.py @@ -3,14 +3,64 @@ """ import re +import json +from subprocess import CalledProcessError from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.copr import copr_cli, CoprCliCommandError, full_name # pylint:disable=import-error,no-name-in-module +from ansible.module_utils.obal import get_srpm_name, get_srpm_nevr # pylint:disable=import-error,no-name-in-module +def get_package_info(module, user, project, package, config_file=None): + """ + Fetch package info from Copr + """ + command = [ + 'get-package', + full_name(user, project), + '--name', + package, + '--with-all-builds' + ] + + try: + info = json.loads(copr_cli(command, config_file=config_file)) + except CoprCliCommandError as error: + if "Error: No package with name {} in copr {}".format(package, project) in error.message: + info = {} + else: + module.fail_json(msg='Retrieval of package from Copr failed', command=command, output=error.message, + repo_name=full_name(user, project), package=package) + + return info + +def build_exists(nevr, package_info, chroot=None): + """ + Determine if build exists in Copr for a given NEVR + """ + exists = False + + if 'builds' in package_info: + chroots = [ + build for build in package_info['builds'] + if chroot in build['chroots'] + ] + successful_builds = [ + build for build in chroots + if build['state'] == 'succeeded' + ] + successful_nevrs = [ + "{}-{}".format(build['source_package']['name'], build['source_package']['version']) + for build in successful_builds + ] + + exists = nevr in set(successful_nevrs) + + return exists + def main(): """ - Release a package to Copr + Release a package to Copr """ module = AnsibleModule( argument_spec=dict( @@ -18,8 +68,9 @@ def main(): project=dict(type='str', required=True), srpm=dict(type='path', required=True), wait=dict(type='bool', required=False, default=False), - chroots=dict(type='list', required=False), + chroot=dict(type='str', required=False), config_file=dict(type='str', required=False), + force=dict(type='bool', required=False, default=False) ) ) @@ -27,32 +78,42 @@ def main(): project = module.params['project'] srpm = module.params['srpm'] wait = module.params['wait'] - chroots = module.params['chroots'] + chroot = module.params['chroot'] config_file = module.params['config_file'] + force = module.params['force'] - command = [ - 'build', - full_name(user, project), - srpm - ] + try: + package_name = get_srpm_name(srpm) + nevr = get_srpm_nevr(srpm) + except CalledProcessError as error: + module.fail_json(msg="{} / {}".format(error.stdout, error.stderr), command=error.cmd, output=error.output) - if not wait: - command.append('--nowait') + package_info = get_package_info(module, user, project, package_name, config_file) - if chroots: - for chroot in chroots: - command.extend(['--chroot', chroot]) + if force or not build_exists(nevr, package_info, chroot): + command = [ + 'build', + full_name(user, project), + srpm, + '--chroot', + chroot + ] - try: - output = copr_cli(command, config_file=config_file) - except CoprCliCommandError as error: - module.fail_json(msg='Copr build failed', command=error.command, output=error.message, - repo_name=full_name(user, project), srpm=srpm) + if not wait: + command.append('--nowait') + + try: + output = copr_cli(command, config_file=config_file) + except CoprCliCommandError as error: + module.fail_json(msg='Copr build failed', command=error.command, output=error.message, + repo_name=full_name(user, project), srpm=srpm) - build_urls = re.findall(r'^Build was added to.+:\n^\s+(.+)\s*', output, re.MULTILINE) - builds = re.findall(r'^Created builds:\s(\d+)', output, re.MULTILINE) + build_urls = re.findall(r'^Build was added to.+:\n^\s+(.+)\s*', output, re.MULTILINE) + builds = re.findall(r'^Created builds:\s(\d+)', output, re.MULTILINE) - module.exit_json(changed=True, output=output, builds=builds, build_urls=build_urls) + module.exit_json(changed=True, output=output, builds=builds, build_urls=build_urls) + else: + module.exit_json(changed=False) if __name__ == '__main__': diff --git a/obal/data/modules/copr_build_info.py b/obal/data/modules/copr_build_info.py index 8f9ec235..b891dc35 100644 --- a/obal/data/modules/copr_build_info.py +++ b/obal/data/modules/copr_build_info.py @@ -48,7 +48,7 @@ def main(): successful_nevrs = ("{}-{}".format(package, build['source_package']['version']) for build in successful_builds) exists = nevr in successful_nevrs - module.exit_json(exists=exists) + module.exit_json(info=package_info, exists=exists) if __name__ == '__main__': diff --git a/obal/data/roles/build_package/tasks/copr_build.yml b/obal/data/roles/build_package/tasks/copr_build.yml index 5799c5bf..6639bb5c 100644 --- a/obal/data/roles/build_package/tasks/copr_build.yml +++ b/obal/data/roles/build_package/tasks/copr_build.yml @@ -1,111 +1,69 @@ --- -- when: - - not build_package_scratch - - not build_package_copr_rebuild - block: - - name: "Package name" - rpm_nevr: - spec_file: "{{ spec_file_path }}" - scl: "{{ tag.scl | default(omit) }}" - dist: "{{ tag.dist | default(omit) }}" - macros: "{{ tag.macros | default(omit) }}" - register: package_nevr - - - name: Get information about Copr build - copr_build_info: - user: "{{ copr_project_user }}" - project: "{{ copr_project['copr_project_name'] }}" - nevr: "{{ package_nevr.nevr }}" - package: "{{ package_nevr.name }}" - config_file: "{{ build_package_copr_config | default(omit) }}" - register: build_info - -- name: Create temporary build directory - tempfile: - state: directory - suffix: srpms - register: srpm_directory - when: srpm_directory is not defined or ('exists' in build_info and not build_info.exists) or build_package_copr_rebuild - -- name: 'Build SRPM' - srpm: - package: "{{ inventory_dir }}/{{ package_base_dir }}/{{ inventory_hostname }}" - output: "{{ srpm_directory.path if 'path' in srpm_directory else srpm_directory }}" - source_location: "{{ source_location | default(omit) }}" - source_system: "{{ source_system | default(omit) }}" - register: srpm_build - when: (srpm_directory is defined and (build_info.exists is defined and not build_info.exists)) or build_package_copr_rebuild or build_package_scratch - -- when: (not build_package_scratch and (build_info.exists is defined and not build_info.exists)) or build_package_copr_rebuild - block: - - name: 'Run build' - copr_build: - user: "{{ copr_project_user }}" - project: "{{ copr_project['copr_project_name'] }}" - srpm: "{{ srpm_build.path }}" - chroots: "{{ chroot }}" - config_file: "{{ build_package_copr_config | default(omit) }}" - register: copr_builds - ignore_errors: "{{ build_package_skip_failed_build | default(false) }}" - loop: "{{ copr_project['copr_project_chroots'] | map(attribute='name') | list }}" - loop_control: - loop_var: chroot - - - name: 'Created tasks' - debug: - msg: "{{ copr_builds['results'] | map(attribute='build_urls') | list | flatten }}" +- block: + - name: Create temporary build directory + tempfile: + state: directory + suffix: srpms + register: srpm_directory + when: srpm_directory is not defined - - name: 'Wait for tasks to finish' - include_tasks: wait.yml - when: build_package_wait|bool + - include_role: + name: build_srpm vars: - copr_build_ids: "{{ copr_builds['results'] | map(attribute='builds') | list | flatten }}" + build_srpm_output_dir: "{{ srpm_directory.path if 'path' in srpm_directory else srpm_directory }}" + +- name: Define Copr project user + set_fact: + build_package_project_user: "{{ copr_scratch_user | default(copr_project_user) }}" - when: build_package_scratch block: - name: Define Copr scratch project name set_fact: - copr_scratch_project: "{{ copr_project['copr_project_name'] }}-scratch-{{ 999999999 | random | to_uuid }}" - when: copr_scratch_project is not defined - - - name: Define Copr scatch user name - set_fact: - copr_scratch_user: "{{ copr_project_user }}" - when: copr_scratch_user is not defined + build_package_project_name: "{{ copr_scratch_project | default(copr_project['copr_project_name'] + '-scratch-' + (999999999 | random | to_uuid)) }}" - include_role: name: copr_project vars: - copr_project_user: "{{ copr_scratch_user }}" - copr_project_name: "{{ copr_scratch_project }}" + copr_project_user: "{{ build_package_project_user }}" + copr_project_name: "{{ build_package_project_name }}" copr_project_chroots: "{{ copr_project['copr_project_chroots'] }}" copr_project_description: "{{ copr_project['copr_project_description'] | default('') }}" copr_project_delete_after_days: "4" copr_project_unlisted_on_homepage: "{{ copr_project['copr_project_unlisted_on_homepage'] | default(true) }}" copr_project_copr_config: "{{ build_package_copr_config | default('') }}" - - name: 'Run build' - copr_build: - user: "{{ copr_scratch_user }}" - project: "{{ copr_scratch_project }}" - srpm: "{{ srpm_build.path }}" - chroots: "{{ chroot }}" - config_file: "{{ build_package_copr_config | default(omit) }}" - register: copr_builds - loop: "{{ copr_project['copr_project_chroots'] | map(attribute='name') | list }}" - loop_control: - loop_var: chroot +- name: 'Run build' + copr_build: + user: "{{ build_package_project_user }}" + project: "{{ build_package_project_name | default(copr_project['copr_project_name']) }}" + srpm: "{{ srpm_build.path }}" + chroot: "{{ chroot }}" + config_file: "{{ build_package_copr_config | default(omit) }}" + force: "{{ build_package_copr_rebuild }}" + register: copr_builds + ignore_errors: "{{ build_package_skip_failed_build | default(false) }}" + loop: "{{ copr_project['copr_project_chroots'] | map(attribute='name') | list }}" + loop_control: + loop_var: chroot - - name: 'Created tasks' - debug: - msg: "{{ copr_builds['results'] | map(attribute='build_urls') | list | flatten }}" +- name: 'Created tasks' + when: copr_builds is changed + debug: + msg: "{{ copr_builds['results'] | selectattr('build_urls', 'defined') | map(attribute='build_urls') | list | flatten }}" - - name: 'Wait for tasks to finish' - include_tasks: wait.yml - when: build_package_wait|bool - vars: - copr_build_ids: "{{ copr_builds['results'] | map(attribute='builds') | list | flatten }}" +- name: 'Wait for tasks to finish' + when: + - build_package_wait|bool + - copr_builds is changed + include_tasks: wait.yml + vars: + copr_build_ids: "{{ copr_builds['results'] | selectattr('builds', 'defined') | map(attribute='builds') | list | flatten }}" +- when: + - build_package_archive_build_info + - copr_builds is changed + block: - name: Create build info directory file: path: "{{ inventory_dir }}/copr_build_info" @@ -120,8 +78,8 @@ content: "{{ copr_builds | to_yaml }}" mode: '0644' - - name: 'Download builds' - include_tasks: download_rpms.yml - when: build_package_download_rpms|bool - vars: - chroots: "{{ copr_project['copr_project_chroots'] | map(attribute='name') | list }}" +- name: 'Download builds' + include_tasks: download_rpms.yml + when: build_package_download_rpms|bool + vars: + chroots: "{{ copr_project['copr_project_chroots'] | map(attribute='name') | list }}" diff --git a/tests/fixtures/mockbin/mockbin b/tests/fixtures/mockbin/mockbin index 56a86c30..f59c4059 100755 --- a/tests/fixtures/mockbin/mockbin +++ b/tests/fixtures/mockbin/mockbin @@ -199,7 +199,9 @@ elif prog in ['copr-cli']: if args.subcommand == 'get-package': #json_data = json.loads('{"builds": [{"state": "succeeded", "source_package": {"version": "0.0.1"}}]}') - json_data = {"builds": [{"state": "succeeded", "source_package": {"version": "0.0.1"}}]} + json_data = {"builds": [ + {"chroots": ["rhel-7-x86_64"], "state": "succeeded", "source_package": {"version": "0.0.1"}} + ]} print(json.dumps(json_data, indent=2)) elif args.subcommand == 'build': print("Build was added to foreman-1234:") diff --git a/tests/test_functional.py b/tests/test_functional.py index e29d33c6..046a9ba4 100644 --- a/tests/test_functional.py +++ b/tests/test_functional.py @@ -665,7 +665,8 @@ def test_obal_scratch_copr_hello_nowait(): "copr-cli list example", "copr-cli create example/foreman-1234 --description foreman-1234 --appstream off --chroot epel-7-x86_64 --unlisted-on-hp on --delete-after-days 4", # noqa: E501 "copr-cli edit-chroot example/foreman-1234/epel-7-x86_64 --repos http://mirror.centos.org/centos/7/sclo/x86_64/rh/ --packages 'rh-ruby24-build scl-utils-build'", # noqa: E501 - "copr-cli build example/foreman-1234 /tmp/SRPMs/hello-2.10-1.src.rpm --nowait --chroot epel-7-x86_64" + "copr-cli get-package example/foreman-1234 --name hello --with-all-builds", + "copr-cli build example/foreman-1234 /tmp/SRPMs/hello-2.10-1.src.rpm --chroot epel-7-x86_64 --nowait" ] assert_mockbin_log(expected_log) @@ -680,7 +681,8 @@ def test_obal_scratch_copr_hello(): "copr-cli list example", "copr-cli create example/foreman-1234 --description foreman-1234 --appstream off --chroot epel-7-x86_64 --unlisted-on-hp on --delete-after-days 4", # noqa: E501 "copr-cli edit-chroot example/foreman-1234/epel-7-x86_64 --repos http://mirror.centos.org/centos/7/sclo/x86_64/rh/ --packages 'rh-ruby24-build scl-utils-build'", # noqa: E501 - "copr-cli build example/foreman-1234 /tmp/SRPMs/hello-2.10-1.src.rpm --nowait --chroot epel-7-x86_64", + "copr-cli get-package example/foreman-1234 --name hello --with-all-builds", + "copr-cli build example/foreman-1234 /tmp/SRPMs/hello-2.10-1.src.rpm --chroot epel-7-x86_64 --nowait", "copr-cli watch-build 5678" ] assert_mockbin_log(expected_log) @@ -694,7 +696,7 @@ def test_obal_release_copr_hello_nowait(): expected_log = [ "copr-cli get-package example/foreman --name hello --with-all-builds", - "copr-cli build example/foreman /tmp/SRPMs/hello-2.10-1.src.rpm --nowait --chroot epel-7-x86_64" + "copr-cli build example/foreman /tmp/SRPMs/hello-2.10-1.src.rpm --chroot epel-7-x86_64 --nowait" ] assert_mockbin_log(expected_log) @@ -707,7 +709,7 @@ def test_obal_release_copr_hello(): expected_log = [ "copr-cli get-package example/foreman --name hello --with-all-builds", - "copr-cli build example/foreman /tmp/SRPMs/hello-2.10-1.src.rpm --nowait --chroot epel-7-x86_64", + "copr-cli build example/foreman /tmp/SRPMs/hello-2.10-1.src.rpm --chroot epel-7-x86_64 --nowait", "copr-cli watch-build 5678" ] assert_mockbin_log(expected_log)