Skip to content

runners: Add terraform module for scale-cycle #6893

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

Open
wants to merge 2 commits into
base: gh/seemethere/3/head
Choose a base branch
from

Conversation

seemethere
Copy link
Member

Adds terraform files required for deploying scale-cycle. This also adds
a switch to turn on scale-cycle or not with the default position being
off. This should make it easier to deploy scale-cycle in the future and
could also act as a gate for easily enabling / disabling scale-up's
tryReuseRunner functionality.

Signed-off-by: Eli Uriegas [email protected]

[ghstack-poisoned]
@seemethere
Copy link
Member Author

seemethere commented Jul 7, 2025

Copy link

vercel bot commented Jul 7, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Updated (UTC)
torchci ⬜️ Ignored (Inspect) Visit Preview Jul 7, 2025 9:31pm

seemethere added a commit that referenced this pull request Jul 7, 2025
Adds terraform files required for deploying scale-cycle. This also adds
a switch to turn on scale-cycle or not with the default position being
off. This should make it easier to deploy scale-cycle in the future and
could also act as a gate for easily enabling / disabling scale-up's
tryReuseRunner functionality.

Signed-off-by: Eli Uriegas <[email protected]>
ghstack-source-id: 3d9260c
ghstack-comment-id: 3046573503
Pull-Request: #6893
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 7, 2025
[ghstack-poisoned]
seemethere added a commit that referenced this pull request Jul 7, 2025
Adds terraform files required for deploying scale-cycle. This also adds
a switch to turn on scale-cycle or not with the default position being
off. This should make it easier to deploy scale-cycle in the future and
could also act as a gate for easily enabling / disabling scale-up's
tryReuseRunner functionality.

Signed-off-by: Eli Uriegas <[email protected]>
ghstack-source-id: 87583f4
ghstack-comment-id: 3046573503
Pull-Request: #6893
Signed-off-by: Eli Uriegas <[email protected]>
@seemethere seemethere closed this Jul 7, 2025
@seemethere seemethere reopened this Jul 7, 2025
@@ -374,6 +374,24 @@ variable "retry_scale_up_chron_hud_query_url" {
default = ""
}

variable "enable_scale_cycle" {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: enabling could be inferred based on whether scale_cycle_schedule_expression set

Comment on lines +10 to +12
"ec2:CreateNetworkInterface",
"ec2:DescribeNetworkInterfaces",
"ec2:DeleteNetworkInterface",
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these actually needed? I do see that scale up has these as well, but it seems odd that they'd require this permission.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is techincally what is needed to create an EC2 instance

Copy link
Collaborator

@zxiiro zxiiro Jul 16, 2025

Choose a reason for hiding this comment

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

I believe this is only necessary if we need more complex networking outside of what EC2 instance launch creates when you create a new EC2 instance. Like an instance with multiple NICs connected to more than 1 network.

Edit: I think ec2:RunInstance already creates the default NIC.

"ec2:CreateTags",
"ec2:DeleteTags",
"ec2:CreateReplaceRootVolumeTask",
"ec2:DescribeReplaceRootVolumeTasks"
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this permission needed for? Scale up/down don't use it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants