From 09bf04ed2cb3a58a9f00b33628a73cdecd2a543d Mon Sep 17 00:00:00 2001 From: Jonathan Philip Knoblauch Date: Mon, 24 Oct 2016 17:13:34 +0200 Subject: [PATCH] Fix for the Bug - deleted targets are now considered when calculating threshold for success condition - added junit test Signed-off-by: Jonathan Philip Knoblauch --- ...ThresholdRolloutGroupSuccessCondition.java | 36 ++++++++--- .../repository/jpa/RolloutManagementTest.java | 62 +++++++++++++++++++ 2 files changed, 88 insertions(+), 10 deletions(-) diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rollout/condition/ThresholdRolloutGroupSuccessCondition.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rollout/condition/ThresholdRolloutGroupSuccessCondition.java index 676cd9ad1..3c18b25ae 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rollout/condition/ThresholdRolloutGroupSuccessCondition.java +++ b/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 targetsOfRolloutGroup = rolloutGroupManagement.findRolloutGroupTargets(rolloutGroup, + new OffsetBasedPageRequest(0, 1, null)); + return rolloutGroup.getTotalTargets() - targetsOfRolloutGroup.getTotalElements(); + } + } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/RolloutManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/RolloutManagementTest.java index a23ada6e6..9ae9b5b1d 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/RolloutManagementTest.java +++ b/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 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 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() {