From f7c3687ed282668140fae39596aa3c45ceb864fd Mon Sep 17 00:00:00 2001 From: Kai Zimmermann Date: Thu, 1 Mar 2018 14:36:39 +0100 Subject: [PATCH] Fix problem where autoclose feature is ignored in rollouts. (#642) Signed-off-by: kaizimmerm --- 3rd-dependencies/Release_0_2_0.md | 1 + .../jpa/JpaDeploymentManagement.java | 14 +++- .../repository/jpa/RolloutManagementTest.java | 44 ++++++++++++ .../jpa/autoassign/AutoAssignCheckerTest.java | 71 +++++++++++++++---- 4 files changed, 116 insertions(+), 14 deletions(-) diff --git a/3rd-dependencies/Release_0_2_0.md b/3rd-dependencies/Release_0_2_0.md index 56113baf2..005fafd7f 100644 --- a/3rd-dependencies/Release_0_2_0.md +++ b/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) | diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaDeploymentManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaDeploymentManagement.java index 8988d713c..470f8a110 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaDeploymentManagement.java +++ b/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 overrideObsoleteUpdateActions = onlineDsAssignmentStrategy - .overrideObsoleteUpdateActions(Collections.singletonList(action.getTarget().getId())); + final List overrideObsoleteUpdateActions; + + 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); 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 8a2a336cf..bc2431cc7 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 @@ -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 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.") diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/autoassign/AutoAssignCheckerTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/autoassign/AutoAssignCheckerTest.java index bce2ff35b..ab7238163 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/autoassign/AutoAssignCheckerTest.java +++ b/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 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())) - .isEqualTo(90); + assertThat(targetManagement.countByRsqlAndNonDS(setA.getId(), targetFilterQuery.getQuery())).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() - .create().name("dsA").version("1").type(testdataFactory.findOrCreateDefaultTestDsType())); + final DistributionSet setF = distributionSetManagement.create(entityFactory.distributionSet().create() + .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.create(entityFactory.targetFilterQuery().create() - .name("filterA").query("id==" + targetDsFIdPref + "*")).getId(), - setF.getId()); + targetFilterQueryManagement.updateAutoAssignDS(targetFilterQueryManagement.create( + entityFactory.targetFilterQuery().create().name("filterA").query("id==" + targetDsFIdPref + "*")) + .getId(), setF.getId()); // target filter query that matches failed bunch of targets - targetFilterQueryManagement.updateAutoAssignDS( - targetFilterQueryManagement.create(entityFactory.targetFilterQuery().create() - .name("filterB").query("id==" + targetDsAIdPref + "*")).getId(), - setA.getId()); + targetFilterQueryManagement.updateAutoAssignDS(targetFilterQueryManagement.create( + entityFactory.targetFilterQuery().create().name("filterB").query("id==" + targetDsAIdPref + "*")) + .getId(), setA.getId()); final List targetsF = testdataFactory.createTargets(10, targetDsFIdPref, targetDsFIdPref.concat(" description"));