fix(monitored deploy): properly initialize defaults (#7473)

* use `defaultsDeep` to correctly initlialize and not fail when `rollback` object is undefined * also, show better UX when a stage hasn't run yet
spinnaker · Oct 2, 2019 · 0d0caa9 · 0d0caa9
1 parent c6c69a3
commit 0d0caa9
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 16 deletions.
diff --git a/app/scripts/modules/core/src/deploymentStrategy/strategies/monitored/monitored.strategy.ts b/app/scripts/modules/core/src/deploymentStrategy/strategies/monitored/monitored.strategy.ts
@@ -1,3 +1,4 @@
+import { isEmpty, defaultsDeep } from 'lodash';
 import { DeploymentStrategyRegistry } from 'core/deploymentStrategy/deploymentStrategy.registry';
 
 import { AdditionalFields } from './AdditionalFields';
@@ -10,11 +11,15 @@ DeploymentStrategyRegistry.registerStrategy({
   additionalFields: ['deploySteps', 'scaleDown'],
   AdditionalFieldsComponent: AdditionalFields,
   initializationMethod: command => {
-    if (!command.deploySteps) {
+    defaultsDeep(command, {
+      rollback: { onFailure: true },
+      deploymentMonitor: { id: '' },
+      maxRemainingAsgs: 2,
+    });
+
+    // defaultsDeep will merge the arrays, which we don't want hence check deploySteps separately
+    if (isEmpty(command.deploySteps)) {
       command.deploySteps = [10, 40, 100];
-      command.rollback.onFailure = true;
-      command.deploymentMonitor = { id: '' };
-      command.maxRemainingAsgs = 2;
     }
   },
 });
diff --git a/...les/core/src/pipeline/config/stages/monitoreddeploy/DeploymentMonitorExecutionDetails.tsx b/...les/core/src/pipeline/config/stages/monitoreddeploy/DeploymentMonitorExecutionDetails.tsx
@@ -15,8 +15,11 @@ interface IAdditionalData {
 }
 
 function getDeploymentMonitorSummary(stage: IExecutionStage) {
-  const { context } = stage;
+  if (stage.hasNotStarted) {
+    return "Not available because the stage hasn't run yet";
+  }
 
+  const { context } = stage;
   return get(context, 'deploymentMonitorReasons.summary', '<NOT PROVIDED>');
 }
 
@@ -108,17 +111,21 @@ export class DeploymentMonitorExecutionDetails extends React.Component<
             <dl className="dl-narrow dl-horizontal">
               <dt>Summary</dt>
               <dd>{getDeploymentMonitorSummary(stage)}</dd>
-              <dt className="sp-margin-l-bottom">Deploy %</dt>
-              <dd className="sp-margin-l-bottom">
-                <span>{stage.context.currentProgress}%</span>
-              </dd>
-              <dt>Details</dt>
-              <dd>{details || notProvidedFragment}</dd>
-              <dt className="sp-margin-l-bottom">Log</dt>
-              <dd className="sp-margin-l-bottom">{log ? <pre>{log}</pre> : notProvidedFragment}</dd>
-              <dt>More info</dt>
-              <dd>{additionalDetails ? '' : notProvidedFragment}</dd>
-              {additionalDetails}
+              {stage.hasNotStarted ? null : (
+                <>
+                  <dt className="sp-margin-l-bottom">Deploy %</dt>
+                  <dd className="sp-margin-l-bottom">
+                    <span>{stage.context.currentProgress}%</span>
+                  </dd>
+                  <dt>Details</dt>
+                  <dd>{details || notProvidedFragment}</dd>
+                  <dt className="sp-margin-l-bottom">Log</dt>
+                  <dd className="sp-margin-l-bottom">{log ? <pre>{log}</pre> : notProvidedFragment}</dd>
+                  <dt>More info</dt>
+                  <dd>{additionalDetails ? null : notProvidedFragment}</dd>
+                  {additionalDetails}
+                </>
+              )}
             </dl>
             {deploymentMonitors && (
               <div className="alert alert-info">