Overhaul update "met requirements" checks

There are several problems in the general area of "checking if
an update 'meets requirements'".

Most obviously, as discussed in the comments on
#3938 , there are
two similar-but-different functions for this,
`Update.meets_testing_requirements()` and
`Update.check_requirements()`. `Update.set_request()` also sort
of re-implements the same thing in-line, and there is also
`Update.critpath_approved()` which does something similar.
These paths all implement somewhat different requirements,
which has various weird and confusing results (like the way
you can sometimes push an update stable from the CLI but not
from the web UI, or the way sometimes updates can be submitted
stable successfully but then get ejected from the stable push
for not meeting requirements).

This commit introduces `meets_requirements_why`, which returns
a boolean and a string explanation, and turns the existing
methods into thin wrappers around it.

The block in the API `set_request()` function which checked
requirements is dropped because it's now redundant; the DB
model `set_request()` which it calls does the same requirement
check. There's no need to do it twice.

The code block commented "Disable pushing critical path updates
for pending releases directly to stable" does not really do
that. It just applied the requirements for critical path
updates on push to stable (the block immediately under it
did the same for non-critpath updates). The boutique requirements
to do with a "critpath.num_admin_approvals" setting were for a
very old policy which gave special status to feedback from
"proven testers"; that setting has been 0 in both Bodhi
deployments for years and is not likely to come back, so we can
just get rid of the setting and simplify the code path.

This commit clarifies and simplifies the meanings of the two
types of stable push threshold settings, and renames some of
them. The `stable_karma` and `stable_days` properties of each
update are now *only* relevant to autopush. They are never used
in "can this update be pushed manually?" calculations. The
`mandatory_days_in_testing` and `min_karma` settings act *only*
as policy minimums for manual push eligibility. They can still
be set with prefix modifiers for the release and the 'status',
as before (to allow Fedora to apply different policies to
different releases at different points in the cycle). If an
update reaches either currently-applicable policy minimum, it
is eligible for stable push. If it does not, it is not. This
results in clearer logic and more consistent requirements that
are more in line with the distribution policies.

critpath.stable_after_days_without_negative_karma is renamed
to critpath.mandatory_days_in_testing and now simply sets
a days-in-testing threshold that applies to critical path
updates only. The "without_negative_karma" logic is removed,
because it has no basis in the Fedora or EPEL update policies;
they never specified such a rule.

critpath.min_karma is renamed to min_karma and applied to all
updates, because this is what the current Fedora and EPEL update
policies say. They both apply the same minimum karma thresholds
to *all* updates. We *could* instead add min_karma and keep
critpath.min_karma, but this is somewhat more work and I don't
want to do more work than is actually necessary here. If the
distros ever do decide to split the thresholds again, we can
re-introduce critpath.min_karma.

The functions and properties in the models are renamed in line
with the new names of the settings, again for sanity's sake.

This is of course an API change and a backwards-incompatible
configuration change. As there are only two real deployments
of Bodhi (both run out of the same ansible repo) and I don't
think anything uses the code interfaces, this seems like it
shouldn't be too much of a problem.

Signed-off-by: Adam Williamson <[email protected]>
(cherry picked from commit 93598c7)

bodhi-server/bodhi/server/config.py

@@ -360,13 +360,7 @@ class BodhiConfig(dict):
         'critpath.jsonpath': {
             'value': '/etc/bodhi/critpath',
             'validator': str},
-        'critpath.min_karma': {
-            'value': 2,
-            'validator': int},
-        'critpath.num_admin_approvals': {
-            'value': 2,
-            'validator': int},
-        'critpath.stable_after_days_without_negative_karma': {
+        'critpath.mandatory_days_in_testing': {
             'value': 14,
             'validator': int},
         'critpath.type': {
@@ -452,6 +446,9 @@ class BodhiConfig(dict):
         'mako.directories': {
             'value': 'bodhi.server:templates',
             'validator': str},
+        'min_karma': {
+            'value': 2,
+            'validator': int},
         'mandatory_packager_groups': {
             'value': ['packager'],
             'validator': _generate_list_validator()},

bodhi-server/bodhi/server/services/updates.py

@@ -211,21 +211,10 @@ def set_request(request):
                            "Pushing back to testing a stable update is not allowed")
         return
 
-    if action == UpdateRequest.stable:
-        settings = request.registry.settings
-        result, reason = update.check_requirements(request.db, settings)
-        if not result:
-            log.info(
-                f'Unable to set request for {update.alias} to stable due to failed requirements: '
-                f'{reason}')
-            request.errors.add('body', 'request',
-                               'Requirement not met %s' % reason)
-            return
-
     try:
         update.set_request(request.db, action, request.identity.name)
     except BodhiException as e:
-        log.info("Failed to set the request: %s", e)
+        log.info(f"Failed to set the request: {e}")
         request.errors.add('body', 'request', str(e))
     except Exception as e:
         log.exception("Unhandled exception in set_request")

bodhi-server/bodhi/server/tasks/composer.py

@@ -469,7 +469,7 @@ def perform_gating(self):
         """Eject Updates that don't meet testing requirements from the compose."""
         log.debug('Performing gating.')
         for update in self.compose.updates:
-            result, reason = update.check_requirements(self.db, config)
+            result, reason = update.meets_requirements_why
             if not result:
                 log.warning("%s failed gating: %s" % (update.alias, reason))
                 self.eject_from_compose(update, reason)

bodhi-server/production.ini

@@ -408,10 +408,10 @@ use = egg:bodhi-server
 # critpath.num_admin_approvals = 2
 
 # The net karma required to submit a critical path update to a pending release.
-# critpath.min_karma = 2
+# min_karma = 2
 
 # Allow critpath to submit for stable after 2 weeks with no negative karma
-# critpath.stable_after_days_without_negative_karma = 14
+# critpath.mandatory_days_in_testing = 14
 
 # The minimum amount of time an update must spend in testing before
 # it can reach the stable repository
@@ -431,12 +431,12 @@ fedora_epel.mandatory_days_in_testing = 14
 #f28.status = pre_beta
 #f28.pre_beta.mandatory_days_in_testing = 3
 #f28.pre_beta.critpath.num_admin_approvals = 0
-#f28.pre_beta.critpath.min_karma = 1
-#f28.pre_beta.critpath.stable_after_days_without_negative_karma = 3
+#f28.pre_beta.min_karma = 1
+#f28.pre_beta.critpath.mandatory_days_in_testing = 3
 #f28.post_beta.mandatory_days_in_testing = 7
 #f28.post_beta.critpath.num_admin_approvals = 0
-#f28.post_beta.critpath.min_karma = 2
-#f28.post_beta.critpath.stable_after_days_without_negative_karma = 5
+#f28.post_beta.min_karma = 2
+#f28.post_beta.critpath.mandatory_days_in_testing = 5
 
 
 ##