From 0b8f2e2c6f80b0370290e6b32ba1e7add977018c Mon Sep 17 00:00:00 2001 From: Pavel Boldyrev <627562+bpg@users.noreply.github.com> Date: Sun, 1 Oct 2023 22:23:11 -0400 Subject: [PATCH] fet(vm): allow `scsi` and `sata` interfaces for CloudInit Drive (#598) * fet(vm): allow `scsi` and `sata` interfaces for CloudInit Drive --------- Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> --- docs/resources/virtual_environment_vm.md | 5 ++-- example/resource_virtual_environment_vm.tf | 10 +++++-- proxmox/nodes/tasks/tasks.go | 34 ++++++++++++++++++++++ proxmox/nodes/tasks/tasks_types.go | 11 +++++++ proxmox/nodes/vms/vms.go | 23 ++++++++++++--- proxmoxtf/resource/validator/vm.go | 13 ++++----- proxmoxtf/resource/vm.go | 24 +++++++++++---- 7 files changed, 99 insertions(+), 21 deletions(-) diff --git a/docs/resources/virtual_environment_vm.md b/docs/resources/virtual_environment_vm.md index 43ac9c6ca..068ee2148 100644 --- a/docs/resources/virtual_environment_vm.md +++ b/docs/resources/virtual_environment_vm.md @@ -300,8 +300,9 @@ output "ubuntu_vm_public_key" { - `datastore_id` - (Optional) The identifier for the datastore to create the cloud-init disk in (defaults to `local-lvm`). - `interface` - (Optional) The hardware interface to connect the cloud-init - image to. Must be `ideN`. Will be detected if the setting is missing but a - cloud-init image is present, otherwise defaults to `ide2`. + image to. Must be one of `ide0..3`, `sata0..5`, `scsi0..30`. Will be + detected if the setting is missing but a cloud-init image is present, + otherwise defaults to `ide2`. - `dns` - (Optional) The DNS configuration. - `domain` - (Optional) The DNS search domain. - `server` - (Optional) The DNS server. diff --git a/example/resource_virtual_environment_vm.tf b/example/resource_virtual_environment_vm.tf index 1a172a698..811302d4c 100644 --- a/example/resource_virtual_environment_vm.tf +++ b/example/resource_virtual_environment_vm.tf @@ -7,6 +7,7 @@ resource "proxmox_virtual_environment_vm" "example_template" { enabled = true } + bios = "ovmf" description = "Managed by Terraform" cpu { @@ -56,7 +57,7 @@ resource "proxmox_virtual_environment_vm" "example_template" { initialization { datastore_id = local.datastore_id - # interface = "ide2" + interface = "scsi4" dns { server = "1.1.1.1" @@ -76,7 +77,8 @@ resource "proxmox_virtual_environment_vm" "example_template" { meta_data_file_id = proxmox_virtual_environment_file.meta_config.id } - name = "terraform-provider-proxmox-example-template" + machine = "q35" + name = "terraform-provider-proxmox-example-template" network_device { mtu = 1450 @@ -113,6 +115,8 @@ resource "proxmox_virtual_environment_vm" "example" { vm_id = proxmox_virtual_environment_vm.example_template.id } + machine = "q35" + memory { dedicated = 768 } @@ -135,7 +139,7 @@ resource "proxmox_virtual_environment_vm" "example" { // if unspecified: // - autodetected if there is a cloud-init device on the template // - otherwise defaults to ide2 - interface = "ide0" + interface = "scsi4" dns { server = "8.8.8.8" diff --git a/proxmox/nodes/tasks/tasks.go b/proxmox/nodes/tasks/tasks.go index d21c683e0..90c88f92d 100644 --- a/proxmox/nodes/tasks/tasks.go +++ b/proxmox/nodes/tasks/tasks.go @@ -43,6 +43,40 @@ func (c *Client) GetTaskStatus(ctx context.Context, upid string) (*GetTaskStatus return resBody.Data, nil } +// GetTaskLog retrieves the log of a task. The log is returned as an array of +// lines. Each line is an object with a line number and the text of the line. +// Reads first 50 lines by default. +func (c *Client) GetTaskLog(ctx context.Context, upid string) ([]string, error) { + resBody := &GetTaskLogResponseBody{} + lines := []string{} + + path, err := c.BuildPath(upid, "log") + if err != nil { + return lines, fmt.Errorf("error building path for task status: %w", err) + } + + err = c.DoRequest( + ctx, + http.MethodGet, + path, + nil, + resBody, + ) + if err != nil { + return lines, fmt.Errorf("error retrieving task status: %w", err) + } + + if resBody.Data == nil { + return lines, api.ErrNoDataObjectInResponse + } + + for _, line := range resBody.Data { + lines = append(lines, line.LineText) + } + + return lines, nil +} + // WaitForTask waits for a specific task to complete. func (c *Client) WaitForTask(ctx context.Context, upid string, timeoutSec, delaySec int) error { timeDelay := int64(delaySec) diff --git a/proxmox/nodes/tasks/tasks_types.go b/proxmox/nodes/tasks/tasks_types.go index 249cb293d..725f6abe5 100644 --- a/proxmox/nodes/tasks/tasks_types.go +++ b/proxmox/nodes/tasks/tasks_types.go @@ -25,6 +25,17 @@ type GetTaskStatusResponseData struct { ExitCode string `json:"exitstatus,omitempty"` } +// GetTaskLogResponseBody contains the body from a node get task log response. +type GetTaskLogResponseBody struct { + Data []*GetTaskLogResponseData `json:"data,omitempty"` +} + +// GetTaskLogResponseData contains the data from a node get task log response. +type GetTaskLogResponseData struct { + LineNumber int `json:"n,omitempty"` + LineText string `json:"t,omitempty"` +} + // TaskID contains the components of a PVE task ID. type TaskID struct { NodeName string diff --git a/proxmox/nodes/vms/vms.go b/proxmox/nodes/vms/vms.go index 79aacb28b..9c3cb3791 100644 --- a/proxmox/nodes/vms/vms.go +++ b/proxmox/nodes/vms/vms.go @@ -330,18 +330,33 @@ func (c *Client) ShutdownVMAsync(ctx context.Context, d *ShutdownRequestBody) (* } // StartVM starts a virtual machine. -func (c *Client) StartVM(ctx context.Context, timeout int) error { +// Returns the task log if the VM had warnings at startup, or fails to start. +func (c *Client) StartVM(ctx context.Context, timeout int) ([]string, error) { taskID, err := c.StartVMAsync(ctx) if err != nil { - return err + return nil, err } err = c.Tasks().WaitForTask(ctx, *taskID, timeout, 5) if err != nil { - return fmt.Errorf("error waiting for VM start: %w", err) + log, e := c.Tasks().GetTaskLog(ctx, *taskID) + if e != nil { + tflog.Error(ctx, "error retrieving task log", map[string]interface{}{ + "task_id": *taskID, + "error": e.Error(), + }) + + log = []string{} + } + + if strings.Contains(err.Error(), "WARNING") && len(log) > 0 { + return log, nil + } + + return log, fmt.Errorf("error waiting for VM start: %w", err) } - return nil + return nil, nil } // StartVMAsync starts a virtual machine asynchronously. diff --git a/proxmoxtf/resource/validator/vm.go b/proxmoxtf/resource/validator/vm.go index 19e260a71..725cb9157 100644 --- a/proxmoxtf/resource/validator/vm.go +++ b/proxmoxtf/resource/validator/vm.go @@ -256,13 +256,12 @@ func IDEInterface() schema.SchemaValidateDiagFunc { // CloudInitInterface is a schema validation function that accepts either an IDE interface identifier or an // empty string, which is used as the default and means "detect which interface should be used automatically". func CloudInitInterface() schema.SchemaValidateDiagFunc { - return validation.ToDiagFunc(validation.StringInSlice([]string{ - "", - "ide0", - "ide1", - "ide2", - "ide3", - }, false)) + r := regexp.MustCompile(`^ide[0-3]|sata[0-5]|scsi(?:30|[12][0-9]|[0-9])$`) + + return validation.ToDiagFunc(validation.Any( + validation.StringIsEmpty, + validation.StringMatch(r, "one of ide0..3|sata0..5|scsi0..30"), + )) } // CloudInitType is a schema validation function for cloud-init types. diff --git a/proxmoxtf/resource/vm.go b/proxmoxtf/resource/vm.go index c3d821dfa..f63c4306d 100644 --- a/proxmoxtf/resource/vm.go +++ b/proxmoxtf/resource/vm.go @@ -1602,16 +1602,26 @@ func deleteIdeDrives(ctx context.Context, vmAPI *vms.Client, itf1 string, itf2 s // Start the VM, then wait for it to actually start; it may not be started immediately if running in HA mode. func vmStart(ctx context.Context, vmAPI *vms.Client, d *schema.ResourceData) diag.Diagnostics { + var diags diag.Diagnostics + tflog.Debug(ctx, "Starting VM") startVMTimeout := d.Get(mkResourceVirtualEnvironmentVMTimeoutStartVM).(int) - e := vmAPI.StartVM(ctx, startVMTimeout) + log, e := vmAPI.StartVM(ctx, startVMTimeout) if e != nil { - return diag.FromErr(e) + return append(diags, diag.FromErr(e)...) + } + + if len(log) > 0 { + lines := "\n\t| " + strings.Join(log, "\n\t| ") + diags = append(diags, diag.Diagnostic{ + Severity: diag.Warning, + Summary: fmt.Sprintf("the VM startup task finished with a warning, task log:\n%s", lines), + }) } - return diag.FromErr(vmAPI.WaitForVMState(ctx, "running", startVMTimeout, 1)) + return append(diags, diag.FromErr(vmAPI.WaitForVMState(ctx, "running", startVMTimeout, 1))...) } // Shutdown the VM, then wait for it to actually shut down (it may not be shut down immediately if @@ -3779,12 +3789,16 @@ func vmReadCustom( diskObjects := getDiskInfo(vmConfig, d) for di, dd := range diskObjects { - disk := map[string]interface{}{} - if dd == nil || dd.FileVolume == "none" || strings.HasPrefix(di, "ide") { continue } + if strings.HasSuffix(dd.FileVolume, fmt.Sprintf("vm-%d-cloudinit", vmID)) { + continue + } + + disk := map[string]interface{}{} + fileIDParts := strings.Split(dd.FileVolume, ":") disk[mkResourceVirtualEnvironmentVMDiskDatastoreID] = fileIDParts[0]