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

Support Spot provisioning in VM instance module #283

Merged
merged 3 commits into from
May 10, 2022
Merged

Support Spot provisioning in VM instance module #283

merged 3 commits into from
May 10, 2022

Conversation

tpdownes
Copy link
Member

@tpdownes tpdownes commented May 9, 2022

  • Enable Spot provisioning model when var.spot is set
    • do not support out-dated "preemptible" model now that Spot is GA
  • disable boot disk deletion by Compute Engine in favor of allowing Terraform to destroy the disk; otherwise terraform destroy is in a race condition with GCE and fails with some high %

Submission Checklist

  • Have you installed and run this change against pre-commit? pre-commit install
  • Are all tests passing? make tests
  • If applicable, have you written additional unit tests to cover this
    change?
  • Is unit test coverage still above 80%?
  • Have you updated any application documentation such as READMEs and user
    guides?
  • Have you followed the guidelines in our Contributing document?

@tpdownes
Copy link
Member Author

tpdownes commented May 9, 2022

Triggered validation test with Omnia that uses vm-instance module.

modules/compute/vm-instance/variables.tf Outdated Show resolved Hide resolved
modules/compute/vm-instance/main.tf Outdated Show resolved Hide resolved
@nick-stroud nick-stroud assigned tpdownes and unassigned nick-stroud May 10, 2022
@tpdownes
Copy link
Member Author

tpdownes commented May 10, 2022

I am force pushing a change with this local diff relative to the branch on my fork (i.e. what you just reviewed):

diff --git a/modules/compute/vm-instance/main.tf b/modules/compute/vm-instance/main.tf
index 8cccacc..a789957 100644
--- a/modules/compute/vm-instance/main.tf
+++ b/modules/compute/vm-instance/main.tf
@@ -89,7 +89,7 @@ resource "google_compute_instance" "compute_vm" {
   boot_disk {
     source      = google_compute_disk.boot_disk[count.index].self_link
     device_name = google_compute_disk.boot_disk[count.index].name
-    auto_delete = false
+    auto_delete = true
   }

   network_interface {
diff --git a/modules/compute/vm-instance/variables.tf b/modules/compute/vm-instance/variables.tf
index d749d5e..084496e 100644
--- a/modules/compute/vm-instance/variables.tf
+++ b/modules/compute/vm-instance/variables.tf
@@ -184,4 +184,4 @@ variable "preemptible" {
   description = "Provision VMs using discounted Spot pricing, allowing for preemption"
   type        = bool
   default     = false
-}
\ No newline at end of file
+}

@nick-stroud nick-stroud assigned tpdownes and unassigned nick-stroud May 10, 2022
@tpdownes tpdownes merged commit ae7f780 into GoogleCloudPlatform:develop May 10, 2022
@tpdownes tpdownes deleted the feat_vm_instance_spot branch May 11, 2022 15:39
heyealex pushed a commit to heyealex/hpc-toolkit that referenced this pull request Nov 7, 2022
)

Add support for Spot VMs to vm-instance module
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.

2 participants