fix(runJob/kubernetes): use explicit pod name (#7039)

the previous implementation of run job used manifest events to get the name of the pod created by the job. this proved unreliable as events roll off quickly in large environments. this commit implements a `PodNameProvider` interface which supplies `JobManifestPodLogs` with the name of the Pod to fetch logs from. Run Job supplies these Pod names by collecting all Pods owned by the Job. For components which are still event based, the `JobEventBasedPodNameProvider` implements the previously used logic for getting the Pod name from the event.
spinnaker · May 22, 2019 · f0287a1 · f0287a1
1 parent fc1cd1b
commit f0287a1
Show file tree

Hide file tree

Showing 9 changed files with 162 additions and 39 deletions.
diff --git a/app/scripts/modules/core/src/domain/IManifest.ts b/app/scripts/modules/core/src/domain/IManifest.ts
@@ -29,3 +29,8 @@ export interface IManifestEvent {
   reason: string;
   type: string;
 }
+
+export interface IJobOwnedPodStatus {
+  name: string;
+  status: any;
+}
diff --git a/app/scripts/modules/core/src/manifest/PodNameProvider.spec.ts b/app/scripts/modules/core/src/manifest/PodNameProvider.spec.ts
@@ -0,0 +1,38 @@
+import { DefaultPodNameProvider, JobEventBasedPodNameProvider } from './PodNameProvider';
+import { IManifest, IManifestEvent } from 'core/domain';
+
+describe('PodNameProvider', function() {
+  describe('DefaultPodNameProvider', function() {
+    it('returns the pod name supplied to it', function() {
+      const podName = 'test';
+      const provider = new DefaultPodNameProvider(podName);
+      expect(provider.getPodName()).toBe(podName);
+    });
+  });
+
+  describe('JobEventBasedPodNameProvider', function() {
+    it('returns a pod name parsed from a message', function() {
+      const podName = 'test';
+      const manifest = { manifest: { kind: 'Job', status: true } } as IManifest;
+      const manifestEvent = { message: `Created pod: ${podName}` } as IManifestEvent;
+      const provider = new JobEventBasedPodNameProvider(manifest, manifestEvent);
+      expect(provider.getPodName()).toBe(podName);
+    });
+
+    it('returns a empty string if manifest is not of type Job', function() {
+      const podName = 'test';
+      const manifest = { manifest: { kind: 'Deployment', status: true } } as IManifest;
+      const manifestEvent = { message: `Created pod: ${podName}` } as IManifestEvent;
+      const provider = new JobEventBasedPodNameProvider(manifest, manifestEvent);
+      expect(provider.getPodName()).toBe('');
+    });
+
+    it('returns empty string for messages that do not start with Created pod', function() {
+      const podName = 'test';
+      const manifest = { manifest: { kind: 'Job', status: true } } as IManifest;
+      const manifestEvent = { message: `Killed pod: ${podName}` } as IManifestEvent;
+      const provider = new JobEventBasedPodNameProvider(manifest, manifestEvent);
+      expect(provider.getPodName()).toBe('');
+    });
+  });
+});
diff --git a/app/scripts/modules/core/src/manifest/PodNameProvider.ts b/app/scripts/modules/core/src/manifest/PodNameProvider.ts
@@ -0,0 +1,50 @@
+import { IManifest, IManifestEvent } from 'core/domain';
+import { trim } from 'lodash';
+
+// when fetching logs from Pods there are some instances
+// where we know the name of the Pod ahead of time and some
+// where we must extract the name of the Pod from a Kubernetes
+// event. IPodNameProvider allows us to inject a Pod name into
+// components which need them based on a given situation where
+// the caller knows how to the name should be supplied.
+export interface IPodNameProvider {
+  getPodName(): string;
+}
+
+export class DefaultPodNameProvider implements IPodNameProvider {
+  private podName: string;
+
+  constructor(podName: string) {
+    this.podName = podName;
+  }
+
+  public getPodName(): string {
+    return this.podName;
+  }
+}
+
+export class JobEventBasedPodNameProvider implements IPodNameProvider {
+  private manifest: IManifest;
+  private manifestEvent: IManifestEvent;
+
+  constructor(manifest: IManifest, manifestEvent: IManifestEvent) {
+    this.manifest = manifest;
+    this.manifestEvent = manifestEvent;
+  }
+
+  public getPodName(): string {
+    const { manifestEvent } = this;
+    return this.canParsePodName() ? trim(manifestEvent.message.split(':')[1]) : '';
+  }
+
+  private canParsePodName(): boolean {
+    const { manifest, manifestEvent } = this;
+    return (
+      !!manifest.manifest &&
+      !!manifest.manifest.status &&
+      !!manifestEvent &&
+      !!manifestEvent.message.startsWith('Created pod') &&
+      manifest.manifest.kind.toLowerCase() === 'job'
+    );
+  }
+}
diff --git a/app/scripts/modules/core/src/manifest/index.ts b/app/scripts/modules/core/src/manifest/index.ts
@@ -5,3 +5,4 @@ export * from './IManifestSubscription';
 export * from './ManifestService';
 export * from './stage/JobManifestPodLogs';
 export * from './ManifestYaml';
+export * from './PodNameProvider';
diff --git a/app/scripts/modules/core/src/manifest/stage/JobManifestPodLogs.tsx b/app/scripts/modules/core/src/manifest/stage/JobManifestPodLogs.tsx
@@ -1,18 +1,17 @@
 import * as React from 'react';
 import { Modal, Button } from 'react-bootstrap';
 import * as classNames from 'classnames';
+import { bindAll } from 'lodash';
 
 import { InstanceReader, IInstanceConsoleOutput, IInstanceMultiOutputLog } from 'core/instance/InstanceReader';
-
-import { IManifestEvent, IManifest } from 'core/domain/IManifest';
-
-import { get, trim, bindAll } from 'lodash';
+import { IPodNameProvider } from '../PodNameProvider';
 
 // IJobManifestPodLogs is the data needed to get logs
 export interface IJobManifestPodLogsProps {
-  manifest: IManifest;
-  manifestEvent: IManifestEvent;
+  account: string;
+  location: string;
   linkName: string;
+  podNameProvider: IPodNameProvider;
 }
 
 export interface IJobManifestPodLogsState {
@@ -36,24 +35,17 @@ export class JobManifestPodLogs extends React.Component<IJobManifestPodLogsProps
   }
 
   private canShow(): boolean {
-    const { manifest, manifestEvent } = this.props;
-    return (
-      !!manifest.manifest &&
-      !!manifest.manifest.status &&
-      !!manifestEvent &&
-      !!manifestEvent.message.startsWith('Created pod') &&
-      manifest.manifest.kind.toLowerCase() === 'job'
-    );
+    const { podNameProvider } = this.props;
+    return podNameProvider.getPodName() !== '';
   }
 
   private resourceRegion(): string {
-    return trim(
-      get(this.props, ['manifest', 'manifest', 'metadata', 'annotations', 'artifact.spinnaker.io/location'], ''),
-    );
+    return this.props.location;
   }
 
   private podName(): string {
-    return `pod ${trim(this.props.manifestEvent.message.split(':')[1])}`;
+    const { podNameProvider } = this.props;
+    return `pod ${podNameProvider.getPodName()}`;
   }
 
   public close() {
@@ -65,9 +57,9 @@ export class JobManifestPodLogs extends React.Component<IJobManifestPodLogsProps
   }
 
   public onClick() {
-    const { manifest } = this.props;
+    const { account } = this.props;
     const region = this.resourceRegion();
-    InstanceReader.getConsoleOutput(manifest.account, region, this.podName(), 'kubernetes')
+    InstanceReader.getConsoleOutput(account, region, this.podName(), 'kubernetes')
       .then((response: IInstanceConsoleOutput) => {
         this.setState({
           containerLogs: response.output as IInstanceMultiOutputLog[],

diff --git a/app/scripts/modules/core/src/manifest/stage/JobStageExecutionLogs.tsx b/app/scripts/modules/core/src/manifest/stage/JobStageExecutionLogs.tsx
@@ -1,19 +1,20 @@
 import * as React from 'react';
+import { get, template, isEmpty, trim } from 'lodash';
 
 import { IManifestSubscription } from '../IManifestSubscription';
 import { IStageManifest, ManifestService } from '../ManifestService';
 import { JobManifestPodLogs } from './JobManifestPodLogs';
 import { IManifest } from 'core/domain/IManifest';
-
-import { get, template, isEmpty } from 'lodash';
 import { Application } from 'core/application';
+import { IPodNameProvider } from '../PodNameProvider';
 
 interface IJobStageExecutionLogsProps {
   manifest: IStageManifest;
   deployedName: string;
   account: string;
   application: Application;
   externalLink: string;
+  podNameProvider: IPodNameProvider;
 }
 
 interface IJobStageExecutionLogsState {
@@ -96,7 +97,10 @@ export class JobStageExecutionLogs extends React.Component<IJobStageExecutionLog
 
   public render() {
     const { manifest } = this.state.subscription;
-    const { externalLink } = this.props;
+    const { externalLink, podNameProvider } = this.props;
+    const namespace = trim(
+      get(manifest, ['manifest', 'metadata', 'annotations', 'artifact.spinnaker.io/location'], ''),
+    );
 
     // prefer links to external logging platforms
     if (!isEmpty(manifest) && externalLink) {
@@ -107,15 +111,13 @@ export class JobStageExecutionLogs extends React.Component<IJobStageExecutionLog
       );
     }
 
-    let event: any = null;
-    if (manifest && manifest.events) {
-      event = manifest.events.find((e: any) => e.message.startsWith('Created pod'));
-    }
-
-    if (isEmpty(manifest) && isEmpty(event)) {
-      return <div>No Console Output</div>;
-    }
-
-    return <JobManifestPodLogs manifest={manifest} manifestEvent={event} linkName="Console Output" />;
+    return (
+      <JobManifestPodLogs
+        account={manifest.account}
+        location={namespace}
+        podNameProvider={podNameProvider}
+        linkName="Console Output"
+      />
+    );
   }
 }
diff --git a/...les/core/src/pipeline/config/stages/preconfiguredJob/PreconfiguredJobExecutionDetails.tsx b/...les/core/src/pipeline/config/stages/preconfiguredJob/PreconfiguredJobExecutionDetails.tsx
@@ -1,4 +1,5 @@
 import * as React from 'react';
+import { get, sortBy, last } from 'lodash';
 
 import {
   IExecutionDetailsSectionProps,
@@ -8,9 +9,9 @@ import {
 } from 'core/pipeline';
 import { IPreconfiguredJobParameter } from './preconfiguredJobStage';
 import { JobStageExecutionLogs } from 'core/manifest/stage/JobStageExecutionLogs';
-import { get } from 'lodash';
-import { IStage } from 'core/domain';
+import { IStage, IJobOwnedPodStatus } from 'core/domain';
 import { AccountService } from 'core/account';
+import { DefaultPodNameProvider } from 'core/manifest';
 
 export interface ITitusExecutionLogsProps {
   stage: IStage;
@@ -64,6 +65,12 @@ export class TitusExecutionLogs extends React.Component<ITitusExecutionLogsProps
 export class PreconfiguredJobExecutionDetails extends React.Component<IExecutionDetailsSectionProps> {
   public static title = 'preconfiguredJobConfig';
 
+  private mostRecentlyCreatedPodName(podsStatuses: IJobOwnedPodStatus[]): string {
+    const sorted = sortBy(podsStatuses, (p: IJobOwnedPodStatus) => p.status.startTime);
+    const mostRecent = last(sorted);
+    return mostRecent ? mostRecent.name : '';
+  }
+
   private executionLogsComponent(cloudProvider: string) {
     const { stage } = this.props;
 
@@ -73,6 +80,8 @@ export class PreconfiguredJobExecutionDetails extends React.Component<IExecution
       const deployedJobs = get(this.props.stage, ['context', 'deploy.jobs']);
       const deployedName = get(deployedJobs, namespace, [])[0] || '';
       const externalLink = get<string>(this.props.stage, ['context', 'execution', 'logs']);
+      const podName = this.mostRecentlyCreatedPodName(get(stage.context, ['jobStatus', 'pods'], []));
+      const podNameProvider = new DefaultPodNameProvider(podName);
       return (
         <div className="well">
           <JobStageExecutionLogs
@@ -81,6 +90,7 @@ export class PreconfiguredJobExecutionDetails extends React.Component<IExecution
             account={this.props.stage.context.account}
             application={this.props.application}
             externalLink={externalLink}
+            podNameProvider={podNameProvider}
           />
         </div>
       );

diff --git a/...dules/kubernetes/src/v2/pipelines/stages/deployManifest/manifestStatus/ManifestEvents.tsx b/...dules/kubernetes/src/v2/pipelines/stages/deployManifest/manifestStatus/ManifestEvents.tsx
@@ -1,7 +1,13 @@
 import * as React from 'react';
-import { get } from 'lodash';
+import { get, trim } from 'lodash';
 import { DateTime } from 'luxon';
-import { IManifest, IManifestEvent, relativeTime, JobManifestPodLogs } from '@spinnaker/core';
+import {
+  IManifest,
+  IManifestEvent,
+  relativeTime,
+  JobManifestPodLogs,
+  JobEventBasedPodNameProvider,
+} from '@spinnaker/core';
 
 export interface IManifestEventsProps {
   manifest: IManifest;
@@ -26,6 +32,9 @@ export class ManifestEvents extends React.Component<IManifestEventsProps> {
       return <div>No recent events found - Kubernetes does not store events for long.</div>;
     }
     const { events } = this.props.manifest;
+    const namespace = trim(
+      get(this.props.manifest, ['manifest', 'metadata', 'annotations', 'artifact.spinnaker.io/location'], ''),
+    );
     return events.map((e, i) => {
       const firstTimestamp = get(e, 'firstTimestamp', '');
       const lastTimestamp = get(e, 'lastTimestamp', '');
@@ -67,7 +76,12 @@ export class ManifestEvents extends React.Component<IManifestEventsProps> {
           )}
           <div>{e.message}</div>
           <div>
-            <JobManifestPodLogs manifest={this.props.manifest} manifestEvent={e} linkName="Console Output (Raw)" />
+            <JobManifestPodLogs
+              account={this.props.manifest.account}
+              location={namespace}
+              podNameProvider={new JobEventBasedPodNameProvider(this.props.manifest, e)}
+              linkName="Console Output (Raw)"
+            />
           </div>
           {i !== events.length - 1 && <br />}
         </div>

diff --git a/app/scripts/modules/kubernetes/src/v2/pipelines/stages/runJob/RunJobExecutionDetails.tsx b/app/scripts/modules/kubernetes/src/v2/pipelines/stages/runJob/RunJobExecutionDetails.tsx
@@ -1,12 +1,14 @@
 import * as React from 'react';
-import { get } from 'lodash';
+import { get, sortBy, last } from 'lodash';
 
 import {
   IExecutionDetailsSectionProps,
   ExecutionDetailsSection,
   AccountTag,
   JobStageExecutionLogs,
   IStageManifest,
+  DefaultPodNameProvider,
+  IJobOwnedPodStatus,
 } from '@spinnaker/core';
 
 interface IStageDeployedJobs {
@@ -22,13 +24,21 @@ export class RunJobExecutionDetails extends React.Component<IExecutionDetailsSec
     return jobNames.length > 0 ? jobNames[0] : '';
   }
 
+  private mostRecentlyCreatedPodName(podsStatuses: IJobOwnedPodStatus[]): string {
+    const sorted = sortBy(podsStatuses, (p: IJobOwnedPodStatus) => p.status.startTime);
+    const mostRecent = last(sorted);
+    return mostRecent ? mostRecent.name : '';
+  }
+
   public render() {
     const { stage, name, current } = this.props;
     const { context } = stage;
 
     const { manifest } = context;
     const deployedName = this.extractDeployedJobName(manifest, get(context, ['deploy.jobs']));
     const externalLink = get<string>(stage, ['context', 'execution', 'logs']);
+    const podName = this.mostRecentlyCreatedPodName(get(stage.context, ['jobStatus', 'pods'], []));
+    const podNameProvider = new DefaultPodNameProvider(podName);
 
     return (
       <ExecutionDetailsSection name={name} current={current}>
@@ -53,6 +63,7 @@ export class RunJobExecutionDetails extends React.Component<IExecutionDetailsSec
                   account={this.props.stage.context.account}
                   application={this.props.application}
                   externalLink={externalLink}
+                  podNameProvider={podNameProvider}
                 />
               </dd>
             </dl>