fix cluster upgrade modal not showing intermittently (#4488)

* fix cluster upgrade modal not showing intermittently * FE unit tests * remove GetLatestInstallation function
replicatedhq · Mar 4, 2024 · ad728ed · ad728ed
1 parent 30eeb4b
commit ad728ed
Show file tree

Hide file tree

Showing 5 changed files with 84 additions and 17 deletions.
diff --git a/pkg/api/handlers/types/types.go b/pkg/api/handlers/types/types.go
@@ -97,6 +97,8 @@ type ResponseCluster struct {
 	RequiresUpgrade bool `json:"requiresUpgrade"`
 	// State represents the current state of the most recently deployed embedded cluster config
 	State string `json:"state,omitempty"`
+	// NumInstallations represents the number of installation objects in the cluster
+	NumInstallations int `json:"numInstallations"`
 }
 
 type GetPendingAppResponse struct {

diff --git a/pkg/embeddedcluster/util.go b/pkg/embeddedcluster/util.go
@@ -60,6 +60,20 @@ func RequiresUpgrade(ctx context.Context, newcfg embeddedclusterv1beta1.ConfigSp
 
 // GetCurrentInstallation returns the most recent installation object from the cluster.
 func GetCurrentInstallation(ctx context.Context) (*embeddedclusterv1beta1.Installation, error) {
+	installations, err := ListInstallations(ctx)
+	if err != nil {
+		return nil, fmt.Errorf("failed to list installations: %w", err)
+	}
+	if len(installations) == 0 {
+		return nil, ErrNoInstallations
+	}
+	sort.SliceStable(installations, func(i, j int) bool {
+		return installations[j].CreationTimestamp.Before(&installations[i].CreationTimestamp)
+	})
+	return &installations[0], nil
+}
+
+func ListInstallations(ctx context.Context) ([]embeddedclusterv1beta1.Installation, error) {
 	clientConfig, err := k8sutil.GetClusterConfig()
 	if err != nil {
 		return nil, fmt.Errorf("failed to get cluster config: %w", err)
@@ -75,14 +89,7 @@ func GetCurrentInstallation(ctx context.Context) (*embeddedclusterv1beta1.Instal
 	if err != nil {
 		return nil, fmt.Errorf("failed to list installations: %w", err)
 	}
-	if len(installationList.Items) == 0 {
-		return nil, ErrNoInstallations
-	}
-	items := installationList.Items
-	sort.SliceStable(items, func(i, j int) bool {
-		return items[j].CreationTimestamp.Before(&items[i].CreationTimestamp)
-	})
-	return &installationList.Items[0], nil
+	return installationList.Items, nil
 }
 
 // ClusterConfig will extract the current cluster configuration from the latest installation

diff --git a/pkg/handlers/app.go b/pkg/handlers/app.go
@@ -341,11 +341,21 @@ func responseAppFromApp(a *apptypes.App) (*types.ResponseApp, error) {
 				return nil, errors.Wrap(err, "failed to check if cluster requires upgrade")
 			}
 
-			embeddedClusterInstallation, err := embeddedcluster.GetCurrentInstallation(context.TODO())
+			embeddedClusterInstallations, err := embeddedcluster.ListInstallations(context.TODO())
 			if err != nil {
-				return nil, errors.Wrap(err, "failed to get current installation")
+				return nil, errors.Wrap(err, "failed to list installations")
+			}
+
+			cluster.NumInstallations = len(embeddedClusterInstallations)
+
+			currentInstallation, err := embeddedcluster.GetCurrentInstallation(context.TODO())
+			if err != nil {
+				return nil, errors.Wrap(err, "failed to get latest installation")
+			}
+
+			if currentInstallation != nil {
+				cluster.State = string(currentInstallation.Status.State)
 			}
-			cluster.State = string(embeddedClusterInstallation.Status.State)
 		}
 	}
 

diff --git a/web/src/utilities/utilities.js b/web/src/utilities/utilities.js
@@ -672,11 +672,25 @@ export const Utilities = {
     }
   },
 
-  isClusterUpgrading(state) {
-    const normalizedState = this.clusterState(state);
-    return (
-      normalizedState === "Upgrading" || normalizedState === "Upgrading addons"
-    );
+  isClusterUpgrading(cluster) {
+    if (!cluster) {
+      return false;
+    }
+    const normalizedState = this.clusterState(cluster.state);
+    if (
+      normalizedState === "Upgrading" ||
+      normalizedState === "Upgrading addons"
+    ) {
+      return true;
+    }
+
+    if (cluster.numInstallations > 1 && !cluster.state) {
+      // if there multiple installations and no state, assume it's upgrading
+      // as it's possible the downtime begins before a state is reported
+      return true;
+    }
+
+    return false;
   },
 
   isPendingClusterUpgrade(app) {
@@ -696,7 +710,7 @@ export const Utilities = {
     // and the cluster will upgrade or is already upgrading
     return (
       app.downstream?.cluster?.requiresUpgrade ||
-      Utilities.isClusterUpgrading(app.downstream?.cluster?.state)
+      Utilities.isClusterUpgrading(app.downstream?.cluster)
     );
   },
 

diff --git a/web/src/utilities/utilities.test.js b/web/src/utilities/utilities.test.js
@@ -100,6 +100,40 @@ describe("Utilities", () => {
       ];
       expect(Utilities.shouldShowClusterUpgradeModal(apps)).toBe(true);
     });
+
+    it("should return false if there are is one installation that does not have a state", () => {
+      const apps = [
+        {
+          downstream: {
+            currentVersion: {
+              status: "deployed",
+            },
+            cluster: {
+              requiresUpgrade: false,
+              numInstallations: 1,
+            },
+          },
+        },
+      ];
+      expect(Utilities.shouldShowClusterUpgradeModal(apps)).toBe(false);
+    });
+
+    it("should return true if there are multiple installations, but the latest does not have a state", () => {
+      const apps = [
+        {
+          downstream: {
+            currentVersion: {
+              status: "deployed",
+            },
+            cluster: {
+              requiresUpgrade: false,
+              numInstallations: 2,
+            },
+          },
+        },
+      ];
+      expect(Utilities.shouldShowClusterUpgradeModal(apps)).toBe(true);
+    });
   });
 
   describe("isInitialAppInstall", () => {