From 9b7af72891221f68dd552a7a5dbaa9b9f1e765b1 Mon Sep 17 00:00:00 2001 From: Dominique Leuenberger Date: Thu, 17 Oct 2019 15:05:44 +0200 Subject: [PATCH 1/2] StagingAPI: complex delete requests are no longer used * Simplify the 'accept' code openSUSE:Factory/snapshot is newly setup as a Download on Demand repository and we can thus safe ourselves the hassle of having to take extra care of handling delete requests. They no longer bleed directly into the /snapshot area and can now be accepted like any other request * Drop vdelreq plugin Since we no longer do a virtual accept of delete requests, we also don't have to monitor their state anymore. Drop the plugin that served only this usecase * Drop the acheck command As a consequence, the command 'staging acheck' serves no purpose anymore: /totest was just there to ensure we have something to copy to /snapshot. This entire repo will vanish, and it is thus no longer needed to verify if it is in sync before accepting new data. --- dist/package/openSUSE-release-tools.spec | 18 +---- osc-staging.py | 60 ++++----------- osc-vdelreq.py | 95 ------------------------ osclib/accept_command.py | 78 ++----------------- 4 files changed, 24 insertions(+), 227 deletions(-) delete mode 100644 osc-vdelreq.py diff --git a/dist/package/openSUSE-release-tools.spec b/dist/package/openSUSE-release-tools.spec index 30f68062..382146cb 100644 --- a/dist/package/openSUSE-release-tools.spec +++ b/dist/package/openSUSE-release-tools.spec @@ -74,6 +74,8 @@ Requires: osclib = %{version} Obsoletes: %{name}-totest-manager # no longer supported Obsoletes: osc-plugin-check_dups +# vdelreq is no longer needed/supported; delete requests are handled immediately again +Obsoletes: osc-plugin-vdelreq # Avoid needlessly building on s390x and such in various repos. # Must include noarch for older systems even though it makes no sense due to @@ -305,16 +307,6 @@ Requires: osclib = %{version} %description -n osc-plugin-staging OSC plugin for the staging workflow, see `osc staging --help`. -%package -n osc-plugin-vdelreq -Summary: OSC plugin to check for virtually accepted request -Group: Development/Tools/Other -BuildArch: noarch -Requires: osc >= 0.165.1 -Requires: osclib = %{version} - -%description -n osc-plugin-vdelreq -OSC plugin to check for virtually accepted request, see `osc vdelreq --help`. - %prep %setup -q @@ -457,7 +449,6 @@ exit 0 %exclude %{_datadir}/%{source_dir}/osc-cycle.py %exclude %{_datadir}/%{source_dir}/osc-origin.py %exclude %{_datadir}/%{source_dir}/osc-staging.py -%exclude %{_datadir}/%{source_dir}/osc-vdelreq.py %exclude %{_datadir}/%{source_dir}/update_crawler.py %exclude %{_datadir}/%{source_dir}/findfileconflicts %exclude %{_datadir}/%{source_dir}/write_repo_susetags_file.pl @@ -623,9 +614,4 @@ exit 0 %{_datadir}/%{source_dir}/osc-staging.py %{osc_plugin_dir}/osc-staging.py -%files -n osc-plugin-vdelreq -%defattr(-,root,root,-) -%{_datadir}/%{source_dir}/osc-vdelreq.py -%{osc_plugin_dir}/osc-vdelreq.py - %changelog diff --git a/osc-staging.py b/osc-staging.py index bec5c9f3..a9ca0db4 100644 --- a/osc-staging.py +++ b/osc-staging.py @@ -69,7 +69,7 @@ def _full_project_name(self, project): def lock_needed(cmd, opts): return not( - cmd in ('acheck', 'check', 'check_duplicate_binaries', 'frozenage', 'rebuild', 'unlock') or + cmd in ('check', 'check_duplicate_binaries', 'frozenage', 'rebuild', 'unlock') or (cmd == 'list' and not opts.supersede) ) @@ -136,11 +136,6 @@ def do_staging(self, subcmd, opts, *args): If openSUSE:* project, requests marked ready from adi stagings will also be accepted. - "acheck" will check if it is safe to accept new staging projects - As $PROJECT is syncing the right package versions between - /standard, /totest and /snapshot, it is important that the projects - are clean prior to a checkin round. - "adi" will list already staged requests, stage new requests, and supersede requests where applicable. New adi stagings will be created for new packages based on the grouping options used. The default grouping is by @@ -336,7 +331,6 @@ def do_staging(self, subcmd, opts, *args): Usage: osc staging accept [--force] [--no-cleanup] [LETTER...] - osc staging acheck osc staging adi [--move] [--by-develproject] [--split] [REQUEST...] osc staging check [--old] [STAGING...] osc staging check_duplicate_binaries @@ -389,7 +383,6 @@ def do_staging(self, subcmd, opts, *args): ): min_args, max_args = 1, None elif cmd in ( - 'acheck', 'check_duplicate_binaries', 'cleanup_rings', 'list', @@ -482,44 +475,21 @@ def do_staging(self, subcmd, opts, *args): Fore.YELLOW + prj + Fore.RESET, Fore.GREEN if api.prj_frozen_enough(prj) else Fore.RED, api.days_since_last_freeze(prj))) - elif cmd == 'acheck': - # Is it safe to accept? Meaning: /totest contains what it should and is not dirty - version_totest = api.get_binary_version(api.project, "openSUSE-release.rpm", repository="totest", arch="x86_64") - if version_totest: - version_openqa = api.pseudometa_file_load('version_totest') - totest_dirty = api.is_repo_dirty(api.project, 'totest') - print("version_openqa: %s / version_totest: %s / totest_dirty: %s\n" % (version_openqa, version_totest, totest_dirty)) - else: - print("acheck is unavailable in %s!\n" % (api.project)) elif cmd == 'accept': - # Is it safe to accept? Meaning: /totest contains what it should and is not dirty - version_totest = api.get_binary_version(api.project, "openSUSE-release.rpm", repository="totest", arch="x86_64") - - if version_totest is None or opts.force: - # SLE does not have a totest_version or openqa_version - ignore it - version_openqa = version_totest - totest_dirty = False - else: - version_openqa = api.pseudometa_file_load('version_totest') - totest_dirty = api.is_repo_dirty(api.project, 'totest') - - if version_openqa == version_totest and not totest_dirty: - cmd = AcceptCommand(api) - for prj in args[1:]: - if cmd.perform(api.prj_from_letter(prj), opts.force): - cmd.reset_rebuild_data(prj) - else: - return - if not opts.no_cleanup: - if api.item_exists(api.prj_from_letter(prj)): - cmd.cleanup(api.prj_from_letter(prj)) - cmd.accept_other_new() - if opts.project.startswith('openSUSE:'): - cmd.update_factory_version() - if api.item_exists(api.crebuild): - cmd.sync_buildfailures() - else: - print("Not safe to accept: /totest is not yet synced") + cmd = AcceptCommand(api) + for prj in args[1:]: + if cmd.perform(api.prj_from_letter(prj), opts.force): + cmd.reset_rebuild_data(prj) + else: + return + if not opts.no_cleanup: + if api.item_exists(api.prj_from_letter(prj)): + cmd.cleanup(api.prj_from_letter(prj)) + cmd.accept_other_new() + if opts.project.startswith('openSUSE:'): + cmd.update_factory_version() + if api.item_exists(api.crebuild): + cmd.sync_buildfailures() elif cmd == 'unselect': if opts.message: print('Ignoring requests first') diff --git a/osc-vdelreq.py b/osc-vdelreq.py deleted file mode 100644 index 77c8065e..00000000 --- a/osc-vdelreq.py +++ /dev/null @@ -1,95 +0,0 @@ -#!/usr/bin/python - -from __future__ import print_function - -import os -import os.path -import sys - -from xml.etree import cElementTree as ET - -try: - from urllib.error import HTTPError -except ImportError: - from urllib2 import HTTPError - -import osc.core -import osc.conf - -from osc import cmdln -from osc import oscerr - -def _has_binary(self, project, package): - query = {'view': 'binarylist', 'package': package, 'multibuild': '1'} - pkg_binarylist = ET.parse(osc.core.http_GET(osc.core.makeurl(self.apiurl, ['build', project, '_result'], query=query))).getroot() - for binary in pkg_binarylist.findall('./result/binarylist/binary'): - return 'Yes' - return 'No' - - -def list_virtually_accepted_request(self, project, opts): - state_cond = "state/@name='review'+or+state/@name='revoked'" - if opts.all: - state_cond += "+or+state/@name='accepted'" - query = "match=({})+and+(action/target/@project='{}'+and+action/@type='delete')+and+"\ - "((review/@state='new'+or+review/@state='accepted')+and+review/@by_group='{}')".format(state_cond, project, opts.delreq_review) - url = osc.core.makeurl(self.apiurl, ['search', 'request'], query) - f = osc.core.http_GET(url) - root = ET.parse(f).getroot() - rqs = [] - for rq in root.findall('request'): - has_binary = 'No' - id = rq.attrib['id'] - rq_state = rq.find('state').get('name') - pkg = rq.find('action/target').get('package') - try: - osc.core.show_package_meta(self.apiurl, project, pkg) - except HTTPError as err: - if err.code == 404: - continue - raise err - - if rq_state != 'accepted': - has_binary = self._has_binary(project, pkg) - for review in rq.findall('review'): - if review.get('by_group') and review.attrib['by_group'] == opts.delreq_review: - delreq_review_state = review.attrib['state'] - - content = {"id": int(id), "package": pkg, "rq_state": rq_state, "delreq_review_state": delreq_review_state, "has_binary": has_binary} - rqs.append(content) - - rqs.sort(key=lambda d: d['id']) - for rq in rqs: - print("{} {} state is {} \n {} Virtually accept review is {} ( binary: {} )".format(str(rq['id']), - rq['package'], rq['rq_state'], "-".rjust(len(str(rq['id']))+1, ' '), rq['delreq_review_state'], rq['has_binary'])) - -@cmdln.option('--delreq-review', dest='delreq_review', metavar='DELREQREVIEW', default='factory-maintainers', - help='the additional reviews') -@cmdln.option('--all', action='store_true', default=False, help='shows all requests including accepted request') -def do_vdelreq(self, subcmd, opts, *args): - """${cmd_name}: display pending virtual accept delete request - - osc vdelreq [OPT] COMMAND PROJECT - Shows pending the virtual accept delete requests and the current state. - - ${cmd_option_list} - - "list" will list virtually accepted delete request. - - Usage: - osc vdelreq [--delreq-review DELREQREVIEW] list PROJECT - """ - - self.apiurl = self.get_api_url() - - if len(args) == 0: - raise oscerr.WrongArgs('No command given, see "osc help vdelreq"!') - if len(args) < 2: - raise oscerr.WrongArgs('No project given, see "osc help vdelreq"!') - - cmd = args[0] - if cmd in ('list'): - self.list_virtually_accepted_request(args[1], opts) - else: - raise oscerr.WrongArgs('Unknown command: %s' % cmd) - diff --git a/osclib/accept_command.py b/osclib/accept_command.py index 30217bc1..26e636d0 100644 --- a/osclib/accept_command.py +++ b/osclib/accept_command.py @@ -46,37 +46,6 @@ class AcceptCommand(object): rqs.append({'id': int(rq.get('id')), 'packages': pkgs, 'type': act_type}) return rqs - def virtual_accept_request_has_no_binary(self, project, package): - filelist = self.api.get_filelist_for_package(pkgname=package, project=self.api.project, expand='1', extension='spec') - pkgs = self.api.extract_specfile_short(filelist) - - for pkg in pkgs: - query = {'view': 'binarylist', 'package': pkg, 'multibuild': '1'} - pkg_binarylist = ET.parse(http_GET(self.api.makeurl(['build', project, '_result'], query=query))).getroot() - for binary in pkg_binarylist.findall('./result/binarylist/binary'): - return False - - return True - - def find_virtually_accepted_requests(self, project): - query = "match=state/@name='review'+and+(action/target/@project='{}'+and+action/@type='delete')+and+(review/@state='new'+and+review/@by_group='{}')".format(project, self.api.cdelreq_review) - url = self.api.makeurl(['search', 'request'], query) - - f = http_GET(url) - root = ET.parse(f).getroot() - - rqs = [] - for rq in root.findall('request'): - pkgs = [] - actions = rq.findall('action') - for action in actions: - targets = action.findall('target') - for t in targets: - pkgs.append(str(t.get('package'))) - - rqs.append({'id': int(rq.get('id')), 'packages': pkgs}) - return rqs - def reset_rebuild_data(self, project): data = self.api.pseudometa_file_load('support_pkg_rebuild') if data is None: @@ -94,23 +63,6 @@ class AcceptCommand(object): if content != data: self.api.pseudometa_file_save('support_pkg_rebuild', content, 'accept command update') - def virtually_accept_delete(self, request_id, package): - self.api.add_review(request_id, by_group=self.api.cdelreq_review, msg='Request accepted. Cleanup in progress - DO NOT REVOKE!') - - filelist = self.api.get_filelist_for_package(pkgname=package, project=self.api.project, expand='1', extension='spec') - pkgs = self.api.extract_specfile_short(filelist) - - # Disable build and wipes the binary to the package and the sub-package - for pkg in pkgs: - meta = decode_it(b''.join(show_package_meta(self.api.apiurl, self.api.project, pkg))) - # Update package meta to disable build - self.api.create_package_container(self.api.project, pkg, meta=meta, disable_build=True) - wipebinaries(self.api.apiurl, self.api.project, package=pkg, repo=self.api.cmain_repo) - - # Remove package from Rings - if self.api.ring_packages.get(pkg): - delete_package(self.api.apiurl, self.api.ring_packages.get(pkg), pkg, force=True, msg="Cleanup package in Rings") - def perform(self, project, force=False): """Accept the staging project for review and submit to Factory / openSUSE 13.2 ... @@ -132,24 +84,17 @@ class AcceptCommand(object): for req in meta['requests']: self.api.rm_from_prj(project, request_id=req['id'], msg='ready to accept') packages.append(req['package']) - msg = 'Accepting staging review for {}'.format(req['package']) oldspecs = self.api.get_filelist_for_package(pkgname=req['package'], project=self.api.project, extension='spec') - if 'type' in req and req['type'] == 'delete' and self.api.cdelreq_review: - msg += ' and started handling of virtual accept process' - print(msg) - # Virtually accept the delete request - self.virtually_accept_delete(req['id'], req['package']) - else: - print(msg) - change_request_state(self.api.apiurl, - str(req['id']), - 'accepted', - message='Accept to %s' % self.api.project, - force=force) - self.create_new_links(self.api.project, req['package'], oldspecs) + print('Accepting staging review for {}'.format(req['package'])) + change_request_state(self.api.apiurl, + str(req['id']), + 'accepted', + message='Accept to %s' % self.api.project, + force=force) + self.create_new_links(self.api.project, req['package'], oldspecs) self.api.accept_status_comment(project, packages) self.api.staging_deactivate(project) @@ -197,15 +142,6 @@ class AcceptCommand(object): def accept_other_new(self): changed = False - if self.api.cdelreq_review: - rqlist = self.find_virtually_accepted_requests(self.api.project) - for req in rqlist: - if self.virtual_accept_request_has_no_binary(self.api.project, req['packages'][0]): - # Accepting delreq-review review - self.api.do_change_review_state(req['id'], 'accepted', - by_group=self.api.cdelreq_review, - message='Virtually accepted delete {}'.format(req['packages'][0])) - rqlist = self.find_new_requests(self.api.project) if self.api.cnonfree: rqlist += self.find_new_requests(self.api.cnonfree) From 58bc706bf0dde3eacf2cd9b1967042baed384648 Mon Sep 17 00:00:00 2001 From: Dominique Leuenberger Date: Fri, 18 Oct 2019 10:37:46 +0200 Subject: [PATCH 2/2] conf.py: remove delreq-review default setting With the removal of the complex delete request handling, we no longer need the option to assign a specific reviewer to delreqs in process --- osclib/conf.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/osclib/conf.py b/osclib/conf.py index 879989e3..d28d2c9a 100644 --- a/osclib/conf.py +++ b/osclib/conf.py @@ -31,7 +31,6 @@ DEFAULT = { 'openqa': 'https://openqa.opensuse.org', 'lock': 'openSUSE:%(project)s:Staging', 'lock-ns': 'openSUSE', - 'delreq-review': 'factory-maintainers', 'main-repo': 'standard', 'pseudometa_package': 'openSUSE:%(project)s:Staging/dashboard', 'download-baseurl': 'http://download.opensuse.org/tumbleweed/', @@ -73,7 +72,6 @@ DEFAULT = { 'lock': 'openSUSE:%(project)s:Staging', 'lock-ns': 'openSUSE', 'leaper-override-group': 'leap-reviewers', - 'delreq-review': '', 'main-repo': 'standard', 'pseudometa_package': 'openSUSE:%(project)s:Staging/dashboard', 'download-baseurl': 'http://download.opensuse.org/distribution/leap/%(version)s/', @@ -167,7 +165,6 @@ DEFAULT = { 'openqa': '', 'lock': '', 'lock-ns': '', - 'delreq-review': '', '_priority': '0', # Apply defaults first }, }