fix(lxc): prevent spurious dns config change when updating initialization block (#1859)

* fix(lxc): prevent spurious `dns` config change when updating `initialization` block

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

docs/resources/virtual_environment_container.md

@@ -39,6 +39,11 @@ resource "proxmox_virtual_environment_container" "ubuntu_container" {
     name = "veth0"
   }
 
+  disk {
+    datastore_id = "local-lvm"
+    size         = 4
+  }
+  
   operating_system {
     template_file_id = proxmox_virtual_environment_download_file.latest_ubuntu_22_jammy_lxc_img.id
     # Or you can use a volume ID, as obtained from a "pvesm list <storage>"

fwprovider/test/resource_container_test.go

@@ -61,11 +61,14 @@ func TestAccResourceContainer(t *testing.T) {
 		name string
 		step []resource.TestStep
 	}{
-		{"create and start container", []resource.TestStep{{
+		{"create, start and update container", []resource.TestStep{
+			{
 				Config: te.RenderConfig(`
 				resource "proxmox_virtual_environment_container" "test_container" {
 					node_name = "{{.NodeName}}"
 					vm_id     = {{.TestContainerID}}
+					timeout_delete = 10
+					unprivileged = true
 					disk {
 						datastore_id = "local-lvm"
 						size         = 4
@@ -100,8 +103,11 @@ func TestAccResourceContainer(t *testing.T) {
 					}
 				}`, WithRootUser()),
 				Check: resource.ComposeTestCheckFunc(
-				resource.TestCheckResourceAttr(accTestContainerName, "description", "my\ndescription\nvalue\n"),
+					ResourceAttributes(accTestContainerName, map[string]string{
-				resource.TestCheckResourceAttr(accTestContainerName, "device_passthrough.#", "1"),
+						"description":            "my\ndescription\nvalue\n",
+						"device_passthrough.#":   "1",
+						"initialization.0.dns.#": "0",
+					}),
 					func(*terraform.State) error {
 						ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 						defer cancel()
@@ -117,7 +123,57 @@ func TestAccResourceContainer(t *testing.T) {
 						return nil
 					},
 				),
-		}}},
+			},
+			{
+				Config: te.RenderConfig(`
+				resource "proxmox_virtual_environment_container" "test_container" {
+					node_name = "{{.NodeName}}"
+					vm_id     = {{.TestContainerID}}
+					timeout_delete = 10
+					unprivileged = true
+					disk {
+						datastore_id = "local-lvm"
+						size         = 4
+					}
+					mount_point {
+						volume = "local-lvm"
+						size   = "4G"
+						path   = "mnt/local"
+					}
+					device_passthrough {
+						path = "/dev/zero"
+					}
+					description = <<-EOT
+						my
+						description
+						value
+					EOT
+					initialization {
+						hostname = "test"
+						ip_config {
+							ipv4 {
+								address = "172.16.10.10/15"
+								gateway = "172.16.0.1"
+							}
+						}
+					}
+					network_interface {
+						name = "vmbr0"
+					}
+					operating_system {
+						template_file_id = "local:vztmpl/{{.ImageFileName}}"
+						type             = "ubuntu"
+					}
+				}`, WithRootUser()),
+				Check: resource.ComposeTestCheckFunc(
+					ResourceAttributes(accTestContainerName, map[string]string{
+						"description":            "my\ndescription\nvalue\n",
+						"device_passthrough.#":   "1",
+						"initialization.0.dns.#": "0",
+					}),
+				),
+			},
+		}},
 		{"update mount points", []resource.TestStep{
 			{
 				Config: te.RenderConfig(`

proxmox/nodes/containers/containers.go

@@ -114,14 +114,35 @@ func (c *Client) RebootContainer(ctx context.Context, d *RebootRequestBody) erro
 
 // ShutdownContainer shuts down a container.
 func (c *Client) ShutdownContainer(ctx context.Context, d *ShutdownRequestBody) error {
-	err := c.DoRequest(ctx, http.MethodPost, c.ExpandPath("status/shutdown"), d, nil)
+	taskID, err := c.ShutdownContainerAsync(ctx, d)
 	if err != nil {
-		return fmt.Errorf("error shutting down container: %w", err)
+		return err
+	}
+
+	err = c.Tasks().WaitForTask(ctx, *taskID)
+	if err != nil {
+		return fmt.Errorf("error waiting for container shut down: %w", err)
 	}
 
 	return nil
 }
 
+// ShutdownContainerAsync shuts down a container asynchronously.
+func (c *Client) ShutdownContainerAsync(ctx context.Context, d *ShutdownRequestBody) (*string, error) {
+	resBody := &ShutdownResponseBody{}
+
+	err := c.DoRequest(ctx, http.MethodPost, c.ExpandPath("status/shutdown"), d, resBody)
+	if err != nil {
+		return nil, fmt.Errorf("error shutting down container: %w", err)
+	}
+
+	if resBody.Data == nil {
+		return nil, api.ErrNoDataObjectInResponse
+	}
+
+	return resBody.Data, nil
+}
+
 // StartContainer starts a container if is not already running.
 func (c *Client) StartContainer(ctx context.Context) error {
 	status, err := c.GetContainerStatus(ctx)

proxmox/nodes/containers/containers_types.go

@@ -159,6 +159,8 @@ type CreateResponseBody struct {
 	Data *string `json:"data,omitempty"`
 }
 
+type ShutdownResponseBody = CreateResponseBody
+
 // GetResponseBody contains the body from a user get response.
 type GetResponseBody struct {
 	Data *GetResponseData `json:"data,omitempty"`

proxmoxtf/resource/container/container.go

@@ -22,6 +22,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 
 	"github.com/bpg/terraform-provider-proxmox/proxmox/api"
+	"github.com/bpg/terraform-provider-proxmox/proxmox/helpers/ptr"
 	"github.com/bpg/terraform-provider-proxmox/proxmox/nodes/containers"
 	"github.com/bpg/terraform-provider-proxmox/proxmox/types"
 	"github.com/bpg/terraform-provider-proxmox/proxmoxtf"
@@ -2997,11 +2998,14 @@ func containerUpdate(ctx context.Context, d *schema.ResourceData, m interface{})
 		}
 	}
 
-	if d.HasChange(mkInitialization) {
+	if d.HasChange(mkInitialization + "." + mkInitializationDNS) {
 		updateBody.DNSDomain = &initializationDNSDomain
 		updateBody.DNSServer = &initializationDNSServer
-		updateBody.Hostname = &initializationHostname
+		rebootRequired = true
+	}
 
+	if d.HasChange(mkInitialization + "." + mkInitializationHostname) {
+		updateBody.Hostname = &initializationHostname
 		rebootRequired = true
 	}
 
@@ -3279,11 +3283,16 @@ func containerUpdate(ctx context.Context, d *schema.ResourceData, m interface{})
 			}
 		} else {
 			forceStop := types.CustomBool(true)
-			shutdownTimeout := 300
+			// Using delete timeout here as we're in the similar situation
+			// as in the delete function, where we need to wait for the container
+			// to be stopped before we can proceed with the update.
+			// see `containerDelete` function for more details about the logic here
+			// Needs to be refactored to a common function
+			shutdownTimeoutSec := max(1, d.Get(mkTimeoutDelete).(int)-5)
 
 			e = containerAPI.ShutdownContainer(ctx, &containers.ShutdownRequestBody{
 				ForceStop: &forceStop,
-				Timeout:   &shutdownTimeout,
+				Timeout:   &shutdownTimeoutSec,
 			})
 			if e != nil {
 				return diag.FromErr(e)
@@ -3351,7 +3360,9 @@ func containerDelete(ctx context.Context, d *schema.ResourceData, m interface{})
 			ctx,
 			&containers.ShutdownRequestBody{
 				ForceStop: &forceStop,
-				Timeout:   &deleteTimeoutSec,
+				// the timeout here must be less that the context timeout set above,
+				// otherwise the context will be cancelled before PVE forcefully stops the container
+				Timeout: ptr.Ptr(max(1, deleteTimeoutSec-5)),
 			},
 		)
 		if err != nil {