Handle showing errors for bad kots release blocking diff creation for…

… newer releases (#905) * support showing error when generating a diff was unsuccessful * Do not fail on license sync error on application updates (#899) * Do not fail on license sync error on appication updates * fixes * log error * fail on license sync * Diff renderedcontents handler return error response on failed kustomize build * handle unable to generate diff errors * update var name * update conditional Co-authored-by: emosbaugh <ethan@replicated.com>
replicatedhq · Aug 4, 2020 · 9fa8c74 · 9fa8c74
1 parent 50210c5
commit 9fa8c74
Show file tree

Hide file tree

Showing 4 changed files with 119 additions and 37 deletions.
diff --git a/kotsadm/web/src/components/apps/AppVersionHistory.jsx b/kotsadm/web/src/components/apps/AppVersionHistory.jsx
@@ -62,7 +62,8 @@ class AppVersionHistory extends Component {
     displayShowDetailsModal: false,
     yamlErrorDetails: [],
     deployView: false,
-    selectedSequence: ""
+    selectedSequence: "",
+    releaseWithErr: {}
   }
 
   componentDidMount() {
@@ -119,6 +120,13 @@ class AppVersionHistory extends Component {
     });
   }
 
+  toggleDiffErrModal = (release) => {
+    this.setState({ 
+      showDiffErrModal: !this.state.showDiffErrModal,
+      releaseWithErr: !this.state.showDiffErrModal ? release : {}
+    })
+  }
+
   getVersionDiffSummary = version => {
     if (!version.diffSummary || version.diffSummary === "") {
       return null;
@@ -143,9 +151,9 @@ class AppVersionHistory extends Component {
 
   renderYamlErrors = (yamlErrorsDetails, version) => {
     return (
-      <div className="flex alignItems--center u-marginLeft--10">
+      <div className="flex alignItems--center">
         <span className="icon error-small" />
-        <span className="u-fontSize--small u-fontWeight--medium u-lineHeight--normal u-marginLeft--5 u-color--red">{yamlErrorsDetails?.length} Invalid files </span>
+        <span className="u-fontSize--small u-fontWeight--medium u-lineHeight--normal u-marginLeft--5 u-color--red">{yamlErrorsDetails?.length} Invalid file{yamlErrorsDetails?.length !== 1 ? "s" : ""} </span>
         <span className="replicated-link u-marginLeft--5 u-fontSize--small" onClick={() => this.toggleShowDetailsModal(yamlErrorsDetails, version.sequence)}> See details </span>
       </div>
     )
@@ -155,35 +163,45 @@ class AppVersionHistory extends Component {
     const { app } = this.props;
     const downstream = app.downstreams?.length && app.downstreams[0];
     const diffSummary = this.getVersionDiffSummary(version);
+    const hasDiffSummaryError = version.diffSummaryError && version.diffSummaryError.length > 0;
 
-    return (
-      <div>
-        {diffSummary ?
-          (diffSummary.filesChanged > 0 ?
-            <div
-              className="DiffSummary u-cursor--pointer"
-              onClick={() => {
-                if (!downstream.gitops?.enabled) {
-                  this.setState({
-                    showDiffOverlay: true,
-                    firstSequence: version.parentSequence - 1,
-                    secondSequence: version.parentSequence
-                  });
-                }
-              }}
-            >
-              <span className="files">{diffSummary.filesChanged} files changed </span>
-              <span className="lines-added">+{diffSummary.linesAdded} </span>
-              <span className="lines-removed">-{diffSummary.linesRemoved}</span>
-            </div>
-            :
-            <div className="DiffSummary">
-              <span className="files">No changes</span>
-            </div>
-          )
-          : <span>&nbsp;</span>}
+    if (hasDiffSummaryError) {
+      return (
+        <div className="flex flex1 alignItems--center u-marginRight--10">
+          <span className="u-fontSize--small u-fontWeight--medium u-lineHeight--normal u-color--dustyGray">Cannot generate diff <span className="replicated-link" onClick={() => this.toggleDiffErrModal(version)}>Why?</span></span>
       </div>
-    );
+      );
+    } else {
+      return (
+        <div>
+          {diffSummary ?
+            (diffSummary.filesChanged > 0 ?
+              <div
+                className="DiffSummary u-cursor--pointer u-marginRight--10"
+                onClick={() => {
+                  if (!downstream.gitops?.enabled) {
+                    this.setState({
+                      showDiffOverlay: true,
+                      firstSequence: version.parentSequence - 1,
+                      secondSequence: version.parentSequence
+                    });
+                  }
+                }}
+              >
+                <span className="files">{diffSummary.filesChanged} files changed </span>
+                <span className="lines-added">+{diffSummary.linesAdded} </span>
+                <span className="lines-removed">-{diffSummary.linesRemoved}</span>
+              </div>
+              :
+              <div className="DiffSummary">
+                <span className="files">No changes</span>
+              </div>
+            )
+            : <span>&nbsp;</span>}
+        </div>
+      );
+    }
+
   }
 
   renderVersionAction = (version, nothingToCommitDiff) => {
@@ -448,10 +466,13 @@ class AppVersionHistory extends Component {
     });
   }
 
-  hideDiffOverlay = () => {
+  hideDiffOverlay = (closeReleaseSelect) => {
     this.setState({
       showDiffOverlay: false
     });
+    if (closeReleaseSelect) {
+      this.onCloseReleasesToDiff();
+    }
   }
 
   onSelectReleasesToDiff = () => {
@@ -1193,6 +1214,25 @@ class AppVersionHistory extends Component {
             <button className="btn primary" onClick={this.hideDownstreamReleaseNotes}>Close</button>
           </div>
         </Modal>
+
+        <Modal
+          isOpen={this.state.showDiffErrModal}
+          onRequestClose={this.toggleDiffErrModal}
+          contentLabel="Unable to Get Diff"
+          ariaHideApp={false}
+          className="Modal MediumSize"
+        >
+          <div className="Modal-body">
+            <p className="u-fontSize--largest u-fontWeight--bold u-color--tuna u-lineHeight--normal u-marginBottom--10">Unable to generate a file diff for release</p>
+            <p className="u-fontSize--normal u-color--dustyGray u-lineHeight--normal u-marginBottom--20">The release with the <span className="u-fontWeight--bold">Upstream {this.state.releaseWithErr.title}, Sequence {this.state.releaseWithErr.sequence}</span> was unable to generate a files diff because the following error:</p>
+            <div className="error-block-wrapper u-marginBottom--30 flex flex1">
+              <span className="u-color--chestnut">{this.state.releaseWithErr.diffSummaryError}</span>
+            </div>
+            <div className="flex u-marginBottom--10">
+              <button className="btn primary" onClick={this.toggleDiffErrModal}>Ok, got it!</button>
+            </div>
+          </div>
+        </Modal>
 
         {showUpdateCheckerModal &&
           <UpdateCheckerModal

diff --git a/kotsadm/web/src/components/watches/DownstreamWatchVersionDiff.jsx b/kotsadm/web/src/components/watches/DownstreamWatchVersionDiff.jsx
@@ -13,15 +13,20 @@ class DownstreamWatchVersionDiff extends React.Component {
   constructor() {
     super();
     this.state = {
+      loadingFileTrees: false,
       firstApplicationTree: {},
       secondApplicationTree: {},
       fileLoading: false,
       fileLoadErr: false,
       fileLoadErrMessage: "",
+      hasErrSettingDiff: false,
+      errSettingDiff: "",
+      failedSequence: undefined
     };
   }
 
   fetchRenderedApplicationTree = (sequence, isFirst) => {
+    this.setState({ loadingFileTrees: true });
     const url = `${window.env.API_ENDPOINT}/app/${this.props.slug}/sequence/${sequence}/renderedcontents`;
     fetch(url, {
       headers: {
@@ -31,13 +36,25 @@ class DownstreamWatchVersionDiff extends React.Component {
     })
     .then(res => res.json())
     .then(async (files) => {
+      if (files.error) {
+        return this.setState({
+          hasErrSettingDiff: true,
+          errSettingDiff: files.error,
+          loadingFileTrees: false,
+          failedSequence: sequence
+        })
+      }
       if (isFirst) {
-        this.setState({firstApplicationTree: files});
+        this.setState({ firstApplicationTree: files });
       } else {
-        this.setState({secondApplicationTree: files});
+        this.setState({ secondApplicationTree: files });
+      }
+      if (this.state.firstApplicationTree.files && this.state.secondApplicationTree.files) {
+        this.setState({ loadingFileTrees: false });  
       }
     })
     .catch((err) => {
+      this.setState({ loadingFileTrees: false });
       throw err;
     });
   }
@@ -73,22 +90,37 @@ class DownstreamWatchVersionDiff extends React.Component {
 
   goBack = () => {
     if (this.props.onBackClick) {
-      this.props.onBackClick();
+      this.props.onBackClick(true);
     }
   }
 
   render() {
-    const { firstApplicationTree, secondApplicationTree } = this.state;
+    const { firstApplicationTree, secondApplicationTree, loadingFileTrees, hasErrSettingDiff, errSettingDiff, failedSequence } = this.state;
     const { firstSequence, secondSequence } = this.props;
 
-    if (!firstApplicationTree.files || !secondApplicationTree.files) {
+    if (loadingFileTrees) {
       return (
         <div className="u-height--full u-width--full flex alignItems--center justifyContent--center u-marginTop--15">
           <Loader size="60" />
         </div>
       );
     }
 
+    if (hasErrSettingDiff) {
+      return (
+        <div className="u-height--full u-width--full flex-column alignItems--center justifyContent--center u-marginTop--15">
+          <p className="u-fontSize--largest u-fontWeight--bold u-color--tuna u-lineHeight--normal u-marginBottom--10">Unable to generate a file diff for the selected releases</p>
+          <p className="u-fontSize--normal u-color--dustyGray u-lineHeight--normal u-marginBottom--20">The release with the sequence <span className="u-fontWeight--bold">{failedSequence}</span> contains invalid YAML or config values and is unable to generate a diff. The full error is below.</p>
+          <div className="error-block-wrapper u-marginBottom--30 flex flex1">
+            <span className="u-color--chestnut">{errSettingDiff}</span>
+          </div>
+          <div className="flex u-marginBottom--10">
+            <button className="btn primary" onClick={() => this.goBack()}>Back to all versions</button>
+          </div>
+        </div>
+      )
+    }
+
     const filesToInclude = [];
     for (const filename in firstApplicationTree.files) {
       if (firstApplicationTree.files[filename] === secondApplicationTree.files[filename]) {

diff --git a/kotsadm/web/src/scss/components/apps/AppVersionHistory.scss b/kotsadm/web/src/scss/components/apps/AppVersionHistory.scss
@@ -234,7 +234,7 @@ $cell-width: 140px;
 .overlay {
   user-select: none;
   cursor: pointer;
-  a, button {
+  a, button, span {
     pointer-events: none;
   }
 }

diff --git a/kotsadm/web/src/scss/utilities/base.scss b/kotsadm/web/src/scss/utilities/base.scss
@@ -204,6 +204,16 @@ code {
   padding: 10px;
 }
 
+.error-block-wrapper {
+  background-color: #FBECEA;
+  border-radius: 4px;
+  padding: 15px;
+  margin: 0 auto;
+  font-size: 14px;
+  line-height: 1.4;
+  font-weight: 500;
+}
+
 .errors {
   display: inline-block;
   color: #FFFFFF;