feat: add support for Spot VMs and validation for provisioning options#213
feat: add support for Spot VMs and validation for provisioning options#213OliverTrautvetter wants to merge 9 commits intomainfrom
Conversation
Signed-off-by: OliverTrautvetter <66372584+OliverTrautvetter@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds Spot VM support to the GCP bootstrap flow, including input validation and behavior changes to reuse/restart existing instances.
Changes:
- Add
--spotCLI flag + docs and validation to prevent combining Spot and Preemptible. - Extend instance provisioning to support Spot scheduling with capacity-exhaustion fallback to standard VMs.
- Reuse existing instances (including starting stopped instances) and update mocks/tests accordingly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/bootstrap/gcp/gcp.go | Implements Spot scheduling + fallback, adds provisioning validation, and starts/reuses existing instances. |
| internal/bootstrap/gcp/gcp_client.go | Extends GCPClientManager with StartInstance and implements it in GCPClient. |
| internal/bootstrap/gcp/mocks.go | Updates generated mock to include StartInstance. |
| internal/bootstrap/gcp/gcp_test.go | Updates tests for new instance lookup behavior; adds coverage for spot/fallback/start scenarios and validation. |
| cli/cmd/bootstrap_gcp.go | Wires new --spot flag into CLI. |
| docs/oms_beta_bootstrap-gcp.md | Documents the new --spot flag in command help output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…re-cloud/oms into bootstrap_with_spot_vms
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| externalIP := "" | ||
| internalIP := "" | ||
| if len(existingInstance.GetNetworkInterfaces()) > 0 { | ||
| internalIP = existingInstance.GetNetworkInterfaces()[0].GetNetworkIP() | ||
| if len(existingInstance.GetNetworkInterfaces()[0].GetAccessConfigs()) > 0 { | ||
| externalIP = existingInstance.GetNetworkInterfaces()[0].GetAccessConfigs()[0].GetNatIP() | ||
| } | ||
| } |
There was a problem hiding this comment.
When an instance already exists, this branch returns IPs from the current instance record without ensuring the VM is actually ready (e.g., RUNNING) and without validating that required IPs are present. If the instance is in an intermediate state or the network interface/NAT IP isn’t populated yet, this will record empty IPs and likely break subsequent SSH/provisioning steps. Consider polling GetInstance until status is RUNNING and the expected internal/external IPs are non-empty (or returning a clear error if they can’t be obtained).
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Add the ability to bootstrap private-cloud infrastructure for quick testing on spot instances, so that the cost is minimized
Clickup