Always obsolete updates that reach unstable_karma threshold

Before #901, we obsoleted updates that reached their
unstable_karma threshold regardless of autokarma setting, but
only for updates in 'testing' status, not 'pending' status. #901
intended to fix that.

In the review, @bowlofeggs said "we should only do this [obsolete
updates in pending state that reach their unstable_karma
threshold] if autokarma is turned on", but he did not apparently
realize that was inconsistent with the existing behavior in the
testing state. So after #901, we would obsolete
updates from pending only if autokarma was enabled, but we would
always obsolete updates from testing regardless of the autokarma
setting. This behaviour remained the same afer #1556.

I don't think it makes any sense to preserve this inconsistent
behavior. This PR comes down on the side of always obsoleting
regardless of the setting of autokarma, as I think in practice
this happens most often to updates in 'testing' state not
updates in 'pending' state, so it's the most similar behaviour.
It's also the older behaviour. It would be easy to flip it to
only ever unpush if autokarma is enabled, if we decide that's
better.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
(cherry picked from commit 25200ad)

bodhi-server/bodhi/server/models.py

@@ -3847,13 +3847,10 @@ def check_karma_thresholds(self, db, agent):
                     "%s update has reached the stable karma threshold and can be pushed to "
                     "stable now if the maintainer wishes"), self.alias)
         elif self.unstable_karma and self.karma <= self.unstable_karma:
-            if self.status is UpdateStatus.pending and not self.autokarma:
-                pass
-            else:
-                log.info("Automatically unpushing %s", self.alias)
-                self.obsolete(db)
-                notifications.publish(update_schemas.UpdateKarmaThresholdV1.from_dict(
-                    dict(update=self, status='unstable')))
+            log.info("Automatically obsoleting %s (reached unstable karma threshold)", self.alias)
+            self.obsolete(db)
+            notifications.publish(update_schemas.UpdateKarmaThresholdV1.from_dict(
+                dict(update=self, status='unstable')))
 
     @property
     def builds_json(self):

bodhi-server/bodhi/server/templates/new_update.html

@@ -318,7 +318,7 @@ <h6 class="fw-bold">Name</h6>
           <div class="card-body pt-0">
               <div class="row ms-3">
                   <div class="col-6">
-                      <h6 class="fw-bold" title="This is the threshold of positive karma required to automatically push an update from testing to stable.">Stable Karma</h6>
+                      <h6 class="fw-bold" title="This is the threshold of positive karma required to automatically push an update from testing to stable. This happens only if 'Auto-request stable based on karma?' is checked.">Stable Karma</h6>
                       <input type="number" class="form-control" name="stable_karma" min="1" required
                       % if update:
                       value="${update.stable_karma}"
@@ -328,7 +328,7 @@ <h6 class="fw-bold" title="This is the threshold of positive karma required to a
                     />
                   </div>
                   <div class="col-6">
-                      <h6 class="fw-bold" title="If checked, this option allows bodhi to automatically move your update from testing to stable once enough time was spent in testing.">Unstable Karma</h6>
+                      <h6 class="fw-bold" title="This is the threshold of negative karma required to automatically unpush an update. This will happen whether or not 'Auto-request stable based on karma?' is checked.">Unstable Karma</h6>
                       <input type="number" class="form-control" name="unstable_karma" max="-1" required
                       % if update:
                       value="${update.unstable_karma}"

bodhi-server/tests/services/test_updates.py

@@ -4770,11 +4770,10 @@ def test_karma_threshold_with_disabled_autopush(self, *args):
     @pytest.mark.parametrize("composed_by_bodhi", (True, False))
     @pytest.mark.parametrize("autokarma", (True, False))
     def test_obsolete_if_unstable(self, autokarma, composed_by_bodhi, req, status):
-        """Pending or testing updates should be obsoleted on reaching the auto-unpush threshold,
-        except if the status is pending and autokarma is disabled. Whether the update is
-        composed_by_bodhi should not matter. "Obsoleted" should mean the update status is
-        set to obsolete and its request to None. Obsoletion should not happen with just one
-        negative karma (if the threshold is -2)."""
+        """Pending or testing updates should be obsoleted on reaching the auto-unpush threshold.
+        Whether the update is composed_by_bodhi should not matter. "Obsoleted" should mean the
+        update status is set to obsolete and its request to None. Obsoletion should not happen
+        with just one negative karma (if the threshold is -2)."""
         nvr = 'bodhi-2.0.0-2.fc17'
         args = self.get_update(nvr)
         args['autokarma'] = autokarma
@@ -4802,23 +4801,16 @@ def test_obsolete_if_unstable(self, autokarma, composed_by_bodhi, req, status):
         # caused by the obsoletion to have taken effect when we construct the
         # expected message
         up.comment(self.db, "foo", -1, 'bowlofeggs')
-        if status is UpdateStatus.pending and not autokarma:
-            with fml_testing.mock_sends(update_schemas.UpdateCommentV1):
-                # doing a db commit causes the message to be published
-                self.db.commit()
-            assert up.status is status
-            assert up.request is req
-        else:
-            with fml_testing.mock_sends(
-                update_schemas.UpdateKarmaThresholdV1.from_dict(
-                    {'update': up, 'status': 'unstable'}
-                ),
-                update_schemas.UpdateCommentV1
-            ):
-                # doing a db commit causes the message to be published
-                self.db.commit()
-            assert up.status is UpdateStatus.obsolete
-            assert up.request is None
+        with fml_testing.mock_sends(
+            update_schemas.UpdateKarmaThresholdV1.from_dict(
+                {'update': up, 'status': 'unstable'}
+            ),
+            update_schemas.UpdateCommentV1
+        ):
+            # doing a db commit causes the message to be published
+            self.db.commit()
+        assert up.status is UpdateStatus.obsolete
+        assert up.request is None
         # the obsolete path should not disable autokarma, but we can't assert
         # this unconditionally because we might have hit the earlier disable-
         # autokarma path