-
Notifications
You must be signed in to change notification settings - Fork 115
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
Allow fast boot disable on region #509
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this looks good. I left a suggestion on the documentation and a question about permissions, if needed.
if templateSpec.EnableFalseLaunch == config.TriFalse { | ||
log.Printf("[INFO] fast-launch explicitly disabled for region %q", region) | ||
templateIDsByRegion[region] = TemplateSpec{ | ||
Enabled: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not to clear on how this works by default but from what I gather (looking at the issue) fast launch seems to be enabled by default on the region. Is that correct?
If so, does the user need any particular permissions to disable on the region?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No permissions necessary, we enable fast-launch for each region in a later step, so if you don't want it on a particular region, we don't request it be enabled
The fast-launch configuration allows setupping fast-launch for Windows virtual machines, either with a common configuration, or with a region-by-region configuration. However if someone wants to only enable fast-launch on a subset of regions, there wasn't any way to express that, leading to either a waste of resources, or a need to manually cleanup the extra snapshots created. Therefore this commit adds an extra trilean to the regional configuration for fast-launch, so it can be explicitly disabled on selected regions.
Those tests are long-running, and can run concurrently as they don't use the same resources, so in order to reduce the length of running those tests, we enable parallelisation on them.
21d02e8
to
72a149e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clarification. LGTM
The fast-launch configuration allows setupping fast-launch for Windows virtual machines, either with a common configuration, or with a region-by-region configuration.
However if someone wants to only enable fast-launch on a subset of regions, there wasn't any way to express that, leading to either a waste of resources, or a need to manually cleanup the extra snapshots created.
Therefore this commit adds an extra trilean to the regional configuration for fast-launch, so it can be explicitly disabled on selected regions.
Closes: #506