Skip to content

Commit

Permalink
Remove prestart tasks table
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
backspace committed Aug 26, 2020
1 parent 14bfb9a commit 7cf9a43
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 123 deletions.
11 changes: 0 additions & 11 deletions ui/app/controllers/allocations/allocation/task/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,6 @@ 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;
Expand Down
32 changes: 0 additions & 32 deletions ui/app/templates/allocations/allocation/task/index.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -93,38 +93,6 @@
</div>
</div>

{{#if (and (not this.model.task.lifecycle) this.prestartTaskStates)}}
<div class="boxed-section" data-test-prestart-tasks>
<div class="boxed-section-head">
Prestart Tasks
</div>
<div class="boxed-section-body is-full-bleed">
<ListTable @source={{this.prestartTaskStates}} as |t|>
<t.head>
<th class="is-narrow"></th>
<th>Task</th>
<th>State</th>
<th>Lifecycle</th>
</t.head>
<t.body as |row|>
<tr data-test-prestart-task>
<td class="is-narrow">
{{#if (and row.model.isRunning (eq row.model.task.lifecycleName "prestart"))}}
<span class="tooltip text-center" role="tooltip" aria-label="Lifecycle constraints not met">
{{x-icon "warning" class="is-warning"}}
</span>
{{/if}}
</td>
<td data-test-name>{{row.model.task.name}}</td>
<td data-test-state>{{row.model.state}}</td>
<td data-test-lifecycle>{{row.model.task.lifecycleName}}</td>
</tr>
</t.body>
</ListTable>
</div>
</div>
{{/if}}

{{#if this.model.task.volumeMounts.length}}
<div data-test-volumes class="boxed-section">
<div class="boxed-section-head">
Expand Down
80 changes: 0 additions & 80 deletions ui/tests/acceptance/task-detail-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 });

Expand Down

0 comments on commit 7cf9a43

Please sign in to comment.