From 3469cb2e04a458c3162434367052ed5f1f47a7c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Moritz=20R=C3=B6hrich?= Date: Tue, 4 Jun 2024 15:25:46 +0200 Subject: [PATCH] Add timeout blocks for Terraform resources (#97) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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: https://github.com/harvester/harvester/issues/5632 Signed-off-by: Moritz Röhrich * 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 --------- Signed-off-by: Moritz Röhrich --- .../resource_cloudinitsecret.go | 9 ++++ .../clusternetwork/resource_clusternetwork.go | 8 ++++ internal/provider/image/resource_image.go | 21 +++++++++ internal/provider/keypair/resource_keypair.go | 8 ++++ internal/provider/network/resource_network.go | 19 ++++++-- internal/provider/provider.go | 47 ++++++++++--------- .../storageclass/resource_storageclass.go | 8 ++++ .../virtualmachine/resource_virtualmachine.go | 18 +++++-- .../vlanconfig/resource_vlanconfig.go | 19 ++++++-- internal/provider/volume/resource_volume.go | 19 +++++--- internal/util/state.go | 5 +- pkg/constants/constants_image.go | 1 + 12 files changed, 135 insertions(+), 47 deletions(-) diff --git a/internal/provider/cloudinitsecret/resource_cloudinitsecret.go b/internal/provider/cloudinitsecret/resource_cloudinitsecret.go index c0c0a329..8d26f859 100644 --- 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 index c249cf31..d23d1f6a 100644 --- 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 index 2d06d635..0e55d6b6 100644 --- 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 index 71497617..6225de6c 100644 --- 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 index 04e6effb..b7834e30 100644 --- 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 index ef2e433a..0206e983 100644 --- 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 index b10a6b9d..1b45a19c 100644 --- 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 index 0e958b4a..5020d5ae 100644 --- 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 index 9532d36c..a774a0e7 100644 --- 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 index 4ab2b984..7d9d8ef2 100644 --- 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 index 3791be1c..00630460 100644 --- 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 }(), } } diff --git a/pkg/constants/constants_image.go b/pkg/constants/constants_image.go index 8e89f37f..a6f156a0 100644 --- a/pkg/constants/constants_image.go +++ b/pkg/constants/constants_image.go @@ -18,4 +18,5 @@ const ( StateImageDownloading = "Downloading" StateImageExporting = "Exporting" StateImageInitializing = "Initializing" + StateImageTerminating = "Terminating" )