From 963b2d97b2488bd930d0e332e83aa2c4399dd1c1 Mon Sep 17 00:00:00 2001 From: hc-github-team-nomad-core <82989552+hc-github-team-nomad-core@users.noreply.github.com> Date: Thu, 20 Jul 2023 11:37:06 -0500 Subject: [PATCH] Backport of [ui] When a purged/404-ing job is detected, boot the user out of that job and back to the index into release/1.6.x (#18009) This pull request was automerged via backport-assistant --- ui/app/controllers/jobs/job.js | 23 ++++++++ ui/app/routes/jobs/job.js | 19 ++++++- ui/app/routes/jobs/job/index.js | 3 - ui/app/templates/jobs/job.hbs | 1 + ui/app/utils/properties/watch.js | 16 +++++- ui/tests/acceptance/job-detail-test.js | 78 +++++++++++++++++++++++++- 6 files changed, 134 insertions(+), 6 deletions(-) diff --git a/ui/app/controllers/jobs/job.js b/ui/app/controllers/jobs/job.js index fe406e7d4..09d879ad9 100644 --- a/ui/app/controllers/jobs/job.js +++ b/ui/app/controllers/jobs/job.js @@ -3,9 +3,15 @@ * SPDX-License-Identifier: MPL-2.0 */ +// @ts-check import Controller from '@ember/controller'; +import { action } from '@ember/object'; +import { inject as service } from '@ember/service'; export default class JobController extends Controller { + @service router; + @service notifications; + @service store; queryParams = [ { jobNamespace: 'namespace', @@ -16,4 +22,21 @@ export default class JobController extends Controller { get job() { return this.model; } + + @action notFoundJobHandler() { + if ( + this.watchers.job.isError && + this.watchers.job.error?.errors?.some((e) => e.status === '404') + ) { + this.notifications.add({ + title: `Job "${this.job.name}" has been deleted`, + message: + 'The job you were looking at has been deleted; this is usually because it was purged from elsewhere.', + color: 'critical', + sticky: true, + }); + this.store.unloadRecord(this.job); + this.router.transitionTo('jobs'); + } + } } diff --git a/ui/app/routes/jobs/job.js b/ui/app/routes/jobs/job.js index 2a7e614b8..49942a6f2 100644 --- a/ui/app/routes/jobs/job.js +++ b/ui/app/routes/jobs/job.js @@ -8,13 +8,17 @@ import Route from '@ember/routing/route'; import RSVP from 'rsvp'; import notifyError from 'nomad-ui/utils/notify-error'; import classic from 'ember-classic-decorator'; +import { watchRecord } from 'nomad-ui/utils/properties/watch'; +import { collect } from '@ember/object/computed'; +import WithWatchers from 'nomad-ui/mixins/with-watchers'; @classic -export default class JobRoute extends Route { +export default class JobRoute extends Route.extend(WithWatchers) { @service can; @service store; @service token; @service router; + @service notifications; serialize(model) { return { job_name: model.get('idWithNamespace') }; @@ -57,4 +61,17 @@ export default class JobRoute extends Route { }) .catch(notifyError(this)); } + + startWatchers(controller, model) { + if (!model) { + return; + } + controller.set('watchers', { + job: this.watch.perform(model), + }); + } + + @watchRecord('job', { shouldSurfaceErrors: true }) watch; + @collect('watch') + watchers; } diff --git a/ui/app/routes/jobs/job/index.js b/ui/app/routes/jobs/job/index.js index 8282b5579..052982fa7 100644 --- a/ui/app/routes/jobs/job/index.js +++ b/ui/app/routes/jobs/job/index.js @@ -27,7 +27,6 @@ export default class IndexRoute extends Route.extend(WithWatchers) { return; } controller.set('watchers', { - model: this.watch.perform(model), summary: this.watchSummary.perform(model.get('summary')), allocations: this.watchAllocations.perform(model), evaluations: this.watchEvaluations.perform(model), @@ -59,7 +58,6 @@ export default class IndexRoute extends Route.extend(WithWatchers) { return super.setupController(...arguments); } - @watchRecord('job') watch; @watchQuery('job') watchAllJobs; @watchAll('node') watchNodes; @watchRecord('job-summary') watchSummary; @@ -68,7 +66,6 @@ export default class IndexRoute extends Route.extend(WithWatchers) { @watchRelationship('latestDeployment') watchLatestDeployment; @collect( - 'watch', 'watchAllJobs', 'watchSummary', 'watchAllocations', diff --git a/ui/app/templates/jobs/job.hbs b/ui/app/templates/jobs/job.hbs index 57710f394..613124fbb 100644 --- a/ui/app/templates/jobs/job.hbs +++ b/ui/app/templates/jobs/job.hbs @@ -3,4 +3,5 @@ SPDX-License-Identifier: MPL-2.0 ~}} +{{did-update this.notFoundJobHandler this.watchers.job.isError}} {{outlet}} \ No newline at end of file diff --git a/ui/app/utils/properties/watch.js b/ui/app/utils/properties/watch.js index 41de757b5..386395d20 100644 --- a/ui/app/utils/properties/watch.js +++ b/ui/app/utils/properties/watch.js @@ -3,6 +3,8 @@ * SPDX-License-Identifier: MPL-2.0 */ +// @ts-check + import Ember from 'ember'; import { get } from '@ember/object'; import { assert } from '@ember/debug'; @@ -15,7 +17,16 @@ import config from 'nomad-ui/config/environment'; const isEnabled = config.APP.blockingQueries !== false; -export function watchRecord(modelName) { +/** + * @typedef watchRecordOptions + * @property {boolean} [shouldSurfaceErrors=false] - If true, the task will throw errors instead of yielding them. + */ + +/** + * @param {string} modelName - The name of the model to watch. + * @param {watchRecordOptions} [options] + */ +export function watchRecord(modelName, { shouldSurfaceErrors = false } = {}) { return task(function* (id, throttle = 2000) { assert( 'To watch a record, the record adapter MUST extend Watchable', @@ -35,6 +46,9 @@ export function watchRecord(modelName) { wait(throttle), ]); } catch (e) { + if (shouldSurfaceErrors) { + throw e; + } yield e; break; } finally { diff --git a/ui/tests/acceptance/job-detail-test.js b/ui/tests/acceptance/job-detail-test.js index 4517df0e4..bff0ab535 100644 --- a/ui/tests/acceptance/job-detail-test.js +++ b/ui/tests/acceptance/job-detail-test.js @@ -5,7 +5,7 @@ /* eslint-disable ember/no-test-module-for */ /* eslint-disable qunit/require-expect */ -import { currentURL } from '@ember/test-helpers'; +import { currentURL, settled } from '@ember/test-helpers'; import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; import { setupMirage } from 'ember-cli-mirage/test-support'; @@ -15,6 +15,7 @@ import moduleForJob, { moduleForJobWithClientStatus, } from 'nomad-ui/tests/helpers/module-for-job'; import JobDetail from 'nomad-ui/tests/pages/jobs/detail'; +import percySnapshot from '@percy/ember'; moduleForJob('Acceptance | job detail (batch)', 'allocations', () => server.create('job', { @@ -615,4 +616,79 @@ module('Acceptance | job detail (with namespaces)', function (hooks) { await JobDetail.visit({ id: `${job2.id}@${secondNamespace.name}` }); assert.ok(JobDetail.incrementButton.isDisabled); }); + + test('handles when a job is remotely purged', async function (assert) { + const namespace = server.create('namespace'); + const job = server.create('job', { + namespaceId: namespace.id, + status: 'running', + type: 'service', + shallow: true, + noActiveDeployment: true, + createAllocations: true, + groupsCount: 1, + groupTaskCount: 1, + allocStatusDistribution: { + running: 1, + }, + }); + + await JobDetail.visit({ id: `${job.id}@${namespace.id}` }); + + assert.equal(currentURL(), `/jobs/${job.id}%40${namespace.id}`); + + // Simulate a 404 error on the job watcher + const controller = this.owner.lookup('controller:jobs.job'); + let jobWatcher = controller.watchers.job; + jobWatcher.isError = true; + jobWatcher.error = { errors: [{ status: '404' }] }; + await settled(); + + // User should be booted off the page + assert.equal(currentURL(), '/jobs?namespace=*'); + + // A notification should be present + assert + .dom('.flash-message.alert-critical') + .exists('A toast error message pops up.'); + + await percySnapshot(assert); + }); + + test('handles when a job is remotely purged, from a job subnav page', async function (assert) { + const namespace = server.create('namespace'); + const job = server.create('job', { + namespaceId: namespace.id, + status: 'running', + type: 'service', + shallow: true, + noActiveDeployment: true, + createAllocations: true, + groupsCount: 1, + groupTaskCount: 1, + allocStatusDistribution: { + running: 1, + }, + }); + + await JobDetail.visit({ id: `${job.id}@${namespace.id}` }); + await JobDetail.tabFor('allocations').visit(); + + assert.equal(currentURL(), `/jobs/${job.id}@${namespace.id}/allocations`); + + // Simulate a 404 error on the job watcher + const controller = this.owner.lookup('controller:jobs.job'); + let jobWatcher = controller.watchers.job; + jobWatcher.isError = true; + jobWatcher.error = { errors: [{ status: '404' }] }; + await settled(); + + // User should be booted off the page + assert.equal(currentURL(), '/jobs?namespace=*'); + + // A notification should be present + assert + .dom('.flash-message.alert-critical') + .exists('A toast error message pops up.'); + }); });