From de07e8f859e0e182155790a220e208237386f668 Mon Sep 17 00:00:00 2001 From: Volker Theile Date: Tue, 1 Oct 2024 13:08:41 +0200 Subject: [PATCH] Make timeout during waiting for uploadImage configurable Add the fields `UploadImageRetryCount` and `UploadImageRetryDelay` to `OpenstackSourceSpec`. Related to: https://github.com/harvester/harvester/issues/6675 Signed-off-by: Volker Theile --- .../v1beta1/common.go | 5 ++ .../v1beta1/openstack.go | 34 +++++++++- .../v1beta1/vmware.go | 4 ++ pkg/controllers/migration/openstack.go | 2 +- pkg/controllers/migration/virtualmachine.go | 3 +- pkg/source/openstack/client.go | 20 +++--- pkg/source/openstack/client_test.go | 64 ++++++++++++++++++- 7 files changed, 117 insertions(+), 15 deletions(-) diff --git a/pkg/apis/migration.harvesterhci.io/v1beta1/common.go b/pkg/apis/migration.harvesterhci.io/v1beta1/common.go index cff0cbc..bc2965d 100644 --- 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 index 1daeeae..6d1f5ff 100644 --- 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 index 6394133..aae1828 100644 --- 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 index 2b6c139..ea36017 100644 --- 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 index 5fa48b5..db9c428 100644 --- 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 index 3bbbb7a..d0ebfaa 100644 --- 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 index 123f773..32d1cd6 100644 --- 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) + } +}