From 0d885bec09d0d6765fbf33eb72435798fa5617de Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Wed, 26 Aug 2020 09:46:10 -0500 Subject: [PATCH] Remove prestart tasks table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My suggestion is that this table isn’t sufficiently useful to keep around with the combinatoric explosion of other lifecycle phases. The logic was that someone might wonder “why isn’t my main task starting?” and this table would show that the prestart tasks hadn’t yet completed. One might wonder the same about any task that has prerequisites, so should a poststart task have a table that shows main tasks? And so on. Since the route hierarchy guarantees that one has already passed through a template that shows the lifecycle chart before one can reach the template where this table is displayed, I believe this table is redundant. It also conveys information in a more abstract way than the chart, which is dense and more easily understood, to me. --- .../allocations/allocation/task/index.js | 12 --- .../allocations/allocation/task/index.hbs | 32 -------- ui/tests/acceptance/task-detail-test.js | 80 ------------------- 3 files changed, 124 deletions(-) diff --git a/ui/app/controllers/allocations/allocation/task/index.js b/ui/app/controllers/allocations/allocation/task/index.js index 6af893dff..57e661d1f 100644 --- a/ui/app/controllers/allocations/allocation/task/index.js +++ b/ui/app/controllers/allocations/allocation/task/index.js @@ -1,22 +1,10 @@ import Controller from '@ember/controller'; -import { computed } from '@ember/object'; import { computed as overridable } from 'ember-overridable-computed'; import { task } from 'ember-concurrency'; import classic from 'ember-classic-decorator'; @classic export default class IndexController extends Controller { - @computed('model.task.taskGroup.tasks.@each.name') - get otherTaskStates() { - const taskName = this.model.task.name; - return this.model.allocation.states.rejectBy('name', taskName); - } - - @computed('otherTaskStates.@each.lifecycle') - get prestartTaskStates() { - return this.otherTaskStates.filterBy('task.lifecycle'); - } - @overridable(() => { // { title, description } return null; diff --git a/ui/app/templates/allocations/allocation/task/index.hbs b/ui/app/templates/allocations/allocation/task/index.hbs index 4b36e9147..72dbd3edf 100644 --- a/ui/app/templates/allocations/allocation/task/index.hbs +++ b/ui/app/templates/allocations/allocation/task/index.hbs @@ -93,38 +93,6 @@ - {{#if (and (not this.model.task.lifecycle) this.prestartTaskStates)}} -
-
- Prestart Tasks -
-
- - - - Task - State - Lifecycle - - - - - {{#if (and row.model.isRunning (eq row.model.task.lifecycleName "prestart"))}} - - {{x-icon "warning" class="is-warning"}} - - {{/if}} - - {{row.model.task.name}} - {{row.model.state}} - {{row.model.task.lifecycleName}} - - - -
-
- {{/if}} - {{#if this.model.task.volumeMounts.length}}
diff --git a/ui/tests/acceptance/task-detail-test.js b/ui/tests/acceptance/task-detail-test.js index 10e150ffc..21b78e78e 100644 --- a/ui/tests/acceptance/task-detail-test.js +++ b/ui/tests/acceptance/task-detail-test.js @@ -101,86 +101,6 @@ module('Acceptance | task detail', function(hooks) { assert.equal(Task.resourceCharts.objectAt(1).name, 'Memory', 'Second chart is Memory'); }); - test('/allocation/:id/:task_name lists related prestart tasks for a main task when they exist', async function(assert) { - const job = server.create('job', { - groupsCount: 2, - groupTaskCount: 3, - createAllocations: false, - status: 'running', - }); - - job.task_group_ids.forEach(taskGroupId => { - server.create('allocation', { - jobId: job.id, - taskGroup: server.db.taskGroups.find(taskGroupId).name, - forceRunningClientStatus: true, - }); - }); - - const taskGroup = job.task_groups.models[0]; - const [mainTask, sidecarTask, prestartTask] = taskGroup.tasks.models; - - mainTask.attrs.Lifecycle = null; - mainTask.save(); - - sidecarTask.attrs.Lifecycle = { Sidecar: true, Hook: 'prestart' }; - sidecarTask.save(); - - prestartTask.attrs.Lifecycle = { Sidecar: false, Hook: 'prestart' }; - prestartTask.save(); - - taskGroup.save(); - - const noPrestartTasksTaskGroup = job.task_groups.models[1]; - noPrestartTasksTaskGroup.tasks.models.forEach(task => { - task.attrs.Lifecycle = null; - task.save(); - }); - - const mainTaskState = server.schema.taskStates.findBy({ name: mainTask.name }); - const sidecarTaskState = server.schema.taskStates.findBy({ name: sidecarTask.name }); - const prestartTaskState = server.schema.taskStates.findBy({ name: prestartTask.name }); - - prestartTaskState.attrs.state = 'running'; - prestartTaskState.attrs.finishedAt = null; - prestartTaskState.save(); - - await Task.visit({ id: mainTaskState.allocationId, name: mainTask.name }); - - assert.ok(Task.hasPrestartTasks); - assert.equal(Task.prestartTasks.length, 2); - - Task.prestartTasks[0].as(SidecarTask => { - assert.equal(SidecarTask.name, sidecarTask.name); - assert.equal(SidecarTask.state, sidecarTaskState.state); - assert.equal(SidecarTask.lifecycle, 'sidecar'); - assert.notOk(SidecarTask.isBlocking); - }); - - Task.prestartTasks[1].as(PrestartTask => { - assert.equal(PrestartTask.name, prestartTask.name); - assert.equal(PrestartTask.state, prestartTaskState.state); - assert.equal(PrestartTask.lifecycle, 'prestart'); - assert.ok(PrestartTask.isBlocking); - }); - - await Task.visit({ id: sidecarTaskState.allocationId, name: sidecarTask.name }); - - assert.notOk(Task.hasPrestartTasks); - - const noPrestartTasksTask = noPrestartTasksTaskGroup.tasks.models[0]; - const noPrestartTasksTaskState = server.db.taskStates.findBy({ - name: noPrestartTasksTask.name, - }); - - await Task.visit({ - id: noPrestartTasksTaskState.allocationId, - name: noPrestartTasksTaskState.name, - }); - - assert.notOk(Task.hasPrestartTasks); - }); - test('the events table lists all recent events', async function(assert) { const events = server.db.taskEvents.where({ taskStateId: task.id });