feat(worker-manager): Timeout leads to stuck process (#5293)

lotas · web-flow · commit 0c0f32deb6c8 · 2022-03-17T14:43:14.000+01:00
Force provisioner loop to exit in case of timeouts
diff --git a/changelog/issue-5003.md b/changelog/issue-5003.md
@@ -0,0 +1,6 @@
+audience: general
+level: patch
+reference: issue 5003
+---
+
+Allow provisioner to exit instead of being stuck in delayed loop.
diff --git a/services/worker-manager/src/provisioner.js b/services/worker-manager/src/provisioner.js
@@ -1,6 +1,7 @@
 const Iterate = require('taskcluster-lib-iterate');
 const { paginatedIterator } = require('taskcluster-lib-postgres');
 const { WorkerPool, Worker } = require('./data');
+const { ApiError } = require('./providers/provider');
 
 /**
  * Run all provisioning logic
@@ -64,8 +65,10 @@ class Provisioner {
    */
   async provision() {
     if (this.provisioningLoopAlive) {
-      this.monitor.notice('loop-interference', {});
-      return;
+      this.monitor.alert('loop-interference', {});
+      // should be treated as terminal error
+      // to let the pod to restart and avoid getting stuck in a loop
+      throw new ApiError('provision loop interference');
     }
     try {
       this.provisioningLoopAlive = true;
diff --git a/services/worker-manager/test/fakes/aws.js b/services/worker-manager/test/fakes/aws.js
@@ -47,7 +47,7 @@ const ec2Method = (context, method) => {
  * A fake region-specific EC2 client.  This has the SDK methods (all with a `.promise()`
  * trailer).  It also has properties:
  *
- *  - runInstancesCalls: an array of the launch config passsed to each runInstances call
+ *  - runInstancesCalls: an array of the launch config passed to each runInstances call
  *  - terminatedInstances: instance IDs for which terminateInstances has been called
  *  - instanceStatuses: values returned from describeInstanceStatus, in the form {instanceId: state}
  */
diff --git a/services/worker-manager/test/fakes/azure.js b/services/worker-manager/test/fakes/azure.js
@@ -36,7 +36,7 @@ class FakeAzure extends FakeCloud {
   }
 
   _reset() {
-    // managers indexed by resoruceType
+    // managers indexed by resourceType
     this._managers = {
       vm: new VMResourceManager(this, 'vm', 'azure-vm.yml'),
       // (no schema for disks as provider does not create them directly)
diff --git a/services/worker-manager/test/provisioner_test.js b/services/worker-manager/test/provisioner_test.js
@@ -4,6 +4,7 @@ const testing = require('taskcluster-lib-testing');
 const taskcluster = require('taskcluster-client');
 const { LEVELS } = require('taskcluster-lib-monitor');
 const { WorkerPool, Worker } = require('../src/data');
+const { ApiError } = require('../src/providers/provider');
 
 helper.secrets.mockSuite(testing.suiteName(), [], function(mock, skipping) {
   helper.withDb(mock, skipping);
@@ -358,6 +359,38 @@ helper.secrets.mockSuite(testing.suiteName(), [], function(mock, skipping) {
           Severity: LEVELS.info,
         });
     });
+
+    test("provision loop is not running in parallel", async function() {
+      await WorkerPool.fromApi({
+        workerPoolId: 'pp/ww',
+        providerId: 'testing1',
+        previousProviderIds: ['NO-SUCH'],
+        description: '',
+        created: new Date(),
+        lastModified: new Date(),
+        config: {},
+        owner: 'me@example.com',
+        emailOnError: false,
+        providerData: {},
+      }).create(helper.db);
+      const provisioner = await helper.load('provisioner');
+
+      await assert.rejects(async () => {
+        await Promise.all([
+          provisioner.provision(),
+          provisioner.provision(),
+        ]);
+      }, new ApiError('provision loop interference'));
+
+      assert.deepEqual(
+        monitor.manager.messages.find(msg => msg.Type === 'loop-interference'), {
+          Fields: {},
+          Logger: 'taskcluster.test.provisioner',
+          Severity: 1,
+          Type: 'loop-interference',
+        });
+      monitor.manager.reset();
+    });
   });
 
   suite('deprovisioning loop', function() {

Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ const ec2Method = (context, method) => {`
`47`	`47`	* A fake region-specific EC2 client. This has the SDK methods (all with a `.promise()`
`48`	`48`	`* trailer). It also has properties:`
`49`	`49`	`*`
`50`		`- * - runInstancesCalls: an array of the launch config passsed to each runInstances call`
	`50`	`+ * - runInstancesCalls: an array of the launch config passed to each runInstances call`
`51`	`51`	`* - terminatedInstances: instance IDs for which terminateInstances has been called`
`52`	`52`	`* - instanceStatuses: values returned from describeInstanceStatus, in the form {instanceId: state}`
`53`	`53`	`*/`
Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,7 @@ class FakeAzure extends FakeCloud {`
`36`	`36`	`}`
`37`	`37`
`38`	`38`	`_reset() {`
`39`		`- // managers indexed by resoruceType`
	`39`	`+ // managers indexed by resourceType`
`40`	`40`	`this._managers = {`
`41`	`41`	`vm: new VMResourceManager(this, 'vm', 'azure-vm.yml'),`
`42`	`42`	`// (no schema for disks as provider does not create them directly)`