Skip to content

Commit

Permalink
Gating: assume 'waiting' for two hours on missing results (#4219)
Browse files Browse the repository at this point in the history 
This is part of addressing #4219. Currently we always count the
gating status as failed if requirements are unsatisfied. However,
it's usual for test results to not be present yet for a little
while after the update is created or edited. Ideally we would
have a mechanism for knowing for sure whether the tests in
question are pending, but we don't, so let's use a dumb rule
instead: if there are no failures (only missing results), and
it's less than two hours since the update was last modified, we'll
return 'waiting' as the status. After two hours, maybe the tests
actually didn't get scheduled or the policy is wrong, so we'll
return 'failed'.

As part of this we change the mock greenwave return value used
in a lot of other tests to use test-result-failed not -missing,
because they currently use -missing but expect the gating status
to be 'failed', which it no longer always will be.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
  • Loading branch information
@AdamWill
AdamWill committed May 19, 2021
1 parent 82168da commit e90270c
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 27 deletions.
9 changes: 8 additions & 1 deletion bodhi/server/models.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

from collections import defaultdict
from copy import copy
from datetime import datetime
from datetime import datetime, timedelta
from textwrap import wrap
import hashlib
import json
Expand Down Expand Up @@ -2234,6 +2234,13 @@ def _get_test_gating_status(self):
for data in self.greenwave_request_batches(verbose=False):
response = util.greenwave_api_post(self._greenwave_api_url, data)
if not response['policies_satisfied']:
# if there are no failures (only missing results) and
# it's less than two hours since the update was modified,
# let's assume tests are in progress
unsats = response.get('unsatisfied_requirements', [])
if unsats and all(req.get('type', '') == 'test-result-missing' for req in unsats):
if datetime.utcnow() - self.last_modified < timedelta(hours=2):
return TestGatingStatus.waiting
return TestGatingStatus.failed

if status != TestGatingStatus.ignored or response['summary'] != 'no tests are required':
Expand Down
18 changes: 9 additions & 9 deletions bodhi/tests/server/services/test_updates.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5843,7 +5843,7 @@ def test_waive_test_results_1_unsatisfied_requirement(
},
'scenario': None,
'testcase': 'dist.rpmdeplint',
'type': 'test-result-missing'
'type': 'test-result-failed'
}
],
}
Expand Down Expand Up @@ -5912,7 +5912,7 @@ def test_waive_test_results_2_unsatisfied_requirements(
},
'scenario': None,
'testcase': 'dist.rpmdeplint',
'type': 'test-result-missing'
'type': 'test-result-failed'
},
{
'item': {
Expand All @@ -5921,7 +5921,7 @@ def test_waive_test_results_2_unsatisfied_requirements(
},
'scenario': None,
'testcase': 'atomic_ci_pipeline_results',
'type': 'test-result-missing'
'type': 'test-result-failed'
}
],
}
Expand Down Expand Up @@ -6006,7 +6006,7 @@ def test_waive_test_results_1_of_2_unsatisfied_requirements(
},
'scenario': None,
'testcase': 'dist.rpmdeplint',
'type': 'test-result-missing'
'type': 'test-result-failed'
},
{
'item': {
Expand All @@ -6015,7 +6015,7 @@ def test_waive_test_results_1_of_2_unsatisfied_requirements(
},
'scenario': None,
'testcase': 'atomic_ci_pipeline_results',
'type': 'test-result-missing'
'type': 'test-result-failed'
}
],
}
Expand Down Expand Up @@ -6088,7 +6088,7 @@ def test_waive_test_results_2_of_2_unsatisfied_requirements(
},
'scenario': None,
'testcase': 'dist.rpmdeplint',
'type': 'test-result-missing'
'type': 'test-result-failed'
},
{
'item': {
Expand All @@ -6097,7 +6097,7 @@ def test_waive_test_results_2_of_2_unsatisfied_requirements(
},
'scenario': None,
'testcase': 'atomic_ci_pipeline_results',
'type': 'test-result-missing'
'type': 'test-result-failed'
}
],
}
Expand Down Expand Up @@ -6186,7 +6186,7 @@ def test_waive_test_results_unfailing_tests(
},
'scenario': None,
'testcase': 'dist.rpmdeplint',
'type': 'test-result-missing'
'type': 'test-result-failed'
},
{
'item': {
Expand All @@ -6195,7 +6195,7 @@ def test_waive_test_results_unfailing_tests(
},
'scenario': None,
'testcase': 'atomic_ci_pipeline_results',
'type': 'test-result-missing'
'type': 'test-result-failed'
}
],
}
Expand Down
95 changes: 92 additions & 3 deletions bodhi/tests/server/tasks/test_check_policies.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,17 +110,64 @@ def test_policies_pending_satisfied(self):
expected_query)

@patch.dict(config, [('greenwave_api_url', 'http://domain.local')])
def test_policies_unsatisfied(self):
"""Assert correct behavior when the policies enforced by Greenwave are unsatisfied"""
def test_policies_unsatisfied_waiting(self):
"""Assert correct behavior when the policies enforced by Greenwave are unsatisfied:
results missing, no failures, less than two hours since update creation results
in 'waiting' status.
"""
update = self.db.query(models.Update).all()[0]
update.status = models.UpdateStatus.testing
# Clear pending messages
self.db.info['messages'] = []
update.date_submitted = datetime.datetime.utcnow()
self.db.commit()
with patch('bodhi.server.models.util.greenwave_api_post') as mock_greenwave:
greenwave_response = {
'policies_satisfied': False,
'summary': '2 of 2 required test results missing',
'applicable_policies': ['taskotron_release_critical_tasks'],
'unsatisfied_requirements': [
{'testcase': 'dist.rpmdeplint',
'item': {'item': 'glibc-1.0-1.f26', 'type': 'koji_build'},
'type': 'test-result-missing', 'scenario': None},
{'testcase': 'dist.rpmdeplint',
'item': {'item': update.alias, 'type': 'bodhi_update'},
'type': 'test-result-missing', 'scenario': None}]}
mock_greenwave.return_value = greenwave_response
check_policies_main()
update = self.db.query(models.Update).filter(models.Update.id == update.id).one()
assert update.test_gating_status == models.TestGatingStatus.waiting
# Check for the comment
expected_comment = "This update's test gating status has been changed to 'waiting'."
assert update.comments[-1].text == expected_comment

expected_query = {
'product_version': 'fedora-17', 'decision_context': 'bodhi_update_push_stable',
'subject': [
{'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
{'item': 'FEDORA-{}-a3bbe1a8f2'.format(datetime.datetime.utcnow().year),
'type': 'bodhi_update'}],
'verbose': False
}
mock_greenwave.assert_called_once_with(config['greenwave_api_url'] + '/decision',
expected_query)

@patch.dict(config, [('greenwave_api_url', 'http://domain.local')])
def test_policies_unsatisfied_waiting_too_long(self):
"""Assert correct behavior when the policies enforced by Greenwave are unsatisfied:
results missing, no failures, more than two hours since update modification results
in 'failed' status.
"""
update = self.db.query(models.Update).all()[0]
update.status = models.UpdateStatus.testing
# Clear pending messages
self.db.info['messages'] = []
update.date_modifed = datetime.datetime.utcnow() - datetime.timedelta(days=1)
self.db.commit()
with patch('bodhi.server.models.util.greenwave_api_post') as mock_greenwave:
greenwave_response = {
'policies_satisfied': False,
'summary': '1 of 2 tests are failed',
'summary': '2 of 2 required test results missing',
'applicable_policies': ['taskotron_release_critical_tasks'],
'unsatisfied_requirements': [
{'testcase': 'dist.rpmdeplint',
Expand Down Expand Up @@ -148,6 +195,48 @@ def test_policies_unsatisfied(self):
mock_greenwave.assert_called_once_with(config['greenwave_api_url'] + '/decision',
expected_query)

@patch.dict(config, [('greenwave_api_url', 'http://domain.local')])
def test_policies_unsatisfied_failed(self):
"""Assert correct behavior when the policies enforced by Greenwave are unsatisfied:
failed tests always means failed status.
"""
update = self.db.query(models.Update).all()[0]
update.status = models.UpdateStatus.testing
update.date_modifed = datetime.datetime.utcnow()
# Clear pending messages
self.db.info['messages'] = []
self.db.commit()
with patch('bodhi.server.models.util.greenwave_api_post') as mock_greenwave:
greenwave_response = {
'policies_satisfied': False,
'summary': '1 of 2 required tests failed, 1 of 2 required test results missing',
'applicable_policies': ['taskotron_release_critical_tasks'],
'unsatisfied_requirements': [
{'testcase': 'dist.rpmdeplint',
'item': {'item': 'glibc-1.0-1.f26', 'type': 'koji_build'},
'type': 'test-result-failed', 'scenario': None},
{'testcase': 'dist.rpmdeplint',
'item': {'item': update.alias, 'type': 'bodhi_update'},
'type': 'test-result-missing', 'scenario': None}]}
mock_greenwave.return_value = greenwave_response
check_policies_main()
update = self.db.query(models.Update).filter(models.Update.id == update.id).one()
assert update.test_gating_status == models.TestGatingStatus.failed
# Check for the comment
expected_comment = "This update's test gating status has been changed to 'failed'."
assert update.comments[-1].text == expected_comment

expected_query = {
'product_version': 'fedora-17', 'decision_context': 'bodhi_update_push_stable',
'subject': [
{'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
{'item': 'FEDORA-{}-a3bbe1a8f2'.format(datetime.datetime.utcnow().year),
'type': 'bodhi_update'}],
'verbose': False
}
mock_greenwave.assert_called_once_with(config['greenwave_api_url'] + '/decision',
expected_query)

@patch.dict(config, [('greenwave_api_url', 'http://domain.local')])
def test_no_policies_enforced(self):
"""
Expand Down
28 changes: 14 additions & 14 deletions bodhi/tests/server/test_models.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1898,10 +1898,10 @@ def test_gating_required_false(self):
'unsatisfied_requirements': [
{'testcase': 'dist.rpmdeplint',
'item': {'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
'type': 'test-result-missing', 'scenario': None},
'type': 'test-result-failed', 'scenario': None},
{'testcase': 'dist.rpmdeplint',
'item': {'item': update.alias, 'type': 'bodhi_update'},
'type': 'test-result-missing', 'scenario': None}]}
'type': 'test-result-failed', 'scenario': None}]}
mock_greenwave.return_value = greenwave_response
model.Update.edit(request, data)

Expand All @@ -1928,10 +1928,10 @@ def test_gating_required_true(self):
'unsatisfied_requirements': [
{'testcase': 'dist.rpmdeplint',
'item': {'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
'type': 'test-result-missing', 'scenario': None},
'type': 'test-result-failed', 'scenario': None},
{'testcase': 'dist.rpmdeplint',
'item': {'item': update.alias, 'type': 'bodhi_update'},
'type': 'test-result-missing', 'scenario': None}]}
'type': 'test-result-failed', 'scenario': None}]}
mock_greenwave.return_value = greenwave_response
model.Update.edit(request, data)

Expand Down Expand Up @@ -1964,10 +1964,10 @@ def test_rawhide_update_edit_move_to_testing(self):
'unsatisfied_requirements': [
{'testcase': 'dist.rpmdeplint',
'item': {'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
'type': 'test-result-missing', 'scenario': None},
'type': 'test-result-failed', 'scenario': None},
{'testcase': 'dist.rpmdeplint',
'item': {'item': update.alias, 'type': 'bodhi_update'},
'type': 'test-result-missing', 'scenario': None}]}
'type': 'test-result-failed', 'scenario': None}]}
mock_greenwave.return_value = greenwave_response
model.Update.edit(request, data)

Expand Down Expand Up @@ -2000,10 +2000,10 @@ def test_rawhide_update_edit_stays_pending(self):
'unsatisfied_requirements': [
{'testcase': 'dist.rpmdeplint',
'item': {'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
'type': 'test-result-missing', 'scenario': None},
'type': 'test-result-failed', 'scenario': None},
{'testcase': 'dist.rpmdeplint',
'item': {'item': update.alias, 'type': 'bodhi_update'},
'type': 'test-result-missing', 'scenario': None}]}
'type': 'test-result-failed', 'scenario': None}]}
mock_greenwave.return_value = greenwave_response
model.Update.edit(request, data)

Expand Down Expand Up @@ -2036,10 +2036,10 @@ def test_not_rawhide_update_signed_stays_pending(self):
'unsatisfied_requirements': [
{'testcase': 'dist.rpmdeplint',
'item': {'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
'type': 'test-result-missing', 'scenario': None},
'type': 'test-result-failed', 'scenario': None},
{'testcase': 'dist.rpmdeplint',
'item': {'item': update.alias, 'type': 'bodhi_update'},
'type': 'test-result-missing', 'scenario': None}]}
'type': 'test-result-failed', 'scenario': None}]}
mock_greenwave.return_value = greenwave_response
model.Update.edit(request, data)

Expand Down Expand Up @@ -3693,10 +3693,10 @@ def test_set_request_pending_testing_gating_false(self):
'unsatisfied_requirements': [
{'testcase': 'dist.rpmdeplint',
'item': {'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
'type': 'test-result-missing', 'scenario': None},
'type': 'test-result-failed', 'scenario': None},
{'testcase': 'dist.rpmdeplint',
'item': {'item': self.obj.alias, 'type': 'bodhi_update'},
'type': 'test-result-missing', 'scenario': None}]}
'type': 'test-result-failed', 'scenario': None}]}
mock_greenwave.return_value = greenwave_response
with mock_sends(Message):
self.obj.set_request(self.db, UpdateRequest.testing, req.user.name)
Expand All @@ -3722,10 +3722,10 @@ def test_set_request_pending_testing_gating_true(self):
'unsatisfied_requirements': [
{'testcase': 'dist.rpmdeplint',
'item': {'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
'type': 'test-result-missing', 'scenario': None},
'type': 'test-result-failed', 'scenario': None},
{'testcase': 'dist.rpmdeplint',
'item': {'item': self.obj.alias, 'type': 'bodhi_update'},
'type': 'test-result-missing', 'scenario': None}]}
'type': 'test-result-failed', 'scenario': None}]}
mock_greenwave.return_value = greenwave_response
with mock_sends(Message):
self.obj.set_request(self.db, UpdateRequest.testing, req.user.name)
Expand Down

0 comments on commit e90270c

Please sign in to comment.