Skip to content

Commit

Permalink
EC2: HTTP Client
Browse files Browse the repository at this point in the history
The following is a work in progress, one that tries to create a client
as a dependency and NOT patch the value at test time. Instead we should
learn about our dependencies and not throw stuff at the wall and hope it
sticks!
  • Loading branch information
SimonRichardson committed Jun 16, 2021
1 parent 118a44f commit 19275d9
Show file tree
Hide file tree
Showing 13 changed files with 236 additions and 109 deletions.
16 changes: 11 additions & 5 deletions environs/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,16 +122,22 @@ type BootstrapResult struct {
CaasBootstrapFinalizer
}

// BootstrapContext is an interface that is passed to
// Environ.Bootstrap, providing a means of obtaining
// information about and manipulating the context in which
// it is being invoked.
type BootstrapContext interface {
// BootstrapLogger defines the logger used during a bootstrap.
type BootstrapLogger interface {
GetStdin() io.Reader
GetStdout() io.Writer
GetStderr() io.Writer

Infof(format string, params ...interface{})
Verbosef(format string, params ...interface{})
}

// BootstrapContext is an interface that is passed to
// Environ.Bootstrap, providing a means of obtaining
// information about and manipulating the context in which
// it is being invoked.
type BootstrapContext interface {
BootstrapLogger

// InterruptNotify starts watching for interrupt signals
// on behalf of the caller, sending them to the supplied
Expand Down
24 changes: 16 additions & 8 deletions environs/jujutest/livetests.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ type LiveTests struct {
// calls to a cloud provider.
ProviderCallContext context.ProviderCallContext

// BootstrapContext holds the context to bootstrap a test environment.
BootstrapContext environs.BootstrapContext

prepared bool
bootstrapped bool
toolsStorage storage.Storage
Expand All @@ -134,6 +137,13 @@ func (t *LiveTests) SetUpTest(c *gc.C) {
t.toolsStorage = stor
t.CleanupSuite.PatchValue(&envtools.BundleTools, envtoolstesting.GetMockBundleTools(c, nil))
t.ProviderCallContext = context.NewCloudCallContext(stdcontext.Background())

ss := simplestreams.NewSimpleStreams(sstesting.TestDataSourceFactory())

ctx := stdcontext.TODO()
ctx = stdcontext.WithValue(ctx, bootstrap.SimplestreamsFetcherContextKey, ss)

t.BootstrapContext = envtesting.BootstrapContext(ctx, c)
}

func (t *LiveTests) TearDownSuite(c *gc.C) {
Expand All @@ -155,7 +165,7 @@ func (t *LiveTests) PrepareOnce(c *gc.C) {
}

args := t.prepareForBootstrapParams(c)
e, err := bootstrap.PrepareController(false, envtesting.BootstrapContext(stdcontext.TODO(), c), t.ControllerStore, args)
e, err := bootstrap.PrepareController(false, t.BootstrapContext, t.ControllerStore, args)
c.Assert(err, gc.IsNil, gc.Commentf("preparing environ %#v", t.TestConfig))
c.Assert(e, gc.NotNil)
t.Env = e.(environs.Environ)
Expand Down Expand Up @@ -217,6 +227,7 @@ func (t *LiveTests) BootstrapOnce(c *gc.C) {
if t.bootstrapped {
return
}

t.PrepareOnce(c)
// We only build and upload tools if there will be a state agent that
// we could connect to (actual live tests, rather than local-only)
Expand All @@ -230,10 +241,7 @@ func (t *LiveTests) BootstrapOnce(c *gc.C) {
args.BootstrapConstraints = cons
args.ModelConstraints = cons

ss := simplestreams.NewSimpleStreams(sstesting.TestDataSourceFactory())
ctx := stdcontext.WithValue(stdcontext.TODO(), bootstrap.SimplestreamsFetcherContextKey, ss)

err := bootstrap.Bootstrap(envtesting.BootstrapContext(ctx, c), t.Env, t.ProviderCallContext, args)
err := bootstrap.Bootstrap(t.BootstrapContext, t.Env, t.ProviderCallContext, args)
c.Assert(err, jc.ErrorIsNil)
t.bootstrapped = true
}
Expand Down Expand Up @@ -961,7 +969,7 @@ func (t *LiveTests) TestBootstrapWithDefaultSeries(c *gc.C) {
})
args := t.prepareForBootstrapParams(c)
args.ModelConfig = dummyCfg
e, err := bootstrap.PrepareController(false, envtesting.BootstrapContext(stdcontext.TODO(), c),
e, err := bootstrap.PrepareController(false, t.BootstrapContext,
jujuclient.NewMemStore(),
args,
)
Expand All @@ -975,13 +983,13 @@ func (t *LiveTests) TestBootstrapWithDefaultSeries(c *gc.C) {
"default-series": "quantal",
})
args.ModelConfig = attrs
env, err := bootstrap.PrepareController(false, envtesting.BootstrapContext(stdcontext.TODO(), c),
env, err := bootstrap.PrepareController(false, t.BootstrapContext,
t.ControllerStore,
args)
c.Assert(err, jc.ErrorIsNil)
defer func() { _ = environs.Destroy("livetests", env, t.ProviderCallContext, t.ControllerStore) }()

err = bootstrap.Bootstrap(envtesting.BootstrapContext(stdcontext.TODO(), c), env, t.ProviderCallContext, t.bootstrapParams())
err = bootstrap.Bootstrap(t.BootstrapContext, env, t.ProviderCallContext, t.bootstrapParams())
c.Assert(err, jc.ErrorIsNil)

st := t.Env.(jujutesting.GetStater).GetStateInAPIServer()
Expand Down
2 changes: 2 additions & 0 deletions environs/jujutest/tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ func (t *Tests) Prepare(c *gc.C) environs.Environ {

// PrepareWithParams prepares an instance of the testing environment.
func (t *Tests) PrepareWithParams(c *gc.C, params bootstrap.PrepareParams) environs.Environ {
panic("X")
e, err := bootstrap.PrepareController(false, envtesting.BootstrapContext(stdcontext.TODO(), c), t.ControllerStore, params)
c.Assert(err, gc.IsNil, gc.Commentf("preparing environ %#v", params.ModelConfig))
c.Assert(e, gc.NotNil)
Expand All @@ -110,6 +111,7 @@ func (t *Tests) PrepareWithParams(c *gc.C, params bootstrap.PrepareParams) envir
}

func (t *Tests) AssertPrepareFailsWithConfig(c *gc.C, badConfig coretesting.Attrs, errorMatches string) error {
panic("Y")
args := t.PrepareParams(c)
args.ModelConfig = coretesting.Attrs(args.ModelConfig).Merge(badConfig)

Expand Down
84 changes: 84 additions & 0 deletions provider/ec2/client.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// Copyright 2020 Canonical Ltd.
// Licensed under the AGPLv3, see LICENCE file for details.

package ec2

import (
"net/http"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/client"
"github.com/aws/aws-sdk-go/aws/credentials"
"github.com/aws/aws-sdk-go/aws/session"
"github.com/aws/aws-sdk-go/service/ec2"
)

// ClientOption to be passed into the transport construction to customize the
// default transport.
type ClientOption func(*clientOptions)

type clientOptions struct {
httpClient *http.Client
}

// WithHTTPClient allows to define the http.Client to use.
func WithHTTPClient(value *http.Client) ClientOption {
return func(opt *clientOptions) {
opt.httpClient = value
}
}

// Create a clientOptions instance with default values.
func newOptions() *clientOptions {
defaultCopy := *http.DefaultClient
return &clientOptions{
httpClient: &defaultCopy,
}
}

type ClientFunc = func(string, string, string, ...ClientOption) Client

// The subset of *ec2.EC2 methods that we currently use.
type Client interface {
DescribeAvailabilityZones(*ec2.DescribeAvailabilityZonesInput) (*ec2.DescribeAvailabilityZonesOutput, error)
DescribeInstances(*ec2.DescribeInstancesInput) (*ec2.DescribeInstancesOutput, error)
DescribeInstanceTypeOfferings(*ec2.DescribeInstanceTypeOfferingsInput) (*ec2.DescribeInstanceTypeOfferingsOutput, error)
DescribeInstanceTypes(*ec2.DescribeInstanceTypesInput) (*ec2.DescribeInstanceTypesOutput, error)
DescribeSpotPriceHistory(*ec2.DescribeSpotPriceHistoryInput) (*ec2.DescribeSpotPriceHistoryOutput, error)
}

// EC2Session returns a session with the given credentials.
func clientFunc(region, accessKey, secretKey string, clientOptions ...ClientOption) Client {
panic("Z")
opts := newOptions()
for _, option := range clientOptions {
option(opts)
}

sess := session.Must(session.NewSession())
config := &aws.Config{
HTTPClient: opts.httpClient,
Retryer: client.DefaultRetryer{ // these roughly match retry params in gopkg.in/amz.v3/ec2/ec2.go:EC2.query
NumMaxRetries: 10,
MinRetryDelay: time.Second,
MinThrottleDelay: time.Second,
MaxRetryDelay: time.Minute,
MaxThrottleDelay: time.Minute,
},
Region: aws.String(region),
Credentials: credentials.NewStaticCredentialsFromCreds(credentials.Value{
AccessKeyID: accessKey,
SecretAccessKey: secretKey,
}),
}

// Enable request and response logging, but only if TRACE is enabled (as
// they're probably fairly expensive to produce).
if logger.IsTraceEnabled() {
config.Logger = awsLogger{sess}
config.LogLevel = aws.LogLevel(aws.LogDebug | aws.LogDebugWithRequestErrors | aws.LogDebugWithRequestRetries)
}

return ec2.New(sess, config)
}
2 changes: 1 addition & 1 deletion provider/ec2/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (e environProviderCredentials) DetectCredentials(cloudName string) (*cloud.
AuthCredentials: make(map[string]cloud.Credential),
}
for _, credName := range credInfo.SectionStrings() {
if credName == ini.DEFAULT_SECTION {
if credName == ini.DefaultSection {
// No credentials at top level.
continue
}
Expand Down
10 changes: 5 additions & 5 deletions provider/ec2/ebs.go
Original file line number Diff line number Diff line change
Expand Up @@ -405,8 +405,8 @@ func (v *ebsVolumeSource) createVolume(ctx context.ProviderCallContext, p storag
}

volume := storage.Volume{
p.Tag,
storage.VolumeInfo{
Tag: p.Tag,
VolumeInfo: storage.VolumeInfo{
VolumeId: volumeId,
Size: gibToMib(uint64(resp.Size)),
Persistent: true,
Expand Down Expand Up @@ -758,9 +758,9 @@ func (v *ebsVolumeSource) AttachVolumes(ctx context.ProviderCallContext, attachP
attachmentInfo.DeviceName = deviceName

results[i].VolumeAttachment = &storage.VolumeAttachment{
params.Volume,
params.Machine,
attachmentInfo,
Volume: params.Volume,
Machine: params.Machine,
VolumeAttachmentInfo: attachmentInfo,
}
}
return results, nil
Expand Down
49 changes: 36 additions & 13 deletions provider/ec2/environ.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/juju/clock"
"github.com/juju/collections/set"
"github.com/juju/errors"
jujuhttp "github.com/juju/http/v2"
"github.com/juju/names/v4"
"github.com/juju/retry"
"github.com/juju/utils/v2"
Expand All @@ -26,6 +27,7 @@ import (
"github.com/juju/juju/cloudconfig/instancecfg"
"github.com/juju/juju/cloudconfig/providerinit"
"github.com/juju/juju/core/constraints"
corecontext "github.com/juju/juju/core/context"
"github.com/juju/juju/core/instance"
"github.com/juju/juju/core/network"
"github.com/juju/juju/core/network/firewall"
Expand All @@ -43,6 +45,12 @@ import (
"github.com/juju/juju/tools"
)

const (
// AWSClientContextKey defines a way to change the aws client func within
// a context.
AWSClientContextKey corecontext.ContextKey = "aws-client-func"
)

const (
invalidParameterValue = "InvalidParameterValue"

Expand All @@ -67,16 +75,7 @@ var (
_ environs.FirewallFeatureQuerier = (*environ)(nil)
)

// The subset of *ec2.EC2 methods that we currently use.
type ec2Client interface {
DescribeAvailabilityZones(*ec2.DescribeAvailabilityZonesInput) (*ec2.DescribeAvailabilityZonesOutput, error)
DescribeInstances(*ec2.DescribeInstancesInput) (*ec2.DescribeInstancesOutput, error)
DescribeInstanceTypeOfferings(*ec2.DescribeInstanceTypeOfferingsInput) (*ec2.DescribeInstanceTypeOfferingsOutput, error)
DescribeInstanceTypes(*ec2.DescribeInstanceTypesInput) (*ec2.DescribeInstanceTypesOutput, error)
DescribeSpotPriceHistory(*ec2.DescribeSpotPriceHistoryInput) (*ec2.DescribeSpotPriceHistoryOutput, error)
}

var _ ec2Client = (*ec2.EC2)(nil)
var _ Client = (*ec2.EC2)(nil)

type environ struct {
name string
Expand All @@ -86,7 +85,8 @@ type environ struct {
// favour of the Amazon-provided awk-sdk-go (see ec2Client below).
ec2 *amzec2.EC2

ec2Client ec2Client
ec2Client Client
ec2ClientFunc ClientFunc

// ecfgMutex protects the *Unlocked fields below.
ecfgMutex sync.Mutex
Expand All @@ -105,6 +105,12 @@ type environ struct {
ensureGroupMutex sync.Mutex
}

func newEnviron() *environ {
return &environ{
ec2ClientFunc: clientFunc,
}
}

var _ environs.Environ = (*environ)(nil)
var _ environs.Networking = (*environ)(nil)

Expand Down Expand Up @@ -136,6 +142,17 @@ func (e *environ) Name() string {

// PrepareForBootstrap is part of the Environ interface.
func (e *environ) PrepareForBootstrap(ctx environs.BootstrapContext, controllerName string) error {
// Allow the passing of a client func through the context. This allows
// passing the client from outside of the environ, one that allows for
// custom http.Clients.
if value := ctx.Context().Value(AWSClientContextKey); value == nil {
e.ec2ClientFunc = clientFunc
} else if s, ok := value.(ClientFunc); ok {
e.ec2ClientFunc = s
} else {
return errors.Errorf("expected a valid client function type")
}

callCtx := context.NewCloudCallContext(ctx.Context())
// Cannot really invalidate a credential here since nothing is bootstrapped yet.
callCtx.InvalidateCredentialFunc = func(string) error { return nil }
Expand Down Expand Up @@ -282,7 +299,7 @@ type AvailabilityZoner interface {
}

func gatherAvailabilityZones(instances []instances.Instance) map[instance.Id]string {
zones := make(map[instance.Id]string, 0)
zones := make(map[instance.Id]string)
for _, inst := range instances {
if inst == nil {
continue
Expand Down Expand Up @@ -2530,7 +2547,13 @@ func (e *environ) SetCloudSpec(spec environscloudspec.CloudSpec) error {
return errors.Trace(err)
}

e.ec2Client = EC2Session(e.cloud.Region, e.ec2.AccessKey, e.ec2.SecretKey)
httpClient := jujuhttp.NewClient(
jujuhttp.WithLogger(logger.Child("http")),
)

e.ec2Client = e.ec2ClientFunc(e.cloud.Region, e.ec2.AccessKey, e.ec2.SecretKey,
WithHTTPClient(httpClient.Client()),
)

return nil
}
Expand Down
2 changes: 0 additions & 2 deletions provider/ec2/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import (
jujustorage "github.com/juju/juju/storage"
)

type EC2Client = ec2Client

func StorageEC2(vs jujustorage.VolumeSource) *amzec2.EC2 {
return vs.(*ebsVolumeSource).env.ec2
}
Expand Down
Loading

0 comments on commit 19275d9

Please sign in to comment.