Merge pull request #394 from MusicDin/fix/state-refresh

Remove refresh_interval attribute from provider

docs/index.md

@@ -75,10 +75,6 @@ The following arguments are supported:
 	also be set with the `LXD_ACCEPT_SERVER_CERTIFICATE` environment variable.
   Defaults to `false`
 
-* `refresh_interval` - *Optional* - How often to poll during state change.
-	Defaults to "10s", or 10 seconds. Valid values are a Go-style parsable time
-	duration (`10s`, `1m`, `5h`).
-
 The `remote` block supports:
 
 * `address` - *Optional* - The address of the LXD remote.

internal/acctest/provider_factory.go

@@ -2,7 +2,6 @@ package acctest
 
 import (
 	"sync"
-	"time"
 
 	lxd_config "github.com/canonical/lxd/lxc/config"
 	"github.com/hashicorp/terraform-plugin-framework/providerserver"
@@ -29,9 +28,8 @@ func testProvider() *provider_config.LxdProviderConfig {
 
 	if testProviderConfig == nil {
 		config := lxd_config.DefaultConfig()
-		refreshInterval := time.Duration(2 * time.Second)
 		acceptClientCert := true
-		testProviderConfig = provider_config.NewLxdProvider(config, refreshInterval, acceptClientCert)
+		testProviderConfig = provider_config.NewLxdProvider(config, acceptClientCert)
 	}
 
 	return testProviderConfig
@@ -42,5 +40,5 @@ func testProvider() *provider_config.LxdProviderConfig {
 // CLI command executed to create a provider server to which the CLI can
 // reattach.
 var ProtoV6ProviderFactories = map[string]func() (tfprotov6.ProviderServer, error){
-	"lxd": providerserver.NewProtocol6WithError(provider.NewLxdProvider("test", "2s")()),
+	"lxd": providerserver.NewProtocol6WithError(provider.NewLxdProvider("test")()),
 }

internal/instance/resource_instance.go

@@ -489,7 +489,7 @@ func (r InstanceResource) Create(ctx context.Context, req resource.CreateRequest
 
 	if plan.Running.ValueBool() {
 		// Start the instance.
-		diag := startInstance(ctx, server, instance.Name, r.provider.RefreshInterval())
+		diag := startInstance(ctx, server, instance.Name)
 		if diag != nil {
 			resp.Diagnostics.Append(diag)
 			return
@@ -498,7 +498,7 @@ func (r InstanceResource) Create(ctx context.Context, req resource.CreateRequest
 		// Wait for the instance to obtain an IP address if network
 		// availability is requested by the user.
 		if plan.WaitForNetwork.ValueBool() {
-			diag := waitInstanceNetwork(ctx, server, instance.Name, r.provider.RefreshInterval())
+			diag := waitInstanceNetwork(ctx, server, instance.Name)
 			if diag != nil {
 				resp.Diagnostics.Append(diag)
 				return
@@ -573,7 +573,7 @@ func (r InstanceResource) Update(ctx context.Context, req resource.UpdateRequest
 	// First ensure the desired state of the instance (stopped/running).
 	// This ensures we fail fast if instance runs into an issue.
 	if plan.Running.ValueBool() && !isInstanceOperational(*instanceState) {
-		diag := startInstance(ctx, server, instanceName, r.provider.RefreshInterval())
+		diag := startInstance(ctx, server, instanceName)
 		if diag != nil {
 			resp.Diagnostics.Append(diag)
 			return
@@ -582,15 +582,15 @@ func (r InstanceResource) Update(ctx context.Context, req resource.UpdateRequest
 		// If instance is freshly started, we should also wait for
 		// network (if user requested that).
 		if plan.WaitForNetwork.ValueBool() {
-			diag := waitInstanceNetwork(ctx, server, instanceName, r.provider.RefreshInterval())
+			diag := waitInstanceNetwork(ctx, server, instanceName)
 			if diag != nil {
 				resp.Diagnostics.Append(diag)
 				return
 			}
 		}
 	} else if !plan.Running.ValueBool() && !isInstanceStopped(*instanceState) {
 		// Stop the instance gracefully.
-		_, diag := stopInstance(ctx, server, instanceName, r.provider.RefreshInterval(), false)
+		_, diag := stopInstance(ctx, server, instanceName, false)
 		if diag != nil {
 			resp.Diagnostics.Append(diag)
 			return
@@ -719,7 +719,7 @@ func (r InstanceResource) Delete(ctx context.Context, req resource.DeleteRequest
 	instanceName := state.Name.ValueString()
 
 	// Force stop the instance, because we are deleting it anyway.
-	isFound, diag := stopInstance(ctx, server, instanceName, r.provider.RefreshInterval(), true)
+	isFound, diag := stopInstance(ctx, server, instanceName, true)
 	if diag != nil {
 		// Ephemeral instances will be removed when stopped.
 		if !isFound {
@@ -922,7 +922,7 @@ func ToProfileListType(ctx context.Context, profiles []string) (types.List, diag
 
 // startInstance starts an instance with the given name. It also waits
 // for it to become fully operational.
-func startInstance(ctx context.Context, server lxd.InstanceServer, instanceName string, refInterval time.Duration) diag.Diagnostic {
+func startInstance(ctx context.Context, server lxd.InstanceServer, instanceName string) diag.Diagnostic {
 	st, etag, err := server.GetInstanceState(instanceName)
 	if err != nil {
 		return diag.NewErrorDiagnostic(fmt.Sprintf("Failed to retrieve state of instance %q", instanceName), err.Error())
@@ -966,15 +966,7 @@ func startInstance(ctx context.Context, server lxd.InstanceServer, instanceName
 
 	// Even though op.Wait has completed, wait until we can see
 	// the instance is fully started via a new API call.
-	stateConf := &retry.StateChangeConf{
-		Target:     []string{api.Running.String()},
-		Timeout:    3 * time.Minute,
-		MinTimeout: 3 * time.Second,
-		Delay:      refInterval,
-		Refresh:    instanceStartedCheck,
-	}
-
-	_, err = stateConf.WaitForStateContext(ctx)
+	_, err = waitForState(ctx, instanceStartedCheck, api.Running.String())
 	if err != nil {
 		return diag.NewErrorDiagnostic(fmt.Sprintf("Failed to wait for instance %q to start", instanceName), err.Error())
 	}
@@ -986,7 +978,7 @@ func startInstance(ctx context.Context, server lxd.InstanceServer, instanceName
 // status to become Stopped or the instance to be removed (not found) in
 // case of an ephemeral instance. In the latter case, false is returned
 // along an error.
-func stopInstance(ctx context.Context, server lxd.InstanceServer, instanceName string, refInterval time.Duration, force bool) (bool, diag.Diagnostic) {
+func stopInstance(ctx context.Context, server lxd.InstanceServer, instanceName string, force bool) (bool, diag.Diagnostic) {
 	st, etag, err := server.GetInstanceState(instanceName)
 	if err != nil {
 		return true, diag.NewErrorDiagnostic(fmt.Sprintf("Failed to retrieve state of instance %q", instanceName), err.Error())
@@ -1022,17 +1014,9 @@ func stopInstance(ctx context.Context, server lxd.InstanceServer, instanceName s
 		return st, st.Status, nil
 	}
 
-	stateConf := &retry.StateChangeConf{
-		Target:     []string{api.Stopped.String()},
-		Timeout:    3 * time.Minute,
-		MinTimeout: 3 * time.Second,
-		Delay:      refInterval,
-		Refresh:    instanceStoppedCheck,
-	}
-
 	// Even though op.Wait has completed, wait until we can see
 	// the instance is stopped via a new API call.
-	_, err = stateConf.WaitForStateContext(ctx)
+	_, err = waitForState(ctx, instanceStoppedCheck, api.Stopped.String())
 	if err != nil {
 		found := !errors.IsNotFoundError(err)
 		return found, diag.NewErrorDiagnostic(fmt.Sprintf("Failed to wait for instance %q to stop", instanceName), err.Error())
@@ -1044,7 +1028,7 @@ func stopInstance(ctx context.Context, server lxd.InstanceServer, instanceName s
 // waitInstanceNetwork waits for an instance with the given name to receive
 // an IPv4 address on any interface (excluding loopback). This should be
 // called only if the instance is running.
-func waitInstanceNetwork(ctx context.Context, server lxd.InstanceServer, instanceName string, refInterval time.Duration) diag.Diagnostic {
+func waitInstanceNetwork(ctx context.Context, server lxd.InstanceServer, instanceName string) diag.Diagnostic {
 	// instanceNetworkCheck function checks whether instance has
 	// received an IP address.
 	instanceNetworkCheck := func() (any, string, error) {
@@ -1068,24 +1052,28 @@ func waitInstanceNetwork(ctx context.Context, server lxd.InstanceServer, instanc
 		return st, "Waiting for network", nil
 	}
 
-	// LXD will return "Running" even if "inet" has not yet
-	// been set. Therefore, wait until we see an "inet" IP.
-	networkConf := &retry.StateChangeConf{
-		Target:     []string{"OK"},
-		Timeout:    3 * time.Minute,
-		MinTimeout: 3 * time.Second,
-		Delay:      refInterval,
-		Refresh:    instanceNetworkCheck,
-	}
-
-	_, err := networkConf.WaitForStateContext(ctx)
+	_, err := waitForState(ctx, instanceNetworkCheck, "OK")
 	if err != nil {
 		return diag.NewErrorDiagnostic(fmt.Sprintf("Failed to wait for instance %q to get an IP address", instanceName), err.Error())
 	}
 
 	return nil
 }
 
+// waitForState waits until the provided function reports one of the target
+// states. It returns either the resulting state or an error.
+func waitForState(ctx context.Context, refreshFunc retry.StateRefreshFunc, targets ...string) (any, error) {
+	stateRefreshConf := &retry.StateChangeConf{
+		Refresh:    refreshFunc,
+		Target:     targets,
+		Timeout:    3 * time.Minute,
+		MinTimeout: 2 * time.Second, // Timeout increases: 2, 4, 8, 10, 10, ...
+		Delay:      2 * time.Second, // Delay before the first check/refresh.
+	}
+
+	return stateRefreshConf.WaitForStateContext(ctx)
+}
+
 // isInstanceOperational determines if an instance is fully operational based
 // on its state. It returns true if the instance is running and the reported
 // process count is positive. Checking for a positive process count is esential

internal/provider-config/config.go

@@ -6,7 +6,6 @@ import (
 	"os"
 	"path/filepath"
 	"sync"
-	"time"
 
 	lxd "github.com/canonical/lxd/client"
 	lxd_config "github.com/canonical/lxd/lxc/config"
@@ -39,10 +38,6 @@ type LxdProviderConfig struct {
 	// should be accepted.
 	acceptServerCertificate bool
 
-	// refreshInterval is a custom interval for communicating with remote
-	// LXD servers.
-	refreshInterval time.Duration
-
 	// LXDConfig is the converted form of terraformLXDConfig
 	// in LXD's native data structure. This is lazy-loaded / created
 	// only when a connection to an LXD remote/server happens.
@@ -69,10 +64,9 @@ type LxdProviderConfig struct {
 // NewLxdProvider returns initialized LXD provider structure. This struct is
 // used to store information about this Terraform provider's configuration for
 // reference throughout the lifecycle.
-func NewLxdProvider(lxdConfig *lxd_config.Config, refreshInterval time.Duration, acceptServerCert bool) *LxdProviderConfig {
+func NewLxdProvider(lxdConfig *lxd_config.Config, acceptServerCert bool) *LxdProviderConfig {
 	return &LxdProviderConfig{
 		acceptServerCertificate: acceptServerCert,
-		refreshInterval:         refreshInterval,
 		lxdConfig:               lxdConfig,
 		remotes:                 make(map[string]LxdProviderRemoteConfig),
 		servers:                 make(map[string]lxd.Server),
@@ -496,9 +490,3 @@ func (p *LxdProviderConfig) getLxdConfigImageServer(remoteName string) (lxd.Imag
 	defer p.mux.RUnlock()
 	return p.lxdConfig.GetImageServer(remoteName)
 }
-
-// RefreshInterval returns a time interval on which provider should
-// retry to communicate with the LXD server.
-func (p *LxdProviderConfig) RefreshInterval() time.Duration {
-	return p.refreshInterval
-}

internal/provider/provider.go

@@ -5,13 +5,11 @@ import (
 	"log"
 	"os"
 	"path/filepath"
-	"time"
 
 	lxd_config "github.com/canonical/lxd/lxc/config"
 	lxd_shared "github.com/canonical/lxd/shared"
 	"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
 	"github.com/hashicorp/terraform-plugin-framework/datasource"
-	"github.com/hashicorp/terraform-plugin-framework/path"
 	"github.com/hashicorp/terraform-plugin-framework/provider"
 	"github.com/hashicorp/terraform-plugin-framework/provider/schema"
 	"github.com/hashicorp/terraform-plugin-framework/resource"
@@ -41,27 +39,20 @@ type LxdProviderModel struct {
 	Remotes                    []LxdProviderRemoteModel `tfsdk:"remote"`
 	ConfigDir                  types.String             `tfsdk:"config_dir"`
 	Project                    types.String             `tfsdk:"project"`
-	RefreshInterval            types.String             `tfsdk:"refresh_interval"`
 	AcceptRemoteCertificate    types.Bool               `tfsdk:"accept_remote_certificate"`
 	GenerateClientCertificates types.Bool               `tfsdk:"generate_client_certificates"`
 }
 
 // LxdProvider ...
 type LxdProvider struct {
-	version         string
-	refreshInterval string
+	version string
 }
 
 // New returns LXD provider with the given version set.
-func NewLxdProvider(version string, refreshInterval string) func() provider.Provider {
-	if refreshInterval == "" {
-		refreshInterval = "10s"
-	}
-
+func NewLxdProvider(version string) func() provider.Provider {
 	return func() provider.Provider {
 		return &LxdProvider{
-			version:         version,
-			refreshInterval: refreshInterval,
+			version: version,
 		}
 	}
 }
@@ -89,11 +80,6 @@ func (p *LxdProvider) Schema(_ context.Context, _ provider.SchemaRequest, resp *
 				Description: "Accept the server certificate.",
 			},
 
-			"refresh_interval": schema.StringAttribute{
-				Optional:    true,
-				Description: "How often to poll during state changes. (default = 10s)",
-			},
-
 			"project": schema.StringAttribute{
 				Optional:    true,
 				Description: "The project where project-scoped resources will be created. Can be overridden in individual resources. (default = default)",
@@ -176,18 +162,6 @@ func (p *LxdProvider) Configure(ctx context.Context, req provider.ConfigureReque
 
 	log.Printf("[DEBUG] LXD Config: %#v", config)
 
-	// Determine custom refresh interval. Default is 10 seconds.
-	refreshIntervalString := data.RefreshInterval.ValueString()
-	if refreshIntervalString == "" {
-		refreshIntervalString = p.refreshInterval
-	}
-
-	refreshInterval, err := time.ParseDuration(refreshIntervalString)
-	if err != nil {
-		resp.Diagnostics.AddAttributeError(path.Root("refresh_interval"), "Invalid refresh interval", err.Error())
-		return
-	}
-
 	// Determine if the LXD server's SSL certificates should be
 	// accepted. If this is set to false and if the remote's
 	// certificates haven't already been accepted, the user will
@@ -227,7 +201,7 @@ func (p *LxdProvider) Configure(ctx context.Context, req provider.ConfigureReque
 	// Initialize global LxdProvider struct.
 	// This struct is used to store information about this Terraform
 	// provider's configuration for reference throughout the lifecycle.
-	lxdProvider := provider_config.NewLxdProvider(config, refreshInterval, acceptServerCertificate)
+	lxdProvider := provider_config.NewLxdProvider(config, acceptServerCertificate)
 
 	// Create LXD remote from environment variables (if defined).
 	// This emulates the Terraform provider "remote" config:

main.go

@@ -10,7 +10,7 @@ import (
 )
 
 // version indicates provider's version. The appropriate value
-// for the compile binary will be set by the goreleaser.
+// for the compiled binary will be set by the goreleaser.
 // See: https://goreleaser.com/cookbooks/using-main.version/
 var version = "dev"
 
@@ -34,7 +34,7 @@ func main() {
 		ProtocolVersion: 6,
 	}
 
-	err := providerserver.Serve(context.Background(), provider.NewLxdProvider(version, "10s"), opts)
+	err := providerserver.Serve(context.Background(), provider.NewLxdProvider(version), opts)
 	if err != nil {
 		log.Fatal(err.Error())
 	}