fix(runJob/kubernetes): reliably display logs (#7060)

* fix(runJob/kubernetes): fix log display fix a case where the log link was being displayed before all the data needed to render it was available. this caused some confusing cases where the link would be rendered but users would be met with a Not Found error in the popup. ultimately this was due to a missing namespace. resolved by pulling the namespace off of the job status instead of depending on the manifest subscription. * fix(runJob/kubernetes): fetch manifest once jobs collect owned pods when collecting job status so we are no longer dependent on subscribing to manifest events to get the pod names created by the job. instead, simply fetch the manifest once. this is more reliable than subscribing to manifest updates.
spinnaker · May 24, 2019 · 89e4e78 · 89e4e78
1 parent b712673
commit 89e4e78
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 88 deletions.
diff --git a/app/scripts/modules/core/src/manifest/stage/JobStageExecutionLogs.tsx b/app/scripts/modules/core/src/manifest/stage/JobStageExecutionLogs.tsx
@@ -1,12 +1,13 @@
 import * as React from 'react';
-import { get, template, isEmpty, trim } from 'lodash';
+import { template, isEmpty } from 'lodash';
+import { Observable, Subject } from 'rxjs';
 
-import { IManifestSubscription } from '../IManifestSubscription';
-import { IStageManifest, ManifestService } from '../ManifestService';
+import { IStageManifest } from '../ManifestService';
 import { JobManifestPodLogs } from './JobManifestPodLogs';
 import { IManifest } from 'core/domain/IManifest';
 import { Application } from 'core/application';
 import { IPodNameProvider } from '../PodNameProvider';
+import { ManifestReader } from 'core/manifest';
 
 interface IJobStageExecutionLogsProps {
   manifest: IStageManifest;
@@ -15,75 +16,25 @@ interface IJobStageExecutionLogsProps {
   application: Application;
   externalLink: string;
   podNameProvider: IPodNameProvider;
+  location: string;
 }
 
 interface IJobStageExecutionLogsState {
-  subscription: IManifestSubscription;
-  manifestId: string;
+  manifest?: IManifest;
 }
 
 export class JobStageExecutionLogs extends React.Component<IJobStageExecutionLogsProps, IJobStageExecutionLogsState> {
   public state = {
-    subscription: { id: '', unsubscribe: () => {}, manifest: {} } as IManifestSubscription,
-    manifestId: '',
+    manifest: {} as IManifest,
   };
 
-  public componentDidMount() {
-    this.componentDidUpdate(this.props, this.state);
-  }
-
-  public componentWillUnmount() {
-    this.unsubscribe();
-  }
-
-  private unsubscribe() {
-    this.state.subscription && this.state.subscription.unsubscribe && this.state.subscription.unsubscribe();
-  }
-
-  public componentDidUpdate(_prevPropds: IJobStageExecutionLogsProps, prevState: IJobStageExecutionLogsState) {
-    const { manifest } = this.props;
-    const manifestId = ManifestService.manifestIdentifier(manifest);
-    if (prevState.manifestId === manifestId) {
-      return;
-    }
-    this.refreshSubscription(manifestId, manifest);
-  }
-
-  private refreshSubscription(manifestId: string, manifest: IStageManifest) {
-    const subscription = {
-      id: manifestId,
-      manifest: this.stageManifestToIManifest(manifest, this.props.deployedName, this.props.account),
-      unsubscribe: this.subscribeToManifestUpdates(manifest),
-    };
-    this.setState({ subscription, manifestId });
-  }
-
-  private subscribeToManifestUpdates(manifest: IStageManifest): () => void {
-    const params = {
-      account: this.props.account,
-      name: this.props.deployedName,
-      location: manifest.metadata.namespace == null ? '_' : manifest.metadata.namespace,
-    };
-    return ManifestService.subscribe(this.props.application, params, (updated: IManifest) => {
-      const subscription = { ...this.state.subscription, manifest: updated };
-      this.setState({ subscription });
-    });
-  }
-
-  private stageManifestToIManifest(manifest: IStageManifest, deployedName: string, account: string): IManifest {
-    const namespace = get(manifest, ['metadata', 'namespace'], '');
+  private destroy$ = new Subject();
 
-    return {
-      name: deployedName,
-      moniker: null,
-      account,
-      cloudProvider: 'kubernetes',
-      location: namespace,
-      manifest,
-      status: {},
-      artifacts: [],
-      events: [],
-    };
+  public componentDidMount() {
+    const { account, location, deployedName } = this.props;
+    Observable.from(ManifestReader.getManifest(account, location, deployedName))
+      .takeUntil(this.destroy$)
+      .subscribe(manifest => this.setState({ manifest }), () => {});
   }
 
   private renderExternalLink(link: string, manifest: IManifest): string {
@@ -96,11 +47,8 @@ export class JobStageExecutionLogs extends React.Component<IJobStageExecutionLog
   }
 
   public render() {
-    const { manifest } = this.state.subscription;
-    const { externalLink, podNameProvider } = this.props;
-    const namespace = trim(
-      get(manifest, ['manifest', 'metadata', 'annotations', 'artifact.spinnaker.io/location'], ''),
-    );
+    const { manifest } = this.state;
+    const { externalLink, podNameProvider, location, account } = this.props;
 
     // prefer links to external logging platforms
     if (!isEmpty(manifest) && externalLink) {
@@ -112,12 +60,16 @@ export class JobStageExecutionLogs extends React.Component<IJobStageExecutionLog
     }
 
     return (
-      <JobManifestPodLogs
-        account={manifest.account}
-        location={namespace}
-        podNameProvider={podNameProvider}
-        linkName="Console Output"
-      />
+      <>
+        {location && (
+          <JobManifestPodLogs
+            account={account}
+            location={location}
+            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
@@ -76,7 +76,7 @@ export class PreconfiguredJobExecutionDetails extends React.Component<IExecution
 
     if (cloudProvider === 'kubernetes') {
       const manifest = get(stage, ['context', 'manifest'], null);
-      const namespace = get(manifest, ['metadata', 'namespace']);
+      const namespace = get<string>(stage, ['context', 'jobStatus', 'location']);
       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']);
@@ -89,6 +89,7 @@ export class PreconfiguredJobExecutionDetails extends React.Component<IExecution
             deployedName={deployedName}
             account={this.props.stage.context.account}
             application={this.props.application}
+            location={namespace}
             externalLink={externalLink}
             podNameProvider={podNameProvider}
           />

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
@@ -39,6 +39,7 @@ export class RunJobExecutionDetails extends React.Component<IExecutionDetailsSec
     const externalLink = get<string>(stage, ['context', 'execution', 'logs']);
     const podName = this.mostRecentlyCreatedPodName(get(stage.context, ['jobStatus', 'pods'], []));
     const podNameProvider = new DefaultPodNameProvider(podName);
+    const namespace = get(stage, ['context', 'jobStatus', 'location'], '');
 
     return (
       <ExecutionDetailsSection name={name} current={current}>
@@ -49,23 +50,25 @@ export class RunJobExecutionDetails extends React.Component<IExecutionDetailsSec
               <dd>
                 <AccountTag account={context.account} />
               </dd>
-              {stage.context.jobStatus && stage.context.jobStatus.location && (
-                <span>
+              {namespace && (
+                <>
                   <dt>Namespace</dt>
                   <dd>{stage.context.jobStatus.location}</dd>
-                </span>
+                  <dt>Logs</dt>
+                  <dd>
+                    <JobStageExecutionLogs
+                      manifest={manifest}
+                      deployedName={deployedName}
+                      account={this.props.stage.context.account}
+                      location={namespace}
+                      application={this.props.application}
+                      externalLink={externalLink}
+                      podNameProvider={podNameProvider}
+                    />
+                  </dd>
+                </>
               )}
-              <dt>Logs</dt>
-              <dd>
-                <JobStageExecutionLogs
-                  manifest={manifest}
-                  deployedName={deployedName}
-                  account={this.props.stage.context.account}
-                  application={this.props.application}
-                  externalLink={externalLink}
-                  podNameProvider={podNameProvider}
-                />
-              </dd>
+              {!namespace && <div className="well">Collecting additional details...</div>}
             </dl>
           </div>
         </div>