refactor: remove Timeout option from WaitFor* methods (#291)

CHANGELOG.md

@@ -15,6 +15,7 @@ See updating [Changelog example here](https://keepachangelog.com/en/1.0.0/)
 
 ### Removed
 - **Breaking**, Managed Database: connection related methods in favor of session
+- **Breaking**: remove `Timeout` option from `WaitFor*` methods. Use `context.WithTimeout` to define a timeout for these functions.
 
 ## [6.12.0]
 

README.md

@@ -155,7 +155,6 @@ fmt.Println(fmt.Sprintf("Server %s with UUID %s created", serverDetails.Title, s
 err = svc.WaitForServerState(context.Background(), &request.WaitForServerStateRequest{
 	UUID:         serverDetails.UUID,
 	DesiredState: upcloud.ServerStateStarted,
-	Timeout:      time.Minute * 5,
 })
 
 if err != nil {

examples/upcloud-cli/main.go

@@ -6,7 +6,6 @@ import (
 	"flag"
 	"fmt"
 	"os"
-	"time"
 
 	"github.com/UpCloudLtd/upcloud-go-api/v6/upcloud"
 	"github.com/UpCloudLtd/upcloud-go-api/v6/upcloud/client"
@@ -121,7 +120,6 @@ func createServer(s *service.Service) error {
 	details, err = s.WaitForServerState(ctx, &request.WaitForServerStateRequest{
 		UUID:         details.UUID,
 		DesiredState: upcloud.ServerStateStarted,
-		Timeout:      1 * time.Minute,
 	})
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "Unable to wait for server: %#v", err)
@@ -160,7 +158,6 @@ func deleteServers(s *service.Service) error {
 				_, err = s.WaitForServerState(ctx, &request.WaitForServerStateRequest{
 					UUID:         server.UUID,
 					DesiredState: upcloud.ServerStateStopped,
-					Timeout:      1 * time.Minute,
 				})
 				if err != nil {
 					fmt.Fprintf(os.Stderr, "Failed to wait for server to reach desired state: %#v", err)

upcloud/request/kubernetes.go

@@ -3,7 +3,6 @@ package request
 import (
 	"encoding/json"
 	"fmt"
-	"time"
 
 	"github.com/UpCloudLtd/upcloud-go-api/v6/upcloud"
 )
@@ -93,7 +92,6 @@ func (r *DeleteKubernetesClusterRequest) RequestURL() string {
 // to enter a desired state
 type WaitForKubernetesClusterStateRequest struct {
 	DesiredState upcloud.KubernetesClusterState `json:"-"`
-	Timeout      time.Duration                  `json:"-"`
 	UUID         string                         `json:"-"`
 }
 
@@ -105,7 +103,6 @@ func (r *WaitForKubernetesClusterStateRequest) RequestURL() string {
 // to enter a desired state
 type WaitForKubernetesNodeGroupStateRequest struct {
 	DesiredState upcloud.KubernetesNodeGroupState `json:"-"`
-	Timeout      time.Duration                    `json:"-"`
 	ClusterUUID  string                           `json:"-"`
 	Name         string                           `json:"-"`
 }

upcloud/request/managed_database.go

@@ -477,7 +477,6 @@ func (r *GetManagedDatabaseVersionsRequest) RequestURL() string {
 type WaitForManagedDatabaseStateRequest struct {
 	UUID         string
 	DesiredState upcloud.ManagedDatabaseState
-	Timeout      time.Duration
 }
 
 // StartManagedDatabaseRequest represents a request to start an existing managed database instance

upcloud/request/managed_object_storage.go

@@ -2,7 +2,6 @@ package request
 
 import (
 	"fmt"
-	"time"
 
 	"github.com/UpCloudLtd/upcloud-go-api/v6/upcloud"
 )
@@ -291,7 +290,6 @@ func (r *DeleteManagedObjectStorageUserAccessKeyRequest) RequestURL() string {
 // to enter a desired state
 type WaitForManagedObjectStorageOperationalStateRequest struct {
 	DesiredState upcloud.ManagedObjectStorageOperationalState `json:"-"`
-	Timeout      time.Duration                                `json:"-"`
 	UUID         string                                       `json:"-"`
 }
 
@@ -304,7 +302,6 @@ func (r *WaitForManagedObjectStorageOperationalStateRequest) RequestURL() string
 // to enter a desired state
 type WaitForManagedObjectStorageUserOperationalStateRequest struct {
 	DesiredState upcloud.ManagedObjectStorageUserOperationalState `json:"-"`
-	Timeout      time.Duration                                    `json:"-"`
 	ServiceUUID  string                                           `json:"-"`
 	Username     string                                           `json:"-"`
 }
@@ -318,7 +315,6 @@ func (r *WaitForManagedObjectStorageUserOperationalStateRequest) RequestURL() st
 // to be deleted
 type WaitForManagedObjectStorageDeletionRequest struct {
 	DesiredState upcloud.ManagedObjectStorageOperationalState `json:"-"`
-	Timeout      time.Duration                                `json:"-"`
 	UUID         string                                       `json:"-"`
 }
 

upcloud/request/server.go

@@ -229,7 +229,6 @@ type WaitForServerStateRequest struct {
 	UUID           string
 	DesiredState   string
 	UndesiredState string
-	Timeout        time.Duration
 }
 
 // StartServerRequest represents a request to start a server

upcloud/request/storage.go

@@ -3,7 +3,6 @@ package request
 import (
 	"encoding/json"
 	"fmt"
-	"time"
 
 	"github.com/UpCloudLtd/upcloud-go-api/v6/upcloud"
 )
@@ -238,7 +237,6 @@ func (r TemplatizeStorageRequest) MarshalJSON() ([]byte, error) {
 type WaitForStorageStateRequest struct {
 	UUID         string
 	DesiredState string
-	Timeout      time.Duration
 }
 
 // LoadCDROMRequest represents a request to load a storage as a CD-ROM in the CD-ROM device of a server
@@ -355,7 +353,6 @@ func (r *GetStorageImportDetailsRequest) RequestURL() string {
 // for storage import to complete.
 type WaitForStorageImportCompletionRequest struct {
 	StorageUUID string
-	Timeout     time.Duration
 }
 
 // ResizeStorageFilesystemRequest represents a request to resize storage filesystem

upcloud/service/kubernetes.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"log"
 	"net/http"
-	"time"
 
 	"github.com/UpCloudLtd/upcloud-go-api/v6/upcloud"
 	"github.com/UpCloudLtd/upcloud-go-api/v6/upcloud/request"
@@ -77,20 +76,14 @@ func (s *Service) DeleteKubernetesCluster(ctx context.Context, r *request.Delete
 // specified state. If the state changes favorably, cluster details are returned. The method will give up
 // after the specified timeout
 func (s *Service) WaitForKubernetesClusterState(ctx context.Context, r *request.WaitForKubernetesClusterStateRequest) (*upcloud.KubernetesCluster, error) {
-	attempts := 0
-	sleepDuration := time.Second * 5
-
-	for {
-		attempts++
-		time.Sleep(sleepDuration)
-
-		details, err := s.GetKubernetesCluster(ctx, &request.GetKubernetesClusterRequest{
+	return retry(ctx, func(i int, c context.Context) (*upcloud.KubernetesCluster, error) {
+		details, err := s.GetKubernetesCluster(c, &request.GetKubernetesClusterRequest{
 			UUID: r.UUID,
 		})
 		if err != nil {
 			// Ignore first two 404 responses to avoid errors caused by possible false NOT_FOUND responses right after cluster has been created.
 			var ucErr *upcloud.Problem
-			if errors.As(err, &ucErr) && ucErr.Status == http.StatusNotFound && attempts < 3 {
+			if errors.As(err, &ucErr) && ucErr.Status == http.StatusNotFound && i < 3 {
 				log.Printf("ERROR: %+v", err)
 			} else {
 				return nil, err
@@ -100,57 +93,44 @@ func (s *Service) WaitForKubernetesClusterState(ctx context.Context, r *request.
 		if details.State == r.DesiredState {
 			return details, nil
 		}
-
-		if time.Duration(attempts)*sleepDuration >= r.Timeout {
-			return nil, fmt.Errorf("timeout reached while waiting for Kubernetes cluster to enter state \"%s\"", r.DesiredState)
-		}
-	}
+		return nil, nil
+	}, nil)
 }
 
 // WaitForKubernetesNodeGroupState blocks execution until the specified Kubernetes node group has entered the
 // specified state. If the state changes favorably, node group is returned. The method will give up
 // after the specified timeout
 func (s *Service) WaitForKubernetesNodeGroupState(ctx context.Context, r *request.WaitForKubernetesNodeGroupStateRequest) (*upcloud.KubernetesNodeGroup, error) {
-	attempts := 0
-	sleepDuration := time.Second * 5
-
-	for {
-		attempts++
-		time.Sleep(sleepDuration)
-
-		ng, err := s.GetKubernetesNodeGroup(ctx, &request.GetKubernetesNodeGroupRequest{
+	return retry(ctx, func(i int, c context.Context) (*upcloud.KubernetesNodeGroup, error) {
+		ng, err := s.GetKubernetesNodeGroup(c, &request.GetKubernetesNodeGroupRequest{
 			ClusterUUID: r.ClusterUUID,
 			Name:        r.Name,
 		})
 		if err != nil {
-			// Ignore first two 404 responses to avoid errors caused by possible false NOT_FOUND responses right after cluster or node group has been created.
+			// Ignore first two 404 responses to avoid errors caused by possible false NOT_FOUND responses right after cluster has been created.
 			var ucErr *upcloud.Problem
-			if !(errors.As(err, &ucErr) && ucErr.Status == http.StatusNotFound) || attempts >= 3 {
+			if errors.As(err, &ucErr) && ucErr.Status == http.StatusNotFound && i < 3 {
+				log.Printf("ERROR: %+v", err)
+			} else {
 				return nil, err
 			}
 		}
 
 		if ng.State == r.DesiredState {
 			return &ng.KubernetesNodeGroup, nil
 		}
-
-		if time.Duration(attempts)*sleepDuration >= r.Timeout {
-			return nil, fmt.Errorf("timeout reached while waiting for Kubernetes node group to enter state \"%s\"", r.DesiredState)
-		}
-	}
+		return nil, nil
+	}, nil)
 }
 
 // GetKubernetesKubeconfig retrieves kubeconfig of a Kubernetes cluster.
 func (s *Service) GetKubernetesKubeconfig(ctx context.Context, r *request.GetKubernetesKubeconfigRequest) (string, error) {
-	// TODO: should timeout be part of GetKubernetesKubeconfigRequest ?
-	const timeout time.Duration = 10 * time.Minute
 	data := struct {
 		Kubeconfig string `json:"kubeconfig"`
 	}{}
 
 	_, err := s.WaitForKubernetesClusterState(ctx, &request.WaitForKubernetesClusterStateRequest{
 		DesiredState: upcloud.KubernetesClusterStateRunning,
-		Timeout:      timeout,
 		UUID:         r.UUID,
 	})
 	if err != nil {

upcloud/service/kubernetes_test.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"net/http"
 	"testing"
-	"time"
 
 	"github.com/UpCloudLtd/upcloud-go-api/v6/upcloud"
 	"github.com/UpCloudLtd/upcloud-go-api/v6/upcloud/client"
@@ -524,7 +523,6 @@ func TestWaitForKubernetesClusterState(t *testing.T) {
 	_, err := svc.WaitForKubernetesClusterState(context.Background(), &request.WaitForKubernetesClusterStateRequest{
 		UUID:         "_UUID_",
 		DesiredState: upcloud.KubernetesClusterStateRunning,
-		Timeout:      time.Second * 20,
 	})
 	assert.NoError(t, err)
 	assert.Equal(t, 3, requestsMade)
@@ -584,7 +582,6 @@ func TestWaitForKubernetesNodeGroupState(t *testing.T) {
 	_, err := svc.WaitForKubernetesNodeGroupState(context.Background(), &request.WaitForKubernetesNodeGroupStateRequest{
 		ClusterUUID:  "_UUID_",
 		DesiredState: upcloud.KubernetesNodeGroupStateRunning,
-		Timeout:      time.Second * 20,
 		Name:         "_NAME_",
 	})
 	assert.NoError(t, err)

upcloud/service/managed_database.go

@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"time"
 
 	"github.com/UpCloudLtd/upcloud-go-api/v6/upcloud"
 	"github.com/UpCloudLtd/upcloud-go-api/v6/upcloud/request"
@@ -170,13 +169,8 @@ func (s *Service) GetManagedDatabaseVersions(ctx context.Context, r *request.Get
 // specified state. If the state changes favorably, the new managed database details is returned. The method will give up
 // after the specified timeout
 func (s *Service) WaitForManagedDatabaseState(ctx context.Context, r *request.WaitForManagedDatabaseStateRequest) (*upcloud.ManagedDatabase, error) {
-	attempts := 0
-	sleepDuration := time.Second * 5
-
-	for {
-		attempts++
-
-		details, err := s.GetManagedDatabase(ctx, &request.GetManagedDatabaseRequest{
+	return retry(ctx, func(_ int, c context.Context) (*upcloud.ManagedDatabase, error) {
+		details, err := s.GetManagedDatabase(c, &request.GetManagedDatabaseRequest{
 			UUID: r.UUID,
 		})
 		if err != nil {
@@ -186,13 +180,8 @@ func (s *Service) WaitForManagedDatabaseState(ctx context.Context, r *request.Wa
 		if details.State == r.DesiredState {
 			return details, nil
 		}
-
-		time.Sleep(sleepDuration)
-
-		if time.Duration(attempts)*sleepDuration >= r.Timeout {
-			return nil, fmt.Errorf("timeout reached while waiting for managed database instance to enter state \"%s\"", r.DesiredState)
-		}
-	}
+		return nil, nil
+	}, nil)
 }
 
 // StartManagedDatabase starts a shut down existing managed database instance

upcloud/service/managed_database_test.go

@@ -997,7 +997,6 @@ func waitForManagedDatabaseRunningState(ctx context.Context, rec *recorder.Recor
 	_, err := svc.WaitForManagedDatabaseState(ctx, &request.WaitForManagedDatabaseStateRequest{
 		UUID:         dbUUID,
 		DesiredState: upcloud.ManagedDatabaseStateRunning,
-		Timeout:      waitTimeout,
 	})
 
 	return err