Skip to content

Commit

Permalink
Query all greenwave decision contexts together
Browse files Browse the repository at this point in the history 
Since release-engineering/greenwave#95 ,
greenwave allows clients to query on multiple decision contexts
at once. All relevant policies for all specified decision
contexts are then applied to the decision. This means we no
longer need to iterate over all relevant decision contexts and
send a separate decision query for each one; we can just send
one query (per `greenwave_batch_size` subjects, still) with all
the decision contexts in it. This makes the code simpler and will
make Bodhi more efficient.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
  • Loading branch information
@AdamWill
AdamWill committed Nov 4, 2022
1 parent 37c57e1 commit b470aa7
Show file tree
Hide file tree
Showing 6 changed files with 309 additions and 396 deletions.
13 changes: 6 additions & 7 deletions bodhi-server/bodhi/server/models.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2242,13 +2242,12 @@ def greenwave_request_batches(self, verbose):
subjects = self.greenwave_subject
data = []
while count < len(subjects):
for context in self._greenwave_decision_contexts:
data.append({
'product_version': self.product_version,
'decision_context': context,
'subject': subjects[count:count + batch_size],
'verbose': verbose,
})
data.append({
'product_version': self.product_version,
'decision_context': self._greenwave_decision_contexts,
'subject': subjects[count:count + batch_size],
'verbose': verbose,
})
count += batch_size
return data

Expand Down
7 changes: 1 addition & 6 deletions bodhi-server/bodhi/server/templates/update.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -863,12 +863,7 @@ <h3 class="modal-title" id="waiveModalLabel">Trigger Tests</h3>
handle_unsatisfied_requirements(data);
reqs_counter += data['unsatisfied_requirements'].length;
reqs_counter += data['satisfied_requirements'].length;
// we don't want to generate result rows for both the
// critpath and non-critpath decision contexts, as both
// queries return the same set of test results
if (!(request.decision_context.endsWith("critpath"))) {
handle_results(data.results, request.subject);
}
handle_results(data.results, request.subject);
after_success();
},
error: function (jqxhr, status, error) {
Expand Down
111 changes: 55 additions & 56 deletions bodhi-server/tests/services/test_updates.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1407,8 +1407,8 @@ def test_decision_context_pending_stable(self):

res = self.app.get(f'/updates/{u.alias}', status=200, headers={'Accept': 'text/html'})

assert '"decision_context": "bodhi_update_push_testing",' not in res
assert '"decision_context": "bodhi_update_push_stable",' in res
assert '"decision_context": ["bodhi_update_push_testing"],' not in res
assert '"decision_context": ["bodhi_update_push_stable"],' in res

def test_decision_context_pending_testing(self):
"""The HTML should include the correct decision context for Pending/Testing updates."""
Expand All @@ -1418,8 +1418,8 @@ def test_decision_context_pending_testing(self):

res = self.app.get(f'/updates/{u.alias}', status=200, headers={'Accept': 'text/html'})

assert '"decision_context": "bodhi_update_push_stable",' not in res
assert '"decision_context": "bodhi_update_push_testing",' in res
assert '"decision_context": ["bodhi_update_push_stable"],' not in res
assert '"decision_context": ["bodhi_update_push_testing"],' in res

def test_decision_context_testing(self):
"""The HTML should include the correct decision context for Testing updates."""
Expand All @@ -1431,8 +1431,8 @@ def test_decision_context_testing(self):

res = self.app.get(f'/updates/{u.alias}', status=200, headers={'Accept': 'text/html'})

assert '"decision_context": "bodhi_update_push_testing",' not in res
assert '"decision_context": "bodhi_update_push_stable",' in res
assert '"decision_context": ["bodhi_update_push_testing"],' not in res
assert '"decision_context": ["bodhi_update_push_stable"],' in res

def test_home_html_legal(self):
"""Test the home page HTML when a legal link is configured."""
Expand Down Expand Up @@ -6190,7 +6190,7 @@ def test_waive_test_results_1_unsatisfied_requirement(
'https://greenwave-web-greenwave.app.os.fedoraproject.org/api/v1.0/decision',
{
'product_version': 'fedora-17',
'decision_context': 'bodhi_update_push_testing',
'decision_context': ['bodhi_update_push_testing'],
'subject': [
{'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
{'item': up.alias, 'type': 'bodhi_update'}
Expand Down Expand Up @@ -6269,7 +6269,7 @@ def test_waive_test_results_2_unsatisfied_requirements(
'https://greenwave-web-greenwave.app.os.fedoraproject.org/api/v1.0/decision',
{
'product_version': 'fedora-17',
'decision_context': 'bodhi_update_push_testing',
'decision_context': ['bodhi_update_push_testing'],
'subject': [
{'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
{'item': up.alias, 'type': 'bodhi_update'}
Expand Down Expand Up @@ -6369,7 +6369,7 @@ def test_waive_test_results_1_of_2_unsatisfied_requirements(
'https://greenwave-web-greenwave.app.os.fedoraproject.org/api/v1.0/decision',
{
'product_version': 'fedora-17',
'decision_context': 'bodhi_update_push_testing',
'decision_context': ['bodhi_update_push_testing'],
'subject': [
{'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
{'item': up.alias, 'type': 'bodhi_update'}
Expand Down Expand Up @@ -6452,7 +6452,7 @@ def test_waive_test_results_2_of_2_unsatisfied_requirements(
'https://greenwave-web-greenwave.app.os.fedoraproject.org/api/v1.0/decision',
{
'product_version': 'fedora-17',
'decision_context': 'bodhi_update_push_testing',
'decision_context': ['bodhi_update_push_testing'],
'subject': [
{'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
{'item': up.alias, 'type': 'bodhi_update'}
Expand Down Expand Up @@ -6552,7 +6552,7 @@ def test_waive_test_results_unfailing_tests(
'https://greenwave-web-greenwave.app.os.fedoraproject.org/api/v1.0/decision',
{
'product_version': 'fedora-17',
'decision_context': 'bodhi_update_push_testing',
'decision_context': ['bodhi_update_push_testing'],
'subject': [
{'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
{'item': up.alias, 'type': 'bodhi_update'}
Expand Down Expand Up @@ -6696,7 +6696,7 @@ def test_get_test_results_erroring_on_greenwave(self, call_api, *args):
'https://greenwave.api/decision',
data={
'product_version': 'fedora-17',
'decision_context': 'bodhi_update_push_testing',
'decision_context': ['bodhi_update_push_testing'],
'subject': [
{'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
{'item': update.alias, 'type': 'bodhi_update'}
Expand Down Expand Up @@ -6736,7 +6736,7 @@ def test_get_test_results_timing_out_on_greenwave(self, call_api, *args):
'https://greenwave.api/decision',
data={
'product_version': 'fedora-17',
'decision_context': 'bodhi_update_push_testing',
'decision_context': ['bodhi_update_push_testing'],
'subject': [
{'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
{'item': update.alias, 'type': 'bodhi_update'}
Expand Down Expand Up @@ -6778,7 +6778,7 @@ def test_get_test_results_calling_greenwave(self, call_api, *args):
'https://greenwave.api/decision',
data={
'product_version': 'fedora-17',
'decision_context': 'bodhi_update_push_testing',
'decision_context': ['bodhi_update_push_testing'],
'subject': [
{'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
{'item': update.alias, 'type': 'bodhi_update'}
Expand Down Expand Up @@ -6806,25 +6806,26 @@ def test_get_test_results_calling_greenwave_critpath(self, call_api, *args):

res = self.app.get(f'/updates/{update.alias}/get-test-results')

assert call_api.call_args_list == [
mock.call(
'https://greenwave.api/decision',
data={
'product_version': 'fedora-17',
'decision_context': context,
'subject': [
{'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
{'item': update.alias, 'type': 'bodhi_update'}
],
'verbose': True,
},
method='POST',
retries=3,
service_name='Greenwave'
) for context in ('bodhi_update_push_testing_critpath', 'bodhi_update_push_testing')
]
call_api.assert_called_once_with(
'https://greenwave.api/decision',
data={
'product_version': 'fedora-17',
'decision_context': [
'bodhi_update_push_testing_critpath',
'bodhi_update_push_testing'
],
'subject': [
{'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
{'item': update.alias, 'type': 'bodhi_update'}
],
'verbose': True,
},
method='POST',
retries=3,
service_name='Greenwave'
)

assert res.json_body == {'decisions': [{'foo': 'bar'}, {'foo': 'bar'}]}
assert res.json_body == {'decisions': [{'foo': 'bar'}]}

@mock.patch.dict(config, [('greenwave_api_url', 'https://greenwave.api')])
@mock.patch('bodhi.server.util.call_api')
Expand All @@ -6841,29 +6842,27 @@ def test_get_test_results_calling_greenwave_critpath_groups(self, call_api, *arg

res = self.app.get(f'/updates/{update.alias}/get-test-results')

assert call_api.call_args_list == [
mock.call(
'https://greenwave.api/decision',
data={
'product_version': 'fedora-17',
'decision_context': context,
'subject': [
{'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
{'item': update.alias, 'type': 'bodhi_update'}
],
'verbose': True,
},
method='POST',
retries=3,
service_name='Greenwave'
) for context in (
'bodhi_update_push_testing_critical-path-apps_critpath',
'bodhi_update_push_testing_core_critpath',
'bodhi_update_push_testing',
)
]
call_api.assert_called_once_with(
'https://greenwave.api/decision',
data={
'product_version': 'fedora-17',
'decision_context': [
'bodhi_update_push_testing_critical-path-apps_critpath',
'bodhi_update_push_testing_core_critpath',
'bodhi_update_push_testing'
],
'subject': [
{'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
{'item': update.alias, 'type': 'bodhi_update'}
],
'verbose': True,
},
method='POST',
retries=3,
service_name='Greenwave'
)

assert res.json_body == {'decisions': [{'foo': 'bar'}, {'foo': 'bar'}, {'foo': 'bar'}]}
assert res.json_body == {'decisions': [{'foo': 'bar'}]}

@mock.patch.dict(config, [('greenwave_api_url', 'https://greenwave.api')])
@mock.patch('bodhi.server.util.call_api')
Expand All @@ -6884,7 +6883,7 @@ def test_get_test_results_calling_greenwave_critpath_groups_empty(self, call_api
'https://greenwave.api/decision',
data={
'product_version': 'fedora-17',
'decision_context': 'bodhi_update_push_testing',
'decision_context': ['bodhi_update_push_testing'],
'subject': [
{'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
{'item': update.alias, 'type': 'bodhi_update'}
Expand Down Expand Up @@ -6917,7 +6916,7 @@ def test_get_test_results_calling_greenwave_no_session(self, call_api, *args):
'https://greenwave.api/decision',
data={
'product_version': 'fedora-17',
'decision_context': 'bodhi_update_push_testing',
'decision_context': ['bodhi_update_push_testing'],
'subject': [
{'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
{'item': update.alias, 'type': 'bodhi_update'}
Expand Down
Loading

0 comments on commit b470aa7

Please sign in to comment.