chore: refactor container acc test (#1408)

+ beautify test output on CI

---------

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

.github/workflows/test.yml

@@ -73,11 +73,21 @@ jobs:
         if: ${{ steps.filter.outputs.go == 'true' }}
         run: go mod download
 
+      - name: Set up gotestfmt
+        run: go install github.com/gotesttools/gotestfmt/v2/cmd/gotestfmt@latest
+
       - name: Unit tests
         if: ${{ steps.filter.outputs.go == 'true' }}
         timeout-minutes: 10
-        run: go test -v -cover ./...
+        run: go test -json -v ./... 2>&1 | tee /tmp/gotest.log | gotestfmt -hide empty-packages
+
+      - name: Upload test log
+        uses: actions/upload-artifact@v4
+        if: always()
+        with:
+          name: test-log
+          path: /tmp/gotest.log
+          if-no-files-found: error
 
       - name: Check for uncommitted changes in generated docs
         run: make docs && git diff --exit-code
-

.github/workflows/testacc.yml

@@ -69,4 +69,13 @@ jobs:
           PROXMOX_VE_ACC_NODE_SSH_PORT: ${{ matrix.port }}
           PROXMOX_VE_ACC_CLOUD_IMAGES_SERVER: ${{ secrets.PROXMOX_VE_ACC_CLOUD_IMAGES_SERVER }}
           PROXMOX_VE_ACC_CONTAINER_IMAGES_SERVER: ${{ secrets.PROXMOX_VE_ACC_CONTAINER_IMAGES_SERVER }}
-        run: make testacc
+        run:
+          go test -json --timeout=30m --tags=acceptance -count=1 -v github.com/bpg/terraform-provider-proxmox/fwprovider/... 2>&1 | tee /tmp/gotest.log | gotestfmt -hide empty-packages
+
+      - name: Upload test log
+        uses: actions/upload-artifact@v4
+        if: always()
+        with:
+          name: test-log
+          path: /tmp/gotest.log
+          if-no-files-found: error

.golangci.yml

@@ -11,14 +11,14 @@ issues:
     - EXC0012
     - EXC0014
   exclude-rules:
-    # Exclude duplicate code and function length and complexity checking in test
-    # files (due to common repeats and long functions in test code)
+    # Exclude some checks in tests
     - path: _(test|gen)\.go
       linters:
         - cyclop
         - dupl
-        - gocognit
         - funlen
+        - gocognit
+        - gosec
         - lll
     - path: _types\.go
       linters:

Makefile

@@ -95,7 +95,7 @@ test:
 .PHONY: testacc
 testacc:
 	@# explicitly add TF_ACC=1 to trigger the acceptance tests, `testacc.env` might be missing or incomplete
-	@TF_ACC=1 env $$(cat testacc.env | xargs) go test --timeout=30m -count=1 -v github.com/bpg/terraform-provider-proxmox/fwprovider/...
+	@TF_ACC=1 env $$(cat testacc.env | xargs) go test --timeout=30m --tags=acceptance -count=1 -v github.com/bpg/terraform-provider-proxmox/fwprovider/...
 
 .PHONY: lint
 lint:

fwprovider/access/resource_acl_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/access/resource_user_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/hardwaremapping/resource_hardware_mapping_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/network/resource_linux_bridge_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/network/resource_linux_vlan_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/resource_download_file_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/resource_options_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/datasource_node_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/datasource_version_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/resource_container_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
@@ -13,146 +15,248 @@ import (
 	"testing"
 	"time"
 
+	"github.com/brianvoe/gofakeit/v7"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
 	"github.com/hashicorp/terraform-plugin-testing/terraform"
 	"github.com/stretchr/testify/require"
+
+	"github.com/bpg/terraform-provider-proxmox/proxmox/helpers/ptr"
+	"github.com/bpg/terraform-provider-proxmox/proxmox/nodes/storage"
 )
 
 const (
 	accTestContainerName = "proxmox_virtual_environment_container.test_container"
 )
 
-//nolint:gochecknoglobals
-var (
-	accTestContainerID  = 100000 + rand.Intn(99999) //nolint:gosec
-	accCloneContainerID = 200000 + rand.Intn(99999) //nolint:gosec
-)
-
-func TestAccResourceContainer(t *testing.T) { //nolint:wsl
-	// download fails with 404 or "exit code 8" if run in parallel
-	// t.Parallel()
+func TestAccResourceContainer(t *testing.T) {
+	t.Parallel()
 
 	te := InitEnvironment(t)
 
-	resource.Test(t, resource.TestCase{
-		ProtoV6ProviderFactories: te.AccProviders,
-		Steps: []resource.TestStep{
-			{
-				Config: te.RenderConfig(testAccResourceContainerCreateConfig(te, false)),
-				Check:  testAccResourceContainerCreateCheck(te),
-			},
-			{
-				Config: te.RenderConfig(testAccResourceContainerCreateConfig(te, true) + testAccResourceContainerCreateCloneConfig(te)),
-				Check:  testAccResourceContainerCreateCloneCheck(te),
-			},
-		},
+	imageFileName := gofakeit.Word() + "-ubuntu-23.04-standard_23.04-1_amd64.tar.zst"
+	accTestContainerID := 100000 + rand.Intn(99999)
+	accTestContainerIDClone := 100000 + rand.Intn(99999)
+
+	te.AddTemplateVars(map[string]interface{}{
+		"ImageFileName":        imageFileName,
+		"TestContainerID":      accTestContainerID,
+		"TestContainerIDClone": accTestContainerIDClone,
 	})
-}
-
-func testAccResourceContainerCreateConfig(te *Environment, isTemplate bool) string {
-	te.t.Helper()
-
-	return fmt.Sprintf(`
-resource "proxmox_virtual_environment_download_file" "ubuntu_container_template" {
-	content_type = "vztmpl"
-	datastore_id = "local"
-	node_name = "{{.NodeName}}"
-	url = "{{.ContainerImagesServer}}/images/system/ubuntu-23.04-standard_23.04-1_amd64.tar.zst"
-    overwrite_unmanaged = true
-}
-resource "proxmox_virtual_environment_container" "test_container" {
-  node_name = "{{.NodeName}}"
-  vm_id     = %d
-  template  = %t
-
-  disk {
-    datastore_id = "local-lvm"
-    size         = 4
-  }
-
-  mount_point {
-    volume = "local-lvm"
-    size   = "4G"
-    path   = "mnt/local"
-  }
-
-  description = <<-EOT
-    my
-    description
-    value
-  EOT
-
-  initialization {
-    hostname = "test"
-
-    ip_config {
-      ipv4 {
-        address = "dhcp"
-      }
-    }
-  }
-
-  network_interface {
-    name = "vmbr0"
-  }
-
-  operating_system {
-	template_file_id = proxmox_virtual_environment_download_file.ubuntu_container_template.id
-    type             = "ubuntu"
-  }
-}
-`, accTestContainerID, isTemplate)
-}
-
-func testAccResourceContainerCreateCheck(te *Environment) resource.TestCheckFunc {
-	te.t.Helper()
-
-	return resource.ComposeTestCheckFunc(
-		resource.TestCheckResourceAttr(accTestContainerName, "description", "my\ndescription\nvalue\n"),
-		func(*terraform.State) error {
-			ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
-			defer cancel()
-
-			err := te.NodeClient().Container(accTestContainerID).WaitForContainerStatus(ctx, "running")
-			require.NoError(te.t, err, "container did not start")
-
-			return nil
-		},
-	)
-}
-
-func testAccResourceContainerCreateCloneConfig(te *Environment) string {
-	te.t.Helper()
-
-	return fmt.Sprintf(`
-resource "proxmox_virtual_environment_container" "test_container_clone" {
-  depends_on = [proxmox_virtual_environment_container.test_container]
-  node_name = "{{.NodeName}}"
-  vm_id     = %d
-
-  clone {
-	vm_id = proxmox_virtual_environment_container.test_container.id
-  }
-
-  initialization {
-    hostname = "test-clone"
-  }
-}
-`, accCloneContainerID)
-}
-
-func testAccResourceContainerCreateCloneCheck(te *Environment) resource.TestCheckFunc {
-	te.t.Helper()
-
-	return resource.ComposeTestCheckFunc(
-		func(*terraform.State) error {
-			ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
-			defer cancel()
-
-			err := te.NodeClient().Container(accCloneContainerID).WaitForContainerStatus(ctx, "running")
-			require.NoError(te.t, err, "container did not start")
-
-			return nil
-		},
-	)
+
+	err := te.NodeStorageClient().DownloadFileByURL(context.Background(), &storage.DownloadURLPostRequestBody{
+		Content:  ptr.Ptr("vztmpl"),
+		FileName: ptr.Ptr(imageFileName),
+		Node:     ptr.Ptr(te.NodeName),
+		Storage:  ptr.Ptr(te.DatastoreID),
+		URL:      ptr.Ptr(fmt.Sprintf("%s/images/system/ubuntu-23.04-standard_23.04-1_amd64.tar.zst", te.ContainerImagesServer)),
+	})
+	require.NoError(t, err)
+
+	t.Cleanup(func() {
+		e := te.NodeStorageClient().DeleteDatastoreFile(context.Background(), fmt.Sprintf("vztmpl/%s", imageFileName))
+		require.NoError(t, e)
+	})
+
+	tests := []struct {
+		name string
+		step []resource.TestStep
+	}{
+		{"crete and start container", []resource.TestStep{{
+			Config: te.RenderConfig(`
+			resource "proxmox_virtual_environment_container" "test_container" {
+				node_name = "{{.NodeName}}"
+				vm_id     = {{.TestContainerID}}
+				disk {
+					datastore_id = "local-lvm"
+					size         = 4
+				}
+				mount_point {
+					volume = "local-lvm"
+					size   = "4G"
+					path   = "mnt/local"
+				}
+				description = <<-EOT
+					my
+					description
+					value
+				EOT
+				initialization {
+					hostname = "test"
+					ip_config {
+						ipv4 {
+						  address = "dhcp"
+						}
+					}
+				}
+				network_interface {
+					name = "vmbr0"
+				}
+				operating_system {
+					template_file_id = "local:vztmpl/{{.ImageFileName}}"
+					type             = "ubuntu"
+				}
+			}`),
+			Check: resource.ComposeTestCheckFunc(
+				resource.TestCheckResourceAttr(accTestContainerName, "description", "my\ndescription\nvalue\n"),
+				func(*terraform.State) error {
+					ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+					defer cancel()
+
+					err := te.NodeClient().Container(accTestContainerID).WaitForContainerStatus(ctx, "running")
+					require.NoError(te.t, err, "container did not start")
+
+					return nil
+				},
+			),
+		}}},
+		{"update mount points", []resource.TestStep{
+			{
+				SkipFunc: func() (bool, error) {
+					// mount point deletion: https://github.com/bpg/terraform-provider-proxmox/issues/1392
+					return true, nil
+				},
+				Config: te.RenderConfig(`
+				resource "proxmox_virtual_environment_container" "test_container" {
+				    node_name = "{{.NodeName}}"
+				    vm_id     = {{.RandomVMID}}
+				  	started   = false
+				    disk {
+						datastore_id = "local-lvm"
+						size         = 4
+					}
+				    mount_point {
+						volume = "local-lvm"
+						size   = "4G"
+						path   = "mnt/local1"
+				    }
+					mount_point {
+						volume = "local"
+						size   = "8G"
+						path   = "mnt/local2"
+				    }
+				    initialization {
+				  		hostname = "test"
+						ip_config {
+						  	ipv4 {
+								address = "dhcp"
+						  	}
+						}
+				    }
+				    network_interface {
+				  	    name = "vmbr0"
+				    }
+				    operating_system {
+						template_file_id = "local:vztmpl/{{.ImageFileName}}"
+						type             = "ubuntu"
+				    }
+				}`),
+				Check: ResourceAttributes("proxmox_virtual_environment_container.test_container", map[string]string{
+					"mount_point.#": "2",
+				}),
+			},
+			{
+				SkipFunc: func() (bool, error) {
+					// mount point deletion: https://github.com/bpg/terraform-provider-proxmox/issues/1392
+					return true, nil
+				},
+				Config: te.RenderConfig(`
+				resource "proxmox_virtual_environment_container" "test_container" {
+				    node_name = "{{.NodeName}}"
+				  	started   = false
+				    disk {
+						datastore_id = "local-lvm"
+						size         = 4
+					}
+				    mount_point {
+						volume = "local-lvm"
+						size   = "4G"
+						path   = "mnt/local1"
+				    }
+				    initialization {
+				  		hostname = "test"
+						ip_config {
+						  	ipv4 {
+								address = "dhcp"
+						  	}
+						}
+				    }
+				    network_interface {
+				  	    name = "vmbr0"
+				    }
+				    operating_system {
+						template_file_id = "local:vztmpl/{{.ImageFileName}}"
+						type             = "ubuntu"
+				    }
+				}`),
+				Check: ResourceAttributes("proxmox_virtual_environment_container.test_container", map[string]string{
+					"mount_point.#": "2",
+				}),
+			},
+		}},
+		{"clone container", []resource.TestStep{{
+			Config: te.RenderConfig(`
+			resource "proxmox_virtual_environment_container" "test_container" {
+				node_name = "{{.NodeName}}"
+				vm_id     = {{.RandomVMID}}
+				template  = true
+				disk {
+					datastore_id = "local-lvm"
+					size         = 4
+				}
+				mount_point {
+					volume = "local-lvm"
+					size   = "4G"
+					path   = "mnt/local"
+				}
+				initialization {
+					hostname = "test"
+					ip_config {
+						ipv4 {
+						  address = "dhcp"
+						}
+					}
+				}
+				network_interface {
+					name = "vmbr0"
+				}
+				operating_system {
+					template_file_id = "local:vztmpl/{{.ImageFileName}}"
+					type             = "ubuntu"
+				}
+			}
+			resource "proxmox_virtual_environment_container" "test_container_clone" {
+				depends_on = [proxmox_virtual_environment_container.test_container]
+				node_name  = "{{.NodeName}}"
+				vm_id      = {{.TestContainerIDClone}}
+				
+				clone {
+					vm_id = proxmox_virtual_environment_container.test_container.id
+				}
+				
+				initialization {
+					hostname = "test-clone"
+				}
+			}`),
+			Check: resource.ComposeTestCheckFunc(
+				func(*terraform.State) error {
+					ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+					defer cancel()
+
+					err := te.NodeClient().Container(accTestContainerIDClone).WaitForContainerStatus(ctx, "running")
+					require.NoError(te.t, err, "container did not start")
+
+					return nil
+				},
+			),
+		}}},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			resource.ParallelTest(t, resource.TestCase{
+				ProtoV6ProviderFactories: te.AccProviders,
+				Steps:                    tt.step,
+			})
+		})
+	}
 }

fwprovider/test/resource_file_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/resource_firewall_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/resource_time_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/resource_vm_cdrom_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/resource_vm_disks_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/resource_vm_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

@@ -1,3 +1,9 @@
+/*
+ * 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
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
+
 package test
 
 import (
@@ -36,9 +42,11 @@ type Environment struct {
 	NodeName       string
 	DatastoreID    string
 
-	AccProviders map[string]func() (tfprotov6.ProviderServer, error)
-	once         sync.Once
-	c            api.Client
+	AccProviders          map[string]func() (tfprotov6.ProviderServer, error)
+	once                  sync.Once
+	c                     api.Client
+	CloudImagesServer     string
+	ContainerImagesServer string
 }
 
 // InitEnvironment initializes a new test environment for acceptance tests.
@@ -97,10 +105,12 @@ provider "proxmox" {
 			"CloudImagesServer":     cloudImagesServer,
 			"ContainerImagesServer": containerImagesServer,
 		},
-		providerConfig: pc,
-		NodeName:       nodeName,
-		DatastoreID:    datastoreID,
-		AccProviders:   muxProviders(t),
+		providerConfig:        pc,
+		NodeName:              nodeName,
+		DatastoreID:           datastoreID,
+		AccProviders:          muxProviders(t),
+		CloudImagesServer:     cloudImagesServer,
+		ContainerImagesServer: containerImagesServer,
 	}
 }
 

fwprovider/vm/cdrom/resource_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/vm/cpu/resource_test.go

@@ -1,3 +1,11 @@
+//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
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
+
 package cpu_test
 
 import (

fwprovider/vm/resource_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/vm/vga/resource_test.go

@@ -1,3 +1,11 @@
+//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
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
+
 package vga_test
 
 import (

proxmoxtf/resource/container/container.go

@@ -2831,13 +2831,15 @@ func containerUpdate(ctx context.Context, d *schema.ResourceData, m interface{})
 
 	// Prepare the new mount point configuration.
 	if d.HasChange(mkMountPoint) {
-		mountPoint := d.Get(mkMountPoint).([]interface{})
+		_, newMountPoints := d.GetChange(mkMountPoint)
+
+		mountPoints := newMountPoints.([]interface{})
 		mountPointArray := make(
 			containers.CustomMountPointArray,
-			len(mountPoint),
+			len(mountPoints),
 		)
 
-		for i, mp := range mountPoint {
+		for i, mp := range mountPoints {
 			mountPointMap := mp.(map[string]interface{})
 			mountPointObject := containers.CustomMountPoint{}
 
@@ -2861,7 +2863,7 @@ func containerUpdate(ctx context.Context, d *schema.ResourceData, m interface{})
 			mountPointObject.Volume = volume
 
 			if len(mountOptions) > 0 {
-				mountOptionsArray := make([]string, 0, len(mountPoint))
+				mountOptionsArray := make([]string, 0, len(mountPoints))
 
 				for _, option := range mountOptions {
 					mountOptionsArray = append(mountOptionsArray, option.(string))
@@ -3040,7 +3042,7 @@ func containerUpdate(ctx context.Context, d *schema.ResourceData, m interface{})
 	}
 
 	// As a final step in the update procedure, we might need to reboot the container.
-	if !bool(template) && rebootRequired {
+	if !bool(template) && started && rebootRequired {
 		rebootTimeout := 300
 
 		e = containerAPI.RebootContainer(