From 655b6fa97f8b9865117732b2f96bbc089d211ab1 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, 30 Nov 2023 09:58:50 -0600 Subject: [PATCH] backport of commit 7ab7edf9cdec6ceab92e47c1472a43d802de0486 (#19240) Co-authored-by: Phil Renaud --- .changelog/19225.txt | 3 ++ ui/app/components/job-editor/review.js | 18 ++++++++++ ui/app/models/job-plan.js | 3 ++ .../components/job-editor/review.hbs | 34 ++++++++++++------- ui/mirage/config.js | 12 ++++++- .../integration/components/job-editor-test.js | 25 ++++++++++++-- ui/tests/pages/components/job-editor.js | 9 +++-- 7 files changed, 87 insertions(+), 17 deletions(-) create mode 100644 .changelog/19225.txt create mode 100644 ui/app/components/job-editor/review.js diff --git a/.changelog/19225.txt b/.changelog/19225.txt new file mode 100644 index 000000000..852f7d956 --- /dev/null +++ b/.changelog/19225.txt @@ -0,0 +1,3 @@ +```release-note:improvement +ui: show plan output warnings alongside placement failures and dry-run info when running a job through the web ui +``` diff --git a/ui/app/components/job-editor/review.js b/ui/app/components/job-editor/review.js new file mode 100644 index 000000000..134be915f --- /dev/null +++ b/ui/app/components/job-editor/review.js @@ -0,0 +1,18 @@ +/** + * Copyright (c) HashiCorp, Inc. + * SPDX-License-Identifier: BUSL-1.1 + */ + +import Component from '@glimmer/component'; +import { htmlSafe } from '@ember/template'; + +export default class JobEditorReviewComponent extends Component { + // Slightly formats the warning string to be more readable + get warnings() { + return htmlSafe( + (this.args.data.planOutput.warnings || '') + .replace(/\n/g, '
') + .replace(/\t/g, '    ') + ); + } +} diff --git a/ui/app/models/job-plan.js b/ui/app/models/job-plan.js index 2dd946182..fdd3d478f 100644 --- a/ui/app/models/job-plan.js +++ b/ui/app/models/job-plan.js @@ -12,5 +12,8 @@ export default class JobPlan extends Model { @attr() diff; @fragmentArray('placement-failure', { defaultValue: () => [] }) failedTGAllocs; + @hasMany('allocation') preemptions; + + @attr('string') warnings; } diff --git a/ui/app/templates/components/job-editor/review.hbs b/ui/app/templates/components/job-editor/review.hbs index 9eb1243f3..4a1428d7b 100644 --- a/ui/app/templates/components/job-editor/review.hbs +++ b/ui/app/templates/components/job-editor/review.hbs @@ -13,22 +13,32 @@ /> -
-
Scheduler dry-run
-
+ + + Scheduler dry-run + {{#if @data.planOutput.failedTGAllocs}} - {{#each @data.planOutput.failedTGAllocs as |placementFailure|}} + {{#each @data.planOutput.failedTGAllocs as |placementFailure|}} - {{/each}} + {{/each}} {{else}} - All tasks successfully allocated. + All tasks successfully allocated. {{/if}} -
-
+ + +
+ +{{#if this.warnings}} + + +

+ {{this.warnings}} +

+ + +
+{{/if}} + {{#if (and @data.planOutput.preemptions.isFulfilled @data.planOutput.preemptions.length diff --git a/ui/mirage/config.js b/ui/mirage/config.js index 20f2e6729..5b55ab420 100644 --- a/ui/mirage/config.js +++ b/ui/mirage/config.js @@ -139,10 +139,16 @@ export default function () { const FailedTGAllocs = body.Job.Unschedulable && generateFailedTGAllocs(body.Job); + const jobPlanWarnings = body.Job.WithWarnings && generateWarnings(); + return new Response( 200, {}, - JSON.stringify({ FailedTGAllocs, Diff: generateDiff(req.params.id) }) + JSON.stringify({ + FailedTGAllocs, + Warnings: jobPlanWarnings, + Diff: generateDiff(req.params.id), + }) ); }); @@ -1224,3 +1230,7 @@ function generateFailedTGAllocs(job, taskGroups) { return hash; }, {}); } + +function generateWarnings() { + return '2 warnings:\n\n* Group "yourtask" has warnings: 1 error occurred:\n\t* Task "yourtask" has warnings: 1 error occurred:\n\t* 2 errors occurred:\n\t* Identity[vault_default] identities without an audience are insecure\n\t* Identity[vault_default] identities without an expiration are insecure\n* Task yourtask has an identity called vault_default but no vault block'; +} diff --git a/ui/tests/integration/components/job-editor-test.js b/ui/tests/integration/components/job-editor-test.js index 0d084e498..c63b53552 100644 --- a/ui/tests/integration/components/job-editor-test.js +++ b/ui/tests/integration/components/job-editor-test.js @@ -15,6 +15,7 @@ import jobEditor from 'nomad-ui/tests/pages/components/job-editor'; import { initialize as fragmentSerializerInitializer } from 'nomad-ui/initializers/fragment-serializer'; import setupCodeMirror from 'nomad-ui/tests/helpers/codemirror'; import { componentA11yAudit } from 'nomad-ui/tests/helpers/a11y-audit'; +import percySnapshot from '@percy/ember'; const Editor = create(jobEditor()); @@ -290,8 +291,8 @@ module('Integration | Component | job-editor', function (hooks) { await componentA11yAudit(this.element, assert); }); - test('when the scheduler dry-run has warnings, the warnings are shown to the user', async function (assert) { - assert.expect(4); + test('when the scheduler dry-run has errors, the errors are shown to the user', async function (assert) { + assert.expect(5); const spec = jsonJob({ Unschedulable: true }); const job = await this.store.createRecord('job'); @@ -312,7 +313,27 @@ module('Integration | Component | job-editor', function (hooks) { 'The scheduler dry-run message includes the warning from send back by the API' ); + assert.notOk( + Editor.warningMessage.isPresent, + 'The scheduler dry-run warning block is not present when there is an error but no warnings' + ); + await componentA11yAudit(this.element, assert); + + await percySnapshot(assert); + }); + + test('When the scheduler dry-run has warnings, the warnings are shown to the user', async function (assert) { + assert.expect(1); + const spec = jsonJob({ WithWarnings: true }); + const job = await this.store.createRecord('job'); + await renderNewJob(this, job); + await planJob(spec); + assert.ok( + Editor.warningMessage.isPresent, + 'The scheduler dry-run warning block is shown to the user' + ); + await percySnapshot(assert); }); test('when the scheduler dry-run has no warnings, a success message is shown to the user', async function (assert) { diff --git a/ui/tests/pages/components/job-editor.js b/ui/tests/pages/components/job-editor.js index b46b0c69d..3b5f54e9f 100644 --- a/ui/tests/pages/components/job-editor.js +++ b/ui/tests/pages/components/job-editor.js @@ -41,7 +41,12 @@ export default () => ({ scope: '[data-test-dry-run-message]', title: text('[data-test-dry-run-title]'), body: text('[data-test-dry-run-body]'), - errored: hasClass('is-warning'), - succeeded: hasClass('is-primary'), + errored: hasClass('hds-alert--color-critical'), + succeeded: hasClass('hds-alert--color-success'), + }, + + warningMessage: { + scope: '[data-test-dry-run-warnings]', + body: text('[data-test-dry-run-warning-body]'), }, });