Fix for the Bug

- deleted targets are now considered when calculating threshold for success condition
- added junit test 

Signed-off-by: Jonathan Philip Knoblauch <JonathanPhilip.Knoblauch@bosch-si.com>

hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rollout/condition/ThresholdRolloutGroupSuccessCondition.java

@@ -8,45 +8,61 @@
  */
 package org.eclipse.hawkbit.repository.jpa.rollout.condition;
 
+import org.eclipse.hawkbit.repository.OffsetBasedPageRequest;
+import org.eclipse.hawkbit.repository.RolloutGroupManagement;
 import org.eclipse.hawkbit.repository.jpa.ActionRepository;
 import org.eclipse.hawkbit.repository.model.Action;
 import org.eclipse.hawkbit.repository.model.Rollout;
 import org.eclipse.hawkbit.repository.model.RolloutGroup;
+import org.eclipse.hawkbit.repository.model.Target;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.data.domain.Page;
 import org.springframework.stereotype.Component;
 
 /**
- *
+ * Threshold to calculate if rollout group success condition is reached and the
+ * next rollout group can get started.
  */
 @Component("thresholdRolloutGroupSuccessCondition")
 public class ThresholdRolloutGroupSuccessCondition implements RolloutGroupConditionEvaluator {
     private static final Logger LOGGER = LoggerFactory.getLogger(ThresholdRolloutGroupSuccessCondition.class);
 
+    @Autowired
+    private RolloutGroupManagement rolloutGroupManagement;
+
     @Autowired
     private ActionRepository actionRepository;
 
     @Override
     public boolean eval(final Rollout rollout, final RolloutGroup rolloutGroup, final String expression) {
+
         final long totalGroup = rolloutGroup.getTotalTargets();
-        final long finished = this.actionRepository.countByRolloutIdAndRolloutGroupIdAndStatus(rollout.getId(),
+        if (totalGroup == 0) {
+            // in case e.g. targets has been deleted we don't have any
+            // actions left for this group, so the group is finished
+            return true;
+        }
+
+        final long finishedByStatus = this.actionRepository.countByRolloutIdAndRolloutGroupIdAndStatus(rollout.getId(),
                 rolloutGroup.getId(), Action.Status.FINISHED);
+        final long finished = finishedByStatus + countDeletedTargets(rolloutGroup);
+
         try {
             final Integer threshold = Integer.valueOf(expression);
-
-            if (totalGroup == 0) {
-                // in case e.g. targets has been deleted we don't have any
-                // actions
-                // left for this group, so the group is finished
-                return true;
-            }
             // calculate threshold
-            return ((float) finished / (float) totalGroup) >= ((float) threshold / 100F);
+            return ((float) finished / totalGroup) >= ((float) threshold / 100F);
         } catch (final NumberFormatException e) {
             LOGGER.error("Cannot evaluate condition expression " + expression, e);
             return false;
         }
     }
 
+    private long countDeletedTargets(final RolloutGroup rolloutGroup) {
+        final Page<Target> targetsOfRolloutGroup = rolloutGroupManagement.findRolloutGroupTargets(rolloutGroup,
+                new OffsetBasedPageRequest(0, 1, null));
+        return rolloutGroup.getTotalTargets() - targetsOfRolloutGroup.getTotalElements();
+    }
+
 }

hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/RolloutManagementTest.java

@@ -160,6 +160,68 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest {
                         + group.getStatus() + " state"));
     }
 
+    @Test
+    @Description("Verfiying that next group is started when targets of the group have been deleted.")
+    public void checkRunningRolloutsStartsNextGroupIfTargetsDeleted() {
+        final int amountTargetsForRollout = 15;
+        final int amountOtherTargets = 0;
+        final int amountGroups = 3;
+        final String successCondition = "100";
+        final String errorCondition = "80";
+        final Rollout createdRollout = createSimpleTestRolloutWithTargetsAndDistributionSet(amountTargetsForRollout,
+                amountOtherTargets, amountGroups, successCondition, errorCondition);
+
+        rolloutManagement.startRollout(createdRollout);
+
+        // finish group one by finishing targets and deleting targets
+        List<Action> runningActions = deploymentManagement.findActionsByRolloutAndStatus(createdRollout,
+                Status.RUNNING);
+        finishAction(runningActions.get(0));
+        finishAction(runningActions.get(1));
+        finishAction(runningActions.get(2));
+        targetManagement.deleteTargets(runningActions.get(3).getTarget().getId(),
+                runningActions.get(4).getTarget().getId());
+
+        rolloutManagement.checkRunningRollouts(0);
+
+        // validate that the second group is in running state
+        List<RolloutGroup> runningRolloutGroups = rolloutGroupManagement.findRolloutGroupsByRolloutId(
+                createdRollout.getId(), new OffsetBasedPageRequest(0, 10, new Sort(Direction.ASC, "id"))).getContent();
+        assertThat(runningRolloutGroups.get(0).getStatus()).isEqualTo(RolloutGroupStatus.FINISHED);
+        assertThat(runningRolloutGroups.get(1).getStatus()).isEqualTo(RolloutGroupStatus.RUNNING);
+
+        // finish one action and delete the other targets of group two
+        runningActions = deploymentManagement.findActionsByRolloutAndStatus(createdRollout, Status.RUNNING);
+        finishAction(runningActions.get(0));
+        targetManagement.deleteTargets(runningActions.get(1).getTarget().getId(),
+                runningActions.get(2).getTarget().getId(), runningActions.get(3).getTarget().getId(),
+                runningActions.get(4).getTarget().getId());
+
+        // delete all targets of the third group
+        runningActions = deploymentManagement.findActionsByRolloutAndStatus(createdRollout, Status.SCHEDULED);
+        targetManagement.deleteTargets(runningActions.get(0).getTarget().getId(),
+                runningActions.get(1).getTarget().getId(), runningActions.get(2).getTarget().getId(),
+                runningActions.get(3).getTarget().getId(), runningActions.get(4).getTarget().getId());
+
+        // validate that groups and rollout finished correctly
+        rolloutManagement.checkRunningRollouts(0);
+        runningRolloutGroups = rolloutGroupManagement.findRolloutGroupsByRolloutId(createdRollout.getId(),
+                new OffsetBasedPageRequest(0, 10, new Sort(Direction.ASC, "id"))).getContent();
+        assertThat(runningRolloutGroups.get(0).getStatus()).isEqualTo(RolloutGroupStatus.FINISHED);
+        assertThat(runningRolloutGroups.get(1).getStatus()).isEqualTo(RolloutGroupStatus.FINISHED);
+        assertThat(runningRolloutGroups.get(2).getStatus()).isEqualTo(RolloutGroupStatus.FINISHED);
+        assertThat(rolloutManagement.findRolloutById(createdRollout.getId()).getStatus())
+                .isEqualTo(RolloutStatus.FINISHED);
+
+    }
+
+    private void finishAction(final Action action) {
+        final JpaAction jpaAction = (JpaAction) action;
+        action.setStatus(Status.FINISHED);
+        controllerManagament
+                .addUpdateActionStatus(new JpaActionStatus(jpaAction, Status.FINISHED, System.currentTimeMillis(), ""));
+    }
+
     @Test
     @Description("Verfiying that the error handling action of a group is executed to pause the current rollout")
     public void checkErrorHitOfGroupCallsErrorActionToPauseTheRollout() {