Add timeout blocks for Terraform resources (#97)

* add timeouts By convention (https://developer.hashicorp.com/terraform/language/resources/syntax#operation-timeouts) terraform resources can have timeouts associated with their operations through `timeout` blocks. This change sets the default timeout to 2 minutes, except where other defaults were hard-coded before and allows users to specify timeout blocks without causing parsing errors. Use the contexts to setup and propagate deadlines for each operation according to what is set in the terraform file. Since each API call happens within the same context, the same deadline applies, giving each operation (create, update, read, delete) as a whole the configured timeout rather than setting this timeout on individual API calls of the operation. Where applicable, the timeout of resources is measured against the observed changes of the API resources or events rather than completion of API calls. This is particularly important for resources where operations can take a lot longer than the API call. E.g. for the Delete operation on a Volume, the time until a matching event is observed is counted. For the Update operation of a VirtualMachine, the time (if necessary) until the VM resource has been restarted and is observed in running state is counted. For a contrast, the Delete operation on a storage class happens almost immediately and the timeout is only counted against the API call. fixes: harvester/harvester#5632 Signed-off-by: Moritz Röhrich <[email protected]> * cloud-init secret: no wait on delete Don't wait for a state change when deleting a cloud-init secret. The cloud-init secret is just a normal core/v1 Secret API object. These don't have a state, they either exist or they don't but there is not any intermediate state like the is with a VM or a Pod. As a consequence, it's not necessary to wait for a state change when deleting a secret. Signed-off-by: Moritz Röhrich <[email protected]> --------- Signed-off-by: Moritz Röhrich <[email protected]>
harvester · Jun 4, 2024 · 3469cb2 · 3469cb2
1 parent 4e07ed0
commit 3469cb2
Show file tree

Hide file tree

Showing 12 changed files with 135 additions and 47 deletions.
diff --git a/internal/provider/cloudinitsecret/resource_cloudinitsecret.go b/internal/provider/cloudinitsecret/resource_cloudinitsecret.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/base64"
 	"fmt"
+	"time"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -28,6 +29,13 @@ func ResourceCloudInitSecret() *schema.Resource {
 			StateContext: schema.ImportStatePassthroughContext,
 		},
 		Schema: Schema(),
+		Timeouts: &schema.ResourceTimeout{
+			Create:  schema.DefaultTimeout(2 * time.Minute),
+			Read:    schema.DefaultTimeout(2 * time.Minute),
+			Update:  schema.DefaultTimeout(2 * time.Minute),
+			Delete:  schema.DefaultTimeout(2 * time.Minute),
+			Default: schema.DefaultTimeout(2 * time.Minute),
+		},
 	}
 }
 
@@ -100,6 +108,7 @@ func resourceCloudInitSecretDelete(ctx context.Context, d *schema.ResourceData,
 	if err != nil && !apierrors.IsNotFound(err) {
 		return diag.FromErr(err)
 	}
+
 	d.SetId("")
 	return nil
 }

diff --git a/internal/provider/clusternetwork/resource_clusternetwork.go b/internal/provider/clusternetwork/resource_clusternetwork.go
@@ -3,6 +3,7 @@ package clusternetwork
 import (
 	"context"
 	"fmt"
+	"time"
 
 	harvsternetworkv1 "github.com/harvester/harvester-network-controller/pkg/apis/network.harvesterhci.io/v1beta1"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
@@ -27,6 +28,13 @@ func ResourceClusterNetwork() *schema.Resource {
 			StateContext: schema.ImportStatePassthroughContext,
 		},
 		Schema: Schema(),
+		Timeouts: &schema.ResourceTimeout{
+			Create:  schema.DefaultTimeout(2 * time.Minute),
+			Read:    schema.DefaultTimeout(2 * time.Minute),
+			Update:  schema.DefaultTimeout(2 * time.Minute),
+			Delete:  schema.DefaultTimeout(2 * time.Minute),
+			Default: schema.DefaultTimeout(2 * time.Minute),
+		},
 	}
 }
 

diff --git a/internal/provider/image/resource_image.go b/internal/provider/image/resource_image.go
@@ -29,6 +29,13 @@ func ResourceImage() *schema.Resource {
 			StateContext: schema.ImportStatePassthroughContext,
 		},
 		Schema: Schema(),
+		Timeouts: &schema.ResourceTimeout{
+			Create:  schema.DefaultTimeout(5 * time.Minute),
+			Read:    schema.DefaultTimeout(2 * time.Minute),
+			Update:  schema.DefaultTimeout(5 * time.Minute),
+			Delete:  schema.DefaultTimeout(2 * time.Minute),
+			Default: schema.DefaultTimeout(2 * time.Minute),
+		},
 	}
 }
 
@@ -100,6 +107,20 @@ func resourceImageDelete(ctx context.Context, d *schema.ResourceData, meta inter
 	if err != nil && !apierrors.IsNotFound(err) {
 		return diag.FromErr(err)
 	}
+
+	stateConf := &resource.StateChangeConf{
+		Pending:    []string{constants.StateImageTerminating, constants.StateCommonActive},
+		Target:     []string{constants.StateCommonRemoved},
+		Refresh:    resourceImageRefresh(ctx, d, meta),
+		Timeout:    d.Timeout(schema.TimeoutDelete),
+		Delay:      1 * time.Second,
+		MinTimeout: 3 * time.Second,
+	}
+	_, err = stateConf.WaitForStateContext(ctx)
+	if err != nil {
+		return diag.FromErr(err)
+	}
+
 	d.SetId("")
 	return nil
 }

diff --git a/internal/provider/keypair/resource_keypair.go b/internal/provider/keypair/resource_keypair.go
@@ -2,6 +2,7 @@ package keypair
 
 import (
 	"context"
+	"time"
 
 	harvsterv1 "github.com/harvester/harvester/pkg/apis/harvesterhci.io/v1beta1"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
@@ -26,6 +27,13 @@ func ResourceKeypair() *schema.Resource {
 			StateContext: schema.ImportStatePassthroughContext,
 		},
 		Schema: Schema(),
+		Timeouts: &schema.ResourceTimeout{
+			Create:  schema.DefaultTimeout(2 * time.Minute),
+			Read:    schema.DefaultTimeout(2 * time.Minute),
+			Update:  schema.DefaultTimeout(2 * time.Minute),
+			Delete:  schema.DefaultTimeout(2 * time.Minute),
+			Default: schema.DefaultTimeout(2 * time.Minute),
+		},
 	}
 }
 

diff --git a/internal/provider/network/resource_network.go b/internal/provider/network/resource_network.go
@@ -3,6 +3,7 @@ package network
 import (
 	"context"
 	"fmt"
+	"time"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -17,10 +18,6 @@ import (
 	"github.com/harvester/terraform-provider-harvester/pkg/importer"
 )
 
-const (
-	networkDeleteTimeout = 300
-)
-
 func ResourceNetwork() *schema.Resource {
 	return &schema.Resource{
 		CreateContext: resourceNetworkCreate,
@@ -31,6 +28,13 @@ func ResourceNetwork() *schema.Resource {
 			StateContext: schema.ImportStatePassthroughContext,
 		},
 		Schema: Schema(),
+		Timeouts: &schema.ResourceTimeout{
+			Create:  schema.DefaultTimeout(2 * time.Minute),
+			Read:    schema.DefaultTimeout(2 * time.Minute),
+			Update:  schema.DefaultTimeout(2 * time.Minute),
+			Delete:  schema.DefaultTimeout(5 * time.Minute),
+			Default: schema.DefaultTimeout(2 * time.Minute),
+		},
 	}
 }
 
@@ -101,7 +105,12 @@ func resourceNetworkDelete(ctx context.Context, d *schema.ResourceData, meta int
 	if err = c.HarvesterClient.K8sCniCncfIoV1().NetworkAttachmentDefinitions(namespace).Delete(ctx, name, metav1.DeleteOptions{}); err != nil && !apierrors.IsNotFound(err) {
 		return diag.FromErr(err)
 	}
-	events, err := c.HarvesterClient.K8sCniCncfIoV1().NetworkAttachmentDefinitions(namespace).Watch(ctx, util.WatchOptions(name, int64(networkDeleteTimeout)))
+
+	ctxDeadline, _ := ctx.Deadline()
+	events, err := c.HarvesterClient.
+		K8sCniCncfIoV1().
+		NetworkAttachmentDefinitions(namespace).
+		Watch(ctx, util.WatchOptions(name, time.Until(ctxDeadline)))
 	if err != nil {
 		return diag.FromErr(err)
 	}

diff --git a/internal/provider/provider.go b/internal/provider/provider.go
@@ -61,34 +61,35 @@ func Provider() *schema.Provider {
 			constants.ResourceTypeVLANConfig:      vlanconfig.ResourceVLANConfig(),
 			constants.ResourceTypeCloudInitSecret: cloudinitsecret.ResourceCloudInitSecret(),
 		},
+		ConfigureContextFunc: providerConfig,
 	}
-	p.ConfigureContextFunc = configure(p)
 	return p
 }
 
-func configure(p *schema.Provider) schema.ConfigureContextFunc {
-	return func(ctx context.Context, d *schema.ResourceData) (interface{}, diag.Diagnostics) {
-		kubeContext := d.Get(constants.FieldProviderKubeContext).(string)
-		kubeConfig, err := homedir.Expand(d.Get(constants.FieldProviderKubeConfig).(string))
-		if err != nil {
-			return nil, diag.FromErr(err)
-		}
-
-		c, err := client.NewClient(kubeConfig, kubeContext)
-		if err != nil {
-			return nil, diag.FromErr(err)
-		}
+func providerConfig(ctx context.Context, d *schema.ResourceData) (interface{}, diag.Diagnostics) {
+	kubeContext := d.Get(constants.FieldProviderKubeContext).(string)
+	kubeConfig, err := homedir.Expand(d.Get(constants.FieldProviderKubeConfig).(string))
+	if err != nil {
+		return nil, diag.FromErr(err)
+	}
 
-		// check harvester version from settings
-		serverVersion, err := c.HarvesterClient.HarvesterhciV1beta1().Settings().Get(context.Background(), "server-version", metav1.GetOptions{})
-		if err != nil {
-			return nil, diag.FromErr(err)
-		}
-		// harvester version v1.0-head, v1.0.2, v1.0.3 is not supported
-		if strings.HasPrefix(serverVersion.Value, "v1.0") {
-			return nil, diag.FromErr(fmt.Errorf("current Harvester server version is %s, the minimum supported version is v1.1.0", serverVersion.Value))
-		}
+	c, err := client.NewClient(kubeConfig, kubeContext)
+	if err != nil {
+		return nil, diag.FromErr(err)
+	}
 
-		return c, nil
+	// check harvester version from settings
+	serverVersion, err := c.HarvesterClient.
+		HarvesterhciV1beta1().
+		Settings().
+		Get(ctx, "server-version", metav1.GetOptions{})
+	if err != nil {
+		return nil, diag.FromErr(err)
 	}
+	// harvester version v1.0-head, v1.0.2, v1.0.3 is not supported
+	if strings.HasPrefix(serverVersion.Value, "v1.0") {
+		return nil, diag.FromErr(fmt.Errorf("current Harvester server version is %s, the minimum supported version is v1.1.0", serverVersion.Value))
+	}
+
+	return c, nil
 }
diff --git a/internal/provider/storageclass/resource_storageclass.go b/internal/provider/storageclass/resource_storageclass.go
@@ -2,6 +2,7 @@ package storageclass
 
 import (
 	"context"
+	"time"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -26,6 +27,13 @@ func ResourceStorageClass() *schema.Resource {
 			StateContext: schema.ImportStatePassthroughContext,
 		},
 		Schema: Schema(),
+		Timeouts: &schema.ResourceTimeout{
+			Create:  schema.DefaultTimeout(2 * time.Minute),
+			Read:    schema.DefaultTimeout(2 * time.Minute),
+			Update:  schema.DefaultTimeout(2 * time.Minute),
+			Delete:  schema.DefaultTimeout(2 * time.Minute),
+			Default: schema.DefaultTimeout(2 * time.Minute),
+		},
 	}
 }
 

diff --git a/internal/provider/virtualmachine/resource_virtualmachine.go b/internal/provider/virtualmachine/resource_virtualmachine.go
@@ -21,10 +21,6 @@ import (
 	"github.com/harvester/terraform-provider-harvester/pkg/importer"
 )
 
-const (
-	vmDeleteTimeout = 300
-)
-
 func ResourceVirtualMachine() *schema.Resource {
 	return &schema.Resource{
 		CreateContext: resourceVirtualMachineCreate,
@@ -35,6 +31,13 @@ func ResourceVirtualMachine() *schema.Resource {
 			StateContext: schema.ImportStatePassthroughContext,
 		},
 		Schema: Schema(),
+		Timeouts: &schema.ResourceTimeout{
+			Create:  schema.DefaultTimeout(2 * time.Minute),
+			Read:    schema.DefaultTimeout(2 * time.Minute),
+			Update:  schema.DefaultTimeout(2 * time.Minute),
+			Delete:  schema.DefaultTimeout(5 * time.Minute),
+			Default: schema.DefaultTimeout(2 * time.Minute),
+		},
 	}
 }
 
@@ -173,7 +176,12 @@ func resourceVirtualMachineDelete(ctx context.Context, d *schema.ResourceData, m
 	if err = c.HarvesterClient.KubevirtV1().VirtualMachines(namespace).Delete(ctx, name, deleteOptions); err != nil && !apierrors.IsNotFound(err) {
 		return diag.FromErr(err)
 	}
-	events, err := c.HarvesterClient.KubevirtV1().VirtualMachines(namespace).Watch(ctx, util.WatchOptions(name, int64(vmDeleteTimeout)))
+
+	ctxDeadline, _ := ctx.Deadline()
+	events, err := c.HarvesterClient.
+		KubevirtV1().
+		VirtualMachines(namespace).
+		Watch(ctx, util.WatchOptions(name, time.Until(ctxDeadline)))
 	if err != nil {
 		return diag.FromErr(err)
 	}

diff --git a/internal/provider/vlanconfig/resource_vlanconfig.go b/internal/provider/vlanconfig/resource_vlanconfig.go
@@ -3,6 +3,7 @@ package vlanconfig
 import (
 	"context"
 	"fmt"
+	"time"
 
 	harvsternetworkv1 "github.com/harvester/harvester-network-controller/pkg/apis/network.harvesterhci.io/v1beta1"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
@@ -17,10 +18,6 @@ import (
 	"github.com/harvester/terraform-provider-harvester/pkg/importer"
 )
 
-const (
-	vlanConfigDeleteTimeout = 300
-)
-
 func ResourceVLANConfig() *schema.Resource {
 	return &schema.Resource{
 		CreateContext: resourceVLANConfigCreate,
@@ -31,6 +28,13 @@ func ResourceVLANConfig() *schema.Resource {
 			StateContext: schema.ImportStatePassthroughContext,
 		},
 		Schema: Schema(),
+		Timeouts: &schema.ResourceTimeout{
+			Create:  schema.DefaultTimeout(2 * time.Minute),
+			Read:    schema.DefaultTimeout(2 * time.Minute),
+			Update:  schema.DefaultTimeout(2 * time.Minute),
+			Delete:  schema.DefaultTimeout(5 * time.Minute),
+			Default: schema.DefaultTimeout(2 * time.Minute),
+		},
 	}
 }
 
@@ -100,7 +104,12 @@ func resourceVLANConfigDelete(ctx context.Context, d *schema.ResourceData, meta
 	if err = c.HarvesterNetworkClient.NetworkV1beta1().VlanConfigs().Delete(ctx, name, metav1.DeleteOptions{}); err != nil && !apierrors.IsNotFound(err) {
 		return diag.FromErr(err)
 	}
-	events, err := c.HarvesterNetworkClient.NetworkV1beta1().VlanConfigs().Watch(ctx, util.WatchOptions(name, int64(vlanConfigDeleteTimeout)))
+
+	ctxDeadline, _ := ctx.Deadline()
+	events, err := c.HarvesterNetworkClient.
+		NetworkV1beta1().
+		VlanConfigs().
+		Watch(ctx, util.WatchOptions(name, time.Until(ctxDeadline)))
 	if err != nil {
 		return diag.FromErr(err)
 	}

diff --git a/internal/provider/volume/resource_volume.go b/internal/provider/volume/resource_volume.go
@@ -2,7 +2,7 @@ package volume
 
 import (
 	"context"
-	"fmt"
+	"time"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -17,10 +17,6 @@ import (
 	"github.com/harvester/terraform-provider-harvester/pkg/importer"
 )
 
-const (
-	volumeDeleteTimeout = 300
-)
-
 func ResourceVolume() *schema.Resource {
 	return &schema.Resource{
 		CreateContext: resourceVolumeCreate,
@@ -31,6 +27,13 @@ func ResourceVolume() *schema.Resource {
 			StateContext: schema.ImportStatePassthroughContext,
 		},
 		Schema: Schema(),
+		Timeouts: &schema.ResourceTimeout{
+			Create:  schema.DefaultTimeout(2 * time.Minute),
+			Read:    schema.DefaultTimeout(2 * time.Minute),
+			Update:  schema.DefaultTimeout(2 * time.Minute),
+			Delete:  schema.DefaultTimeout(5 * time.Minute),
+			Default: schema.DefaultTimeout(2 * time.Minute),
+		},
 	}
 }
 
@@ -100,12 +103,14 @@ func resourceVolumeDelete(ctx context.Context, d *schema.ResourceData, meta inte
 	if err = c.KubeClient.CoreV1().PersistentVolumeClaims(namespace).Delete(ctx, name, metav1.DeleteOptions{}); err != nil && !apierrors.IsNotFound(err) {
 		return diag.FromErr(err)
 	}
-	events, err := c.KubeClient.CoreV1().PersistentVolumeClaims(namespace).Watch(ctx, util.WatchOptions(name, int64(volumeDeleteTimeout)))
+
+	ctxDeadline, _ := ctx.Deadline()
+	events, err := c.KubeClient.CoreV1().PersistentVolumeClaims(namespace).Watch(ctx, util.WatchOptions(name, time.Until(ctxDeadline)))
 	if err != nil {
 		return diag.FromErr(err)
 	}
 	if !util.HasDeleted(events) {
-		return diag.FromErr(fmt.Errorf("timeout waiting for volume %s to be deleted", d.Id()))
+		return diag.Errorf("timeout waiting for volume %s to be deleted", d.Id())
 	}
 	d.SetId("")
 	return nil

diff --git a/internal/util/state.go b/internal/util/state.go
@@ -2,6 +2,7 @@ package util
 
 import (
 	"fmt"
+	"time"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -31,10 +32,10 @@ func HasDeleted(events watch.Interface) bool {
 	return deleted
 }
 
-func WatchOptions(name string, timeoutSeconds int64) metav1.ListOptions {
+func WatchOptions(name string, timeout time.Duration) metav1.ListOptions {
 	return metav1.ListOptions{
 		FieldSelector:  fmt.Sprintf("metadata.name=%s", name),
 		Watch:          true,
-		TimeoutSeconds: &timeoutSeconds,
+		TimeoutSeconds: func() *int64 { to := timeout.Milliseconds() / 1000; return &to }(),
 	}
 }