Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: prevent creating duplicate machine ids #116

Merged
merged 2 commits into from
Nov 5, 2024
Merged

fix: prevent creating duplicate machine ids #116

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4d02a9c
fix: prevent creating duplicate machine ids
goller Nov 5, 2024
ffc5a68
refactor: use aws waiters
goller Nov 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion src/utils/aws.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ systemctl start machine-agent.service
}

console.log(`Creating new instance for machine ID ${machine.id}`)
await client.send(
const instance = await client.send(
new RunInstancesCommand({
LaunchTemplate: {
LaunchTemplateId:
Expand Down Expand Up @@ -339,6 +339,25 @@ systemctl start machine-agent.service
UserData: Buffer.from(userData).toString('base64'),
}),
)

if (!instance.Instances || instance.Instances.length === 0 || !instance.Instances[0].InstanceId) {
// TODO: Will this happen?
// If this does happen you'll need to run a different kind of description
// with a filter on the tag machine id.
console.log(`No instances created for machine ID ${machine.id}`)
return
}

const instanceID = instance.Instances[0].InstanceId
let instanceCreated = false

// We are waiting for the instance to exist because we are blocking scheduleTask
// from starting another task with the same machine ID until this one completes.
while (!instanceCreated) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs a sleep otherwise this is a tight loop of API requests

Possibly better and API aware, the AWS SDK should have a wait for method I believe, similar to the Go SDK https://aws.amazon.com/blogs/developer/waiters-in-modular-aws-sdk-for-javascript/

Copy link
Member

@jacobwgillespie jacobwgillespie Nov 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the one, something like https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/Package/-aws-sdk-client-ec2/Variable/waitForInstanceExists/:

const maxWaitTime = 30 // seconds
await waitForInstanceExists({client, maxWaitTime}, {InstanceIds: [instanceID]})

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in ffc5a68

const res = await client.send(new DescribeInstancesCommand({InstanceIds: [instanceID]}))
const instances = res.Reservations?.flatMap((r) => r.Instances || []) || []
instanceCreated = instances.length > 0
}
}

function currentMachineState(instance: Instance): GetDesiredStateResponse_MachineState {
Expand Down