From 61f4d66dc71c451cb75f6f2b526bf6254624efa4 Mon Sep 17 00:00:00 2001 From: Phil Renaud Date: Mon, 1 May 2023 15:24:21 -0400 Subject: [PATCH] [ui, deployments] Restarted and Rescheduled panel cells (#16972) * Status panel shows failed and lost, but probably dont have the condition quite right * Rescheduled and Replaced cells instead of a general failed/lost one * Tests moving to acceptance * Fixed desiredTotal and added acceptance test for restarted * moved integration test into acceptance test generally * Now that we represent Lost in the graph, have to make our unplaced testcase as Unknown * No need to declare new vars for immediately returned getters * Literal restart and resched add to the tallies, rather than 'would have but ran out of attampts' like before * Testfixes now that weve redefined what restarts and reschedules are indicated by --- .../components/job-status/failed-or-lost.hbs | 22 ++++ .../components/job-status/failed-or-lost.js | 7 + .../components/job-status/panel/deploying.hbs | 15 +++ .../components/job-status/panel/deploying.js | 28 +++- ui/app/components/job-status/panel/steady.hbs | 14 ++ ui/app/components/job-status/panel/steady.js | 26 +++- ui/app/models/allocation.js | 24 ++++ .../styles/components/job-status-panel.scss | 15 ++- ui/tests/acceptance/job-status-panel-test.js | 121 +++++++++++++++++- .../job-status/failed-or-lost-test.js | 62 +++++++++ 10 files changed, 324 insertions(+), 10 deletions(-) create mode 100644 ui/app/components/job-status/failed-or-lost.hbs create mode 100644 ui/app/components/job-status/failed-or-lost.js create mode 100644 ui/tests/integration/components/job-status/failed-or-lost-test.js diff --git a/ui/app/components/job-status/failed-or-lost.hbs b/ui/app/components/job-status/failed-or-lost.hbs new file mode 100644 index 000000000..54210c5ec --- /dev/null +++ b/ui/app/components/job-status/failed-or-lost.hbs @@ -0,0 +1,22 @@ +
+

+ {{@title}} + + + +

+ + {{@allocs.length}} + +
\ No newline at end of file diff --git a/ui/app/components/job-status/failed-or-lost.js b/ui/app/components/job-status/failed-or-lost.js new file mode 100644 index 000000000..1de1b55b2 --- /dev/null +++ b/ui/app/components/job-status/failed-or-lost.js @@ -0,0 +1,7 @@ +import Component from '@glimmer/component'; + +export default class JobStatusFailedOrLostComponent extends Component { + get shouldLinkToAllocations() { + return this.args.title !== 'Restarted' && this.args.allocs.length; + } +} diff --git a/ui/app/components/job-status/panel/deploying.hbs b/ui/app/components/job-status/panel/deploying.hbs index 27213a0a2..ade49931f 100644 --- a/ui/app/components/job-status/panel/deploying.hbs +++ b/ui/app/components/job-status/panel/deploying.hbs @@ -102,6 +102,21 @@ + + + + +
diff --git a/ui/app/components/job-status/panel/deploying.js b/ui/app/components/job-status/panel/deploying.js index 53c66f194..ea70f70e9 100644 --- a/ui/app/components/job-status/panel/deploying.js +++ b/ui/app/components/job-status/panel/deploying.js @@ -14,7 +14,7 @@ export default class JobStatusPanelDeployingComponent extends Component { 'pending', 'failed', // 'unknown', - // 'lost', + 'lost', // 'queued', // 'complete', 'unplaced', @@ -61,7 +61,7 @@ export default class JobStatusPanelDeployingComponent extends Component { fail; @alias('job.latestDeployment') deployment; - @alias('deployment.desiredTotal') desiredTotal; + @alias('totalAllocs') desiredTotal; get oldVersionAllocBlocks() { return this.job.allocations @@ -114,6 +114,14 @@ export default class JobStatusPanelDeployingComponent extends Component { : 'unhealthy'; if (allocationCategories[status]) { + // If status is failed or lost, we only want to show it IF it's used up its restarts/rescheds. + // Otherwise, we'd be showing an alloc that had been replaced. + if (alloc.willNotRestart) { + if (!alloc.willNotReschedule) { + // Dont count it + continue; + } + } allocationCategories[status][health][canary].push(alloc); availableSlotsToFill--; } @@ -144,6 +152,22 @@ export default class JobStatusPanelDeployingComponent extends Component { ]; } + get rescheduledAllocs() { + return this.job.allocations.filter( + (a) => + a.jobVersion === this.job.latestDeployment.get('versionNumber') && + a.hasBeenRescheduled + ); + } + + get restartedAllocs() { + return this.job.allocations.filter( + (a) => + a.jobVersion === this.job.latestDeployment.get('versionNumber') && + a.hasBeenRestarted + ); + } + // #region legend get newAllocsByStatus() { return Object.entries(this.newVersionAllocBlocks).reduce( diff --git a/ui/app/components/job-status/panel/steady.hbs b/ui/app/components/job-status/panel/steady.hbs index b76221ef7..4630d3929 100644 --- a/ui/app/components/job-status/panel/steady.hbs +++ b/ui/app/components/job-status/panel/steady.hbs @@ -41,6 +41,20 @@ {{/each}} + + + +

Versions

    diff --git a/ui/app/components/job-status/panel/steady.js b/ui/app/components/job-status/panel/steady.js index 77a391d2a..98537a1e8 100644 --- a/ui/app/components/job-status/panel/steady.js +++ b/ui/app/components/job-status/panel/steady.js @@ -11,7 +11,7 @@ export default class JobStatusPanelSteadyComponent extends Component { 'pending', 'failed', // 'unknown', - // 'lost', + 'lost', // 'queued', // 'complete', 'unplaced', @@ -25,10 +25,10 @@ export default class JobStatusPanelSteadyComponent extends Component { let availableSlotsToFill = this.totalAllocs; // Only fill up to 100% of totalAllocs. Once we've filled up, we can stop counting. let allocationsOfShowableType = this.allocTypes.reduce((blocks, type) => { - const jobAllocsOfType = this.args.job.allocations.filterBy( - 'clientStatus', - type.label - ); + const jobAllocsOfType = this.args.job.allocations + .sortBy('jobVersion') // Try counting from latest deployment's allocs and work backwards if needed + .reverse() + .filterBy('clientStatus', type.label); if (availableSlotsToFill > 0) { blocks[type.label] = { healthy: { @@ -84,4 +84,20 @@ export default class JobStatusPanelSteadyComponent extends Component { [] ); } + + get rescheduledAllocs() { + return this.job.allocations.filter( + (a) => + a.jobVersion === this.job.latestDeployment.get('versionNumber') && + a.hasBeenRescheduled + ); + } + + get restartedAllocs() { + return this.job.allocations.filter( + (a) => + a.jobVersion === this.job.latestDeployment.get('versionNumber') && + a.hasBeenRestarted + ); + } } diff --git a/ui/app/models/allocation.js b/ui/app/models/allocation.js index 0848308f4..172852e3d 100644 --- a/ui/app/models/allocation.js +++ b/ui/app/models/allocation.js @@ -46,6 +46,7 @@ export default class Allocation extends Model { @attr('string') clientStatus; @attr('string') desiredStatus; + @attr() desiredTransition; @attr() deploymentStatus; get isCanary() { @@ -56,6 +57,29 @@ export default class Allocation extends Model { return this.deploymentStatus?.Healthy; } + get willNotRestart() { + return this.clientStatus === 'failed' || this.clientStatus === 'lost'; + } + + get willNotReschedule() { + return ( + this.willNotRestart && + !this.get('nextAllocation.content') && + !this.get('followUpEvaluation.content') + ); + } + + get hasBeenRescheduled() { + return this.get('followUpEvaluation.content'); + } + + get hasBeenRestarted() { + return this.states + .map((s) => s.events.content) + .flat() + .find((e) => e.type === 'Restarting'); + } + @attr healthChecks; async getServiceHealth() { diff --git a/ui/app/styles/components/job-status-panel.scss b/ui/app/styles/components/job-status-panel.scss index fb939f73a..16d26c258 100644 --- a/ui/app/styles/components/job-status-panel.scss +++ b/ui/app/styles/components/job-status-panel.scss @@ -49,7 +49,12 @@ // TODO: may revisit this grid-area later, but is currently used in 2 competing ways display: grid; gap: 0.5rem; - grid-template-columns: 50% 50%; + grid-template-columns: 55% 15% 15% 15%; + + & > section > h4, + & > legend > h4 { + margin-bottom: 0.5rem; + } legend { display: grid; @@ -71,6 +76,14 @@ } } } + + .failed-or-lost { + .failed-or-lost-link { + display: block; + font-size: 1.5rem; + font-weight: bold; + } + } } // #endregion layout diff --git a/ui/tests/acceptance/job-status-panel-test.js b/ui/tests/acceptance/job-status-panel-test.js index 5153864b1..e82f127e8 100644 --- a/ui/tests/acceptance/job-status-panel-test.js +++ b/ui/tests/acceptance/job-status-panel-test.js @@ -8,6 +8,7 @@ import { find, findAll, fillIn, + settled, triggerEvent, } from '@ember/test-helpers'; @@ -276,7 +277,7 @@ module('Acceptance | job status panel', function (hooks) { running: 0.5, failed: 0.3, pending: 0.1, - lost: 0.1, + unknown: 0.1, }, groupTaskCount, shallow: true, @@ -290,7 +291,7 @@ module('Acceptance | job status panel', function (hooks) { // 25 running: 9 ungrouped, 17 grouped // 15 failed: 5 ungrouped, 10 grouped // 5 pending: 0 ungrouped, 5 grouped - // 5 lost: 0 ungrouped, 5 grouped. Represented as "Unplaced" + // 5 unknown: 0 ungrouped, 5 grouped. Represented as "Unplaced" assert .dom('.ungrouped-allocs .represented-allocation.running') @@ -449,6 +450,122 @@ module('Acceptance | job status panel', function (hooks) { ); }); + test('Restarted/Rescheduled/Failed numbers reflected correctly', async function (assert) { + this.store = this.owner.lookup('service:store'); + + let groupTaskCount = 10; + + let job = server.create('job', { + status: 'running', + datacenters: ['*'], + type: 'service', + resourceSpec: ['M: 256, C: 500'], // a single group + createAllocations: true, + allocStatusDistribution: { + running: 0.5, + failed: 0.5, + unknown: 0, + lost: 0, + }, + groupTaskCount, + activeDeployment: true, + shallow: true, + }); + + let state = server.create('task-state'); + state.events = server.schema.taskEvents.where({ taskStateId: state.id }); + server.schema.allocations.where({ jobId: job.id }).update({ + taskStateIds: [state.id], + jobVersion: 0, + }); + + await visit(`/jobs/${job.id}`); + assert.dom('.job-status-panel').exists(); + assert + .dom('.failed-or-lost') + .exists({ count: 2 }, 'Restarted and Rescheduled cells are both present'); + + let rescheduledCell = [...findAll('.failed-or-lost')][0]; + let restartedCell = [...findAll('.failed-or-lost')][1]; + + // Check that the title in each cell has the right text + assert.dom(rescheduledCell.querySelector('h4')).hasText('Rescheduled'); + assert.dom(restartedCell.querySelector('h4')).hasText('Restarted'); + + // Check that both values are zero and non-links + assert + .dom(rescheduledCell.querySelector('a')) + .doesNotExist('Rescheduled cell is not a link'); + assert + .dom(rescheduledCell.querySelector('.failed-or-lost-link')) + .hasText('0', 'Rescheduled cell has zero value'); + assert + .dom(restartedCell.querySelector('a')) + .doesNotExist('Restarted cell is not a link'); + assert + .dom(restartedCell.querySelector('.failed-or-lost-link')) + .hasText('0', 'Restarted cell has zero value'); + + // A wild event appears! Change a recent task event to type "Restarting" in a task state: + this.store + .peekAll('job') + .objectAt(0) + .get('allocations') + .objectAt(0) + .get('states') + .objectAt(0) + .get('events') + .objectAt(0) + .set('type', 'Restarting'); + + await settled(); + + assert + .dom(restartedCell.querySelector('.failed-or-lost-link')) + .hasText( + '1', + 'Restarted cell updates when a task event with type "Restarting" is added' + ); + + this.store + .peekAll('job') + .objectAt(0) + .get('allocations') + .objectAt(1) + .get('states') + .objectAt(0) + .get('events') + .objectAt(0) + .set('type', 'Restarting'); + + await settled(); + + // Trigger a reschedule! Set up a desiredTransition object with a Reschedule property on one of the allocations. + assert + .dom(restartedCell.querySelector('.failed-or-lost-link')) + .hasText( + '2', + 'Restarted cell updates when a second task event with type "Restarting" is added' + ); + + this.store + .peekAll('job') + .objectAt(0) + .get('allocations') + .objectAt(0) + .get('followUpEvaluation') + .set('content', { 'test-key': 'not-empty' }); + + await settled(); + + assert + .dom(rescheduledCell.querySelector('.failed-or-lost-link')) + .hasText('1', 'Rescheduled cell updates when desiredTransition is set'); + assert + .dom(rescheduledCell.querySelector('a')) + .exists('Rescheduled cell with a non-zero number is now a link'); + }); + module('deployment history', function () { test('Deployment history can be searched', async function (assert) { faker.seed(1); diff --git a/ui/tests/integration/components/job-status/failed-or-lost-test.js b/ui/tests/integration/components/job-status/failed-or-lost-test.js new file mode 100644 index 000000000..722302243 --- /dev/null +++ b/ui/tests/integration/components/job-status/failed-or-lost-test.js @@ -0,0 +1,62 @@ +import { module, test } from 'qunit'; +import { setupRenderingTest } from 'ember-qunit'; +import { render } from '@ember/test-helpers'; +import { hbs } from 'ember-cli-htmlbars'; +import { componentA11yAudit } from 'nomad-ui/tests/helpers/a11y-audit'; + +module('Integration | Component | job-status/failed-or-lost', function (hooks) { + setupRenderingTest(hooks); + + test('it renders', async function (assert) { + assert.expect(3); + let allocs = [ + { + id: 1, + name: 'alloc1', + }, + { + id: 2, + name: 'alloc2', + }, + ]; + + this.set('allocs', allocs); + + await render(hbs``); + + assert.dom('h4').hasText('Rescheduled'); + assert.dom('.failed-or-lost-link').hasText('2'); + + await componentA11yAudit(this.element, assert); + }); + + test('it links or does not link appropriately', async function (assert) { + let allocs = [ + { + id: 1, + name: 'alloc1', + }, + { + id: 2, + name: 'alloc2', + }, + ]; + + this.set('allocs', allocs); + + await render(hbs``); + + // Ensure it's of type a + assert.dom('.failed-or-lost-link').hasTagName('a'); + this.set('allocs', []); + assert.dom('.failed-or-lost-link').doesNotHaveTagName('a'); + }); +});