DOP-3591 (#848)

* move search deploy commands into production job handler

* test flag

* fix manifest handler

* save logs to queue message

* construct manifest prefix

* fix broken tests

* add timer for search job

* address comments

Author: seungpark

cdk-infra/utils/ssm.ts

@@ -74,7 +74,7 @@ const workerSecureStrings = [
   '/cdn/client/secret',
 ] as const;
 
-type WorkerSecureString = typeof workerSecureStrings[number];
+type WorkerSecureString = (typeof workerSecureStrings)[number];
 
 const workerParamPathToEnvName = new Map<WorkerSecureString, string>();
 
@@ -113,7 +113,7 @@ const webhookSecureStrings = [
   '/snooty/webhook/secret',
 ] as const;
 
-type WebhookSecureString = typeof webhookSecureStrings[number];
+type WebhookSecureString = (typeof webhookSecureStrings)[number];
 
 const webhookParamPathToEnvName = new Map<WebhookSecureString, string>();
 

src/job/jobHandler.ts

@@ -80,6 +80,7 @@ export abstract class JobHandler {
     this._repoBranchesRepo = repoBranchesRepo;
   }
 
+  // called during build stage of
   abstract prepStageSpecificNextGenCommands(): void;
 
   private logErrorMessage(message: string): void {
@@ -391,6 +392,10 @@ export abstract class JobHandler {
 
   protected abstract prepDeployCommands(): void;
 
+  protected prepSearchDeploy(): void {
+    this._logger.info(this._currJob._id, 'Preparing search deploy');
+  }
+
   // TODO: Reduce state changes
   protected prepBuildCommands(): void {
     this.currJob.buildCommands = [
@@ -459,7 +464,17 @@ export abstract class JobHandler {
     await this._logger.save(this.currJob._id, `${this._config.get<string>('stage').padEnd(15)}Pushing to ${this.name}`);
 
     if ((this.currJob?.deployCommands?.length ?? 0) > 0) {
-      const resp = await this._commandExecutor.execute(this.currJob.deployCommands);
+      // extract search deploy job to time and test
+      const searchCommandIdx = this.currJob.deployCommands.findIndex((c) => c.match(/^mut\-index/));
+      let deployCmdsNoSearch = this.currJob.deployCommands;
+      if (searchCommandIdx > -1) {
+        await this.callWithBenchmark(this.currJob.deployCommands[searchCommandIdx], 'search');
+        deployCmdsNoSearch = this.currJob.deployCommands
+          .slice(0, searchCommandIdx)
+          .concat(this.currJob.deployCommands.slice(searchCommandIdx + 1, this.currJob.deployCommands.length));
+      }
+
+      const resp = await this._commandExecutor.execute(deployCmdsNoSearch);
       if (resp?.error?.includes?.('ERROR')) {
         await this._logger.save(
           this.currJob._id,

src/job/manifestJobHandler.ts

@@ -1,3 +1,6 @@
+// TODO: remove manifest job handler
+// not run as a separate job, handled in productionJobHandler prepSearchDeploy
+
 import { JobHandler } from './jobHandler';
 import { IConfig } from 'config';
 import type { Job } from '../entities/job';
@@ -12,7 +15,7 @@ import { RepoBranchesRepository } from '../repositories/repoBranchesRepository';
 import { InvalidJobError } from '../errors/errors';
 
 // TODO: Move this to a generic util and out of this job file
-const joinUrlAndPrefix = (url, prefix) => {
+export const joinUrlAndPrefix = (url: string, prefix: string) => {
   const needsTrim = url.endsWith('/') && prefix.startsWith('/');
   const needsSlash = !url.endsWith('/') && !prefix.startsWith('/');
 
@@ -94,7 +97,7 @@ export class ManifestJobHandler extends JobHandler {
       `cd repos/${this.currJob.payload.repoName}`,
       'echo IGNORE: testing manifest generation deploy commands',
       'ls -al',
-      `mut-index upload bundle.zip -b ${b} -o ${f}/${maP}.json -u ${jUaP(url, muP)} ${globalSearch}`,
+      `mut-index upload bundle.zip -b ${b} -o ${f}/${maP}.json -u ${jUaP(url, muP || '')} ${globalSearch}`,
     ];
   }
 

src/job/productionJobHandler.ts

@@ -10,6 +10,7 @@ import { IJobRepoLogger } from '../services/logger';
 import { IRepoConnector } from '../services/repo';
 import { JobHandler } from './jobHandler';
 import { IJobValidator } from './jobValidator';
+import { joinUrlAndPrefix } from './manifestJobHandler';
 
 export class ProductionJobHandler extends JobHandler {
   constructor(
@@ -48,18 +49,82 @@ export class ProductionJobHandler extends JobHandler {
 
     // TODO: Reduce confusion between job.manifestPrefix and job.payload.manifestPrefix
     if (this.currJob.payload.isNextGen) {
-      const manifestPrefix = this.currJob.payload.manifestPrefix;
+      this.currJob.manifestPrefix = this.currJob.manifestPrefix ?? this.constructManifestPrefix();
       this.currJob.deployCommands[
         this.currJob.deployCommands.length - 1
       ] = `make next-gen-deploy MUT_PREFIX=${this.currJob.payload.mutPrefix}`;
       // TODO: Remove when satisfied with new manifestJobHandler infrastructure
-      if (manifestPrefix) {
+      if (this.currJob.manifestPrefix) {
         const searchFlag = this.currJob.payload.stable ? '-g' : '';
         this.currJob.deployCommands[
           this.currJob.deployCommands.length - 1
-        ] += ` MANIFEST_PREFIX=${manifestPrefix} GLOBAL_SEARCH_FLAG=${searchFlag}`;
+        ] += ` MANIFEST_PREFIX=${this.currJob.manifestPrefix} GLOBAL_SEARCH_FLAG=${searchFlag}`;
       }
     }
+    // have to combine search deploy commands
+    // manifestJobHandler.prepDeployCommands
+
+    this.currJob.shouldGenerateSearchManifest = this.shouldGenerateSearchManifest();
+    this.logger.save(
+      this.currJob._id,
+      `this.currJob.shouldGenerateSearchManifest ${this.currJob.shouldGenerateSearchManifest}`
+    );
+    if (this.currJob.shouldGenerateSearchManifest) {
+      this.prepSearchDeploy();
+    }
+  }
+
+  prepSearchDeploy(): void {
+    const b = this._config.get<string>('searchIndexBucket') ?? 'docs-search-indexes-test';
+    // /deploy -> send to /prd folder. /test-deploy -> send to /preprd folder
+    const env = this._config.get<string>('env');
+    // Note: mut-index automatically prepends 'search-indexes/' to the folder.
+    const f = this._config.get<string>('searchIndexFolder')?.[env] ?? 'fallback-folder';
+    this.logger.save(this.currJob._id, `Manifest attempt to upload to bucket: ${b}, folder: ${f}`);
+    // Due to the dual existence of prefixes, check for both for redundancy
+    const maP = this.currJob.manifestPrefix ?? this.currJob.payload.manifestPrefix;
+    const muP = this.currJob.mutPrefix ?? this.currJob.payload.mutPrefix;
+    const url = this.currJob.payload?.repoBranches?.url?.[env];
+    const jUaP = joinUrlAndPrefix;
+    const globalSearch = this.currJob.payload.stable ? '-g' : '';
+
+    // Rudimentary error logging
+    if (!b) {
+      this.logger.save(this.currJob._id, `searchIndexBucket not found`);
+    }
+    if (!f) {
+      this.logger.save(this.currJob._id, `searchIndexFolder not found`);
+    }
+
+    if (!url) {
+      this.logger.error(
+        this.currJob._id,
+        `repoBranches.url entry for this environment (${env}) not found for ${this.currJob._id}`
+      );
+      return;
+    }
+
+    if (!this.currJob.manifestPrefix) {
+      this.logger.error(this.currJob._id, `Manifest prefix not found for ${this.currJob._id}`);
+      return;
+    }
+    const searchCommands = [
+      '. /venv/bin/activate',
+      `cd repos/${this.currJob.payload.repoName}`,
+      'echo IGNORE: testing manifest generation deploy commands',
+      'ls -al',
+      // For mut-index usage info, see: https://github.com/mongodb/mut/blob/master/mut/index/main.py#L2
+      `mut-index upload bundle.zip -b ${b} -o ${f}/${maP}.json -u ${jUaP(url, muP || '')} ${globalSearch}`,
+    ];
+    for (const command of searchCommands) {
+      if (this.currJob.deployCommands.indexOf(command) === -1) {
+        this.currJob.deployCommands.push(command);
+      }
+    }
+    this.logger.save(
+      this.currJob._id,
+      `deploy commands: ${this.currJob.deployCommands.map((command) => command + '\n')}`
+    );
   }
 
   prepStageSpecificNextGenCommands(): void {
@@ -126,10 +191,6 @@ export class ProductionJobHandler extends JobHandler {
         await this.logger.save(this.currJob._id, `${'(prod)'.padEnd(15)}Deploy details:\n\n${resp.output}`);
       }
 
-      this.currJob.shouldGenerateSearchManifest = this.shouldGenerateSearchManifest();
-      if (this.currJob.shouldGenerateSearchManifest) {
-        this.queueManifestJob();
-      }
       return resp;
     } catch (errResult) {
       await this.logger.save(this.currJob._id, `${'(prod)'.padEnd(15)}stdErr: ${errResult.stderr}`);

tests/data/data.ts

@@ -266,11 +266,6 @@ export class TestDataProvider {
     const ret = Array<string>().concat(genericCommands.slice(0, genericCommands.length - 1), [
       `make next-gen-deploy MUT_PREFIX=${job.payload.mutPrefix}`,
     ]);
-    if (job.payload.manifestPrefix) {
-      // TODO: is job.payload.stableBranch supposed to be includeInGlobalSearch?
-      const stable = job.payload.stable ? '-g' : '';
-      ret[ret.length - 1] += ` MANIFEST_PREFIX=${job.payload.manifestPrefix} GLOBAL_SEARCH_FLAG=${stable}`;
-    }
     return ret;
   }
 

tests/unit/job/productionJobHandler.test.ts

@@ -173,7 +173,7 @@ describe('ProductionJobHandler Tests', () => {
     );
   });
 
-  test('Default production deploy kicks off manifest generation', async () => {
+  test('Default production deploy does not kick off  manifest generation', async () => {
     jobHandlerTestHelper.jobRepo.insertJob = jest.fn();
     const queueManifestJobSpy = jest.spyOn(jobHandlerTestHelper.jobHandler, 'queueManifestJob');
 
@@ -183,10 +183,8 @@ describe('ProductionJobHandler Tests', () => {
     await jobHandlerTestHelper.jobHandler.execute();
     jobHandlerTestHelper.verifyNextGenSuccess();
 
-    expect(queueManifestJobSpy).toBeCalledTimes(1);
-    expect(jobHandlerTestHelper.jobRepo.insertJob).toBeCalledTimes(1);
-
-    expect(jobHandlerTestHelper.jobRepo.insertJob.mock.calls[0][0]).toEqual(getManifestJobDef());
+    expect(queueManifestJobSpy).toBeCalledTimes(0);
+    expect(jobHandlerTestHelper.jobRepo.insertJob).toBeCalledTimes(0);
   });
 
   test('Production deploy with false shouldGenerateManifest() result does not kick off manifest job', async () => {