Fix problem where autoclose feature is ignored in rollouts. (#642)

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

3rd-dependencies/Release_0_2_0.md

@@ -4,6 +4,7 @@
 
 | Group ID  | Artifact ID  | Version  | CQ  |
 |---|---|---|---|
+|com.cronutils|cron-utils|5.0.5| [CQ15762](https://dev.eclipse.org/ipzilla/show_bug.cgi?id=15762) |
 |com.github.ben-manes.caffeine|caffeine|2.3.5| [CQ13563](https://dev.eclipse.org/ipzilla/show_bug.cgi?id=13563) |
 |aopalliance|aopalliance|1.0| [CQ10346](https://dev.eclipse.org/ipzilla/show_bug.cgi?id=10346) |
 |ch.qos.logback|logback-classic|1.1.3| [CQ10347](https://dev.eclipse.org/ipzilla/show_bug.cgi?id=10347) |

hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaDeploymentManagement.java

@@ -448,8 +448,18 @@ public class JpaDeploymentManagement implements DeploymentManagement {
 
     private JpaAction startScheduledActionIfNoCancelationHasToBeHandledFirst(final JpaAction action) {
         // check if we need to override running update actions
-        final List<Long> overrideObsoleteUpdateActions = onlineDsAssignmentStrategy
+        final List<Long> overrideObsoleteUpdateActions;
-                .overrideObsoleteUpdateActions(Collections.singletonList(action.getTarget().getId()));
+
+        if (systemSecurityContext.runAsSystem(() -> tenantConfigurationManagement
+                .getConfigurationValue(TenantConfigurationKey.REPOSITORY_ACTIONS_AUTOCLOSE_ENABLED, Boolean.class)
+                .getValue())) {
+            overrideObsoleteUpdateActions = Collections.emptyList();
+            onlineDsAssignmentStrategy
+                    .closeObsoleteUpdateActions(Collections.singletonList(action.getTarget().getId()));
+        } else {
+            overrideObsoleteUpdateActions = onlineDsAssignmentStrategy
+                    .overrideObsoleteUpdateActions(Collections.singletonList(action.getTarget().getId()));
+        }
 
         action.setActive(true);
         action.setStatus(Status.RUNNING);

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

@@ -63,6 +63,7 @@ import org.eclipse.hawkbit.repository.model.TargetUpdateStatus;
 import org.eclipse.hawkbit.repository.model.TotalTargetCountStatus;
 import org.eclipse.hawkbit.repository.test.matcher.Expect;
 import org.eclipse.hawkbit.repository.test.matcher.ExpectEvents;
+import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey;
 import org.junit.Test;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.Slice;
@@ -120,6 +121,49 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest {
 
     }
 
+    @Test
+    @Description("Verifies that a running action is auto canceled by a rollout which assigns another distribution-set.")
+    public void rolloutAssignesNewDistributionSetAndAutoCloseActiveActions() {
+        tenantConfigurationManagement
+                .addOrUpdateConfiguration(TenantConfigurationKey.REPOSITORY_ACTIONS_AUTOCLOSE_ENABLED, true);
+
+        try {
+            // manually assign distribution set to target
+            final String knownControllerId = "controller12345";
+            final DistributionSet firstDistributionSet = testdataFactory.createDistributionSet();
+            final DistributionSet secondDistributionSet = testdataFactory.createDistributionSet("second");
+            testdataFactory.createTarget(knownControllerId);
+            final DistributionSetAssignmentResult assignmentResult = deploymentManagement.assignDistributionSet(
+                    firstDistributionSet.getId(), ActionType.FORCED, 0, Collections.singleton(knownControllerId));
+            final Long manuallyAssignedActionId = assignmentResult.getActions().get(0);
+
+            // create rollout with the same distribution set already assigned
+            // start rollout
+            final Rollout rollout = testdataFactory.createRolloutByVariables("rolloutNotCancelRunningAction",
+                    "description", 1, "name==*", secondDistributionSet, "50", "5");
+            rolloutManagement.start(rollout.getId());
+            rolloutManagement.handleRollouts();
+
+            // verify that manually created action is canceled and action
+            // created from rollout is running
+            final List<Action> actionsByKnownTarget = deploymentManagement.findActionsByTarget(knownControllerId, PAGE)
+                    .getContent();
+            // should be 2 actions, one manually and one from the rollout
+            assertThat(actionsByKnownTarget).hasSize(2);
+            // verify that manually assigned action is still running
+            assertThat(deploymentManagement.findAction(manuallyAssignedActionId).get().getStatus())
+                    .isEqualTo(Status.CANCELED);
+            // verify that rollout management created action is running
+            final Action rolloutCreatedAction = actionsByKnownTarget.stream()
+                    .filter(action -> !action.getId().equals(manuallyAssignedActionId)).findAny().get();
+            assertThat(rolloutCreatedAction.getStatus()).isEqualTo(Status.RUNNING);
+        } finally {
+            tenantConfigurationManagement
+                    .addOrUpdateConfiguration(TenantConfigurationKey.REPOSITORY_ACTIONS_AUTOCLOSE_ENABLED, false);
+        }
+
+    }
+
     @Test
     @Description("Verifies that management get access reacts as specfied on calls for non existing entities by means "
             + "of Optional not present.")

hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/autoassign/AutoAssignCheckerTest.java

@@ -10,13 +10,19 @@ package org.eclipse.hawkbit.repository.jpa.autoassign;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+import java.util.Collections;
 import java.util.List;
 import java.util.stream.Collectors;
 
 import org.eclipse.hawkbit.repository.jpa.AbstractJpaIntegrationTest;
+import org.eclipse.hawkbit.repository.model.Action;
+import org.eclipse.hawkbit.repository.model.Action.ActionType;
+import org.eclipse.hawkbit.repository.model.Action.Status;
 import org.eclipse.hawkbit.repository.model.DistributionSet;
+import org.eclipse.hawkbit.repository.model.DistributionSetAssignmentResult;
 import org.eclipse.hawkbit.repository.model.Target;
 import org.eclipse.hawkbit.repository.model.TargetFilterQuery;
+import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey;
 import org.junit.Test;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.data.domain.Slice;
@@ -37,6 +43,50 @@ public class AutoAssignCheckerTest extends AbstractJpaIntegrationTest {
     @Autowired
     private AutoAssignChecker autoAssignChecker;
 
+    @Test
+    @Description("Verifies that a running action is auto canceled by a AutoAssignment which assigns another distribution-set.")
+    public void autoAssignDistributionSetAndAutoCloseOldActions() {
+
+        tenantConfigurationManagement
+                .addOrUpdateConfiguration(TenantConfigurationKey.REPOSITORY_ACTIONS_AUTOCLOSE_ENABLED, true);
+
+        try {
+            final String knownControllerId = "controller12345";
+            final DistributionSet firstDistributionSet = testdataFactory.createDistributionSet();
+            final DistributionSet secondDistributionSet = testdataFactory.createDistributionSet("second");
+            testdataFactory.createTarget(knownControllerId);
+            final DistributionSetAssignmentResult assignmentResult = deploymentManagement.assignDistributionSet(
+                    firstDistributionSet.getId(), ActionType.FORCED, 0, Collections.singleton(knownControllerId));
+            final Long manuallyAssignedActionId = assignmentResult.getActions().get(0);
+
+            // target filter query that matches all targets
+            final TargetFilterQuery targetFilterQuery = targetFilterQueryManagement
+                    .create(entityFactory.targetFilterQuery().create().name("filterA").query("name==*"));
+            targetFilterQueryManagement.updateAutoAssignDS(targetFilterQuery.getId(), secondDistributionSet.getId());
+
+            // Run the check
+            autoAssignChecker.check();
+
+            // verify that manually created action is canceled and action
+            // created from AutoAssign is running
+            final List<Action> actionsByKnownTarget = deploymentManagement.findActionsByTarget(knownControllerId, PAGE)
+                    .getContent();
+            // should be 2 actions, one manually and one from the AutoAssign
+            assertThat(actionsByKnownTarget).hasSize(2);
+            // verify that manually assigned action is still running
+            assertThat(deploymentManagement.findAction(manuallyAssignedActionId).get().getStatus())
+                    .isEqualTo(Status.CANCELED);
+            // verify that AutoAssign created action is running
+            final Action rolloutCreatedAction = actionsByKnownTarget.stream()
+                    .filter(action -> !action.getId().equals(manuallyAssignedActionId)).findAny().get();
+            assertThat(rolloutCreatedAction.getStatus()).isEqualTo(Status.RUNNING);
+
+        } finally {
+            tenantConfigurationManagement
+                    .addOrUpdateConfiguration(TenantConfigurationKey.REPOSITORY_ACTIONS_AUTOCLOSE_ENABLED, false);
+        }
+    }
+
     @Test
     @Description("Test auto assignment of a DS to filtered targets")
     public void checkAutoAssign() {
@@ -72,8 +122,7 @@ public class AutoAssignCheckerTest extends AbstractJpaIntegrationTest {
         verifyThatTargetsHaveDistributionSetAssignment(setB, targets.subList(10, 20), targetsCount);
 
         // Count the number of targets that will be assigned with setA
-        assertThat(targetManagement.countByRsqlAndNonDS(setA.getId(), targetFilterQuery.getQuery()))
+        assertThat(targetManagement.countByRsqlAndNonDS(setA.getId(), targetFilterQuery.getQuery())).isEqualTo(90);
-                .isEqualTo(90);
 
         // Run the check
         autoAssignChecker.check();
@@ -90,8 +139,8 @@ public class AutoAssignCheckerTest extends AbstractJpaIntegrationTest {
     public void checkAutoAssignWithFailures() {
 
         // incomplete distribution set that will be assigned
-        final DistributionSet setF = distributionSetManagement.create(entityFactory.distributionSet()
+        final DistributionSet setF = distributionSetManagement.create(entityFactory.distributionSet().create()
-                .create().name("dsA").version("1").type(testdataFactory.findOrCreateDefaultTestDsType()));
+                .name("dsA").version("1").type(testdataFactory.findOrCreateDefaultTestDsType()));
         final DistributionSet setA = testdataFactory.createDistributionSet("dsA");
         final DistributionSet setB = testdataFactory.createDistributionSet("dsB");
 
@@ -100,16 +149,14 @@ public class AutoAssignCheckerTest extends AbstractJpaIntegrationTest {
 
         // target filter query that matches first bunch of targets, that should
         // fail
-        targetFilterQueryManagement.updateAutoAssignDS(
+        targetFilterQueryManagement.updateAutoAssignDS(targetFilterQueryManagement.create(
-                targetFilterQueryManagement.create(entityFactory.targetFilterQuery().create()
+                entityFactory.targetFilterQuery().create().name("filterA").query("id==" + targetDsFIdPref + "*"))
-                        .name("filterA").query("id==" + targetDsFIdPref + "*")).getId(),
+                .getId(), setF.getId());
-                setF.getId());
 
         // target filter query that matches failed bunch of targets
-        targetFilterQueryManagement.updateAutoAssignDS(
+        targetFilterQueryManagement.updateAutoAssignDS(targetFilterQueryManagement.create(
-                targetFilterQueryManagement.create(entityFactory.targetFilterQuery().create()
+                entityFactory.targetFilterQuery().create().name("filterB").query("id==" + targetDsAIdPref + "*"))
-                        .name("filterB").query("id==" + targetDsAIdPref + "*")).getId(),
+                .getId(), setA.getId());
-                setA.getId());
 
         final List<Target> targetsF = testdataFactory.createTargets(10, targetDsFIdPref,
                 targetDsFIdPref.concat(" description"));