Skip to content

Commit c31c253

Browse files
imbstackimbstack
authored
Merge pull request #2164 from taskcluster/provisioning-hotfix
Fix provisioning logic
2 parents 083a8cc + 4dca3ac commit c31c253

File tree

5 files changed

+120
-28
lines changed

5 files changed

+120
-28
lines changed

‎changelog/Df3me_dPRWa8bJPrvRQfNQ.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
level: patch
2+
---
3+
Provisioning logic now counts workers correctly

‎services/worker-manager/src/providers/testing.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ class TestingProvider extends Provider {
2727
}
2828

2929
async provision({workerPool, existingCapacity}) {
30-
this.monitor.notice('test-provision', {workerPoolId: workerPool.workerPoolId});
30+
this.monitor.notice('test-provision', {workerPoolId: workerPool.workerPoolId, existingCapacity});
3131
}
3232

3333
async deprovision({workerPool}) {

‎services/worker-manager/src/provisioner.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,11 +120,11 @@ class Provisioner {
120120
const v = providersByPool.get(worker.workerPoolId);
121121
if (v) {
122122
v.providers.add(worker.providerId);
123-
v.capacity += worker.workerCapacity;
123+
v.capacity += worker.capacity;
124124
} else {
125125
providersByPool.set(worker.workerPoolId, {
126126
providers: new Set([worker.providerId]),
127-
capacity: worker.workerCapacity,
127+
capacity: worker.capacity,
128128
});
129129
}
130130
};

‎services/worker-manager/test/provisioner_test.js

Lines changed: 57 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,16 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
1515
helper.withProvisioner(mock, skipping);
1616

1717
suite('provisioning loop', function() {
18-
const testCase = ({workers = [], workerPools = [], assertion, expectErrors = false}) => {
19-
return testing.runWithFakeTime(async function() {
20-
await Promise.all(workers.map(w => helper.Worker.create(w)));
21-
await Promise.all(workerPools.map(async wt => {
18+
const testCase = async ({workers = [], workerPools = [], assertion, expectErrors = false}) => {
19+
await Promise.all(workers.map(w => helper.Worker.create(w)));
20+
await Promise.all(workerPools.map(async wt => {
21+
if (wt.input) {
2222
await helper.workerManager.createWorkerPool(wt.workerPoolId, wt.input);
23-
}));
23+
} else {
24+
await helper.WorkerPool.create(wt);
25+
}
26+
}));
27+
return (testing.runWithFakeTime(async function() {
2428

2529
await helper.initiateProvisioner();
2630
await testing.poll(async () => {
@@ -31,20 +35,21 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
3135
}
3236
}
3337
await Promise.all(workerPools.map(async wt => {
38+
const pId = wt.providerId || wt.input.providerId;
3439
assert.deepEqual(
3540
monitorManager.messages.find(
3641
msg => msg.Type === 'worker-pool-provisioned' && msg.Fields.workerPoolId === wt.workerPoolId), {
3742
Logger: 'taskcluster.worker-manager.provisioner',
3843
Type: 'worker-pool-provisioned',
39-
Fields: {workerPoolId: wt.workerPoolId, providerId: wt.input.providerId, v: 1},
44+
Fields: {workerPoolId: wt.workerPoolId, providerId: pId, v: 1},
4045
Severity: LEVELS.info,
4146
});
4247
assert.deepEqual(
4348
monitorManager.messages.find(
4449
msg => msg.Type === 'test-provision' && msg.Fields.workerPoolId === wt.workerPoolId), {
45-
Logger: `taskcluster.worker-manager.provider.${wt.input.providerId}`,
50+
Logger: `taskcluster.worker-manager.provider.${pId}`,
4651
Type: 'test-provision',
47-
Fields: {workerPoolId: wt.workerPoolId},
52+
Fields: {workerPoolId: wt.workerPoolId, existingCapacity: wt.existingCapacity},
4853
Severity: LEVELS.notice,
4954
});
5055
}));
@@ -59,13 +64,14 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
5964
}, {
6065
mock,
6166
maxTime: 30000,
62-
});
67+
}))();
6368
};
6469

65-
test('single worker pool', testCase({
70+
test('single worker pool', () => testCase({
6671
workerPools: [
6772
{
6873
workerPoolId: 'pp/ee',
74+
existingCapacity: 0,
6975
input: {
7076
providerId: 'testing1',
7177
description: 'bar',
@@ -77,10 +83,42 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
7783
],
7884
}));
7985

80-
test('multiple worker pools, same provider', testCase({
86+
test('single worker pool (with running worker)', () => testCase({
87+
workers: [
88+
{
89+
workerPoolId: 'ff/ee',
90+
workerGroup: 'whatever',
91+
workerId: 'testing-OLD',
92+
providerId: 'testing1',
93+
created: new Date(),
94+
lastModified: new Date(),
95+
lastChecked: new Date(),
96+
expires: taskcluster.fromNow('1 hour'),
97+
capacity: 1,
98+
state: helper.Worker.states.RUNNING,
99+
providerData: {},
100+
},
101+
],
102+
workerPools: [
103+
{
104+
workerPoolId: 'ff/ee',
105+
existingCapacity: 1,
106+
input: {
107+
providerId: 'testing1',
108+
description: 'bar',
109+
config: {},
110+
owner: '[email protected]',
111+
emailOnError: false,
112+
},
113+
},
114+
],
115+
}));
116+
117+
test('multiple worker pools, same provider', () => testCase({
81118
workerPools: [
82119
{
83120
workerPoolId: 'pp/ee',
121+
existingCapacity: 0,
84122
input: {
85123
providerId: 'testing1',
86124
description: 'bar',
@@ -91,6 +129,7 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
91129
},
92130
{
93131
workerPoolId: 'pp/ee2',
132+
existingCapacity: 0,
94133
input: {
95134
providerId: 'testing1',
96135
description: 'bar',
@@ -102,10 +141,11 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
102141
],
103142
}));
104143

105-
test('multiple worker pools, different provider', testCase({
144+
test('multiple worker pools, different provider', () => testCase({
106145
workerPools: [
107146
{
108147
workerPoolId: 'pp/ee',
148+
existingCapacity: 0,
109149
input: {
110150
providerId: 'testing1',
111151
description: 'bar',
@@ -116,6 +156,7 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
116156
},
117157
{
118158
workerPoolId: 'pp/ee2',
159+
existingCapacity: 0,
119160
input: {
120161
providerId: 'testing2',
121162
description: 'bar',
@@ -159,6 +200,7 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
159200
workerPools: [
160201
{
161202
workerPoolId: 'ff/ee',
203+
existingCapacity: 1,
162204
providerId: 'testing2',
163205
previousProviderIds: ['testing1'],
164206
description: '',
@@ -175,16 +217,6 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
175217
],
176218
expectErrors: true,
177219
assertion: async () => {
178-
const worker = await helper.Worker.load({
179-
workerPoolId: 'ff/ee',
180-
workerGroup: 'whatever',
181-
workerId: 'testing-123',
182-
});
183-
assert(worker.providerData.checked);
184-
185-
const wp = await helper.WorkerPool.load({workerPoolId: 'ff/ee'});
186-
assert.deepEqual(wp.previousProviderIds, []);
187-
188220
assert.deepEqual(monitorManager.messages.find(
189221
msg => msg.Type === 'remove-resource' && msg.Logger.endsWith('testing1')), {
190222
Logger: `taskcluster.worker-manager.provider.testing1`,
@@ -398,7 +430,7 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
398430
msg => msg.Type === 'test-provision' && msg.Fields.workerPoolId === workerPool.workerPoolId), {
399431
Logger: `taskcluster.worker-manager.provider.${workerPool.input.providerId}`,
400432
Type: 'test-provision',
401-
Fields: {workerPoolId: workerPool.workerPoolId},
433+
Fields: {workerPoolId: workerPool.workerPoolId, existingCapacity: 0},
402434
Severity: LEVELS.notice,
403435
});
404436

@@ -437,7 +469,7 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
437469
msg => msg.Type === 'test-provision' && msg.Fields.workerPoolId === workerPool.workerPoolId), {
438470
Logger: `taskcluster.worker-manager.provider.${workerPool.input.providerId}`,
439471
Type: 'test-provision',
440-
Fields: {workerPoolId: workerPool.workerPoolId},
472+
Fields: {workerPoolId: workerPool.workerPoolId, existingCapacity: 0},
441473
Severity: LEVELS.notice,
442474
});
443475

@@ -460,7 +492,7 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
460492
msg => msg.Type === 'test-provision' && msg.Fields.workerPoolId === workerPool.workerPoolId), {
461493
Logger: `taskcluster.worker-manager.provider.${workerPool.input.providerId}`,
462494
Type: 'test-provision',
463-
Fields: {workerPoolId: workerPool.workerPoolId},
495+
Fields: {workerPoolId: workerPool.workerPoolId, existingCapacity: 0},
464496
Severity: LEVELS.notice,
465497
});
466498
});

‎services/worker-manager/test/worker_scanner_test.js

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,4 +166,61 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
166166
assert(worker2.providerData.checked);
167167
},
168168
}));
169+
170+
test('worker for previous provider is stopped', () => testCase({
171+
workers: [
172+
{
173+
workerPoolId: 'ff/ee',
174+
workerGroup: 'whatever',
175+
workerId: 'testing-OLD',
176+
providerId: 'testing1',
177+
created: new Date(),
178+
lastModified: new Date(),
179+
lastChecked: new Date(),
180+
expires: taskcluster.fromNow('1 hour'),
181+
capacity: 1,
182+
state: helper.Worker.states.STOPPED,
183+
providerData: {},
184+
}, {
185+
workerPoolId: 'ff/ee',
186+
workerGroup: 'whatever',
187+
workerId: 'testing-123',
188+
providerId: 'testing2',
189+
created: new Date(),
190+
lastModified: new Date(),
191+
lastChecked: new Date(),
192+
expires: taskcluster.fromNow('1 hour'),
193+
capacity: 1,
194+
state: helper.Worker.states.REQUESTED,
195+
providerData: {},
196+
},
197+
],
198+
workerPools: [
199+
{
200+
workerPoolId: 'ff/ee',
201+
existingCapacity: 1,
202+
providerId: 'testing2',
203+
previousProviderIds: ['testing1'],
204+
description: '',
205+
created: taskcluster.fromNow('-1 hour'),
206+
lastModified: taskcluster.fromNow('-1 hour'),
207+
config: {},
208+
owner: '[email protected]',
209+
emailOnError: false,
210+
providerData: {
211+
// make removeResources fail on the first try, to test error handling
212+
failRemoveResources: 1,
213+
},
214+
},
215+
],
216+
expectErrors: true,
217+
assertion: async () => {
218+
const worker = await helper.Worker.load({
219+
workerPoolId: 'ff/ee',
220+
workerGroup: 'whatever',
221+
workerId: 'testing-123',
222+
});
223+
assert(worker.providerData.checked);
224+
},
225+
}));
169226
});

0 commit comments

Comments
 (0)