From 38495ac3a67f80c61edb3644b79717e6a800d71d Mon Sep 17 00:00:00 2001 From: Jai Bhagat Date: Tue, 30 Aug 2022 12:50:58 -0400 Subject: [PATCH 01/12] refact: update service-fragment model to include owner info --- ui/app/models/service-fragment.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ui/app/models/service-fragment.js b/ui/app/models/service-fragment.js index 85141579473f..98eb54a6b8aa 100644 --- a/ui/app/models/service-fragment.js +++ b/ui/app/models/service-fragment.js @@ -9,4 +9,12 @@ export default class Service extends Fragment { @attr('string') onUpdate; @attr('string') provider; @fragment('consul-connect') connect; + + get fragOwner() { + return this._internalModel._recordData._owner; + } + + get isTaskLevel() { + return this.fragOwner._internalModel.modelName === 'task'; + } } From 7fcaac71912d807fee3dafe2ed8e2c6900ba9c34 Mon Sep 17 00:00:00 2001 From: Jai Bhagat Date: Tue, 30 Aug 2022 12:51:38 -0400 Subject: [PATCH 02/12] ui: differentiate between task and group-level in derived state comp --- ui/app/controllers/allocations/allocation/index.js | 5 ++++- ui/app/templates/allocations/allocation/index.hbs | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/ui/app/controllers/allocations/allocation/index.js b/ui/app/controllers/allocations/allocation/index.js index 697f4acdc231..1d5cb58352c5 100644 --- a/ui/app/controllers/allocations/allocation/index.js +++ b/ui/app/controllers/allocations/allocation/index.js @@ -73,8 +73,11 @@ export default class IndexController extends Controller.extend(Sortable) { let result = new Map(); Object.values(this.model.healthChecks)?.forEach((service) => { + const isTask = !!service.Task; const currentServiceStatus = service.Status; - const currentServiceName = service.Service; + const currentServiceName = isTask + ? service.Service.concat('@task') + : service.Service; const serviceStatuses = result.get(currentServiceName); if (serviceStatuses) { if (serviceStatuses[currentServiceStatus]) { diff --git a/ui/app/templates/allocations/allocation/index.hbs b/ui/app/templates/allocations/allocation/index.hbs index de14b6a1a857..98dd2575c400 100644 --- a/ui/app/templates/allocations/allocation/index.hbs +++ b/ui/app/templates/allocations/allocation/index.hbs @@ -307,7 +307,11 @@ {{#if (eq row.model.provider "nomad")}}
- + {{#if row.model.isTaskLevel}} + + {{else}} + + {{/if}}
{{/if}} From de393484a4ed52e3c4e4bd7fb61f346baeb68bb3 Mon Sep 17 00:00:00 2001 From: Jai Bhagat Date: Tue, 30 Aug 2022 12:51:59 -0400 Subject: [PATCH 03/12] test: add test to document behavior --- .../allocations/allocation/index-test.js | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/ui/tests/unit/controllers/allocations/allocation/index-test.js b/ui/tests/unit/controllers/allocations/allocation/index-test.js index 0587042ede6f..3b0c675ccf24 100644 --- a/ui/tests/unit/controllers/allocations/allocation/index-test.js +++ b/ui/tests/unit/controllers/allocations/allocation/index-test.js @@ -45,6 +45,56 @@ module('Unit | Controller | allocations/allocation/index', function (hooks) { 'Service Health Check data is transformed and grouped by Service name' ); }); + + test('it handles duplicate names', function (assert) { + let controller = this.owner.lookup( + 'controller:allocations/allocation/index' + ); + + const dupeTaskLevelService = + Allocation.allocationTaskGroup.Tasks[0].Services[0]; + dupeTaskLevelService.Name = 'fake-py@task'; + dupeTaskLevelService.isTaskLevel = true; + + const healthChecks = Allocation.healthChecks; + healthChecks['73ad9b936fb3f3cc4d7f62a1aab6de53'].Service = 'fake-py'; + healthChecks['19421ef816ae0d3eeeb81697bce0e261'].Service = 'fake-py'; + + controller.set('model', Allocation); + + const result = new Map(); + result.set('fake-py', { + failure: 1, + success: 1, + }); + result.set('fake-py@task', { + failure: 1, + success: 1, + }); + result.set('web@task', { + success: 1, + }); + + const fakePy = controller.serviceHealthStatuses.get('fake-py'); + const taskFakePy = controller.serviceHealthStatuses.get('fake-py@task'); + const web = controller.serviceHealthStatuses.get('web@task'); + + assert.deepEqual( + fakePy, + result.get('fake-py'), + 'Service Health Check data is transformed and grouped by Service name' + ); + assert.deepEqual( + taskFakePy, + result.get('fake-py@task'), + 'Service Health Check data is transformed and grouped by Service name' + ); + assert.deepEqual( + web, + result.get('web@task'), + 'Service Health Check data is transformed and grouped by Service name' + ); + }); }); }); From 308d6d9382ed4b954e1edcee6bdfbeb9063bca8a Mon Sep 17 00:00:00 2001 From: Jai Bhagat Date: Tue, 30 Aug 2022 12:53:01 -0400 Subject: [PATCH 04/12] refact: update tests for api change --- .../allocations/allocation/index-test.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/ui/tests/unit/controllers/allocations/allocation/index-test.js b/ui/tests/unit/controllers/allocations/allocation/index-test.js index 3b0c675ccf24..d39ef76bb618 100644 --- a/ui/tests/unit/controllers/allocations/allocation/index-test.js +++ b/ui/tests/unit/controllers/allocations/allocation/index-test.js @@ -17,17 +17,18 @@ module('Unit | Controller | allocations/allocation/index', function (hooks) { failure: 1, success: 1, }); - result.set('task-fake-py', { + result.set('task-fake-py@task', { failure: 1, success: 1, }); - result.set('web', { + result.set('web@task', { success: 1, }); const fakePy = controller.serviceHealthStatuses.get('fake-py'); - const taskFakePy = controller.serviceHealthStatuses.get('task-fake-py'); - const web = controller.serviceHealthStatuses.get('web'); + const taskFakePy = + controller.serviceHealthStatuses.get('task-fake-py@task'); + const web = controller.serviceHealthStatuses.get('web@task'); assert.deepEqual( fakePy, @@ -36,12 +37,12 @@ module('Unit | Controller | allocations/allocation/index', function (hooks) { ); assert.deepEqual( taskFakePy, - result.get('task-fake-py'), + result.get('task-fake-py@task'), 'Service Health Check data is transformed and grouped by Service name' ); assert.deepEqual( web, - result.get('web'), + result.get('web@task'), 'Service Health Check data is transformed and grouped by Service name' ); }); From ffc1ac03d23d72b186c669e6c2c383613331eacd Mon Sep 17 00:00:00 2001 From: Jai Bhagat Date: Tue, 30 Aug 2022 14:14:24 -0400 Subject: [PATCH 05/12] refact: update integration test for API change --- .../components/service-status-bar-test.js | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/ui/tests/integration/components/service-status-bar-test.js b/ui/tests/integration/components/service-status-bar-test.js index ac737181875c..ea2df2517b56 100644 --- a/ui/tests/integration/components/service-status-bar-test.js +++ b/ui/tests/integration/components/service-status-bar-test.js @@ -14,28 +14,30 @@ module('Integration | Component | Service Status Bar', function (hooks) { const component = this; await componentA11yAudit(component, assert); - const healthyService = EmberObject.create({ - id: '1', - status: 'success', - }); - - const failingService = EmberObject.create({ - id: '2', - status: 'failing', - }); - - const pendingService = EmberObject.create({ - id: '3', - status: 'pending', - }); - - const services = A([healthyService, failingService, pendingService]); + const healthyService = { + success: 1, + }; + + const failingService = { + failure: 1, + }; + + const pendingService = { + pending: 1, + }; + + const services = new Map(); + services.set('peter', healthyService); + services.set('peter', { ...services.get('peter'), ...failingService }); + services.set('peter', { ...services.get('peter'), ...pendingService }); + this.set('services', services); await render(hbs`
`); From 0c141c6ed452fd9954f50e663c88aeac7c920b0a Mon Sep 17 00:00:00 2001 From: Jai Bhagat Date: Tue, 30 Aug 2022 20:13:17 -0400 Subject: [PATCH 06/12] chore: remove unsused vars --- ui/tests/integration/components/service-status-bar-test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/ui/tests/integration/components/service-status-bar-test.js b/ui/tests/integration/components/service-status-bar-test.js index ea2df2517b56..c773316a02c7 100644 --- a/ui/tests/integration/components/service-status-bar-test.js +++ b/ui/tests/integration/components/service-status-bar-test.js @@ -1,5 +1,3 @@ -import { A } from '@ember/array'; -import EmberObject from '@ember/object'; import { findAll, render } from '@ember/test-helpers'; import { setupRenderingTest } from 'ember-qunit'; import hbs from 'htmlbars-inline-precompile'; From 640b7b25f8734e144d1a15423375fb3d8be8b78d Mon Sep 17 00:00:00 2001 From: Jai Bhagat Date: Tue, 30 Aug 2022 20:13:38 -0400 Subject: [PATCH 07/12] chore: elvis operator to protect mirage --- ui/app/routes/allocations/allocation.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ui/app/routes/allocations/allocation.js b/ui/app/routes/allocations/allocation.js index 605a76df9f43..2700a6fc6c00 100644 --- a/ui/app/routes/allocations/allocation.js +++ b/ui/app/routes/allocations/allocation.js @@ -16,11 +16,11 @@ export default class AllocationRoute extends Route.extend(WithWatchers) { // Conditionally Long Poll /checks endpoint if alloc has nomad services const doesAllocHaveServices = - !!model.taskGroup.services.filterBy('provider', 'nomad').length || + !!model.taskGroup?.services?.filterBy('provider', 'nomad').length || !!model.states - .mapBy('task') - .map((t) => t && t.get('services'))[0] - .filterBy('provider', 'nomad').length; + ?.mapBy('task') + ?.map((t) => t && t.get('services'))[0] + ?.filterBy('provider', 'nomad').length; if (doesAllocHaveServices) { controller.set( From 08ea6139ac8a79d0db8aebe4d7ffa5d233070e8c Mon Sep 17 00:00:00 2001 From: Jai Bhagat Date: Tue, 30 Aug 2022 21:09:51 -0400 Subject: [PATCH 08/12] refact: create refId instead of internalModel --- ui/app/models/service-fragment.js | 10 ++++------ ui/app/serializers/task-group.js | 6 ++++++ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/ui/app/models/service-fragment.js b/ui/app/models/service-fragment.js index 98eb54a6b8aa..319ca8f42cb0 100644 --- a/ui/app/models/service-fragment.js +++ b/ui/app/models/service-fragment.js @@ -9,12 +9,10 @@ export default class Service extends Fragment { @attr('string') onUpdate; @attr('string') provider; @fragment('consul-connect') connect; + @attr() groupName; + @attr() taskName; - get fragOwner() { - return this._internalModel._recordData._owner; - } - - get isTaskLevel() { - return this.fragOwner._internalModel.modelName === 'task'; + get refID() { + return `${this.groupName || this.taskName}-${this.name}`; } } diff --git a/ui/app/serializers/task-group.js b/ui/app/serializers/task-group.js index b8b66d9deddb..82f792952417 100644 --- a/ui/app/serializers/task-group.js +++ b/ui/app/serializers/task-group.js @@ -8,6 +8,12 @@ export default class TaskGroup extends ApplicationSerializer { mapToArray = ['Volumes']; normalize(typeHash, hash) { + if (hash.Services) { + hash.Services.forEach((service) => { + service.GroupName = hash.Name; + }); + } + // Provide EphemeralDisk to each task hash.Tasks.forEach((task) => { task.EphemeralDisk = copy(hash.EphemeralDisk); From bb4a417a4cb9d5305d6d47cacf51caed729fbd78 Mon Sep 17 00:00:00 2001 From: Jai Bhagat Date: Tue, 30 Aug 2022 21:10:21 -0400 Subject: [PATCH 09/12] refact: update algo --- ui/app/controllers/allocations/allocation/index.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ui/app/controllers/allocations/allocation/index.js b/ui/app/controllers/allocations/allocation/index.js index 1d5cb58352c5..626322ef0752 100644 --- a/ui/app/controllers/allocations/allocation/index.js +++ b/ui/app/controllers/allocations/allocation/index.js @@ -74,10 +74,12 @@ export default class IndexController extends Controller.extend(Sortable) { let result = new Map(); Object.values(this.model.healthChecks)?.forEach((service) => { const isTask = !!service.Task; + const groupName = service.Group.split('.')[1].split('[')[0]; const currentServiceStatus = service.Status; + const currentServiceName = isTask - ? service.Service.concat('@task') - : service.Service; + ? service.Task.concat(`-${service.Service}`) + : groupName.concat(`-${service.Service}`); const serviceStatuses = result.get(currentServiceName); if (serviceStatuses) { if (serviceStatuses[currentServiceStatus]) { From cdc73f0337f53f2fe2b9046142fd528cc2abf4f6 Mon Sep 17 00:00:00 2001 From: Jai Bhagat Date: Tue, 30 Aug 2022 21:10:40 -0400 Subject: [PATCH 10/12] refact: update conditional template logic --- ui/app/templates/allocations/allocation/index.hbs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ui/app/templates/allocations/allocation/index.hbs b/ui/app/templates/allocations/allocation/index.hbs index 98dd2575c400..3f8c069b900a 100644 --- a/ui/app/templates/allocations/allocation/index.hbs +++ b/ui/app/templates/allocations/allocation/index.hbs @@ -307,10 +307,10 @@ {{#if (eq row.model.provider "nomad")}}
- {{#if row.model.isTaskLevel}} - + {{#if (not row.model.groupName)}} + {{else}} - + {{/if}}
{{/if}} From 534a4d889f52aa55b08285cbfe00823643985653 Mon Sep 17 00:00:00 2001 From: Jai Bhagat Date: Tue, 30 Aug 2022 21:10:58 -0400 Subject: [PATCH 11/12] refact: update test for api change: --- .../allocations/allocation/index-test.js | 43 ++++++++++--------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/ui/tests/unit/controllers/allocations/allocation/index-test.js b/ui/tests/unit/controllers/allocations/allocation/index-test.js index d39ef76bb618..ae45d5f7baaf 100644 --- a/ui/tests/unit/controllers/allocations/allocation/index-test.js +++ b/ui/tests/unit/controllers/allocations/allocation/index-test.js @@ -13,36 +13,37 @@ module('Unit | Controller | allocations/allocation/index', function (hooks) { controller.set('model', Allocation); const result = new Map(); - result.set('fake-py', { + result.set('fakepy-fake-py', { failure: 1, success: 1, }); - result.set('task-fake-py@task', { + result.set('http.server-task-fake-py', { failure: 1, success: 1, }); - result.set('web@task', { + result.set('http.server-web', { success: 1, }); - const fakePy = controller.serviceHealthStatuses.get('fake-py'); - const taskFakePy = - controller.serviceHealthStatuses.get('task-fake-py@task'); - const web = controller.serviceHealthStatuses.get('web@task'); + const fakePy = controller.serviceHealthStatuses.get('fakepy-fake-py'); + const taskFakePy = controller.serviceHealthStatuses.get( + 'http.server-task-fake-py' + ); + const web = controller.serviceHealthStatuses.get('http.server-web'); assert.deepEqual( fakePy, - result.get('fake-py'), + result.get('fakepy-fake-py'), 'Service Health Check data is transformed and grouped by Service name' ); assert.deepEqual( taskFakePy, - result.get('task-fake-py@task'), + result.get('http.server-task-fake-py'), 'Service Health Check data is transformed and grouped by Service name' ); assert.deepEqual( web, - result.get('web@task'), + result.get('http.server-web'), 'Service Health Check data is transformed and grouped by Service name' ); }); @@ -54,7 +55,7 @@ module('Unit | Controller | allocations/allocation/index', function (hooks) { const dupeTaskLevelService = Allocation.allocationTaskGroup.Tasks[0].Services[0]; - dupeTaskLevelService.Name = 'fake-py@task'; + dupeTaskLevelService.Name = 'fake-py'; dupeTaskLevelService.isTaskLevel = true; const healthChecks = Allocation.healthChecks; @@ -64,35 +65,37 @@ module('Unit | Controller | allocations/allocation/index', function (hooks) { controller.set('model', Allocation); const result = new Map(); - result.set('fake-py', { + result.set('fakepy-fake-py', { failure: 1, success: 1, }); - result.set('fake-py@task', { + result.set('http.server-fake-py', { failure: 1, success: 1, }); - result.set('web@task', { + result.set('http.server-web', { success: 1, }); - const fakePy = controller.serviceHealthStatuses.get('fake-py'); - const taskFakePy = controller.serviceHealthStatuses.get('fake-py@task'); - const web = controller.serviceHealthStatuses.get('web@task'); + const fakePy = controller.serviceHealthStatuses.get('fakepy-fake-py'); + const taskFakePy = controller.serviceHealthStatuses.get( + 'http.server-fake-py' + ); + const web = controller.serviceHealthStatuses.get('http.server-web'); assert.deepEqual( fakePy, - result.get('fake-py'), + result.get('fakepy-fake-py'), 'Service Health Check data is transformed and grouped by Service name' ); assert.deepEqual( taskFakePy, - result.get('fake-py@task'), + result.get('http.server-fake-py'), 'Service Health Check data is transformed and grouped by Service name' ); assert.deepEqual( web, - result.get('web@task'), + result.get('http.server-web'), 'Service Health Check data is transformed and grouped by Service name' ); }); From 0ce11d5aef0df82d3ced7d3de85ecab9bc59bed4 Mon Sep 17 00:00:00 2001 From: Jai Bhagat Date: Tue, 30 Aug 2022 21:22:27 -0400 Subject: [PATCH 12/12] chore: cant use if and not in hbs conditional --- ui/app/templates/allocations/allocation/index.hbs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ui/app/templates/allocations/allocation/index.hbs b/ui/app/templates/allocations/allocation/index.hbs index 3f8c069b900a..d12944c56216 100644 --- a/ui/app/templates/allocations/allocation/index.hbs +++ b/ui/app/templates/allocations/allocation/index.hbs @@ -307,10 +307,10 @@ {{#if (eq row.model.provider "nomad")}}
- {{#if (not row.model.groupName)}} - - {{else}} + {{#if (is-empty row.model.taskName)}} + {{else}} + {{/if}}
{{/if}}