Merge pull request #4981 from hashicorp/b-ui-hide-stats-graphs-for-non-running-resources

UI: Hide stats graphs for non running resources

ui/app/components/allocation-row.js

@@ -26,7 +26,9 @@ export default Component.extend({
 
   enablePolling: computed(() => !Ember.testing),
 
-  stats: computed('allocation', function() {
+  stats: computed('allocation', 'allocation.isRunning', function() {
+    if (!this.get('allocation.isRunning')) return;
+
     return AllocationStatsTracker.create({
       fetch: url => this.get('token').authorizedRequest(url),
       allocation: this.get('allocation'),
@@ -54,12 +56,15 @@ export default Component.extend({
 
   fetchStats: task(function*() {
     do {
-      try {
-        yield this.get('stats.poll').perform();
-        this.set('statsError', false);
-      } catch (error) {
-        this.set('statsError', true);
+      if (this.get('stats')) {
+        try {
+          yield this.get('stats.poll').perform();
+          this.set('statsError', false);
+        } catch (error) {
+          this.set('statsError', true);
+        }
       }
+
       yield timeout(500);
     } while (this.get('enablePolling'));
   }).drop(),

ui/app/components/task-row.js

@@ -22,13 +22,16 @@ export default Component.extend({
   enablePolling: computed(() => !Ember.testing),
 
   // Since all tasks for an allocation share the same tracker, use the registry
-  stats: computed('task', function() {
+  stats: computed('task', 'task.isRunning', function() {
+    if (!this.get('task.isRunning')) return;
+
     return this.get('statsTrackersRegistry').getTracker(this.get('task.allocation'));
   }),
 
   taskStats: computed('task.name', 'stats.tasks.[]', function() {
-    const ret = this.get('stats.tasks').findBy('task', this.get('task.name'));
-    return ret;
+    if (!this.get('stats')) return;
+
+    return this.get('stats.tasks').findBy('task', this.get('task.name'));
   }),
 
   cpu: alias('taskStats.cpu.lastObject'),
@@ -42,12 +45,15 @@ export default Component.extend({
 
   fetchStats: task(function*() {
     do {
-      try {
-        yield this.get('stats.poll').perform();
-        this.set('statsError', false);
-      } catch (error) {
-        this.set('statsError', true);
+      if (this.get('stats')) {
+        try {
+          yield this.get('stats.poll').perform();
+          this.set('statsError', false);
+        } catch (error) {
+          this.set('statsError', true);
+        }
       }
+
       yield timeout(500);
     } while (this.get('enablePolling'));
   }).drop(),

ui/app/models/allocation.js

@@ -1,5 +1,6 @@
 import { inject as service } from '@ember/service';
 import { computed } from '@ember/object';
+import { equal } from '@ember/object/computed';
 import Model from 'ember-data/model';
 import attr from 'ember-data/attr';
 import { belongsTo } from 'ember-data/relationships';
@@ -38,6 +39,8 @@ export default Model.extend({
     return STATUS_ORDER[this.get('clientStatus')] || 100;
   }),
 
+  isRunning: equal('clientStatus', 'running'),
+
   // When allocations are server-side rescheduled, a paper trail
   // is left linking all reschedule attempts.
   previousAllocation: belongsTo('allocation', { inverse: 'nextAllocation' }),

ui/app/models/task-state.js

@@ -1,11 +1,12 @@
-import { none } from '@ember/object/computed';
 import { computed } from '@ember/object';
-import { alias } from '@ember/object/computed';
+import { alias, none, and } from '@ember/object/computed';
 import Fragment from 'ember-data-model-fragments/fragment';
 import attr from 'ember-data/attr';
 import { fragment, fragmentOwner, fragmentArray } from 'ember-data-model-fragments/attributes';
 
 export default Fragment.extend({
+  allocation: fragmentOwner(),
+
   name: attr('string'),
   state: attr('string'),
   startedAt: attr('date'),
@@ -13,8 +14,8 @@ export default Fragment.extend({
   failed: attr('boolean'),
 
   isActive: none('finishedAt'),
+  isRunning: and('isActive', 'allocation.isRunning'),
 
-  allocation: fragmentOwner(),
   task: computed('allocation.taskGroup.tasks.[]', function() {
     const tasks = this.get('allocation.taskGroup.tasks');
     return tasks && tasks.findBy('name', this.get('name'));

ui/app/templates/allocations/allocation/index.hbs

@@ -22,14 +22,21 @@
       Resource Utilization
     </div>
     <div class="boxed-section-body">
-      <div class="columns">
-        <div class="column">
-          {{primary-metric resource=model metric="cpu"}}
+      {{#if model.isRunning}}
+        <div class="columns">
+          <div class="column">
+            {{primary-metric resource=model metric="cpu"}}
+          </div>
+          <div class="column">
+            {{primary-metric resource=model metric="memory"}}
+          </div>
         </div>
-        <div class="column">
-          {{primary-metric resource=model metric="memory"}}
+      {{else}}
+        <div data-test-resource-error class="empty-message">
+          <h3 data-test-resource-error-headline class="empty-message-headline">Allocation isn't running</h3>
+          <p class="empty-message-body">Only running allocations utilize resources.</p>
         </div>
-      </div>
+      {{/if}}
     </div>
   </div>
 

ui/app/templates/allocations/allocation/task/index.hbs

@@ -30,14 +30,21 @@
       Resource Utilization
     </div>
     <div class="boxed-section-body">
-      <div class="columns">
-        <div class="column">
-          {{primary-metric resource=model metric="cpu"}}
+      {{#if model.isRunning}}
+        <div class="columns">
+          <div class="column">
+            {{primary-metric resource=model metric="cpu"}}
+          </div>
+          <div class="column">
+            {{primary-metric resource=model metric="memory"}}
+          </div>
         </div>
-        <div class="column">
-          {{primary-metric resource=model metric="memory"}}
+      {{else}}
+        <div data-test-resource-error class="empty-message">
+          <h3 data-test-resource-error-headline class="empty-message-headline">Task isn't running</h3>
+          <p class="empty-message-body">Only running tasks utilize resources.</p>
         </div>
-      </div>
+      {{/if}}
     </div>
   </div>
 

ui/app/templates/components/allocation-row.hbs

@@ -42,42 +42,42 @@
   <td data-test-job-version class="is-1">{{allocation.jobVersion}}</td>
 {{/if}}
 <td data-test-cpu class="is-1 has-text-centered">
-  {{#if (and (not cpu) fetchStats.isRunning)}}
-    ...
-  {{else if (not allocation)}}
-    {{! nothing when there's no allocation}}
-  {{else if statsError}}
-    <span class="tooltip text-center" role="tooltip" aria-label="Couldn't collect stats">
-      {{x-icon "warning" class="is-warning"}}
-    </span>
-  {{else}}
-    <div class="inline-chart is-small tooltip" role="tooltip" aria-label="{{cpu.used}} / {{stats.reservedCPU}} MHz">
-      <progress
-        class="progress is-info is-small"
-        value="{{cpu.percent}}"
-        max="1">
-        {{cpu.percent}}
-      </progress>
-    </div>
+  {{#if allocation.isRunning}}
+    {{#if (and (not cpu) fetchStats.isRunning)}}
+      ...
+    {{else if statsError}}
+      <span class="tooltip text-center" role="tooltip" aria-label="Couldn't collect stats">
+        {{x-icon "warning" class="is-warning"}}
+      </span>
+    {{else}}
+      <div class="inline-chart is-small tooltip" role="tooltip" aria-label="{{cpu.used}} / {{stats.reservedCPU}} MHz">
+        <progress
+          class="progress is-info is-small"
+          value="{{cpu.percent}}"
+          max="1">
+          {{cpu.percent}}
+        </progress>
+      </div>
+    {{/if}}
   {{/if}}
 </td>
 <td data-test-mem class="is-1 has-text-centered">
-  {{#if (and (not memory) fetchStats.isRunning)}}
-    ...
-  {{else if (not allocation)}}
-    {{! nothing when there's no allocation}}
-  {{else if statsError}}
-    <span class="tooltip is-small text-center" role="tooltip" aria-label="Couldn't collect stats">
-      {{x-icon "warning" class="is-warning"}}
-    </span>
-  {{else}}
-    <div class="inline-chart tooltip" role="tooltip" aria-label="{{format-bytes memory.used}} / {{stats.reservedMemory}} MiB">
-      <progress
-        class="progress is-danger is-small"
-        value="{{memory.percent}}"
-        max="1">
-        {{memory.percent}}
-      </progress>
-    </div>
+  {{#if allocation.isRunning}}
+    {{#if (and (not memory) fetchStats.isRunning)}}
+      ...
+    {{else if statsError}}
+      <span class="tooltip is-small text-center" role="tooltip" aria-label="Couldn't collect stats">
+        {{x-icon "warning" class="is-warning"}}
+      </span>
+    {{else}}
+      <div class="inline-chart tooltip" role="tooltip" aria-label="{{format-bytes memory.used}} / {{stats.reservedMemory}} MiB">
+        <progress
+          class="progress is-danger is-small"
+          value="{{memory.percent}}"
+          max="1">
+          {{memory.percent}}
+        </progress>
+      </div>
+    {{/if}}
   {{/if}}
 </td>

ui/app/templates/components/task-row.hbs

@@ -38,42 +38,42 @@
   </ul>
 </td>
 <td data-test-cpu class="is-1 has-text-centered">
-  {{#if (and (not cpu) fetchStats.isRunning)}}
-    ...
-  {{else if (not task)}}
-    {{! nothing when there's no task}}
-  {{else if statsError}}
-    <span class="tooltip text-center" role="tooltip" aria-label="Couldn't collect stats">
-      {{x-icon "warning" class="is-warning"}}
-    </span>
-  {{else}}
-    <div class="inline-chart is-small tooltip" role="tooltip" aria-label="{{cpu.used}} / {{taskStats.reservedCPU}} MHz">
-      <progress
-        class="progress is-info is-small"
-        value="{{cpu.percent}}"
-        max="1">
-        {{cpu.percent}}
-      </progress>
-    </div>
+  {{#if task.isRunning}}
+    {{#if (and (not cpu) fetchStats.isRunning)}}
+      ...
+    {{else if statsError}}
+      <span class="tooltip text-center" role="tooltip" aria-label="Couldn't collect stats">
+        {{x-icon "warning" class="is-warning"}}
+      </span>
+    {{else}}
+      <div class="inline-chart is-small tooltip" role="tooltip" aria-label="{{cpu.used}} / {{taskStats.reservedCPU}} MHz">
+        <progress
+          class="progress is-info is-small"
+          value="{{cpu.percent}}"
+          max="1">
+          {{cpu.percent}}
+        </progress>
+      </div>
+    {{/if}}
   {{/if}}
 </td>
 <td data-test-mem class="is-1 has-text-centered">
-  {{#if (and (not memory) fetchStats.isRunning)}}
-    ...
-  {{else if (not task)}}
-    {{! nothing when there's no task}}
-  {{else if statsError}}
-    <span class="tooltip is-small text-center" role="tooltip" aria-label="Couldn't collect stats">
-      {{x-icon "warning" class="is-warning"}}
-    </span>
-  {{else}}
-    <div class="inline-chart tooltip" role="tooltip" aria-label="{{format-bytes memory.used}} / {{taskStats.reservedMemory}} MiB">
-      <progress
-        class="progress is-danger is-small"
-        value="{{memory.percent}}"
-        max="1">
-        {{memory.percent}}
-      </progress>
-    </div>
+  {{#if task.isRunning}}
+    {{#if (and (not memory) fetchStats.isRunning)}}
+      ...
+    {{else if statsError}}
+      <span class="tooltip is-small text-center" role="tooltip" aria-label="Couldn't collect stats">
+        {{x-icon "warning" class="is-warning"}}
+      </span>
+    {{else}}
+      <div class="inline-chart tooltip" role="tooltip" aria-label="{{format-bytes memory.used}} / {{taskStats.reservedMemory}} MiB">
+        <progress
+          class="progress is-danger is-small"
+          value="{{memory.percent}}"
+          max="1">
+          {{memory.percent}}
+        </progress>
+      </div>
+    {{/if}}
   {{/if}}
 </td>

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

@@ -191,3 +191,24 @@ moduleForAcceptance('Acceptance | allocation detail (rescheduled)', {
 test('when the allocation has been rescheduled, the reschedule events section is rendered', function(assert) {
   assert.ok(Allocation.hasRescheduleEvents, 'Reschedule Events section exists');
 });
+
+moduleForAcceptance('Acceptance | allocation detail (not running)', {
+  beforeEach() {
+    server.create('agent');
+
+    node = server.create('node');
+    job = server.create('job', { createAllocations: false });
+    allocation = server.create('allocation', { clientStatus: 'pending' });
+
+    Allocation.visit({ id: allocation.id });
+  },
+});
+
+test('when the allocation is not running, the utilization graphs are replaced by an empty message', function(assert) {
+  assert.equal(Allocation.resourceCharts.length, 0, 'No resource charts');
+  assert.equal(
+    Allocation.resourceEmptyMessage,
+    "Allocation isn't running",
+    'Empty message is appropriate'
+  );
+});

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

@@ -19,7 +19,7 @@ moduleForAcceptance('Acceptance | client detail', {
     // Related models
     server.create('agent');
     server.create('job', { createAllocations: false });
-    server.createList('allocation', 3, { nodeId: node.id });
+    server.createList('allocation', 3, { nodeId: node.id, clientStatus: 'running' });
   },
 });
 
@@ -545,10 +545,14 @@ moduleForAcceptance('Acceptance | client detail (multi-namespace)', {
 
     // Make a job for each namespace, but have both scheduled on the same node
     server.create('job', { id: 'job-1', namespaceId: 'default', createAllocations: false });
-    server.createList('allocation', 3, { nodeId: node.id });
+    server.createList('allocation', 3, { nodeId: node.id, clientStatus: 'running' });
 
     server.create('job', { id: 'job-2', namespaceId: 'other-namespace', createAllocations: false });
-    server.createList('allocation', 3, { nodeId: node.id, jobId: 'job-2' });
+    server.createList('allocation', 3, {
+      nodeId: node.id,
+      jobId: 'job-2',
+      clientStatus: 'running',
+    });
   },
 });
 

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

@@ -271,3 +271,22 @@ test('breadcrumbs match jobs / job / task group / allocation / task', function(a
     );
   });
 });
+
+moduleForAcceptance('Acceptance | task detail (not running)', {
+  beforeEach() {
+    server.create('agent');
+    server.create('node');
+    server.create('namespace');
+    server.create('namespace', { id: 'other-namespace' });
+    server.create('job', { createAllocations: false, namespaceId: 'other-namespace' });
+    allocation = server.create('allocation', 'withTaskWithPorts', { clientStatus: 'complete' });
+    task = server.db.taskStates.where({ allocationId: allocation.id })[0];
+
+    Task.visit({ id: allocation.id, name: task.name });
+  },
+});
+
+test('when the allocation for a task is not running, the resource utilization graphs are replaced by an empty message', function(assert) {
+  assert.equal(Task.resourceCharts.length, 0, 'No resource charts');
+  assert.equal(Task.resourceEmptyMessage, "Task isn't running", 'Empty message is appropriate');
+});

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

@@ -33,6 +33,7 @@ moduleForAcceptance('Acceptance | task group detail', {
     allocations = server.createList('allocation', 2, {
       jobId: job.id,
       taskGroup: taskGroup.name,
+      clientStatus: 'running',
     });
 
     // Allocations associated to a different task group on the job to
@@ -40,6 +41,7 @@ moduleForAcceptance('Acceptance | task group detail', {
     server.createList('allocation', 3, {
       jobId: job.id,
       taskGroup: taskGroups[1].name,
+      clientStatus: 'running',
     });
 
     // Set a static name to make the search test deterministic
@@ -118,6 +120,7 @@ test('/jobs/:id/:task-group should list one page of allocations for the task gro
   server.createList('allocation', TaskGroup.pageSize, {
     jobId: job.id,
     taskGroup: taskGroup.name,
+    clientStatus: 'running',
   });
 
   JobsList.visit();

ui/tests/integration/allocation-row-test.js

@@ -7,6 +7,7 @@ import { startMirage } from 'nomad-ui/initializers/ember-cli-mirage';
 import { find } from 'ember-native-dom-helpers';
 import Response from 'ember-cli-mirage/response';
 import { initialize as fragmentSerializerInitializer } from 'nomad-ui/initializers/fragment-serializer';
+import { Promise, resolve } from 'rsvp';
 
 moduleForComponent('allocation-row', 'Integration | Component | allocation row', {
   integration: true,
@@ -49,7 +50,7 @@ test('Allocation row polls for stats, even when it errors or has an invalid resp
     return new Response(500, {}, '');
   });
 
-  this.server.create('allocation');
+  this.server.create('allocation', { clientStatus: 'running' });
   this.store.findAll('allocation');
 
   let allocation;
@@ -93,7 +94,7 @@ test('Allocation row shows warning when it requires drivers that are unhealthy o
   });
   node.update({ drivers });
 
-  this.server.create('allocation');
+  this.server.create('allocation', { clientStatus: 'running' });
   this.store.findAll('job');
   this.store.findAll('node');
   this.store.findAll('allocation');
@@ -120,3 +121,67 @@ test('Allocation row shows warning when it requires drivers that are unhealthy o
       assert.ok(find('[data-test-icon="unhealthy-driver"]'), 'Unhealthy driver icon is shown');
     });
 });
+
+test('when an allocation is not running, the utilization graphs are omitted', function(assert) {
+  this.setProperties({
+    context: 'job',
+    enablePolling: false,
+  });
+
+  // All non-running statuses need to be tested
+  ['pending', 'complete', 'failed', 'lost'].forEach(clientStatus =>
+    this.server.create('allocation', { clientStatus })
+  );
+
+  this.store.findAll('allocation');
+
+  return wait().then(() => {
+    const allocations = this.store.peekAll('allocation');
+    return waitForEach(
+      allocations.map(allocation => () => {
+        this.set('allocation', allocation);
+        this.render(hbs`
+            {{allocation-row
+              allocation=allocation
+              context=context
+              enablePolling=enablePolling}}
+          `);
+        return wait().then(() => {
+          const status = allocation.get('clientStatus');
+          assert.notOk(find('[data-test-cpu] .inline-chart'), `No CPU chart for ${status}`);
+          assert.notOk(find('[data-test-mem] .inline-chart'), `No Mem chart for ${status}`);
+        });
+      })
+    );
+  });
+});
+
+// A way to loop over asynchronous code. Can be replaced by async/await in the future.
+const waitForEach = fns => {
+  let i = 0;
+  let done = () => {};
+
+  // This function is asynchronous and needs to return a promise
+  const pending = new Promise(resolve => {
+    done = resolve;
+  });
+
+  const step = () => {
+    // The waitForEach promise and this recursive loop are done once
+    // all functions have been called.
+    if (i >= fns.length) {
+      done();
+      return;
+    }
+    // Call the current function
+    const promise = fns[i]() || resolve(true);
+    // Increment the function position
+    i++;
+    // Wait for async behaviors to settle and repeat
+    promise.then(() => wait()).then(step);
+  };
+
+  step();
+
+  return pending;
+};

ui/tests/pages/allocations/detail.js

@@ -28,6 +28,8 @@ export default create({
     chartClass: attribute('class', '[data-test-percentage-chart] progress'),
   }),
 
+  resourceEmptyMessage: text('[data-test-resource-error-headline]'),
+
   tasks: collection('[data-test-task-row]', {
     name: text('[data-test-name]'),
     state: text('[data-test-state]'),

ui/tests/pages/allocations/task/detail.js

@@ -30,6 +30,8 @@ export default create({
     chartClass: attribute('class', '[data-test-percentage-chart] progress'),
   }),
 
+  resourceEmptyMessage: text('[data-test-resource-error-headline]'),
+
   hasAddresses: isPresent('[data-test-task-addresses]'),
   addresses: collection('[data-test-task-address]', {
     name: text('[data-test-task-address-name]'),