ui: use task state to determine if task is active (#14224)

The current implementation uses the task's finishedAt field to determine
if a task is active of not, but this check is not accurate. A task in
the "pending" state will not have finishedAt value but it's also not
active.

This discrepancy results in some components, like the inline stats chart
of the task row component, to be displayed even whey they shouldn't.

Author: lgfa29

.changelog/14224.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+ui: Fixed a bug that caused the allocation details page to display the stats bar chart even if the task was pending.
+```

ui/app/models/task-state.js

@@ -1,5 +1,5 @@
 import { computed } from '@ember/object';
-import { alias, none, and } from '@ember/object/computed';
+import { alias, and } from '@ember/object/computed';
 import Fragment from 'ember-data-model-fragments/fragment';
 import { attr } from '@ember-data/model';
 import { fragment, fragmentOwner, fragmentArray } from 'ember-data-model-fragments/attributes';
@@ -15,7 +15,6 @@ export default class TaskState extends Fragment {
   @attr('date') finishedAt;
   @attr('boolean') failed;
 
-  @none('finishedAt') isActive;
   @and('isActive', 'allocation.isRunning') isRunning;
 
   @computed('task.kind')
@@ -54,6 +53,11 @@ export default class TaskState extends Fragment {
     return classMap[this.state] || 'is-dark';
   }
 
+  @computed('state')
+  get isActive() {
+    return this.state === 'running';
+  }
+
   restart() {
     return this.allocation.restart(this.name);
   }

ui/tests/acceptance/allocation-detail-test.js

@@ -122,22 +122,45 @@ module('Acceptance | allocation detail', function(hooks) {
   });
 
   test('each task row should list high-level information for the task', async function(assert) {
-    const task = server.db.taskStates.where({ allocationId: allocation.id }).sortBy('name')[0];
-    const events = server.db.taskEvents.where({ taskStateId: task.id });
-    const event = events[events.length - 1];
+    const job = server.create('job', {
+      groupsCount: 1,
+      groupTaskCount: 3,
+      withGroupServices: true,
+      createAllocations: false,
+    });
+
+    const allocation = server.create('allocation', 'withTaskWithPorts', {
+      clientStatus: 'running',
+      jobId: job.id,
+    });
 
     const taskGroup = server.schema.taskGroups.where({
       jobId: allocation.jobId,
       name: allocation.taskGroup,
     }).models[0];
 
-    const jobTask = taskGroup.tasks.models.find(m => m.name === task.name);
-    const volumes = jobTask.volumeMounts.map(volume => ({
-      name: volume.Volume,
-      source: taskGroup.volumes[volume.Volume].Source,
-    }));
+    // Set the expected task states.
+    const states = ['running', 'pending', 'dead'];
+    server.db.taskStates
+      .where({ allocationId: allocation.id })
+      .sortBy('name')
+      .forEach((task, i) => {
+        server.db.taskStates.update(task.id, { state: states[i] });
+      });
+
+    await Allocation.visit({ id: allocation.id });
+
+    Allocation.tasks.forEach((taskRow, i) => {
+      const task = server.db.taskStates.where({ allocationId: allocation.id }).sortBy('name')[i];
+      const events = server.db.taskEvents.where({ taskStateId: task.id });
+      const event = events[events.length - 1];
+
+      const jobTask = taskGroup.tasks.models.find(m => m.name === task.name);
+      const volumes = jobTask.volumeMounts.map(volume => ({
+        name: volume.Volume,
+        source: taskGroup.volumes[volume.Volume].Source,
+      }));
 
-    Allocation.tasks[0].as(taskRow => {
       assert.equal(taskRow.name, task.name, 'Name');
       assert.equal(taskRow.state, task.state, 'State');
       assert.equal(taskRow.message, event.displayMessage, 'Event Message');
@@ -147,6 +170,10 @@ module('Acceptance | allocation detail', function(hooks) {
         'Event Time'
       );
 
+      const expectStats = task.state === 'running';
+      assert.equal(taskRow.hasCpuMetrics, expectStats, 'CPU metrics');
+      assert.equal(taskRow.hasMemoryMetrics, expectStats, 'Memory metrics');
+
       const volumesText = taskRow.volumes;
       volumes.forEach(volume => {
         assert.ok(volumesText.includes(volume.name), `Found label ${volume.name}`);
@@ -271,7 +298,9 @@ module('Acceptance | allocation detail', function(hooks) {
     await Allocation.stop.confirm();
 
     assert.equal(
-      server.pretender.handledRequests.reject(request => request.url.includes('fuzzy')).findBy('method', 'POST').url,
+      server.pretender.handledRequests
+        .reject(request => request.url.includes('fuzzy'))
+        .findBy('method', 'POST').url,
       `/v1/allocation/${allocation.id}/stop`,
       'Stop request is made for the allocation'
     );

ui/tests/acceptance/exec-test.js

@@ -26,11 +26,12 @@ module('Acceptance | exec', function(hooks) {
     });
 
     this.job.taskGroups.models.forEach(taskGroup => {
-      server.create('allocation', {
+      const alloc = server.create('allocation', {
         jobId: this.job.id,
         taskGroup: taskGroup.name,
         forceRunningClientStatus: true,
       });
+      server.db.taskStates.update({ allocationId: alloc.id }, { state: 'running' });
     });
   });
 
@@ -127,9 +128,11 @@ module('Acceptance | exec', function(hooks) {
 
     let runningTaskGroup = this.job.taskGroups.models.sortBy('name')[1];
     runningTaskGroup.tasks.models.forEach((task, index) => {
+      let state = 'running';
       if (index > 0) {
-        this.server.db.taskStates.update({ name: task.name }, { finishedAt: new Date() });
+        state = 'dead';
       }
+      this.server.db.taskStates.update({ name: task.name }, { state });
     });
 
     await Exec.visitJob({ job: this.job.id });
@@ -148,9 +151,11 @@ module('Acceptance | exec', function(hooks) {
     let runningTaskGroup = this.job.taskGroups.models.sortBy('name')[1];
     let changingTaskStateName;
     runningTaskGroup.tasks.models.sortBy('name').forEach((task, index) => {
+      let state = 'running';
       if (index > 0) {
-        this.server.db.taskStates.update({ name: task.name }, { finishedAt: new Date() });
+        state = 'dead';
       }
+      this.server.db.taskStates.update({ name: task.name }, { state });
 
       if (index === 1) {
         changingTaskStateName = task.name;
@@ -170,7 +175,7 @@ module('Acceptance | exec', function(hooks) {
         const changingTaskState = allocation.states.findBy('name', changingTaskStateName);
 
         if (changingTaskState) {
-          changingTaskState.set('finishedAt', undefined);
+          changingTaskState.set('state', 'running');
         }
       });
 

ui/tests/acceptance/task-detail-test.js

@@ -18,7 +18,10 @@ module('Acceptance | task detail', function(hooks) {
     server.create('agent');
     server.create('node');
     server.create('job', { createAllocations: false });
-    allocation = server.create('allocation', 'withTaskWithPorts', { clientStatus: 'running' });
+    allocation = server.create('allocation', 'withTaskWithPorts', {
+      clientStatus: 'running',
+    });
+    server.db.taskStates.update({ allocationId: allocation.id }, { state: 'running' });
     task = server.db.taskStates.where({ allocationId: allocation.id })[0];
 
     await Task.visit({ id: allocation.id, name: task.name });

ui/tests/pages/allocations/detail.js

@@ -50,6 +50,8 @@ export default create({
     time: text('[data-test-time]'),
     volumes: text('[data-test-volumes]'),
 
+    hasCpuMetrics: isPresent('[data-test-cpu] .inline-chart progress'),
+    hasMemoryMetrics: isPresent('[data-test-mem] .inline-chart progress'),
     hasUnhealthyDriver: isPresent('[data-test-icon="unhealthy-driver"]'),
     hasProxyTag: isPresent('[data-test-proxy-tag]'),