Query greenwave by critical path group when possible

When possible - i.e. when the critpath.type implementation
provides grouped information, i.e. when it's "json" - this makes
Bodhi query greenwave for a different context for each critical
path group an update contains package(s) from.

The intent is to allow us to run and gate on different sets of
tests for different critical path groups. For instance, if an
update is only in the critical path for Server, it doesn't make
much sense to run and gate on GNOME/Workstation-related tests.
If an update is only in the critical path for LXDE, maybe we
don't need to run or gate on any tests at all (as we don't have
any for LXDE).

This should only be deployed in production after the Fedora
greenwave policy has been updated to contain a policy for each
critical path group defined in `critpath.py`. A risk of this
approach is that we will actually hit an exception if we wind
up querying for a context that has no policies. It may be
possible to use wildcards in the 'null' policy to defend against
this, I will check.

We update a lot of related tests to use this codepath rather
than the old one as we may, in future, want to drop the old
codepath and only support this one. But we keep one test on the
old codepath to make sure it works.

Signed-off-by: Adam Williamson <awilliam@redhat.com>

bodhi-server/bodhi/server/models.py

@@ -2277,16 +2277,27 @@ def _greenwave_api_url(self):
 
     @property
     def _greenwave_decision_contexts(self):
-        # We retrieve updates going to testing (status=pending) and updates
-        # (status=testing) going to stable.
-        # We also query on different contexts for critpath and non-critpath
-        # updates.
+        """
+        Return greenwave contexts that should be queried for this update.
+
+        The base context depends whether the update is going to
+        testing (status=pending) or to stable (status=testing). We
+        also query on additional contexts for critpath updates. If the
+        critpath type provides grouped information, we query on one
+        additional context per critpath group represented in the
+        update's packages. If it does not, we query on the catch-all
+        additional 'critpath' context.
+        """
         # this is correct if update is already in testing...
         contexts = ["bodhi_update_push_stable"]
         if self.request == UpdateRequest.testing and self.status == UpdateStatus.pending:
             # ...but if it is pending, we want to know if it can go to testing
             contexts = ["bodhi_update_push_testing"]
-        if self.critpath:
+        if self.critpath_groups:
+            ocontext = contexts[0]
+            for group in self.critpath_groups.split(" "):
+                contexts.insert(0, f"{ocontext}_{group}_critpath")
+        elif self.critpath:
             contexts.insert(0, contexts[0] + "_critpath")
         return contexts
 

bodhi-server/tests/services/test_updates.py

@@ -6764,9 +6764,12 @@ def test_get_test_results_timing_out_on_greenwave(self, call_api, *args):
     def test_get_test_results_calling_greenwave(self, call_api, *args):
         """
         Ensure if all conditions are met we do try to call greenwave with the proper
-        argument.
+        argument for a non-critical-path update, without critical path group
+        support.
         """
         update = Build.query.filter_by(nvr='bodhi-2.0-1.fc17').one().update
+        update.critpath = False
+        update.critpath_groups = None
         call_api.return_value = {"foo": "bar"}
 
         res = self.app.get(f'/updates/{update.alias}/get-test-results')
@@ -6798,6 +6801,7 @@ def test_get_test_results_calling_greenwave_critpath(self, call_api, *args):
         """
         update = Build.query.filter_by(nvr='bodhi-2.0-1.fc17').one().update
         update.critpath = True
+        update.critpath_groups = None
         call_api.return_value = {"foo": "bar"}
 
         res = self.app.get(f'/updates/{update.alias}/get-test-results')
@@ -6822,6 +6826,78 @@ def test_get_test_results_calling_greenwave_critpath(self, call_api, *args):
 
         assert res.json_body == {'decisions': [{'foo': 'bar'}, {'foo': 'bar'}]}
 
+    @mock.patch.dict(config, [('greenwave_api_url', 'https://greenwave.api')])
+    @mock.patch('bodhi.server.util.call_api')
+    def test_get_test_results_calling_greenwave_critpath_groups(self, call_api, *args):
+        """
+        Ensure if all conditions are met we do try to call greenwave with the proper
+        arguments for a critical path update when critical path group support
+        is present.
+        """
+        update = Build.query.filter_by(nvr='bodhi-2.0-1.fc17').one().update
+        update.critpath = True
+        update.critpath_groups = 'core critical-path-apps'
+        call_api.return_value = {"foo": "bar"}
+
+        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',
+            )
+        ]
+
+        assert res.json_body == {'decisions': [{'foo': 'bar'}, {'foo': 'bar'}, {'foo': 'bar'}]}
+
+    @mock.patch.dict(config, [('greenwave_api_url', 'https://greenwave.api')])
+    @mock.patch('bodhi.server.util.call_api')
+    def test_get_test_results_calling_greenwave_critpath_groups_empty(self, call_api, *args):
+        """
+        Ensure if all conditions are met we do try to call greenwave with the proper
+        arguments for a critical path update when critical path group support
+        is present, but the update is not in any groups.
+        """
+        update = Build.query.filter_by(nvr='bodhi-2.0-1.fc17').one().update
+        update.critpath = False
+        update.critpath_groups = ''
+        call_api.return_value = {"foo": "bar"}
+
+        res = self.app.get(f'/updates/{update.alias}/get-test-results')
+
+        call_api.assert_called_once_with(
+            'https://greenwave.api/decision',
+            data={
+                'product_version': 'fedora-17',
+                'decision_context': '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'}]}
+
     @mock.patch('bodhi.server.util.call_api')
     def test_get_test_results_calling_greenwave_no_session(self, call_api, *args):
         """

bodhi-server/tests/tasks/test_check_policies.py

@@ -53,7 +53,7 @@ def test_policies_satisfied(self):
         """Assert correct behavior when the policies enforced by Greenwave are satisfied"""
         update = self.db.query(models.Update).all()[0]
         update.status = models.UpdateStatus.testing
-        update.critpath = True
+        update.critpath_groups = "core"
         # Clear pending messages
         self.db.info['messages'] = []
         self.db.commit()
@@ -103,7 +103,7 @@ def test_policies_satisfied(self):
                     {'item': 'FEDORA-{}-a3bbe1a8f2'.format(datetime.datetime.utcnow().year),
                      'type': 'bodhi_update'}],
                 'verbose': False
-            } for context in ('bodhi_update_push_stable_critpath', 'bodhi_update_push_stable')
+            } for context in ('bodhi_update_push_stable_core_critpath', 'bodhi_update_push_stable')
         ]
         expected_calls = [
             call(config['greenwave_api_url'] + '/decision', query) for query in expected_queries
@@ -116,7 +116,7 @@ def test_policies_pending_satisfied(self):
         greenwave with the ``bodhi_update_push_testing`` decision context. """
         update = self.db.query(models.Update).all()[0]
         update.status = models.UpdateStatus.pending
-        update.critpath = True
+        update.critpath_groups = "core"
         self.db.commit()
         with patch('bodhi.server.models.util.greenwave_api_post') as mock_greenwave:
             greenwave_responses = [
@@ -165,7 +165,9 @@ def test_policies_pending_satisfied(self):
                     {'item': 'FEDORA-{}-a3bbe1a8f2'.format(datetime.datetime.utcnow().year),
                      'type': 'bodhi_update'}],
                 'verbose': False,
-            } for context in ('bodhi_update_push_testing_critpath', 'bodhi_update_push_testing')
+            } for context in (
+                'bodhi_update_push_testing_core_critpath', 'bodhi_update_push_testing'
+            )
         ]
         expected_calls = [
             call(config['greenwave_api_url'] + '/decision', query) for query in expected_queries
@@ -180,7 +182,7 @@ def test_policies_unsatisfied_waiting(self):
         """
         update = self.db.query(models.Update).all()[0]
         update.status = models.UpdateStatus.testing
-        update.critpath = True
+        update.critpath_groups = "core"
         # Clear pending messages
         self.db.info['messages'] = []
         update.date_submitted = datetime.datetime.utcnow()
@@ -249,7 +251,7 @@ def test_policies_unsatisfied_waiting(self):
                     {'item': 'FEDORA-{}-a3bbe1a8f2'.format(datetime.datetime.utcnow().year),
                      'type': 'bodhi_update'}],
                 'verbose': False
-            } for context in ('bodhi_update_push_stable_critpath', 'bodhi_update_push_stable')
+            } for context in ('bodhi_update_push_stable_core_critpath', 'bodhi_update_push_stable')
         ]
         expected_calls = [
             call(config['greenwave_api_url'] + '/decision', query) for query in expected_queries
@@ -264,7 +266,7 @@ def test_policies_unsatisfied_waiting_too_long(self):
         """
         update = self.db.query(models.Update).all()[0]
         update.status = models.UpdateStatus.testing
-        update.critpath = True
+        update.critpath_groups = "core"
         # Clear pending messages
         self.db.info['messages'] = []
         update.date_submitted = datetime.datetime.utcnow() - datetime.timedelta(days=1)
@@ -326,7 +328,8 @@ def test_policies_unsatisfied_waiting_too_long(self):
             assert update.comments[-1].text == expected_comment
 
         expected_query = {
-            'product_version': 'fedora-17', 'decision_context': 'bodhi_update_push_stable_critpath',
+            'product_version': 'fedora-17',
+            'decision_context': 'bodhi_update_push_stable_core_critpath',
             'subject': [
                 {'item': 'bodhi-2.0-1.fc17', 'type': 'koji_build'},
                 {'item': 'FEDORA-{}-a3bbe1a8f2'.format(datetime.datetime.utcnow().year),
@@ -350,7 +353,7 @@ def test_policies_unsatisfied_failed(self):
         """
         update = self.db.query(models.Update).all()[0]
         update.status = models.UpdateStatus.testing
-        update.critpath = True
+        update.critpath_groups = "core"
         update.date_submitted = datetime.datetime.utcnow()
         # Clear pending messages
         self.db.info['messages'] = []
@@ -421,7 +424,7 @@ def test_policies_unsatisfied_failed(self):
                     {'item': 'FEDORA-{}-a3bbe1a8f2'.format(datetime.datetime.utcnow().year),
                      'type': 'bodhi_update'}],
                 'verbose': False
-            } for context in ('bodhi_update_push_stable_critpath', 'bodhi_update_push_stable')
+            } for context in ('bodhi_update_push_stable_core_critpath', 'bodhi_update_push_stable')
         ]
         expected_calls = [
             call(config['greenwave_api_url'] + '/decision', query) for query in expected_queries
@@ -467,6 +470,9 @@ def test_pushed_update(self):
         update.status = models.UpdateStatus.testing
         # Clear pending messages
         self.db.info['messages'] = []
+        # note: this test is intentionally on the 'older' codepath using
+        # non-grouped critpath info to ensure that path works.
+        update.critpath_groups = None
         update.critpath = True
         update.pushed = True
         self.db.commit()

bodhi-server/tests/test_models.py

@@ -3083,13 +3083,21 @@ def test_greenwave_request_batches_multiple_critpath(self):
         for critpath update with multiple batches.
         """
         with mock.patch.dict('bodhi.server.models.config', {'greenwave_batch_size': 1}):
-            self.obj.critpath = True
+            self.obj.critpath_groups = "core critical-path-apps"
             assert self.obj.greenwave_subject_batch_size == 1
             assert self.obj.greenwave_request_batches(verbose=True) == (
                 [
                     {
                         'product_version': 'fedora-11',
-                        'decision_context': 'bodhi_update_push_testing_critpath',
+                        'decision_context': 'bodhi_update_push_testing_critical-path-apps_critpath',
+                        'verbose': True,
+                        'subject': [
+                            {'item': 'TurboGears-1.0.8-3.fc11', 'type': 'koji_build'},
+                        ]
+                    },
+                    {
+                        'product_version': 'fedora-11',
+                        'decision_context': 'bodhi_update_push_testing_core_critpath',
                         'verbose': True,
                         'subject': [
                             {'item': 'TurboGears-1.0.8-3.fc11', 'type': 'koji_build'},
@@ -3105,7 +3113,15 @@ def test_greenwave_request_batches_multiple_critpath(self):
                     },
                     {
                         'product_version': 'fedora-11',
-                        'decision_context': 'bodhi_update_push_testing_critpath',
+                        'decision_context': 'bodhi_update_push_testing_critical-path-apps_critpath',
+                        'verbose': True,
+                        'subject': [
+                            {'item': self.obj.alias, 'type': 'bodhi_update'},
+                        ]
+                    },
+                    {
+                        'product_version': 'fedora-11',
+                        'decision_context': 'bodhi_update_push_testing_core_critpath',
                         'verbose': True,
                         'subject': [
                             {'item': self.obj.alias, 'type': 'bodhi_update'},