From a637354ae051d652420d7400f29acfa562b9a5b5 Mon Sep 17 00:00:00 2001
From: Phil Renaud <phil.renaud@hashicorp.com>
Date: Fri, 28 Apr 2023 14:52:42 -0400
Subject: [PATCH] [ui, deployments] Don't separate allocation groups based on
 their deployment health unless they're "running" (#17016)

* Group up non-running allocs regardless of deploymenthealth

* Better asynchrony in test
---
 .../components/job-status/panel/deploying.js  | 12 ++-
 ui/mirage/scenarios/default.js                |  6 ++
 .../components/job-status-panel-test.js       | 82 +++++++++++++++++++
 3 files changed, 99 insertions(+), 1 deletion(-)

diff --git a/ui/app/components/job-status/panel/deploying.js b/ui/app/components/job-status/panel/deploying.js
index 966dcac7e..53c66f194 100644
--- a/ui/app/components/job-status/panel/deploying.js
+++ b/ui/app/components/job-status/panel/deploying.js
@@ -100,9 +100,19 @@ export default class JobStatusPanelDeployingComponent extends Component {
         break;
       }
       let status = alloc.clientStatus;
-      let health = alloc.isHealthy ? 'healthy' : 'unhealthy';
       let canary = alloc.isCanary ? 'canary' : 'nonCanary';
 
+      // Health status only matters in the context of a "running" allocation.
+      // However, healthy/unhealthy is never purged when an allocation moves to a different clientStatus
+      // Thus, we should only show something as "healthy" in the event that it is running.
+      // Otherwise, we'd have arbitrary groupings based on previous health status.
+      let health =
+        status === 'running'
+          ? alloc.isHealthy
+            ? 'healthy'
+            : 'unhealthy'
+          : 'unhealthy';
+
       if (allocationCategories[status]) {
         allocationCategories[status][health][canary].push(alloc);
         availableSlotsToFill--;
diff --git a/ui/mirage/scenarios/default.js b/ui/mirage/scenarios/default.js
index 14334d40c..1fb9b0db3 100644
--- a/ui/mirage/scenarios/default.js
+++ b/ui/mirage/scenarios/default.js
@@ -158,6 +158,12 @@ function smallCluster(server) {
     .forEach((a) =>
       a.update({ deploymentStatus: { Healthy: true, Canary: true } })
     );
+  activelyDeployingJobAllocs.models
+    .filter((a) => a.clientStatus === 'failed')
+    .slice(0, 5)
+    .forEach((a) =>
+      a.update({ deploymentStatus: { Healthy: true, Canary: false } })
+    );
 
   //#endregion Active Deployment
 
diff --git a/ui/tests/integration/components/job-status-panel-test.js b/ui/tests/integration/components/job-status-panel-test.js
index 9280e7417..9e8b2c985 100644
--- a/ui/tests/integration/components/job-status-panel-test.js
+++ b/ui/tests/integration/components/job-status-panel-test.js
@@ -384,6 +384,88 @@ module(
       ); //keyframe animation fades from opacity 0
     });
 
+    test('non-running allocations are grouped regardless of health', async function (assert) {
+      this.server.create('node');
+
+      const NUMBER_OF_GROUPS = 1;
+      const ALLOCS_PER_GROUP = 100;
+      const allocStatusDistribution = {
+        running: 0.9,
+        failed: 0.1,
+        unknown: 0,
+        lost: 0,
+        complete: 0,
+        pending: 0,
+      };
+
+      const job = await this.server.create('job', {
+        type: 'service',
+        createAllocations: true,
+        noDeployments: true, // manually created below
+        activeDeployment: true,
+        groupTaskCount: ALLOCS_PER_GROUP,
+        shallow: true,
+        resourceSpec: Array(NUMBER_OF_GROUPS).fill(['M: 257, C: 500']), // length of this array determines number of groups
+        allocStatusDistribution,
+      });
+
+      const jobRecord = await this.store.find(
+        'job',
+        JSON.stringify([job.id, 'default'])
+      );
+      await this.server.create('deployment', false, 'active', {
+        jobId: job.id,
+        groupDesiredTotal: ALLOCS_PER_GROUP,
+        versionNumber: 1,
+        status: 'failed',
+      });
+
+      let activelyDeployingJobAllocs = server.schema.allocations
+        .all()
+        .filter((a) => a.jobId === job.id);
+
+      activelyDeployingJobAllocs.models
+        .filter((a) => a.clientStatus === 'failed')
+        .slice(0, 10)
+        .forEach((a) =>
+          a.update({ deploymentStatus: { Healthy: true, Canary: false } })
+        );
+
+      this.set('job', jobRecord);
+
+      await this.get('job.latestDeployment');
+      await this.set('job.latestDeployment.status', 'running');
+
+      await this.get('job.allocations');
+
+      await render(hbs`
+        <JobStatus::Panel @job={{this.job}} />
+      `);
+
+      assert
+        .dom('.allocation-status-block .represented-allocation.failed')
+        .exists({ count: 1 }, 'Failed block exists only once');
+      assert
+        .dom('.allocation-status-block .represented-allocation.failed')
+        .hasClass('rest', 'Failed block is a summary block');
+
+      await Promise.all(
+        this.get('job.allocations')
+          .filterBy('clientStatus', 'failed')
+          .slice(0, 3)
+          .map(async (a) => {
+            await a.set('deploymentStatus', { Healthy: false, Canary: true });
+          })
+      );
+
+      assert
+        .dom('.represented-allocation.failed.rest')
+        .exists(
+          { count: 2 },
+          'Now that some are canaries, they still make up two blocks'
+        );
+    });
+
     test('when there is no running deployment, the latest deployment section shows up for the last deployment', async function (assert) {
       this.server.create('job', {
         type: 'service',