From 571fff925345ca485062b4db5f3a54807fbe892c Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Dec 21 2017 14:35:30 +0000 Subject: [PATCH 1/2] Waive the absence of a result Changed Greenwave for submition of waiver in Waiverdb, not anymore with the result_id, but with subject and testcase --- diff --git a/functional-tests/conftest.py b/functional-tests/conftest.py index f3be25a..1e788ff 100644 --- a/functional-tests/conftest.py +++ b/functional-tests/conftest.py @@ -141,9 +141,10 @@ class TestDataBuilder(object): response.raise_for_status() return response.json() - def create_waiver(self, result_id, product_version, waived=True): + def create_waiver(self, result, product_version, waived=True): data = { - 'result_id': result_id, + 'result_subject': result['subject'], + 'result_testcase': result['testcase'], 'product_version': product_version, 'waived': waived, } diff --git a/functional-tests/consumers/test_waiverdb.py b/functional-tests/consumers/test_waiverdb.py index c1e4ce2..761c13c 100644 --- a/functional-tests/consumers/test_waiverdb.py +++ b/functional-tests/consumers/test_waiverdb.py @@ -28,7 +28,10 @@ def test_consume_new_waiver( testdatabuilder.create_result(item=nvr, testcase_name=testcase_name, outcome='PASSED') - waiver = testdatabuilder.create_waiver(result_id=result['id'], product_version='fedora-26') + testcase = str(result['testcase']['name']) + waiver = testdatabuilder.create_waiver(result={ + "subject": dict([(str(key), str(value[0])) for key, value in result['data'].items()]), + "testcase": testcase}, product_version='fedora-26') message = { 'body': { 'topic': 'waiver.new', @@ -39,7 +42,8 @@ def test_consume_new_waiver( "waived": "true", "timestamp": "2017-08-10T17:42:04.209638", "product_version": "fedora-26", - "result_id": result['id'], + "result_testcase": waiver['result_testcase'], + "result_subject": [waiver['result_subject']] } } } @@ -64,10 +68,11 @@ def test_consume_new_waiver( assert old_decision['summary'] == '1 of 3 required tests failed' msg = { + 'applicable_policies': ['taskotron_release_critical_tasks_with_blacklist', + 'taskotron_release_critical_tasks'], 'policies_satisfied': True, 'decision_context': 'bodhi_update_push_stable', - 'unsatisfied_requirements': [], - 'summary': 'all required tests passed', + 'previous': old_decision, 'product_version': 'fedora-26', 'subject': [ { @@ -75,9 +80,9 @@ def test_consume_new_waiver( 'type': 'koji_build' } ], - 'applicable_policies': ['taskotron_release_critical_tasks_with_blacklist', - 'taskotron_release_critical_tasks'], - 'previous': old_decision, + 'unsatisfied_requirements': [], + 'summary': 'all required tests passed', + 'result_testcase': testcase, } mock_fedmsg.assert_called_once_with( topic='decision.update', msg=msg) diff --git a/functional-tests/test_api_v1.py b/functional-tests/test_api_v1.py index a62be29..dfd539b 100644 --- a/functional-tests/test_api_v1.py +++ b/functional-tests/test_api_v1.py @@ -83,9 +83,6 @@ TASKTRON_RELEASE_CRITICAL_TASKS = [ 'dist.upgradepath', ] -OPENQA_TASKS = [ - 'compose.install_no_user', -] OPENQA_SCENARIOS = [ 'scenario1', 'scenario2', @@ -193,6 +190,7 @@ def test_make_a_decison_on_passed_result(requests_session, greenwave_server, tes 'product_version': 'rhel-7', 'subject': [{'item': nvr, 'type': 'koji_build'}] } + r = requests_session.post(greenwave_server.url + 'api/v1.0/decision', headers={'Content-Type': 'application/json'}, data=json.dumps(data)) @@ -211,7 +209,9 @@ def test_make_a_decison_on_failed_result_with_waiver( result = testdatabuilder.create_result(item=nvr, testcase_name=all_rpmdiff_testcase_names[0], outcome='FAILED') - testdatabuilder.create_waiver(result_id=result['id'], product_version='rhel-7') + waiver = testdatabuilder.create_waiver(result={ # noqa + "subject": dict([(key, value[0]) for key, value in result['data'].items()]), + "testcase": all_rpmdiff_testcase_names[0]}, product_version='rhel-7') # The rest passed for testcase_name in all_rpmdiff_testcase_names[1:]: testdatabuilder.create_result(item=nvr, @@ -440,12 +440,13 @@ def test_make_a_decison_on_passed_result_with_scenario( If we require two scenarios to pass, and both pass, then we pass. """ compose_id = testdatabuilder.unique_compose_id() - for testcase_name in OPENQA_TASKS: - for scenario in OPENQA_SCENARIOS: - testdatabuilder.create_result(item=compose_id, - testcase_name=testcase_name, - scenario=scenario, - outcome='PASSED') + testcase_name = 'compose.install_no_user' + for scenario in OPENQA_SCENARIOS: + testdatabuilder.create_result( + item=compose_id, + testcase_name=testcase_name, + scenario=scenario, + outcome='PASSED') data = { 'decision_context': 'rawhide_compose_sync_to_mirrors', 'product_version': 'fedora-rawhide', @@ -469,17 +470,19 @@ def test_make_a_decison_on_failing_result_with_scenario( """ compose_id = testdatabuilder.unique_compose_id() - for testcase_name in OPENQA_TASKS: - # Scenario 1 passes.. - testdatabuilder.create_result(item=compose_id, - testcase_name=testcase_name, - scenario='scenario1', - outcome='PASSED') - # But scenario 2 fails! - testdatabuilder.create_result(item=compose_id, - testcase_name=testcase_name, - scenario='scenario2', - outcome='FAILED') + testcase_name = 'compose.install_no_user' + # Scenario 1 passes.. + testdatabuilder.create_result( + item=compose_id, + testcase_name=testcase_name, + scenario='scenario1', + outcome='PASSED') + # But scenario 2 fails! + result = testdatabuilder.create_result( + item=compose_id, + testcase_name=testcase_name, + scenario='scenario2', + outcome='FAILED') data = { 'decision_context': 'rawhide_compose_sync_to_mirrors', 'product_version': 'fedora-rawhide', @@ -494,6 +497,14 @@ def test_make_a_decison_on_failing_result_with_scenario( assert res_data['applicable_policies'] == ['openqa_important_stuff_for_rawhide'] expected_summary = '1 of 2 required tests failed' assert res_data['summary'] == expected_summary + expected_unsatisfied_requirements = [{ + u'item': {u'item': compose_id}, + u'result_id': result['id'], + u'testcase': testcase_name, + u'type': u'test-result-failed', + u'scenario': u'scenario2', + }] + assert res_data['unsatisfied_requirements'] == expected_unsatisfied_requirements def test_ignore_waiver(requests_session, greenwave_server, testdatabuilder): @@ -504,7 +515,9 @@ def test_ignore_waiver(requests_session, greenwave_server, testdatabuilder): result = testdatabuilder.create_result(item=nvr, testcase_name=all_rpmdiff_testcase_names[0], outcome='FAILED') - waiver = testdatabuilder.create_waiver(result_id=result['id'], product_version='rhel-7') + waiver = testdatabuilder.create_waiver(result={ + "subject": dict([(key, value[0]) for key, value in result['data'].items()]), + "testcase": all_rpmdiff_testcase_names[0]}, product_version='rhel-7') # The rest passed for testcase_name in all_rpmdiff_testcase_names[1:]: testdatabuilder.create_result(item=nvr, @@ -515,21 +528,21 @@ def test_ignore_waiver(requests_session, greenwave_server, testdatabuilder): 'product_version': 'rhel-7', 'subject': [{'item': nvr, 'type': 'koji_build'}] } - r = requests_session.post(greenwave_server.url + 'api/v1.0/decision', - headers={'Content-Type': 'application/json'}, - data=json.dumps(data)) - assert r.status_code == 200 - res_data = r.json() + r_ = requests_session.post(greenwave_server.url + 'api/v1.0/decision', + headers={'Content-Type': 'application/json'}, + data=json.dumps(data)) + assert r_.status_code == 200 + res_data = r_.json() assert res_data['policies_satisfied'] is True # Ignore the waiver data.update({ 'ignore_waiver': [waiver['id']] }) - r = requests_session.post(greenwave_server.url + 'api/v1.0/decision', - headers={'Content-Type': 'application/json'}, - data=json.dumps(data)) - assert r.status_code == 200 - res_data = r.json() + r_ = requests_session.post(greenwave_server.url + 'api/v1.0/decision', + headers={'Content-Type': 'application/json'}, + data=json.dumps(data)) + assert r_.status_code == 200 + res_data = r_.json() expected_unsatisfied_requirements = [ { 'item': {'item': nvr, 'type': 'koji_build'}, diff --git a/greenwave/api_v1.py b/greenwave/api_v1.py index 155d819..556b6bb 100644 --- a/greenwave/api_v1.py +++ b/greenwave/api_v1.py @@ -199,13 +199,12 @@ def make_decision(): if not subjects: raise BadRequest('Invalid subject, must be a list of dicts') answers = [] + for item in subjects: results = retrieve_results(item) results = [r for r in results if r['id'] not in ignore_results] - if results: - waivers = retrieve_waivers(product_version, results) - else: - waivers = [] + + waivers = retrieve_waivers(product_version, item) waivers = [w for w in waivers if w['id'] not in ignore_waivers] for policy in applicable_policies: answers.extend(policy.check(item, results, waivers)) diff --git a/greenwave/config.py b/greenwave/config.py index 58e51ae..6027e47 100644 --- a/greenwave/config.py +++ b/greenwave/config.py @@ -18,6 +18,13 @@ class Config(object): # By default, don't cache anything. CACHE = {'backend': 'dogpile.cache.null'} + # These are keys used to construct announcements about decision changes. + ANNOUNCEMENT_SUBJECT_KEYS = [ + ('item', 'type',), + ('original_spec_nvr',), + ('productmd.compose.id',), + ] + class ProductionConfig(Config): DEBUG = False diff --git a/greenwave/consumers/waiverdb.py b/greenwave/consumers/waiverdb.py index a1a245f..e5d1868 100644 --- a/greenwave/consumers/waiverdb.py +++ b/greenwave/consumers/waiverdb.py @@ -61,27 +61,17 @@ class WaiverDBHandler(fedmsg.consumers.FedmsgConsumer): message = message.get('body', message) log.debug('Processing message "%s"', message) msg = message['msg'] - result_id = msg['result_id'] + product_version = msg['product_version'] config = load_config() - timeout = config['REQUESTS_TIMEOUT'] - # Get the waived result to figure out the item - response = requests_session.get( - config['RESULTSDB_API_URL'] + '/results/%d' % result_id, - timeout=timeout) - response.raise_for_status() - testcase = response.json()['testcase']['name'] - # In ResultsDB, 'data' is key -> list of strings. - # But in Greenwave, we only deal in key -> string. - # This is... iffy and might need cleaning up? - item = {k: v[0] for k, v in response.json()['data'].items()} + testcase = msg['result_testcase'] for policy in config['policies']: for rule in policy.rules: if rule.test_case_name == testcase: data = { 'decision_context': policy.decision_context, 'product_version': product_version, - 'subject': [item], + 'subject': msg['result_subject'] } response = requests_session.post( self.fedmsg_config['greenwave_api_url'] + '/decision', @@ -101,9 +91,12 @@ class WaiverDBHandler(fedmsg.consumers.FedmsgConsumer): old_decision = response.json() if decision != old_decision: + subject = [dict((str(k), str(v)) for k, v in item.items()) + for item in msg['result_subject']] msg = decision decision.update({ - 'subject': [item], + 'subject': subject, + 'result_testcase': testcase, 'decision_context': policy.decision_context, 'product_version': product_version, 'previous': old_decision, diff --git a/greenwave/policies.py b/greenwave/policies.py index f90ebb6..1d9a895 100644 --- a/greenwave/policies.py +++ b/greenwave/policies.py @@ -166,7 +166,10 @@ class PassingTestCaseRule(Rule): if matching_result['outcome'] in ['PASSED', 'INFO']: return RuleSatisfied() # XXX limit who is allowed to waive - if any(w['result_id'] == matching_result['id'] and w['waived'] for w in waivers): + if any(w['result_subject'] == dict([(key, value[0]) + for key, value in matching_result['data'].items()]) and + w['result_testcase'] == matching_result['testcase']['name'] and + w['waived'] for w in waivers): return RuleSatisfied() return TestResultFailed(item, self.test_case_name, self._scenario, matching_result['id']) diff --git a/greenwave/resources.py b/greenwave/resources.py index 5f9e21e..5ccab1d 100644 --- a/greenwave/resources.py +++ b/greenwave/resources.py @@ -30,14 +30,14 @@ def retrieve_results(item): # NOTE - not cached, for now. -def retrieve_waivers(product_version, results): +def retrieve_waivers(product_version, item): timeout = current_app.config['REQUESTS_TIMEOUT'] data = { 'product_version': product_version, - 'result_ids': [result['id'] for result in results], + 'results': [{"subject": item}] } response = requests_session.post( - current_app.config['WAIVERDB_API_URL'] + '/waivers/+by-result-ids', + current_app.config['WAIVERDB_API_URL'] + '/waivers/+by-subjects-and-testcases', headers={'Content-Type': 'application/json'}, data=json.dumps(data), timeout=timeout) From 254723399294903a8347c6fbdd358fe59c4d8e8a Mon Sep 17 00:00:00 2001 From: Giulia Naponiello Date: Dec 21 2017 17:37:26 +0000 Subject: [PATCH 2/2] minor fixes --- diff --git a/greenwave/config.py b/greenwave/config.py index 6027e47..58e51ae 100644 --- a/greenwave/config.py +++ b/greenwave/config.py @@ -18,13 +18,6 @@ class Config(object): # By default, don't cache anything. CACHE = {'backend': 'dogpile.cache.null'} - # These are keys used to construct announcements about decision changes. - ANNOUNCEMENT_SUBJECT_KEYS = [ - ('item', 'type',), - ('original_spec_nvr',), - ('productmd.compose.id',), - ] - class ProductionConfig(Config): DEBUG = False diff --git a/greenwave/resources.py b/greenwave/resources.py index 5ccab1d..550fde8 100644 --- a/greenwave/resources.py +++ b/greenwave/resources.py @@ -6,6 +6,7 @@ waiverdb, etc..). """ +import pdb import requests import json from flask import current_app @@ -34,7 +35,7 @@ def retrieve_waivers(product_version, item): timeout = current_app.config['REQUESTS_TIMEOUT'] data = { 'product_version': product_version, - 'results': [{"subject": item}] + 'results': [{'subject': item}] } response = requests_session.post( current_app.config['WAIVERDB_API_URL'] + '/waivers/+by-subjects-and-testcases', @@ -42,4 +43,5 @@ def retrieve_waivers(product_version, item): data=json.dumps(data), timeout=timeout) response.raise_for_status() + #pdb.set_trace() return response.json()['data']