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.

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

feat: cancel spot requests #653

Merged
merged 8 commits into from
Mar 2, 2023

Conversation

kayman-mk
Copy link
Collaborator

@kayman-mk kayman-mk commented Jan 9, 2023

Description

Whenever a spot executor is created a spot request is created. Occasionally there might be open spot requests which have not been fulfilled. These spot requests have to be deleted as soon as the agent terminates as no executors are needed.

Challenge: Make sure that spot requests are deleted only which belong to our module. Unfortunately the GitLab Runner does not tag them. We use the following logic for determinination:

  • state is open or active
  • they are associated to a SSH key which name starts with runner
  • they have the var.environment/var.overrides['name_docker_machine'] somewhere in the name

Solves part of #623
Closes #493

Migrations required

No.

Verification

  • kill an agent manually and check the Cloudwatch logs. The function should log Removing open spot requests for environment and print either Spot requests deleted or No open spot requests found. There are no errors in the log.

@kayman-mk kayman-mk marked this pull request as ready for review January 9, 2023 13:28
@kayman-mk kayman-mk requested a review from npalm as a code owner January 9, 2023 13:28
npalm
npalm previously approved these changes Feb 23, 2023
Copy link
Collaborator

@npalm npalm left a comment

Choose a reason for hiding this comment

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

In general looks good not the time right now to test. Would suggest the following

  • Add a module readme to describe shortly what the lambda does
  • Add a backlog itme to add unti test.

@kayman-mk
Copy link
Collaborator Author

kayman-mk commented Feb 27, 2023

Oh yes, missed to update the README.md. It's now done.

Issue for unit tests created (#721)

npalm
npalm previously approved these changes Feb 27, 2023
@kayman-mk
Copy link
Collaborator Author

merge conflicts fixed

@kayman-mk kayman-mk merged commit f1b4f4a into cattle-ops:main Mar 2, 2023
@kayman-mk kayman-mk deleted the kayma/remove-spot-requests branch March 2, 2023 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cancel-spot-instances.sh fails in Terraform container
3 participants