Merge pull request #34 from ScaleComputing/fix-vm-shutdown

Fix vm shutdown

Author: justinc1

internal/provider/hypercore_vm_power_state_resource.go

@@ -125,14 +125,7 @@ func (r *HypercoreVMPowerStateResource) Create(ctx context.Context, req resource
 	// we need to check with the NEEDED_ACTION_FOR_POWER_STATE.
 	// actionType := utils.NEEDED_ACTION_FOR_POWER_STATE[data.State.ValueString()]
 	actionType := utils.GetNeededActionForState(data.State.ValueString(), data.ForceSutoff.ValueBool())
-	createPayload := []map[string]any{
-		{
-			"virDomainUUID": data.VmUUID.ValueString(),
-			"actionType":    actionType,
-			"cause":         "INTERNAL",
-		},
-	}
-	diag := utils.ModifyVMPowerState(*r.client, data.VmUUID.ValueString(), createPayload, ctx)
+	diag := utils.ModifyVMPowerState(*r.client, data.VmUUID.ValueString(), actionType, ctx)
 	if diag != nil {
 		resp.Diagnostics.AddWarning(diag.Summary(), diag.Detail())
 	}
@@ -248,14 +241,7 @@ func (r *HypercoreVMPowerStateResource) Update(ctx context.Context, req resource
 	// we need to check with the NEEDED_ACTION_FOR_POWER_STATE.
 	// actionType := utils.NEEDED_ACTION_FOR_POWER_STATE[vmDesiredState]
 	actionType := utils.GetNeededActionForState(vmDesiredState, forceShutoff)
-	updatePayload := []map[string]any{
-		{
-			"virDomainUUID": vmUUID,
-			"actionType":    actionType,
-			"cause":         "INTERNAL",
-		},
-	}
-	diag := utils.ModifyVMPowerState(restClient, vmUUID, updatePayload, ctx)
+	diag := utils.ModifyVMPowerState(restClient, vmUUID, actionType, ctx)
 	if diag != nil {
 		resp.Diagnostics.AddWarning(diag.Summary(), diag.Detail())
 	}

internal/provider/hypercore_vm_resource.go

@@ -6,8 +6,12 @@ package provider
 import (
 	"context"
 	"fmt"
+	"os"
+	"strconv"
+	"time"
 
 	"github.com/hashicorp/terraform-plugin-framework/attr"
+	"github.com/hashicorp/terraform-plugin-framework/diag"
 	"github.com/hashicorp/terraform-plugin-framework/path"
 	"github.com/hashicorp/terraform-plugin-framework/resource"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema"
@@ -393,6 +397,12 @@ func (r *HypercoreVMResource) Delete(ctx context.Context, req resource.DeleteReq
 
 	restClient := *r.client
 	vm_uuid := data.Id.ValueString()
+	err := shutdownVM(ctx, vm_uuid, &restClient)
+	if err != nil {
+		resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to delete shutdown VM, got error: %s", err))
+		return
+	}
+
 	taskTag := restClient.DeleteRecord(
 		fmt.Sprintf("/rest/v1/VirDomain/%s", vm_uuid),
 		-1,
@@ -405,3 +415,91 @@ func (r *HypercoreVMResource) ImportState(ctx context.Context, req resource.Impo
 	tflog.Info(ctx, "TTRT HypercoreVMResource IMPORT_STATE")
 	resource.ImportStatePassthroughID(ctx, path.Root("id"), req, resp)
 }
+
+func shutdownVM(ctx context.Context, vmUUID string, restClient *utils.RestClient) diag.Diagnostic {
+	currentState, err := utils.GetVMPowerState(vmUUID, *restClient)
+	if err != nil {
+		return err
+	}
+	if currentState == "SHUTOFF" {
+		return nil
+	}
+	// VM needs to be shutdown
+
+	// VM shutdown might be already initiated, but not yet fully done.
+	// Send ACPI shutdown if needed.
+	desiredState, err := utils.GetVMDesiredState(vmUUID, *restClient)
+	if err != nil {
+		return err
+	}
+	if desiredState != "SHUTOFF" {
+		err = utils.ModifyVMPowerState(*restClient, vmUUID, "SHUTDOWN", ctx)
+		if err != nil {
+			return err
+		}
+	}
+	var maxWaitTime int = 300
+	envMaxWaitTime := os.Getenv("HC_VM_SHUTDOWN_TIMEOUT")
+	if envMaxWaitTime != "" {
+		val, err := strconv.Atoi(envMaxWaitTime)
+		if err != nil {
+			err_msg := fmt.Sprintf("TTRT HypercoreVMResource Destroy, environ HC_VM_SHUTDOWN_TIMEOUT=%s is not number, err=%s", envMaxWaitTime, err)
+			tflog.Error(ctx, err_msg)
+			var diags diag.Diagnostics
+			diags.AddError(
+				"Invalid environ variable HC_VM_SHUTDOWN_TIMEOUT",
+				err_msg,
+			)
+			return diags[0]
+		}
+		maxWaitTime = val
+	}
+	var waitSleep = 5
+	var waitTime = 0
+	for {
+		// wait on VM to stop
+		currentState, err := utils.GetVMPowerState(vmUUID, *restClient)
+		if err != nil {
+			return err
+		}
+		if currentState == "SHUTOFF" {
+			return nil
+		}
+		if waitTime > maxWaitTime {
+			break
+		}
+		time.Sleep(time.Duration(waitSleep) * time.Second)
+		waitTime += waitSleep
+	}
+
+	// force shutdown is needed
+	tflog.Warn(ctx, "TTRT HypercoreVMResource Destroy, VM is still running after nice ACPI shutdown, VM will be force shutoff.")
+	err = utils.ModifyVMPowerState(*restClient, vmUUID, "STOP", ctx)
+	if err != nil {
+		return err
+	}
+	waitTime = 0
+	for {
+		// wait on VM to stop
+		currentState, err := utils.GetVMPowerState(vmUUID, *restClient)
+		if err != nil {
+			return err
+		}
+		if currentState == "SHUTOFF" {
+			return nil
+		}
+		if waitTime > maxWaitTime {
+			break
+		}
+		time.Sleep(time.Duration(waitSleep) * time.Second)
+		waitTime += waitSleep
+	}
+
+	tflog.Error(ctx, "TTRT HypercoreVMResource Destroy, VM is still running after force shutdown")
+	var diags diag.Diagnostics
+	diags.AddError(
+		"Error Shutting down VM",
+		"Unable to shutdown VM with ACPI shutdown or force shutdown.",
+	)
+	return diags[0]
+}

internal/utils/vm_power_state.go

@@ -33,10 +33,17 @@ func GetNeededActionForState(desiredState string, forceShutoff bool) string {
 func ModifyVMPowerState(
 	restClient RestClient,
 	vmUUID string,
-	payload []map[string]any,
+	actionType string,
 	ctx context.Context,
 ) diag.Diagnostic {
 
+	payload := []map[string]any{
+		{
+			"virDomainUUID": vmUUID,
+			"actionType":    actionType,
+			"cause":         "INTERNAL",
+		},
+	}
 	taskTag, _, err := restClient.CreateRecordWithList(
 		"/rest/v1/VirDomain/action",
 		payload,
@@ -70,19 +77,19 @@ func GetVMPowerState(vmUUID string, restClient RestClient) (string, diag.Diagnos
 	return powerState, nil
 }
 
-func GetVMDesiredState(vmUUID string, restClient RestClient) (*string, diag.Diagnostic) {
+func GetVMDesiredState(vmUUID string, restClient RestClient) (string, diag.Diagnostic) {
 	vm, err := GetOneVMWithError(vmUUID, restClient)
 
 	if err != nil {
-		return nil, diag.NewErrorDiagnostic(
+		return "", diag.NewErrorDiagnostic(
 			"VM not found",
 			err.Error(),
 		)
 	}
 
 	powerState := AnyToString((*vm)["desiredDisposition"])
 
-	return &powerState, nil
+	return powerState, nil
 }
 
 func ValidatePowerState(desiredState string) diag.Diagnostic {

local/main.tf

@@ -9,18 +9,38 @@ terraform {
   }
 }
 
+locals {
+  vm_name = "testtf-remove-running"
+  src_vm_name = "testtf-src-empty"
+}
+
 provider "hypercore" {}
 
-data "hypercore_remote_cluster_connection" "all_clusters" {}
+data "hypercore_vm" "src_empty" {
+  name = local.src_vm_name
+}
 
-data "hypercore_remote_cluster_connection" "cluster-a" {
-  remote_cluster_name = "cluster-a"
+resource "hypercore_vm" "vm_on" {
+  group       = "testtf"
+  name        = local.vm_name
+  description = "VM to be removed"
+  vcpu        = 1
+  memory      = 1234  # MiB
+  clone = {
+    source_vm_uuid = data.hypercore_vm.src_empty.vms.0.uuid
+    meta_data = ""
+    user_data = ""
+  }
 }
 
-output "all_remote_clusters" {
-  value = jsonencode(data.hypercore_remote_cluster_connection.all_clusters)
+resource "hypercore_vm_power_state" "vm_on" {
+  vm_uuid = hypercore_vm.vm_on.id
+  state = "RUNNING"
 }
 
-output "filtered_remote_cluster" {
-  value = jsonencode(data.hypercore_remote_cluster_connection.cluster-a)
+output "vm_on_uuid" {
+  value = hypercore_vm.vm_on.id
+}
+output "power_state" {
+  value = hypercore_vm_power_state.vm_on.state
 }