From 7869865ccc3ed1b0db8089287c4615dc37d56597 Mon Sep 17 00:00:00 2001 From: rtjd6554 <174791724+rtjd6554@users.noreply.github.com> Date: Tue, 17 Sep 2024 10:10:14 +0100 Subject: [PATCH 01/22] Relocate the deploy config into the properties --- .../main/java/sleeper/clients/deploy/DeployNewInstance.java | 4 ++-- .../sleeper/clients/deploy/PopulateInstanceProperties.java | 2 +- .../{ => properties}/deploy/DeployInstanceConfiguration.java | 2 +- .../deploy/DeployInstanceConfigurationFromTemplates.java | 2 +- .../configuration/deploy/DeployInstanceConfigurationIT.java | 2 ++ .../sleeper/systemtest/drivers/cdk/DeployNewTestInstance.java | 2 +- .../systemtest/drivers/instance/AwsSleeperInstanceDriver.java | 2 +- .../drivers/testutil/LocalStackSleeperInstanceDriver.java | 2 +- .../systemtest/drivers/testutil/LocalStackTestInstance.java | 2 +- .../systemtest/dsl/instance/DeployedSleeperInstance.java | 2 +- .../systemtest/dsl/instance/SleeperInstanceDriver.java | 2 +- .../dsl/instance/SystemTestInstanceConfiguration.java | 2 +- .../sleeper/systemtest/dsl/instance/SystemTestParameters.java | 2 +- .../systemtest/dsl/instance/SleeperInstanceTablesTest.java | 2 +- .../sleeper/systemtest/dsl/testutil/InMemoryTestInstance.java | 2 +- .../dsl/testutil/drivers/InMemorySleeperInstanceDriver.java | 2 +- .../sleeper/systemtest/suite/fixtures/SystemTestInstance.java | 2 +- .../systemtest/suite/fixtures/SystemTestInstanceTest.java | 2 +- 18 files changed, 20 insertions(+), 18 deletions(-) rename java/configuration/src/main/java/sleeper/configuration/{ => properties}/deploy/DeployInstanceConfiguration.java (98%) rename java/configuration/src/main/java/sleeper/configuration/{ => properties}/deploy/DeployInstanceConfigurationFromTemplates.java (99%) diff --git a/java/clients/src/main/java/sleeper/clients/deploy/DeployNewInstance.java b/java/clients/src/main/java/sleeper/clients/deploy/DeployNewInstance.java index b9155c9f2a..d45d0effc4 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/DeployNewInstance.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/DeployNewInstance.java @@ -35,9 +35,9 @@ import sleeper.clients.util.EcrRepositoryCreator; import sleeper.clients.util.cdk.CdkCommand; import sleeper.clients.util.cdk.InvokeCdkForInstance; -import sleeper.configuration.deploy.DeployInstanceConfiguration; -import sleeper.configuration.deploy.DeployInstanceConfigurationFromTemplates; import sleeper.configuration.properties.SleeperPropertiesValidationReporter; +import sleeper.configuration.properties.deploy.DeployInstanceConfiguration; +import sleeper.configuration.properties.deploy.DeployInstanceConfigurationFromTemplates; import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.local.SaveLocalProperties; import sleeper.configuration.properties.table.TableProperties; diff --git a/java/clients/src/main/java/sleeper/clients/deploy/PopulateInstanceProperties.java b/java/clients/src/main/java/sleeper/clients/deploy/PopulateInstanceProperties.java index 39a3a0321f..78018935ad 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/PopulateInstanceProperties.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/PopulateInstanceProperties.java @@ -20,8 +20,8 @@ import com.amazonaws.services.securitytoken.model.GetCallerIdentityRequest; import software.amazon.awssdk.regions.providers.AwsRegionProvider; -import sleeper.configuration.deploy.DeployInstanceConfiguration; import sleeper.configuration.properties.SleeperScheduleRule; +import sleeper.configuration.properties.deploy.DeployInstanceConfiguration; import sleeper.configuration.properties.instance.InstanceProperties; import java.nio.file.Path; diff --git a/java/configuration/src/main/java/sleeper/configuration/deploy/DeployInstanceConfiguration.java b/java/configuration/src/main/java/sleeper/configuration/properties/deploy/DeployInstanceConfiguration.java similarity index 98% rename from java/configuration/src/main/java/sleeper/configuration/deploy/DeployInstanceConfiguration.java rename to java/configuration/src/main/java/sleeper/configuration/properties/deploy/DeployInstanceConfiguration.java index 655698295e..5486bad2eb 100644 --- a/java/configuration/src/main/java/sleeper/configuration/deploy/DeployInstanceConfiguration.java +++ b/java/configuration/src/main/java/sleeper/configuration/properties/deploy/DeployInstanceConfiguration.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package sleeper.configuration.deploy; +package sleeper.configuration.properties.deploy; import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.table.TableProperties; diff --git a/java/configuration/src/main/java/sleeper/configuration/deploy/DeployInstanceConfigurationFromTemplates.java b/java/configuration/src/main/java/sleeper/configuration/properties/deploy/DeployInstanceConfigurationFromTemplates.java similarity index 99% rename from java/configuration/src/main/java/sleeper/configuration/deploy/DeployInstanceConfigurationFromTemplates.java rename to java/configuration/src/main/java/sleeper/configuration/properties/deploy/DeployInstanceConfigurationFromTemplates.java index 81f6b04e1a..e4f672d009 100644 --- a/java/configuration/src/main/java/sleeper/configuration/deploy/DeployInstanceConfigurationFromTemplates.java +++ b/java/configuration/src/main/java/sleeper/configuration/properties/deploy/DeployInstanceConfigurationFromTemplates.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package sleeper.configuration.deploy; +package sleeper.configuration.properties.deploy; import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.local.LoadLocalProperties; diff --git a/java/configuration/src/test/java/sleeper/configuration/deploy/DeployInstanceConfigurationIT.java b/java/configuration/src/test/java/sleeper/configuration/deploy/DeployInstanceConfigurationIT.java index 1c8e27befb..bc0f8024f4 100644 --- a/java/configuration/src/test/java/sleeper/configuration/deploy/DeployInstanceConfigurationIT.java +++ b/java/configuration/src/test/java/sleeper/configuration/deploy/DeployInstanceConfigurationIT.java @@ -21,6 +21,8 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import sleeper.configuration.properties.deploy.DeployInstanceConfiguration; +import sleeper.configuration.properties.deploy.DeployInstanceConfigurationFromTemplates; import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.table.TableProperties; import sleeper.core.schema.SchemaSerDe; diff --git a/java/system-test/system-test-drivers/src/main/java/sleeper/systemtest/drivers/cdk/DeployNewTestInstance.java b/java/system-test/system-test-drivers/src/main/java/sleeper/systemtest/drivers/cdk/DeployNewTestInstance.java index 911d5b73f7..c38260f9b5 100644 --- a/java/system-test/system-test-drivers/src/main/java/sleeper/systemtest/drivers/cdk/DeployNewTestInstance.java +++ b/java/system-test/system-test-drivers/src/main/java/sleeper/systemtest/drivers/cdk/DeployNewTestInstance.java @@ -18,7 +18,7 @@ import sleeper.clients.deploy.DeployNewInstance; import sleeper.clients.deploy.StackDockerImage; import sleeper.clients.util.cdk.InvokeCdkForInstance; -import sleeper.configuration.deploy.DeployInstanceConfigurationFromTemplates; +import sleeper.configuration.properties.deploy.DeployInstanceConfigurationFromTemplates; import java.io.IOException; import java.nio.file.Path; diff --git a/java/system-test/system-test-drivers/src/main/java/sleeper/systemtest/drivers/instance/AwsSleeperInstanceDriver.java b/java/system-test/system-test-drivers/src/main/java/sleeper/systemtest/drivers/instance/AwsSleeperInstanceDriver.java index 0588cc7ee5..74100654eb 100644 --- a/java/system-test/system-test-drivers/src/main/java/sleeper/systemtest/drivers/instance/AwsSleeperInstanceDriver.java +++ b/java/system-test/system-test-drivers/src/main/java/sleeper/systemtest/drivers/instance/AwsSleeperInstanceDriver.java @@ -33,7 +33,7 @@ import sleeper.clients.util.ClientUtils; import sleeper.clients.util.cdk.CdkCommand; import sleeper.clients.util.cdk.InvokeCdkForInstance; -import sleeper.configuration.deploy.DeployInstanceConfiguration; +import sleeper.configuration.properties.deploy.DeployInstanceConfiguration; import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.table.TableProperties; import sleeper.systemtest.drivers.util.SystemTestClients; diff --git a/java/system-test/system-test-drivers/src/test/java/sleeper/systemtest/drivers/testutil/LocalStackSleeperInstanceDriver.java b/java/system-test/system-test-drivers/src/test/java/sleeper/systemtest/drivers/testutil/LocalStackSleeperInstanceDriver.java index 41eeea9cf4..2197a3be5a 100644 --- a/java/system-test/system-test-drivers/src/test/java/sleeper/systemtest/drivers/testutil/LocalStackSleeperInstanceDriver.java +++ b/java/system-test/system-test-drivers/src/test/java/sleeper/systemtest/drivers/testutil/LocalStackSleeperInstanceDriver.java @@ -21,7 +21,7 @@ import org.slf4j.LoggerFactory; import sleeper.clients.docker.DeployDockerInstance; -import sleeper.configuration.deploy.DeployInstanceConfiguration; +import sleeper.configuration.properties.deploy.DeployInstanceConfiguration; import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.table.TableProperties; import sleeper.core.SleeperVersion; diff --git a/java/system-test/system-test-drivers/src/test/java/sleeper/systemtest/drivers/testutil/LocalStackTestInstance.java b/java/system-test/system-test-drivers/src/test/java/sleeper/systemtest/drivers/testutil/LocalStackTestInstance.java index 596871080e..61affce64a 100644 --- a/java/system-test/system-test-drivers/src/test/java/sleeper/systemtest/drivers/testutil/LocalStackTestInstance.java +++ b/java/system-test/system-test-drivers/src/test/java/sleeper/systemtest/drivers/testutil/LocalStackTestInstance.java @@ -15,7 +15,7 @@ */ package sleeper.systemtest.drivers.testutil; -import sleeper.configuration.deploy.DeployInstanceConfiguration; +import sleeper.configuration.properties.deploy.DeployInstanceConfiguration; import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.table.TableProperties; import sleeper.core.schema.Field; diff --git a/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/DeployedSleeperInstance.java b/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/DeployedSleeperInstance.java index de677b506b..86dadb130f 100644 --- a/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/DeployedSleeperInstance.java +++ b/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/DeployedSleeperInstance.java @@ -19,9 +19,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import sleeper.configuration.deploy.DeployInstanceConfiguration; import sleeper.configuration.properties.SleeperProperties; import sleeper.configuration.properties.SleeperProperty; +import sleeper.configuration.properties.deploy.DeployInstanceConfiguration; import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.instance.UserDefinedInstanceProperty; import sleeper.configuration.properties.table.TableProperties; diff --git a/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/SleeperInstanceDriver.java b/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/SleeperInstanceDriver.java index 6fcb56fd1b..a2012a8b18 100644 --- a/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/SleeperInstanceDriver.java +++ b/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/SleeperInstanceDriver.java @@ -16,7 +16,7 @@ package sleeper.systemtest.dsl.instance; -import sleeper.configuration.deploy.DeployInstanceConfiguration; +import sleeper.configuration.properties.deploy.DeployInstanceConfiguration; import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.table.TableProperties; diff --git a/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/SystemTestInstanceConfiguration.java b/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/SystemTestInstanceConfiguration.java index 30d08970b3..c6cf237c76 100644 --- a/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/SystemTestInstanceConfiguration.java +++ b/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/SystemTestInstanceConfiguration.java @@ -16,7 +16,7 @@ package sleeper.systemtest.dsl.instance; -import sleeper.configuration.deploy.DeployInstanceConfiguration; +import sleeper.configuration.properties.deploy.DeployInstanceConfiguration; import sleeper.configuration.properties.instance.InstanceProperties; import java.util.function.Supplier; diff --git a/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/SystemTestParameters.java b/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/SystemTestParameters.java index db648c6f9f..3a575aaa43 100644 --- a/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/SystemTestParameters.java +++ b/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/SystemTestParameters.java @@ -16,7 +16,7 @@ package sleeper.systemtest.dsl.instance; -import sleeper.configuration.deploy.DeployInstanceConfiguration; +import sleeper.configuration.properties.deploy.DeployInstanceConfiguration; import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.table.TableProperties; import sleeper.core.schema.Schema; diff --git a/java/system-test/system-test-dsl/src/test/java/sleeper/systemtest/dsl/instance/SleeperInstanceTablesTest.java b/java/system-test/system-test-dsl/src/test/java/sleeper/systemtest/dsl/instance/SleeperInstanceTablesTest.java index 83497a10d2..c775d4bd7b 100644 --- a/java/system-test/system-test-dsl/src/test/java/sleeper/systemtest/dsl/instance/SleeperInstanceTablesTest.java +++ b/java/system-test/system-test-dsl/src/test/java/sleeper/systemtest/dsl/instance/SleeperInstanceTablesTest.java @@ -21,8 +21,8 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -import sleeper.configuration.deploy.DeployInstanceConfiguration; import sleeper.configuration.properties.SleeperPropertiesInvalidException; +import sleeper.configuration.properties.deploy.DeployInstanceConfiguration; import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.table.TableProperties; import sleeper.core.partition.PartitionTree; diff --git a/java/system-test/system-test-dsl/src/test/java/sleeper/systemtest/dsl/testutil/InMemoryTestInstance.java b/java/system-test/system-test-dsl/src/test/java/sleeper/systemtest/dsl/testutil/InMemoryTestInstance.java index e162017854..14b6b9427f 100644 --- a/java/system-test/system-test-dsl/src/test/java/sleeper/systemtest/dsl/testutil/InMemoryTestInstance.java +++ b/java/system-test/system-test-dsl/src/test/java/sleeper/systemtest/dsl/testutil/InMemoryTestInstance.java @@ -16,7 +16,7 @@ package sleeper.systemtest.dsl.testutil; -import sleeper.configuration.deploy.DeployInstanceConfiguration; +import sleeper.configuration.properties.deploy.DeployInstanceConfiguration; import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.table.TableProperties; import sleeper.core.schema.Field; diff --git a/java/system-test/system-test-dsl/src/test/java/sleeper/systemtest/dsl/testutil/drivers/InMemorySleeperInstanceDriver.java b/java/system-test/system-test-dsl/src/test/java/sleeper/systemtest/dsl/testutil/drivers/InMemorySleeperInstanceDriver.java index 47ed8ecef1..154219fe3d 100644 --- a/java/system-test/system-test-dsl/src/test/java/sleeper/systemtest/dsl/testutil/drivers/InMemorySleeperInstanceDriver.java +++ b/java/system-test/system-test-dsl/src/test/java/sleeper/systemtest/dsl/testutil/drivers/InMemorySleeperInstanceDriver.java @@ -16,7 +16,7 @@ package sleeper.systemtest.dsl.testutil.drivers; -import sleeper.configuration.deploy.DeployInstanceConfiguration; +import sleeper.configuration.properties.deploy.DeployInstanceConfiguration; import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.table.TableProperties; import sleeper.systemtest.dsl.instance.SleeperInstanceDriver; diff --git a/java/system-test/system-test-suite/src/main/java/sleeper/systemtest/suite/fixtures/SystemTestInstance.java b/java/system-test/system-test-suite/src/main/java/sleeper/systemtest/suite/fixtures/SystemTestInstance.java index 435af75a98..7320b86c5c 100644 --- a/java/system-test/system-test-suite/src/main/java/sleeper/systemtest/suite/fixtures/SystemTestInstance.java +++ b/java/system-test/system-test-suite/src/main/java/sleeper/systemtest/suite/fixtures/SystemTestInstance.java @@ -16,7 +16,7 @@ package sleeper.systemtest.suite.fixtures; -import sleeper.configuration.deploy.DeployInstanceConfiguration; +import sleeper.configuration.properties.deploy.DeployInstanceConfiguration; import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.table.TableProperties; import sleeper.configuration.properties.validation.EmrInstanceArchitecture; diff --git a/java/system-test/system-test-suite/src/test/java/sleeper/systemtest/suite/fixtures/SystemTestInstanceTest.java b/java/system-test/system-test-suite/src/test/java/sleeper/systemtest/suite/fixtures/SystemTestInstanceTest.java index 19a5748080..f10c7ca70d 100644 --- a/java/system-test/system-test-suite/src/test/java/sleeper/systemtest/suite/fixtures/SystemTestInstanceTest.java +++ b/java/system-test/system-test-suite/src/test/java/sleeper/systemtest/suite/fixtures/SystemTestInstanceTest.java @@ -18,7 +18,7 @@ import org.junit.jupiter.api.Test; -import sleeper.configuration.deploy.DeployInstanceConfiguration; +import sleeper.configuration.properties.deploy.DeployInstanceConfiguration; import sleeper.configuration.properties.instance.CommonProperty; import sleeper.configuration.properties.table.TableProperty; import sleeper.systemtest.dsl.instance.SystemTestInstanceConfiguration; From b9d1946aa97e5ecfb595cc82fe1a1491f020c7e8 Mon Sep 17 00:00:00 2001 From: rtjd6554 <174791724+rtjd6554@users.noreply.github.com> Date: Tue, 17 Sep 2024 10:11:47 +0100 Subject: [PATCH 02/22] Relocation of testing deploy to properties --- .../deploy/DeployInstanceConfigurationIT.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) rename java/configuration/src/test/java/sleeper/configuration/{ => properties}/deploy/DeployInstanceConfigurationIT.java (98%) diff --git a/java/configuration/src/test/java/sleeper/configuration/deploy/DeployInstanceConfigurationIT.java b/java/configuration/src/test/java/sleeper/configuration/properties/deploy/DeployInstanceConfigurationIT.java similarity index 98% rename from java/configuration/src/test/java/sleeper/configuration/deploy/DeployInstanceConfigurationIT.java rename to java/configuration/src/test/java/sleeper/configuration/properties/deploy/DeployInstanceConfigurationIT.java index bc0f8024f4..005d52040d 100644 --- a/java/configuration/src/test/java/sleeper/configuration/deploy/DeployInstanceConfigurationIT.java +++ b/java/configuration/src/test/java/sleeper/configuration/properties/deploy/DeployInstanceConfigurationIT.java @@ -14,15 +14,13 @@ * limitations under the License. */ -package sleeper.configuration.deploy; +package sleeper.configuration.properties.deploy; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; -import sleeper.configuration.properties.deploy.DeployInstanceConfiguration; -import sleeper.configuration.properties.deploy.DeployInstanceConfigurationFromTemplates; import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.table.TableProperties; import sleeper.core.schema.SchemaSerDe; From 341bee35bc9e5e3d913bd15a5243adffd2fae2d2 Mon Sep 17 00:00:00 2001 From: rtjd6554 <174791724+rtjd6554@users.noreply.github.com> Date: Tue, 17 Sep 2024 10:42:56 +0100 Subject: [PATCH 03/22] Generation of OptionalStack class and defaultValue test --- .../properties/instance/CommonProperty.java | 5 +- .../properties/validation/OptionalStack.java | 47 +++++++++++++++++++ .../validation/OptionalStackEnumTest.java | 31 ++++++++++++ 3 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java create mode 100644 java/configuration/src/test/java/sleeper/configuration/properties/validation/OptionalStackEnumTest.java diff --git a/java/configuration/src/main/java/sleeper/configuration/properties/instance/CommonProperty.java b/java/configuration/src/main/java/sleeper/configuration/properties/instance/CommonProperty.java index de6d0d3e81..92b900e5af 100644 --- a/java/configuration/src/main/java/sleeper/configuration/properties/instance/CommonProperty.java +++ b/java/configuration/src/main/java/sleeper/configuration/properties/instance/CommonProperty.java @@ -18,6 +18,7 @@ import sleeper.configuration.Utils; import sleeper.configuration.properties.SleeperPropertyIndex; +import sleeper.configuration.properties.validation.OptionalStack; import java.util.List; import java.util.Objects; @@ -66,9 +67,7 @@ public interface CommonProperty { .includedInBasicTemplate(true).build(); UserDefinedInstanceProperty OPTIONAL_STACKS = Index.propertyBuilder("sleeper.optional.stacks") .description("The optional stacks to deploy.") - .defaultValue("CompactionStack,GarbageCollectorStack,IngestStack,IngestBatcherStack," + - "PartitionSplittingStack,QueryStack,AthenaStack,EmrServerlessBulkImportStack,EmrStudioStack," + - "DashboardStack,TableMetricsStack") + .defaultValue(OptionalStack.getDefaultList()) .propertyGroup(InstancePropertyGroup.COMMON) .runCdkDeployWhenChanged(true) .includedInBasicTemplate(true) diff --git a/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java b/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java new file mode 100644 index 0000000000..2529644549 --- /dev/null +++ b/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java @@ -0,0 +1,47 @@ +/* + * Copyright 2022-2024 Crown Copyright + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package sleeper.configuration.properties.validation; + +import org.apache.commons.lang3.EnumUtils; + +import java.util.stream.Collectors; +import java.util.stream.Stream; + +public enum OptionalStack { + CompactionStack, + GarbageCollectorStack, + IngestStack, + IngestBatcherStack, + PartitionSplittingStack, + QueryStack, + AthenaStack, + EmrServerlessBulkImportStack, + EmrStudioStack, + DashboardStack, + TableMetricsStack; + + public static boolean isValid(String value) { + return EnumUtils.isValidEnumIgnoreCase(OptionalStack.class, value); + } + + public static String getDefaultList() { + return Stream.of(CompactionStack, GarbageCollectorStack, IngestStack, IngestBatcherStack, PartitionSplittingStack, + QueryStack, AthenaStack, EmrServerlessBulkImportStack, EmrStudioStack, DashboardStack, TableMetricsStack) + .map(a -> a.toString()) + .collect(Collectors.joining(",")); + + } +} diff --git a/java/configuration/src/test/java/sleeper/configuration/properties/validation/OptionalStackEnumTest.java b/java/configuration/src/test/java/sleeper/configuration/properties/validation/OptionalStackEnumTest.java new file mode 100644 index 0000000000..3c785c4160 --- /dev/null +++ b/java/configuration/src/test/java/sleeper/configuration/properties/validation/OptionalStackEnumTest.java @@ -0,0 +1,31 @@ +/* + * Copyright 2022-2024 Crown Copyright + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package sleeper.configuration.properties.validation; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class OptionalStackEnumTest { + + @Test + void shouldGenerateListOfDefaultValueForOptionalStack() { + assertThat(OptionalStack.getDefaultList()) + .isEqualTo( + "CompactionStack,GarbageCollectorStack,IngestStack,IngestBatcherStack,PartitionSplittingStack,QueryStack,AthenaStack,EmrServerlessBulkImportStack,EmrStudioStack,DashboardStack,TableMetricsStack"); + + } +} From eb7a7f9988e35e82fbe835b6d224b64afddbcffc Mon Sep 17 00:00:00 2001 From: patchwork01 <110390516+patchwork01@users.noreply.github.com> Date: Tue, 17 Sep 2024 10:04:40 +0000 Subject: [PATCH 04/22] Validate optional stacks --- .../properties/SleeperPropertyValues.java | 6 ++ .../properties/instance/CommonProperty.java | 3 +- .../properties/validation/OptionalStack.java | 8 +- .../validation/OptionalStackEnumTest.java | 31 ------- .../validation/OptionalStackTest.java | 84 +++++++++++++++++++ 5 files changed, 98 insertions(+), 34 deletions(-) delete mode 100644 java/configuration/src/test/java/sleeper/configuration/properties/validation/OptionalStackEnumTest.java create mode 100644 java/configuration/src/test/java/sleeper/configuration/properties/validation/OptionalStackTest.java diff --git a/java/configuration/src/main/java/sleeper/configuration/properties/SleeperPropertyValues.java b/java/configuration/src/main/java/sleeper/configuration/properties/SleeperPropertyValues.java index 74ae670c73..63d351293a 100644 --- a/java/configuration/src/main/java/sleeper/configuration/properties/SleeperPropertyValues.java +++ b/java/configuration/src/main/java/sleeper/configuration/properties/SleeperPropertyValues.java @@ -25,6 +25,8 @@ import java.util.Optional; import java.util.stream.Stream; +import static java.util.stream.Collectors.toUnmodifiableList; + @FunctionalInterface public interface SleeperPropertyValues { @@ -59,6 +61,10 @@ default List getList(T property) { return SleeperPropertyValues.readList(get(property)); } + default > List getEnumList(T property, Class enumClass) { + return streamEnumList(property, enumClass).collect(toUnmodifiableList()); + } + default > Stream streamEnumList(T property, Class enumClass) { return getList(property).stream() .map(value -> Optional.ofNullable(EnumUtils.getEnumIgnoreCase(enumClass, value)) diff --git a/java/configuration/src/main/java/sleeper/configuration/properties/instance/CommonProperty.java b/java/configuration/src/main/java/sleeper/configuration/properties/instance/CommonProperty.java index 92b900e5af..58f2895f71 100644 --- a/java/configuration/src/main/java/sleeper/configuration/properties/instance/CommonProperty.java +++ b/java/configuration/src/main/java/sleeper/configuration/properties/instance/CommonProperty.java @@ -67,7 +67,8 @@ public interface CommonProperty { .includedInBasicTemplate(true).build(); UserDefinedInstanceProperty OPTIONAL_STACKS = Index.propertyBuilder("sleeper.optional.stacks") .description("The optional stacks to deploy.") - .defaultValue(OptionalStack.getDefaultList()) + .defaultValue(OptionalStack.getDefaultValue()) + .validationPredicate(OptionalStack::isValid) .propertyGroup(InstancePropertyGroup.COMMON) .runCdkDeployWhenChanged(true) .includedInBasicTemplate(true) diff --git a/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java b/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java index 2529644549..a4e11664ba 100644 --- a/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java +++ b/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java @@ -34,10 +34,14 @@ public enum OptionalStack { TableMetricsStack; public static boolean isValid(String value) { - return EnumUtils.isValidEnumIgnoreCase(OptionalStack.class, value); + if (value.isEmpty()) { + return true; + } + return Stream.of(value.split(",")) + .allMatch(item -> EnumUtils.isValidEnumIgnoreCase(OptionalStack.class, item)); } - public static String getDefaultList() { + public static String getDefaultValue() { return Stream.of(CompactionStack, GarbageCollectorStack, IngestStack, IngestBatcherStack, PartitionSplittingStack, QueryStack, AthenaStack, EmrServerlessBulkImportStack, EmrStudioStack, DashboardStack, TableMetricsStack) .map(a -> a.toString()) diff --git a/java/configuration/src/test/java/sleeper/configuration/properties/validation/OptionalStackEnumTest.java b/java/configuration/src/test/java/sleeper/configuration/properties/validation/OptionalStackEnumTest.java deleted file mode 100644 index 3c785c4160..0000000000 --- a/java/configuration/src/test/java/sleeper/configuration/properties/validation/OptionalStackEnumTest.java +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright 2022-2024 Crown Copyright - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package sleeper.configuration.properties.validation; - -import org.junit.jupiter.api.Test; - -import static org.assertj.core.api.Assertions.assertThat; - -public class OptionalStackEnumTest { - - @Test - void shouldGenerateListOfDefaultValueForOptionalStack() { - assertThat(OptionalStack.getDefaultList()) - .isEqualTo( - "CompactionStack,GarbageCollectorStack,IngestStack,IngestBatcherStack,PartitionSplittingStack,QueryStack,AthenaStack,EmrServerlessBulkImportStack,EmrStudioStack,DashboardStack,TableMetricsStack"); - - } -} diff --git a/java/configuration/src/test/java/sleeper/configuration/properties/validation/OptionalStackTest.java b/java/configuration/src/test/java/sleeper/configuration/properties/validation/OptionalStackTest.java new file mode 100644 index 0000000000..bce4e0377b --- /dev/null +++ b/java/configuration/src/test/java/sleeper/configuration/properties/validation/OptionalStackTest.java @@ -0,0 +1,84 @@ +/* + * Copyright 2022-2024 Crown Copyright + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package sleeper.configuration.properties.validation; + +import org.junit.jupiter.api.Test; + +import sleeper.configuration.properties.SleeperPropertiesInvalidException; +import sleeper.configuration.properties.instance.InstanceProperties; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static sleeper.configuration.properties.InstancePropertiesTestHelper.createTestInstanceProperties; +import static sleeper.configuration.properties.instance.CommonProperty.OPTIONAL_STACKS; +import static sleeper.configuration.properties.validation.OptionalStack.CompactionStack; +import static sleeper.configuration.properties.validation.OptionalStack.GarbageCollectorStack; + +public class OptionalStackTest { + + @Test + void shouldGenerateListOfDefaultValueForOptionalStack() { + InstanceProperties properties = new InstanceProperties(); + assertThat(properties.get(OPTIONAL_STACKS)) + .isEqualTo("CompactionStack,GarbageCollectorStack,IngestStack,IngestBatcherStack," + + "PartitionSplittingStack,QueryStack,AthenaStack,EmrServerlessBulkImportStack," + + "EmrStudioStack,DashboardStack,TableMetricsStack"); + } + + @Test + void shouldValidateWithOneValidOptionalStack() { + InstanceProperties properties = createTestInstanceProperties(); + properties.setEnumList(OPTIONAL_STACKS, List.of(CompactionStack)); + + assertThatCode(properties::validate).doesNotThrowAnyException(); + assertThat(properties.getEnumList(OPTIONAL_STACKS, OptionalStack.class)) + .containsExactly(CompactionStack); + } + + @Test + void shouldValidateWithTwoValidOptionalStacks() { + InstanceProperties properties = createTestInstanceProperties(); + properties.setEnumList(OPTIONAL_STACKS, List.of(CompactionStack, GarbageCollectorStack)); + + assertThatCode(properties::validate).doesNotThrowAnyException(); + assertThat(properties.getEnumList(OPTIONAL_STACKS, OptionalStack.class)) + .containsExactly(CompactionStack, GarbageCollectorStack); + } + + @Test + void shouldValidateWithEmptyProperty() { + InstanceProperties properties = createTestInstanceProperties(); + properties.setEnumList(OPTIONAL_STACKS, List.of()); + + assertThatCode(properties::validate).doesNotThrowAnyException(); + assertThat(properties.getEnumList(OPTIONAL_STACKS, OptionalStack.class)) + .isEmpty(); + } + + @Test + void shouldFailValidationWithMisspeltOptionalStack() { + InstanceProperties properties = createTestInstanceProperties(); + properties.setList(OPTIONAL_STACKS, List.of("CompacctionStack")); + + assertThatThrownBy(properties::validate) + .isInstanceOf(SleeperPropertiesInvalidException.class) + .hasMessageContaining("sleeper.optional.stacks") + .hasMessageContaining("CompacctionStack"); + } +} From 00d46ed1c25733f58f4012b28812fa794e44eb68 Mon Sep 17 00:00:00 2001 From: patchwork01 <110390516+patchwork01@users.noreply.github.com> Date: Tue, 17 Sep 2024 10:06:54 +0000 Subject: [PATCH 05/22] Use SleeperPropertyValues.readList for validation --- .../configuration/properties/validation/OptionalStack.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java b/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java index a4e11664ba..6531f3a790 100644 --- a/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java +++ b/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java @@ -17,6 +17,8 @@ import org.apache.commons.lang3.EnumUtils; +import sleeper.configuration.properties.SleeperPropertyValues; + import java.util.stream.Collectors; import java.util.stream.Stream; @@ -34,10 +36,7 @@ public enum OptionalStack { TableMetricsStack; public static boolean isValid(String value) { - if (value.isEmpty()) { - return true; - } - return Stream.of(value.split(",")) + return SleeperPropertyValues.readList(value).stream() .allMatch(item -> EnumUtils.isValidEnumIgnoreCase(OptionalStack.class, item)); } From 456334bbd625ef1697f112e35e03014fbc17f3de Mon Sep 17 00:00:00 2001 From: patchwork01 <110390516+patchwork01@users.noreply.github.com> Date: Tue, 17 Sep 2024 10:15:28 +0000 Subject: [PATCH 06/22] Use OptionalStack enum in CDK --- .../main/java/sleeper/cdk/SleeperCdkApp.java | 70 +++++++------------ .../properties/validation/OptionalStack.java | 47 +++++++++++-- 2 files changed, 66 insertions(+), 51 deletions(-) diff --git a/java/cdk/src/main/java/sleeper/cdk/SleeperCdkApp.java b/java/cdk/src/main/java/sleeper/cdk/SleeperCdkApp.java index c68c67a58d..769d1ee7e9 100644 --- a/java/cdk/src/main/java/sleeper/cdk/SleeperCdkApp.java +++ b/java/cdk/src/main/java/sleeper/cdk/SleeperCdkApp.java @@ -68,12 +68,13 @@ import sleeper.cdk.stack.bulkimport.EmrStudioStack; import sleeper.cdk.stack.bulkimport.PersistentEmrBulkImportStack; import sleeper.configuration.properties.instance.InstanceProperties; +import sleeper.configuration.properties.validation.OptionalStack; import java.util.ArrayList; import java.util.List; -import java.util.stream.Collectors; -import java.util.stream.Stream; +import java.util.Set; +import static java.util.stream.Collectors.toUnmodifiableSet; import static sleeper.configuration.properties.instance.CommonProperty.ACCOUNT; import static sleeper.configuration.properties.instance.CommonProperty.ID; import static sleeper.configuration.properties.instance.CommonProperty.JARS_BUCKET; @@ -108,35 +109,12 @@ public SleeperCdkApp(App app, String id, StackProps props, InstanceProperties in this.jars = jars; } - private static final List BULK_IMPORT_STACK_NAMES = Stream.of( - EmrBulkImportStack.class, - EmrServerlessBulkImportStack.class, - PersistentEmrBulkImportStack.class, - EksBulkImportStack.class) - .map(Class::getSimpleName).collect(Collectors.toList()); - - private static final List EMR_BULK_IMPORT_STACK_NAMES = Stream.of( - EmrBulkImportStack.class, - EmrServerlessBulkImportStack.class, - PersistentEmrBulkImportStack.class) - .map(Class::getSimpleName).collect(Collectors.toList()); - - public static final List INGEST_STACK_NAMES = Stream.of( - IngestStack.class, - EmrBulkImportStack.class, - EmrServerlessBulkImportStack.class, - PersistentEmrBulkImportStack.class, - EksBulkImportStack.class) - .map(Class::getSimpleName).collect(Collectors.toList()); - public static final List QUERY_STACK_NAMES = Stream.of( - QueryStack.class, - WebSocketQueryStack.class) - .map(Class::getSimpleName).collect(Collectors.toList()); - @SuppressWarnings("checkstyle:methodlength") public void create() { // Optional stacks to be included - List optionalStacks = instanceProperties.getList(OPTIONAL_STACKS); + Set optionalStacks = instanceProperties + .streamEnumList(OPTIONAL_STACKS, OptionalStack.class) + .collect(toUnmodifiableSet()); List errorMetrics = new ArrayList<>(); // Stack for Checking VPC configuration @@ -173,25 +151,25 @@ public void create() { instanceProperties, jars, coreStacks, transactionLogStateStoreStack, topicStack.getTopic(), errorMetrics); new TransactionLogTransactionStack(this, "TransactionLogTransaction", instanceProperties, jars, coreStacks, transactionLogStateStoreStack, topicStack.getTopic(), errorMetrics); - if (optionalStacks.contains(TableMetricsStack.class.getSimpleName())) { + if (optionalStacks.contains(OptionalStack.TableMetricsStack)) { new TableMetricsStack(this, "TableMetrics", instanceProperties, jars, topicStack.getTopic(), coreStacks, errorMetrics); } // Stack for Athena analytics - if (optionalStacks.contains(AthenaStack.class.getSimpleName())) { + if (optionalStacks.contains(OptionalStack.AthenaStack)) { new AthenaStack(this, "Athena", instanceProperties, jars, coreStacks); } - if (BULK_IMPORT_STACK_NAMES.stream().anyMatch(optionalStacks::contains)) { + if (OptionalStack.BULK_IMPORT_STACKS.stream().anyMatch(optionalStacks::contains)) { bulkImportBucketStack = new BulkImportBucketStack(this, "BulkImportBucket", instanceProperties, coreStacks); } - if (EMR_BULK_IMPORT_STACK_NAMES.stream().anyMatch(optionalStacks::contains)) { + if (OptionalStack.EMR_BULK_IMPORT_STACKS.stream().anyMatch(optionalStacks::contains)) { emrBulkImportCommonStack = new CommonEmrBulkImportStack(this, "BulkImportEMRCommon", instanceProperties, coreStacks, bulkImportBucketStack); } // Stack to run bulk import jobs via EMR Serverless - if (optionalStacks.contains(EmrServerlessBulkImportStack.class.getSimpleName())) { + if (optionalStacks.contains(OptionalStack.EmrServerlessBulkImportStack)) { emrServerlessBulkImportStack = new EmrServerlessBulkImportStack(this, "BulkImportEMRServerless", instanceProperties, jars, topicStack.getTopic(), @@ -200,12 +178,12 @@ public void create() { errorMetrics); // Stack to created EMR studio to be used to access EMR Serverless - if (optionalStacks.contains(EmrStudioStack.class.getSimpleName())) { + if (optionalStacks.contains(OptionalStack.EmrStudioStack)) { new EmrStudioStack(this, "EmrStudio", instanceProperties); } } // Stack to run bulk import jobs via EMR (one cluster per bulk import job) - if (optionalStacks.contains(EmrBulkImportStack.class.getSimpleName())) { + if (optionalStacks.contains(OptionalStack.EmrBulkImportStack)) { emrBulkImportStack = new EmrBulkImportStack(this, "BulkImportEMR", instanceProperties, jars, topicStack.getTopic(), @@ -216,7 +194,7 @@ public void create() { } // Stack to run bulk import jobs via a persistent EMR cluster - if (optionalStacks.contains(PersistentEmrBulkImportStack.class.getSimpleName())) { + if (optionalStacks.contains(OptionalStack.PersistentEmrBulkImportStack)) { persistentEmrBulkImportStack = new PersistentEmrBulkImportStack(this, "BulkImportPersistentEMR", instanceProperties, jars, topicStack.getTopic(), @@ -227,7 +205,7 @@ public void create() { } // Stack to run bulk import jobs via EKS - if (optionalStacks.contains(EksBulkImportStack.class.getSimpleName())) { + if (optionalStacks.contains(OptionalStack.EksBulkImportStack)) { eksBulkImportStack = new EksBulkImportStack(this, "BulkImportEKS", instanceProperties, jars, topicStack.getTopic(), @@ -237,7 +215,7 @@ public void create() { } // Stack to garbage collect old files - if (optionalStacks.contains(GarbageCollectorStack.class.getSimpleName())) { + if (optionalStacks.contains(OptionalStack.GarbageCollectorStack)) { new GarbageCollectorStack(this, "GarbageCollector", instanceProperties, jars, @@ -246,7 +224,7 @@ public void create() { errorMetrics); } // Stack for containers for compactions and splitting compactions - if (optionalStacks.contains(CompactionStack.class.getSimpleName())) { + if (optionalStacks.contains(OptionalStack.CompactionStack)) { compactionStack = new CompactionStack(this, "Compaction", instanceProperties, jars, @@ -256,7 +234,7 @@ public void create() { } // Stack to split partitions - if (optionalStacks.contains(PartitionSplittingStack.class.getSimpleName())) { + if (optionalStacks.contains(OptionalStack.PartitionSplittingStack)) { partitionSplittingStack = new PartitionSplittingStack(this, "PartitionSplitting", instanceProperties, jars, @@ -267,7 +245,7 @@ public void create() { QueryStack queryStack = null; // Stack to execute queries - if (QUERY_STACK_NAMES.stream().anyMatch(optionalStacks::contains)) { + if (OptionalStack.QUERY_STACKS.stream().anyMatch(optionalStacks::contains)) { queryQueueStack = new QueryQueueStack(this, "QueryQueue", instanceProperties, topicStack.getTopic(), coreStacks, @@ -279,7 +257,7 @@ public void create() { coreStacks, queryQueueStack, errorMetrics); // Stack to execute queries using the web socket API - if (optionalStacks.contains(WebSocketQueryStack.class.getSimpleName())) { + if (optionalStacks.contains(OptionalStack.WebSocketQueryStack)) { new WebSocketQueryStack(this, "WebSocketQuery", instanceProperties, jars, @@ -287,7 +265,7 @@ public void create() { } } // Stack for ingest jobs - if (optionalStacks.contains(IngestStack.class.getSimpleName())) { + if (optionalStacks.contains(OptionalStack.IngestStack)) { ingestStack = new IngestStack(this, "Ingest", instanceProperties, jars, @@ -300,7 +278,7 @@ public void create() { ingestStacks = new IngestStacks(ingestStack, emrBulkImportStack, persistentEmrBulkImportStack, eksBulkImportStack, emrServerlessBulkImportStack); // Stack to batch up files to ingest and create jobs - if (optionalStacks.contains(IngestBatcherStack.class.getSimpleName())) { + if (optionalStacks.contains(OptionalStack.IngestBatcherStack)) { ingestBatcherStack = new IngestBatcherStack(this, "IngestBatcher", instanceProperties, jars, topicStack.getTopic(), @@ -309,7 +287,7 @@ public void create() { errorMetrics); } - if (optionalStacks.contains(DashboardStack.class.getSimpleName())) { + if (optionalStacks.contains(OptionalStack.DashboardStack)) { new DashboardStack(this, "Dashboard", ingestStack, @@ -319,7 +297,7 @@ public void create() { errorMetrics); } - if (optionalStacks.contains(KeepLambdaWarmStack.class.getSimpleName())) { + if (optionalStacks.contains(OptionalStack.KeepLambdaWarmStack)) { new KeepLambdaWarmStack(this, "KeepLambdaWarmExecution", instanceProperties, diff --git a/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java b/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java index 6531f3a790..a65c08327e 100644 --- a/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java +++ b/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java @@ -19,22 +19,59 @@ import sleeper.configuration.properties.SleeperPropertyValues; +import java.util.List; import java.util.stream.Collectors; import java.util.stream.Stream; public enum OptionalStack { - CompactionStack, - GarbageCollectorStack, + + // Ingest IngestStack, IngestBatcherStack, - PartitionSplittingStack, - QueryStack, - AthenaStack, + + // Bulk import + EmrBulkImportStack, EmrServerlessBulkImportStack, + PersistentEmrBulkImportStack, + EksBulkImportStack, EmrStudioStack, + + // Query + QueryStack, + WebSocketQueryStack, + AthenaStack, + KeepLambdaWarmStack, + + // Data maintenance + CompactionStack, + GarbageCollectorStack, + PartitionSplittingStack, + + // Metrics DashboardStack, TableMetricsStack; + public static final List BULK_IMPORT_STACKS = List.of( + EmrBulkImportStack, + EmrServerlessBulkImportStack, + PersistentEmrBulkImportStack, + EksBulkImportStack); + + public static final List EMR_BULK_IMPORT_STACKS = List.of( + EmrBulkImportStack, + EmrServerlessBulkImportStack, + PersistentEmrBulkImportStack); + + public static final List INGEST_STACKS = List.of( + IngestStack, + EmrBulkImportStack, + EmrServerlessBulkImportStack, + PersistentEmrBulkImportStack, + EksBulkImportStack); + public static final List QUERY_STACKS = List.of( + QueryStack, + WebSocketQueryStack); + public static boolean isValid(String value) { return SleeperPropertyValues.readList(value).stream() .allMatch(item -> EnumUtils.isValidEnumIgnoreCase(OptionalStack.class, item)); From e86b219919abf5c9c80a4f89928ca798ed54a194 Mon Sep 17 00:00:00 2001 From: rtjd6554 <174791724+rtjd6554@users.noreply.github.com> Date: Tue, 17 Sep 2024 11:26:36 +0100 Subject: [PATCH 07/22] Fix for compilatin error --- .../configuration/properties/validation/OptionalStackTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/configuration/src/test/java/sleeper/configuration/properties/validation/OptionalStackTest.java b/java/configuration/src/test/java/sleeper/configuration/properties/validation/OptionalStackTest.java index bce4e0377b..d904130751 100644 --- a/java/configuration/src/test/java/sleeper/configuration/properties/validation/OptionalStackTest.java +++ b/java/configuration/src/test/java/sleeper/configuration/properties/validation/OptionalStackTest.java @@ -64,7 +64,7 @@ void shouldValidateWithTwoValidOptionalStacks() { @Test void shouldValidateWithEmptyProperty() { InstanceProperties properties = createTestInstanceProperties(); - properties.setEnumList(OPTIONAL_STACKS, List.of()); + properties.setEnumList(OPTIONAL_STACKS, List.of()); assertThatCode(properties::validate).doesNotThrowAnyException(); assertThat(properties.getEnumList(OPTIONAL_STACKS, OptionalStack.class)) From d1e5abec377423acb9452107d7a0dd31154a9fbe Mon Sep 17 00:00:00 2001 From: rtjd6554 <174791724+rtjd6554@users.noreply.github.com> Date: Tue, 17 Sep 2024 11:36:57 +0100 Subject: [PATCH 08/22] Update for SystemTestInstance usage of enum --- .../properties/validation/OptionalStack.java | 14 +++++++++++++- .../suite/fixtures/SystemTestInstance.java | 4 ++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java b/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java index a65c08327e..34b677e63c 100644 --- a/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java +++ b/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java @@ -72,6 +72,19 @@ public enum OptionalStack { QueryStack, WebSocketQueryStack); + public static final List SYSTEM_TEST_STACKS = List.of( + IngestStack, + EmrBulkImportStack, + EmrServerlessBulkImportStack, + IngestBatcherStack, + CompactionStack, + GarbageCollectorStack, + PartitionSplittingStack, + QueryStack, + WebSocketQueryStack, + TableMetricsStack, + AthenaStack); + public static boolean isValid(String value) { return SleeperPropertyValues.readList(value).stream() .allMatch(item -> EnumUtils.isValidEnumIgnoreCase(OptionalStack.class, item)); @@ -82,6 +95,5 @@ public static String getDefaultValue() { QueryStack, AthenaStack, EmrServerlessBulkImportStack, EmrStudioStack, DashboardStack, TableMetricsStack) .map(a -> a.toString()) .collect(Collectors.joining(",")); - } } diff --git a/java/system-test/system-test-suite/src/main/java/sleeper/systemtest/suite/fixtures/SystemTestInstance.java b/java/system-test/system-test-suite/src/main/java/sleeper/systemtest/suite/fixtures/SystemTestInstance.java index 7320b86c5c..f5826d40ef 100644 --- a/java/system-test/system-test-suite/src/main/java/sleeper/systemtest/suite/fixtures/SystemTestInstance.java +++ b/java/system-test/system-test-suite/src/main/java/sleeper/systemtest/suite/fixtures/SystemTestInstance.java @@ -20,6 +20,7 @@ import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.table.TableProperties; import sleeper.configuration.properties.validation.EmrInstanceArchitecture; +import sleeper.configuration.properties.validation.OptionalStack; import sleeper.systemtest.dsl.instance.SystemTestInstanceConfiguration; import java.util.HashMap; @@ -82,8 +83,7 @@ private SystemTestInstance() { private static DeployInstanceConfiguration buildMainConfiguration() { InstanceProperties properties = new InstanceProperties(); properties.set(LOGGING_LEVEL, "debug"); - properties.set(OPTIONAL_STACKS, "IngestStack,EmrBulkImportStack,EmrServerlessBulkImportStack,IngestBatcherStack," + - "CompactionStack,GarbageCollectorStack,PartitionSplittingStack,QueryStack,WebSocketQueryStack,TableMetricsStack"); + properties.setEnumList(OPTIONAL_STACKS, OptionalStack.SYSTEM_TEST_STACKS); properties.set(RETAIN_INFRA_AFTER_DESTROY, "false"); properties.set(FORCE_RELOAD_PROPERTIES, "true"); properties.set(DEFAULT_DYNAMO_STRONGLY_CONSISTENT_READS, "true"); From 794c02f6a8b2a74052b91870e030374abb4d88f6 Mon Sep 17 00:00:00 2001 From: rtjd6554 <174791724+rtjd6554@users.noreply.github.com> Date: Tue, 17 Sep 2024 11:40:25 +0100 Subject: [PATCH 09/22] Adjust enum usage with SystemTest Instances --- .../suite/fixtures/SystemTestInstance.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/java/system-test/system-test-suite/src/main/java/sleeper/systemtest/suite/fixtures/SystemTestInstance.java b/java/system-test/system-test-suite/src/main/java/sleeper/systemtest/suite/fixtures/SystemTestInstance.java index f5826d40ef..528a1108e0 100644 --- a/java/system-test/system-test-suite/src/main/java/sleeper/systemtest/suite/fixtures/SystemTestInstance.java +++ b/java/system-test/system-test-suite/src/main/java/sleeper/systemtest/suite/fixtures/SystemTestInstance.java @@ -24,6 +24,7 @@ import sleeper.systemtest.dsl.instance.SystemTestInstanceConfiguration; import java.util.HashMap; +import java.util.List; import java.util.Map; import static sleeper.configuration.properties.instance.ArrowIngestProperty.ARROW_INGEST_BATCH_BUFFER_BYTES; @@ -120,7 +121,7 @@ private static DeployInstanceConfiguration buildMainConfiguration() { private static DeployInstanceConfiguration buildIngestPerformanceConfiguration() { DeployInstanceConfiguration configuration = buildMainConfiguration(); InstanceProperties properties = configuration.getInstanceProperties(); - properties.set(OPTIONAL_STACKS, "IngestStack"); + properties.setEnum(OPTIONAL_STACKS, OptionalStack.IngestStack); properties.set(MAXIMUM_CONCURRENT_INGEST_TASKS, "11"); properties.set(MAXIMUM_CONNECTIONS_TO_S3, "25"); properties.set(DEFAULT_INGEST_RECORD_BATCH_TYPE, "arrow"); @@ -142,7 +143,7 @@ private static DeployInstanceConfiguration buildIngestPerformanceConfiguration() private static DeployInstanceConfiguration buildCompactionPerformanceConfiguration() { DeployInstanceConfiguration configuration = buildMainConfiguration(); InstanceProperties properties = configuration.getInstanceProperties(); - properties.set(OPTIONAL_STACKS, "CompactionStack"); + properties.setEnum(OPTIONAL_STACKS, OptionalStack.CompactionStack); properties.set(COMPACTION_ECS_LAUNCHTYPE, "EC2"); properties.set(COMPACTION_TASK_CPU_ARCHITECTURE, "X86_64"); properties.set(COMPACTION_TASK_X86_CPU, "1024"); @@ -163,7 +164,7 @@ private static DeployInstanceConfiguration buildCompactionPerformanceConfigurati private static DeployInstanceConfiguration buildBulkImportPerformanceConfiguration() { DeployInstanceConfiguration configuration = buildMainConfiguration(); InstanceProperties properties = configuration.getInstanceProperties(); - properties.set(OPTIONAL_STACKS, "EmrBulkImportStack"); + properties.setEnum(OPTIONAL_STACKS, OptionalStack.EmrBulkImportStack); properties.set(DEFAULT_BULK_IMPORT_EMR_MAX_EXECUTOR_CAPACITY, "5"); properties.set(MAXIMUM_CONNECTIONS_TO_S3, "25"); Map tags = new HashMap<>(properties.getTags()); @@ -176,7 +177,7 @@ private static DeployInstanceConfiguration buildBulkImportPerformanceConfigurati private static DeployInstanceConfiguration buildCompactionOnEC2Configuration() { DeployInstanceConfiguration configuration = buildMainConfiguration(); InstanceProperties properties = configuration.getInstanceProperties(); - properties.set(OPTIONAL_STACKS, "CompactionStack"); + properties.setEnum(OPTIONAL_STACKS, OptionalStack.CompactionStack); properties.set(COMPACTION_ECS_LAUNCHTYPE, "EC2"); Map tags = new HashMap<>(properties.getTags()); @@ -189,7 +190,7 @@ private static DeployInstanceConfiguration buildCompactionOnEC2Configuration() { private static DeployInstanceConfiguration buildCompactionInParallelConfiguration() { DeployInstanceConfiguration configuration = buildMainConfiguration(); InstanceProperties properties = configuration.getInstanceProperties(); - properties.set(OPTIONAL_STACKS, "CompactionStack"); + properties.setEnum(OPTIONAL_STACKS, OptionalStack.CompactionStack); properties.set(MAXIMUM_CONCURRENT_COMPACTION_TASKS, "300"); Map tags = new HashMap<>(properties.getTags()); @@ -203,8 +204,7 @@ private static DeployInstanceConfiguration buildStateStoreCommitterThroughputCon DeployInstanceConfiguration configuration = buildMainConfiguration(); InstanceProperties properties = configuration.getInstanceProperties(); - // At time of writing, setting an empty list would make it use the default value instead. See issue #3089. - properties.set(OPTIONAL_STACKS, "None"); + properties.setList(OPTIONAL_STACKS, List.of()); Map tags = new HashMap<>(properties.getTags()); tags.put("SystemTestInstance", "stateStoreCommitterThroughput"); From b50654bf8e735bbfbb3596de4770d97c263c7cb2 Mon Sep 17 00:00:00 2001 From: rtjd6554 <174791724+rtjd6554@users.noreply.github.com> Date: Tue, 17 Sep 2024 11:43:38 +0100 Subject: [PATCH 10/22] Adjust for IngestBatcherStore usage of OptionalStack --- .../ingest/batcher/store/IngestBatcherStoreFactory.java | 3 ++- .../ingest/batcher/store/IngestBatcherStoreFactoryTest.java | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/java/ingest/ingest-batcher-store/src/main/java/sleeper/ingest/batcher/store/IngestBatcherStoreFactory.java b/java/ingest/ingest-batcher-store/src/main/java/sleeper/ingest/batcher/store/IngestBatcherStoreFactory.java index 73b035fb1b..79d3e5df7b 100644 --- a/java/ingest/ingest-batcher-store/src/main/java/sleeper/ingest/batcher/store/IngestBatcherStoreFactory.java +++ b/java/ingest/ingest-batcher-store/src/main/java/sleeper/ingest/batcher/store/IngestBatcherStoreFactory.java @@ -20,6 +20,7 @@ import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.table.TablePropertiesProvider; +import sleeper.configuration.properties.validation.OptionalStack; import sleeper.ingest.batcher.IngestBatcherStore; import java.util.Optional; @@ -32,7 +33,7 @@ private IngestBatcherStoreFactory() { public static Optional getStore( AmazonDynamoDB dynamoDB, InstanceProperties properties, TablePropertiesProvider tablePropertiesProvider) { - if (properties.getList(OPTIONAL_STACKS).contains("IngestBatcherStack")) { + if (properties.getEnumList(OPTIONAL_STACKS, OptionalStack.class).contains(OptionalStack.IngestBatcherStack)) { return Optional.of(new DynamoDBIngestBatcherStore(dynamoDB, properties, tablePropertiesProvider)); } return Optional.empty(); diff --git a/java/ingest/ingest-batcher-store/src/test/java/sleeper/ingest/batcher/store/IngestBatcherStoreFactoryTest.java b/java/ingest/ingest-batcher-store/src/test/java/sleeper/ingest/batcher/store/IngestBatcherStoreFactoryTest.java index 8bcf4bbb9d..174d17a348 100644 --- a/java/ingest/ingest-batcher-store/src/test/java/sleeper/ingest/batcher/store/IngestBatcherStoreFactoryTest.java +++ b/java/ingest/ingest-batcher-store/src/test/java/sleeper/ingest/batcher/store/IngestBatcherStoreFactoryTest.java @@ -22,6 +22,7 @@ import sleeper.configuration.properties.table.FixedTablePropertiesProvider; import sleeper.configuration.properties.table.TableProperties; import sleeper.configuration.properties.table.TablePropertiesProvider; +import sleeper.configuration.properties.validation.OptionalStack; import sleeper.ingest.batcher.IngestBatcherStore; import java.util.Optional; @@ -37,7 +38,7 @@ public class IngestBatcherStoreFactoryTest { void shouldGetIngestBatcherStoreWhenOptionalStackEnabled() { // Given InstanceProperties properties = createTestInstanceProperties(); - properties.set(OPTIONAL_STACKS, "IngestBatcherStack"); + properties.setEnum(OPTIONAL_STACKS, OptionalStack.IngestBatcherStack); // When/Then assertThat(getStore(properties)) @@ -48,7 +49,7 @@ void shouldGetIngestBatcherStoreWhenOptionalStackEnabled() { void shouldNotGetIngestBatcherStoreWhenOptionalStackDisabled() { // Given InstanceProperties properties = createTestInstanceProperties(); - properties.set(OPTIONAL_STACKS, "CompactionStack"); + properties.setEnum(OPTIONAL_STACKS, OptionalStack.CompactionStack); // When/Then assertThat(getStore(properties)) From 0695e12fb09ec4db9922aff436eb6b6d6f5248ce Mon Sep 17 00:00:00 2001 From: rtjd6554 <174791724+rtjd6554@users.noreply.github.com> Date: Tue, 17 Sep 2024 12:02:51 +0100 Subject: [PATCH 11/22] Adjust usages for DockerImageConfiguration --- .../deploy/DockerImageConfiguration.java | 22 ++++++----- .../deploy/UploadDockerImagesTest.java | 39 ++++++++++--------- 2 files changed, 33 insertions(+), 28 deletions(-) diff --git a/java/clients/src/main/java/sleeper/clients/deploy/DockerImageConfiguration.java b/java/clients/src/main/java/sleeper/clients/deploy/DockerImageConfiguration.java index 723b0a3217..c0e04e85ff 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/DockerImageConfiguration.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/DockerImageConfiguration.java @@ -16,6 +16,10 @@ package sleeper.clients.deploy; +import org.apache.commons.lang3.EnumUtils; + +import sleeper.configuration.properties.validation.OptionalStack; + import java.util.Collection; import java.util.List; import java.util.Map; @@ -28,19 +32,19 @@ import static sleeper.clients.deploy.StackDockerImage.emrServerlessImage; public class DockerImageConfiguration { - private static final Map DEFAULT_DOCKER_IMAGE_BY_STACK = Map.of( - "IngestStack", dockerBuildImage("ingest"), - "EksBulkImportStack", dockerBuildImage("bulk-import-runner"), - "CompactionStack", dockerBuildxImage("compaction-job-execution"), - "EmrServerlessBulkImportStack", emrServerlessImage("bulk-import-runner-emr-serverless")); + private static final Map DEFAULT_DOCKER_IMAGE_BY_STACK = Map.of( + OptionalStack.IngestStack, dockerBuildImage("ingest"), + OptionalStack.EksBulkImportStack, dockerBuildImage("bulk-import-runner"), + OptionalStack.CompactionStack, dockerBuildxImage("compaction-job-execution"), + OptionalStack.EmrServerlessBulkImportStack, emrServerlessImage("bulk-import-runner-emr-serverless")); - private final Map imageByStack; + private final Map imageByStack; public DockerImageConfiguration() { this(DEFAULT_DOCKER_IMAGE_BY_STACK); } - public DockerImageConfiguration(Map imageByStack) { + public DockerImageConfiguration(Map imageByStack) { this.imageByStack = imageByStack; } @@ -57,8 +61,8 @@ public List getStacksToDeploy(Collection stacks, List< .collect(toUnmodifiableList()); } - public Optional getStackImage(String stack) { - return Optional.ofNullable(imageByStack.get(stack)); + private Optional getStackImage(String stack) { + return Optional.ofNullable(imageByStack.get(EnumUtils.getEnumIgnoreCase(OptionalStack.class, stack))); } public Optional getInstanceIdFromRepoName(String repositoryName) { diff --git a/java/clients/src/test/java/sleeper/clients/deploy/UploadDockerImagesTest.java b/java/clients/src/test/java/sleeper/clients/deploy/UploadDockerImagesTest.java index c9e2f48acd..27f4d03ff4 100644 --- a/java/clients/src/test/java/sleeper/clients/deploy/UploadDockerImagesTest.java +++ b/java/clients/src/test/java/sleeper/clients/deploy/UploadDockerImagesTest.java @@ -26,6 +26,7 @@ import sleeper.clients.util.CommandPipeline; import sleeper.clients.util.InMemoryEcrRepositories; import sleeper.configuration.properties.instance.InstanceProperties; +import sleeper.configuration.properties.validation.OptionalStack; import java.io.IOException; import java.nio.file.Path; @@ -52,11 +53,11 @@ import static sleeper.configuration.properties.instance.CommonProperty.REGION; public class UploadDockerImagesTest { - private static final Map STACK_DOCKER_IMAGES = Map.of( - "IngestStack", dockerBuildImage("ingest"), - "EksBulkImportStack", dockerBuildImage("bulk-import-runner"), - "BuildxStack", dockerBuildxImage("buildx"), - "EmrServerlessBulkImportStack", emrServerlessImage("bulk-import-runner-emr-serverless")); + private static final Map STACK_DOCKER_IMAGES = Map.of( + OptionalStack.IngestStack, dockerBuildImage("ingest"), + OptionalStack.EksBulkImportStack, dockerBuildImage("bulk-import-runner"), + OptionalStack.CompactionStack, dockerBuildxImage("buildx"), + OptionalStack.EmrServerlessBulkImportStack, emrServerlessImage("bulk-import-runner-emr-serverless")); private final InMemoryEcrRepositories ecrClient = new InMemoryEcrRepositories(); private final InstanceProperties properties = createTestInstanceProperties(); private final DockerImageConfiguration dockerImageConfiguration = new DockerImageConfiguration(STACK_DOCKER_IMAGES); @@ -88,7 +89,7 @@ class UploadImages { @Test void shouldCreateRepositoryAndPushImageForIngestStack() throws Exception { // Given - properties.set(OPTIONAL_STACKS, "IngestStack"); + properties.setEnum(OPTIONAL_STACKS, OptionalStack.IngestStack); // When List commandsThatRan = pipelinesRunOn(upload(properties)); @@ -107,7 +108,7 @@ void shouldCreateRepositoryAndPushImageForIngestStack() throws Exception { @Test void shouldCreateRepositoriesAndPushImagesForTwoStacks() throws IOException, InterruptedException { // Given - properties.set(OPTIONAL_STACKS, "IngestStack,EksBulkImportStack"); + properties.setEnumList(OPTIONAL_STACKS, List.of(OptionalStack.IngestStack, OptionalStack.EksBulkImportStack)); // When List commandsThatRan = pipelinesRunOn(upload(properties)); @@ -130,7 +131,7 @@ void shouldCreateRepositoriesAndPushImagesForTwoStacks() throws IOException, Int void shouldCreateRepositoryAndPushImageWhenEcrRepositoryPrefixIsSet() throws Exception { // Given properties.set(ECR_REPOSITORY_PREFIX, "custom-ecr-prefix"); - properties.set(OPTIONAL_STACKS, "IngestStack"); + properties.setEnum(OPTIONAL_STACKS, OptionalStack.IngestStack); // When List commandsThatRan = pipelinesRunOn(upload(properties)); @@ -152,7 +153,7 @@ class HandleStacksNotNeedingUploads { @Test void shouldDoNothingWhenStackHasNoDockerImage() throws Exception { // Given - properties.set(OPTIONAL_STACKS, "OtherStack"); + properties.setEnum(OPTIONAL_STACKS, OptionalStack.AthenaStack); // When List commandsThatRan = pipelinesRunOn(upload(properties)); @@ -165,7 +166,7 @@ void shouldDoNothingWhenStackHasNoDockerImage() throws Exception { @Test void shouldCreateRepositoryAndPushImageWhenPreviousStackHasNoDockerImage() throws Exception { // Given - properties.set(OPTIONAL_STACKS, "OtherStack,IngestStack"); + properties.setEnumList(OPTIONAL_STACKS, List.of(OptionalStack.AthenaStack, OptionalStack.IngestStack)); // When List commandsThatRan = pipelinesRunOn(upload(properties)); @@ -186,7 +187,7 @@ class BuildImagesWithBuildx { @Test void shouldCreateRepositoryAndPushImageWhenImageNeedsToBeBuiltByBuildx() throws IOException, InterruptedException { // Given - properties.set(OPTIONAL_STACKS, "BuildxStack"); + properties.setEnum(OPTIONAL_STACKS, OptionalStack.CompactionStack); // When List commandsThatRan = pipelinesRunOn(upload(properties)); @@ -206,7 +207,7 @@ void shouldCreateRepositoryAndPushImageWhenImageNeedsToBeBuiltByBuildx() throws @Test void shouldCreateRepositoryAndPushImageWhenOnlyOneImageNeedsToBeBuiltByBuildx() throws IOException, InterruptedException { // Given - properties.set(OPTIONAL_STACKS, "IngestStack,BuildxStack"); + properties.setEnumList(OPTIONAL_STACKS, List.of(OptionalStack.IngestStack, OptionalStack.CompactionStack)); // When List commandsThatRan = pipelinesRunOn(upload(properties)); @@ -234,7 +235,7 @@ class CreateEMRServerlessSecurityPolicy { @Test void shouldCreateSecurityPolicyToGiveAccessToEmrServerlessImageOnly() throws IOException, InterruptedException { // Given - properties.set(OPTIONAL_STACKS, "IngestStack,EmrServerlessBulkImportStack"); + properties.setEnumList(OPTIONAL_STACKS, List.of(OptionalStack.IngestStack, OptionalStack.EmrServerlessBulkImportStack)); // When List commandsThatRan = pipelinesRunOn(upload(properties)); @@ -263,7 +264,7 @@ class HandleCommandFailures { @Test void shouldFailWhenDockerLoginFails() { // Given - properties.set(OPTIONAL_STACKS, "IngestStack"); + properties.setEnum(OPTIONAL_STACKS, OptionalStack.IngestStack); // When / Then assertThatThrownBy(() -> uploader().upload(returningExitCode(123), StacksForDockerUpload.from(properties))) @@ -277,7 +278,7 @@ void shouldFailWhenDockerLoginFails() { @Test void shouldNotFailWhenRemoveBuildxBuilderFails() throws Exception { // Given - properties.set(OPTIONAL_STACKS, "BuildxStack"); + properties.setEnum(OPTIONAL_STACKS, OptionalStack.CompactionStack); // When List commandsThatRan = pipelinesRunOn(upload(properties), @@ -298,7 +299,7 @@ void shouldNotFailWhenRemoveBuildxBuilderFails() throws Exception { @Test void shouldFailWhenCreateBuildxBuilderFails() { // Given - properties.set(OPTIONAL_STACKS, "BuildxStack"); + properties.setEnum(OPTIONAL_STACKS, OptionalStack.CompactionStack); // When / Then assertThatThrownBy(() -> uploader().upload( @@ -314,7 +315,7 @@ void shouldFailWhenCreateBuildxBuilderFails() { @Test void shouldDeleteRepositoryWhenDockerBuildFails() { // Given - properties.set(OPTIONAL_STACKS, "IngestStack"); + properties.setEnum(OPTIONAL_STACKS, OptionalStack.IngestStack); // When / Then CommandPipeline buildImageCommand = buildImageCommand( @@ -337,7 +338,7 @@ class HandleExistingImages { @Test void shouldBuildAndPushImageIfImageWithDifferentVersionExists() throws IOException, InterruptedException { // Given - properties.set(OPTIONAL_STACKS, "IngestStack"); + properties.setEnum(OPTIONAL_STACKS, OptionalStack.IngestStack); ecrClient.createRepository("test-instance/ingest"); ecrClient.addVersionToRepository("test-instance/ingest", "0.9.0"); @@ -358,7 +359,7 @@ void shouldBuildAndPushImageIfImageWithDifferentVersionExists() throws IOExcepti @Test void shouldNotBuildAndPushImageIfImageWithMatchingVersionExists() throws IOException, InterruptedException { // Given - properties.set(OPTIONAL_STACKS, "IngestStack"); + properties.setEnum(OPTIONAL_STACKS, OptionalStack.IngestStack); ecrClient.createRepository("test-instance/ingest"); ecrClient.addVersionToRepository("test-instance/ingest", "1.0.0"); From 8667f0c7e9acd16580cde83906265d513ac3d708 Mon Sep 17 00:00:00 2001 From: patchwork01 <110390516+patchwork01@users.noreply.github.com> Date: Tue, 17 Sep 2024 11:47:33 +0000 Subject: [PATCH 12/22] Fix compilation in CleanUpDeletedSleeperInstancesTest --- .../drivers/cdk/CleanUpDeletedSleeperInstancesTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/java/system-test/system-test-drivers/src/test/java/sleeper/systemtest/drivers/cdk/CleanUpDeletedSleeperInstancesTest.java b/java/system-test/system-test-drivers/src/test/java/sleeper/systemtest/drivers/cdk/CleanUpDeletedSleeperInstancesTest.java index f4177365d8..c39d9971a8 100644 --- a/java/system-test/system-test-drivers/src/test/java/sleeper/systemtest/drivers/cdk/CleanUpDeletedSleeperInstancesTest.java +++ b/java/system-test/system-test-drivers/src/test/java/sleeper/systemtest/drivers/cdk/CleanUpDeletedSleeperInstancesTest.java @@ -19,6 +19,7 @@ import org.junit.jupiter.api.Test; import sleeper.clients.deploy.DockerImageConfiguration; +import sleeper.configuration.properties.validation.OptionalStack; import java.util.Map; import java.util.stream.Stream; @@ -46,8 +47,8 @@ void shouldGetInstanceIdsFromJarsBuckets() { void shouldGetInstanceIdsFromEcrRepositories() { assertThat(instanceIdsByEcrRepositories( new DockerImageConfiguration(Map.of( - "IngestStack", dockerBuildImage("ingest"), - "CompactionStack", dockerBuildxImage("compaction-job-execution"))), + OptionalStack.IngestStack, dockerBuildImage("ingest"), + OptionalStack.CompactionStack, dockerBuildxImage("compaction-job-execution"))), Stream.of("an-instance/ingest", "not-sleeper/something", "not-an-instance", From fdc39a20c011bdf2ed1beb5e625b86374fc08a66 Mon Sep 17 00:00:00 2001 From: patchwork01 <110390516+patchwork01@users.noreply.github.com> Date: Tue, 17 Sep 2024 13:13:42 +0000 Subject: [PATCH 13/22] Pass OptionalStack to DockerImageConfiguration from AdminClientPropertiesStore --- .../properties/AdminClientPropertiesStore.java | 16 +++++++++++----- .../clients/deploy/DockerImageConfiguration.java | 14 ++++++++++---- .../properties/SleeperPropertyValues.java | 10 +++++++--- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/java/clients/src/main/java/sleeper/clients/admin/properties/AdminClientPropertiesStore.java b/java/clients/src/main/java/sleeper/clients/admin/properties/AdminClientPropertiesStore.java index ac700837c3..cb90c1b59c 100644 --- a/java/clients/src/main/java/sleeper/clients/admin/properties/AdminClientPropertiesStore.java +++ b/java/clients/src/main/java/sleeper/clients/admin/properties/AdminClientPropertiesStore.java @@ -29,12 +29,14 @@ import sleeper.clients.util.cdk.CdkCommand; import sleeper.clients.util.cdk.InvokeCdkForInstance; import sleeper.clients.util.console.ConsoleOutput; +import sleeper.configuration.properties.SleeperPropertyValues; import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.instance.InstanceProperty; import sleeper.configuration.properties.local.SaveLocalProperties; import sleeper.configuration.properties.table.S3TableProperties; import sleeper.configuration.properties.table.TableProperties; import sleeper.configuration.properties.table.TablePropertiesProvider; +import sleeper.configuration.properties.validation.OptionalStack; import sleeper.core.statestore.StateStore; import sleeper.core.table.TableNotFoundException; import sleeper.statestore.StateStoreFactory; @@ -42,13 +44,13 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.util.HashSet; import java.util.List; import java.util.Optional; import java.util.Set; import java.util.stream.Stream; -import static sleeper.configuration.properties.SleeperPropertyValues.readList; +import static java.util.function.Predicate.not; +import static java.util.stream.Collectors.toUnmodifiableSet; import static sleeper.configuration.properties.instance.CommonProperty.ID; import static sleeper.configuration.properties.instance.CommonProperty.OPTIONAL_STACKS; import static sleeper.configuration.properties.instance.InstanceProperties.loadPropertiesFromS3GivenInstanceId; @@ -137,9 +139,13 @@ private boolean shouldUploadDockerImages(PropertiesDiff diff) { return false; } PropertyDiff stackDiff = stackDiffOptional.get(); - Set stacksBefore = new HashSet<>(readList(stackDiff.getOldValue())); - Set newStacks = new HashSet<>(readList(stackDiff.getNewValue())); - newStacks.removeAll(stacksBefore); + Set stacksBefore = SleeperPropertyValues + .streamEnumList(OPTIONAL_STACKS, stackDiff.getOldValue(), OptionalStack.class) + .collect(toUnmodifiableSet()); + Set newStacks = SleeperPropertyValues + .streamEnumList(OPTIONAL_STACKS, stackDiff.getNewValue(), OptionalStack.class) + .filter(not(stacksBefore::contains)) + .collect(toUnmodifiableSet()); return !dockerImageConfiguration.getStacksToDeploy(newStacks).isEmpty(); } diff --git a/java/clients/src/main/java/sleeper/clients/deploy/DockerImageConfiguration.java b/java/clients/src/main/java/sleeper/clients/deploy/DockerImageConfiguration.java index c0e04e85ff..19b1a6b078 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/DockerImageConfiguration.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/DockerImageConfiguration.java @@ -48,11 +48,17 @@ public DockerImageConfiguration(Map imageByStac this.imageByStack = imageByStack; } - public List getStacksToDeploy(Collection stacks) { - return getStacksToDeploy(stacks, List.of()); + public List getStacksToDeploy(Collection stacks) { + return getStacksToDeployFromEnum(stacks, List.of()); } public List getStacksToDeploy(Collection stacks, List extraDockerImages) { + return getStacksToDeployFromEnum(stacks.stream() + .map(str -> EnumUtils.getEnumIgnoreCase(OptionalStack.class, str)) + .collect(toUnmodifiableList()), extraDockerImages); + } + + public List getStacksToDeployFromEnum(Collection stacks, List extraDockerImages) { return Stream.concat( stacks.stream() .map(this::getStackImage) @@ -61,8 +67,8 @@ public List getStacksToDeploy(Collection stacks, List< .collect(toUnmodifiableList()); } - private Optional getStackImage(String stack) { - return Optional.ofNullable(imageByStack.get(EnumUtils.getEnumIgnoreCase(OptionalStack.class, stack))); + private Optional getStackImage(OptionalStack stack) { + return Optional.ofNullable(imageByStack.get(stack)); } public Optional getInstanceIdFromRepoName(String repositoryName) { diff --git a/java/configuration/src/main/java/sleeper/configuration/properties/SleeperPropertyValues.java b/java/configuration/src/main/java/sleeper/configuration/properties/SleeperPropertyValues.java index 63d351293a..27880a79bf 100644 --- a/java/configuration/src/main/java/sleeper/configuration/properties/SleeperPropertyValues.java +++ b/java/configuration/src/main/java/sleeper/configuration/properties/SleeperPropertyValues.java @@ -66,9 +66,7 @@ default > List getEnumList(T property, Class enumClass) } default > Stream streamEnumList(T property, Class enumClass) { - return getList(property).stream() - .map(value -> Optional.ofNullable(EnumUtils.getEnumIgnoreCase(enumClass, value)) - .orElseThrow(() -> new IllegalArgumentException("Unrecognised value for " + property + ": " + value))); + return streamEnumList(property, get(property), enumClass); } default > E getEnumValue(T property, Class enumClass) { @@ -85,4 +83,10 @@ static List readList(String value) { return Lists.newArrayList(value.split(",")); } } + + static > Stream streamEnumList(SleeperProperty property, String value, Class enumClass) { + return readList(value).stream() + .map(item -> Optional.ofNullable(EnumUtils.getEnumIgnoreCase(enumClass, item)) + .orElseThrow(() -> new IllegalArgumentException("Unrecognised value for " + property + ": " + item))); + } } From 298d74ae330ed2a2ae30a5bb018366b5cfff9455 Mon Sep 17 00:00:00 2001 From: patchwork01 <110390516+patchwork01@users.noreply.github.com> Date: Tue, 17 Sep 2024 13:16:38 +0000 Subject: [PATCH 14/22] Change type of field StacksForDockerUpload.stacks --- .../clients/deploy/StacksForDockerUpload.java | 14 ++++++++++---- .../sleeper/clients/deploy/UploadDockerImages.java | 2 +- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/java/clients/src/main/java/sleeper/clients/deploy/StacksForDockerUpload.java b/java/clients/src/main/java/sleeper/clients/deploy/StacksForDockerUpload.java index 5a968b76eb..046b700c82 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/StacksForDockerUpload.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/StacksForDockerUpload.java @@ -16,13 +16,17 @@ package sleeper.clients.deploy; +import org.apache.commons.lang3.EnumUtils; + import sleeper.configuration.properties.instance.InstanceProperties; +import sleeper.configuration.properties.validation.OptionalStack; import java.util.List; import java.util.Objects; import java.util.Optional; import static java.util.Objects.requireNonNull; +import static java.util.stream.Collectors.toUnmodifiableList; import static sleeper.configuration.properties.instance.CdkDefinedInstanceProperty.VERSION; import static sleeper.configuration.properties.instance.CommonProperty.ACCOUNT; import static sleeper.configuration.properties.instance.CommonProperty.ECR_REPOSITORY_PREFIX; @@ -35,7 +39,7 @@ public class StacksForDockerUpload { private final String account; private final String region; private final String version; - private final List stacks; + private final List stacks; private StacksForDockerUpload(Builder builder) { ecrPrefix = requireNonNull(builder.ecrPrefix, "ecrPrefix must not be null"); @@ -79,7 +83,7 @@ public String getVersion() { return version; } - public List getStacks() { + public List getStacks() { return stacks; } @@ -120,7 +124,7 @@ public static final class Builder { private String account; private String region; private String version; - private List stacks; + private List stacks; private Builder() { } @@ -146,7 +150,9 @@ public Builder version(String version) { } public Builder stacks(List stacks) { - this.stacks = stacks; + this.stacks = stacks.stream() + .map(str -> EnumUtils.getEnumIgnoreCase(OptionalStack.class, str)) + .collect(toUnmodifiableList()); return this; } diff --git a/java/clients/src/main/java/sleeper/clients/deploy/UploadDockerImages.java b/java/clients/src/main/java/sleeper/clients/deploy/UploadDockerImages.java index 6604679727..dba6cbdc10 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/UploadDockerImages.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/UploadDockerImages.java @@ -58,7 +58,7 @@ public void upload(CommandPipelineRunner runCommand, StacksForDockerUpload data) public void upload(CommandPipelineRunner runCommand, StacksForDockerUpload data, List extraDockerImages) throws IOException, InterruptedException { String repositoryHost = String.format("%s.dkr.ecr.%s.amazonaws.com", data.getAccount(), data.getRegion()); - List stacksToUpload = dockerImageConfig.getStacksToDeploy(data.getStacks(), extraDockerImages); + List stacksToUpload = dockerImageConfig.getStacksToDeployFromEnum(data.getStacks(), extraDockerImages); List stacksToBuild = stacksToUpload.stream() .filter(stackDockerImage -> imageDoesNotExistInRepositoryWithVersion(stackDockerImage, data)) .collect(Collectors.toUnmodifiableList()); From c9b6e64b32b22312d21b76f44922fcbd04079c23 Mon Sep 17 00:00:00 2001 From: patchwork01 <110390516+patchwork01@users.noreply.github.com> Date: Tue, 17 Sep 2024 13:19:39 +0000 Subject: [PATCH 15/22] Pass OptionalStack into StacksForDockerUpload --- .../clients/deploy/StacksForDockerUpload.java | 12 ++++-------- .../properties/AdminClientPropertiesStoreIT.java | 7 ++++--- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/java/clients/src/main/java/sleeper/clients/deploy/StacksForDockerUpload.java b/java/clients/src/main/java/sleeper/clients/deploy/StacksForDockerUpload.java index 046b700c82..b41f5865e1 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/StacksForDockerUpload.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/StacksForDockerUpload.java @@ -16,8 +16,6 @@ package sleeper.clients.deploy; -import org.apache.commons.lang3.EnumUtils; - import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.validation.OptionalStack; @@ -26,7 +24,6 @@ import java.util.Optional; import static java.util.Objects.requireNonNull; -import static java.util.stream.Collectors.toUnmodifiableList; import static sleeper.configuration.properties.instance.CdkDefinedInstanceProperty.VERSION; import static sleeper.configuration.properties.instance.CommonProperty.ACCOUNT; import static sleeper.configuration.properties.instance.CommonProperty.ECR_REPOSITORY_PREFIX; @@ -64,7 +61,8 @@ public static StacksForDockerUpload from(InstanceProperties properties, String v .account(properties.get(ACCOUNT)) .region(properties.get(REGION)) .version(version) - .stacks(properties.getList(OPTIONAL_STACKS)).build(); + .stacks(properties.getEnumList(OPTIONAL_STACKS, OptionalStack.class)) + .build(); } public String getEcrPrefix() { @@ -149,10 +147,8 @@ public Builder version(String version) { return this; } - public Builder stacks(List stacks) { - this.stacks = stacks.stream() - .map(str -> EnumUtils.getEnumIgnoreCase(OptionalStack.class, str)) - .collect(toUnmodifiableList()); + public Builder stacks(List stacks) { + this.stacks = stacks; return this; } diff --git a/java/clients/src/test/java/sleeper/clients/admin/properties/AdminClientPropertiesStoreIT.java b/java/clients/src/test/java/sleeper/clients/admin/properties/AdminClientPropertiesStoreIT.java index 654f1d3202..9b595400b9 100644 --- a/java/clients/src/test/java/sleeper/clients/admin/properties/AdminClientPropertiesStoreIT.java +++ b/java/clients/src/test/java/sleeper/clients/admin/properties/AdminClientPropertiesStoreIT.java @@ -28,6 +28,7 @@ import sleeper.configuration.properties.instance.InstanceProperty; import sleeper.configuration.properties.table.TableProperties; import sleeper.configuration.properties.table.TableProperty; +import sleeper.configuration.properties.validation.OptionalStack; import java.io.IOException; import java.nio.file.Path; @@ -344,7 +345,7 @@ void shouldUploadDockerImagesWhenOneStackEnabled() throws IOException, Interrupt updateInstanceProperty(instanceId, OPTIONAL_STACKS, "QueryStack,CompactionStack,IngestStack"); // Then - verify(uploadDockerImages).upload(withStacks("QueryStack", "CompactionStack", "IngestStack")); + verify(uploadDockerImages).upload(withStacks(OptionalStack.QueryStack, OptionalStack.CompactionStack, OptionalStack.IngestStack)); } @Test @@ -371,7 +372,7 @@ void shouldUploadDockerImagesWhenOneStackIsEnabledAndAnotherStackIsDisabled() th updateInstanceProperty(instanceId, OPTIONAL_STACKS, "QueryStack,IngestStack"); // Then - verify(uploadDockerImages).upload(withStacks("QueryStack", "IngestStack")); + verify(uploadDockerImages).upload(withStacks(OptionalStack.QueryStack, OptionalStack.IngestStack)); } @Test @@ -388,7 +389,7 @@ private void updateInstanceProperty(String instanceId, InstanceProperty property updateInstanceProperty(store(), instanceId, property, value); } - private StacksForDockerUpload withStacks(String... stacks) { + private StacksForDockerUpload withStacks(OptionalStack... stacks) { return StacksForDockerUpload.builder() .ecrPrefix(instanceProperties.get(ID)) .account(instanceProperties.get(ACCOUNT)) From eac888a687d65a417788acd6db4dbe131a72b333 Mon Sep 17 00:00:00 2001 From: patchwork01 <110390516+patchwork01@users.noreply.github.com> Date: Tue, 17 Sep 2024 13:20:57 +0000 Subject: [PATCH 16/22] Only use enum in DockerImageConfiguration --- .../clients/deploy/DockerImageConfiguration.java | 12 ++---------- .../sleeper/clients/deploy/UploadDockerImages.java | 2 +- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/java/clients/src/main/java/sleeper/clients/deploy/DockerImageConfiguration.java b/java/clients/src/main/java/sleeper/clients/deploy/DockerImageConfiguration.java index 19b1a6b078..7921ba6b4a 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/DockerImageConfiguration.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/DockerImageConfiguration.java @@ -16,8 +16,6 @@ package sleeper.clients.deploy; -import org.apache.commons.lang3.EnumUtils; - import sleeper.configuration.properties.validation.OptionalStack; import java.util.Collection; @@ -49,16 +47,10 @@ public DockerImageConfiguration(Map imageByStac } public List getStacksToDeploy(Collection stacks) { - return getStacksToDeployFromEnum(stacks, List.of()); - } - - public List getStacksToDeploy(Collection stacks, List extraDockerImages) { - return getStacksToDeployFromEnum(stacks.stream() - .map(str -> EnumUtils.getEnumIgnoreCase(OptionalStack.class, str)) - .collect(toUnmodifiableList()), extraDockerImages); + return getStacksToDeploy(stacks, List.of()); } - public List getStacksToDeployFromEnum(Collection stacks, List extraDockerImages) { + public List getStacksToDeploy(Collection stacks, List extraDockerImages) { return Stream.concat( stacks.stream() .map(this::getStackImage) diff --git a/java/clients/src/main/java/sleeper/clients/deploy/UploadDockerImages.java b/java/clients/src/main/java/sleeper/clients/deploy/UploadDockerImages.java index dba6cbdc10..6604679727 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/UploadDockerImages.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/UploadDockerImages.java @@ -58,7 +58,7 @@ public void upload(CommandPipelineRunner runCommand, StacksForDockerUpload data) public void upload(CommandPipelineRunner runCommand, StacksForDockerUpload data, List extraDockerImages) throws IOException, InterruptedException { String repositoryHost = String.format("%s.dkr.ecr.%s.amazonaws.com", data.getAccount(), data.getRegion()); - List stacksToUpload = dockerImageConfig.getStacksToDeployFromEnum(data.getStacks(), extraDockerImages); + List stacksToUpload = dockerImageConfig.getStacksToDeploy(data.getStacks(), extraDockerImages); List stacksToBuild = stacksToUpload.stream() .filter(stackDockerImage -> imageDoesNotExistInRepositoryWithVersion(stackDockerImage, data)) .collect(Collectors.toUnmodifiableList()); From 26f9dfd52d4cdf7d8f788bb5d11a995dbb4f620c Mon Sep 17 00:00:00 2001 From: patchwork01 <110390516+patchwork01@users.noreply.github.com> Date: Tue, 17 Sep 2024 13:23:27 +0000 Subject: [PATCH 17/22] Use OptionalStack in DeployDockerInstance --- .../java/sleeper/clients/docker/DeployDockerInstance.java | 3 ++- .../configuration/properties/validation/OptionalStack.java | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/java/clients/src/main/java/sleeper/clients/docker/DeployDockerInstance.java b/java/clients/src/main/java/sleeper/clients/docker/DeployDockerInstance.java index f44658f31d..cef124d071 100644 --- a/java/clients/src/main/java/sleeper/clients/docker/DeployDockerInstance.java +++ b/java/clients/src/main/java/sleeper/clients/docker/DeployDockerInstance.java @@ -33,6 +33,7 @@ import sleeper.clients.status.update.AddTable; import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.table.TableProperties; +import sleeper.configuration.properties.validation.OptionalStack; import sleeper.core.schema.Field; import sleeper.core.schema.Schema; import sleeper.core.schema.type.StringType; @@ -129,7 +130,7 @@ public void deploy(InstanceProperties instanceProperties, List private static void setForcedInstanceProperties(InstanceProperties instanceProperties) { String instanceId = instanceProperties.get(ID); instanceProperties.set(CONFIG_BUCKET, getConfigBucketFromInstanceId(instanceId)); - instanceProperties.set(OPTIONAL_STACKS, "IngestStack,CompactionStack,PartitionSplittingStack,QueryStack"); + instanceProperties.setEnumList(OPTIONAL_STACKS, OptionalStack.LOCALSTACK_STACKS); instanceProperties.set(ACCOUNT, "test-account"); instanceProperties.set(VPC_ID, "test-vpc"); instanceProperties.set(SUBNETS, "test-subnet"); diff --git a/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java b/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java index 34b677e63c..d8e467af60 100644 --- a/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java +++ b/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java @@ -68,6 +68,7 @@ public enum OptionalStack { EmrServerlessBulkImportStack, PersistentEmrBulkImportStack, EksBulkImportStack); + public static final List QUERY_STACKS = List.of( QueryStack, WebSocketQueryStack); @@ -85,6 +86,12 @@ public enum OptionalStack { TableMetricsStack, AthenaStack); + public static final List LOCALSTACK_STACKS = List.of( + IngestStack, + CompactionStack, + PartitionSplittingStack, + QueryStack); + public static boolean isValid(String value) { return SleeperPropertyValues.readList(value).stream() .allMatch(item -> EnumUtils.isValidEnumIgnoreCase(OptionalStack.class, item)); From 0230a8e65bafb1ce93fce4b9ef74210dc5c6aef6 Mon Sep 17 00:00:00 2001 From: patchwork01 <110390516+patchwork01@users.noreply.github.com> Date: Tue, 17 Sep 2024 13:26:28 +0000 Subject: [PATCH 18/22] Update test usages of OptionalStack --- .../clients/admin/InstanceConfigurationScreenTest.java | 5 +++-- .../admin/properties/AdminClientPropertiesStoreIT.java | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/java/clients/src/test/java/sleeper/clients/admin/InstanceConfigurationScreenTest.java b/java/clients/src/test/java/sleeper/clients/admin/InstanceConfigurationScreenTest.java index 54c9b5e419..51a6385bb9 100644 --- a/java/clients/src/test/java/sleeper/clients/admin/InstanceConfigurationScreenTest.java +++ b/java/clients/src/test/java/sleeper/clients/admin/InstanceConfigurationScreenTest.java @@ -32,6 +32,7 @@ import sleeper.configuration.properties.instance.InstancePropertyGroup; import sleeper.configuration.properties.table.TableProperties; import sleeper.configuration.properties.table.TablePropertyGroup; +import sleeper.configuration.properties.validation.OptionalStack; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; @@ -266,7 +267,7 @@ void shouldOrderKnownPropertiesInTheOrderTheyAreDefinedInTheirGroups() throws Ex // Given InstanceProperties before = createValidInstanceProperties(); InstanceProperties after = InstanceProperties.copyOf(before); - after.set(OPTIONAL_STACKS, "CompactionStack"); + after.setEnum(OPTIONAL_STACKS, OptionalStack.CompactionStack); after.set(DEFAULT_S3A_READAHEAD_RANGE, "123"); after.set(DEFAULT_PAGE_SIZE, "456"); @@ -285,7 +286,7 @@ void shouldOrderUnknownPropertiesAfterKnownProperties() throws Exception { // Given InstanceProperties before = createValidInstanceProperties(); InstanceProperties after = InstanceProperties.copyOf(before); - after.set(OPTIONAL_STACKS, "CompactionStack"); + after.setEnum(OPTIONAL_STACKS, OptionalStack.CompactionStack); after.getProperties().setProperty("some.unknown.property", "a-value"); after.getProperties().setProperty("an.unknown.property", "other-value"); diff --git a/java/clients/src/test/java/sleeper/clients/admin/properties/AdminClientPropertiesStoreIT.java b/java/clients/src/test/java/sleeper/clients/admin/properties/AdminClientPropertiesStoreIT.java index 9b595400b9..b96d84d086 100644 --- a/java/clients/src/test/java/sleeper/clients/admin/properties/AdminClientPropertiesStoreIT.java +++ b/java/clients/src/test/java/sleeper/clients/admin/properties/AdminClientPropertiesStoreIT.java @@ -335,7 +335,7 @@ void shouldCreateGeneratedDirectoryWhenSavingTableProperties() { class UploadDockerImages { @BeforeEach void setup() { - instanceProperties.set(OPTIONAL_STACKS, "QueryStack,CompactionStack"); + instanceProperties.setEnumList(OPTIONAL_STACKS, List.of(OptionalStack.QueryStack, OptionalStack.CompactionStack)); instanceProperties.saveToS3(s3); } From d9b618142af2fefc0c1d24a78efa24630ac3da99 Mon Sep 17 00:00:00 2001 From: patchwork01 <110390516+patchwork01@users.noreply.github.com> Date: Tue, 17 Sep 2024 13:31:13 +0000 Subject: [PATCH 19/22] Refactor SystemTestOptionalStacks --- .../instance/SystemTestOptionalStacks.java | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/SystemTestOptionalStacks.java b/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/SystemTestOptionalStacks.java index f99745c955..91593c7bca 100644 --- a/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/SystemTestOptionalStacks.java +++ b/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/SystemTestOptionalStacks.java @@ -16,11 +16,14 @@ package sleeper.systemtest.dsl.instance; +import org.apache.commons.lang3.EnumUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sleeper.configuration.properties.instance.InstanceProperties; +import sleeper.configuration.properties.validation.OptionalStack; +import java.util.ArrayList; import java.util.LinkedHashSet; import java.util.Set; import java.util.function.Consumer; @@ -37,25 +40,37 @@ public SystemTestOptionalStacks(SystemTestInstanceContext instance) { } public void addOptionalStack(Class stackClass) { - LOGGER.info("Adding optional stack: {}", stackClass); - updateOptionalStacks(stacks -> stacks.add(stackClass.getSimpleName())); + addOptionalStack(stack(stackClass)); } public void removeOptionalStack(Class stackClass) { - LOGGER.info("Removing optional stack: {}", stackClass); - updateOptionalStacks(stacks -> stacks.remove(stackClass.getSimpleName())); + removeOptionalStack(stack(stackClass)); } - private void updateOptionalStacks(Consumer> update) { + public void addOptionalStack(OptionalStack stack) { + LOGGER.info("Adding optional stack: {}", stack); + updateOptionalStacks(stacks -> stacks.add(stack)); + } + + public void removeOptionalStack(OptionalStack stack) { + LOGGER.info("Removing optional stack: {}", stack); + updateOptionalStacks(stacks -> stacks.remove(stack)); + } + + private OptionalStack stack(Class stackClass) { + return EnumUtils.getEnumIgnoreCase(OptionalStack.class, stackClass.getSimpleName()); + } + + private void updateOptionalStacks(Consumer> update) { InstanceProperties properties = instance.getInstanceProperties(); - Set optionalStacks = new LinkedHashSet<>(properties.getList(OPTIONAL_STACKS)); - Set before = new LinkedHashSet<>(optionalStacks); + Set optionalStacks = new LinkedHashSet<>(properties.getEnumList(OPTIONAL_STACKS, OptionalStack.class)); + Set before = new LinkedHashSet<>(optionalStacks); update.accept(optionalStacks); if (before.equals(optionalStacks)) { LOGGER.info("Optional stacks unchanged, not redeploying"); return; } - properties.set(OPTIONAL_STACKS, String.join(",", optionalStacks)); + properties.setEnumList(OPTIONAL_STACKS, new ArrayList<>(optionalStacks)); instance.redeployCurrentInstance(); } } From 3ffdc43f130cc8128d2d9e790dd1360732e3dce4 Mon Sep 17 00:00:00 2001 From: patchwork01 <110390516+patchwork01@users.noreply.github.com> Date: Tue, 17 Sep 2024 13:32:34 +0000 Subject: [PATCH 20/22] Use OptionalStack to enable/disable in system tests --- .../java/sleeper/systemtest/dsl/SleeperSystemTest.java | 9 +++++---- .../java/sleeper/systemtest/suite/EksBulkImportST.java | 6 +++--- .../systemtest/suite/EmrPersistentBulkImportST.java | 6 +++--- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/SleeperSystemTest.java b/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/SleeperSystemTest.java index d800c1f49d..467262673b 100644 --- a/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/SleeperSystemTest.java +++ b/java/system-test/system-test-dsl/src/main/java/sleeper/systemtest/dsl/SleeperSystemTest.java @@ -19,6 +19,7 @@ import sleeper.configuration.properties.instance.InstanceProperties; import sleeper.configuration.properties.table.TableProperties; import sleeper.configuration.properties.table.TableProperty; +import sleeper.configuration.properties.validation.OptionalStack; import sleeper.core.record.Record; import sleeper.core.schema.Schema; import sleeper.systemtest.dsl.compaction.SystemTestCompaction; @@ -170,12 +171,12 @@ public Path getSplitPointsDirectory() { .resolve("test/splitpoints"); } - public void enableOptionalStack(Class stackClass) { - new SystemTestOptionalStacks(context.instance()).addOptionalStack(stackClass); + public void enableOptionalStack(OptionalStack stack) { + new SystemTestOptionalStacks(context.instance()).addOptionalStack(stack); } - public void disableOptionalStack(Class stackClass) { - new SystemTestOptionalStacks(context.instance()).removeOptionalStack(stackClass); + public void disableOptionalStack(OptionalStack stack) { + new SystemTestOptionalStacks(context.instance()).removeOptionalStack(stack); } public SystemTestStateStore stateStore() { diff --git a/java/system-test/system-test-suite/src/test/java/sleeper/systemtest/suite/EksBulkImportST.java b/java/system-test/system-test-suite/src/test/java/sleeper/systemtest/suite/EksBulkImportST.java index 0ac54407b1..09ec83c407 100644 --- a/java/system-test/system-test-suite/src/test/java/sleeper/systemtest/suite/EksBulkImportST.java +++ b/java/system-test/system-test-suite/src/test/java/sleeper/systemtest/suite/EksBulkImportST.java @@ -20,7 +20,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import sleeper.cdk.stack.bulkimport.EksBulkImportStack; +import sleeper.configuration.properties.validation.OptionalStack; import sleeper.systemtest.dsl.SleeperSystemTest; import sleeper.systemtest.dsl.extension.AfterTestReports; import sleeper.systemtest.dsl.reporting.SystemTestReports; @@ -50,13 +50,13 @@ public class EksBulkImportST { @BeforeEach void setUp(SleeperSystemTest sleeper, AfterTestReports reporting) { sleeper.connectToInstance(MAIN); - sleeper.enableOptionalStack(EksBulkImportStack.class); + sleeper.enableOptionalStack(OptionalStack.EksBulkImportStack); reporting.reportIfTestFailed(SystemTestReports.SystemTestBuilder::ingestJobs); } @AfterEach void tearDown(SleeperSystemTest sleeper) { - sleeper.disableOptionalStack(EksBulkImportStack.class); + sleeper.disableOptionalStack(OptionalStack.EksBulkImportStack); } @Test diff --git a/java/system-test/system-test-suite/src/test/java/sleeper/systemtest/suite/EmrPersistentBulkImportST.java b/java/system-test/system-test-suite/src/test/java/sleeper/systemtest/suite/EmrPersistentBulkImportST.java index 6a0cfab146..1e8654aab5 100644 --- a/java/system-test/system-test-suite/src/test/java/sleeper/systemtest/suite/EmrPersistentBulkImportST.java +++ b/java/system-test/system-test-suite/src/test/java/sleeper/systemtest/suite/EmrPersistentBulkImportST.java @@ -20,7 +20,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import sleeper.cdk.stack.bulkimport.PersistentEmrBulkImportStack; +import sleeper.configuration.properties.validation.OptionalStack; import sleeper.systemtest.dsl.SleeperSystemTest; import sleeper.systemtest.dsl.extension.AfterTestReports; import sleeper.systemtest.dsl.reporting.SystemTestReports; @@ -51,7 +51,7 @@ public class EmrPersistentBulkImportST { @BeforeEach void setUp(SleeperSystemTest sleeper, AfterTestReports reporting) { sleeper.connectToInstance(MAIN); - sleeper.enableOptionalStack(PersistentEmrBulkImportStack.class); + sleeper.enableOptionalStack(OptionalStack.PersistentEmrBulkImportStack); reporting.reportAlways(SystemTestReports.SystemTestBuilder::ingestJobs); // Note that we don't purge the bulk import job queue when the test fails, // because it is deleted when the optional stack is disabled. @@ -59,7 +59,7 @@ void setUp(SleeperSystemTest sleeper, AfterTestReports reporting) { @AfterEach void tearDown(SleeperSystemTest sleeper) { - sleeper.disableOptionalStack(PersistentEmrBulkImportStack.class); + sleeper.disableOptionalStack(OptionalStack.PersistentEmrBulkImportStack); } @Test From adec1ffdcd3174025446b378023fd89d823be8a4 Mon Sep 17 00:00:00 2001 From: patchwork01 <110390516+patchwork01@users.noreply.github.com> Date: Tue, 17 Sep 2024 13:34:57 +0000 Subject: [PATCH 21/22] Test case insensitivity in OptionalStackTest --- .../properties/validation/OptionalStackTest.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/java/configuration/src/test/java/sleeper/configuration/properties/validation/OptionalStackTest.java b/java/configuration/src/test/java/sleeper/configuration/properties/validation/OptionalStackTest.java index d904130751..d00ab426fa 100644 --- a/java/configuration/src/test/java/sleeper/configuration/properties/validation/OptionalStackTest.java +++ b/java/configuration/src/test/java/sleeper/configuration/properties/validation/OptionalStackTest.java @@ -61,6 +61,16 @@ void shouldValidateWithTwoValidOptionalStacks() { .containsExactly(CompactionStack, GarbageCollectorStack); } + @Test + void shouldValidateWithOneStackNotMatchingCase() { + InstanceProperties properties = createTestInstanceProperties(); + properties.set(OPTIONAL_STACKS, "compactionstack"); + + assertThatCode(properties::validate).doesNotThrowAnyException(); + assertThat(properties.getEnumList(OPTIONAL_STACKS, OptionalStack.class)) + .containsExactly(CompactionStack); + } + @Test void shouldValidateWithEmptyProperty() { InstanceProperties properties = createTestInstanceProperties(); From 6e7bf2e2c2cf953f3d02d3c9ca0ad9c31104fe2d Mon Sep 17 00:00:00 2001 From: patchwork01 <110390516+patchwork01@users.noreply.github.com> Date: Tue, 17 Sep 2024 14:23:07 +0000 Subject: [PATCH 22/22] Update description of optional stacks property --- example/basic/instance.properties | 6 +++++- example/full/instance.properties | 6 +++++- .../src/main/java/sleeper/configuration/Utils.java | 6 +++++- .../configuration/properties/instance/CommonProperty.java | 3 ++- .../configuration/properties/validation/OptionalStack.java | 2 +- scripts/templates/instanceproperties.template | 6 +++++- 6 files changed, 23 insertions(+), 6 deletions(-) diff --git a/example/basic/instance.properties b/example/basic/instance.properties index 8bab5eec21..7a2ad83532 100644 --- a/example/basic/instance.properties +++ b/example/basic/instance.properties @@ -18,7 +18,11 @@ sleeper.jars.bucket=the name of the bucket containing your jars, e.g. sleeper-> String describeEnumValuesInLowerCase(Class .map(String::toLowerCase).collect(Collectors.toList()).toString(); } + public static > String describeEnumValues(Class cls) { + return Stream.of(cls.getEnumConstants()).map(Enum::toString).collect(Collectors.toList()).toString(); + } + } diff --git a/java/configuration/src/main/java/sleeper/configuration/properties/instance/CommonProperty.java b/java/configuration/src/main/java/sleeper/configuration/properties/instance/CommonProperty.java index 58f2895f71..4a32c21a30 100644 --- a/java/configuration/src/main/java/sleeper/configuration/properties/instance/CommonProperty.java +++ b/java/configuration/src/main/java/sleeper/configuration/properties/instance/CommonProperty.java @@ -66,7 +66,8 @@ public interface CommonProperty { .runCdkDeployWhenChanged(true) .includedInBasicTemplate(true).build(); UserDefinedInstanceProperty OPTIONAL_STACKS = Index.propertyBuilder("sleeper.optional.stacks") - .description("The optional stacks to deploy.") + .description("The optional stacks to deploy. Not case sensitive.\n" + + "Valid values: " + Utils.describeEnumValues(OptionalStack.class)) .defaultValue(OptionalStack.getDefaultValue()) .validationPredicate(OptionalStack::isValid) .propertyGroup(InstancePropertyGroup.COMMON) diff --git a/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java b/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java index d8e467af60..d261d6e279 100644 --- a/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java +++ b/java/configuration/src/main/java/sleeper/configuration/properties/validation/OptionalStack.java @@ -30,8 +30,8 @@ public enum OptionalStack { IngestBatcherStack, // Bulk import - EmrBulkImportStack, EmrServerlessBulkImportStack, + EmrBulkImportStack, PersistentEmrBulkImportStack, EksBulkImportStack, EmrStudioStack, diff --git a/scripts/templates/instanceproperties.template b/scripts/templates/instanceproperties.template index 2d2c98e914..c3f7b13f9d 100644 --- a/scripts/templates/instanceproperties.template +++ b/scripts/templates/instanceproperties.template @@ -77,7 +77,11 @@ sleeper.stack.tag.name=DeploymentStack # instance is destroyed. sleeper.retain.infra.after.destroy=true -# The optional stacks to deploy. +# The optional stacks to deploy. Not case sensitive. +# Valid values: [IngestStack, IngestBatcherStack, EmrServerlessBulkImportStack, EmrBulkImportStack, +# PersistentEmrBulkImportStack, EksBulkImportStack, EmrStudioStack, QueryStack, WebSocketQueryStack, +# AthenaStack, KeepLambdaWarmStack, CompactionStack, GarbageCollectorStack, PartitionSplittingStack, +# DashboardStack, TableMetricsStack] sleeper.optional.stacks=CompactionStack,GarbageCollectorStack,IngestStack,IngestBatcherStack,PartitionSplittingStack,QueryStack,AthenaStack,EmrServerlessBulkImportStack,EmrStudioStack,DashboardStack,TableMetricsStack # Whether to check that the VPC that the instance is deployed to has an S3 endpoint. If there is no S3