Retain tests longer (#188)

* Added new tags for extra files from tests - Test files will be the yaml file, status file, and results files (these will be kept longer) - Test Extra files will be log files, environment variables, and all other files needed by the yaml or logged by the yaml * Added code to tag extra files correctly - Stdout, stderr, and all other files generated by the yaml will be tagged as Test Extra files and be tagged for deletion accordingly * Added code to tag extra files correctly - Additional files uploaded to the test, or additional files being copied from the old s3 location will be tagged as Test Extra files - Environment variable files will be tagged as Test Extra files - When changing from recurring to non-recurring, only yaml and test status files will be tagged as Test files, all others will be tagged as Extra files * Updated the bucket expiration so that test files are 2 years and all extra files are 6 months * Updated the integration tests to check the new test extra tags * Added code to validate that the logger files are tagged as extra * Added code to clear tests off of the calendar after 1 year (or configured) - if RUN_HISTORICAL_DELETE is true, then it will remove things off the calendar after DELETE_OLD_FILES_DAYS (365 default) days. * Cleaned up some of the logging on the historical delete * Made changes to allow testing faster locally
FamilySearch · Dec 28, 2023 · d9aa805 · d9aa805
1 parent 4ecb1d8
commit d9aa805
Show file tree

Hide file tree

Showing 8 changed files with 236 additions and 27 deletions.
diff --git a/agent/createtest/pewpewtest.spec.ts b/agent/createtest/pewpewtest.spec.ts
@@ -11,6 +11,7 @@ import {
   TestStatus,
   TestStatusMessage,
   log,
+  logger,
   s3,
   sqs,
   util
@@ -147,11 +148,21 @@ describe("PewPewTest Create Test", () => {
             // Validate S3
           const filename: string = basename(test!.getResultsFile()!);
           const result = await s3.getObject(`${ppaasTestId!.s3Folder}/${filename}`);
-            expect(result).to.not.equal(undefined);
-            expect(result.ContentType).to.equal("application/json");
-            done();
-      })
-      .catch((error) => {
+          expect(result, "result").to.not.equal(undefined);
+          expect(result.ContentType, "result.ContentType").to.equal("application/json");
+          const [tagKeyExtra, tagValueExtra]: [string, string] = s3.defaultTestExtraFileTags().entries().next().value;
+          const stdoutFilename = logger.pewpewStdOutFilename(ppaasTestId!.testId);
+          const stderrFilename = logger.pewpewStdErrFilename(ppaasTestId!.testId);
+          const stdouttags = await s3.getTags({ s3Folder: ppaasTestId!.s3Folder, filename: stdoutFilename });
+          expect(stdouttags, "stdouttags").to.not.equal(undefined);
+          expect(stdouttags!.size, "stdouttags.size").to.be.greaterThan(0);
+          expect(stdouttags!.get(tagKeyExtra), `stdouttags!.get("${tagKeyExtra}")`).to.equal(tagValueExtra);
+          const stderrtags = await s3.getTags({ s3Folder: ppaasTestId!.s3Folder, filename: stderrFilename });
+          expect(stderrtags, "stderrtags").to.not.equal(undefined);
+          expect(stderrtags!.size, "stderrtags.size").to.be.greaterThan(0);
+          expect(stderrtags!.get(tagKeyExtra), `stderrtags!.get("${tagKeyExtra}")`).to.equal(tagValueExtra);
+          done();
+      }).catch((error) => {
         done(error);
       });
     });

diff --git a/agent/src/pewpewtest.ts b/agent/src/pewpewtest.ts
@@ -15,6 +15,7 @@ import {
   ec2,
   log,
   logger,
+  s3,
   sqs,
   util
 } from "@fs/ppaas-common";
@@ -418,13 +419,15 @@ export class PewPewTest {
       this.pewpewStdOutS3File = new PpaasS3File({
         filename: logger.pewpewStdOutFilename(this.testMessage.testId),
         s3Folder,
-        localDirectory: logConfig.LogFileLocation
+        localDirectory: logConfig.LogFileLocation,
+        tags: s3.defaultTestExtraFileTags()
       });
       this.log(`pewpewStdOutFilename = ${this.pewpewStdOutS3File.localFilePath}`, LogLevel.DEBUG);
       this.pewpewStdErrS3File =  new PpaasS3File({
         filename: logger.pewpewStdErrFilename(this.testMessage.testId),
         s3Folder,
-        localDirectory: logConfig.LogFileLocation
+        localDirectory: logConfig.LogFileLocation,
+        tags: s3.defaultTestExtraFileTags()
       });
       this.log(`pewpewStdErrS3File = ${this.pewpewStdErrS3File.localFilePath}`, LogLevel.DEBUG);
 
@@ -734,7 +737,8 @@ export class PewPewTest {
                 const foundS3File: PpaasS3File = new PpaasS3File({
                   filename: foundFile,
                   s3Folder: this.testMessage.s3Folder,
-                  localDirectory: this.localPath
+                  localDirectory: this.localPath,
+                  tags: s3.defaultTestExtraFileTags()
                 });
                 yamlCreatedFiles.set(foundFile, foundS3File);
               }

diff --git a/common/src/util/s3.ts b/common/src/util/s3.ts
@@ -56,10 +56,14 @@ export const config: { s3Client: S3Client } = {
  * has tags passed in of "key2=value3" then only key1=value1 would be added. WARNING: Is only initialized after s3.init() called.
  */
 export const ADDITIONAL_TAGS_ON_ALL = new Map<string, string>();
-// Don't export so that the original can't be modified
+// Don't export so that the original can't be modified,
+// we'll return a new copy of the Map each time so the original can't be modified
 const TEST_FILE_TAGS_INTERNAL = new Map<string, string>([["test", "true"]]);
-/** Returns a new copy of the Map each time so the original can't be modified */
+const TEST_EXTRA_FILE_TAGS_INTERNAL = new Map<string, string>([["test", "false"]]);
+/** Default tags on all test files (yaml, results, status) from the tests */
 export const defaultTestFileTags = (): Map<string, string> => new Map(TEST_FILE_TAGS_INTERNAL);
+/** Default tags on all extra files from the tests */
+export const defaultTestExtraFileTags = (): Map<string, string> => new Map(TEST_EXTRA_FILE_TAGS_INTERNAL);
 
 /**
  * Initializes the S3 object using environment variables. Runs later so it doesn't throw on start-up

diff --git a/controller/integration/testmanager.spec.ts b/controller/integration/testmanager.spec.ts
@@ -25,12 +25,12 @@ import {
   s3
 } from "@fs/ppaas-common";
 import { TestManager, defaultRecurringFileTags} from "../pages/api/util/testmanager";
+import { isYamlFile, latestPewPewVersion } from "../pages/api/util/clientutil";
 import { EventInput } from "@fullcalendar/core";
 import { PpaasEncryptEnvironmentFile } from "../pages/api/util/ppaasencryptenvfile";
 import { TestScheduler } from "../pages/api/util/testscheduler";
 import { expect } from "chai";
 import { getPewPewVersionsInS3 } from "../pages/api/util/pewpew";
-import { latestPewPewVersion } from "../pages/api/util/clientutil";
 import path from "path";
 
 logger.config.LogFileName = "ppaas-controller";
@@ -183,10 +183,15 @@ describe("TestManager Integration", () => {
           expect(s3Files.length, "s3Files.length").to.equal(3);
           // Check that the test=true tag is added
           const [tagKey, tagValue]: [string, string] = s3.defaultTestFileTags().entries().next().value;
+          const [tagKeyExtra, tagValueExtra]: [string, string] = s3.defaultTestExtraFileTags().entries().next().value;
           expect(typeof tagKey, "typeof tagKey").to.equal("string");
           for (const s3File of s3Files) {
             expect(s3File.tags, "s3File.tags").to.not.equal(undefined);
-            expect(s3File.tags?.get(tagKey), `${s3File.filename}.tags?.get("${tagKey}")`).to.equal(tagValue);
+            if (isYamlFile(s3File.filename) || s3File.filename.endsWith(".info")) {
+              expect(s3File.tags?.get(tagKey), `${s3File.filename}.tags?.get("${tagKey}")`).to.equal(tagValue);
+            } else {
+              expect(s3File.tags?.get(tagKeyExtra), `${s3File.filename}.tags?.get("${tagKeyExtra}")`).to.equal(tagValueExtra);
+            }
           }
           done();
         }).catch((error) => {
@@ -510,10 +515,15 @@ describe("TestManager Integration", () => {
           expect(s3Files.length, "s3Files.length").to.equal(5);
           // Check that the test=true tag is added
           const [tagKey, tagValue]: [string, string] = s3.defaultTestFileTags().entries().next().value;
+          const [tagKeyExtra, tagValueExtra]: [string, string] = s3.defaultTestExtraFileTags().entries().next().value;
           expect(typeof tagKey, "typeof tagKey").to.equal("string");
           for (const s3File of s3Files) {
             expect(s3File.tags, "s3File.tags").to.not.equal(undefined);
-            expect(s3File.tags?.get(tagKey), `${s3File.filename}.tags?.get("${tagKey}")`).to.equal(tagValue);
+            if (isYamlFile(s3File.filename) || s3File.filename.endsWith(".info")) {
+              expect(s3File.tags?.get(tagKey), `${s3File.filename}.tags?.get("${tagKey}")`).to.equal(tagValue);
+            } else {
+              expect(s3File.tags?.get(tagKeyExtra), `${s3File.filename}.tags?.get("${tagKeyExtra}")`).to.equal(tagValueExtra);
+            }
           }
           done();
         }).catch((error) => {
@@ -609,10 +619,15 @@ describe("TestManager Integration", () => {
           expect(s3Files.length, "s3Files.length").to.equal(3);
           // Check that the recurring=true tag is added
           const [tagKey, tagValue]: [string, string] = s3.defaultTestFileTags().entries().next().value;
+          const [tagKeyExtra, tagValueExtra]: [string, string] = s3.defaultTestExtraFileTags().entries().next().value;
           expect(typeof tagKey, "typeof tagKey").to.equal("string");
           for (const s3File of s3Files) {
             expect(s3File.tags, "s3File.tags").to.not.equal(undefined);
-            expect(s3File.tags?.get(tagKey), `${s3File.filename}.tags?.get("${tagKey}")`).to.equal(tagValue);
+            if (isYamlFile(s3File.filename) || s3File.filename.endsWith(".info")) {
+              expect(s3File.tags?.get(tagKey), `${s3File.filename}.tags?.get("${tagKey}")`).to.equal(tagValue);
+            } else {
+              expect(s3File.tags?.get(tagKeyExtra), `${s3File.filename}.tags?.get("${tagKeyExtra}")`).to.equal(tagValueExtra);
+            }
           }
           done();
         }).catch((error) => {
@@ -707,10 +722,15 @@ describe("TestManager Integration", () => {
           expect(s3Files.length, "s3Files.length").to.equal(5);
           // Check that the recurring=true tag is added
           const [tagKey, tagValue]: [string, string] = s3.defaultTestFileTags().entries().next().value;
+          const [tagKeyExtra, tagValueExtra]: [string, string] = s3.defaultTestExtraFileTags().entries().next().value;
           expect(typeof tagKey, "typeof tagKey").to.equal("string");
           for (const s3File of s3Files) {
             expect(s3File.tags, "s3File.tags").to.not.equal(undefined);
-            expect(s3File.tags?.get(tagKey), `${s3File.filename}.tags?.get("${tagKey}")`).to.equal(tagValue);
+            if (isYamlFile(s3File.filename) || s3File.filename.endsWith(".info")) {
+              expect(s3File.tags?.get(tagKey), `${s3File.filename}.tags?.get("${tagKey}")`).to.equal(tagValue);
+            } else {
+              expect(s3File.tags?.get(tagKeyExtra), `${s3File.filename}.tags?.get("${tagKeyExtra}")`).to.equal(tagValueExtra);
+            }
           }
           done();
         }).catch((error) => {
@@ -1713,12 +1733,17 @@ describe("TestManager Integration", () => {
             expect(s3Files.length, "s3Files.length").to.equal(3);
             // Check that the test=true tag is added and recurring=true is removed
             const [testTagKey, testTagValue]: [string, string] = s3.defaultTestFileTags().entries().next().value;
-            // eslint-disable-next-line @typescript-eslint/no-unused-vars
+            const [tagKeyExtra, tagValueExtra]: [string, string] = s3.defaultTestExtraFileTags().entries().next().value;
+              // eslint-disable-next-line @typescript-eslint/no-unused-vars
             const [recurringTagKey, recurringTagValue]: [string, string] = defaultRecurringFileTags().entries().next().value;
             expect(typeof testTagKey, "typeof tagKey").to.equal("string");
             for (const s3File of s3Files) {
               expect(s3File.tags, "s3File.tags").to.not.equal(undefined);
-              expect(s3File.tags?.get(testTagKey), `${s3File.filename}.tags?.get("${testTagKey}")`).to.equal(testTagValue);
+              if (isYamlFile(s3File.filename) || s3File.filename.endsWith(".info")) {
+                expect(s3File.tags?.get(testTagKey), `${s3File.filename}.tags?.get("${testTagKey}")`).to.equal(testTagValue);
+              } else {
+                expect(s3File.tags?.get(tagKeyExtra), `${s3File.filename}.tags?.get("${tagKeyExtra}")`).to.equal(tagValueExtra);
+              }
               expect(s3File.tags?.get(recurringTagKey), `${s3File.filename}.tags?.get("${recurringTagKey}")`).to.equal(undefined);
             }
             done();

diff --git a/controller/pages/api/util/testmanager.ts b/controller/pages/api/util/testmanager.ts
@@ -1333,23 +1333,28 @@ export abstract class TestManager {
         // Upload files
         const uploadPromises: Promise<PpaasS3File | void>[] = [uploadFile(yamlFile, s3Folder, fileTags)];
         // additionalFiles - Do this last so we can upload them at the same time
-        uploadPromises.push(...(additionalFiles.map((file: File) => uploadFile(file, s3Folder, fileTags))));
+        uploadPromises.push(...(additionalFiles.map((file: File) => uploadFile(file, s3Folder, fileTags || s3.defaultTestExtraFileTags()))));
         if (!editSchedule) {
           // copyFiles, just copy them from the old s3 location to the new one
           uploadPromises.push(...(copyFiles.map((file: PpaasS3File) => {
-            file.tags = fileTags;
+            file.tags = fileTags || s3.defaultTestExtraFileTags();
             return file.copy({ destinationS3Folder: s3Folder });
           })));
         } else {
-          // If we're changing from non-recurring to recurring or vice-versa we need to edit the existing file tags.
-          const updateTags: Map<string, string> = fileTags || s3.defaultTestFileTags(); // If fileTags is truthy it's recurring
+          const s3StatusFilename: string = createS3StatusFilename(ppaasTestId);
           uploadPromises.push(...(copyFiles.map((file: PpaasS3File) => {
-            file.tags = updateTags;
+            // If we're changing from non-recurring to recurring or vice-versa we need to edit the existing file tags.
+            // yaml and status files need defaultTestFileTags, all others should be defaultTestExtraFileTags
+            file.tags = fileTags
+              ? fileTags
+              : isYamlFile(file.filename) || file.filename === s3StatusFilename // yaml and status files are test files
+                ? s3.defaultTestFileTags()
+                : s3.defaultTestExtraFileTags();
             return file.updateTags();
           })));
         }
         // Store encrypted environment variables in s3
-        uploadPromises.push(new PpaasEncryptEnvironmentFile({ s3Folder, environmentVariablesFile, tags: fileTags }).upload());
+        uploadPromises.push(new PpaasEncryptEnvironmentFile({ s3Folder, environmentVariablesFile, tags: fileTags || s3.defaultTestExtraFileTags() }).upload());
         // Wait for all uploads to complete
         await Promise.all(uploadPromises);
 

diff --git a/controller/pages/api/util/testscheduler.ts b/controller/pages/api/util/testscheduler.ts
@@ -36,14 +36,17 @@ import {
 import { TestManager, defaultRecurringFileTags } from "./testmanager";
 import { formatError, getHourMinuteFromTimestamp } from "./clientutil";
 import type { EventInput } from "@fullcalendar/core";
+import { IS_RUNNING_IN_AWS } from "./authclient";
 import { PpaasEncryptS3File } from "./ppaasencrypts3file";
 
 const { sleep } = util;
 logger.config.LogFileName = "ppaas-controller";
 
 const TEST_SCHEDULER_POLL_INTERVAL_MS: number = parseInt(process.env.TEST_SCHEDULER_POLL_INTERVAL_MS || "0", 10) || 60000;
-const RUN_HISTORICAL_SEARCH: boolean = process.env.RUN_HISTORICAL_SEARCH === "true";
+const RUN_HISTORICAL_SEARCH: boolean = process.env.RUN_HISTORICAL_SEARCH?.toLowerCase() === "true";
 const HISTORICAL_SEARCH_MAX_FILES: number = parseInt(process.env.HISTORICAL_SEARCH_MAX_FILES || "0", 10) || 100000;
+const RUN_HISTORICAL_DELETE: boolean = process.env.RUN_HISTORICAL_DELETE?.toLowerCase() === "true";
+const DELETE_OLD_FILES_DAYS: number = parseInt(process.env.DELETE_OLD_FILES_DAYS || "0") || 365;
 const ONE_DAY: number = 24 * 60 * 60000;
 export const AUTH_PERMISSIONS_SCHEDULER: AuthPermissions = { authPermission: AuthPermission.Admin, token: "startTestSchedulerLoop", userId: "controller" };
 export const TEST_HISTORY_FILENAME = "testhistory.json";
@@ -391,6 +394,37 @@ export class TestScheduler implements TestSchedulerItem {
     if (RUN_HISTORICAL_SEARCH) {
       TestScheduler.runHistoricalSearch().catch(() => { /* already logs error, swallow */ });
     }
+    if (RUN_HISTORICAL_DELETE) {
+      // Start background task to delete old files
+      (async () => {
+        // Don't start right away, delay to sometime within today
+        let nextLoop: number = Date.now() + (IS_RUNNING_IN_AWS ? Math.floor(Math.random() * ONE_DAY) : 0);
+        if (nextLoop > Date.now()) {
+          const delay = nextLoop - Date.now();
+          log("Delete Historical Loop: nextLoop: " + new Date(nextLoop), LogLevel.DEBUG, { delay, nextLoop });
+          await sleep(delay);
+        }
+        while (global.testSchedulerLoopRunning) {
+          const loopStart = Date.now();
+          try {
+            await TestScheduler.runHistoricalDelete();
+          } catch (error) {
+            log("Delete Historical Loop: Error running runHistoricalDelete", LogLevel.ERROR, error);
+          }
+          // If Date.now() is exactly the same time we need to check the next one
+          nextLoop += ONE_DAY;
+          const delay = nextLoop - Date.now();
+          log("Delete Historical Loop: nextLoop: " + new Date(nextLoop), LogLevel.DEBUG, { loopStart, delay, nextLoop });
+          if (delay > 0) {
+            await sleep(delay);
+          }
+        }
+        // We'll only reach here if we got some kind of sigterm message or an unhandled exception. Shut down this loop so we can be restarted or replaced
+        log("Shutting Down Delete Historical Loop.", LogLevel.INFO);
+      })().catch((err) => {
+        log("Error during Delete Historical Loop", LogLevel.ERROR, err);
+      });
+    }
     (async () => {
       // We'll never set this to false unless something really bad happens
       while (global.testSchedulerLoopRunning) {
@@ -424,9 +458,9 @@ export class TestScheduler implements TestSchedulerItem {
         const nextStartTime = TestScheduler.nextStart || Number.MAX_VALUE;
         const delay = Math.min(nextPollTime - Date.now(), nextStartTime - Date.now(), TEST_SCHEDULER_POLL_INTERVAL_MS);
         log(
-          "Test Scheduler Loop: nextPollTime: " + nextPollTime,
+          "Test Scheduler Loop: nextPollTime: " + new Date(nextPollTime),
           LogLevel.DEBUG,
-          { loopStart, nextPollTime, nextStartTime, now: Date.now(), delay, TEST_SCHEDULER_POLL_INTERVAL_MS }
+          { loopStart, nextPollTime, nextStartTime, delay, TEST_SCHEDULER_POLL_INTERVAL_MS }
         );
         if (delay > 0) {
           await sleep(delay);
@@ -828,6 +862,34 @@ export class TestScheduler implements TestSchedulerItem {
     }
   }
 
+  protected static async runHistoricalDelete (deleteOldFilesDays: number = DELETE_OLD_FILES_DAYS): Promise<number> {
+    let deletedCount: number = 0;
+    try {
+      // Load existing ones
+      await TestScheduler.loadHistoricalFromS3();
+      const oldDatetime: number = Date.now() - (deleteOldFilesDays * ONE_DAY);
+      const sizeBefore = TestScheduler.historicalTests!.size;
+      log("Starting Test Historical Delete", LogLevel.INFO, { sizeBefore, oldDatetime: new Date(oldDatetime), deleteOldFilesDays });
+
+      // Delete old ones off the historical Calendar. These will be cleaned up in S3 by Bucket Expiration Policy
+      for (const [testId, eventInput] of TestScheduler.historicalTests!) {
+        if ((typeof eventInput.end === "number" && eventInput.end < oldDatetime)
+          || (eventInput.end instanceof Date && (eventInput as Date).getTime() < oldDatetime)) {
+          log("Deleting Historical Test " + testId, LogLevel.INFO, eventInput);
+          // Delete
+          TestScheduler.historicalTests!.delete(testId);
+          deletedCount++;
+        }
+      }
+      await TestScheduler.saveHistoricalToS3();
+      log("Finished Test Historical Delete", LogLevel.INFO, { deletedCount, sizeBefore, sizeAfter: TestScheduler.historicalTests!.size });
+      return deletedCount;
+    } catch (error) {
+      log("Error running Historical Delete", LogLevel.ERROR, error, { deletedCount });
+      throw error; // Throw for testing, but the loop will catch and noop
+    }
+  }
+
   protected static async loadHistoricalFromS3 (force?: boolean): Promise<void> {
     log("TestScheduler: loadHistoricalFromS3", LogLevel.DEBUG, {
       thisScheduledTests: (TestScheduler.historicalTests !== undefined)

diff --git a/controller/policy/bucket-expiration-policy.xml b/controller/policy/bucket-expiration-policy.xml
@@ -18,6 +18,19 @@
       </Tag>
     </Filter>
     <Status>Enabled</Status>
+    <Expiration>
+      <Days>730</Days>
+    </Expiration>
+  </Rule>
+  <Rule>
+    <ID>Expire Extra File Uploads</ID>
+    <Filter>
+      <Tag>
+        <Key>test</Key>
+        <Value>false</Value>
+      </Tag>
+    </Filter>
+    <Status>Enabled</Status>
     <Expiration>
       <Days>180</Days>
     </Expiration>