Make timeout during waiting for uploadImage configurable

Add the fields `UploadImageRetryCount` and `UploadImageRetryDelay` to `OpenstackSourceSpec`. Related to: harvester/harvester#6675 Signed-off-by: Volker Theile <[email protected]>
harvester · Oct 16, 2024 · 9dd40c1 · 9dd40c1
1 parent c1562ab
commit 9dd40c1
Show file tree

Hide file tree

Showing 7 changed files with 117 additions and 15 deletions.
diff --git a/pkg/apis/migration.harvesterhci.io/v1beta1/common.go b/pkg/apis/migration.harvesterhci.io/v1beta1/common.go
@@ -8,5 +8,10 @@ type SourceInterface interface {
 	ClusterStatus() ClusterStatus
 	SecretReference() corev1.SecretReference
 	GetKind() string
+
+	// GetConnectionInfo returns the connection information of the Source.
 	GetConnectionInfo() (string, string)
+
+	// GetOptions returns the additional configuration options of the Source.
+	GetOptions() interface{}
 }
diff --git a/pkg/apis/migration.harvesterhci.io/v1beta1/openstack.go b/pkg/apis/migration.harvesterhci.io/v1beta1/openstack.go
@@ -7,6 +7,11 @@ import (
 	"github.com/harvester/vm-import-controller/pkg/apis/common"
 )
 
+const (
+	OpenstackDefaultRetryCount = 30
+	OpenstackDefaultRetryDelay = 10
+)
+
 // +genclient
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 
@@ -18,9 +23,10 @@ type OpenstackSource struct {
 }
 
 type OpenstackSourceSpec struct {
-	EndpointAddress string                 `json:"endpoint"`
-	Region          string                 `json:"region"`
-	Credentials     corev1.SecretReference `json:"credentials"`
+	EndpointAddress        string                 `json:"endpoint"`
+	Region                 string                 `json:"region"`
+	Credentials            corev1.SecretReference `json:"credentials"`
+	OpenstackSourceOptions `json:",inline"`
 }
 
 type OpenstackSourceStatus struct {
@@ -29,6 +35,15 @@ type OpenstackSourceStatus struct {
 	Conditions []common.Condition `json:"conditions,omitempty"`
 }
 
+type OpenstackSourceOptions struct {
+	// +optional
+	// The number of max. retries for uploading an image.
+	UploadImageRetryCount int `json:"uploadImageRetryCount,omitempty"`
+	// +optional
+	// The upload retry delay in seconds.
+	UploadImageRetryDelay int `json:"uploadImageRetryDelay,omitempty"`
+}
+
 func (o *OpenstackSource) ClusterStatus() ClusterStatus {
 	return o.Status.Status
 }
@@ -44,3 +59,16 @@ func (o *OpenstackSource) GetKind() string {
 func (o *OpenstackSource) GetConnectionInfo() (string, string) {
 	return o.Spec.EndpointAddress, o.Spec.Region
 }
+
+// GetOptions returns the sanitized OpenstackSourceOptions. This means,
+// optional values are set to their default values.
+func (o *OpenstackSource) GetOptions() interface{} {
+	options := o.Spec.OpenstackSourceOptions
+	if options.UploadImageRetryCount <= 0 {
+		options.UploadImageRetryCount = OpenstackDefaultRetryCount
+	}
+	if options.UploadImageRetryDelay <= 0 {
+		options.UploadImageRetryDelay = OpenstackDefaultRetryDelay
+	}
+	return options
+}
diff --git a/pkg/apis/migration.harvesterhci.io/v1beta1/vmware.go b/pkg/apis/migration.harvesterhci.io/v1beta1/vmware.go
@@ -54,3 +54,7 @@ func (v *VmwareSource) GetKind() string {
 func (v *VmwareSource) GetConnectionInfo() (string, string) {
 	return v.Spec.EndpointAddress, v.Spec.Datacenter
 }
+
+func (v *VmwareSource) GetOptions() interface{} {
+	return nil
+}
diff --git a/pkg/controllers/migration/openstack.go b/pkg/controllers/migration/openstack.go
@@ -50,7 +50,7 @@ func (h *openstackHandler) OnSourceChange(_ string, o *migration.OpenstackSource
 			return o, fmt.Errorf("error looking up secret for openstacksource: %v", err)
 		}
 
-		client, err := openstack.NewClient(h.ctx, o.Spec.EndpointAddress, o.Spec.Region, secretObj)
+		client, err := openstack.NewClient(h.ctx, o.Spec.EndpointAddress, o.Spec.Region, secretObj, o.GetOptions().(migration.OpenstackSourceOptions))
 		if err != nil {
 			return o, fmt.Errorf("error generating openstack client for openstack migration: %s: %v", o.Name, err)
 		}

diff --git a/pkg/controllers/migration/virtualmachine.go b/pkg/controllers/migration/virtualmachine.go
@@ -407,7 +407,8 @@ func (h *virtualMachineHandler) generateVMO(vm *migration.VirtualMachineImport)
 
 	if source.GetKind() == strings.ToLower("openstacksource") {
 		endpoint, region := source.GetConnectionInfo()
-		return openstack.NewClient(h.ctx, endpoint, region, secret)
+		options := source.GetOptions().(migration.OpenstackSourceOptions)
+		return openstack.NewClient(h.ctx, endpoint, region, secret, options)
 	}
 
 	return nil, fmt.Errorf("unsupport source kind")

diff --git a/pkg/source/openstack/client.go b/pkg/source/openstack/client.go
@@ -36,8 +36,6 @@ import (
 const (
 	NotUniqueName         = "notUniqueName"
 	NotServerFound        = "noServerFound"
-	defaultInterval       = 10 * time.Second
-	defaultCount          = 30
 	pollingTimeout        = 2 * 60 * 60 // in seconds
 	annotationDescription = "field.cattle.io/description"
 )
@@ -49,6 +47,7 @@ type Client struct {
 	storageClient *gophercloud.ServiceClient
 	computeClient *gophercloud.ServiceClient
 	imageClient   *gophercloud.ServiceClient
+	options       migration.OpenstackSourceOptions
 }
 
 type ExtendedVolume struct {
@@ -65,7 +64,7 @@ type ExtendedServer struct {
 }
 
 // NewClient will generate a GopherCloud client
-func NewClient(ctx context.Context, endpoint string, region string, secret *corev1.Secret) (*Client, error) {
+func NewClient(ctx context.Context, endpoint string, region string, secret *corev1.Secret, options migration.OpenstackSourceOptions) (*Client, error) {
 	username, ok := secret.Data["username"]
 	if !ok {
 		return nil, fmt.Errorf("no username provided in secret %s", secret.Name)
@@ -86,18 +85,18 @@ func NewClient(ctx context.Context, endpoint string, region string, secret *core
 		return nil, fmt.Errorf("no domain_name provided in secret %s", secret.Name)
 	}
 
-	config := &tls.Config{}
+	tlsClientConfig := &tls.Config{}
 
 	customCA, ok := secret.Data["ca_cert"]
 	if ok {
 		caCertPool := x509.NewCertPool()
 		caCertPool.AppendCertsFromPEM(customCA)
-		config.RootCAs = caCertPool
+		tlsClientConfig.RootCAs = caCertPool
 	} else {
-		config.InsecureSkipVerify = true
+		tlsClientConfig.InsecureSkipVerify = true
 	}
 
-	tr := &http.Transport{TLSClientConfig: config}
+	tr := &http.Transport{TLSClientConfig: tlsClientConfig}
 
 	authOpts := gophercloud.AuthOptions{
 		IdentityEndpoint: endpoint,
@@ -143,6 +142,7 @@ func NewClient(ctx context.Context, endpoint string, region string, secret *core
 		storageClient: storageClient,
 		computeClient: computeClient,
 		imageClient:   imageClient,
+		options:       options,
 	}, nil
 }
 
@@ -237,6 +237,8 @@ func (c *Client) ExportVirtualMachine(vm *migration.VirtualMachineImport) error
 			"volume.snapshotID":       volObj.SnapshotID,
 			"volume.size":             volObj.Size,
 			"volume.status":           volObj.Status,
+			"retryCount":              c.options.UploadImageRetryCount,
+			"retryDelay":              c.options.UploadImageRetryDelay,
 		}).Info("Waiting for volume to be available")
 
 		if err := volumes.WaitForStatus(c.storageClient, volObj.ID, "available", pollingTimeout); err != nil {
@@ -252,7 +254,7 @@ func (c *Client) ExportVirtualMachine(vm *migration.VirtualMachineImport) error
 		}
 
 		// wait for image to be ready
-		for i := 0; i < defaultCount; i++ {
+		for i := 0; i < c.options.UploadImageRetryCount; i++ {
 			imgObj, err := images.Get(c.imageClient, volImage.ImageID).Extract()
 			if err != nil {
 				return fmt.Errorf("error checking status of volume image %s: %v", volImage.ImageID, err)
@@ -267,7 +269,7 @@ func (c *Client) ExportVirtualMachine(vm *migration.VirtualMachineImport) error
 				"image.id":                imgObj.ID,
 				"image.status":            imgObj.Status,
 			}).Info("Waiting for raw image to be available")
-			time.Sleep(defaultInterval)
+			time.Sleep(time.Duration(c.options.UploadImageRetryDelay) * time.Second)
 		}
 
 		contents, err := imagedata.Download(c.imageClient, volImage.ImageID).Extract()

diff --git a/pkg/source/openstack/client_test.go b/pkg/source/openstack/client_test.go
@@ -36,7 +36,10 @@ func TestMain(t *testing.M) {
 		logrus.Fatal(err)
 	}
 
-	c, err = NewClient(context.TODO(), endpoint, region, s)
+	source := migration.OpenstackSource{}
+	options := source.GetOptions().(migration.OpenstackSourceOptions)
+
+	c, err = NewClient(context.TODO(), endpoint, region, s, options)
 	if err != nil {
 		logrus.Fatal(err)
 	}
@@ -138,3 +141,62 @@ func Test_generateNetworkInfo(t *testing.T) {
 	assert.Len(vmInterfaceDetails, 2, "expected to find 2 interfaces only")
 
 }
+
+func Test_ClientOptions(t *testing.T) {
+	assert := require.New(t)
+	assert.Equal(c.options.UploadImageRetryCount, migration.OpenstackDefaultRetryCount)
+	assert.Equal(c.options.UploadImageRetryDelay, migration.OpenstackDefaultRetryDelay)
+}
+
+func Test_SourceGetOptions(t *testing.T) {
+	assert := require.New(t)
+	testCases := []struct {
+		desc     string
+		options  migration.OpenstackSourceOptions
+		expected migration.OpenstackSourceOptions
+	}{
+		{
+			desc: "custom count and delay",
+			options: migration.OpenstackSourceOptions{
+				UploadImageRetryCount: 25,
+				UploadImageRetryDelay: 15,
+			},
+			expected: migration.OpenstackSourceOptions{
+				UploadImageRetryCount: 25,
+				UploadImageRetryDelay: 15,
+			},
+		},
+		{
+			desc: "custom count and default delay",
+			options: migration.OpenstackSourceOptions{
+				UploadImageRetryCount: 100,
+			},
+			expected: migration.OpenstackSourceOptions{
+				UploadImageRetryCount: 100,
+				UploadImageRetryDelay: migration.OpenstackDefaultRetryDelay,
+			},
+		},
+		{
+			desc: "default count and custom delay",
+			options: migration.OpenstackSourceOptions{
+				UploadImageRetryDelay: 50,
+			},
+			expected: migration.OpenstackSourceOptions{
+				UploadImageRetryCount: migration.OpenstackDefaultRetryCount,
+				UploadImageRetryDelay: 50,
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		source := migration.OpenstackSource{
+			Spec: migration.OpenstackSourceSpec{
+				OpenstackSourceOptions: tc.options,
+			},
+		}
+		options := source.GetOptions().(migration.OpenstackSourceOptions)
+
+		assert.Equal(options.UploadImageRetryCount, tc.expected.UploadImageRetryCount, tc.desc)
+		assert.Equal(options.UploadImageRetryDelay, tc.expected.UploadImageRetryDelay, tc.desc)
+	}
+}