diff --git a/ui/app/services/system.js b/ui/app/services/system.js index 8acc729ddfd..2e49b2b37ba 100644 --- a/ui/app/services/system.js +++ b/ui/app/services/system.js @@ -1,13 +1,9 @@ import Service, { inject as service } from '@ember/service'; import { computed } from '@ember/object'; -import { copy } from '@ember/object/internals'; import PromiseObject from '../utils/classes/promise-object'; import PromiseArray from '../utils/classes/promise-array'; import { namespace } from '../adapters/application'; - -// When the request isn't ok (e.g., forbidden) handle gracefully -const jsonWithDefault = defaultResponse => res => - res.ok ? res.json() : copy(defaultResponse, true); +import jsonWithDefault from '../utils/json-with-default'; export default Service.extend({ token: service(), diff --git a/ui/app/utils/classes/abstract-stats-tracker.js b/ui/app/utils/classes/abstract-stats-tracker.js index 08a8f03f556..68f2b5e56fd 100644 --- a/ui/app/utils/classes/abstract-stats-tracker.js +++ b/ui/app/utils/classes/abstract-stats-tracker.js @@ -1,12 +1,21 @@ +import Ember from 'ember'; import Mixin from '@ember/object/mixin'; import { assert } from '@ember/debug'; import { task, timeout } from 'ember-concurrency'; +import jsonWithDefault from 'nomad-ui/utils/json-with-default'; export default Mixin.create({ url: '', + // The max number of data points tracked. Once the max is reached, + // data points at the head of the list are removed in favor of new + // data appended at the tail bufferSize: 500, + // The number of consecutive request failures that can occur before an + // empty frame is appended + maxFrameMisses: 5, + fetch() { assert('StatsTrackers need a fetch method, which should have an interface like window.fetch'); }, @@ -23,6 +32,25 @@ export default Mixin.create({ ); }, + frameMisses: 0, + + handleResponse(frame) { + if (frame.error) { + this.incrementProperty('frameMisses'); + if (this.get('frameMisses') >= this.get('maxFrameMisses')) { + // Missing enough data consecutively is effectively a pause + this.pause(); + this.set('frameMisses', 0); + } + return; + } else { + this.set('frameMisses', 0); + + // Only append non-error frames + this.append(frame); + } + }, + // Uses EC as a form of debounce to prevent multiple // references to the same tracker from flooding the tracker, // but also avoiding the issue where different places where the @@ -36,18 +64,18 @@ export default Mixin.create({ assert('Url must be defined', url); yield this.get('fetch')(url) - .then(res => res.json()) - .then(frame => this.append(frame)); + .then(jsonWithDefault({ error: true })) + .then(frame => this.handleResponse(frame)); } catch (error) { throw new Error(error); } - yield timeout(2000); + yield timeout(Ember.testing ? 0 : 2000); }).drop(), signalPause: task(function*() { // wait 2 seconds - yield timeout(2000); + yield timeout(Ember.testing ? 0 : 2000); // if no poll called in 2 seconds, pause this.pause(); }).drop(), diff --git a/ui/app/utils/json-with-default.js b/ui/app/utils/json-with-default.js new file mode 100644 index 00000000000..d71fa93ba9b --- /dev/null +++ b/ui/app/utils/json-with-default.js @@ -0,0 +1,10 @@ +import { copy } from '@ember/object/internals'; + +// Used with fetch. +// Fetch only goes into the promise catch if there is a network error. +// This means that handling a 4xx or 5xx error is the responsibility +// of the developer. +const jsonWithDefault = defaultResponse => res => + res.ok ? res.json() : copy(defaultResponse, true); + +export default jsonWithDefault; diff --git a/ui/mirage/config.js b/ui/mirage/config.js index d0b834a4294..c98588b5ce6 100644 --- a/ui/mirage/config.js +++ b/ui/mirage/config.js @@ -307,7 +307,17 @@ export default function() { this.get('/client/fs/logs/:allocation_id', clientAllocationLog); this.get('/client/stats', function({ clientStats }, { queryParams }) { - return this.serialize(clientStats.find(queryParams.node_id)); + const seed = Math.random(); + if (seed > 0.8) { + const stats = clientStats.find(queryParams.node_id); + stats.update({ + timestamp: Date.now() * 1000000, + CPUTicksConsumed: stats.CPUTicksConsumed + (Math.random() * 20 - 10), + }); + return this.serialize(stats); + } else { + return new Response(500, {}, null); + } }); // TODO: in the future, this hack may be replaceable with dynamic host name diff --git a/ui/mirage/factories/client-stats.js b/ui/mirage/factories/client-stats.js index 6924f07f641..8d7b27edd1b 100644 --- a/ui/mirage/factories/client-stats.js +++ b/ui/mirage/factories/client-stats.js @@ -23,7 +23,7 @@ export default Factory.extend({ })), ], - CPUTicksConsumed: 1000000, + CPUTicksConsumed: faker.random.number({ min: 100, max: 1000 }), diskStats: () => [ Array(faker.random.number({ min: 1, max: 5 })) @@ -46,6 +46,6 @@ export default Factory.extend({ Used: 10000000000, }), - timestamp: 149000000000, + timestamp: Date.now() * 1000000, uptime: 193838, }); diff --git a/ui/mirage/factories/node.js b/ui/mirage/factories/node.js index e8ec62a9d91..a7c66cd3ad2 100644 --- a/ui/mirage/factories/node.js +++ b/ui/mirage/factories/node.js @@ -1,6 +1,6 @@ import { Factory, faker, trait } from 'ember-cli-mirage'; import { provide } from '../utils'; -import { DATACENTERS, HOSTS } from '../common'; +import { DATACENTERS, HOSTS, generateResources } from '../common'; import moment from 'moment'; const UUIDS = provide(100, faker.random.uuid.bind(faker.random)); @@ -65,6 +65,8 @@ export default Factory.extend({ drivers: makeDrivers, + resources: generateResources, + attributes() { // TODO add variability to these return { diff --git a/ui/tests/unit/adapters/node-test.js b/ui/tests/unit/adapters/node-test.js index 67e0e8c30d3..214864c027e 100644 --- a/ui/tests/unit/adapters/node-test.js +++ b/ui/tests/unit/adapters/node-test.js @@ -12,6 +12,8 @@ moduleForAdapter('node', 'Unit | Adapter | Node', { 'model:node-driver', 'model:node-event', 'model:evaluation', + 'model:resources', + 'model:network', 'model:job', 'serializer:application', 'serializer:node', @@ -21,6 +23,7 @@ moduleForAdapter('node', 'Unit | Adapter | Node', { 'service:watchList', 'transform:fragment', 'transform:fragment-array', + 'transform:array', ], beforeEach() { this.server = startMirage(); diff --git a/ui/tests/unit/utils/allocation-stats-tracker-test.js b/ui/tests/unit/utils/allocation-stats-tracker-test.js index cbfa181fa4f..5af885a8054 100644 --- a/ui/tests/unit/utils/allocation-stats-tracker-test.js +++ b/ui/tests/unit/utils/allocation-stats-tracker-test.js @@ -6,6 +6,7 @@ import sinon from 'sinon'; import Pretender from 'pretender'; import AllocationStatsTracker, { stats } from 'nomad-ui/utils/classes/allocation-stats-tracker'; import fetch from 'nomad-ui/utils/fetch'; +import statsTrackerFrameMissingBehavior from './behaviors/stats-tracker-frame-missing'; module('Unit | Util | AllocationStatsTracker'); @@ -453,3 +454,27 @@ test('changing the value of the allocationProp constructs a new AllocationStatsT 'Changing the value of alloc results in creating a new AllocationStatsTracker instance' ); }); + +statsTrackerFrameMissingBehavior({ + resourceName: 'allocation', + ResourceConstructor: MockAllocation, + TrackerConstructor: AllocationStatsTracker, + mockFrame, + compileResources(frame) { + const timestamp = makeDate(frame.Timestamp); + const cpu = frame.ResourceUsage.CpuStats.TotalTicks; + const memory = frame.ResourceUsage.MemoryStats.RSS; + return [ + { + timestamp, + used: cpu, + percent: cpu / 200, + }, + { + timestamp, + used: memory, + percent: memory / 1024 / 1024 / 512, + }, + ]; + }, +}); diff --git a/ui/tests/unit/utils/behaviors/stats-tracker-frame-missing.js b/ui/tests/unit/utils/behaviors/stats-tracker-frame-missing.js new file mode 100644 index 00000000000..679143848c1 --- /dev/null +++ b/ui/tests/unit/utils/behaviors/stats-tracker-frame-missing.js @@ -0,0 +1,97 @@ +import { resolve } from 'rsvp'; +import wait from 'ember-test-helpers/wait'; +import { test } from 'ember-qunit'; +import sinon from 'sinon'; + +const MockResponse = json => ({ + ok: true, + json() { + return resolve(json); + }, +}); + +export default function statsTrackerFrameMissing({ + resourceName, + TrackerConstructor, + ResourceConstructor, + mockFrame, + compileResources, +}) { + test('a bad response from a fetch request is handled gracefully', function(assert) { + const frame = mockFrame(1); + const [compiledCPU, compiledMemory] = compileResources(frame); + + let shouldFail = false; + const fetch = () => { + return resolve(shouldFail ? { ok: false } : new MockResponse(frame)); + }; + + const resource = ResourceConstructor(); + const tracker = TrackerConstructor.create({ fetch, [resourceName]: resource }); + + tracker.get('poll').perform(); + + return wait() + .then(() => { + assert.deepEqual(tracker.get('cpu'), [compiledCPU], 'One frame of cpu'); + assert.deepEqual(tracker.get('memory'), [compiledMemory], 'One frame of memory'); + + shouldFail = true; + tracker.get('poll').perform(); + return wait(); + }) + .then(() => { + assert.deepEqual(tracker.get('cpu'), [compiledCPU], 'Still one frame of cpu'); + assert.deepEqual(tracker.get('memory'), [compiledMemory], 'Still one frame of memory'); + assert.equal(tracker.get('frameMisses'), 1, 'Frame miss is tracked'); + + shouldFail = false; + tracker.get('poll').perform(); + return wait(); + }) + .then(() => { + assert.deepEqual(tracker.get('cpu'), [compiledCPU, compiledCPU], 'Still one frame of cpu'); + assert.deepEqual( + tracker.get('memory'), + [compiledMemory, compiledMemory], + 'Still one frame of memory' + ); + assert.equal(tracker.get('frameMisses'), 0, 'Frame misses is reset'); + }); + }); + + test('enough bad responses from fetch consecutively (as set by maxFrameMisses) results in a pause', function(assert) { + const fetch = () => { + return resolve({ ok: false }); + }; + + const resource = ResourceConstructor(); + const tracker = TrackerConstructor.create({ + fetch, + [resourceName]: resource, + maxFrameMisses: 3, + pause: sinon.spy(), + }); + + tracker.get('poll').perform(); + return wait() + .then(() => { + assert.equal(tracker.get('frameMisses'), 1, 'Tick misses'); + assert.notOk(tracker.pause.called, 'Pause not called yet'); + + tracker.get('poll').perform(); + return wait(); + }) + .then(() => { + assert.equal(tracker.get('frameMisses'), 2, 'Tick misses'); + assert.notOk(tracker.pause.called, 'Pause still not called yet'); + + tracker.get('poll').perform(); + return wait(); + }) + .then(() => { + assert.equal(tracker.get('frameMisses'), 0, 'Misses reset'); + assert.ok(tracker.pause.called, 'Pause called now that frameMisses == maxFrameMisses'); + }); + }); +} diff --git a/ui/tests/unit/utils/node-stats-tracker-test.js b/ui/tests/unit/utils/node-stats-tracker-test.js index 5dfbb19ff41..cfcf1426dda 100644 --- a/ui/tests/unit/utils/node-stats-tracker-test.js +++ b/ui/tests/unit/utils/node-stats-tracker-test.js @@ -6,6 +6,7 @@ import sinon from 'sinon'; import Pretender from 'pretender'; import NodeStatsTracker, { stats } from 'nomad-ui/utils/classes/node-stats-tracker'; import fetch from 'nomad-ui/utils/fetch'; +import statsTrackerFrameMissingBehavior from './behaviors/stats-tracker-frame-missing'; module('Unit | Util | NodeStatsTracker'); @@ -221,3 +222,25 @@ test('changing the value of the nodeProp constructs a new NodeStatsTracker', fun 'Changing the value of the node results in creating a new NodeStatsTracker instance' ); }); + +statsTrackerFrameMissingBehavior({ + resourceName: 'node', + ResourceConstructor: MockNode, + TrackerConstructor: NodeStatsTracker, + mockFrame, + compileResources(frame) { + const timestamp = makeDate(frame.Timestamp); + return [ + { + timestamp, + used: frame.CPUTicksConsumed, + percent: frame.CPUTicksConsumed / 2000, + }, + { + timestamp, + used: frame.Memory.Used, + percent: frame.Memory.Used / 1024 / 1024 / 4096, + }, + ]; + }, +});