diff --git a/dist/index.js b/dist/index.js index 54250cdb..674df534 100644 --- a/dist/index.js +++ b/dist/index.js @@ -104662,6 +104662,7 @@ const { ModifyInstanceAttributeCommand, AssociateAddressCommand, waitUntilInstanceRunning, + waitUntilInstanceStopped, } = __nccwpck_require__(5193); const fs = __nccwpck_require__(9896); const core = __nccwpck_require__(7484); @@ -105835,11 +105836,32 @@ async function warmStartInstance(instanceId, { userData, label }) { } // Stop (not terminate) an instance so it can be reused from the pool. +// +// Blocks until the instance actually reaches the `stopped` state before +// returning. StopInstances alone only moves it to `stopping`, and a warm-pool +// lookup that runs during that window (findStoppedPoolInstance filters on +// instance-state-name=stopped) sees an empty pool and cold-launches a +// duplicate instance the reaper won't drain until its stopped-max-age passes. +// Waiting here makes "mode: stop finished" imply "instance is reusable", which +// is the serialization boundary queued follow-up workflow runs rely on. async function stopInstanceById(instanceId) { const client = ec2Client(); await withRetry('stop_instance', () => client.send(new StopInstancesCommand({ InstanceIds: [instanceId] }))); log.info('stop_instance', { instance_id: instanceId }); - core.info(`AWS EC2 instance ${instanceId} is stopped (warm pool)`); + const start = Date.now(); + log.info('wait_for_instance_stopped', { instance_id: instanceId }); + try { + await waitUntilInstanceStopped( + { client, maxWaitTime: 300 }, + { InstanceIds: [instanceId] }, + ); + log.info('wait_for_instance_stopped', { instance_id: instanceId, elapsed_ms: Date.now() - start }); + core.info(`AWS EC2 instance ${instanceId} is stopped (warm pool)`); + } catch (error) { + log.error('wait_for_instance_stopped', { instance_id: instanceId, error: error.name, message: error.message }); + core.error(`AWS EC2 instance ${instanceId} did not reach the stopped state within the wait window`); + throw error; + } } // Read the reuse-cycle counter tag for an instance (0 when absent). diff --git a/src/aws.js b/src/aws.js index 854f6164..1d319877 100644 --- a/src/aws.js +++ b/src/aws.js @@ -12,6 +12,7 @@ const { ModifyInstanceAttributeCommand, AssociateAddressCommand, waitUntilInstanceRunning, + waitUntilInstanceStopped, } = require('@aws-sdk/client-ec2'); const fs = require('fs'); const core = require('@actions/core'); @@ -1185,11 +1186,32 @@ async function warmStartInstance(instanceId, { userData, label }) { } // Stop (not terminate) an instance so it can be reused from the pool. +// +// Blocks until the instance actually reaches the `stopped` state before +// returning. StopInstances alone only moves it to `stopping`, and a warm-pool +// lookup that runs during that window (findStoppedPoolInstance filters on +// instance-state-name=stopped) sees an empty pool and cold-launches a +// duplicate instance the reaper won't drain until its stopped-max-age passes. +// Waiting here makes "mode: stop finished" imply "instance is reusable", which +// is the serialization boundary queued follow-up workflow runs rely on. async function stopInstanceById(instanceId) { const client = ec2Client(); await withRetry('stop_instance', () => client.send(new StopInstancesCommand({ InstanceIds: [instanceId] }))); log.info('stop_instance', { instance_id: instanceId }); - core.info(`AWS EC2 instance ${instanceId} is stopped (warm pool)`); + const start = Date.now(); + log.info('wait_for_instance_stopped', { instance_id: instanceId }); + try { + await waitUntilInstanceStopped( + { client, maxWaitTime: 300 }, + { InstanceIds: [instanceId] }, + ); + log.info('wait_for_instance_stopped', { instance_id: instanceId, elapsed_ms: Date.now() - start }); + core.info(`AWS EC2 instance ${instanceId} is stopped (warm pool)`); + } catch (error) { + log.error('wait_for_instance_stopped', { instance_id: instanceId, error: error.name, message: error.message }); + core.error(`AWS EC2 instance ${instanceId} did not reach the stopped state within the wait window`); + throw error; + } } // Read the reuse-cycle counter tag for an instance (0 when absent). diff --git a/tests/warmpool.test.js b/tests/warmpool.test.js index 787e5b02..11fe2477 100644 --- a/tests/warmpool.test.js +++ b/tests/warmpool.test.js @@ -18,6 +18,7 @@ jest.mock('@aws-sdk/client-ec2', () => ({ ModifyInstanceAttributeCommand: jest.fn((p) => ({ __command: 'ModifyInstanceAttribute', ...p })), AssociateAddressCommand: jest.fn((p) => ({ __command: 'AssociateAddress', ...p })), waitUntilInstanceRunning: jest.fn(), + waitUntilInstanceStopped: jest.fn(), })); jest.mock('../src/retry', () => ({ withRetry: (_step, fn) => fn() })); jest.mock('../src/config', () => ({ @@ -101,11 +102,27 @@ describe('warmStartInstance', () => { }); describe('stopInstanceById', () => { - test('sends StopInstances', async () => { + const { waitUntilInstanceStopped } = require('@aws-sdk/client-ec2'); + + beforeEach(() => waitUntilInstanceStopped.mockReset()); + + test('sends StopInstances, then waits for the stopped state', async () => { mockSend.mockResolvedValue({}); + waitUntilInstanceStopped.mockResolvedValueOnce({ state: 'SUCCESS' }); await aws.stopInstanceById('i-9'); expect(commandsSent()).toEqual(['StopInstances']); expect(mockSend.mock.calls[0][0].InstanceIds).toEqual(['i-9']); + // The waiter is the whole point: returning at `stopping` lets a queued + // follow-up run see an empty pool and cold-launch a duplicate instance. + expect(waitUntilInstanceStopped).toHaveBeenCalledTimes(1); + expect(waitUntilInstanceStopped.mock.calls[0][1]).toEqual({ InstanceIds: ['i-9'] }); + }); + + test('throws when the instance never reaches stopped', async () => { + mockSend.mockResolvedValue({}); + waitUntilInstanceStopped.mockRejectedValueOnce(new Error('TimeoutError')); + await expect(aws.stopInstanceById('i-9')).rejects.toThrow('TimeoutError'); + expect(commandsSent()).toEqual(['StopInstances']); }); });