diff --git a/ui/app/components/task-log.js b/ui/app/components/task-log.js index 277023859068..2cc8617e4363 100644 --- a/ui/app/components/task-log.js +++ b/ui/app/components/task-log.js @@ -2,9 +2,11 @@ import { inject as service } from '@ember/service'; import Component from '@ember/component'; import { computed } from '@ember/object'; import { run } from '@ember/runloop'; +import RSVP from 'rsvp'; import { task } from 'ember-concurrency'; import { logger } from 'nomad-ui/utils/classes/log'; import WindowResizable from 'nomad-ui/mixins/window-resizable'; +import timeout from 'nomad-ui/utils/timeout'; export default Component.extend(WindowResizable, { token: service(), @@ -14,6 +16,15 @@ export default Component.extend(WindowResizable, { allocation: null, task: null, + // When true, request logs from the server agent + useServer: false, + + // When true, logs cannot be fetched from either the client or the server + noConnection: false, + + clientTimeout: 1000, + serverTimeout: 5000, + didReceiveAttrs() { if (this.get('allocation') && this.get('task')) { this.send('toggleStream'); @@ -37,11 +48,12 @@ export default Component.extend(WindowResizable, { mode: 'stdout', - logUrl: computed('allocation.id', 'allocation.node.httpAddr', function() { + logUrl: computed('allocation.id', 'allocation.node.httpAddr', 'useServer', function() { const address = this.get('allocation.node.httpAddr'); const allocation = this.get('allocation.id'); - return `//${address}/v1/client/fs/logs/${allocation}`; + const url = `/v1/client/fs/logs/${allocation}`; + return this.get('useServer') ? url : `//${address}${url}`; }), logParams: computed('task', 'mode', function() { @@ -51,9 +63,23 @@ export default Component.extend(WindowResizable, { }; }), - logger: logger('logUrl', 'logParams', function() { - const token = this.get('token'); - return token.authorizedRequest.bind(token); + logger: logger('logUrl', 'logParams', function logFetch() { + // If the log request can't settle in one second, the client + // must be unavailable and the server should be used instead + const timing = this.get('useServer') ? this.get('serverTimeout') : this.get('clientTimeout'); + return url => + RSVP.race([this.get('token').authorizedRequest(url), timeout(timing)]).then( + response => response, + error => { + if (this.get('useServer')) { + this.set('noConnection', true); + } else { + this.send('failoverToServer'); + this.get('stream').perform(); + } + throw error; + } + ); }), head: task(function*() { @@ -100,5 +126,8 @@ export default Component.extend(WindowResizable, { this.get('stream').perform(); } }, + failoverToServer() { + this.set('useServer', true); + }, }, }); diff --git a/ui/app/models/allocation.js b/ui/app/models/allocation.js index 617c1d3f7442..fe6cd2d62816 100644 --- a/ui/app/models/allocation.js +++ b/ui/app/models/allocation.js @@ -7,7 +7,6 @@ import attr from 'ember-data/attr'; import { belongsTo } from 'ember-data/relationships'; import { fragment, fragmentArray } from 'ember-data-model-fragments/attributes'; import PromiseObject from '../utils/classes/promise-object'; -import timeout from '../utils/timeout'; import shortUUIDProperty from '../utils/properties/short-uuid'; const STATUS_ORDER = { @@ -92,14 +91,11 @@ export default Model.extend({ }); } - const url = `//${this.get('node.httpAddr')}/v1/client/allocation/${this.get('id')}/stats`; + const url = `/v1/client/allocation/${this.get('id')}/stats`; return PromiseObject.create({ - promise: RSVP.Promise.race([ - this.get('token') - .authorizedRequest(url) - .then(res => res.json()), - timeout(2000), - ]), + promise: this.get('token') + .authorizedRequest(url) + .then(res => res.json()), }); }), diff --git a/ui/app/templates/components/task-log.hbs b/ui/app/templates/components/task-log.hbs index ce0d0ae9c870..6c3991a359c2 100644 --- a/ui/app/templates/components/task-log.hbs +++ b/ui/app/templates/components/task-log.hbs @@ -1,3 +1,9 @@ +{{#if noConnection}} +
+

Cannot fetch logs

+

The logs for this task are inaccessible. Check the condition of the node the allocation is on.

+
+{{/if}}
diff --git a/ui/app/utils/classes/log.js b/ui/app/utils/classes/log.js index 5e873182a97d..7791aef49a03 100644 --- a/ui/app/utils/classes/log.js +++ b/ui/app/utils/classes/log.js @@ -1,3 +1,4 @@ +import Ember from 'ember'; import { alias } from '@ember/object/computed'; import { assert } from '@ember/debug'; import Evented from '@ember/object/evented'; @@ -10,6 +11,8 @@ import PollLogger from 'nomad-ui/utils/classes/poll-logger'; const MAX_OUTPUT_LENGTH = 50000; +export const fetchFailure = url => () => Ember.Logger.warn(`LOG FETCH: Couldn't connect to ${url}`); + const Log = EmberObject.extend(Evented, { // Parameters @@ -74,9 +77,9 @@ const Log = EmberObject.extend(Evented, { const url = `${this.get('url')}?${queryParams}`; this.stop(); - let text = yield logFetch(url).then(res => res.text()); + let text = yield logFetch(url).then(res => res.text(), fetchFailure(url)); - if (text.length > MAX_OUTPUT_LENGTH) { + if (text && text.length > MAX_OUTPUT_LENGTH) { text = text.substr(0, MAX_OUTPUT_LENGTH); text += '\n\n---------- TRUNCATED: Click "tail" to view the bottom of the log ----------'; } @@ -96,7 +99,7 @@ const Log = EmberObject.extend(Evented, { const url = `${this.get('url')}?${queryParams}`; this.stop(); - let text = yield logFetch(url).then(res => res.text()); + let text = yield logFetch(url).then(res => res.text(), fetchFailure(url)); this.set('tail', text); this.set('logPointer', 'tail'); diff --git a/ui/app/utils/classes/poll-logger.js b/ui/app/utils/classes/poll-logger.js index e6c7078a7786..4bc73b14d64b 100644 --- a/ui/app/utils/classes/poll-logger.js +++ b/ui/app/utils/classes/poll-logger.js @@ -1,6 +1,7 @@ import EmberObject from '@ember/object'; import { task, timeout } from 'ember-concurrency'; import AbstractLogger from './abstract-logger'; +import { fetchFailure } from './log'; export default EmberObject.extend(AbstractLogger, { interval: 1000, @@ -18,7 +19,14 @@ export default EmberObject.extend(AbstractLogger, { poll: task(function*() { const { interval, logFetch } = this.getProperties('interval', 'logFetch'); while (true) { - let text = yield logFetch(this.get('fullUrl')).then(res => res.text()); + const url = this.get('fullUrl'); + let response = yield logFetch(url).then(res => res, fetchFailure(url)); + + if (!response) { + return; + } + + let text = yield response.text(); if (text) { const lines = text.replace(/\}\{/g, '}\n{').split('\n'); diff --git a/ui/app/utils/classes/stream-logger.js b/ui/app/utils/classes/stream-logger.js index ee1018734cb4..da4acc751dac 100644 --- a/ui/app/utils/classes/stream-logger.js +++ b/ui/app/utils/classes/stream-logger.js @@ -2,6 +2,7 @@ import EmberObject, { computed } from '@ember/object'; import { task } from 'ember-concurrency'; import TextDecoder from 'nomad-ui/utils/classes/text-decoder'; import AbstractLogger from './abstract-logger'; +import { fetchFailure } from './log'; export default EmberObject.extend(AbstractLogger, { reader: null, @@ -30,7 +31,11 @@ export default EmberObject.extend(AbstractLogger, { let buffer = ''; const decoder = new TextDecoder(); - const reader = yield logFetch(url).then(res => res.body.getReader()); + const reader = yield logFetch(url).then(res => res.body.getReader(), fetchFailure(url)); + + if (!reader) { + return; + } this.set('reader', reader); diff --git a/ui/mirage/config.js b/ui/mirage/config.js index c17c7d3e5096..b760fb673084 100644 --- a/ui/mirage/config.js +++ b/ui/mirage/config.js @@ -178,37 +178,42 @@ export default function() { return new Response(403, {}, null); }); + const clientAllocationStatsHandler = function({ clientAllocationStats }, { params }) { + return this.serialize(clientAllocationStats.find(params.id)); + }; + + const clientAllocationLog = function(server, { params, queryParams }) { + const allocation = server.allocations.find(params.allocation_id); + const tasks = allocation.taskStateIds.map(id => server.taskStates.find(id)); + + if (!tasks.mapBy('name').includes(queryParams.task)) { + return new Response(400, {}, 'must include task name'); + } + + if (queryParams.plain) { + return logFrames.join(''); + } + + return logEncode(logFrames, logFrames.length - 1); + }; + + // Client requests are available on the server and the client + this.get('/client/allocation/:id/stats', clientAllocationStatsHandler); + this.get('/client/fs/logs/:allocation_id', clientAllocationLog); + + this.get('/client/v1/client/stats', function({ clientStats }, { queryParams }) { + return this.serialize(clientStats.find(queryParams.node_id)); + }); + // TODO: in the future, this hack may be replaceable with dynamic host name // support in pretender: https://github.com/pretenderjs/pretender/issues/210 HOSTS.forEach(host => { - this.get(`http://${host}/v1/client/allocation/:id/stats`, function( - { clientAllocationStats }, - { params } - ) { - return this.serialize(clientAllocationStats.find(params.id)); - }); + this.get(`http://${host}/v1/client/allocation/:id/stats`, clientAllocationStatsHandler); + this.get(`http://${host}/v1/client/fs/logs/:allocation_id`, clientAllocationLog); this.get(`http://${host}/v1/client/stats`, function({ clientStats }) { return this.serialize(clientStats.find(host)); }); - - this.get(`http://${host}/v1/client/fs/logs/:allocation_id`, function( - server, - { params, queryParams } - ) { - const allocation = server.allocations.find(params.allocation_id); - const tasks = allocation.taskStateIds.map(id => server.taskStates.find(id)); - - if (!tasks.mapBy('name').includes(queryParams.task)) { - return new Response(400, {}, 'must include task name'); - } - - if (queryParams.plain) { - return logFrames.join(''); - } - - return logEncode(logFrames, logFrames.length - 1); - }); }); } diff --git a/ui/tests/acceptance/task-group-detail-test.js b/ui/tests/acceptance/task-group-detail-test.js index c9665b96617b..d6b2935730cc 100644 --- a/ui/tests/acceptance/task-group-detail-test.js +++ b/ui/tests/acceptance/task-group-detail-test.js @@ -101,9 +101,7 @@ test('/jobs/:id/:task-group first breadcrumb should link to jobs', function(asse }); }); -test('/jobs/:id/:task-group second breadcrumb should link to the job for the task group', function( - assert -) { +test('/jobs/:id/:task-group second breadcrumb should link to the job for the task group', function(assert) { click(`[data-test-breadcrumb="${job.name}"]`); andThen(() => { assert.equal( @@ -114,9 +112,7 @@ test('/jobs/:id/:task-group second breadcrumb should link to the job for the tas }); }); -test('/jobs/:id/:task-group should list one page of allocations for the task group', function( - assert -) { +test('/jobs/:id/:task-group should list one page of allocations for the task group', function(assert) { const pageSize = 10; server.createList('allocation', 10, { @@ -185,9 +181,7 @@ test('each allocation should show basic information about the allocation', funct }); }); -test('each allocation should show stats about the allocation, retrieved directly from the node', function( - assert -) { +test('each allocation should show stats about the allocation', function(assert) { const allocation = allocations.sortBy('name')[0]; const allocationRow = find('[data-test-allocation]'); const allocStats = server.db.clientAllocationStats.find(allocation.id); @@ -219,14 +213,6 @@ test('each allocation should show stats about the allocation, retrieved directly `${formatBytes([allocStats.resourceUsage.MemoryStats.RSS])} / ${memoryUsed} MiB`, 'Detailed memory information is in a tooltip' ); - - const node = server.db.nodes.find(allocation.nodeId); - const nodeStatsUrl = `//${node.httpAddr}/v1/client/allocation/${allocation.id}/stats`; - - assert.ok( - server.pretender.handledRequests.some(req => req.url === nodeStatsUrl), - `Requests ${nodeStatsUrl}` - ); }); test('when the allocation search has no matches, there is an empty message', function(assert) { diff --git a/ui/tests/integration/task-log-test.js b/ui/tests/integration/task-log-test.js index 62f849ae6b0c..f6a7416fdca0 100644 --- a/ui/tests/integration/task-log-test.js +++ b/ui/tests/integration/task-log-test.js @@ -7,6 +7,7 @@ import Pretender from 'pretender'; import { logEncode } from '../../mirage/data/logs'; const HOST = '1.1.1.1:1111'; +const allowedConnectionTime = 100; const commonProps = { interval: 50, allocation: { @@ -16,6 +17,8 @@ const commonProps = { }, }, task: 'task-name', + clientTimeout: allowedConnectionTime, + serverTimeout: allowedConnectionTime, }; const logHead = ['HEAD']; @@ -26,30 +29,33 @@ let streamPointer = 0; moduleForComponent('task-log', 'Integration | Component | task log', { integration: true, beforeEach() { + const handler = ({ queryParams }) => { + const { origin, offset, plain, follow } = queryParams; + + let frames; + let data; + + if (origin === 'start' && offset === '0' && plain && !follow) { + frames = logHead; + } else if (origin === 'end' && plain && !follow) { + frames = logTail; + } else { + frames = streamFrames; + } + + if (frames === streamFrames) { + data = queryParams.plain ? frames[streamPointer] : logEncode(frames, streamPointer); + streamPointer++; + } else { + data = queryParams.plain ? frames.join('') : logEncode(frames, frames.length - 1); + } + + return [200, {}, data]; + }; + this.server = new Pretender(function() { - this.get(`http://${HOST}/v1/client/fs/logs/:allocation_id`, ({ queryParams }) => { - const { origin, offset, plain, follow } = queryParams; - - let frames; - let data; - - if (origin === 'start' && offset === '0' && plain && !follow) { - frames = logHead; - } else if (origin === 'end' && plain && !follow) { - frames = logTail; - } else { - frames = streamFrames; - } - - if (frames === streamFrames) { - data = queryParams.plain ? frames[streamPointer] : logEncode(frames, streamPointer); - streamPointer++; - } else { - data = queryParams.plain ? frames.join('') : logEncode(frames, frames.length - 1); - } - - return [200, {}, data]; - }); + this.get(`http://${HOST}/v1/client/fs/logs/:allocation_id`, handler); + this.get('/v1/client/fs/logs/:allocation_id', handler); }); }, afterEach() { @@ -174,3 +180,76 @@ test('Clicking stderr switches the log to standard error', function(assert) { ); }); }); + +test('When the client is inaccessible, task-log falls back to requesting logs through the server', function(assert) { + run.later(run, run.cancelTimers, allowedConnectionTime * 2); + + // override client response to timeout + this.server.get( + `http://${HOST}/v1/client/fs/logs/:allocation_id`, + () => [400, {}, ''], + allowedConnectionTime * 2 + ); + + this.setProperties(commonProps); + this.render( + hbs`{{task-log + allocation=allocation + task=task + clientTimeout=clientTimeout + serverTimeout=serverTimeout}}` + ); + + const clientUrlRegex = new RegExp(`${HOST}/v1/client/fs/logs/${commonProps.allocation.id}`); + assert.ok( + this.server.handledRequests.filter(req => clientUrlRegex.test(req.url)).length, + 'Log request was initially made directly to the client' + ); + + return wait().then(() => { + const serverUrl = `/v1/client/fs/logs/${commonProps.allocation.id}`; + assert.ok( + this.server.handledRequests.filter(req => req.url.startsWith(serverUrl)).length, + 'Log request was later made to the server' + ); + }); +}); + +test('When both the client and the server are inaccessible, an error message is shown', function(assert) { + run.later(run, run.cancelTimers, allowedConnectionTime * 5); + + // override client and server responses to timeout + this.server.get( + `http://${HOST}/v1/client/fs/logs/:allocation_id`, + () => [400, {}, ''], + allowedConnectionTime * 2 + ); + this.server.get( + '/v1/client/fs/logs/:allocation_id', + () => [400, {}, ''], + allowedConnectionTime * 2 + ); + + this.setProperties(commonProps); + this.render( + hbs`{{task-log + allocation=allocation + task=task + clientTimeout=clientTimeout + serverTimeout=serverTimeout}}` + ); + + return wait().then(() => { + const clientUrlRegex = new RegExp(`${HOST}/v1/client/fs/logs/${commonProps.allocation.id}`); + assert.ok( + this.server.handledRequests.filter(req => clientUrlRegex.test(req.url)).length, + 'Log request was initially made directly to the client' + ); + const serverUrl = `/v1/client/fs/logs/${commonProps.allocation.id}`; + assert.ok( + this.server.handledRequests.filter(req => req.url.startsWith(serverUrl)).length, + 'Log request was later made to the server' + ); + assert.ok(find('[data-test-connection-error]'), 'An error message is shown'); + }); +});