From 6d9db434668775516f10743ea0882dea4e83ca86 Mon Sep 17 00:00:00 2001 From: Pavel Silin Date: Mon, 8 Jul 2024 15:34:47 +0200 Subject: [PATCH] Cleanup and tests --- .../epam/pipeline/acl/run/RunApiService.java | 2 +- .../entity/utils/TwoBoundaryLimit.java | 8 ++++ .../PipelineRunDockerOperationManager.java | 32 ++++++++----- .../manager/preference/SystemPreferences.java | 11 +++-- ...PipelineRunDockerOperationManagerTest.java | 48 +++++++++++++++++++ 5 files changed, 83 insertions(+), 18 deletions(-) diff --git a/api/src/main/java/com/epam/pipeline/acl/run/RunApiService.java b/api/src/main/java/com/epam/pipeline/acl/run/RunApiService.java index bcde8aff5a..79c274e4f9 100644 --- a/api/src/main/java/com/epam/pipeline/acl/run/RunApiService.java +++ b/api/src/main/java/com/epam/pipeline/acl/run/RunApiService.java @@ -328,7 +328,7 @@ public void prolongIdleRun(Long runId) { @PreAuthorize(RUN_ID_READ) public CommitRunConditions getCommitRunCheckResult(final Long runId) { return CommitRunConditions.builder() - .containerSize(pipelineRunDockerOperationManager.getContainerSize(runId)) + .containerSize(pipelineRunDockerOperationManager.checkContainerSizeForCommit(runId)) .enoughSpace(pipelineRunDockerOperationManager.checkFreeSpaceAvailable(runId)) .build(); } diff --git a/api/src/main/java/com/epam/pipeline/entity/utils/TwoBoundaryLimit.java b/api/src/main/java/com/epam/pipeline/entity/utils/TwoBoundaryLimit.java index 840aa117d2..0f6ca5bbfd 100644 --- a/api/src/main/java/com/epam/pipeline/entity/utils/TwoBoundaryLimit.java +++ b/api/src/main/java/com/epam/pipeline/entity/utils/TwoBoundaryLimit.java @@ -25,4 +25,12 @@ public class TwoBoundaryLimit { private Long soft; private Long hard; + + public boolean isSoftLimitDefined() { + return soft != null && soft > 0; + } + + public boolean isHardLimitDefined() { + return hard != null && hard > 0; + } } diff --git a/api/src/main/java/com/epam/pipeline/manager/pipeline/PipelineRunDockerOperationManager.java b/api/src/main/java/com/epam/pipeline/manager/pipeline/PipelineRunDockerOperationManager.java index 7e742cb51a..d08bad6131 100644 --- a/api/src/main/java/com/epam/pipeline/manager/pipeline/PipelineRunDockerOperationManager.java +++ b/api/src/main/java/com/epam/pipeline/manager/pipeline/PipelineRunDockerOperationManager.java @@ -122,11 +122,11 @@ public Long getContainerLayers(final Long id) { return dockerContainerOperationManager.getContainerLayers(pipelineRun); } - public ConditionCheck getContainerSize(final Long id) { + public ConditionCheck checkContainerSizeForCommit(final Long id) { final PipelineRun pipelineRun = pipelineRunDao.loadPipelineRun(id); final long containerSize = dockerContainerOperationManager.getContainerSize(pipelineRun); final TwoBoundaryLimit limits = preferenceManager.getPreference(SystemPreferences.COMMIT_TOOL_SIZE_LIMITS); - final Pair containerSizeCheck = getCommitRunCheckStatus(containerSize, limits); + final Pair containerSizeCheck = mapContainerSizeOnLimits(containerSize, limits); return ConditionCheck.builder() .result(containerSizeCheck.getKey()) .message(containerSizeCheck.getValue()) @@ -224,6 +224,23 @@ public void rerunPauseAndResume() { ListUtils.emptyIfNull(resumingRuns).forEach(this::rerunResumeRun); } + static Pair mapContainerSizeOnLimits( + final long containerSize, final TwoBoundaryLimit containerSizeLimits) { + if (containerSizeLimits == null) { + return Pair.create(ConditionCheck.Result.OK, null); + } + + if (containerSizeLimits.isHardLimitDefined() && containerSize >= containerSizeLimits.getHard()) { + return Pair.create(ConditionCheck.Result.FAIL, "Container is too big to commit!"); + } else { + if (containerSizeLimits.isSoftLimitDefined() && containerSize >= containerSizeLimits.getSoft()) { + return Pair.create(ConditionCheck.Result.WARN, + "Container image size may be quite big! Are you sure you want to commit?"); + } + } + return Pair.create(ConditionCheck.Result.OK, null); + } + @SuppressWarnings("PMD.AvoidCatchingGenericException") private void rerunPauseRun(final PipelineRun run) { try { @@ -315,15 +332,4 @@ private long getDiskSpaceAvailable(final PipelineRun pipelineRun) { return Long.MAX_VALUE; } } - - private static Pair getCommitRunCheckStatus( - final long containerSize, final TwoBoundaryLimit containerSizeLimits) { - if (containerSizeLimits == null || containerSize <= containerSizeLimits.getSoft()) { - return Pair.create(ConditionCheck.Result.OK, null); - } else if (containerSize <= containerSizeLimits.getHard()) { - return Pair.create(ConditionCheck.Result.WARN, "Container image size may be quite big!"); - } else { - return Pair.create(ConditionCheck.Result.FAIL, "Container is too big for commit."); - } - } } diff --git a/api/src/main/java/com/epam/pipeline/manager/preference/SystemPreferences.java b/api/src/main/java/com/epam/pipeline/manager/preference/SystemPreferences.java index c824158445..f49cf6f620 100644 --- a/api/src/main/java/com/epam/pipeline/manager/preference/SystemPreferences.java +++ b/api/src/main/java/com/epam/pipeline/manager/preference/SystemPreferences.java @@ -169,8 +169,10 @@ public class SystemPreferences { public static final IntPreference COMMIT_MAX_LAYERS = new IntPreference("commit.max.layers", 127, COMMIT_GROUP, isGreaterThan(0)); public static final ObjectPreference COMMIT_TOOL_SIZE_LIMITS = new ObjectPreference<>( - "commit.container.size.limits", null, new TypeReference() {}, - COMMIT_GROUP, isNullOrValidJson(new TypeReference() {})); + "commit.container.size.limits", TwoBoundaryLimit.builder().soft(0L).hard(0L).build(), + new TypeReference() {}, COMMIT_GROUP, + isNullOrValidJson(new TypeReference() {}), true + ); public static final IntPreference GET_CONTAINER_SIZE_TIMEOUT = new IntPreference("get.container.size.timeout", 600, COMMIT_GROUP, isGreaterThan(0)); @@ -732,8 +734,9 @@ public class SystemPreferences { "launch.run.reschedule.enabled", true, LAUNCH_GROUP, pass); public static final ObjectPreference RUN_TOOL_SIZE_LIMITS = new ObjectPreference<>( - "launch.tool.size.limits", null, new TypeReference() {}, - LAUNCH_GROUP, isNullOrValidJson(new TypeReference() {})); + "launch.tool.size.limits", TwoBoundaryLimit.builder().soft(0L).hard(0L).build(), + new TypeReference() {}, LAUNCH_GROUP, + isNullOrValidJson(new TypeReference() {}), true); /** * Specifies a comma-separated list of environment variables that should be inherited by DIND containers diff --git a/api/src/test/java/com/epam/pipeline/manager/pipeline/PipelineRunDockerOperationManagerTest.java b/api/src/test/java/com/epam/pipeline/manager/pipeline/PipelineRunDockerOperationManagerTest.java index 10402e0556..8bb5366104 100644 --- a/api/src/test/java/com/epam/pipeline/manager/pipeline/PipelineRunDockerOperationManagerTest.java +++ b/api/src/test/java/com/epam/pipeline/manager/pipeline/PipelineRunDockerOperationManagerTest.java @@ -23,6 +23,8 @@ import com.epam.pipeline.entity.pipeline.TaskStatus; import com.epam.pipeline.entity.pipeline.Tool; import com.epam.pipeline.entity.pipeline.run.RunStatus; +import com.epam.pipeline.entity.utils.ConditionCheck; +import com.epam.pipeline.entity.utils.TwoBoundaryLimit; import com.epam.pipeline.manager.ObjectCreatorUtils; import com.epam.pipeline.manager.cluster.performancemonitoring.UsageMonitoringManager; import com.epam.pipeline.manager.docker.DockerContainerOperationManager; @@ -30,6 +32,7 @@ import com.epam.pipeline.manager.preference.PreferenceManager; import com.epam.pipeline.manager.quota.RunLimitsService; import org.apache.commons.lang.time.DateUtils; +import org.junit.Assert; import org.junit.Test; import java.time.LocalDateTime; @@ -156,6 +159,51 @@ public void shouldNotFailResumeIfPauseFailed() { verify(dockerContainerOperationManager).resumeRun(runToResume, TEST_NAMES); } + @Test + public void shouldCorrectlyCalculateContainerSizeAgainstLimits() { + TwoBoundaryLimit limit = TwoBoundaryLimit.builder().soft(Long.MAX_VALUE / 2).hard(Long.MAX_VALUE).build(); + Assert.assertEquals( + ConditionCheck.Result.OK, + PipelineRunDockerOperationManager.mapContainerSizeOnLimits(Long.MAX_VALUE / 4, limit).getKey() + ); + + limit = TwoBoundaryLimit.builder().soft(Long.MAX_VALUE / 2).hard(Long.MAX_VALUE).build(); + Assert.assertEquals( + ConditionCheck.Result.WARN, + PipelineRunDockerOperationManager.mapContainerSizeOnLimits(Long.MAX_VALUE / 2 + 1, limit).getKey() + ); + + limit = TwoBoundaryLimit.builder().soft(Long.MAX_VALUE / 2).hard(Long.MAX_VALUE - 1).build(); + Assert.assertEquals( + ConditionCheck.Result.FAIL, + PipelineRunDockerOperationManager.mapContainerSizeOnLimits(Long.MAX_VALUE, limit).getKey() + ); + + limit = TwoBoundaryLimit.builder().hard(Long.MAX_VALUE).build(); + Assert.assertEquals( + ConditionCheck.Result.OK, + PipelineRunDockerOperationManager.mapContainerSizeOnLimits(Long.MAX_VALUE - 1, limit).getKey() + ); + + limit = TwoBoundaryLimit.builder().soft(0L).hard(Long.MAX_VALUE).build(); + Assert.assertEquals( + ConditionCheck.Result.OK, + PipelineRunDockerOperationManager.mapContainerSizeOnLimits(Long.MAX_VALUE - 1, limit).getKey() + ); + + limit = TwoBoundaryLimit.builder().hard(Long.MAX_VALUE / 2).build(); + Assert.assertEquals( + ConditionCheck.Result.OK, + PipelineRunDockerOperationManager.mapContainerSizeOnLimits(Long.MAX_VALUE / 4, limit).getKey() + ); + + limit = TwoBoundaryLimit.builder().hard(Long.MAX_VALUE / 2).build(); + Assert.assertEquals( + ConditionCheck.Result.FAIL, + PipelineRunDockerOperationManager.mapContainerSizeOnLimits(Long.MAX_VALUE / 2 + 1, limit).getKey() + ); + } + private PipelineRun pipelineRun() { return ObjectCreatorUtils.createPipelineRun(RUN_ID, null, null, TEST_ID); }