From bd9cbde85d439a7d39a843f1f4eceb43b930a47c Mon Sep 17 00:00:00 2001 From: Arun Stanislavose Date: Tue, 24 Mar 2026 00:36:17 +0530 Subject: [PATCH 01/11] Ensure cached trackers are not updated if they have been used in the render --- ui/app/services/stats-trackers-registry.js | 40 ++++++++++++++++++---- 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/ui/app/services/stats-trackers-registry.js b/ui/app/services/stats-trackers-registry.js index 3d8f6155fc7..7ae4efbc8d2 100644 --- a/ui/app/services/stats-trackers-registry.js +++ b/ui/app/services/stats-trackers-registry.js @@ -44,18 +44,46 @@ export default class StatsTrackersRegistryService extends Service { if (!resource) return; const type = resource && resource.constructor.modelName; - const key = `${type}:${resource.get('id')}`; + const resourceId = resource.get('id'); + + if (!resourceId) { + return; + } + + const key = `${type}:${resourceId}`; const Constructor = type === 'node' ? NodeStatsTracker : AllocationStatsTracker; const resourceProp = type === 'node' ? 'node' : 'allocation'; const cachedTracker = registry.get(key); if (cachedTracker) { - // It's possible for the resource on a cachedTracker to have been - // deleted. Rebind it if that's the case. - if (!exists(cachedTracker, resourceProp)) - cachedTracker.set(resourceProp, resource); - return cachedTracker; + const cachedResource = cachedTracker.get(resourceProp); + const shouldReuse = + exists(cachedTracker, resourceProp) && cachedResource === resource; + + if (shouldReuse) { + return cachedTracker; + } + + // Replace stale/mismatched trackers instead of mutating an already-used + // tracker during render. + if (cachedTracker.poll && typeof cachedTracker.poll.cancelAll === 'function') { + cachedTracker.poll.cancelAll(); + } + if ( + cachedTracker.signalPause && + typeof cachedTracker.signalPause.cancelAll === 'function' + ) { + cachedTracker.signalPause.cancelAll(); + } + + const replacementTracker = Constructor.create({ + fetch: (url) => this.token.authorizedRequest(url), + [resourceProp]: resource, + }); + + registry.set(key, replacementTracker); + return replacementTracker; } const tracker = Constructor.create({ From 7639d55e3316c74545f6ebfad3164804afc0ac7e Mon Sep 17 00:00:00 2001 From: Arun Stanislavose Date: Tue, 24 Mar 2026 00:37:58 +0530 Subject: [PATCH 02/11] Added guards before perfoming requests --- ui/app/components/primary-metric/node.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/ui/app/components/primary-metric/node.js b/ui/app/components/primary-metric/node.js index d0fc56e66da..0499fcc48f0 100644 --- a/ui/app/components/primary-metric/node.js +++ b/ui/app/components/primary-metric/node.js @@ -74,7 +74,11 @@ export default class NodePrimaryMetric extends Component { @task(function* () { do { - this.tracker.poll.perform(); + const tracker = this.tracker; + const nodeId = tracker && get(tracker, 'node.id'); + if (tracker && nodeId) { + tracker.poll.perform(); + } yield timeout(100); } while (!Ember.testing); }) @@ -88,6 +92,8 @@ export default class NodePrimaryMetric extends Component { willDestroy() { super.willDestroy(...arguments); this.poller.cancelAll(); - this.tracker.signalPause.perform(); + if (this.tracker) { + this.tracker.signalPause.perform(); + } } } From 6312bc052a0ac4764b64cf4fb6ce0005932b25ac Mon Sep 17 00:00:00 2001 From: Arun Stanislavose Date: Tue, 24 Mar 2026 00:39:09 +0530 Subject: [PATCH 03/11] Removed unused method --- ui/app/services/stats-trackers-registry.js | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/ui/app/services/stats-trackers-registry.js b/ui/app/services/stats-trackers-registry.js index 7ae4efbc8d2..671b6f888fd 100644 --- a/ui/app/services/stats-trackers-registry.js +++ b/ui/app/services/stats-trackers-registry.js @@ -95,21 +95,4 @@ export default class StatsTrackersRegistryService extends Service { return tracker; } - - // utility method to cancel all active stats polling tasks across all trackers in the registry. - cancelAll() { - registry.forEach((tracker) => { - // Cancel the poll task if it exists and is running - if (tracker.poll && typeof tracker.poll.cancelAll === 'function') { - tracker.poll.cancelAll(); - } - // Cancel the signalPause task if it exists - if ( - tracker.signalPause && - typeof tracker.signalPause.cancelAll === 'function' - ) { - tracker.signalPause.cancelAll(); - } - }); - } } From 8bc5c666eada8bd2deb7a8009cd17c041aba0648 Mon Sep 17 00:00:00 2001 From: Arun Stanislavose Date: Fri, 10 Apr 2026 23:19:50 +0530 Subject: [PATCH 04/11] Added tests for region switcher logic in jobs page --- ui/tests/acceptance/regions-test.js | 109 ++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/ui/tests/acceptance/regions-test.js b/ui/tests/acceptance/regions-test.js index 06ebdf57b0e..8115456d040 100644 --- a/ui/tests/acceptance/regions-test.js +++ b/ui/tests/acceptance/regions-test.js @@ -22,6 +22,7 @@ module('Acceptance | regions (only one)', function (hooks) { setupMirage(hooks); hooks.beforeEach(function () { + window.localStorage.clear(); server.create('agent'); server.create('node-pool'); server.create('node'); @@ -106,7 +107,11 @@ module('Acceptance | regions (many)', function (hooks) { }); test('the region switcher is rendered in the nav bar and the region is in the page title', async function (assert) { + let managementToken = server.create('token'); + window.localStorage.nomadTokenSecret = managementToken.secretId; + await JobsList.visit(); + await settled(); assert.ok( Layout.navbar.regionSwitcher.isPresent, @@ -147,6 +152,110 @@ module('Acceptance | regions (many)', function (hooks) { ); }); + test('switching regions on a query-param-only transition refreshes the active route model', async function (assert) { + const newRegion = server.db.regions[1].id; + + await JobsList.visit(); + + const jobsRequestsBeforeSwitch = server.pretender.handledRequests.filter( + (request) => request.url.includes('/v1/jobs') + ).length; + + await selectChoose('[data-test-region-switcher-parent]', newRegion); + await settled(); + + const jobsRequestsAfterSwitch = server.pretender.handledRequests.filter( + (request) => request.url.includes('/v1/jobs') + ); + + assert.ok( + jobsRequestsAfterSwitch.length > jobsRequestsBeforeSwitch, + 'Jobs model request is issued again after region query-param switch' + ); + + assert.ok( + jobsRequestsAfterSwitch + .slice(jobsRequestsBeforeSwitch) + .some((request) => request.url.includes(`region=${newRegion}`)), + 'Refreshed jobs request uses the selected region' + ); + }); + + test('switching regions on job detail reloads job, allocations, and evaluations', async function (assert) { + const newRegion = server.db.regions[1].id; + + await JobsList.visit(); + const jobId = JobsList.jobs.objectAt(0).id; + await JobsList.jobs.objectAt(0).clickRow(); + await settled(); + + const isForJobPath = (request, path) => { + const url = new URL(request.url, window.location.origin); + return url.pathname === `/v1/job/${jobId}${path}`; + }; + + const jobRequestsBeforeSwitch = server.pretender.handledRequests.filter( + (request) => isForJobPath(request, '') + ).length; + + const allocationRequestsBeforeSwitch = + server.pretender.handledRequests.filter((request) => + isForJobPath(request, '/allocations') + ).length; + + const evaluationRequestsBeforeSwitch = + server.pretender.handledRequests.filter((request) => + isForJobPath(request, '/evaluations') + ).length; + + await selectChoose('[data-test-region-switcher-parent]', newRegion); + await settled(); + + const jobRequestsAfterSwitch = server.pretender.handledRequests.filter( + (request) => isForJobPath(request, '') + ); + const allocationRequestsAfterSwitch = + server.pretender.handledRequests.filter((request) => + isForJobPath(request, '/allocations') + ); + const evaluationRequestsAfterSwitch = + server.pretender.handledRequests.filter((request) => + isForJobPath(request, '/evaluations') + ); + + assert.ok( + jobRequestsAfterSwitch.length > jobRequestsBeforeSwitch, + 'Job record is fetched again after switching regions on job detail' + ); + assert.ok( + allocationRequestsAfterSwitch.length > allocationRequestsBeforeSwitch, + 'Job allocations are fetched again after switching regions on job detail' + ); + assert.ok( + evaluationRequestsAfterSwitch.length > evaluationRequestsBeforeSwitch, + 'Job evaluations are fetched again after switching regions on job detail' + ); + + assert.ok( + jobRequestsAfterSwitch + .slice(jobRequestsBeforeSwitch) + .some((request) => request.url.includes(`region=${newRegion}`)), + 'Refetched job request includes selected region' + ); + assert.ok( + allocationRequestsAfterSwitch + .slice(allocationRequestsBeforeSwitch) + .some((request) => request.url.includes(`region=${newRegion}`)), + 'Refetched allocations request includes selected region' + ); + assert.ok( + evaluationRequestsAfterSwitch + .slice(evaluationRequestsBeforeSwitch) + .some((request) => request.url.includes(`region=${newRegion}`)), + 'Refetched evaluations request includes selected region' + ); + }); + test('switching regions to the default region, unsets the region query param', async function (assert) { let managementToken = server.create('token'); window.localStorage.nomadTokenSecret = managementToken.secretId; From fa362479f5654daa581288f90d843c01573c1e50 Mon Sep 17 00:00:00 2001 From: Arun Stanislavose Date: Tue, 14 Apr 2026 00:31:00 +0530 Subject: [PATCH 05/11] fix linter error --- ui/app/services/stats-trackers-registry.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ui/app/services/stats-trackers-registry.js b/ui/app/services/stats-trackers-registry.js index 671b6f888fd..607315d4803 100644 --- a/ui/app/services/stats-trackers-registry.js +++ b/ui/app/services/stats-trackers-registry.js @@ -67,7 +67,10 @@ export default class StatsTrackersRegistryService extends Service { // Replace stale/mismatched trackers instead of mutating an already-used // tracker during render. - if (cachedTracker.poll && typeof cachedTracker.poll.cancelAll === 'function') { + if ( + cachedTracker.poll && + typeof cachedTracker.poll.cancelAll === 'function' + ) { cachedTracker.poll.cancelAll(); } if ( From 4ea709471dd1f2a5f93dfae1016abc2d646f84f0 Mon Sep 17 00:00:00 2001 From: Arun Stanislavose Date: Wed, 15 Apr 2026 07:59:09 +0530 Subject: [PATCH 06/11] Updated to fix test for cached trackers --- .../services/stats-trackers-registry-test.js | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/ui/tests/unit/services/stats-trackers-registry-test.js b/ui/tests/unit/services/stats-trackers-registry-test.js index fe964b27c74..0007c226097 100644 --- a/ui/tests/unit/services/stats-trackers-registry-test.js +++ b/ui/tests/unit/services/stats-trackers-registry-test.js @@ -107,7 +107,7 @@ module('Unit | Service | Stats Trackers Registry', function (hooks) { ); }); - test('Registry does not depend on persistent object references', function (assert) { + test('Registry replaces cached tracker when resource reference changes for same id', function (assert) { const registry = this.subject(); const id = 'some-id'; @@ -122,10 +122,18 @@ module('Unit | Service | Stats Trackers Registry', function (hooks) { 'And the same className' ); + const tracker1 = registry.getTracker(node1); + const tracker2 = registry.getTracker(node2); + + assert.notEqual( + tracker1, + tracker2, + 'Returns a replacement tracker for a different resource reference with same id' + ); assert.equal( - registry.getTracker(node1), - registry.getTracker(node2), - 'Return the same tracker' + tracker2.get('node'), + node2, + 'The replacement tracker is bound to the latest resource reference' ); assert.equal( registry.get('registryRef').size, From 1bb5a157025eca0b59a9694b7984f32b01c93f34 Mon Sep 17 00:00:00 2001 From: Arun Stanislavose Date: Fri, 17 Apr 2026 00:18:03 +0530 Subject: [PATCH 07/11] Update test to handle change in logic to load allocations and relationships of the job explicitly --- ui/tests/acceptance/job-definition-test.js | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/ui/tests/acceptance/job-definition-test.js b/ui/tests/acceptance/job-definition-test.js index bf0281b4ae0..3dc264fbd31 100644 --- a/ui/tests/acceptance/job-definition-test.js +++ b/ui/tests/acceptance/job-definition-test.js @@ -12,7 +12,6 @@ import { setupMirage } from 'ember-cli-mirage/test-support'; import a11yAudit from 'nomad-ui/tests/helpers/a11y-audit'; import setupCodeMirror from 'nomad-ui/tests/helpers/codemirror'; import Definition from 'nomad-ui/tests/pages/jobs/job/definition'; -import { JOB_JSON } from 'nomad-ui/tests/utils/generate-raw-json-job'; let job; @@ -154,7 +153,7 @@ module('Acceptance | job definition | full specification', function (hooks) { }); test('it allows users to select between full specification and JSON definition', async function (assert) { - assert.expect(3); + assert.expect(5); const specification_response = { Format: 'hcl2', JobID: 'example', @@ -166,7 +165,7 @@ module('Acceptance | job definition | full specification', function (hooks) { Variables: '', Version: 0, }; - server.get('/job/:id', () => JOB_JSON); + server.get('/job/:id/submission', () => specification_response); await Definition.visit({ id: job.id }); @@ -184,6 +183,20 @@ module('Acceptance | job definition | full specification', function (hooks) { await click('[data-test-select-full]'); codeMirror = getCodeMirrorInstance('[data-test-editor]'); - assert.propContains(JSON.parse(codeMirror.getValue()), JOB_JSON); + const fullDefinition = JSON.parse(codeMirror.getValue()); + assert.equal( + fullDefinition.ID, + job.id, + 'Full definition shows the correct job ID' + ); + assert.equal( + fullDefinition.Name, + job.name, + 'Full definition shows the correct job name' + ); + assert.ok( + fullDefinition.TaskGroups, + 'Full definition includes task groups' + ); }); }); From 68fe29e14acf1a52e46d4e1629be9606c143b99a Mon Sep 17 00:00:00 2001 From: Arun Stanislavose Date: Mon, 20 Apr 2026 10:34:02 +0530 Subject: [PATCH 08/11] Ensure to load linked variables after job data is reloaded bypassing cache --- ui/app/routes/allocations/allocation/task.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ui/app/routes/allocations/allocation/task.js b/ui/app/routes/allocations/allocation/task.js index 501c563aa04..3cfc29b3ea0 100644 --- a/ui/app/routes/allocations/allocation/task.js +++ b/ui/app/routes/allocations/allocation/task.js @@ -30,4 +30,11 @@ export default class TaskRoute extends Route { return task; } + + async afterModel(model) { + if (model && model.task) { + // Preload the job's variables so pathLinkedVariable can work synchronously + await model.task.getPathLinkedVariable(); + } + } } From 0dc7e3db6261e64e10baf3dca45a9bdf0f94229f Mon Sep 17 00:00:00 2001 From: Arun Stanislavose Date: Mon, 20 Apr 2026 11:05:11 +0530 Subject: [PATCH 09/11] Added an id to ensure node tracker test work as expectyed --- .../integration/components/primary-metric/primary-metric.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ui/tests/integration/components/primary-metric/primary-metric.js b/ui/tests/integration/components/primary-metric/primary-metric.js index 1ca49220e77..0e6bafef45a 100644 --- a/ui/tests/integration/components/primary-metric/primary-metric.js +++ b/ui/tests/integration/components/primary-metric/primary-metric.js @@ -17,6 +17,9 @@ export function setupPrimaryMetricMocks(hooks, tasks = []) { const trackerSignalPauseSpy = (this.trackerSignalPauseSpy = sinon.spy()); const MockTracker = EmberObject.extend({ + node: computed(function () { + return { id: 'test-node-id' }; + }), poll: task(function* () { yield trackerPollSpy(); }), From 60e409d204cd5e9bff4a406147f703ef791b379f Mon Sep 17 00:00:00 2001 From: Arun Stanislavose Date: Tue, 21 Apr 2026 00:16:49 +0530 Subject: [PATCH 10/11] Move logic to load path linked variables from route to model level --- ui/app/models/task.js | 30 ++++++++++++-------- ui/app/routes/allocations/allocation/task.js | 7 ----- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/ui/app/models/task.js b/ui/app/models/task.js index 4697a35851e..6a137d76f84 100644 --- a/ui/app/models/task.js +++ b/ui/app/models/task.js @@ -61,6 +61,19 @@ export default class Task extends Fragment { @fragmentArray('volume-mount', { defaultValue: () => [] }) volumeMounts; + _pathLinkedVariableJobID() { + let jobID = this._job.plainId; + const parentID = this._job.belongsTo('parent').id() + ? JSON.parse(this._job.belongsTo('parent').id())[0] + : null; + + if (parentID) { + jobID = parentID; + } + + return jobID; + } + async _fetchParentJob() { let job = this.store.peekRecord('job', this.taskGroup.job.id); if (!job) { @@ -69,6 +82,9 @@ export default class Task extends Fragment { }); } this._job = job; + await this._job.variables; + // pathLinkedVariable is consumed by a sync template condition, so notify after async load. + this.notifyPropertyChange('pathLinkedVariable'); } get pathLinkedVariable() { @@ -76,10 +92,7 @@ export default class Task extends Fragment { this._fetchParentJob(); return null; } else { - let jobID = this._job.plainId; - if (this._job.parent.get('plainId')) { - jobID = this._job.parent.get('plainId'); - } + let jobID = this._pathLinkedVariableJobID(); return this._job.variables?.findBy( 'path', `nomad/jobs/${jobID}/${this.taskGroup.name}/${this.name}` @@ -93,14 +106,7 @@ export default class Task extends Fragment { await this._fetchParentJob(); } await this._job.variables; - let jobID = this._job.plainId; - // not getting plainID because we dont know the resolution status of the task's job's parent yet - let parentID = this._job.belongsTo('parent').id() - ? JSON.parse(this._job.belongsTo('parent').id())[0] - : null; - if (parentID) { - jobID = parentID; - } + let jobID = this._pathLinkedVariableJobID(); return await this._job.variables?.findBy( 'path', `nomad/jobs/${jobID}/${this.taskGroup.name}/${this.name}` diff --git a/ui/app/routes/allocations/allocation/task.js b/ui/app/routes/allocations/allocation/task.js index 3cfc29b3ea0..501c563aa04 100644 --- a/ui/app/routes/allocations/allocation/task.js +++ b/ui/app/routes/allocations/allocation/task.js @@ -30,11 +30,4 @@ export default class TaskRoute extends Route { return task; } - - async afterModel(model) { - if (model && model.task) { - // Preload the job's variables so pathLinkedVariable can work synchronously - await model.task.getPathLinkedVariable(); - } - } } From f1b9fc7193028ccd7090d6744904bff26d3c5464 Mon Sep 17 00:00:00 2001 From: Arun Stanislavose Date: Tue, 21 Apr 2026 00:21:14 +0530 Subject: [PATCH 11/11] Fix linter error --- ui/app/models/task.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/app/models/task.js b/ui/app/models/task.js index 6a137d76f84..d99ff727bfe 100644 --- a/ui/app/models/task.js +++ b/ui/app/models/task.js @@ -10,7 +10,7 @@ import { fragmentArray, fragmentOwner, } from 'ember-data-model-fragments/attributes'; -import { computed } from '@ember/object'; +import { computed, notifyPropertyChange } from '@ember/object'; export default class Task extends Fragment { @fragmentOwner() taskGroup; @@ -84,7 +84,7 @@ export default class Task extends Fragment { this._job = job; await this._job.variables; // pathLinkedVariable is consumed by a sync template condition, so notify after async load. - this.notifyPropertyChange('pathLinkedVariable'); + notifyPropertyChange(this, 'pathLinkedVariable'); } get pathLinkedVariable() {