fix(lxc): cloned container does not start by default (#615)

* fix(lxc): cloned container does not start by default

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

fwprovider/tests/resource_container_test.go

@@ -0,0 +1,126 @@
+/*
+ * 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 tests
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	accTestContainerName      = "proxmox_virtual_environment_container.test_container"
+	accTestContainerCloneName = "proxmox_virtual_environment_container.test_container_clone"
+)
+
+func TestAccResourceContainer(t *testing.T) {
+	t.Parallel()
+
+	accProviders := testAccMuxProviders(context.Background(), t)
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: accProviders,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccResourceContainerCreateConfig(false),
+				Check:  testAccResourceContainerCreateCheck(t),
+			},
+			{
+				Config: testAccResourceContainerCreateConfig(true) + testAccResourceContainerCreateCloneConfig(),
+				Check:  testAccResourceContainerCreateCloneCheck(t),
+			},
+		},
+	})
+}
+
+func testAccResourceContainerCreateConfig(isTemplate bool) string {
+	return fmt.Sprintf(`
+resource "proxmox_virtual_environment_container" "test_container" {
+  node_name = "%s"
+  vm_id     = 1100
+  template  = %t
+
+  disk {
+    datastore_id = "local-lvm"
+    size         = 8
+  }
+
+  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/ubuntu-23.04-standard_23.04-1_amd64.tar.zst"
+    type             = "ubuntu"
+  }
+}
+`, accTestNodeName, isTemplate)
+}
+
+func testAccResourceContainerCreateCheck(t *testing.T) resource.TestCheckFunc {
+	t.Helper()
+
+	return resource.ComposeTestCheckFunc(
+		resource.TestCheckResourceAttr(accTestContainerName, "description", "my\ndescription\nvalue\n"),
+		func(*terraform.State) error {
+			err := getNodesClient().Container(1100).WaitForContainerStatus(context.Background(), "running", 10, 1)
+			require.NoError(t, err, "container did not start")
+			return nil
+		},
+	)
+}
+
+func testAccResourceContainerCreateCloneConfig() string {
+	return fmt.Sprintf(`
+resource "proxmox_virtual_environment_container" "test_container_clone" {
+  depends_on = [proxmox_virtual_environment_container.test_container]
+
+  node_name = "%s"
+  vm_id     = 1101
+
+  clone {
+	vm_id = 1100
+  }
+
+  initialization {
+    hostname = "test-clone"
+  }
+}
+`, accTestNodeName)
+}
+
+func testAccResourceContainerCreateCloneCheck(t *testing.T) resource.TestCheckFunc {
+	t.Helper()
+
+	return resource.ComposeTestCheckFunc(
+		func(*terraform.State) error {
+			err := getNodesClient().Container(1101).WaitForContainerStatus(context.Background(), "running", 10, 1)
+			require.NoError(t, err, "container did not start")
+			return nil
+		},
+	)
+}

fwprovider/tests/resource_linux_vlan_test.go

@@ -25,7 +25,7 @@ func TestAccResourceLinuxVLAN(t *testing.T) {
 
 	accProviders := testAccMuxProviders(context.Background(), t)
 
-	iface := "eno0"
+	iface := "enp6s18"
 	vlan1 := gofakeit.Number(10, 4094)
 	customName := fmt.Sprintf("iface_%s", gofakeit.Word())
 	vlan2 := gofakeit.Number(10, 4094)

fwprovider/tests/test_support.go

@@ -9,6 +9,7 @@ package tests
 import (
 	"context"
 	"fmt"
+	"sync"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-framework/providerserver"
@@ -20,7 +21,10 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/bpg/terraform-provider-proxmox/fwprovider"
+	"github.com/bpg/terraform-provider-proxmox/proxmox/api"
+	"github.com/bpg/terraform-provider-proxmox/proxmox/nodes"
 	sdkV2provider "github.com/bpg/terraform-provider-proxmox/proxmoxtf/provider"
+	"github.com/bpg/terraform-provider-proxmox/utils"
 )
 
 const (
@@ -65,3 +69,40 @@ func testAccMuxProviders(ctx context.Context, t *testing.T) map[string]func() (t
 
 	return muxServers
 }
+
+//nolint:gochecknoglobals
+var (
+	once        sync.Once
+	nodesClient *nodes.Client
+)
+
+func getNodesClient() *nodes.Client {
+	if nodesClient == nil {
+		once.Do(
+			func() {
+				username := utils.GetAnyStringEnv("PROXMOX_VE_USERNAME")
+				password := utils.GetAnyStringEnv("PROXMOX_VE_PASSWORD")
+				endpoint := utils.GetAnyStringEnv("PROXMOX_VE_ENDPOINT")
+				apiToken := utils.GetAnyStringEnv("PROXMOX_VE_API_TOKEN")
+
+				creds, err := api.NewCredentials(username, password, "", apiToken)
+				if err != nil {
+					panic(err)
+				}
+
+				conn, err := api.NewConnection(endpoint, true)
+				if err != nil {
+					panic(err)
+				}
+
+				client, err := api.NewClient(creds, conn)
+				if err != nil {
+					panic(err)
+				}
+
+				nodesClient = &nodes.Client{Client: client, NodeName: accTestNodeName}
+			})
+	}
+
+	return nodesClient
+}

proxmox/nodes/containers/containers.go

@@ -119,11 +119,20 @@ func (c *Client) ShutdownContainer(ctx context.Context, d *ShutdownRequestBody)
 	return nil
 }
 
-// StartContainer starts a container.
+// StartContainer starts a container if is not already running.
 func (c *Client) StartContainer(ctx context.Context, timeout int) error {
+	status, err := c.GetContainerStatus(ctx)
+	if err != nil {
+		return fmt.Errorf("error retrieving container status: %w", err)
+	}
+
+	if status.Status == "running" {
+		return nil
+	}
+
 	taskID, err := c.StartContainerAsync(ctx)
 	if err != nil {
-		return err
+		return fmt.Errorf("error starting container: %w", err)
 	}
 
 	err = c.Tasks().WaitForTask(ctx, *taskID, timeout, 5)
@@ -131,6 +140,12 @@ func (c *Client) StartContainer(ctx context.Context, timeout int) error {
 		return fmt.Errorf("error waiting for container start: %w", err)
 	}
 
+	// the timeout here should probably be configurable
+	err = c.WaitForContainerStatus(ctx, "running", timeout*2, 5)
+	if err != nil {
+		return fmt.Errorf("error waiting for container start: %w", err)
+	}
+
 	return nil
 }
 
@@ -170,9 +185,9 @@ func (c *Client) UpdateContainer(ctx context.Context, d *UpdateRequestBody) erro
 	return nil
 }
 
-// WaitForContainerState waits for a container to reach a specific state.
-func (c *Client) WaitForContainerState(ctx context.Context, state string, timeout int, delay int) error {
-	state = strings.ToLower(state)
+// WaitForContainerStatus waits for a container to reach a specific state.
+func (c *Client) WaitForContainerStatus(ctx context.Context, status string, timeout int, delay int) error {
+	status = strings.ToLower(status)
 
 	timeDelay := int64(delay)
 	timeMax := float64(timeout)
@@ -186,7 +201,7 @@ func (c *Client) WaitForContainerState(ctx context.Context, state string, timeou
 				return fmt.Errorf("error retrieving container status: %w", err)
 			}
 
-			if data.Status == state {
+			if data.Status == status {
 				return nil
 			}
 
@@ -203,9 +218,9 @@ func (c *Client) WaitForContainerState(ctx context.Context, state string, timeou
 	}
 
 	return fmt.Errorf(
-		"timeout while waiting for container \"%d\" to enter the state \"%s\"",
+		"timeout while waiting for container \"%d\" to enter the status \"%s\"",
 		c.VMID,
-		state,
+		status,
 	)
 }
 

proxmoxtf/resource/container.go

@@ -1575,8 +1575,8 @@ func containerCreateStart(ctx context.Context, d *schema.ResourceData, m interfa
 
 	containerAPI := api.Node(nodeName).Container(vmID)
 
-	// A container was started by the create operation, so wait for it to be running.
-	err = containerAPI.WaitForContainerState(ctx, "running", 120, 5)
+	// Start the container and wait for it to reach a running state before continuing.
+	err = containerAPI.StartContainer(ctx, 60)
 	if err != nil {
 		return diag.FromErr(err)
 	}
@@ -2636,11 +2636,6 @@ func containerUpdate(ctx context.Context, d *schema.ResourceData, m interface{})
 			if e != nil {
 				return diag.FromErr(e)
 			}
-
-			e = containerAPI.WaitForContainerState(ctx, "running", 300, 5)
-			if e != nil {
-				return diag.FromErr(e)
-			}
 		} else {
 			forceStop := types.CustomBool(true)
 			shutdownTimeout := 300
@@ -2653,7 +2648,7 @@ func containerUpdate(ctx context.Context, d *schema.ResourceData, m interface{})
 				return diag.FromErr(e)
 			}
 
-			e = containerAPI.WaitForContainerState(ctx, "stopped", 300, 5)
+			e = containerAPI.WaitForContainerStatus(ctx, "stopped", 300, 5)
 			if e != nil {
 				return diag.FromErr(e)
 			}
@@ -2716,7 +2711,7 @@ func containerDelete(ctx context.Context, d *schema.ResourceData, m interface{})
 			return diag.FromErr(err)
 		}
 
-		err = containerAPI.WaitForContainerState(ctx, "stopped", 30, 5)
+		err = containerAPI.WaitForContainerStatus(ctx, "stopped", 30, 5)
 		if err != nil {
 			return diag.FromErr(err)
 		}
@@ -2734,7 +2729,7 @@ func containerDelete(ctx context.Context, d *schema.ResourceData, m interface{})
 	}
 
 	// Wait for the state to become unavailable as that clearly indicates the destruction of the container.
-	err = containerAPI.WaitForContainerState(ctx, "", 60, 2)
+	err = containerAPI.WaitForContainerStatus(ctx, "", 60, 2)
 	if err == nil {
 		return diag.Errorf("failed to delete container \"%d\"", vmID)
 	}