Improve resilience of UI to namespace / resource deletion

Depending on timing / order of events, deleting a Pipeline(Run) or namespace can cause the client to render a blank page. This change improves the resilience of the client code to these issues ensuring that at the very least the UI shell remains, and an error notification informing the user that the resource cannot be loaded.
jessm12 · Sep 3, 2019 · df5b0b2 · df5b0b2
1 parent c82a077
commit df5b0b2
Show file tree

Hide file tree

Showing 6 changed files with 97 additions and 61 deletions.
diff --git a/packages/components/src/components/Log/Log.js b/packages/components/src/components/Log/Log.js
@@ -100,6 +100,7 @@ export class LogContainer extends Component {
         });
       } catch {
         this.setState({
+          loading: false,
           logs: [
             intl.formatMessage({
               id: 'dashboard.pipelineRun.logFailed',

diff --git a/src/containers/PipelineRun/PipelineRun.js b/src/containers/PipelineRun/PipelineRun.js
@@ -116,46 +116,59 @@ export /* istanbul ignore next */ class PipelineRunContainer extends Component {
   };
 
   loadTaskRuns = (pipelineRun, taskRunNames) => {
-    let runs = taskRunNames.map(taskRunName =>
-      getTaskRun(getStore().getState(), {
-        name: taskRunName,
-        namespace: pipelineRun.metadata.namespace
-      })
-    );
+    if (!pipelineRun || !taskRunNames) {
+      return [];
+    }
+
+    let runs = taskRunNames
+      .map(taskRunName =>
+        getTaskRun(getStore().getState(), {
+          name: taskRunName,
+          namespace: pipelineRun.metadata.namespace
+        })
+      )
+      .filter(Boolean);
 
     const {
       status: { taskRuns: taskRunDetails }
     } = pipelineRun;
 
-    runs = runs.map(taskRun => {
-      const taskName = taskRun.spec.taskRef.name;
-      const taskKind = taskRun.spec.taskRef.kind;
-      const task = selectedTask(
-        taskName,
-        taskKind === 'ClusterTask' ? this.props.clusterTasks : this.props.tasks
-      );
-      const taskRunName = taskRun.metadata.name;
-      const taskRunNamespace = taskRun.metadata.namespace;
-      const { reason, status: succeeded } = getStatus(taskRun);
-      const { pipelineTaskName } = taskRunDetails[taskRunName];
-      const { params, resources: inputResources } = taskRun.spec.inputs;
-      const { resources: outputResources } = taskRun.spec.outputs;
-      const steps = stepsStatus(task.spec.steps, taskRun.status.steps);
-      return {
-        id: taskRun.metadata.uid,
-        pipelineTaskName,
-        pod: taskRun.status.podName,
-        reason,
-        steps,
-        succeeded,
-        taskName,
-        taskRunName,
-        namespace: taskRunNamespace,
-        inputResources,
-        outputResources,
-        params
-      };
-    });
+    runs = runs
+      .map(taskRun => {
+        const taskName = taskRun.spec.taskRef.name;
+        const taskKind = taskRun.spec.taskRef.kind;
+        const task = selectedTask(
+          taskName,
+          taskKind === 'ClusterTask'
+            ? this.props.clusterTasks
+            : this.props.tasks
+        );
+        if (!task) {
+          return null;
+        }
+        const taskRunName = taskRun.metadata.name;
+        const taskRunNamespace = taskRun.metadata.namespace;
+        const { reason, status: succeeded } = getStatus(taskRun);
+        const { pipelineTaskName } = taskRunDetails[taskRunName];
+        const { params, resources: inputResources } = taskRun.spec.inputs;
+        const { resources: outputResources } = taskRun.spec.outputs;
+        const steps = stepsStatus(task.spec.steps, taskRun.status.steps);
+        return {
+          id: taskRun.metadata.uid,
+          pipelineTaskName,
+          pod: taskRun.status.podName,
+          reason,
+          steps,
+          succeeded,
+          taskName,
+          taskRunName,
+          namespace: taskRunNamespace,
+          inputResources,
+          outputResources,
+          params
+        };
+      })
+      .filter(Boolean);
 
     return runs;
   };
@@ -261,6 +274,7 @@ export /* istanbul ignore next */ class PipelineRunContainer extends Component {
 
     const logContainer = (
       <Log
+        key={stepName}
         fetchLogs={() => fetchLogs(stepName, stepStatus, taskRun)}
         stepStatus={stepStatus}
       />

diff --git a/src/containers/TaskRun/TaskRun.js b/src/containers/TaskRun/TaskRun.js
@@ -89,6 +89,9 @@ export /* istanbul ignore next */ class TaskRunContainer extends Component {
   loadTaskRun = () => {
     const { task } = this.props;
     let { taskRun } = this.props;
+    if (!taskRun) {
+      return null;
+    }
     let { steps } = taskRun.status;
     if (task) {
       steps = task.spec.steps; // eslint-disable-line

diff --git a/src/containers/TaskRuns/TaskRuns.js b/src/containers/TaskRuns/TaskRuns.js
@@ -96,32 +96,37 @@ export /* istanbul ignore next */ class TaskRunsContainer extends Component {
   loadTaskRuns = () => {
     const { task } = this.props;
     let { taskRuns } = this.props;
-    taskRuns = taskRuns.map(taskRun => {
-      const taskName = taskRun.spec.taskRef.name;
-      const taskRunName = taskRun.metadata.name;
-      const taskRunNamespace = taskRun.metadata.namespace;
-      const { reason, status: succeeded } = getStatus(taskRun);
-      const pipelineTaskName = taskRunName;
-      const runSteps = stepsStatus(task.spec.steps, taskRun.status.steps);
-      const { params, resources: inputResources } = taskRun.spec.inputs;
-      const { resources: outputResources } = taskRun.spec.outputs;
-      const { startTime } = taskRun.status;
-      return {
-        id: taskRun.metadata.uid,
-        pipelineTaskName,
-        pod: taskRun.status.podName,
-        reason,
-        steps: runSteps,
-        succeeded,
-        taskName,
-        taskRunName,
-        startTime,
-        namespace: taskRunNamespace,
-        params,
-        inputResources,
-        outputResources
-      };
-    });
+    taskRuns = taskRuns
+      .map(taskRun => {
+        if (!taskRun) {
+          return null;
+        }
+        const taskName = taskRun.spec.taskRef.name;
+        const taskRunName = taskRun.metadata.name;
+        const taskRunNamespace = taskRun.metadata.namespace;
+        const { reason, status: succeeded } = getStatus(taskRun);
+        const pipelineTaskName = taskRunName;
+        const runSteps = stepsStatus(task.spec.steps, taskRun.status.steps);
+        const { params, resources: inputResources } = taskRun.spec.inputs;
+        const { resources: outputResources } = taskRun.spec.outputs;
+        const { startTime } = taskRun.status;
+        return {
+          id: taskRun.metadata.uid,
+          pipelineTaskName,
+          pod: taskRun.status.podName,
+          reason,
+          steps: runSteps,
+          succeeded,
+          taskName,
+          taskRunName,
+          startTime,
+          namespace: taskRunNamespace,
+          params,
+          inputResources,
+          outputResources
+        };
+      })
+      .filter(Boolean);
     return taskRuns;
   };
 

diff --git a/src/reducers/reducerCreators.js b/src/reducers/reducerCreators.js
@@ -53,6 +53,9 @@ function createByNamespaceReducer({ type }) {
         return merge({}, state, resource);
       case `${type}Deleted`:
         const newState = { ...state };
+        if (!newState[action.payload.metadata.namespace]) {
+          return newState;
+        }
         delete newState[action.payload.metadata.namespace][
           action.payload.metadata.name
         ];

diff --git a/src/reducers/reducerCreators.test.js b/src/reducers/reducerCreators.test.js
@@ -131,6 +131,16 @@ it('PipelineResource Events', () => {
   ).toBeNull();
 });
 
+it('delete from missing namespace', () => {
+  const deleteAction = {
+    type: 'PipelineResourceDeleted',
+    payload: pipelineResource,
+    namespace
+  };
+
+  expect(() => reducer({}, deleteAction)).not.toThrow();
+});
+
 it('getPipelineResources', () => {
   const selectedNamespace = 'namespace';
   const state = { byNamespace: {} };