diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/AutoAssignExecutor.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/AutoAssignHandler.java similarity index 86% rename from hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/AutoAssignExecutor.java rename to hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/AutoAssignHandler.java index fc0ece26a..cf7c5a279 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/AutoAssignExecutor.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/AutoAssignHandler.java @@ -12,18 +12,18 @@ package org.eclipse.hawkbit.repository; /** * An interface declaration which contains the check for the auto assignment logic. */ -public interface AutoAssignExecutor { +public interface AutoAssignHandler { /** * Checks all target filter queries with an auto assign distribution set and triggers the check and assignment to targets that don't have * the design DS yet */ - void checkAllTargets(); + void handleAll(); /** * Method performs an auto assign check for a specific device only * * @param controllerId of the device to check */ - void checkSingleTarget(String controllerId); + void handleSingleTarget(String controllerId); } \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RolloutHandler.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RolloutHandler.java index aa27640a2..329088c7e 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RolloutHandler.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RolloutHandler.java @@ -43,4 +43,4 @@ public interface RolloutHandler { */ @PreAuthorize(SpringEvalExpressions.IS_SYSTEM_CODE) void handleAll(); -} +} \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRepositoryConfiguration.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRepositoryConfiguration.java index 14348e388..06bbad913 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRepositoryConfiguration.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRepositoryConfiguration.java @@ -28,7 +28,7 @@ import org.eclipse.hawkbit.ql.jpa.QLSupport; import org.eclipse.hawkbit.ql.jpa.QLSupport.NodeTransformer; import org.eclipse.hawkbit.ql.jpa.QLSupport.QueryParser; import org.eclipse.hawkbit.ql.rsql.RsqlParser; -import org.eclipse.hawkbit.repository.AutoAssignExecutor; +import org.eclipse.hawkbit.repository.AutoAssignHandler; import org.eclipse.hawkbit.repository.DeploymentManagement; import org.eclipse.hawkbit.repository.PropertiesQuotaManagement; import org.eclipse.hawkbit.repository.QuotaManagement; @@ -341,10 +341,10 @@ public class JpaRepositoryConfiguration { @Profile("!test") @ConditionalOnProperty(prefix = "hawkbit.autoassign.scheduler", name = "enabled", matchIfMissing = true) AutoAssignScheduler autoAssignScheduler( - final SystemManagement systemManagement, final AutoAssignExecutor autoAssignExecutor, + final SystemManagement systemManagement, final AutoAssignHandler autoAssignHandler, @Value("${hawkbit.autoassign.executor.thread-pool.size:1}") final int threadPoolSize, final LockRegistry lockRegistry, final Optional meterRegistry) { - return new AutoAssignScheduler(systemManagement, autoAssignExecutor, threadPoolSize, lockRegistry, meterRegistry); + return new AutoAssignScheduler(systemManagement, autoAssignHandler, threadPoolSize, meterRegistry); } /** diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/scheduler/AutoAssignScheduler.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/scheduler/AutoAssignScheduler.java index 3905f37c7..6550c8cf3 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/scheduler/AutoAssignScheduler.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/scheduler/AutoAssignScheduler.java @@ -14,14 +14,11 @@ import static org.eclipse.hawkbit.context.AccessContext.asSystemAsTenant; import java.util.Optional; import java.util.concurrent.TimeUnit; -import java.util.concurrent.locks.Lock; import io.micrometer.core.instrument.MeterRegistry; import lombok.extern.slf4j.Slf4j; -import org.eclipse.hawkbit.repository.AutoAssignExecutor; +import org.eclipse.hawkbit.repository.AutoAssignHandler; import org.eclipse.hawkbit.repository.SystemManagement; -import org.eclipse.hawkbit.tenancy.DefaultTenantConfiguration; -import org.springframework.integration.support.locks.LockRegistry; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor; @@ -34,18 +31,16 @@ public class AutoAssignScheduler { private static final String PROP_SCHEDULER_DELAY_PLACEHOLDER = "${hawkbit.autoassign.scheduler.fixedDelay:2000}"; private final SystemManagement systemManagement; - private final AutoAssignExecutor autoAssignExecutor; - private final LockRegistry lockRegistry; + private final AutoAssignHandler autoAssignHandler; private final Optional meterRegistry; + private final ThreadPoolTaskExecutor autoAssignTaskExecutor; public AutoAssignScheduler( - final SystemManagement systemManagement, final AutoAssignExecutor autoAssignExecutor, - final int threadPoolSize, - final LockRegistry lockRegistry, final Optional meterRegistry) { + final SystemManagement systemManagement, final AutoAssignHandler autoAssignHandler, + final int threadPoolSize, final Optional meterRegistry) { this.systemManagement = systemManagement; - this.autoAssignExecutor = autoAssignExecutor; - this.lockRegistry = lockRegistry; + this.autoAssignHandler = autoAssignHandler; this.meterRegistry = meterRegistry; autoAssignTaskExecutor = SchedulerUtils.threadPoolTaskExecutor("auto-assign-exec-", threadPoolSize); } @@ -58,7 +53,8 @@ public class AutoAssignScheduler { public void autoAssignScheduler() { // run this code in system code privileged to have the necessary permission to query and create entities. log.debug("Triggered auto-assign scheduler."); - final long startNano = java.lang.System.nanoTime(); + final long startNano = System.nanoTime(); + asSystem(() -> systemManagement.forEachTenantAsSystem(tenant -> { if (autoAssignTaskExecutor == null) {// sync @@ -68,31 +64,19 @@ public class AutoAssignScheduler { } }) ); - meterRegistry - .map(mReg -> mReg.timer("hawkbit.autoassign.scheduler.all")) - .ifPresent(timer -> timer.record(java.lang.System.nanoTime() - startNano, TimeUnit.NANOSECONDS)); + + meterRegistry // handle all tenants (some could be skipped if lock could not be obtained) + .map(mReg -> mReg.timer("hawkbit.autoassign.scheduler")) + .ifPresent(timer -> timer.record(System.nanoTime() - startNano, TimeUnit.NANOSECONDS)); log.debug("Finished auto-assign scheduler run."); } private void handleAll(final String tenant) { - final Lock lock = lockRegistry.obtain(createAutoAssignmentLockKey(tenant)); - if (!lock.tryLock()) { - return; - } - final long startNanoT = System.nanoTime(); + log.trace("Handling auto-assignments for tenant: {}", tenant); try { - autoAssignExecutor.checkAllTargets(); - } finally { - lock.unlock(); - meterRegistry - .map(mReg -> mReg.timer( - "hawkbit.autoassign.scheduler", - DefaultTenantConfiguration.TENANT_TAG, tenant)) - .ifPresent(timer -> timer.record(System.nanoTime() - startNanoT, TimeUnit.NANOSECONDS)); + autoAssignHandler.handleAll(); + } catch (final Exception e) { + log.error("Error auto-assignments rollout for tenant {}", tenant, e); } } - - private static String createAutoAssignmentLockKey(final String tenant) { - return tenant + "-auto-assign"; - } } \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/scheduler/JpaAutoAssignExecutor.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/scheduler/JpaAutoAssignHandler.java similarity index 61% rename from hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/scheduler/JpaAutoAssignExecutor.java rename to hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/scheduler/JpaAutoAssignHandler.java index 5d4460f28..679528fc3 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/scheduler/JpaAutoAssignExecutor.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/scheduler/JpaAutoAssignHandler.java @@ -11,16 +11,24 @@ package org.eclipse.hawkbit.repository.jpa.scheduler; import static org.eclipse.hawkbit.context.AccessContext.asActor; import static org.eclipse.hawkbit.context.AccessContext.withSecurityContext; +import static org.eclipse.hawkbit.tenancy.DefaultTenantConfiguration.TENANT_TAG; import java.util.Collections; import java.util.List; +import java.util.Optional; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.Lock; import java.util.function.Consumer; +import jakarta.persistence.LockTimeoutException; import jakarta.persistence.PersistenceException; +import io.micrometer.core.instrument.MeterRegistry; import lombok.extern.slf4j.Slf4j; +import org.eclipse.hawkbit.context.AccessContext; import org.eclipse.hawkbit.exception.AbstractServerRtException; -import org.eclipse.hawkbit.repository.AutoAssignExecutor; +import org.eclipse.hawkbit.repository.AutoAssignHandler; import org.eclipse.hawkbit.repository.DeploymentManagement; import org.eclipse.hawkbit.repository.TargetFilterQueryManagement; import org.eclipse.hawkbit.repository.TargetManagement; @@ -33,6 +41,7 @@ import org.eclipse.hawkbit.repository.model.TargetFilterQuery; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Slice; +import org.springframework.integration.support.locks.LockRegistry; import org.springframework.stereotype.Service; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.annotation.Isolation; @@ -47,7 +56,7 @@ import org.springframework.util.StringUtils; */ @Slf4j @Service -public class JpaAutoAssignExecutor implements AutoAssignExecutor { +public class JpaAutoAssignHandler implements AutoAssignHandler { /** * The message which is added to the action status when a distribution set is assigned to a target. @@ -60,33 +69,98 @@ public class JpaAutoAssignExecutor implements AutoAssignExecutor { */ private static final int PAGE_SIZE = 1000; + private static final LockTimeoutException LOCK_TIMEOUT_EXCEPTION = new LockTimeoutException("Could not obtain lock for auto assignment"); + private final TargetFilterQueryManagement targetFilterQueryManagement; private final TargetManagement targetManagement; private final DeploymentManagement deploymentManagement; private final PlatformTransactionManager transactionManager; + private final LockRegistry lockRegistry; + private final Optional meterRegistry; - public JpaAutoAssignExecutor( + public JpaAutoAssignHandler( final TargetFilterQueryManagement targetFilterQueryManagement, final TargetManagement targetManagement, final DeploymentManagement deploymentManagement, - final PlatformTransactionManager transactionManager) { + final PlatformTransactionManager transactionManager, final LockRegistry lockRegistry, final Optional meterRegistry) { this.targetFilterQueryManagement = targetFilterQueryManagement; this.targetManagement = targetManagement; this.deploymentManagement = deploymentManagement; this.transactionManager = transactionManager; + this.lockRegistry = lockRegistry; + this.meterRegistry = meterRegistry; } @Override @Transactional(propagation = Propagation.REQUIRES_NEW) - public void checkAllTargets() { - log.debug("Auto assign check call started"); - forEachFilterWithAutoAssignDS(this::checkByTargetFilterQueryAndAssignDS); - log.debug("Auto assign check call finished"); + public void handleAll() { + final long startNano = System.nanoTime(); + + final AtomicReference lockRef = new AtomicReference<>(); + try { + forEachFilterWithAutoAssignDS(targetFilterQuery -> { + if (lockRef.get() == null) { + // only if there are targetFilterQueries to process we try to obtain the lock (on the first one) + final Lock lock = lockRegistry.obtain(createAutoAssignmentLockKey(AccessContext.tenant())); + if (!lock.tryLock()) { + if (log.isTraceEnabled()) { + log.trace("Could not obtain lock {}", lock); + } + throw LOCK_TIMEOUT_EXCEPTION; + } else { + lockRef.set(lock); + + log.debug("Start auto-assign handling"); + } + } + + final long startNanoPartial = System.nanoTime(); + try { + checkByTargetFilterQueryAndAssignDS(targetFilterQuery); + } finally { + meterRegistry // handle single targetFilterQuery + .map(mReg -> mReg.timer( + "hawkbit.autoassign.handle", TENANT_TAG, AccessContext.tenant(), "targetFilterQuery", + String.valueOf(targetFilterQuery.getId()))) + .ifPresent(timer -> timer.record(System.nanoTime() - startNanoPartial, TimeUnit.NANOSECONDS)); + } + }); + } finally { + final Lock lock = lockRef.get(); + if (lock != null) { + lock.unlock(); + + // only if there is at least one targetFilterQuery and lock has been obtained then will be measured (as in rollouts) + meterRegistry // handle single targetFilterQuery for single target + .map(mReg -> mReg.timer( + "hawkbit.autoassign.handle.all", TENANT_TAG, AccessContext.tenant())) + .ifPresent(timer -> timer.record(System.nanoTime() - startNano, TimeUnit.NANOSECONDS)); + log.debug("Auto assign check all targets finished"); + } + } } @Override - public void checkSingleTarget(final String controllerId) { + public void handleSingleTarget(final String controllerId) { log.debug("Auto assign check call for device {} started", controllerId); - forEachFilterWithAutoAssignDS(filter -> checkForDevice(controllerId, filter)); + final long startNano = System.nanoTime(); + + forEachFilterWithAutoAssignDS(targetFilterQuery -> { + final long startNanoPartial = System.nanoTime(); + try { + checkForDevice(controllerId, targetFilterQuery); + } finally { + meterRegistry // handle single targetFilterQuery for single target + .map(mReg -> mReg.timer( + "hawkbit.autoassign.handle.single", TENANT_TAG, AccessContext.tenant(), "targetFilterQuery", + String.valueOf(targetFilterQuery.getId()))) + .ifPresent(timer -> timer.record(System.nanoTime() - startNanoPartial, TimeUnit.NANOSECONDS)); + } + }); + + meterRegistry // handle single targetFilterQuery for single target + .map(mReg -> mReg.timer( + "hawkbit.autoassign.handle.single.all", TENANT_TAG, AccessContext.tenant())) + .ifPresent(timer -> timer.record(System.nanoTime() - startNano, TimeUnit.NANOSECONDS)); log.debug("Auto assign check call for device {} finished", controllerId); } @@ -134,25 +208,32 @@ public class JpaAutoAssignExecutor implements AutoAssignExecutor { do { filterQueries = targetFilterQueryManagement.findWithAutoAssignDS(query); - filterQueries.forEach(filterQuery -> { - try { - filterQuery.getAccessControlContext().ifPresentOrElse( - // has stored context - executes it with it - context -> withSecurityContext(context, () -> consumer.accept(filterQuery)), - // has no stored context - executes it in the tenant & user scope - () -> asActor(getAutoAssignmentInitiatedBy(filterQuery), () -> consumer.accept(filterQuery)) - ); - } catch (final RuntimeException ex) { - if (log.isDebugEnabled()) { - log.debug("Exception on forEachFilterWithAutoAssignDS execution for filter id {}. Continue with next filter query.", - filterQuery.getId(), ex); - } else { - log.error( - "Exception on forEachFilterWithAutoAssignDS execution for filter id {} and error message [{}]. Continue with next filter query.", - filterQuery.getId(), ex.getMessage()); + try { + filterQueries.forEach(filterQuery -> { + try { + filterQuery.getAccessControlContext().ifPresentOrElse( + // has stored context - executes it with it + context -> withSecurityContext(context, () -> consumer.accept(filterQuery)), + // has no stored context - executes it in the tenant & user scope + () -> asActor(getAutoAssignmentInitiatedBy(filterQuery), () -> consumer.accept(filterQuery))); + } catch (final RuntimeException ex) { + if (ex == LOCK_TIMEOUT_EXCEPTION) { + // expected - just stop processing further + throw ex; // throw in order to break + } + if (log.isDebugEnabled()) { + log.debug("Exception on forEachFilterWithAutoAssignDS execution for filter id {}. Continue with next filter query.", + filterQuery.getId(), ex); + } else { + log.error( + "Exception on forEachFilterWithAutoAssignDS execution for filter id {} and error message [{}]. Continue with next filter query.", + filterQuery.getId(), ex.getMessage()); + } } - } - }); + }); + } catch (final LockTimeoutException lte) { + break; // lock not found + } } while (filterQueries.hasNext() && (query = filterQueries.nextPageable()) != Pageable.unpaged()); } @@ -209,4 +290,8 @@ public class JpaAutoAssignExecutor implements AutoAssignExecutor { } log.debug("Auto assign check call for target filter query id {} for device {} finished", targetFilterQuery.getId(), controllerId); } + + private static String createAutoAssignmentLockKey(final String tenant) { + return tenant + "-auto-assign"; + } } \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/scheduler/JpaRolloutHandler.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/scheduler/JpaRolloutHandler.java index 4bf2c65cd..2c9ef2585 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/scheduler/JpaRolloutHandler.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/scheduler/JpaRolloutHandler.java @@ -9,6 +9,8 @@ */ package org.eclipse.hawkbit.repository.jpa.scheduler; +import static org.eclipse.hawkbit.tenancy.DefaultTenantConfiguration.TENANT_TAG; + import java.util.List; import java.util.Optional; import java.util.concurrent.TimeUnit; @@ -21,7 +23,6 @@ import org.eclipse.hawkbit.repository.RolloutExecutor; import org.eclipse.hawkbit.repository.RolloutHandler; import org.eclipse.hawkbit.repository.RolloutManagement; import org.eclipse.hawkbit.repository.jpa.utils.DeploymentHelper; -import org.eclipse.hawkbit.tenancy.DefaultTenantConfiguration; import org.springframework.integration.support.locks.LockRegistry; import org.springframework.transaction.PlatformTransactionManager; @@ -57,6 +58,8 @@ public class JpaRolloutHandler implements RolloutHandler { @Override public void handleAll() { + final long startNano = System.nanoTime(); + final List rollouts = rolloutManagement.findActiveRollouts(); if (rollouts.isEmpty()) { return; @@ -66,15 +69,13 @@ public class JpaRolloutHandler implements RolloutHandler { final Lock lock = lockRegistry.obtain(handlerId); if (!lock.tryLock()) { if (log.isTraceEnabled()) { - log.trace("Could not perform lock {}", lock); + log.trace("Could not obtain lock {}", lock); } return; } try { - log.debug("Trigger handling {} rollouts.", rollouts.size()); - - final long startNano = System.nanoTime(); + log.debug("Start handling {} rollouts.", rollouts.size()); rollouts.forEach(rolloutId -> { try { handleRolloutInNewTransaction(rolloutId, handlerId); @@ -82,16 +83,16 @@ public class JpaRolloutHandler implements RolloutHandler { log.error("Failed to process rollout with id {}", rolloutId, throwable); } }); - meterRegistry - .map(mReg -> mReg.timer("hawkbit.rollout.handler.all", DefaultTenantConfiguration.TENANT_TAG, AccessContext.tenant())) - .ifPresent(timer -> timer.record(System.nanoTime() - startNano, TimeUnit.NANOSECONDS)); - log.debug("Finished handling of the rollouts."); } finally { + lock.unlock(); + if (log.isTraceEnabled()) { log.trace("Unlock lock {}", lock); } - lock.unlock(); + meterRegistry // handle all rollouts of a tenant + .map(mReg -> mReg.timer("hawkbit.rollout.handle.all", TENANT_TAG, AccessContext.tenant())) + .ifPresent(timer -> timer.record(System.nanoTime() - startNano, TimeUnit.NANOSECONDS)); } } @@ -110,11 +111,9 @@ public class JpaRolloutHandler implements RolloutHandler { return 0L; }); - meterRegistry + meterRegistry // handle single rollout .map(mReg -> mReg.timer( - "hawkbit.rollout.handler", - DefaultTenantConfiguration.TENANT_TAG, AccessContext.tenant(), - "rollout", String.valueOf(rolloutId))) + "hawkbit.rollout.handle", TENANT_TAG, AccessContext.tenant(), "rollout", String.valueOf(rolloutId))) .ifPresent(timer -> timer.record(System.nanoTime() - startNano, TimeUnit.NANOSECONDS)); } } \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/scheduler/RolloutScheduler.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/scheduler/RolloutScheduler.java index 0ef68d840..7b323133f 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/scheduler/RolloutScheduler.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/scheduler/RolloutScheduler.java @@ -19,7 +19,6 @@ import io.micrometer.core.instrument.MeterRegistry; import lombok.extern.slf4j.Slf4j; import org.eclipse.hawkbit.repository.RolloutHandler; import org.eclipse.hawkbit.repository.SystemManagement; -import org.eclipse.hawkbit.tenancy.DefaultTenantConfiguration; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor; @@ -54,8 +53,8 @@ public class RolloutScheduler { */ @Scheduled(initialDelayString = PROP_SCHEDULER_DELAY_PLACEHOLDER, fixedDelayString = PROP_SCHEDULER_DELAY_PLACEHOLDER) public void runningRolloutScheduler() { - log.debug("rollout schedule checker has been triggered."); - final long startNano = java.lang.System.nanoTime(); + log.debug("Rollout scheduler has been triggered."); + final long startNano = System.nanoTime(); // run this code in system code privileged to have the necessary system code permission execute forEachTenant asSystem(() -> @@ -66,28 +65,21 @@ public class RolloutScheduler { if (rolloutTaskExecutor == null) { handleAll(tenant); } else { - rolloutTaskExecutor.submit(() -> asSystemAsTenant(tenant, () -> handleAll(tenant))); + rolloutTaskExecutor.execute(() -> asSystemAsTenant(tenant, () -> handleAll(tenant))); } })); - meterRegistry - .map(mReg -> mReg.timer("hawkbit.rollout.scheduler.all")) - .ifPresent(timer -> timer.record(java.lang.System.nanoTime() - startNano, TimeUnit.NANOSECONDS)); + meterRegistry // handle all tenants (some could be skipped if lock could not be obtained) + .map(mReg -> mReg.timer("hawkbit.rollout.scheduler")) + .ifPresent(timer -> timer.record(System.nanoTime() - startNano, TimeUnit.NANOSECONDS)); } private void handleAll(final String tenant) { - log.trace("Handling rollout for tenant: {}", tenant); - final long startNano = java.lang.System.nanoTime(); - + log.trace("Handling rollouts for tenant: {}", tenant); try { rolloutHandler.handleAll(); } catch (final Exception e) { log.error("Error processing rollout for tenant {}", tenant, e); } - - meterRegistry - .map(mReg -> mReg.timer("hawkbit.rollout.scheduler", DefaultTenantConfiguration.TENANT_TAG, tenant)) - .ifPresent(timer -> timer.record(java.lang.System.nanoTime() - startNano, TimeUnit.NANOSECONDS)); } - } \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/AutoAssignTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/AutoAssignTest.java index 47fbf74a3..faa783a28 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/AutoAssignTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/AutoAssignTest.java @@ -19,7 +19,7 @@ import static org.eclipse.hawkbit.repository.test.util.SecurityContextSwitch.cal import java.util.Optional; -import org.eclipse.hawkbit.repository.AutoAssignExecutor; +import org.eclipse.hawkbit.repository.AutoAssignHandler; import org.eclipse.hawkbit.repository.Identifiable; import org.eclipse.hawkbit.repository.TargetFilterQueryManagement; import org.eclipse.hawkbit.repository.TargetFilterQueryManagement.AutoAssignDistributionSetUpdate; @@ -27,34 +27,30 @@ import org.eclipse.hawkbit.repository.jpa.scheduler.AutoAssignScheduler; import org.eclipse.hawkbit.repository.model.TargetFilterQuery; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.integration.support.locks.LockRegistry; class AutoAssignTest extends AbstractAccessControllerManagementTest { @Autowired - AutoAssignExecutor autoAssignExecutor; - - @Autowired - LockRegistry lockRegistry; + AutoAssignHandler autoAssignHandler; @Test void verifyOnlyUpdatableTargetsArePartOfAutoAssignmentByScheduler() throws Exception { // auto assign scheduler apply stored access control context and the context is correctly applied verifyOnlyUpdatableTargetsArePartOfAutoAssignment( - () -> new AutoAssignScheduler(systemManagement, autoAssignExecutor, 1, lockRegistry, Optional.empty()).autoAssignScheduler()); + () -> new AutoAssignScheduler(systemManagement, autoAssignHandler, 1, Optional.empty()).autoAssignScheduler()); } @Test void verifyOnlyUpdatableTargetsArePartOfAutoAssignment() throws Exception { - verifyOnlyUpdatableTargetsArePartOfAutoAssignment(autoAssignExecutor::checkAllTargets); + verifyOnlyUpdatableTargetsArePartOfAutoAssignment(autoAssignHandler::handleAll); } @Test void verifyOnlyUpdatableTargetsWillGetAssignmentOnSingleCheck() throws Exception { verifyOnlyUpdatableTargetsArePartOfAutoAssignment(() -> { - autoAssignExecutor.checkSingleTarget(target1Type1.getControllerId()); - autoAssignExecutor.checkSingleTarget(target2Type2.getControllerId()); - autoAssignExecutor.checkSingleTarget(target3Type2.getControllerId()); + autoAssignHandler.handleSingleTarget(target1Type1.getControllerId()); + autoAssignHandler.handleSingleTarget(target2Type2.getControllerId()); + autoAssignHandler.handleSingleTarget(target3Type2.getControllerId()); }); } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/SecurityContextCtxTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/SecurityContextCtxTest.java index 1f7c0e185..40d281105 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/SecurityContextCtxTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/SecurityContextCtxTest.java @@ -21,7 +21,7 @@ import java.util.function.Supplier; import lombok.SneakyThrows; import org.eclipse.hawkbit.auth.SpPermission; import org.eclipse.hawkbit.context.AccessContext; -import org.eclipse.hawkbit.repository.AutoAssignExecutor; +import org.eclipse.hawkbit.repository.AutoAssignHandler; import org.eclipse.hawkbit.repository.jpa.AbstractJpaIntegrationTest; import org.eclipse.hawkbit.repository.model.Rollout; import org.eclipse.hawkbit.repository.model.TargetFilterQuery; @@ -45,7 +45,7 @@ class SecurityContextCtxTest extends AbstractJpaIntegrationTest { private static final Set AUTHORITIES = SpPermission.getAllAuthorities(); @Autowired - AutoAssignExecutor autoAssignExecutor; + AutoAssignHandler autoAssignHandler; /** * Verifies acm context is persisted when creating Rollout @@ -97,7 +97,7 @@ class SecurityContextCtxTest extends AbstractJpaIntegrationTest { try (final MockedStatic mocked = mockStatic(AccessContext.class, Mockito.CALLS_REAL_METHODS)) { withSecurityContext(userContext, testdataFactory::createTargetFilterWithTargetsAndActiveAutoAssignment); withSecurityContext(userContext, () -> { - autoAssignExecutor.checkAllTargets(); + autoAssignHandler.handleAll(); return null; }); @@ -118,7 +118,7 @@ class SecurityContextCtxTest extends AbstractJpaIntegrationTest { withSecurityContext(userContext, testdataFactory::createTargetFilterWithTargetsAndActiveAutoAssignment); withSecurityContext(userContext, () -> { - autoAssignExecutor.checkSingleTarget(targetManagement.findAll(Pageable.ofSize(1)).getContent().get(0).getControllerId()); + autoAssignHandler.handleSingleTarget(targetManagement.findAll(Pageable.ofSize(1)).getContent().get(0).getControllerId()); return null; }); diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/scheduler/AutoAssignExecutorIntTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/scheduler/AutoAssignHandlerIntTest.java similarity index 97% rename from hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/scheduler/AutoAssignExecutorIntTest.java rename to hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/scheduler/AutoAssignHandlerIntTest.java index 6c323e58a..9f63ad964 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/scheduler/AutoAssignExecutorIntTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/scheduler/AutoAssignHandlerIntTest.java @@ -44,18 +44,18 @@ import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Slice; /** - * Test class for {@link JpaAutoAssignExecutor}. + * Test class for {@link JpaAutoAssignHandler}. *

* Feature: Component Tests - Repository
* Story: Auto assign checker */ @SuppressWarnings("java:S6813") // constructor injects are not possible for test classes -class AutoAssignExecutorIntTest extends AbstractJpaIntegrationTest { +class AutoAssignHandlerIntTest extends AbstractJpaIntegrationTest { private static final String SPACE_AND_DESCRIPTION = " description"; @Autowired - private JpaAutoAssignExecutor autoAssignChecker; + private JpaAutoAssignHandler autoAssignChecker; @Autowired private DeploymentManagement deploymentManagement; @@ -81,7 +81,7 @@ class AutoAssignExecutorIntTest extends AbstractJpaIntegrationTest { targetFilterQueryManagement.updateAutoAssignDS( new AutoAssignDistributionSetUpdate(targetFilterQuery.getId()).ds(secondDistributionSet.getId())); // Run the check - autoAssignChecker.checkAllTargets(); + autoAssignChecker.handleAll(); // verify that manually created action is canceled and action // created from AutoAssign is running @@ -144,7 +144,7 @@ class AutoAssignExecutorIntTest extends AbstractJpaIntegrationTest { assertThat(targetManagement.countByRsqlAndNonDsAndCompatibleAndUpdatable(setA.getId(), targetFilterQuery.getQuery())).isEqualTo(15); // Run the check - autoAssignChecker.checkAllTargets(); + autoAssignChecker.handleAll(); verifyThatTargetsHaveDistributionSetAssignment(setA, targets.subList(5, 25), targetsCount); @@ -173,7 +173,7 @@ class AutoAssignExecutorIntTest extends AbstractJpaIntegrationTest { final int targetsCount = targets.size(); // Run the check - autoAssignChecker.checkSingleTarget(targets.get(0).getControllerId()); + autoAssignChecker.handleSingleTarget(targets.get(0).getControllerId()); verifyThatTargetsHaveDistributionSetAssignment(toAssignDs, targets.subList(0, 1), targetsCount); @@ -204,7 +204,7 @@ class AutoAssignExecutorIntTest extends AbstractJpaIntegrationTest { targetDsAIdPref.concat(SPACE_AND_DESCRIPTION)); // Run the check - autoAssignChecker.checkAllTargets(); + autoAssignChecker.handleAll(); verifyThatTargetsHaveDistributionSetAssignedAndActionStatus(distributionSet, targets, expectedStatus); } @@ -232,7 +232,7 @@ class AutoAssignExecutorIntTest extends AbstractJpaIntegrationTest { final List targets = testdataFactory.createTargets(25); // Run the check - autoAssignChecker.checkSingleTarget(targets.get(0).getControllerId()); + autoAssignChecker.handleSingleTarget(targets.get(0).getControllerId()); verifyThatTargetsHaveDistributionSetAssignedAndActionStatus(toAssignDs, targets.subList(0, 1), expectedStatus); verifyThatTargetsNotHaveDistributionSetAssignment(targets.subList(1, 25)); @@ -281,7 +281,7 @@ class AutoAssignExecutorIntTest extends AbstractJpaIntegrationTest { verifyThatTargetsHaveDistributionSetAssignment(setB, targetsF.subList(0, 5), targetsCount); // Run the check - autoAssignChecker.checkAllTargets(); + autoAssignChecker.handleAll(); // first 5 targets of the fail group should still have setB verifyThatTargetsHaveDistributionSetAssignment(setB, targetsF.subList(0, 5), targetsCount); @@ -309,7 +309,7 @@ class AutoAssignExecutorIntTest extends AbstractJpaIntegrationTest { final int targetsCount = targetsA.size() + targetsB.size() + targetsC.size(); - autoAssignChecker.checkAllTargets(); + autoAssignChecker.handleAll(); verifyThatTargetsHaveDistributionSetAssignment(distributionSet, targetsA, targetsCount); verifyThatTargetsHaveDistributionSetAssignment(distributionSet, targetsB, targetsCount); @@ -334,7 +334,7 @@ class AutoAssignExecutorIntTest extends AbstractJpaIntegrationTest { .name("a").query("name==*").autoAssignDistributionSet(ds).autoAssignWeight(weight) .build()); testdataFactory.createTargets(amountOfTargets); - autoAssignChecker.checkAllTargets(); + autoAssignChecker.handleAll(); final List actions = deploymentManagement.findActionsAll(PAGE).getContent(); assertThat(actions) @@ -353,7 +353,7 @@ class AutoAssignExecutorIntTest extends AbstractJpaIntegrationTest { enableMultiAssignments(); testdataFactory.createTargets(amountOfTargets); - autoAssignChecker.checkAllTargets(); + autoAssignChecker.handleAll(); final List actions = deploymentManagement.findActionsAll(PAGE).getContent(); assertThat(actions) @@ -404,7 +404,7 @@ class AutoAssignExecutorIntTest extends AbstractJpaIntegrationTest { testFilter.getQuery()); assertThat(compatibleCount).isEqualTo(compatibleTargets.size()); - autoAssignChecker.checkAllTargets(); + autoAssignChecker.handleAll(); final List actions = deploymentManagement.findActionsAll(Pageable.unpaged()).getContent(); assertThat(actions).hasSize(compatibleTargets.size()); diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/scheduler/AutoAssignExecutorTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/scheduler/AutoAssignHandlerTest.java similarity index 77% rename from hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/scheduler/AutoAssignExecutorTest.java rename to hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/scheduler/AutoAssignHandlerTest.java index eb2f430cd..8db7361f8 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/scheduler/AutoAssignExecutorTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/scheduler/AutoAssignHandlerTest.java @@ -9,6 +9,7 @@ */ package org.eclipse.hawkbit.repository.jpa.scheduler; +import static org.assertj.core.api.Assertions.assertThatNoException; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; @@ -17,9 +18,12 @@ import static org.mockito.Mockito.when; import java.util.Arrays; import java.util.List; +import java.util.Optional; import java.util.UUID; import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.locks.Lock; +import org.assertj.core.api.Assertions; import org.eclipse.hawkbit.repository.DeploymentManagement; import org.eclipse.hawkbit.repository.TargetFilterQueryManagement; import org.eclipse.hawkbit.repository.TargetManagement; @@ -35,6 +39,7 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.data.domain.SliceImpl; +import org.springframework.integration.support.locks.LockRegistry; import org.springframework.transaction.PlatformTransactionManager; /** @@ -42,7 +47,7 @@ import org.springframework.transaction.PlatformTransactionManager; * Story: Auto assign checker */ @ExtendWith(MockitoExtension.class) -class AutoAssignExecutorTest { +class AutoAssignHandlerTest { @Mock private TargetFilterQueryManagement targetFilterQueryManagement; @@ -53,19 +58,36 @@ class AutoAssignExecutorTest { @Mock private PlatformTransactionManager transactionManager; - private JpaAutoAssignExecutor autoAssignChecker; + @Mock + LockRegistry lockRegistry; + + private JpaAutoAssignHandler autoAssignHandler; @BeforeEach void before() { - autoAssignChecker = new JpaAutoAssignExecutor( - targetFilterQueryManagement, targetManagement, deploymentManagement, transactionManager); + autoAssignHandler = new JpaAutoAssignHandler( + targetFilterQueryManagement, targetManagement, deploymentManagement, transactionManager, lockRegistry, Optional.empty()); } /** * Single device check triggers update for matching auto assignment filter. */ @Test - void checkForDevice() { + void failToLockInHandleAll() { + final Lock lock = mock(Lock.class); + when(lock.tryLock()).thenReturn(false); + when(lockRegistry.obtain(any())).thenReturn(lock); + final TargetFilterQuery matching = mock(TargetFilterQuery.class); + when(targetFilterQueryManagement.findWithAutoAssignDS(any())).thenReturn(new SliceImpl<>(Arrays.asList(matching))); + + assertThatNoException().isThrownBy(autoAssignHandler::handleAll); + } + + /** + * Single device check triggers update for matching auto assignment filter. + */ + @Test + void handleSingleTarget() { final String target = getRandomString(); final long ds = getRandomLong(); final TargetFilterQuery matching = mockFilterQuery(ds); @@ -75,7 +97,7 @@ class AutoAssignExecutorTest { when(targetManagement.isTargetMatchingQueryAndDSNotAssignedAndCompatibleAndUpdatable(target, ds, notMatching.getQuery())) .thenReturn(false); - autoAssignChecker.checkSingleTarget(target); + autoAssignHandler.handleSingleTarget(target); verify(deploymentManagement).assignDistributionSets(Mockito.argThat(deployReqMatcher(target, ds)), any()); Mockito.verifyNoMoreInteractions(deploymentManagement); diff --git a/hawkbit-rest/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/SecurityManagedConfiguration.java b/hawkbit-rest/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/SecurityManagedConfiguration.java index 1a8f8d29e..dd11e6c4c 100644 --- a/hawkbit-rest/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/SecurityManagedConfiguration.java +++ b/hawkbit-rest/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/SecurityManagedConfiguration.java @@ -41,13 +41,10 @@ import org.springframework.util.CollectionUtils; @PropertySource("classpath:hawkbit-security-defaults.properties") public class SecurityManagedConfiguration { - public static final String ANONYMOUS_CONTROLLER_SECURITY_ENABLED_SHOULD_ONLY_BE_USED_FOR_DEVELOPMENT_PURPOSES = """ - ****************** - ** Anonymous controller security enabled, should only be used for development purposes ** - ******************"""; public static final int DOS_FILTER_ORDER = -200; - public static FilterRegistrationBean dosFilter(final Collection includeAntPaths, + public static FilterRegistrationBean dosFilter( + final Collection includeAntPaths, final HawkbitSecurityProperties.Dos.Filter filterProperties, final HawkbitSecurityProperties.Clients clientProperties) { final FilterRegistrationBean filterRegBean = new FilterRegistrationBean<>(); @@ -120,4 +117,4 @@ public class SecurityManagedConfiguration { return super.getFirewalledRequest(request); } } -} +} \ No newline at end of file