From 54aafa574d2dc4bd281b28f45dc00593c1e337da Mon Sep 17 00:00:00 2001 From: Phil Renaud Date: Wed, 26 Jun 2024 21:51:18 -0400 Subject: [PATCH] [ui] Jobs list should handle 502s and 504s gracefully (#23427) * UI handles 502s and 504s gracefully * Test and cleanup --- .changelog/23427.txt | 3 + ui/app/controllers/jobs/index.js | 77 +++ ui/app/serializers/job.js | 8 +- ui/app/styles/components/jobs-list.scss | 4 + ui/app/templates/jobs/index.hbs | 642 ++++++++++++------------ ui/tests/acceptance/jobs-list-test.js | 49 ++ 6 files changed, 467 insertions(+), 316 deletions(-) create mode 100644 .changelog/23427.txt diff --git a/.changelog/23427.txt b/.changelog/23427.txt new file mode 100644 index 00000000000..ef374cce6a1 --- /dev/null +++ b/.changelog/23427.txt @@ -0,0 +1,3 @@ +```release-note:bug +ui: fix an issue where gateway timeouts would cause the jobs list to revert to null, gives users a Pause Fetch option +``` diff --git a/ui/app/controllers/jobs/index.js b/ui/app/controllers/jobs/index.js index 733eb89ce16..4601eeb18cc 100644 --- a/ui/app/controllers/jobs/index.js +++ b/ui/app/controllers/jobs/index.js @@ -22,6 +22,7 @@ export default class JobsIndexController extends Controller { @service store; @service userSettings; @service watchList; + @service notifications; @tracked pageSize; @@ -156,12 +157,70 @@ export default class JobsIndexController extends Controller { ); } + /** + * In case the user wants to specifically stop polling for new jobs + */ + @action pauseJobFetching() { + let notification = this.notifications.queue.find( + (n) => n.title === 'Error fetching jobs' + ); + if (notification) { + notification.destroyMessage(); + } + this.watchList.jobsIndexIDsController.abort(); + this.watchList.jobsIndexDetailsController.abort(); + this.watchJobIDs.cancelAll(); + this.watchJobs.cancelAll(); + } + + @action restartJobList() { + this.showingCachedJobs = false; + let notification = this.notifications.queue.find( + (n) => n.title === 'Error fetching jobs' + ); + if (notification) { + notification.destroyMessage(); + } + this.watchList.jobsIndexIDsController.abort(); + this.watchList.jobsIndexDetailsController.abort(); + this.watchJobIDs.cancelAll(); + this.watchJobs.cancelAll(); + this.watchJobIDs.perform({}, JOB_LIST_THROTTLE); + this.watchJobs.perform(this.jobIDs, JOB_DETAILS_THROTTLE); + } + @localStorageProperty('nomadLiveUpdateJobsIndex', true) liveUpdatesEnabled; // #endregion pagination //#region querying + /** + * + * Let the user know that there was difficulty fetching jobs, but don't overload their screen with notifications. + * Set showingCachedJobs to tell the template to prompt them to extend timeouts + * @param {Error} e + */ + notifyFetchError(e) { + const firstError = e.errors[0]; + this.notifications.add({ + title: 'Error fetching jobs', + message: `The backend returned an error with status ${firstError.status} while fetching jobs`, + color: 'critical', + sticky: true, + preventDuplicates: true, + }); + // Specific check for a proxy timeout error + if ( + !this.showingCachedJobs && + (firstError.status === '502' || firstError.status === '504') + ) { + this.showingCachedJobs = true; + } + } + + @tracked showingCachedJobs = false; + jobQuery(params) { this.watchList.jobsIndexIDsController.abort(); this.watchList.jobsIndexIDsController = new AbortController(); @@ -172,9 +231,17 @@ export default class JobsIndexController extends Controller { abortController: this.watchList.jobsIndexIDsController, }, }) + .then((jobs) => { + this.showingCachedJobs = false; + return jobs; + }) .catch((e) => { if (e.name !== 'AbortError') { console.log('error fetching job ids', e); + this.notifyFetchError(e); + } + if (this.jobs.length) { + return this.jobs; } return; }); @@ -194,6 +261,10 @@ export default class JobsIndexController extends Controller { .catch((e) => { if (e.name !== 'AbortError') { console.log('error fetching job allocs', e); + this.notifyFetchError(e); + } + if (this.jobs.length) { + return this.jobs; } return; }); @@ -257,8 +328,14 @@ export default class JobsIndexController extends Controller { this.pendingJobIDs = jobIDs; this.pendingJobs = newJobs; } + if (Ember.testing) { + break; + } yield timeout(throttle); } else { + if (Ember.testing) { + break; + } // This returns undefined on page change / cursorAt change, resulting from the aborting of the old query. yield timeout(throttle); this.watchJobs.perform(this.jobIDs, throttle); diff --git a/ui/app/serializers/job.js b/ui/app/serializers/job.js index 940e558b82b..15d495a3f8d 100644 --- a/ui/app/serializers/job.js +++ b/ui/app/serializers/job.js @@ -71,7 +71,13 @@ export default class JobSerializer extends ApplicationSerializer { return super.normalize(typeHash, hash); } - normalizeQueryResponse(store, primaryModelClass, payload, id, requestType) { + normalizeQueryResponse( + store, + primaryModelClass, + payload = [], + id, + requestType + ) { // What jobs did we ask for? if (payload._requestBody?.jobs) { let requestedJobIDs = payload._requestBody.jobs; diff --git a/ui/app/styles/components/jobs-list.scss b/ui/app/styles/components/jobs-list.scss index 337aa686670..03f252cc2be 100644 --- a/ui/app/styles/components/jobs-list.scss +++ b/ui/app/styles/components/jobs-list.scss @@ -43,6 +43,10 @@ } } +#jobs-list-cache-warning { + margin-bottom: 1rem; +} + .status-cell { display: flex; gap: 0.5rem; diff --git a/ui/app/templates/jobs/index.hbs b/ui/app/templates/jobs/index.hbs index 2f9b21f7ff3..57ef6f8a533 100644 --- a/ui/app/templates/jobs/index.hbs +++ b/ui/app/templates/jobs/index.hbs @@ -5,337 +5,349 @@ {{page-title "Jobs"}}
- - + {{#if this.showingCachedJobs}} + + Error fetching jobs — shown jobs are cached + Jobs shown are cached and may be out of date. This is often due to a short timeout in proxy configurations. + {{#if this.watchJobIDs.isRunning}} + + {{/if}} + + + + {{/if}} - + + - - - {{#each this.filterFacets as |group|}} - - - {{#each group.options as |option|}} - - {{option.key}} - - {{else}} - - No {{group.label}} filters - - {{/each}} - - {{/each}} + - {{#if this.system.shouldShowNamespaces}} - - - - - - {{#each this.shownNamespaces as |option|}} - - {{option.label}} - - {{/each}} - - {{/if}} + - {{#if this.filter}} - + - {{/if}} + {{#each group.options as |option|}} + + {{option.key}} + + {{else}} + + No {{group.label}} filters + + {{/each}} + + {{/each}} - + {{#if this.system.shouldShowNamespaces}} + + + + + + {{#each this.shownNamespaces as |option|}} + + {{option.label}} + + {{/each}} + + {{/if}} - {{#if this.pendingJobIDDiff}} - {{/if}} -
- -
+ - - + {{#if this.pendingJobIDDiff}} + + {{/if}} - {{#if this.isForbidden}} - - {{else if this.jobs.length}} - - <:body as |B|> - {{!-- TODO: use --}} - - {{!-- {{#each this.tableColumns as |column|}} - {{get B.data (lowercase column.label)}} - {{/each}} --}} - - {{#if B.data.assumeGC}} - {{B.data.name}} - {{else}} - - {{B.data.name}} - {{!-- TODO: going to lose .meta with statuses endpoint! --}} - {{#if B.data.meta.structured.pack}} - - {{x-icon "box" class= "test"}} - Pack - - {{/if}} - + + + + + + + {{#if this.isForbidden}} + + {{else if this.jobs.length}} + + <:body as |B|> + {{!-- TODO: use --}} + + {{!-- {{#each this.tableColumns as |column|}} + {{get B.data (lowercase column.label)}} + {{/each}} --}} + + {{#if B.data.assumeGC}} + {{B.data.name}} + {{else}} + + {{B.data.name}} + {{!-- TODO: going to lose .meta with statuses endpoint! --}} + {{#if B.data.meta.structured.pack}} + + {{x-icon "box" class= "test"}} + Pack + {{/if}} - - {{#if this.system.shouldShowNamespaces}} - {{B.data.namespace.id}} + {{/if}} - -
+ + {{#if this.system.shouldShowNamespaces}} + {{B.data.namespace.id}} + {{/if}} + +
+ {{#if (not (eq B.data.childStatuses null))}} + {{#if B.data.childStatusBreakdown.running}} + + {{else if B.data.childStatusBreakdown.pending}} + + {{else if B.data.childStatusBreakdown.dead}} + + {{else if (not B.data.childStatuses.length)}} + + {{/if}} + {{else}} + + {{/if}} + {{#if B.data.hasPausedTask}} + + + + {{/if}} +
+ + + {{B.data.type}} + + {{#if this.system.shouldShowNodepools}} + {{B.data.nodePool}} + {{/if}} + +
+ {{#unless B.data.assumeGC}} {{#if (not (eq B.data.childStatuses null))}} - {{#if B.data.childStatusBreakdown.running}} - - {{else if B.data.childStatusBreakdown.pending}} - - {{else if B.data.childStatusBreakdown.dead}} - - {{else if (not B.data.childStatuses.length)}} - + {{#if B.data.childStatuses.length}} + + {{else}} + -- {{/if}} {{else}} - - {{/if}} - {{#if B.data.hasPausedTask}} - - - + {{/if}} -
- - - {{B.data.type}} - - {{#if this.system.shouldShowNodepools}} - {{B.data.nodePool}} - {{/if}} - -
- {{#unless B.data.assumeGC}} - {{#if (not (eq B.data.childStatuses null))}} - {{#if B.data.childStatuses.length}} - - {{else}} - -- - {{/if}} - {{else}} - - {{/if}} - {{/unless}} -
- - - - + {{/unless}} +
+ + + + -
-
- - - - - - - - - - +
+
+ + + + + + + + + + + + +
+
+ +
+
+ {{else}} + + {{#if this.filter}} + + + {{this.humanizedFilterError}} +

+ {{#if this.model.error.correction}} + Did you mean - -
-
- -
-
- {{else}} - - {{#if this.filter}} - - - {{this.humanizedFilterError}} -

- {{#if this.model.error.correction}} - Did you mean - ? - {{else if this.model.error.suggestion}} -
    - {{#each this.model.error.suggestion as |suggestion|}} -
    • - {{/each}} -
- {{else}} - {{!-- This is the "Nothing was found for your otherwise valid filter" option. Give them suggestions --}} - Did you know: you can try using filter expressions to search through your jobs. - Try ? + {{else if this.model.error.suggestion}} +
    + {{#each this.model.error.suggestion as |suggestion|}} +
  • - {{/if}} - + {{on "click" (action this.suggestFilter suggestion)}} + />
    • + {{/each}} +
+ {{else}} + {{!-- This is the "Nothing was found for your otherwise valid filter" option. Give them suggestions --}} + Did you know: you can try using filter expressions to search through your jobs. + Try + {{/if}} + - {{else}} - - - - - - {{/if}} - - {{/if}} + {{else}} + + + + + + {{/if}} + + {{/if}}
diff --git a/ui/tests/acceptance/jobs-list-test.js b/ui/tests/acceptance/jobs-list-test.js index e27f24f7f65..ae9f48266ab 100644 --- a/ui/tests/acceptance/jobs-list-test.js +++ b/ui/tests/acceptance/jobs-list-test.js @@ -250,6 +250,55 @@ module('Acceptance | jobs list', function (hooks) { assert.equal(currentURL(), '/settings/tokens'); }); + test('when a gateway timeout error occurs, appropriate options are shown', async function (assert) { + // Initial request is fine + await JobsList.visit(); + + assert.dom('#jobs-list-cache-warning').doesNotExist(); + + server.pretender.get('/v1/jobs/statuses', () => [ + 504, + { + errors: [ + { + status: '504', + }, + ], + }, + null, + ]); + const controller = this.owner.lookup('controller:jobs.index'); + let currentParams = { + per_page: 10, + }; + + await controller.watchJobIDs.perform(currentParams, 0); + // Manually set its "isRunning" attribute for testing purposes + // (existence of one of the buttons depends on blocking query running, which Ember testing doesnt really support) + controller.watchJobIDs.isRunning = true; + await settled(); + + assert.dom('#jobs-list-cache-warning').exists(); + + assert + .dom('.flash-message.alert-critical') + .exists('A toast error message pops up.'); + + await percySnapshot(assert); + + await click('[data-test-pause-fetching]'); + assert + .dom('.flash-message.alert-critical') + .doesNotExist('Error message removed when fetrching is paused'); + assert.dom('#jobs-list-cache-warning').exists('Cache warning remains'); + + server.pretender.get('/v1/jobs/statuses', () => [200, {}, null]); + await click('[data-test-restart-fetching]'); + assert + .dom('#jobs-list-cache-warning') + .doesNotExist('Cache warning removed when fetching is restarted'); + }); + function typeForJob(job) { return job.periodic ? 'periodic'