Merge pull request #3572 from gchq/3571-random-nightly-test-names

Issue 3571 - Avoid conflicting nightly test names
gchq · Oct 28, 2024 · 5fc32b7 · 5fc32b7
2 parents 055e57e + 4476a4d
commit 5fc32b7
Show file tree

Hide file tree

Showing 12 changed files with 72 additions and 11 deletions.
diff --git a/java/cdk-environment/src/main/java/sleeper/environment/cdk/buildec2/BuildEC2Parameters.java b/java/cdk-environment/src/main/java/sleeper/environment/cdk/buildec2/BuildEC2Parameters.java
@@ -27,6 +27,7 @@
 
 import static java.util.stream.Collectors.toUnmodifiableList;
 import static sleeper.environment.cdk.config.AppParameters.NIGHTLY_TEST_BUCKET;
+import static sleeper.environment.cdk.config.AppParameters.NIGHTLY_TEST_DEPLOY_ID;
 import static sleeper.environment.cdk.config.AppParameters.NIGHTLY_TEST_RUN_ENABLED;
 import static sleeper.environment.cdk.config.AppParameters.NIGHTLY_TEST_RUN_HOUR_UTC;
 import static sleeper.environment.cdk.config.AppParameters.NIGHTLY_TEST_SUBNETS;
@@ -43,6 +44,7 @@ public class BuildEC2Parameters {
     private final String branch;
     private final BuildEC2Image image;
     private final boolean nightlyTestEnabled;
+    private final String nightlyTestDeployId;
     private final String testHour;
     private final String testBucket;
     private final String vpc;
@@ -56,6 +58,11 @@ private BuildEC2Parameters(Builder builder) {
         image = BuildEC2Image.from(context);
         nightlyTestEnabled = context.get(NIGHTLY_TEST_RUN_ENABLED);
         if (nightlyTestEnabled) {
+            nightlyTestDeployId = context.get(NIGHTLY_TEST_DEPLOY_ID)
+                    .orElseThrow(() -> new IllegalArgumentException("nightlyTestDeployId must be set (up to 2 characters)"));
+            if (nightlyTestDeployId.length() > 2) {
+                throw new IllegalArgumentException("nightlyTestDeployId must be at most 2 characters long");
+            }
             testHour = "" + context.get(NIGHTLY_TEST_RUN_HOUR_UTC);
             testBucket = context.get(NIGHTLY_TEST_BUCKET)
                     .orElseGet(() -> Objects.requireNonNull(builder.testBucket, "testBucket must not be null"));
@@ -66,6 +73,7 @@ private BuildEC2Parameters(Builder builder) {
             }
             subnets = String.join(",", subnetsList);
         } else {
+            nightlyTestDeployId = null;
             testHour = null;
             testBucket = null;
             vpc = null;
@@ -95,6 +103,7 @@ String fillUserDataTemplate(String template) {
             return noNightlyTests;
         }
         return noNightlyTests
+                .replace("${deployId}", nightlyTestDeployId)
                 .replace("${testHour}", testHour)
                 .replace("${testBucket}", testBucket)
                 .replace("${vpc}", vpc)

diff --git a/java/cdk-environment/src/main/java/sleeper/environment/cdk/config/AppParameters.java b/java/cdk-environment/src/main/java/sleeper/environment/cdk/config/AppParameters.java
@@ -39,6 +39,7 @@ private AppParameters() {
     public static final StringListParameter AUTO_SHUTDOWN_EXISTING_EC2_IDS = StringListParameter.key("autoShutdownExistingEc2Ids");
     public static final IntParameter AUTO_SHUTDOWN_HOUR_UTC = IntParameter.keyAndDefault("autoShutdownHourUtc", 19);
     public static final BooleanParameter NIGHTLY_TEST_RUN_ENABLED = BooleanParameter.keyAndDefault("nightlyTestsEnabled", false);
+    public static final OptionalStringParameter NIGHTLY_TEST_DEPLOY_ID = OptionalStringParameter.key("nightlyTestDeployId");
     public static final IntParameter NIGHTLY_TEST_RUN_HOUR_UTC = IntParameter.keyAndDefault("nightlyTestHourUtc", 3);
     public static final OptionalStringParameter NIGHTLY_TEST_BUCKET = OptionalStringParameter.key("nightlyTestBucket");
     public static final StringListParameter NIGHTLY_TEST_SUBNETS = StringListParameter.key("subnetIds");

diff --git a/java/cdk-environment/src/main/resources/nightlyTestSettings.json b/java/cdk-environment/src/main/resources/nightlyTestSettings.json
@@ -1,4 +1,5 @@
 {
+  "deployId": "${deployId}",
   "vpc": "${vpc}",
   "subnets": "${subnets}",
   "resultsBucket": "${testBucket}",

diff --git a/...dk-environment/src/test/java/sleeper/environment/cdk/buildec2/BuildEC2ParametersTest.java b/...dk-environment/src/test/java/sleeper/environment/cdk/buildec2/BuildEC2ParametersTest.java
@@ -20,10 +20,12 @@
 import sleeper.environment.cdk.config.AppContext;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static sleeper.environment.cdk.buildec2.BuildEC2Image.LOGIN_USER;
 import static sleeper.environment.cdk.buildec2.BuildEC2Parameters.BRANCH;
 import static sleeper.environment.cdk.buildec2.BuildEC2Parameters.FORK;
 import static sleeper.environment.cdk.buildec2.BuildEC2Parameters.REPOSITORY;
+import static sleeper.environment.cdk.config.AppParameters.NIGHTLY_TEST_DEPLOY_ID;
 import static sleeper.environment.cdk.config.AppParameters.NIGHTLY_TEST_RUN_ENABLED;
 import static sleeper.environment.cdk.config.AppParameters.NIGHTLY_TEST_SUBNETS;
 import static sleeper.environment.cdk.config.AppParameters.VPC_ID;
@@ -73,17 +75,20 @@ void shouldFillNightlyTestSettings() {
         assertThat(BuildEC2Parameters.builder()
                 .context(AppContext.of(
                         NIGHTLY_TEST_RUN_ENABLED.value(true),
+                        NIGHTLY_TEST_DEPLOY_ID.value("mt"),
                         VPC_ID.value("my-vpc"),
                         NIGHTLY_TEST_SUBNETS.value("subnet-1,subnet-2"),
                         FORK.value("my-fork"),
                         REPOSITORY.value("my-repo")))
                 .testBucket("nightly-test-results")
                 .build().fillUserDataTemplate("{" +
+                        "\"deployId\": \"${deployId}\"," +
                         "\"vpc\":\"${vpc}\"," +
                         "\"subnets\":\"${subnets}\"," +
                         "\"resultsBucket\":\"${testBucket}\"," +
                         "\"repoPath\":\"${fork}/${repository}\"}"))
                 .isEqualTo("{" +
+                        "\"deployId\": \"mt\"," +
                         "\"vpc\":\"my-vpc\"," +
                         "\"subnets\":\"subnet-1,subnet-2\"," +
                         "\"resultsBucket\":\"nightly-test-results\"," +
@@ -96,15 +101,45 @@ void shouldFillNoNightlyTestSettings() {
                 .context(AppContext.empty())
                 .testBucket(null)
                 .build().fillUserDataTemplate("{" +
+                        "\"deployId\": \"${deployId}\"," +
                         "\"vpc\":\"${vpc}\"," +
                         "\"subnets\":\"${subnets}\"," +
                         "\"resultsBucket\":\"${testBucket}\"," +
                         "\"repoPath\":\"${fork}/${repository}\"}"))
                 .isEqualTo("{" +
+                        "\"deployId\": \"${deployId}\"," +
                         "\"vpc\":\"${vpc}\"," +
                         "\"subnets\":\"${subnets}\"," +
                         "\"resultsBucket\":\"${testBucket}\"," +
                         "\"repoPath\":\"gchq/sleeper\"}");
     }
 
+    @Test
+    void shouldRefuseTooLongNightlyTestDeployId() {
+        AppContext context = AppContext.of(
+                NIGHTLY_TEST_RUN_ENABLED.value(true),
+                NIGHTLY_TEST_DEPLOY_ID.value("abc"),
+                VPC_ID.value("my-vpc"),
+                NIGHTLY_TEST_SUBNETS.value("subnet-1,subnet-2"),
+                FORK.value("my-fork"),
+                REPOSITORY.value("my-repo"));
+
+        assertThatThrownBy(() -> BuildEC2Parameters.from(context))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessage("nightlyTestDeployId must be at most 2 characters long");
+    }
+
+    @Test
+    void shouldRefuseNoNightlyTestDeployId() {
+        AppContext context = AppContext.of(
+                NIGHTLY_TEST_RUN_ENABLED.value(true),
+                VPC_ID.value("my-vpc"),
+                NIGHTLY_TEST_SUBNETS.value("subnet-1,subnet-2"),
+                FORK.value("my-fork"),
+                REPOSITORY.value("my-repo"));
+
+        assertThatThrownBy(() -> BuildEC2Parameters.from(context))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessage("nightlyTestDeployId must be set (up to 2 characters)");
+    }
 }
diff --git a/.../cdk-environment/src/test/java/sleeper/environment/cdk/buildec2/LoadUserDataUtilTest.java b/.../cdk-environment/src/test/java/sleeper/environment/cdk/buildec2/LoadUserDataUtilTest.java
@@ -25,6 +25,7 @@
 import static sleeper.environment.cdk.buildec2.BuildEC2Parameters.FORK;
 import static sleeper.environment.cdk.buildec2.BuildEC2Parameters.REPOSITORY;
 import static sleeper.environment.cdk.config.AppParameters.NIGHTLY_TEST_BUCKET;
+import static sleeper.environment.cdk.config.AppParameters.NIGHTLY_TEST_DEPLOY_ID;
 import static sleeper.environment.cdk.config.AppParameters.NIGHTLY_TEST_RUN_ENABLED;
 import static sleeper.environment.cdk.config.AppParameters.NIGHTLY_TEST_RUN_HOUR_UTC;
 import static sleeper.environment.cdk.config.AppParameters.NIGHTLY_TEST_SUBNETS;
@@ -55,6 +56,7 @@ void shouldLoadUserDataWithNightlyTests() {
                 FORK.value("a-fork"),
                 BRANCH.value("feature/something"),
                 NIGHTLY_TEST_RUN_ENABLED.value(true),
+                NIGHTLY_TEST_DEPLOY_ID.value("mt"),
                 VPC_ID.value("my-vpc"),
                 NIGHTLY_TEST_SUBNETS.value("subnet-1", "subnet-2"),
                 NIGHTLY_TEST_BUCKET.value("my-bucket")))))
@@ -70,11 +72,13 @@ void shouldLoadUserDataWithNightlyTests() {
     void shouldLoadNightlyTestSettings() {
         assertThat(LoadUserDataUtil.nightlyTestSettingsJson(BuildEC2Parameters.from(AppContext.of(
                 NIGHTLY_TEST_RUN_ENABLED.value(true),
+                NIGHTLY_TEST_DEPLOY_ID.value("mt"),
                 VPC_ID.value("my-vpc"),
                 NIGHTLY_TEST_SUBNETS.value("subnet-1", "subnet-2"),
                 NIGHTLY_TEST_BUCKET.value("my-bucket"),
                 FORK.value("my-fork"),
                 REPOSITORY.value("my-repo")))))
+                .contains("\"deployId\": \"mt\"")
                 .contains("\"vpc\": \"my-vpc\"")
                 .contains("\"subnets\": \"subnet-1,subnet-2\"")
                 .contains("\"repoPath\": \"my-fork/my-repo\"")
@@ -85,6 +89,7 @@ void shouldLoadNightlyTestSettings() {
     void shouldLoadCrontab() {
         assertThat(LoadUserDataUtil.crontab(BuildEC2Parameters.from(AppContext.of(
                 NIGHTLY_TEST_RUN_ENABLED.value(true),
+                NIGHTLY_TEST_DEPLOY_ID.value("mt"),
                 NIGHTLY_TEST_RUN_HOUR_UTC.value(3),
                 LOGIN_USER.value("my-user"),
                 VPC_ID.value("my-vpc"),

diff --git a/...st-dsl/src/main/java/sleeper/systemtest/dsl/instance/SystemTestInstanceConfiguration.java b/...st-dsl/src/main/java/sleeper/systemtest/dsl/instance/SystemTestInstanceConfiguration.java
@@ -34,6 +34,7 @@ private SystemTestInstanceConfiguration(Builder builder) {
         deployConfig = builder.deployConfig;
         useSystemTestIngestSourceBucket = builder.useSystemTestIngestSourceBucket;
         disableTransactionLogSnapshots = builder.disableTransactionLogSnapshots;
+        // Combines with SystemTestParameters.shortTestId to create an instance ID within maximum length
         if (shortName.length() > 7) {
             throw new IllegalArgumentException("Instance shortName must not be longer than 7 characters");
         }

diff --git a/...t/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/SystemTestParameters.java b/...t/system-test-dsl/src/main/java/sleeper/systemtest/dsl/instance/SystemTestParameters.java
@@ -78,6 +78,10 @@ private SystemTestParameters(Builder builder) {
         forceStateStoreClassname = builder.forceStateStoreClassname;
         standalonePropertiesTemplate = Objects.requireNonNull(builder.standalonePropertiesTemplate, "standalonePropertiesTemplate must not be null");
         instancePropertiesOverrides = Objects.requireNonNull(builder.instancePropertiesOverrides, "instancePropertiesOverrides must not be null");
+        // Combines with SystemTestInstanceConfiguration.shortName to create an instance ID within maximum length
+        if (shortTestId.length() > 13) {
+            throw new IllegalArgumentException("shortTestId is too long, must be at most 13 characters: " + shortTestId);
+        }
     }
 
     public static Builder builder() {

diff --git a/scripts/cli/builder/Dockerfile b/scripts/cli/builder/Dockerfile
@@ -23,6 +23,7 @@ RUN apt-get update && apt-get install -y \
     python3-venv \
     zip \
     jq \
+    uuid-runtime \
     sudo \
     gcc g++ cmake make \
     pkg-config libssl-dev \

diff --git a/scripts/test/nightly/README.md b/scripts/test/nightly/README.md
@@ -29,7 +29,8 @@ cp /sleeper-builder/sleeper/scripts/test/nightly/nightlyTestSettings.json /sleep
 vim nightlyTestSettings.json
 ```
 
-You'll need to set a VPC, subnets, results S3 bucket and a path to your fork in GitHub.
+You'll need to set a deployment ID, a VPC, subnets, a results S3 bucket and a path to your fork in GitHub. The
+deployment ID is at most 2 characters, to avoid S3 naming conflicts between your deployment and any others.
 
 #### Automatic merge to main
 

diff --git a/scripts/test/nightly/nightlyTestSettings.json b/scripts/test/nightly/nightlyTestSettings.json
@@ -1,4 +1,5 @@
 {
+  "deployId": "my deployment ID (up to 2 characters to distinguish these tests from others running simultaneously)",
   "vpc": "my VPC ID",
   "subnets": "comma separated list of my subnet IDs",
   "resultsBucket": "S3 bucket name",

diff --git a/scripts/test/nightly/runTests.sh b/scripts/test/nightly/runTests.sh
@@ -22,17 +22,18 @@ MAVEN_DIR=$(cd "$SCRIPTS_DIR" && cd ../java && pwd)
 
 pushd "$SCRIPTS_DIR/test"
 
-if [ "$#" -lt 3 ]; then
-  echo "Usage: $0 <vpc> <csv-list-of-subnets> <results-bucket> <optional-test-type> <optional-maven-parameters>"
+if [ "$#" -lt 4 ]; then
+  echo "Usage: $0 <deploy-id> <vpc> <csv-list-of-subnets> <results-bucket> <optional-test-type> <optional-maven-parameters>"
   echo "Valid test types are: performance, functional"
   exit 1
 fi
 
-VPC=$1
-SUBNETS=$2
-RESULTS_BUCKET=$3
-MAIN_SUITE_NAME=$4
-shift 3
+DEPLOY_ID=$1
+VPC=$2
+SUBNETS=$3
+RESULTS_BUCKET=$4
+MAIN_SUITE_NAME=$5
+shift 4
 if [ "$MAIN_SUITE_NAME" == "performance" ]; then
   shift
   MAIN_SUITE_PARAMS=(-Dsleeper.system.test.cluster.enabled=true -DrunIT=NightlyPerformanceSystemTestSuite "$@")
@@ -109,8 +110,8 @@ runMavenSystemTests() {
     echo -n "$TEST_EXIT_CODE $SHORT_ID" > "$OUTPUT_DIR/$TEST_NAME.status"
 }
 
-runMavenSystemTests "mvn-$START_TIME_SHORT" $MAIN_SUITE_NAME "${MAIN_SUITE_PARAMS[@]}"
-runMavenSystemTests "dyn-$START_TIME_SHORT" $SECONDARY_SUITE_NAME "${SECONDARY_SUITE_PARAMS[@]}"
+runMavenSystemTests "${DEPLOY_ID}mvn${START_TIME_SHORT}" $MAIN_SUITE_NAME "${MAIN_SUITE_PARAMS[@]}"
+runMavenSystemTests "${DEPLOY_ID}dyn${START_TIME_SHORT}" $SECONDARY_SUITE_NAME "${SECONDARY_SUITE_PARAMS[@]}"
 
 echo "[$(time_str)] Uploading test output"
 java -cp "${SYSTEM_TEST_JAR}" \

diff --git a/scripts/test/nightly/updateAndRunTests.sh b/scripts/test/nightly/updateAndRunTests.sh
@@ -28,6 +28,7 @@ fi
 SETTINGS_FILE=$1
 TEST_TYPE=$2
 
+DEPLOY_ID=$(jq ".deployId" "$SETTINGS_FILE" --raw-output)
 VPC=$(jq ".vpc" "$SETTINGS_FILE" --raw-output)
 SUBNETS=$(jq ".subnets" "$SETTINGS_FILE" --raw-output)
 RESULTS_BUCKET=$(jq ".resultsBucket" "$SETTINGS_FILE" --raw-output)
@@ -41,7 +42,7 @@ git fetch
 git switch --discard-changes -C develop origin/develop
 
 set +e
-./runTests.sh "$VPC" "$SUBNETS" "$RESULTS_BUCKET" "$TEST_TYPE"
+./runTests.sh "$DEPLOY_ID" "$VPC" "$SUBNETS" "$RESULTS_BUCKET" "$TEST_TYPE"
 EXIT_CODE=$?
 set -e