fix(vm): improve reliability of VM create / get operations (#1431)

* fix(vm): improve reliability of VM create / get operations

- Add retries to GET API calls, fix retrying on POST (VM create) API calls.
- Minor fix in acceptance tests

---------

Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com>

fwprovider/nodes/apt/repo_test.go

@@ -1,3 +1,5 @@
+//go:build acceptance || all
+
 /*
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this

fwprovider/test/test_environment.go

@@ -128,9 +128,9 @@ func (e *Environment) RenderConfig(cfg string) string {
 	tmpl, err := template.New("config").Parse("{{.ProviderConfig}}" + cfg)
 	require.NoError(e.t, err)
 
-	e.templateVars["RandomVMID"] = gofakeit.IntRange(100_000, 1_000_000)
-	e.templateVars["RandomVMID1"] = gofakeit.IntRange(100_000, 1_000_000)
-	e.templateVars["RandomVMID2"] = gofakeit.IntRange(100_000, 1_000_000)
+	e.templateVars["RandomVMID"] = gofakeit.IntRange(100_000, 999_999)
+	e.templateVars["RandomVMID1"] = gofakeit.IntRange(100_000, 999_999)
+	e.templateVars["RandomVMID2"] = gofakeit.IntRange(100_000, 999_999)
 
 	var buf bytes.Buffer
 	err = tmpl.Execute(&buf, e.templateVars)

fwprovider/vm/resource_test.go

@@ -10,10 +10,8 @@ package vm_test
 
 import (
 	"regexp"
-	"strconv"
 	"testing"
 
-	"github.com/brianvoe/gofakeit/v7"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
 
 	"github.com/bpg/terraform-provider-proxmox/fwprovider/test"
@@ -23,10 +21,6 @@ func TestAccResourceVM(t *testing.T) {
 	t.Parallel()
 
 	te := test.InitEnvironment(t)
-	vmID := gofakeit.IntRange(90000, 100000)
-	te.AddTemplateVars(map[string]any{
-		"VMID": vmID,
-	})
 
 	tests := []struct {
 		name  string
@@ -50,15 +44,8 @@ func TestAccResourceVM(t *testing.T) {
 			Config: te.RenderConfig(`
 			resource "proxmox_virtual_environment_vm2" "test_vm" {
 				node_name = "{{.NodeName}}"
-				
-				id = {{.VMID}}
+				id = {{.RandomVMID}}
 			}`),
-			Check: resource.ComposeTestCheckFunc(
-				test.ResourceAttributes("proxmox_virtual_environment_vm2.test_vm", map[string]string{
-					"node_name": te.NodeName,
-					"id":        strconv.Itoa(vmID),
-				}),
-			),
 		}}},
 		{"set an invalid VM name", []resource.TestStep{{
 			Config: te.RenderConfig(`
@@ -207,10 +194,6 @@ func TestAccResourceVM2Clone(t *testing.T) {
 	t.Parallel()
 
 	te := test.InitEnvironment(t)
-	vmID := gofakeit.IntRange(90000, 100000)
-	te.AddTemplateVars(map[string]any{
-		"VMID": vmID,
-	})
 
 	tests := []struct {
 		name  string

proxmox/api/client.go

@@ -19,6 +19,7 @@ import (
 	"sort"
 	"strings"
 
+	"github.com/avast/retry-go/v4"
 	"github.com/google/go-querystring/query"
 	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/logging"
@@ -231,7 +232,22 @@ func (c *client) DoRequest(
 	}
 
 	//nolint:bodyclose
-	res, err := c.conn.httpClient.Do(req)
+	res, err := retry.DoWithData(
+		func() (*http.Response, error) {
+			return c.conn.httpClient.Do(req)
+		},
+		retry.Context(ctx),
+		retry.RetryIf(func(err error) bool {
+			var urlErr *url.Error
+			if errors.As(err, &urlErr) {
+				return strings.ToUpper(urlErr.Op) == http.MethodGet
+			}
+
+			return false
+		}),
+		retry.LastErrorOnly(true),
+		retry.Attempts(3),
+	)
 	if err != nil {
 		return fmt.Errorf("failed to perform HTTP %s request (path: %s) - Reason: %w",
 			method,

proxmox/nodes/vms/vms.go

@@ -79,10 +79,20 @@ func (c *Client) CreateVMAsync(ctx context.Context, d *CreateRequestBody) (*stri
 			return c.DoRequest(ctx, http.MethodPost, c.basePath(), d, resBody)
 		},
 		retry.Context(ctx),
-		retry.RetryIf(func(err error) bool {
-			return strings.Contains(err.Error(), "Reason: got no worker upid")
+		retry.OnRetry(func(n uint, err error) {
+			tflog.Warn(ctx, "retrying VM creation", map[string]interface{}{
+				"attempt": n,
+				"error":   err.Error(),
+			})
+
+			e := c.DoRequest(ctx, http.MethodDelete, c.ExpandPath("?destroy-unreferenced-disks=1&purge=1"), nil, nil)
+			if e != nil {
+				tflog.Warn(ctx, "deleting VM after failed creation", map[string]interface{}{
+					"error": e,
+				})
+			}
 		}),
-		retry.LastErrorOnly(true),
+		retry.LastErrorOnly(false),
 		retry.Attempts(3),
 	)
 	if err != nil {

testacc

@@ -7,4 +7,4 @@
 #
 
 # shellcheck disable=SC2046
-TF_ACC=1 env $(xargs < testacc.env) go test -v -count 1 -timeout 360s -run "$1" github.com/bpg/terraform-provider-proxmox/fwprovider/... $2
+TF_ACC=1 env $(xargs < testacc.env) go test -v -count 1 --tags=acceptance -timeout 360s -run "$1" github.com/bpg/terraform-provider-proxmox/fwprovider/... $2