From c2dcb17a3c117d9d21ebe32b4849b11c86f7b75e Mon Sep 17 00:00:00 2001 From: Vasil Averyanau Date: Tue, 10 Dec 2024 15:44:40 +0100 Subject: [PATCH 1/9] feat: adds cloudmeta package This adds cloudmeta package. For now it's only a basic interface for working with different cloud metadata providers, like aws, gcp, azure. --- pkg/cloudmeta/metadata.go | 72 +++++++++++++++++ pkg/cloudmeta/metadata_test.go | 140 +++++++++++++++++++++++++++++++++ 2 files changed, 212 insertions(+) create mode 100644 pkg/cloudmeta/metadata.go create mode 100644 pkg/cloudmeta/metadata_test.go diff --git a/pkg/cloudmeta/metadata.go b/pkg/cloudmeta/metadata.go new file mode 100644 index 000000000..3c1bb48c9 --- /dev/null +++ b/pkg/cloudmeta/metadata.go @@ -0,0 +1,72 @@ +// Copyright (C) 2024 ScyllaDB + +package cloudmeta + +import ( + "context" + "errors" + "time" +) + +// InstanceMetadata represents metadata returned by cloud provider. +type InstanceMetadata struct { + InstanceType string + CloudProvider CloudProvider +} + +// CloudProvider is enum of supported cloud providers. +type CloudProvider string + +// CloudProviderAWS represents aws provider. +var CloudProviderAWS CloudProvider = "aws" + +// CloudMetadataProvider interface that each metadata provider should implement. +type CloudMetadataProvider interface { + Metadata(ctx context.Context) (InstanceMetadata, error) +} + +// CloudMeta is a wrapper around various cloud metadata providers. +type CloudMeta struct { + providers []CloudMetadataProvider + + providerTimeout time.Duration +} + +// NewCloudMeta creates new CloudMeta provider. +func NewCloudMeta() (*CloudMeta, error) { + // providers will initialized here and added to CloudMeta.providers. + + const defaultTimeout = 5 * time.Second + + return &CloudMeta{ + providers: []CloudMetadataProvider{}, + providerTimeout: defaultTimeout, + }, nil +} + +// GetInstanceMetadata tries to fetch instance metadata from AWS, GCP, Azure providers in order. +func (cloud *CloudMeta) GetInstanceMetadata(ctx context.Context) (InstanceMetadata, error) { + var mErr error + for _, provider := range cloud.providers { + meta, err := cloud.runWithTimeout(ctx, provider) + if err != nil { + mErr = errors.Join(mErr, err) + continue + } + return meta, nil + } + + return InstanceMetadata{}, mErr +} + +func (cloud *CloudMeta) runWithTimeout(ctx context.Context, provider CloudMetadataProvider) (InstanceMetadata, error) { + ctx, cancel := context.WithTimeout(ctx, cloud.providerTimeout) + defer cancel() + + return provider.Metadata(ctx) +} + +// WithProviderTimeout sets per provider timeout. +func (cloud *CloudMeta) WithProviderTimeout(providerTimeout time.Duration) { + cloud.providerTimeout = providerTimeout +} diff --git a/pkg/cloudmeta/metadata_test.go b/pkg/cloudmeta/metadata_test.go new file mode 100644 index 000000000..0d4eb5eca --- /dev/null +++ b/pkg/cloudmeta/metadata_test.go @@ -0,0 +1,140 @@ +// Copyright (C) 2017 ScyllaDB + +package cloudmeta + +import ( + "context" + "fmt" + "testing" +) + +func TestGetInstanceMetadata(t *testing.T) { + t.Run("when there is no active providers", func(t *testing.T) { + cloudmeta := &CloudMeta{} + + meta, err := cloudmeta.GetInstanceMetadata(context.Background()) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if meta.InstanceType != "" { + t.Fatalf("meta.InstanceType should be empty, got %v", meta.InstanceType) + } + + if meta.CloudProvider != "" { + t.Fatalf("meta.CloudProvider should be empty, got %v", meta.CloudProvider) + } + }) + + t.Run("when there is only one active provider", func(t *testing.T) { + cloudmeta := &CloudMeta{ + providers: []CloudMetadataProvider{newTestProvider(t, "test_provider_1", "x-test-1", nil)}, + } + + meta, err := cloudmeta.GetInstanceMetadata(context.Background()) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + + if meta.InstanceType != "x-test-1" { + t.Fatalf("meta.InstanceType should be 'x-test-1', got %v", meta.InstanceType) + } + + if meta.CloudProvider != "test_provider_1" { + t.Fatalf("meta.CloudProvider should be 'test_provider_1', got %v", meta.CloudProvider) + } + }) + + t.Run("when there is more than one active provider", func(t *testing.T) { + cloudmeta := &CloudMeta{ + providers: []CloudMetadataProvider{ + newTestProvider(t, "test_provider_1", "x-test-1", nil), + newTestProvider(t, "test_provider_2", "x-test-2", nil), + }, + } + + // Only first one should be returned. + meta, err := cloudmeta.GetInstanceMetadata(context.Background()) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + + if meta.InstanceType != "x-test-1" { + t.Fatalf("meta.InstanceType should be 'x-test-1', got %v", meta.InstanceType) + } + + if meta.CloudProvider != "test_provider_1" { + t.Fatalf("meta.CloudProvider should be 'test_provider_1', got %v", meta.CloudProvider) + } + }) + t.Run("when there is more than one active provider, but first returns err", func(t *testing.T) { + cloudmeta := &CloudMeta{ + providers: []CloudMetadataProvider{ + newTestProvider(t, "test_provider_1", "x-test-1", fmt.Errorf("'test_provider_1' err")), + newTestProvider(t, "test_provider_2", "x-test-2", nil), + }, + } + + // Only first succesfull one should be returned. + meta, err := cloudmeta.GetInstanceMetadata(context.Background()) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + + if meta.InstanceType != "x-test-2" { + t.Fatalf("meta.InstanceType should be 'x-test-2', got %v", meta.InstanceType) + } + + if meta.CloudProvider != "test_provider_2" { + t.Fatalf("meta.CloudProvider should be 'test_provider_2', got %v", meta.CloudProvider) + } + }) + + t.Run("when there is more than one active provider, but all returns err", func(t *testing.T) { + cloudmeta := &CloudMeta{ + providers: []CloudMetadataProvider{ + newTestProvider(t, "test_provider_1", "x-test-1", fmt.Errorf("'test_provider_1' err")), + newTestProvider(t, "test_provider_2", "x-test-2", fmt.Errorf("'test_provider_2' err")), + }, + } + + // Only first succesfull one should be returned. + meta, err := cloudmeta.GetInstanceMetadata(context.Background()) + if err == nil { + t.Fatalf("expected err, but got: %v", err) + } + + if meta.InstanceType != "" { + t.Fatalf("meta.InstanceType should be empty, got %v", meta.InstanceType) + } + + if meta.CloudProvider != "" { + t.Fatalf("meta.CloudProvider should be empty, got %v", meta.CloudProvider) + } + }) +} + +func newTestProvider(t *testing.T, providerName, instanceType string, err error) *testProvider { + t.Helper() + + return &testProvider{ + name: CloudProvider(providerName), + instanceType: instanceType, + err: err, + } +} + +type testProvider struct { + name CloudProvider + instanceType string + err error +} + +func (tp testProvider) Metadata(ctx context.Context) (InstanceMetadata, error) { + if tp.err != nil { + return InstanceMetadata{}, tp.err + } + return InstanceMetadata{ + CloudProvider: tp.name, + InstanceType: tp.instanceType, + }, nil +} From 88780211beda50bc47b466acfa924778e03be1c7 Mon Sep 17 00:00:00 2001 From: Vasil Averyanau Date: Wed, 11 Dec 2024 10:12:42 +0100 Subject: [PATCH 2/9] feat: adds aws metadata provider This extends cloudmeta package with aws metadata provider. Fixes: #4127 --- pkg/cloudmeta/aws.go | 50 +++++++++++++++++++++++++++++++++++++++ pkg/cloudmeta/metadata.go | 11 ++++++--- 2 files changed, 58 insertions(+), 3 deletions(-) create mode 100644 pkg/cloudmeta/aws.go diff --git a/pkg/cloudmeta/aws.go b/pkg/cloudmeta/aws.go new file mode 100644 index 000000000..eeb38283b --- /dev/null +++ b/pkg/cloudmeta/aws.go @@ -0,0 +1,50 @@ +// Copyright (C) 2024 ScyllaDB + +package cloudmeta + +import ( + "context" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/ec2metadata" + "github.com/aws/aws-sdk-go/aws/session" + "github.com/pkg/errors" +) + +// AWSMetadata is a wrapper around ec2 metadata client. +type AWSMetadata struct { + ec2meta *ec2metadata.EC2Metadata +} + +// NewAWSMetadata is a constructor for AWSMetadata service. +// testEndpoint can be provided if you want to overwrite the default metadata endpoint, otherwise leave it empty. +func NewAWSMetadata(testEndpoint string) (*AWSMetadata, error) { + session, err := session.NewSession() + if err != nil { + return nil, errors.Wrap(err, "session.NewSession") + } + cfg := aws.NewConfig() + if testEndpoint != "" { + cfg = cfg.WithEndpoint(testEndpoint) + } + return &AWSMetadata{ + ec2meta: ec2metadata.New(session, cfg), + }, nil +} + +// Metadata return InstanceMetadata from aws if available. +func (aws *AWSMetadata) Metadata(ctx context.Context) (InstanceMetadata, error) { + if !aws.ec2meta.AvailableWithContext(ctx) { + return InstanceMetadata{}, errors.New("metadata is not available") + } + + instanceData, err := aws.ec2meta.GetInstanceIdentityDocumentWithContext(ctx) + if err != nil { + return InstanceMetadata{}, errors.Wrap(err, "aws.metadataClient.GetInstanceIdentityDocument") + } + + return InstanceMetadata{ + CloudProvider: CloudProviderAWS, + InstanceType: instanceData.InstanceType, + }, nil +} diff --git a/pkg/cloudmeta/metadata.go b/pkg/cloudmeta/metadata.go index 3c1bb48c9..4ccf8d911 100644 --- a/pkg/cloudmeta/metadata.go +++ b/pkg/cloudmeta/metadata.go @@ -34,12 +34,17 @@ type CloudMeta struct { // NewCloudMeta creates new CloudMeta provider. func NewCloudMeta() (*CloudMeta, error) { - // providers will initialized here and added to CloudMeta.providers. - const defaultTimeout = 5 * time.Second + awsMeta, err := NewAWSMetadata("") + if err != nil { + return nil, err + } + return &CloudMeta{ - providers: []CloudMetadataProvider{}, + providers: []CloudMetadataProvider{ + awsMeta, + }, providerTimeout: defaultTimeout, }, nil } From 9c78eee44e7aa37115cf45bc1c9b8c6eb445b7c0 Mon Sep 17 00:00:00 2001 From: Vasil Averyanau Date: Thu, 12 Dec 2024 11:57:35 +0100 Subject: [PATCH 3/9] refactor: adds NewAWSMetadataWithEndpoint constructor. This adds NewAWSMetadataWithEndpoint constructor, so we can have ability to overwrite default aws metadata service endpoint. It might be useful for testing. --- pkg/cloudmeta/aws.go | 19 +++++++++++++++---- pkg/cloudmeta/metadata.go | 2 +- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/pkg/cloudmeta/aws.go b/pkg/cloudmeta/aws.go index eeb38283b..4039f7c78 100644 --- a/pkg/cloudmeta/aws.go +++ b/pkg/cloudmeta/aws.go @@ -17,15 +17,26 @@ type AWSMetadata struct { } // NewAWSMetadata is a constructor for AWSMetadata service. -// testEndpoint can be provided if you want to overwrite the default metadata endpoint, otherwise leave it empty. -func NewAWSMetadata(testEndpoint string) (*AWSMetadata, error) { +func NewAWSMetadata() (*AWSMetadata, error) { + session, err := session.NewSession() + if err != nil { + return nil, errors.Wrap(err, "session.NewSession") + } + return &AWSMetadata{ + ec2meta: ec2metadata.New(session), + }, nil +} + +// NewAWSMetadataWithEndpoint is a constructor for AWSMetadata service that allows your to overwrite default metadata endpoint. +// Might be useful for tests. +func NewAWSMetadataWithEndpoint(endpoint string) (*AWSMetadata, error) { session, err := session.NewSession() if err != nil { return nil, errors.Wrap(err, "session.NewSession") } cfg := aws.NewConfig() - if testEndpoint != "" { - cfg = cfg.WithEndpoint(testEndpoint) + if endpoint != "" { + cfg = cfg.WithEndpoint(endpoint) } return &AWSMetadata{ ec2meta: ec2metadata.New(session, cfg), diff --git a/pkg/cloudmeta/metadata.go b/pkg/cloudmeta/metadata.go index 4ccf8d911..bf7b1e4ef 100644 --- a/pkg/cloudmeta/metadata.go +++ b/pkg/cloudmeta/metadata.go @@ -36,7 +36,7 @@ type CloudMeta struct { func NewCloudMeta() (*CloudMeta, error) { const defaultTimeout = 5 * time.Second - awsMeta, err := NewAWSMetadata("") + awsMeta, err := NewAWSMetadata() if err != nil { return nil, err } From 57f239d85d548a727cc0946d9b009f131c58fa9f Mon Sep 17 00:00:00 2001 From: Vasil Averyanau Date: Thu, 12 Dec 2024 12:04:49 +0100 Subject: [PATCH 4/9] refactor: returns err when CloudMeta is not initialzied with providers. --- pkg/cloudmeta/metadata.go | 6 ++++++ pkg/cloudmeta/metadata_test.go | 5 +++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/pkg/cloudmeta/metadata.go b/pkg/cloudmeta/metadata.go index bf7b1e4ef..7019bdc61 100644 --- a/pkg/cloudmeta/metadata.go +++ b/pkg/cloudmeta/metadata.go @@ -49,8 +49,14 @@ func NewCloudMeta() (*CloudMeta, error) { }, nil } +// ErrNoProviders will be returned by CloudMeta service, when it hasn't been initialized with any metadata provider. +var ErrNoProviders = errors.New("no metadata providers found") + // GetInstanceMetadata tries to fetch instance metadata from AWS, GCP, Azure providers in order. func (cloud *CloudMeta) GetInstanceMetadata(ctx context.Context) (InstanceMetadata, error) { + if len(cloud.providers) == 0 { + return InstanceMetadata{}, ErrNoProviders + } var mErr error for _, provider := range cloud.providers { meta, err := cloud.runWithTimeout(ctx, provider) diff --git a/pkg/cloudmeta/metadata_test.go b/pkg/cloudmeta/metadata_test.go index 0d4eb5eca..736cdfe03 100644 --- a/pkg/cloudmeta/metadata_test.go +++ b/pkg/cloudmeta/metadata_test.go @@ -4,6 +4,7 @@ package cloudmeta import ( "context" + "errors" "fmt" "testing" ) @@ -13,8 +14,8 @@ func TestGetInstanceMetadata(t *testing.T) { cloudmeta := &CloudMeta{} meta, err := cloudmeta.GetInstanceMetadata(context.Background()) - if err != nil { - t.Fatalf("unexpected err: %v", err) + if !errors.Is(err, ErrNoProviders) { + t.Fatalf("expected err, got: %v", err) } if meta.InstanceType != "" { t.Fatalf("meta.InstanceType should be empty, got %v", meta.InstanceType) From 8d86c326216d46f354517973648db9df5969e8b8 Mon Sep 17 00:00:00 2001 From: Vasil Averyanau Date: Thu, 12 Dec 2024 12:17:45 +0100 Subject: [PATCH 5/9] refactor: adds empty lines to metadata_test for consistency. --- pkg/cloudmeta/metadata_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/cloudmeta/metadata_test.go b/pkg/cloudmeta/metadata_test.go index 736cdfe03..940e20524 100644 --- a/pkg/cloudmeta/metadata_test.go +++ b/pkg/cloudmeta/metadata_test.go @@ -17,6 +17,7 @@ func TestGetInstanceMetadata(t *testing.T) { if !errors.Is(err, ErrNoProviders) { t.Fatalf("expected err, got: %v", err) } + if meta.InstanceType != "" { t.Fatalf("meta.InstanceType should be empty, got %v", meta.InstanceType) } From ac8ff49784fdcf18e794724d440f0666a9b28035 Mon Sep 17 00:00:00 2001 From: Vasil Averyanau Date: Thu, 12 Dec 2024 15:49:07 +0100 Subject: [PATCH 6/9] refactor: addresses code review comments. This makes following changes: 1. Use pkg/errors instead of stdlib error 2. Wrap all errors with WithStack 3. Make aws metadata service private 4. Make GetInstanceMetadata to request all providers concurrently 5. Use table driven tests --- pkg/cloudmeta/aws.go | 29 ++---- pkg/cloudmeta/metadata.go | 46 +++++++-- pkg/cloudmeta/metadata_test.go | 184 ++++++++++++++++----------------- 3 files changed, 129 insertions(+), 130 deletions(-) diff --git a/pkg/cloudmeta/aws.go b/pkg/cloudmeta/aws.go index 4039f7c78..c50b0dd46 100644 --- a/pkg/cloudmeta/aws.go +++ b/pkg/cloudmeta/aws.go @@ -5,46 +5,29 @@ package cloudmeta import ( "context" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/ec2metadata" "github.com/aws/aws-sdk-go/aws/session" "github.com/pkg/errors" ) -// AWSMetadata is a wrapper around ec2 metadata client. -type AWSMetadata struct { +// awsMetadata is a wrapper around ec2 metadata client. +type awsMetadata struct { ec2meta *ec2metadata.EC2Metadata } -// NewAWSMetadata is a constructor for AWSMetadata service. -func NewAWSMetadata() (*AWSMetadata, error) { +// newAWSMetadata is a constructor for AWSMetadata service. +func newAWSMetadata() (*awsMetadata, error) { session, err := session.NewSession() if err != nil { return nil, errors.Wrap(err, "session.NewSession") } - return &AWSMetadata{ + return &awsMetadata{ ec2meta: ec2metadata.New(session), }, nil } -// NewAWSMetadataWithEndpoint is a constructor for AWSMetadata service that allows your to overwrite default metadata endpoint. -// Might be useful for tests. -func NewAWSMetadataWithEndpoint(endpoint string) (*AWSMetadata, error) { - session, err := session.NewSession() - if err != nil { - return nil, errors.Wrap(err, "session.NewSession") - } - cfg := aws.NewConfig() - if endpoint != "" { - cfg = cfg.WithEndpoint(endpoint) - } - return &AWSMetadata{ - ec2meta: ec2metadata.New(session, cfg), - }, nil -} - // Metadata return InstanceMetadata from aws if available. -func (aws *AWSMetadata) Metadata(ctx context.Context) (InstanceMetadata, error) { +func (aws *awsMetadata) Metadata(ctx context.Context) (InstanceMetadata, error) { if !aws.ec2meta.AvailableWithContext(ctx) { return InstanceMetadata{}, errors.New("metadata is not available") } diff --git a/pkg/cloudmeta/metadata.go b/pkg/cloudmeta/metadata.go index 7019bdc61..1940af945 100644 --- a/pkg/cloudmeta/metadata.go +++ b/pkg/cloudmeta/metadata.go @@ -4,8 +4,10 @@ package cloudmeta import ( "context" - "errors" "time" + + "github.com/pkg/errors" + "go.uber.org/multierr" ) // InstanceMetadata represents metadata returned by cloud provider. @@ -36,7 +38,7 @@ type CloudMeta struct { func NewCloudMeta() (*CloudMeta, error) { const defaultTimeout = 5 * time.Second - awsMeta, err := NewAWSMetadata() + awsMeta, err := newAWSMetadata() if err != nil { return nil, err } @@ -52,21 +54,45 @@ func NewCloudMeta() (*CloudMeta, error) { // ErrNoProviders will be returned by CloudMeta service, when it hasn't been initialized with any metadata provider. var ErrNoProviders = errors.New("no metadata providers found") -// GetInstanceMetadata tries to fetch instance metadata from AWS, GCP, Azure providers in order. +// GetInstanceMetadata tries to fetch instance metadata from AWS, GCP, Azure concurrently and returns first result. func (cloud *CloudMeta) GetInstanceMetadata(ctx context.Context) (InstanceMetadata, error) { if len(cloud.providers) == 0 { - return InstanceMetadata{}, ErrNoProviders + return InstanceMetadata{}, errors.WithStack(ErrNoProviders) } - var mErr error + + type msg struct { + meta InstanceMetadata + err error + } + + ctx, cancel := context.WithCancel(ctx) + defer cancel() + + results := make(chan msg, len(cloud.providers)) for _, provider := range cloud.providers { - meta, err := cloud.runWithTimeout(ctx, provider) - if err != nil { - mErr = errors.Join(mErr, err) + go func(provider CloudMetadataProvider) { + meta, err := cloud.runWithTimeout(ctx, provider) + + select { + case <-ctx.Done(): + return + default: + } + + results <- msg{meta: meta, err: err} + }(provider) + } + + // Return the first non error result or wait until all providers return err. + var mErr error + for range len(cloud.providers) { + res := <-results + if res.err != nil { + mErr = multierr.Append(mErr, res.err) continue } - return meta, nil + return res.meta, nil } - return InstanceMetadata{}, mErr } diff --git a/pkg/cloudmeta/metadata_test.go b/pkg/cloudmeta/metadata_test.go index 940e20524..6c06e290a 100644 --- a/pkg/cloudmeta/metadata_test.go +++ b/pkg/cloudmeta/metadata_test.go @@ -5,122 +5,109 @@ package cloudmeta import ( "context" "errors" - "fmt" "testing" + "time" ) func TestGetInstanceMetadata(t *testing.T) { - t.Run("when there is no active providers", func(t *testing.T) { - cloudmeta := &CloudMeta{} - - meta, err := cloudmeta.GetInstanceMetadata(context.Background()) - if !errors.Is(err, ErrNoProviders) { - t.Fatalf("expected err, got: %v", err) - } - - if meta.InstanceType != "" { - t.Fatalf("meta.InstanceType should be empty, got %v", meta.InstanceType) - } - - if meta.CloudProvider != "" { - t.Fatalf("meta.CloudProvider should be empty, got %v", meta.CloudProvider) - } - }) - - t.Run("when there is only one active provider", func(t *testing.T) { - cloudmeta := &CloudMeta{ - providers: []CloudMetadataProvider{newTestProvider(t, "test_provider_1", "x-test-1", nil)}, - } - - meta, err := cloudmeta.GetInstanceMetadata(context.Background()) - if err != nil { - t.Fatalf("unexpected err: %v", err) - } - - if meta.InstanceType != "x-test-1" { - t.Fatalf("meta.InstanceType should be 'x-test-1', got %v", meta.InstanceType) - } - - if meta.CloudProvider != "test_provider_1" { - t.Fatalf("meta.CloudProvider should be 'test_provider_1', got %v", meta.CloudProvider) - } - }) - - t.Run("when there is more than one active provider", func(t *testing.T) { - cloudmeta := &CloudMeta{ + testCases := []struct { + name string + providers []CloudMetadataProvider + + expectedErr bool + expectedMeta InstanceMetadata + }{ + { + name: "when there is no active providers", + providers: nil, + + expectedErr: true, + expectedMeta: InstanceMetadata{}, + }, + { + name: "when there is one active providers", providers: []CloudMetadataProvider{ - newTestProvider(t, "test_provider_1", "x-test-1", nil), - newTestProvider(t, "test_provider_2", "x-test-2", nil), + newTestProvider(t, "test_provider_1", "x-test-1", 1*time.Millisecond, nil), }, - } - - // Only first one should be returned. - meta, err := cloudmeta.GetInstanceMetadata(context.Background()) - if err != nil { - t.Fatalf("unexpected err: %v", err) - } - - if meta.InstanceType != "x-test-1" { - t.Fatalf("meta.InstanceType should be 'x-test-1', got %v", meta.InstanceType) - } - - if meta.CloudProvider != "test_provider_1" { - t.Fatalf("meta.CloudProvider should be 'test_provider_1', got %v", meta.CloudProvider) - } - }) - t.Run("when there is more than one active provider, but first returns err", func(t *testing.T) { - cloudmeta := &CloudMeta{ + + expectedErr: false, + expectedMeta: InstanceMetadata{ + CloudProvider: "test_provider_1", + InstanceType: "x-test-1", + }, + }, + { + name: "when there is more than one active provider, fastest should be returned", providers: []CloudMetadataProvider{ - newTestProvider(t, "test_provider_1", "x-test-1", fmt.Errorf("'test_provider_1' err")), - newTestProvider(t, "test_provider_2", "x-test-2", nil), + newTestProvider(t, "test_provider_1", "x-test-1", 1*time.Millisecond, nil), + newTestProvider(t, "test_provider_2", "x-test-2", 2*time.Millisecond, nil), }, - } - - // Only first succesfull one should be returned. - meta, err := cloudmeta.GetInstanceMetadata(context.Background()) - if err != nil { - t.Fatalf("unexpected err: %v", err) - } - if meta.InstanceType != "x-test-2" { - t.Fatalf("meta.InstanceType should be 'x-test-2', got %v", meta.InstanceType) - } - - if meta.CloudProvider != "test_provider_2" { - t.Fatalf("meta.CloudProvider should be 'test_provider_2', got %v", meta.CloudProvider) - } - }) + expectedErr: false, + expectedMeta: InstanceMetadata{ + CloudProvider: "test_provider_1", + InstanceType: "x-test-1", + }, + }, + { + name: "when there is more than one active provider, but fastest returns err", + providers: []CloudMetadataProvider{ + newTestProvider(t, "test_provider_1", "x-test-1", 1*time.Millisecond, errors.New("something went wront")), + newTestProvider(t, "test_provider_2", "x-test-2", 2*time.Millisecond, nil), + }, - t.Run("when there is more than one active provider, but all returns err", func(t *testing.T) { - cloudmeta := &CloudMeta{ + expectedErr: false, + expectedMeta: InstanceMetadata{ + CloudProvider: "test_provider_2", + InstanceType: "x-test-2", + }, + }, + { + name: "when there is more than one active provider, but all returns err", providers: []CloudMetadataProvider{ - newTestProvider(t, "test_provider_1", "x-test-1", fmt.Errorf("'test_provider_1' err")), - newTestProvider(t, "test_provider_2", "x-test-2", fmt.Errorf("'test_provider_2' err")), + newTestProvider(t, "test_provider_1", "x-test-1", 1*time.Millisecond, errors.New("err provider1")), + newTestProvider(t, "test_provider_2", "x-test-2", 1*time.Millisecond, errors.New("err provider2")), }, - } - - // Only first succesfull one should be returned. - meta, err := cloudmeta.GetInstanceMetadata(context.Background()) - if err == nil { - t.Fatalf("expected err, but got: %v", err) - } - - if meta.InstanceType != "" { - t.Fatalf("meta.InstanceType should be empty, got %v", meta.InstanceType) - } - - if meta.CloudProvider != "" { - t.Fatalf("meta.CloudProvider should be empty, got %v", meta.CloudProvider) - } - }) + + expectedErr: true, + expectedMeta: InstanceMetadata{}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + cloudmeta := &CloudMeta{ + providers: tc.providers, + } + + meta, err := cloudmeta.GetInstanceMetadata(context.Background()) + + if tc.expectedErr && err == nil { + t.Fatalf("expected error, got: %v", err) + } + + if !tc.expectedErr && err != nil { + t.Fatalf("unexpected error, got: %v", err) + } + + if tc.expectedMeta.InstanceType != meta.InstanceType { + t.Fatalf("unexpected meta.InstanceType: %s != %s", tc.expectedMeta.InstanceType, meta.InstanceType) + } + + if tc.expectedMeta.CloudProvider != meta.CloudProvider { + t.Fatalf("unexpected meta.CloudProvider: %s != %s", tc.expectedMeta.CloudProvider, meta.CloudProvider) + } + }) + } } -func newTestProvider(t *testing.T, providerName, instanceType string, err error) *testProvider { +func newTestProvider(t *testing.T, providerName, instanceType string, latency time.Duration, err error) *testProvider { t.Helper() return &testProvider{ name: CloudProvider(providerName), instanceType: instanceType, + latency: latency, err: err, } } @@ -128,10 +115,13 @@ func newTestProvider(t *testing.T, providerName, instanceType string, err error) type testProvider struct { name CloudProvider instanceType string + latency time.Duration err error } func (tp testProvider) Metadata(ctx context.Context) (InstanceMetadata, error) { + time.Sleep(tp.latency) + if tp.err != nil { return InstanceMetadata{}, tp.err } From 4e4967517387177f0e0659584f0a105b8b1f36f9 Mon Sep 17 00:00:00 2001 From: Vasil Averyanau Date: Thu, 12 Dec 2024 16:00:10 +0100 Subject: [PATCH 7/9] fix: fixes copyright year in metadata_test :D --- pkg/cloudmeta/metadata_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cloudmeta/metadata_test.go b/pkg/cloudmeta/metadata_test.go index 6c06e290a..363819400 100644 --- a/pkg/cloudmeta/metadata_test.go +++ b/pkg/cloudmeta/metadata_test.go @@ -1,4 +1,4 @@ -// Copyright (C) 2017 ScyllaDB +// Copyright (C) 2024 ScyllaDB package cloudmeta From d85eeeadf1b7c3d87522de218b519ee00be079e4 Mon Sep 17 00:00:00 2001 From: Vasil Averyanau Date: Fri, 13 Dec 2024 09:36:16 +0100 Subject: [PATCH 8/9] refactor: simplifies sending to the channel --- pkg/cloudmeta/metadata.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/cloudmeta/metadata.go b/pkg/cloudmeta/metadata.go index 1940af945..73b6ce0fd 100644 --- a/pkg/cloudmeta/metadata.go +++ b/pkg/cloudmeta/metadata.go @@ -76,10 +76,9 @@ func (cloud *CloudMeta) GetInstanceMetadata(ctx context.Context) (InstanceMetada select { case <-ctx.Done(): return - default: + case results <- msg{meta: meta, err: err}: } - results <- msg{meta: meta, err: err} }(provider) } From 0cd8ad1733e85d2c69c8102118e3a8949fb7ec2d Mon Sep 17 00:00:00 2001 From: Vasil Averyanau Date: Fri, 13 Dec 2024 10:09:10 +0100 Subject: [PATCH 9/9] refactor: makes liner happy: removes unnecessary trailling newline. --- pkg/cloudmeta/metadata.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/cloudmeta/metadata.go b/pkg/cloudmeta/metadata.go index 73b6ce0fd..a0507248c 100644 --- a/pkg/cloudmeta/metadata.go +++ b/pkg/cloudmeta/metadata.go @@ -78,7 +78,6 @@ func (cloud *CloudMeta) GetInstanceMetadata(ctx context.Context) (InstanceMetada return case results <- msg{meta: meta, err: err}: } - }(provider) }