Skip to content

Commit e700a21

Browse files
committed
ui: use task state to determine if task is active (#14224)
The current implementation uses the task's finishedAt field to determine if a task is active of not, but this check is not accurate. A task in the "pending" state will not have finishedAt value but it's also not active. This discrepancy results in some components, like the inline stats chart of the task row component, to be displayed even whey they shouldn't.
1 parent baa5c57 commit e700a21

File tree

6 files changed

+60
-16
lines changed

6 files changed

+60
-16
lines changed

.changelog/14224.txt

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:bug
2+
ui: Fixed a bug that caused the allocation details page to display the stats bar chart even if the task was pending.
3+
```

ui/app/models/task-state.js

+6-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { computed } from '@ember/object';
2-
import { alias, none, and } from '@ember/object/computed';
2+
import { alias, and } from '@ember/object/computed';
33
import Fragment from 'ember-data-model-fragments/fragment';
44
import { attr } from '@ember-data/model';
55
import { fragment, fragmentOwner, fragmentArray } from 'ember-data-model-fragments/attributes';
@@ -15,7 +15,6 @@ export default class TaskState extends Fragment {
1515
@attr('date') finishedAt;
1616
@attr('boolean') failed;
1717

18-
@none('finishedAt') isActive;
1918
@and('isActive', 'allocation.isRunning') isRunning;
2019

2120
@computed('task.kind')
@@ -54,6 +53,11 @@ export default class TaskState extends Fragment {
5453
return classMap[this.state] || 'is-dark';
5554
}
5655

56+
@computed('state')
57+
get isActive() {
58+
return this.state === 'running';
59+
}
60+
5761
restart() {
5862
return this.allocation.restart(this.name);
5963
}

ui/tests/acceptance/allocation-detail-test.js

+36-9
Original file line numberDiff line numberDiff line change
@@ -125,22 +125,45 @@ module('Acceptance | allocation detail', function(hooks) {
125125
});
126126

127127
test('each task row should list high-level information for the task', async function(assert) {
128-
const task = server.db.taskStates.where({ allocationId: allocation.id }).sortBy('name')[0];
129-
const events = server.db.taskEvents.where({ taskStateId: task.id });
130-
const event = events[events.length - 1];
128+
const job = server.create('job', {
129+
groupsCount: 1,
130+
groupTaskCount: 3,
131+
withGroupServices: true,
132+
createAllocations: false,
133+
});
134+
135+
const allocation = server.create('allocation', 'withTaskWithPorts', {
136+
clientStatus: 'running',
137+
jobId: job.id,
138+
});
131139

132140
const taskGroup = server.schema.taskGroups.where({
133141
jobId: allocation.jobId,
134142
name: allocation.taskGroup,
135143
}).models[0];
136144

137-
const jobTask = taskGroup.tasks.models.find(m => m.name === task.name);
138-
const volumes = jobTask.volumeMounts.map(volume => ({
139-
name: volume.Volume,
140-
source: taskGroup.volumes[volume.Volume].Source,
141-
}));
145+
// Set the expected task states.
146+
const states = ['running', 'pending', 'dead'];
147+
server.db.taskStates
148+
.where({ allocationId: allocation.id })
149+
.sortBy('name')
150+
.forEach((task, i) => {
151+
server.db.taskStates.update(task.id, { state: states[i] });
152+
});
153+
154+
await Allocation.visit({ id: allocation.id });
155+
156+
Allocation.tasks.forEach((taskRow, i) => {
157+
const task = server.db.taskStates.where({ allocationId: allocation.id }).sortBy('name')[i];
158+
const events = server.db.taskEvents.where({ taskStateId: task.id });
159+
const event = events[events.length - 1];
160+
161+
const jobTask = taskGroup.tasks.models.find(m => m.name === task.name);
162+
const volumes = jobTask.volumeMounts.map(volume => ({
163+
name: volume.Volume,
164+
source: taskGroup.volumes[volume.Volume].Source,
165+
}));
142166

143-
Allocation.tasks[0].as(taskRow => {
144167
assert.equal(taskRow.name, task.name, 'Name');
145168
assert.equal(taskRow.state, task.state, 'State');
146169
assert.equal(taskRow.message, event.displayMessage, 'Event Message');
@@ -150,6 +173,10 @@ module('Acceptance | allocation detail', function(hooks) {
150173
'Event Time'
151174
);
152175

176+
const expectStats = task.state === 'running';
177+
assert.equal(taskRow.hasCpuMetrics, expectStats, 'CPU metrics');
178+
assert.equal(taskRow.hasMemoryMetrics, expectStats, 'Memory metrics');
179+
153180
const volumesText = taskRow.volumes;
154181
volumes.forEach(volume => {
155182
assert.ok(volumesText.includes(volume.name), `Found label ${volume.name}`);

ui/tests/acceptance/exec-test.js

+9-4
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,12 @@ module('Acceptance | exec', function(hooks) {
2626
});
2727

2828
this.job.taskGroups.models.forEach(taskGroup => {
29-
server.create('allocation', {
29+
const alloc = server.create('allocation', {
3030
jobId: this.job.id,
3131
taskGroup: taskGroup.name,
3232
forceRunningClientStatus: true,
3333
});
34+
server.db.taskStates.update({ allocationId: alloc.id }, { state: 'running' });
3435
});
3536
});
3637

@@ -127,9 +128,11 @@ module('Acceptance | exec', function(hooks) {
127128

128129
let runningTaskGroup = this.job.taskGroups.models.sortBy('name')[1];
129130
runningTaskGroup.tasks.models.forEach((task, index) => {
131+
let state = 'running';
130132
if (index > 0) {
131-
this.server.db.taskStates.update({ name: task.name }, { finishedAt: new Date() });
133+
state = 'dead';
132134
}
135+
this.server.db.taskStates.update({ name: task.name }, { state });
133136
});
134137

135138
await Exec.visitJob({ job: this.job.id });
@@ -148,9 +151,11 @@ module('Acceptance | exec', function(hooks) {
148151
let runningTaskGroup = this.job.taskGroups.models.sortBy('name')[1];
149152
let changingTaskStateName;
150153
runningTaskGroup.tasks.models.sortBy('name').forEach((task, index) => {
154+
let state = 'running';
151155
if (index > 0) {
152-
this.server.db.taskStates.update({ name: task.name }, { finishedAt: new Date() });
156+
state = 'dead';
153157
}
158+
this.server.db.taskStates.update({ name: task.name }, { state });
154159

155160
if (index === 1) {
156161
changingTaskStateName = task.name;
@@ -170,7 +175,7 @@ module('Acceptance | exec', function(hooks) {
170175
const changingTaskState = allocation.states.findBy('name', changingTaskStateName);
171176

172177
if (changingTaskState) {
173-
changingTaskState.set('finishedAt', undefined);
178+
changingTaskState.set('state', 'running');
174179
}
175180
});
176181

ui/tests/acceptance/task-detail-test.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ module('Acceptance | task detail', function(hooks) {
1818
server.create('agent');
1919
server.create('node');
2020
server.create('job', { createAllocations: false });
21-
allocation = server.create('allocation', 'withTaskWithPorts', { clientStatus: 'running' });
21+
allocation = server.create('allocation', 'withTaskWithPorts', {
22+
clientStatus: 'running',
23+
});
24+
server.db.taskStates.update({ allocationId: allocation.id }, { state: 'running' });
2225
task = server.db.taskStates.where({ allocationId: allocation.id })[0];
2326

2427
await Task.visit({ id: allocation.id, name: task.name });

ui/tests/pages/allocations/detail.js

+2
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ export default create({
5050
time: text('[data-test-time]'),
5151
volumes: text('[data-test-volumes]'),
5252

53+
hasCpuMetrics: isPresent('[data-test-cpu] .inline-chart progress'),
54+
hasMemoryMetrics: isPresent('[data-test-mem] .inline-chart progress'),
5355
hasUnhealthyDriver: isPresent('[data-test-icon="unhealthy-driver"]'),
5456
hasProxyTag: isPresent('[data-test-proxy-tag]'),
5557

0 commit comments

Comments
 (0)