Repository: hashicorp/packer-plugin-proxmox (language: go)

Issue:
Client default timeout needs adjustment for longer clones
I believe the default timeout period (30 sec) is being used to wait for completion of a full clone.  When clones take longer than this, I'm seeing timeout errors, I'm guessing this is because my proxmox server is slower or the disks being cloned are relatively large.  When not performing a full_clone, the timeout is not reached and the plugin behaves normally.  Clones in my case take 90 seconds.

Here is where the default is set:
https://github.com/hashicorp/packer-plugin-proxmox/blob/de55cac730fd9626080569bde50c89ae871b2031/builder/proxmox/common/client.go#L11

Which eventually gets passed here where the error occurs:
https://github.com/Telmate/proxmox-api-go/blob/51bd4ae21d9a8ecdd7f040c48c240756616c7d08/proxmox/client.go#L577

Ideally I could imagine this being broken out as an explicit configuration parameter.  A more expedient fix is to just increase the default timeout.

Packer v1.7.2
packer-plugin-proxmox 0.0.2

Relevant log fragment:

```
==> proxmox-clone.autogenerated_1: Creating VM
==> proxmox-clone.autogenerated_1: No VM ID given, getting next free from Proxmox
2021/06/06 10:32:44 packer-builder-proxmox-clone plugin: 500 can't lock file '/var/lock/qemu-server/lock-105.conf' - got timeout
==> proxmox-clone.autogenerated_1: Error creating VM: 500 can't lock file '/var/lock/qemu-server/lock-105.conf' - got timeout
2021/06/06 10:32:44 [INFO] (telemetry) ending 
==> Wait completed after 40 seconds 622 milliseconds
2021/06/06 10:32:44 machine readable: error-count []string{"1"}
==> Some builds didn't complete successfully and had errors:
2021/06/06 10:32:44 machine readable: proxmox-clone.autogenerated_1,error []string{"Error creating VM: 500 can't lock file '/var/lock/qemu-server/lock-105.conf' - got timeout"}
==> Builds finished but no artifacts were created.
2021/06/06 10:32:44 [INFO] (telemetry) Finalizing.
Build 'proxmox-clone.autogenerated_1' errored after 40 seconds 622 milliseconds: Error creating VM: 500 can't lock file '/var/lock/qemu-server/lock-105.conf' - got timeout

==> Wait completed after 40 seconds 622 milliseconds

==> Some builds didn't complete successfully and had errors:
--> proxmox-clone.autogenerated_1: Error creating VM: 500 can't lock file '/var/lock/qemu-server/lock-105.conf' - got timeout

==> Builds finished but no artifacts were created.
```

I'm having trouble explaining why the error occurs consistently after 40 seconds and not 30 seconds.  It is possible I'm mixing up the timeout at fault here.
Client default timeout needs adjustment for longer clones
I believe the default timeout period (30 sec) is being used to wait for completion of a full clone.  When clones take longer than this, I'm seeing timeout errors, I'm guessing this is because my proxmox server is slower or the disks being cloned are relatively large.  When not performing a full_clone, the timeout is not reached and the plugin behaves normally.  Clones in my case take 90 seconds.

Here is where the default is set:
https://github.com/hashicorp/packer-plugin-proxmox/blob/de55cac730fd9626080569bde50c89ae871b2031/builder/proxmox/common/client.go#L11

Which eventually gets passed here where the error occurs:
https://github.com/Telmate/proxmox-api-go/blob/51bd4ae21d9a8ecdd7f040c48c240756616c7d08/proxmox/client.go#L577

Ideally I could imagine this being broken out as an explicit configuration parameter.  A more expedient fix is to just increase the default timeout.

Packer v1.7.2
packer-plugin-proxmox 0.0.2

Relevant log fragment:

```
==> proxmox-clone.autogenerated_1: Creating VM
==> proxmox-clone.autogenerated_1: No VM ID given, getting next free from Proxmox
2021/06/06 10:32:44 packer-builder-proxmox-clone plugin: 500 can't lock file '/var/lock/qemu-server/lock-105.conf' - got timeout
==> proxmox-clone.autogenerated_1: Error creating VM: 500 can't lock file '/var/lock/qemu-server/lock-105.conf' - got timeout
2021/06/06 10:32:44 [INFO] (telemetry) ending 
==> Wait completed after 40 seconds 622 milliseconds
2021/06/06 10:32:44 machine readable: error-count []string{"1"}
==> Some builds didn't complete successfully and had errors:
2021/06/06 10:32:44 machine readable: proxmox-clone.autogenerated_1,error []string{"Error creating VM: 500 can't lock file '/var/lock/qemu-server/lock-105.conf' - got timeout"}
==> Builds finished but no artifacts were created.
2021/06/06 10:32:44 [INFO] (telemetry) Finalizing.
Build 'proxmox-clone.autogenerated_1' errored after 40 seconds 622 milliseconds: Error creating VM: 500 can't lock file '/var/lock/qemu-server/lock-105.conf' - got timeout

==> Wait completed after 40 seconds 622 milliseconds

==> Some builds didn't complete successfully and had errors:
--> proxmox-clone.autogenerated_1: Error creating VM: 500 can't lock file '/var/lock/qemu-server/lock-105.conf' - got timeout

==> Builds finished but no artifacts were created.
```

I'm having trouble explaining why the error occurs consistently after 40 seconds and not 30 seconds.  It is possible I'm mixing up the timeout at fault here.


Discussion / hints:
Hey @phelpsw,
Thanks for reporting! I checked other builders, and there doesn't seem to be an idiom for having granular timeouts, so just having an overall timeout should follow the existing user experience best, I think.
Would you be up for making a PR that adds this as a parameter?
Hey @phelpsw,
Thanks for reporting! I checked other builders, and there doesn't seem to be an idiom for having granular timeouts, so just having an overall timeout should follow the existing user experience best, I think.
Would you be up for making a PR that adds this as a parameter?

Fix patch:
diff --git a/builder/proxmox/clone/config.hcl2spec.go b/builder/proxmox/clone/config.hcl2spec.go
index 829ef9eb..1a6b1927 100644
--- a/builder/proxmox/clone/config.hcl2spec.go
+++ b/builder/proxmox/clone/config.hcl2spec.go
@@ -85,6 +85,7 @@ type FlatConfig struct {
 	Token                     *string                            `mapstructure:"token" cty:"token" hcl:"token"`
 	Node                      *string                            `mapstructure:"node" cty:"node" hcl:"node"`
 	Pool                      *string                            `mapstructure:"pool" cty:"pool" hcl:"pool"`
+	TaskTimeout               *string                            `mapstructure:"task_timeout" cty:"task_timeout" hcl:"task_timeout"`
 	VMName                    *string                            `mapstructure:"vm_name" cty:"vm_name" hcl:"vm_name"`
 	VMID                      *int                               `mapstructure:"vm_id" cty:"vm_id" hcl:"vm_id"`
 	Boot                      *string                            `mapstructure:"boot" cty:"boot" hcl:"boot"`
@@ -196,6 +197,7 @@ func (*FlatConfig) HCL2Spec() map[string]hcldec.Spec {
 		"token":                        &hcldec.AttrSpec{Name: "token", Type: cty.String, Required: false},
 		"node":                         &hcldec.AttrSpec{Name: "node", Type: cty.String, Required: false},
 		"pool":                         &hcldec.AttrSpec{Name: "pool", Type: cty.String, Required: false},
+		"task_timeout":                 &hcldec.AttrSpec{Name: "task_timeout", Type: cty.String, Required: false},
 		"vm_name":                      &hcldec.AttrSpec{Name: "vm_name", Type: cty.String, Required: false},
 		"vm_id":                        &hcldec.AttrSpec{Name: "vm_id", Type: cty.Number, Required: false},
 		"boot":                         &hcldec.AttrSpec{Name: "boot", Type: cty.String, Required: false},
diff --git a/builder/proxmox/common/client.go b/builder/proxmox/common/client.go
index e70f8af5..906e8df1 100644
--- a/builder/proxmox/common/client.go
+++ b/builder/proxmox/common/client.go
@@ -3,19 +3,16 @@ package proxmox
 import (
 	"crypto/tls"
 	"log"
-	"time"
 
 	"github.com/Telmate/proxmox-api-go/proxmox"
 )
 
-const defaultTaskTimeout = 30 * time.Second
-
 func newProxmoxClient(config Config) (*proxmox.Client, error) {
 	tlsConfig := &tls.Config{
 		InsecureSkipVerify: config.SkipCertValidation,
 	}
 
-	client, err := proxmox.NewClient(config.proxmoxURL.String(), nil, tlsConfig, int(defaultTaskTimeout.Seconds()))
+	client, err := proxmox.NewClient(config.proxmoxURL.String(), nil, tlsConfig, int(config.TaskTimeout.Seconds()))
 	if err != nil {
 		return nil, err
 	}
diff --git a/builder/proxmox/common/config.go b/builder/proxmox/common/config.go
index 375070de..7da9a618 100644
--- a/builder/proxmox/common/config.go
+++ b/builder/proxmox/common/config.go
@@ -40,6 +40,7 @@ type Config struct {
 	Token              string `mapstructure:"token"`
 	Node               string `mapstructure:"node"`
 	Pool               string `mapstructure:"pool"`
+	TaskTimeout        time.Duration `mapstructure:"task_timeout"`
 
 	VMName string `mapstructure:"vm_name"`
 	VMID   int    `mapstructure:"vm_id"`
@@ -143,6 +144,9 @@ func (c *Config) Prepare(upper interface{}, raws ...interface{}) ([]string, []st
 	if c.Token == "" {
 		c.Token = os.Getenv("PROXMOX_TOKEN")
 	}
+	if c.TaskTimeout == 0 {
+		c.TaskTimeout = 60 * time.Second
+	}
 	if c.BootKeyInterval == 0 && os.Getenv(bootcommand.PackerKeyEnv) != "" {
 		var err error
 		c.BootKeyInterval, err = time.ParseDuration(os.Getenv(bootcommand.PackerKeyEnv))
diff --git a/builder/proxmox/common/config.hcl2spec.go b/builder/proxmox/common/config.hcl2spec.go
index cb4845f0..36fa4c8b 100644
--- a/builder/proxmox/common/config.hcl2spec.go
+++ b/builder/proxmox/common/config.hcl2spec.go
@@ -84,6 +84,7 @@ type FlatConfig struct {
 	Token                     *string                    `mapstructure:"token" cty:"token" hcl:"token"`
 	Node                      *string                    `mapstructure:"node" cty:"node" hcl:"node"`
 	Pool                      *string                    `mapstructure:"pool" cty:"pool" hcl:"pool"`
+	TaskTimeout               *string                    `mapstructure:"task_timeout" cty:"task_timeout" hcl:"task_timeout"`
 	VMName                    *string                    `mapstructure:"vm_name" cty:"vm_name" hcl:"vm_name"`
 	VMID                      *int                       `mapstructure:"vm_id" cty:"vm_id" hcl:"vm_id"`
 	Boot                      *string                    `mapstructure:"boot" cty:"boot" hcl:"boot"`
@@ -193,6 +194,7 @@ func (*FlatConfig) HCL2Spec() map[string]hcldec.Spec {
 		"token":                        &hcldec.AttrSpec{Name: "token", Type: cty.String, Required: false},
 		"node":                         &hcldec.AttrSpec{Name: "node", Type: cty.String, Required: false},
 		"pool":                         &hcldec.AttrSpec{Name: "pool", Type: cty.String, Required: false},
+		"task_timeout":                 &hcldec.AttrSpec{Name: "task_timeout", Type: cty.String, Required: false},
 		"vm_name":                      &hcldec.AttrSpec{Name: "vm_name", Type: cty.String, Required: false},
 		"vm_id":                        &hcldec.AttrSpec{Name: "vm_id", Type: cty.Number, Required: false},
 		"boot":                         &hcldec.AttrSpec{Name: "boot", Type: cty.String, Required: false},
diff --git a/builder/proxmox/iso/config.hcl2spec.go b/builder/proxmox/iso/config.hcl2spec.go
index d1a20d90..07a5e009 100644
--- a/builder/proxmox/iso/config.hcl2spec.go
+++ b/builder/proxmox/iso/config.hcl2spec.go
@@ -85,6 +85,7 @@ type FlatConfig struct {
 	Token                     *string                            `mapstructure:"token" cty:"token" hcl:"token"`
 	Node                      *string                            `mapstructure:"node" cty:"node" hcl:"node"`
 	Pool                      *string                            `mapstructure:"pool" cty:"pool" hcl:"pool"`
+	TaskTimeout               *string                            `mapstructure:"task_timeout" cty:"task_timeout" hcl:"task_timeout"`
 	VMName                    *string                            `mapstructure:"vm_name" cty:"vm_name" hcl:"vm_name"`
 	VMID                      *int                               `mapstructure:"vm_id" cty:"vm_id" hcl:"vm_id"`
 	Boot                      *string                            `mapstructure:"boot" cty:"boot" hcl:"boot"`
@@ -202,6 +203,7 @@ func (*FlatConfig) HCL2Spec() map[string]hcldec.Spec {
 		"token":                        &hcldec.AttrSpec{Name: "token", Type: cty.String, Required: false},
 		"node":                         &hcldec.AttrSpec{Name: "node", Type: cty.String, Required: false},
 		"pool":                         &hcldec.AttrSpec{Name: "pool", Type: cty.String, Required: false},
+		"task_timeout":                 &hcldec.AttrSpec{Name: "task_timeout", Type: cty.String, Required: false},
 		"vm_name":                      &hcldec.AttrSpec{Name: "vm_name", Type: cty.String, Required: false},
 		"vm_id":                        &hcldec.AttrSpec{Name: "vm_id", Type: cty.Number, Required: false},
 		"boot":                         &hcldec.AttrSpec{Name: "boot", Type: cty.String, Required: false},
diff --git a/docs-partials/builder/proxmox/common/Config-not-required.mdx b/docs-partials/builder/proxmox/common/Config-not-required.mdx
index a74420c9..d4466bd1 100644
--- a/docs-partials/builder/proxmox/common/Config-not-required.mdx
+++ b/docs-partials/builder/proxmox/common/Config-not-required.mdx
@@ -16,6 +16,8 @@
 
 - `pool` (string) - Pool
 
+- `task_timeout` (duration string | ex: "1h5m2s") - Task Timeout
+
 - `vm_name` (string) - VM Name
 
 - `vm_id` (int) - VMID
@@ -38,7 +40,7 @@
 
 - `disks` ([]diskConfig) - Disks
 
-- `qemu_agent` (bool) - Agent
+- `qemu_agent` (boolean) - Agent
 
 - `scsi_controller` (string) - SCSI Controller
 
diff --git a/docs/builders/clone.mdx b/docs/builders/clone.mdx
index bbf594ee..b4d55e1c 100644
--- a/docs/builders/clone.mdx
+++ b/docs/builders/clone.mdx
@@ -68,6 +68,9 @@ in the image's Cloud-Init settings for provisioning.
 
 - `insecure_skip_tls_verify` (bool) - Skip validating the certificate.
 
+- `task_timeout` (duration string | ex: "10m") - The timeout for
+  Promox API operations, e.g. clones. Defaults to 1 minute.
+
 - `pool` (string) - Name of resource pool to create virtual machine in.
 
 - `vm_name` (string) - Name of the virtual machine during creation. If not


Test patch: