diff --git a/controllers/awscluster_controller.go b/controllers/awscluster_controller.go index 48caa16b76..18acedd881 100644 --- a/controllers/awscluster_controller.go +++ b/controllers/awscluster_controller.go @@ -30,6 +30,7 @@ import ( "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/scope" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/ec2" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/elb" + "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/network" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/conditions" @@ -141,6 +142,8 @@ func reconcileDelete(clusterScope *scope.ClusterScope) (reconcile.Result, error) ec2svc := ec2.NewService(clusterScope) elbsvc := elb.NewService(clusterScope) + networkSvc := network.NewService(clusterScope) + awsCluster := clusterScope.AWSCluster if err := elbsvc.DeleteLoadbalancers(); err != nil { @@ -151,7 +154,7 @@ func reconcileDelete(clusterScope *scope.ClusterScope) (reconcile.Result, error) return reconcile.Result{}, errors.Wrapf(err, "error deleting bastion for AWSCluster %s/%s", awsCluster.Namespace, awsCluster.Name) } - if err := ec2svc.DeleteNetwork(); err != nil { + if err := networkSvc.DeleteNetwork(); err != nil { return reconcile.Result{}, errors.Wrapf(err, "error deleting network for AWSCluster %s/%s", awsCluster.Namespace, awsCluster.Name) } @@ -176,8 +179,9 @@ func reconcileNormal(clusterScope *scope.ClusterScope) (reconcile.Result, error) ec2Service := ec2.NewService(clusterScope) elbService := elb.NewService(clusterScope) + networkSvc := network.NewService(clusterScope) - if err := ec2Service.ReconcileNetwork(); err != nil { + if err := networkSvc.ReconcileNetwork(); err != nil { return reconcile.Result{}, errors.Wrapf(err, "failed to reconcile network for AWSCluster %s/%s", awsCluster.Namespace, awsCluster.Name) } diff --git a/pkg/cloud/interfaces.go b/pkg/cloud/interfaces.go new file mode 100644 index 0000000000..b5a2815eaf --- /dev/null +++ b/pkg/cloud/interfaces.go @@ -0,0 +1,76 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cloud + +import ( + "github.com/go-logr/logr" + + "sigs.k8s.io/cluster-api/util/conditions" + "sigs.k8s.io/controller-runtime/pkg/client" + + awsclient "github.com/aws/aws-sdk-go/aws/client" + + infrav1 "sigs.k8s.io/cluster-api-provider-aws/api/v1alpha3" + clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" +) + +// Session represents an AWS session +type Session interface { + Session() awsclient.ConfigProvider +} + +// ScopeUsage is used to indicate which controller is using a scope +type ScopeUsage interface { + // ControllerName returns the name of the controller that created the scope + ControllerName() string +} + +// ClusterObject represents a AWS cluster object +type ClusterObject interface { + conditions.Setter +} + +// ClusterScoper is the interface for a cluster scope +type ClusterScoper interface { + logr.Logger + Session + ScopeUsage + + // Name returns the cluster name. + Name() string + // Namespace returns the cluster namespace. + Namespace() string + // Region returns the cluster region. + Region() string + + // InfraCluster returns the AWS infrastructure cluster object. + InfraCluster() ClusterObject + + // ListOptionsLabelSelector returns a ListOptions with a label selector for clusterName. + ListOptionsLabelSelector() client.ListOption + // APIServerPort returns the port to use when communicating with the API server. + APIServerPort() int32 + // AdditionalTags returns any tags that you would like to attach to AWS resources. The returned value will never be nil. + AdditionalTags() infrav1.Tags + // SetFailureDomain sets the infrastructure provider failure domain key to the spec given as input. + SetFailureDomain(id string, spec clusterv1.FailureDomainSpec) + + // PatchObject persists the cluster configuration and status. + PatchObject() error + // Close closes the current scope persisting the cluster configuration and status. + Close() error +} diff --git a/pkg/cloud/scope/clients.go b/pkg/cloud/scope/clients.go index ca4c806f61..c565f1bc9e 100644 --- a/pkg/cloud/scope/clients.go +++ b/pkg/cloud/scope/clients.go @@ -17,16 +17,79 @@ limitations under the License. package scope import ( + "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/aws/request" + "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/ec2/ec2iface" + "github.com/aws/aws-sdk-go/service/elb" "github.com/aws/aws-sdk-go/service/elb/elbiface" + "github.com/aws/aws-sdk-go/service/resourcegroupstaggingapi" "github.com/aws/aws-sdk-go/service/resourcegroupstaggingapi/resourcegroupstaggingapiiface" + "github.com/aws/aws-sdk-go/service/secretsmanager" "github.com/aws/aws-sdk-go/service/secretsmanager/secretsmanageriface" + + "k8s.io/apimachinery/pkg/runtime" + + "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud" + awsmetrics "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/metrics" + "sigs.k8s.io/cluster-api-provider-aws/pkg/record" + "sigs.k8s.io/cluster-api-provider-aws/version" ) -// AWSClients contains all the aws clients used by the scopes. -type AWSClients struct { - EC2 ec2iface.EC2API - ELB elbiface.ELBAPI - SecretsManager secretsmanageriface.SecretsManagerAPI - ResourceTagging resourcegroupstaggingapiiface.ResourceGroupsTaggingAPIAPI +// NewEC2Client creates a new EC2 API client for a given session +func NewEC2Client(scopeUser cloud.ScopeUsage, session cloud.Session, target runtime.Object) ec2iface.EC2API { + ec2Client := ec2.New(session.Session()) + ec2Client.Handlers.Build.PushFrontNamed(getUserAgentHandler()) + ec2Client.Handlers.CompleteAttempt.PushFront(awsmetrics.CaptureRequestMetrics(scopeUser.ControllerName())) + ec2Client.Handlers.Complete.PushBack(recordAWSPermissionsIssue(target)) + + return ec2Client +} + +// NewELBClient creates a new ELB API client for a given session +func NewELBClient(scopeUser cloud.ScopeUsage, session cloud.Session, target runtime.Object) elbiface.ELBAPI { + elbClient := elb.New(session.Session()) + elbClient.Handlers.Build.PushFrontNamed(getUserAgentHandler()) + elbClient.Handlers.CompleteAttempt.PushFront(awsmetrics.CaptureRequestMetrics(scopeUser.ControllerName())) + elbClient.Handlers.Complete.PushBack(recordAWSPermissionsIssue(target)) + + return elbClient +} + +// NewResourgeTaggingClient creates a new Resource Tagging API client for a given session +func NewResourgeTaggingClient(scopeUser cloud.ScopeUsage, session cloud.Session, target runtime.Object) resourcegroupstaggingapiiface.ResourceGroupsTaggingAPIAPI { + resourceTagging := resourcegroupstaggingapi.New(session.Session()) + resourceTagging.Handlers.Build.PushFrontNamed(getUserAgentHandler()) + resourceTagging.Handlers.CompleteAttempt.PushFront(awsmetrics.CaptureRequestMetrics(scopeUser.ControllerName())) + resourceTagging.Handlers.Complete.PushBack(recordAWSPermissionsIssue(target)) + + return resourceTagging +} + +// NewSecretsManagerClient creates a new Secrets API client for a given session +func NewSecretsManagerClient(scopeUser cloud.ScopeUsage, session cloud.Session, target runtime.Object) secretsmanageriface.SecretsManagerAPI { + secretsClient := secretsmanager.New(session.Session()) + secretsClient.Handlers.Build.PushFrontNamed(getUserAgentHandler()) + secretsClient.Handlers.CompleteAttempt.PushFront(awsmetrics.CaptureRequestMetrics(scopeUser.ControllerName())) + secretsClient.Handlers.Complete.PushBack(recordAWSPermissionsIssue(target)) + + return secretsClient +} + +func recordAWSPermissionsIssue(target runtime.Object) func(r *request.Request) { + return func(r *request.Request) { + if awsErr, ok := r.Error.(awserr.Error); ok { + switch awsErr.Code() { + case "AuthFailure", "UnauthorizedOperation", "NoCredentialProviders": + record.Warnf(target, awsErr.Code(), "Operation %s failed with a credentials or permission issue", r.Operation.Name) + } + } + } +} + +func getUserAgentHandler() request.NamedHandler { + return request.NamedHandler{ + Name: "capa/user-agent", + Fn: request.MakeAddToUserAgentHandler("aws.cluster.x-k8s.io", version.Get().String()), + } } diff --git a/pkg/cloud/scope/cluster.go b/pkg/cloud/scope/cluster.go index 9a97485d23..c5a58d7b1b 100644 --- a/pkg/cloud/scope/cluster.go +++ b/pkg/cloud/scope/cluster.go @@ -20,20 +20,12 @@ import ( "context" "fmt" - "github.com/aws/aws-sdk-go/aws/awserr" - "github.com/aws/aws-sdk-go/aws/request" - "github.com/aws/aws-sdk-go/service/ec2" - "github.com/aws/aws-sdk-go/service/elb" - "github.com/aws/aws-sdk-go/service/resourcegroupstaggingapi" - "github.com/aws/aws-sdk-go/service/secretsmanager" + awsclient "github.com/aws/aws-sdk-go/aws/client" "github.com/go-logr/logr" "github.com/pkg/errors" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/klog/klogr" infrav1 "sigs.k8s.io/cluster-api-provider-aws/api/v1alpha3" - awsmetrics "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/metrics" - "sigs.k8s.io/cluster-api-provider-aws/pkg/record" - "sigs.k8s.io/cluster-api-provider-aws/version" + "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/controller-runtime/pkg/client" @@ -41,12 +33,12 @@ import ( // ClusterScopeParams defines the input parameters used to create a new Scope. type ClusterScopeParams struct { - AWSClients Client client.Client Logger logr.Logger Cluster *clusterv1.Cluster AWSCluster *infrav1.AWSCluster ControllerName string + Session awsclient.ConfigProvider } // NewClusterScope creates a new Scope from the supplied parameters. @@ -68,77 +60,32 @@ func NewClusterScope(params ClusterScopeParams) (*ClusterScope, error) { return nil, errors.Errorf("failed to create aws session: %v", err) } - userAgentHandler := request.NamedHandler{ - Name: "capa/user-agent", - Fn: request.MakeAddToUserAgentHandler("aws.cluster.x-k8s.io", version.Get().String()), - } - - if params.AWSClients.EC2 == nil { - ec2Client := ec2.New(session) - ec2Client.Handlers.Build.PushFrontNamed(userAgentHandler) - ec2Client.Handlers.CompleteAttempt.PushFront(awsmetrics.CaptureRequestMetrics(params.ControllerName)) - ec2Client.Handlers.Complete.PushBack(recordAWSPermissionsIssue(params.AWSCluster)) - params.AWSClients.EC2 = ec2Client - } - - if params.AWSClients.ELB == nil { - elbClient := elb.New(session) - elbClient.Handlers.Build.PushFrontNamed(userAgentHandler) - elbClient.Handlers.CompleteAttempt.PushFront(awsmetrics.CaptureRequestMetrics(params.ControllerName)) - elbClient.Handlers.Complete.PushBack(recordAWSPermissionsIssue(params.AWSCluster)) - params.AWSClients.ELB = elbClient - } - - if params.AWSClients.ResourceTagging == nil { - resourceTagging := resourcegroupstaggingapi.New(session) - resourceTagging.Handlers.Build.PushFrontNamed(userAgentHandler) - resourceTagging.Handlers.CompleteAttempt.PushFront(awsmetrics.CaptureRequestMetrics(params.ControllerName)) - resourceTagging.Handlers.Complete.PushBack(recordAWSPermissionsIssue(params.AWSCluster)) - params.AWSClients.ResourceTagging = resourceTagging - } - - if params.AWSClients.SecretsManager == nil { - sClient := secretsmanager.New(session) - sClient.Handlers.Build.PushFrontNamed(userAgentHandler) - sClient.Handlers.CompleteAttempt.PushFront(awsmetrics.CaptureRequestMetrics(params.ControllerName)) - sClient.Handlers.Complete.PushBack(recordAWSPermissionsIssue(params.AWSCluster)) - params.AWSClients.SecretsManager = sClient - } - helper, err := patch.NewHelper(params.AWSCluster, params.Client) if err != nil { return nil, errors.Wrap(err, "failed to init patch helper") } return &ClusterScope{ - Logger: params.Logger, - client: params.Client, - AWSClients: params.AWSClients, - Cluster: params.Cluster, - AWSCluster: params.AWSCluster, - patchHelper: helper, + Logger: params.Logger, + client: params.Client, + Cluster: params.Cluster, + AWSCluster: params.AWSCluster, + patchHelper: helper, + session: session, + controllerName: params.ControllerName, }, nil } -func recordAWSPermissionsIssue(target runtime.Object) func(r *request.Request) { - return func(r *request.Request) { - if awsErr, ok := r.Error.(awserr.Error); ok { - switch awsErr.Code() { - case "AuthFailure", "UnauthorizedOperation", "NoCredentialProviders": - record.Warnf(target, awsErr.Code(), "Operation %s failed with a credentials or permission issue", r.Operation.Name) - } - } - } -} - // ClusterScope defines the basic context for an actuator to operate upon. type ClusterScope struct { logr.Logger client client.Client patchHelper *patch.Helper - AWSClients Cluster *clusterv1.Cluster AWSCluster *infrav1.AWSCluster + + session awsclient.ConfigProvider + controllerName string } // Network returns the cluster network object. @@ -156,6 +103,11 @@ func (s *ClusterScope) Subnets() infrav1.Subnets { return s.AWSCluster.Spec.NetworkSpec.Subnets } +// SetSubnets updates the clusters subnets. +func (s *ClusterScope) SetSubnets(subnets infrav1.Subnets) { + s.AWSCluster.Spec.NetworkSpec.Subnets = subnets +} + // CNIIngressRules returns the CNI spec ingress rules. func (s *ClusterScope) CNIIngressRules() infrav1.CNIIngressRules { if s.AWSCluster.Spec.NetworkSpec.CNI != nil { @@ -256,3 +208,36 @@ func (s *ClusterScope) SetFailureDomain(id string, spec clusterv1.FailureDomainS } s.AWSCluster.Status.FailureDomains[id] = spec } + +// InfraCluster returns the AWS infrastructure cluster object. +// Initially this will be AWSCluster but in the future it +// could also be AWSManagedCluster +func (s *ClusterScope) InfraCluster() cloud.ClusterObject { + return s.AWSCluster +} + +// Session returns the AWS SDK session. Used for creating clients +func (s *ClusterScope) Session() awsclient.ConfigProvider { + return s.session +} + +// Bastion returns the bastion details. +func (s *ClusterScope) Bastion() *infrav1.Bastion { + return &s.AWSCluster.Spec.Bastion +} + +// SetBastionInstance sets the bastion instance in the status of the cluster. +func (s *ClusterScope) SetBastionInstance(instance *infrav1.Instance) { + s.AWSCluster.Status.Bastion = instance +} + +// SSHKeyName returns the SSH key name to use for instances. +func (s *ClusterScope) SSHKeyName() *string { + return s.AWSCluster.Spec.SSHKeyName +} + +// ControllerName returns the name of the controller that +// created the ClusterScope. +func (s *ClusterScope) ControllerName() string { + return s.controllerName +} diff --git a/pkg/cloud/scope/machine.go b/pkg/cloud/scope/machine.go index 209b72ce3a..2d5e38d316 100644 --- a/pkg/cloud/scope/machine.go +++ b/pkg/cloud/scope/machine.go @@ -38,7 +38,6 @@ import ( // MachineScopeParams defines the input parameters used to create a new MachineScope. type MachineScopeParams struct { - AWSClients Client client.Client Logger logr.Logger Cluster *clusterv1.Cluster diff --git a/pkg/cloud/services/ec2/ami.go b/pkg/cloud/services/ec2/ami.go index 52f8f0258f..7453c6f9a2 100644 --- a/pkg/cloud/services/ec2/ami.go +++ b/pkg/cloud/services/ec2/ami.go @@ -114,9 +114,9 @@ func (s *Service) defaultAMILookup(amiNameFormat, ownerID, baseOS, kubernetesVer }, } - out, err := s.scope.EC2.DescribeImages(describeImageInput) + out, err := s.EC2Client.DescribeImages(describeImageInput) if err != nil { - record.Eventf(s.scope.AWSCluster, "FailedDescribeImages", "Failed to find ami %q: %v", amiName, err) + record.Eventf(s.scope.InfraCluster(), "FailedDescribeImages", "Failed to find ami %q: %v", amiName, err) return "", errors.Wrapf(err, "failed to find ami: %q", amiName) } if len(out.Images) == 0 { diff --git a/pkg/cloud/services/ec2/ami_test.go b/pkg/cloud/services/ec2/ami_test.go index 867905157d..b2d1018daf 100644 --- a/pkg/cloud/services/ec2/ami_test.go +++ b/pkg/cloud/services/ec2/ami_test.go @@ -68,9 +68,6 @@ func TestAMIs(t *testing.T) { scope, err := scope.NewClusterScope(scope.ClusterScopeParams{ Cluster: &clusterv1.Cluster{}, AWSCluster: &infrav1.AWSCluster{}, - AWSClients: scope.AWSClients{ - EC2: ec2Mock, - }, }) if err != nil { t.Fatalf("did not expect err: %v", err) @@ -79,6 +76,8 @@ func TestAMIs(t *testing.T) { tc.expect(ec2Mock.EXPECT()) s := NewService(scope) + s.EC2Client = ec2Mock + id, err := s.defaultAMILookup("", "", "base os-baseos version", "1.11.1") if err != nil { t.Fatalf("did not expect error calling a mock: %v", err) @@ -129,9 +128,6 @@ func TestAMIsWithInvalidCreationDate(t *testing.T) { scope, err := scope.NewClusterScope(scope.ClusterScopeParams{ Cluster: &clusterv1.Cluster{}, AWSCluster: &infrav1.AWSCluster{}, - AWSClients: scope.AWSClients{ - EC2: ec2Mock, - }, }) if err != nil { t.Fatalf("did not expect err: %v", err) @@ -140,6 +136,8 @@ func TestAMIsWithInvalidCreationDate(t *testing.T) { tc.expect(ec2Mock.EXPECT()) s := NewService(scope) + s.EC2Client = ec2Mock + _, err = s.defaultAMILookup("", "", "base os-baseos version", "1.11.1") if err == nil { t.Fatalf("expected an error but did not get one") diff --git a/pkg/cloud/services/ec2/bastion.go b/pkg/cloud/services/ec2/bastion.go index af88f68dde..0b0365bdb2 100644 --- a/pkg/cloud/services/ec2/bastion.go +++ b/pkg/cloud/services/ec2/bastion.go @@ -39,7 +39,7 @@ const ( // ReconcileBastion ensures a bastion is created for the cluster func (s *Service) ReconcileBastion() error { - if !s.scope.AWSCluster.Spec.Bastion.Enabled { + if !s.scope.Bastion().Enabled { s.scope.V(4).Info("Skipping bastion reconcile") _, err := s.describeBastionInstance() if err != nil { @@ -66,19 +66,19 @@ func (s *Service) ReconcileBastion() error { // Describe bastion instance, if any. instance, err := s.describeBastionInstance() if awserrors.IsNotFound(err) { // nolint:nestif - if !conditions.Has(s.scope.AWSCluster, infrav1.BastionHostReadyCondition) { - conditions.MarkFalse(s.scope.AWSCluster, infrav1.BastionHostReadyCondition, infrav1.BastionCreationStartedReason, clusterv1.ConditionSeverityInfo, "") + if !conditions.Has(s.scope.InfraCluster(), infrav1.BastionHostReadyCondition) { + conditions.MarkFalse(s.scope.InfraCluster(), infrav1.BastionHostReadyCondition, infrav1.BastionCreationStartedReason, clusterv1.ConditionSeverityInfo, "") if err := s.scope.PatchObject(); err != nil { return errors.Wrap(err, "failed to patch conditions") } } instance, err = s.runInstance("bastion", spec) if err != nil { - record.Warnf(s.scope.AWSCluster, "FailedCreateBastion", "Failed to create bastion instance: %v", err) + record.Warnf(s.scope.InfraCluster(), "FailedCreateBastion", "Failed to create bastion instance: %v", err) return err } - record.Eventf(s.scope.AWSCluster, "SuccessfulCreateBastion", "Created bastion instance %q", instance.ID) + record.Eventf(s.scope.InfraCluster(), "SuccessfulCreateBastion", "Created bastion instance %q", instance.ID) s.scope.V(2).Info("Created new bastion host", "instance", instance) } else if err != nil { @@ -87,8 +87,8 @@ func (s *Service) ReconcileBastion() error { // TODO(vincepri): check for possible changes between the default spec and the instance. - s.scope.AWSCluster.Status.Bastion = instance.DeepCopy() - conditions.MarkTrue(s.scope.AWSCluster, infrav1.BastionHostReadyCondition) + s.scope.SetBastionInstance(instance.DeepCopy()) + conditions.MarkTrue(s.scope.InfraCluster(), infrav1.BastionHostReadyCondition) s.scope.V(2).Info("Reconcile bastion completed successfully") return nil @@ -106,10 +106,10 @@ func (s *Service) DeleteBastion() error { } if err := s.TerminateInstanceAndWait(instance.ID); err != nil { - record.Warnf(s.scope.AWSCluster, "FailedTerminateBastion", "Failed to terminate bastion instance %q: %v", instance.ID, err) + record.Warnf(s.scope.InfraCluster(), "FailedTerminateBastion", "Failed to terminate bastion instance %q: %v", instance.ID, err) return errors.Wrap(err, "unable to delete bastion instance") } - record.Eventf(s.scope.AWSCluster, "SuccessfulTerminateBastion", "Terminated bastion instance %q", instance.ID) + record.Eventf(s.scope.InfraCluster(), "SuccessfulTerminateBastion", "Terminated bastion instance %q", instance.ID) return nil } @@ -128,9 +128,9 @@ func (s *Service) describeBastionInstance() (*infrav1.Instance, error) { }, } - out, err := s.scope.EC2.DescribeInstances(input) + out, err := s.EC2Client.DescribeInstances(input) if err != nil { - record.Eventf(s.scope.AWSCluster, "FailedDescribeBastionHost", "Failed to describe bastion host: %v", err) + record.Eventf(s.scope.InfraCluster(), "FailedDescribeBastionHost", "Failed to describe bastion host: %v", err) return nil, errors.Wrap(err, "failed to describe bastion host") } @@ -152,7 +152,7 @@ func (s *Service) getDefaultBastion() *infrav1.Instance { userData, _ := userdata.NewBastion(&userdata.BastionInput{}) // If SSHKeyName WAS NOT provided, use the defaultSSHKeyName - keyName := s.scope.AWSCluster.Spec.SSHKeyName + keyName := s.scope.SSHKeyName() if keyName == nil { keyName = aws.String(defaultSSHKeyName) } @@ -160,7 +160,7 @@ func (s *Service) getDefaultBastion() *infrav1.Instance { i := &infrav1.Instance{ Type: "t2.micro", SubnetID: s.scope.Subnets().FilterPublic()[0].ID, - ImageID: s.defaultBastionAMILookup(s.scope.AWSCluster.Spec.Region), + ImageID: s.defaultBastionAMILookup(s.scope.Region()), SSHKeyName: keyName, UserData: aws.String(base64.StdEncoding.EncodeToString([]byte(userData))), SecurityGroupIDs: []string{ diff --git a/pkg/cloud/services/ec2/bastion_test.go b/pkg/cloud/services/ec2/bastion_test.go index 7238288191..b693524926 100644 --- a/pkg/cloud/services/ec2/bastion_test.go +++ b/pkg/cloud/services/ec2/bastion_test.go @@ -168,9 +168,6 @@ func TestDeleteBastion(t *testing.T) { ec2Mock := mock_ec2iface.NewMockEC2API(mockControl) scope, err := scope.NewClusterScope(scope.ClusterScopeParams{ - AWSClients: scope.AWSClients{ - EC2: ec2Mock, - }, Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Namespace: "ns", @@ -197,6 +194,7 @@ func TestDeleteBastion(t *testing.T) { tc.expect(ec2Mock.EXPECT()) s := NewService(scope) + s.EC2Client = ec2Mock err = s.DeleteBastion() if tc.expectError { diff --git a/pkg/cloud/services/ec2/console.go b/pkg/cloud/services/ec2/console.go index c93c4c5215..8449192cfb 100644 --- a/pkg/cloud/services/ec2/console.go +++ b/pkg/cloud/services/ec2/console.go @@ -32,9 +32,9 @@ func (s *Service) GetConsoleOutput(instanceID string) (string, error) { Latest: aws.Bool(true), } - out, err := s.scope.EC2.GetConsoleOutput(input) + out, err := s.EC2Client.GetConsoleOutput(input) if err != nil { - record.Eventf(s.scope.AWSCluster, "FailedGetConsoleOutput", "failed to get console output for instance %q: %v", instanceID, err) + record.Eventf(s.scope.InfraCluster(), "FailedGetConsoleOutput", "failed to get console output for instance %q: %v", instanceID, err) return "", errors.Wrapf(err, "failed to get console output for instance %q", instanceID) } diff --git a/pkg/cloud/services/ec2/instances.go b/pkg/cloud/services/ec2/instances.go index 948ee7fc8f..5ba4051f46 100644 --- a/pkg/cloud/services/ec2/instances.go +++ b/pkg/cloud/services/ec2/instances.go @@ -52,12 +52,12 @@ func (s *Service) GetRunningInstanceByTags(scope *scope.MachineScope) (*infrav1. }, } - out, err := s.scope.EC2.DescribeInstances(input) + out, err := s.EC2Client.DescribeInstances(input) switch { case awserrors.IsNotFound(err): return nil, nil case err != nil: - record.Eventf(s.scope.AWSCluster, "FailedDescribeInstances", "Failed to describe instances by tags: %v", err) + record.Eventf(s.scope.InfraCluster(), "FailedDescribeInstances", "Failed to describe instances by tags: %v", err) return nil, errors.Wrap(err, "failed to describe instances by tags") } @@ -86,12 +86,12 @@ func (s *Service) InstanceIfExists(id *string) (*infrav1.Instance, error) { InstanceIds: []*string{id}, } - out, err := s.scope.EC2.DescribeInstances(input) + out, err := s.EC2Client.DescribeInstances(input) switch { case awserrors.IsNotFound(err): return nil, nil case err != nil: - record.Eventf(s.scope.AWSCluster, "FailedDescribeInstances", "failed to describe instance %q: %v", *id, err) + record.Eventf(s.scope.InfraCluster(), "FailedDescribeInstances", "failed to describe instance %q: %v", *id, err) return nil, errors.Wrapf(err, "failed to describe instance: %q", *id) } @@ -194,7 +194,7 @@ func (s *Service) CreateInstance(scope *scope.MachineScope, userData []byte) (*i case input.SubnetID == "": sns := s.scope.Subnets().FilterPrivate() if len(sns) == 0 { - record.Eventf(s.scope.AWSCluster, "FailedCreateInstance", "Failed to run machine %q, no subnets available", scope.Name()) + record.Eventf(s.scope.InfraCluster(), "FailedCreateInstance", "Failed to run machine %q, no subnets available", scope.Name()) return nil, awserrors.NewFailedDependency( errors.Errorf("failed to run machine %q, no subnets available", scope.Name()), ) @@ -203,7 +203,7 @@ func (s *Service) CreateInstance(scope *scope.MachineScope, userData []byte) (*i } if s.scope.Network().APIServerELB.DNSName == "" { - record.Eventf(s.scope.AWSCluster, "FailedCreateInstance", "Failed to run controlplane, APIServer ELB not available") + record.Eventf(s.scope.InfraCluster(), "FailedCreateInstance", "Failed to run controlplane, APIServer ELB not available") return nil, awserrors.NewFailedDependency( errors.New("failed to run controlplane, APIServer ELB not available"), ) @@ -296,7 +296,7 @@ func (s *Service) TerminateInstance(instanceID string) error { InstanceIds: aws.StringSlice([]string{instanceID}), } - if _, err := s.scope.EC2.TerminateInstances(input); err != nil { + if _, err := s.EC2Client.TerminateInstances(input); err != nil { return errors.Wrapf(err, "failed to terminate instance with id %q", instanceID) } @@ -317,7 +317,7 @@ func (s *Service) TerminateInstanceAndWait(instanceID string) error { InstanceIds: aws.StringSlice([]string{instanceID}), } - if err := s.scope.EC2.WaitUntilInstanceTerminated(input); err != nil { + if err := s.EC2Client.WaitUntilInstanceTerminated(input); err != nil { return errors.Wrapf(err, "failed to wait for instance %q termination", instanceID) } @@ -416,7 +416,7 @@ func (s *Service) runInstance(role string, i *infrav1.Instance) (*infrav1.Instan input.TagSpecifications = append(input.TagSpecifications, spec) } - out, err := s.scope.EC2.RunInstances(input) + out, err := s.EC2Client.RunInstances(input) if err != nil { return nil, errors.Wrap(err, "failed to run instance") } @@ -430,10 +430,10 @@ func (s *Service) runInstance(role string, i *infrav1.Instance) (*infrav1.Instan ctx, cancel := context.WithTimeout(aws.BackgroundContext(), waitTimeout) defer cancel() - if err := s.scope.EC2.WaitUntilInstanceRunningWithContext( + if err := s.EC2Client.WaitUntilInstanceRunningWithContext( ctx, &ec2.DescribeInstancesInput{InstanceIds: []*string{out.Instances[0].InstanceId}}, - request.WithWaiterLogger(&awslog{s.scope.Logger}), + request.WithWaiterLogger(&awslog{s.scope}), ); err != nil { s.scope.V(2).Info("Could not determine if Machine is running. Machine state might be unavailable until next renconciliation.") } @@ -511,7 +511,7 @@ func (s *Service) UpdateResourceTags(resourceID *string, create, remove map[stri } // Create/Update tags in AWS. - if _, err := s.scope.EC2.CreateTags(input); err != nil { + if _, err := s.EC2Client.CreateTags(input); err != nil { return errors.Wrapf(err, "failed to create tags for resource %q: %+v", *resourceID, create) } } @@ -530,7 +530,7 @@ func (s *Service) UpdateResourceTags(resourceID *string, create, remove map[stri } // Delete tags in AWS. - if _, err := s.scope.EC2.DeleteTags(input); err != nil { + if _, err := s.EC2Client.DeleteTags(input); err != nil { return errors.Wrapf(err, "failed to delete tags for resource %q: %v", *resourceID, remove) } } @@ -548,7 +548,7 @@ func (s *Service) getInstanceENIs(instanceID string) ([]*ec2.NetworkInterface, e }, } - output, err := s.scope.EC2.DescribeNetworkInterfaces(input) + output, err := s.EC2Client.DescribeNetworkInterfaces(input) if err != nil { return nil, err } @@ -561,7 +561,7 @@ func (s *Service) getImageRootDevice(imageID string) (*string, error) { ImageIds: []*string{aws.String(imageID)}, } - output, err := s.scope.EC2.DescribeImages(input) + output, err := s.EC2Client.DescribeImages(input) if err != nil { return nil, err } @@ -578,7 +578,7 @@ func (s *Service) getImageSnapshotSize(imageID string) (*int64, error) { ImageIds: []*string{aws.String(imageID)}, } - output, err := s.scope.EC2.DescribeImages(input) + output, err := s.EC2Client.DescribeImages(input) if err != nil { return nil, err } @@ -668,7 +668,7 @@ func (s *Service) getNetworkInterfaceSecurityGroups(interfaceID string) ([]strin NetworkInterfaceId: aws.String(interfaceID), } - output, err := s.scope.EC2.DescribeNetworkInterfaceAttribute(input) + output, err := s.EC2Client.DescribeNetworkInterfaceAttribute(input) if err != nil { return nil, err } @@ -708,7 +708,7 @@ func (s *Service) attachSecurityGroupsToNetworkInterface(groups []string, interf Groups: aws.StringSlice(totalGroups), } - if _, err := s.scope.EC2.ModifyNetworkInterfaceAttribute(input); err != nil { + if _, err := s.EC2Client.ModifyNetworkInterfaceAttribute(input); err != nil { return errors.Wrapf(err, "failed to modify interface %q to have security groups %v", interfaceID, totalGroups) } return nil @@ -732,7 +732,7 @@ func (s *Service) DetachSecurityGroupsFromNetworkInterface(groups []string, inte Groups: aws.StringSlice(remainingGroups), } - if _, err := s.scope.EC2.ModifyNetworkInterfaceAttribute(input); err != nil { + if _, err := s.EC2Client.ModifyNetworkInterfaceAttribute(input); err != nil { return errors.Wrapf(err, "failed to modify interface %q", interfaceID) } return nil diff --git a/pkg/cloud/services/ec2/instances_test.go b/pkg/cloud/services/ec2/instances_test.go index 853e48c07a..62cf8f4119 100644 --- a/pkg/cloud/services/ec2/instances_test.go +++ b/pkg/cloud/services/ec2/instances_test.go @@ -26,12 +26,12 @@ import ( "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/utils/pointer" infrav1 "sigs.k8s.io/cluster-api-provider-aws/api/v1alpha3" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/awserrors" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/scope" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/ec2/mock_ec2iface" - "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/elb/mock_elbiface" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -160,14 +160,9 @@ func TestInstanceIfExists(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { ec2Mock := mock_ec2iface.NewMockEC2API(mockCtrl) - elbMock := mock_elbiface.NewMockELBAPI(mockCtrl) scope, err := scope.NewClusterScope(scope.ClusterScopeParams{ Cluster: &clusterv1.Cluster{}, - AWSClients: scope.AWSClients{ - EC2: ec2Mock, - ELB: elbMock, - }, AWSCluster: &infrav1.AWSCluster{ Spec: infrav1.AWSClusterSpec{ NetworkSpec: infrav1.NetworkSpec{ @@ -185,6 +180,8 @@ func TestInstanceIfExists(t *testing.T) { tc.expect(ec2Mock.EXPECT()) s := NewService(scope) + s.EC2Client = ec2Mock + instance, err := s.InstanceIfExists(&tc.instanceID) tc.check(instance, err) }) @@ -238,13 +235,8 @@ func TestTerminateInstance(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { ec2Mock := mock_ec2iface.NewMockEC2API(mockCtrl) - elbMock := mock_elbiface.NewMockELBAPI(mockCtrl) scope, err := scope.NewClusterScope(scope.ClusterScopeParams{ - AWSClients: scope.AWSClients{ - EC2: ec2Mock, - ELB: elbMock, - }, Cluster: &clusterv1.Cluster{}, AWSCluster: &infrav1.AWSCluster{}, }) @@ -255,6 +247,8 @@ func TestTerminateInstance(t *testing.T) { tc.expect(ec2Mock.EXPECT()) s := NewService(scope) + s.EC2Client = ec2Mock + err = s.TerminateInstance(tc.instanceID) tc.check(err) }) @@ -895,7 +889,11 @@ func TestCreateInstance(t *testing.T) { t.Run(tc.name, func(t *testing.T) { mockCtrl := gomock.NewController(t) ec2Mock := mock_ec2iface.NewMockEC2API(mockCtrl) - elbMock := mock_elbiface.NewMockELBAPI(mockCtrl) + + scheme, err := setupScheme() + if err != nil { + t.Fatalf("failed to create scheme: %v", err) + } cluster := &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ @@ -931,14 +929,10 @@ func TestCreateInstance(t *testing.T) { }, } - client := fake.NewFakeClient(secret, cluster, machine) + client := fake.NewFakeClientWithScheme(scheme, secret, cluster, machine) machineScope, err := scope.NewMachineScope(scope.MachineScopeParams{ - Client: client, - AWSClients: scope.AWSClients{ - EC2: ec2Mock, - ELB: elbMock, - }, + Client: client, Cluster: cluster, Machine: machine, AWSMachine: awsMachine, @@ -951,11 +945,7 @@ func TestCreateInstance(t *testing.T) { tc.expect(ec2Mock.EXPECT()) clusterScope, err := scope.NewClusterScope(scope.ClusterScopeParams{ - Client: client, - AWSClients: scope.AWSClients{ - EC2: ec2Mock, - ELB: elbMock, - }, + Client: client, Cluster: cluster, AWSCluster: tc.awsCluster, }) @@ -964,8 +954,21 @@ func TestCreateInstance(t *testing.T) { } s := NewService(clusterScope) + s.EC2Client = ec2Mock + instance, err := s.CreateInstance(machineScope, []byte("userData")) tc.check(instance, err) }) } } + +func setupScheme() (*runtime.Scheme, error) { + scheme := runtime.NewScheme() + if err := clusterv1.AddToScheme(scheme); err != nil { + return nil, err + } + if err := corev1.AddToScheme(scheme); err != nil { + return nil, err + } + return scheme, nil +} diff --git a/pkg/cloud/services/ec2/service.go b/pkg/cloud/services/ec2/service.go index 80167ad7a6..c90492fdf7 100644 --- a/pkg/cloud/services/ec2/service.go +++ b/pkg/cloud/services/ec2/service.go @@ -17,23 +17,51 @@ limitations under the License. package ec2 import ( + "github.com/aws/aws-sdk-go/service/ec2/ec2iface" + + infrav1 "sigs.k8s.io/cluster-api-provider-aws/api/v1alpha3" + "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/scope" ) -const ( - temporaryResourceID = "temporary-resource-id" -) +// Scope is the interface for the scoep to be used with the ec2 service +type Scope interface { + cloud.ClusterScoper + + // VPC returns the cluster VPC. + VPC() *infrav1.VPCSpec + + // Subnets returns the cluster subnets. + Subnets() infrav1.Subnets + + // Network returns the cluster network object. + Network() *infrav1.Network + + // SecurityGroups returns the cluster security groups as a map, it creates the map if empty. + SecurityGroups() map[infrav1.SecurityGroupRole]infrav1.SecurityGroup + + // Bastion returns the bastion details for the cluster. + Bastion() *infrav1.Bastion + + // SetBastionInstance sets the bastion instance in the status of the cluster. + SetBastionInstance(instance *infrav1.Instance) + + // SSHKeyName returns the SSH key name to use for instances. + SSHKeyName() *string +} // Service holds a collection of interfaces. // The interfaces are broken down like this to group functions together. // One alternative is to have a large list of functions from the ec2 client. type Service struct { - scope *scope.ClusterScope + scope Scope + EC2Client ec2iface.EC2API } // NewService returns a new service given the ec2 api client. -func NewService(scope *scope.ClusterScope) *Service { +func NewService(clusterScope Scope) *Service { return &Service{ - scope: scope, + scope: clusterScope, + EC2Client: scope.NewEC2Client(clusterScope, clusterScope, clusterScope.InfraCluster()), } } diff --git a/pkg/cloud/services/elb/loadbalancer.go b/pkg/cloud/services/elb/loadbalancer.go index 95e1388f03..3a2c73bc44 100644 --- a/pkg/cloud/services/elb/loadbalancer.go +++ b/pkg/cloud/services/elb/loadbalancer.go @@ -77,7 +77,7 @@ func (s *Service) ReconcileLoadbalancers() error { // Reconcile the subnets and availability zones from the spec // and the ones currently attached to the load balancer. if len(apiELB.SubnetIDs) != len(spec.SubnetIDs) { - _, err := s.scope.ELB.AttachLoadBalancerToSubnets(&elb.AttachLoadBalancerToSubnetsInput{ + _, err := s.ELBClient.AttachLoadBalancerToSubnets(&elb.AttachLoadBalancerToSubnetsInput{ LoadBalancerName: &apiELB.Name, Subnets: aws.StringSlice(spec.SubnetIDs), }) @@ -91,7 +91,7 @@ func (s *Service) ReconcileLoadbalancers() error { // Reconcile the security groups from the spec and the ones currently attached to the load balancer if !sets.NewString(apiELB.SecurityGroupIDs...).Equal(sets.NewString(spec.SecurityGroupIDs...)) { - _, err := s.scope.ELB.ApplySecurityGroupsToLoadBalancer(&elb.ApplySecurityGroupsToLoadBalancerInput{ + _, err := s.ELBClient.ApplySecurityGroupsToLoadBalancer(&elb.ApplySecurityGroupsToLoadBalancerInput{ LoadBalancerName: &apiELB.Name, SecurityGroups: aws.StringSlice(spec.SecurityGroupIDs), }) @@ -159,7 +159,7 @@ func (s *Service) RegisterInstanceWithClassicELB(instanceID, loadBalancer string LoadBalancerName: aws.String(loadBalancer), } - _, err := s.scope.ELB.RegisterInstancesWithLoadBalancer(input) + _, err := s.ELBClient.RegisterInstancesWithLoadBalancer(input) if err != nil { return err } @@ -178,7 +178,7 @@ func (s *Service) InstanceIsRegisteredWithAPIServerELB(i *infrav1.Instance) (boo LoadBalancerNames: aws.StringSlice([]string{name}), } - output, err := s.scope.ELB.DescribeLoadBalancers(input) + output, err := s.ELBClient.DescribeLoadBalancers(input) if err != nil { return false, errors.Wrapf(err, "error describing ELB %q", name) } @@ -228,7 +228,7 @@ func (s *Service) RegisterInstanceWithAPIServerELB(i *infrav1.Instance) error { LoadBalancerName: aws.String(name), } - _, err = s.scope.ELB.RegisterInstancesWithLoadBalancer(input) + _, err = s.ELBClient.RegisterInstancesWithLoadBalancer(input) return err } @@ -244,7 +244,7 @@ func (s *Service) DeregisterInstanceFromAPIServerELB(i *infrav1.Instance) error LoadBalancerName: aws.String(name), } - _, err = s.scope.ELB.DeregisterInstancesFromLoadBalancer(input) + _, err = s.ELBClient.DeregisterInstancesFromLoadBalancer(input) if err != nil { if aerr, ok := err.(awserr.Error); ok { switch aerr.Code() { @@ -324,8 +324,8 @@ func (s *Service) getAPIServerClassicELBSpec() (*infrav1.ClassicELB, error) { }, } - if s.scope.AWSCluster.Spec.ControlPlaneLoadBalancer != nil { - res.Attributes.CrossZoneLoadBalancing = s.scope.AWSCluster.Spec.ControlPlaneLoadBalancer.CrossZoneLoadBalancing + if s.scope.ControlPlaneLoadBalancer() != nil { + res.Attributes.CrossZoneLoadBalancing = s.scope.ControlPlaneLoadBalancer().CrossZoneLoadBalancing } res.Tags = infrav1.Build(infrav1.BuildParams{ @@ -376,14 +376,14 @@ func (s *Service) createClassicELB(spec *infrav1.ClassicELB) (*infrav1.ClassicEL }) } - out, err := s.scope.ELB.CreateLoadBalancer(input) + out, err := s.ELBClient.CreateLoadBalancer(input) if err != nil { return nil, errors.Wrapf(err, "failed to create classic load balancer: %v", spec) } if spec.HealthCheck != nil { if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) { - if _, err := s.scope.ELB.ConfigureHealthCheck(&elb.ConfigureHealthCheckInput{ + if _, err := s.ELBClient.ConfigureHealthCheck(&elb.ConfigureHealthCheckInput{ LoadBalancerName: aws.String(spec.Name), HealthCheck: &elb.HealthCheck{ Target: aws.String(spec.HealthCheck.Target), @@ -425,7 +425,7 @@ func (s *Service) configureAttributes(name string, attributes infrav1.ClassicELB } if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) { - if _, err := s.scope.ELB.ModifyLoadBalancerAttributes(attrs); err != nil { + if _, err := s.ELBClient.ModifyLoadBalancerAttributes(attrs); err != nil { return false, err } return true, nil @@ -441,7 +441,7 @@ func (s *Service) deleteClassicELB(name string) error { LoadBalancerName: aws.String(name), } - if _, err := s.scope.ELB.DeleteLoadBalancer(input); err != nil { + if _, err := s.ELBClient.DeleteLoadBalancer(input); err != nil { return err } return nil @@ -460,7 +460,7 @@ func (s *Service) listByTag(tag string) ([]string, error) { names := []string{} - err := s.scope.ResourceTagging.GetResourcesPages(&input, func(r *rgapi.GetResourcesOutput, last bool) bool { + err := s.ResourceTaggingClient.GetResourcesPages(&input, func(r *rgapi.GetResourcesOutput, last bool) bool { for _, tagmapping := range r.ResourceTagMappingList { if tagmapping.ResourceARN != nil { // We can't use arn.Parse because the "Resource" is loadbalancer/ @@ -476,7 +476,7 @@ func (s *Service) listByTag(tag string) ([]string, error) { return true }) if err != nil { - record.Eventf(s.scope.AWSCluster, "FailedListELBsByTag", "Failed to list %s ELB by Tags: %v", s.scope.Name(), err) + record.Eventf(s.scope.InfraCluster(), "FailedListELBsByTag", "Failed to list %s ELB by Tags: %v", s.scope.Name(), err) return nil, errors.Wrapf(err, "failed to list %s ELBs by tag group", s.scope.Name()) } @@ -506,7 +506,7 @@ func (s *Service) describeClassicELB(name string) (*infrav1.ClassicELB, error) { LoadBalancerNames: aws.StringSlice([]string{name}), } - out, err := s.scope.ELB.DescribeLoadBalancers(input) + out, err := s.ELBClient.DescribeLoadBalancers(input) if err != nil { if aerr, ok := err.(awserr.Error); ok { switch aerr.Code() { @@ -532,7 +532,7 @@ func (s *Service) describeClassicELB(name string) (*infrav1.ClassicELB, error) { name, *out.LoadBalancerDescriptions[0].VPCId) } - outAtt, err := s.scope.ELB.DescribeLoadBalancerAttributes(&elb.DescribeLoadBalancerAttributesInput{ + outAtt, err := s.ELBClient.DescribeLoadBalancerAttributes(&elb.DescribeLoadBalancerAttributesInput{ LoadBalancerName: aws.String(name), }) if err != nil { @@ -543,7 +543,7 @@ func (s *Service) describeClassicELB(name string) (*infrav1.ClassicELB, error) { } func (s *Service) reconcileELBTags(name string, desiredTags map[string]string) error { - tags, err := s.scope.ELB.DescribeTags(&elb.DescribeTagsInput{ + tags, err := s.ELBClient.DescribeTags(&elb.DescribeTagsInput{ LoadBalancerNames: []*string{aws.String(name)}, }) if err != nil { @@ -579,13 +579,13 @@ func (s *Service) reconcileELBTags(name string, desiredTags map[string]string) e } if len(addTagsInput.Tags) > 0 { - if _, err := s.scope.ELB.AddTags(addTagsInput); err != nil { + if _, err := s.ELBClient.AddTags(addTagsInput); err != nil { return err } } if len(removeTagsInput.Tags) > 0 { - if _, err := s.scope.ELB.RemoveTags(removeTagsInput); err != nil { + if _, err := s.ELBClient.RemoveTags(removeTagsInput); err != nil { return err } } diff --git a/pkg/cloud/services/elb/service.go b/pkg/cloud/services/elb/service.go index 9c8c701755..aa3657d04d 100644 --- a/pkg/cloud/services/elb/service.go +++ b/pkg/cloud/services/elb/service.go @@ -17,19 +17,51 @@ limitations under the License. package elb import ( + "github.com/aws/aws-sdk-go/service/elb/elbiface" + "github.com/aws/aws-sdk-go/service/resourcegroupstaggingapi/resourcegroupstaggingapiiface" + + infrav1 "sigs.k8s.io/cluster-api-provider-aws/api/v1alpha3" + "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/scope" ) +// Scope is a scope for use with the ELB reconciling service +type Scope interface { + cloud.ClusterScoper + + // Network returns the cluster network object. + Network() *infrav1.Network + + // Subnets returns the cluster subnets. + Subnets() infrav1.Subnets + + // SecurityGroups returns the cluster security groups as a map, it creates the map if empty. + SecurityGroups() map[infrav1.SecurityGroupRole]infrav1.SecurityGroup + + // VPC returns the cluster VPC. + VPC() *infrav1.VPCSpec + + // ControlPlaneLoadBalancer returns the AWSLoadBalancerSpec + ControlPlaneLoadBalancer() *infrav1.AWSLoadBalancerSpec + + // ControlPlaneLoadBalancerScheme returns the Classic ELB scheme (public or internal facing) + ControlPlaneLoadBalancerScheme() infrav1.ClassicELBScheme +} + // Service holds a collection of interfaces. // The interfaces are broken down like this to group functions together. // One alternative is to have a large list of functions from the ec2 client. type Service struct { - scope *scope.ClusterScope + scope Scope + ELBClient elbiface.ELBAPI + ResourceTaggingClient resourcegroupstaggingapiiface.ResourceGroupsTaggingAPIAPI } // NewService returns a new service given the api clients. -func NewService(scope *scope.ClusterScope) *Service { +func NewService(elbScope Scope) *Service { return &Service{ - scope: scope, + scope: elbScope, + ELBClient: scope.NewELBClient(elbScope, elbScope, elbScope.InfraCluster()), + ResourceTaggingClient: scope.NewResourgeTaggingClient(elbScope, elbScope, elbScope.InfraCluster()), } } diff --git a/pkg/cloud/services/ec2/account.go b/pkg/cloud/services/network/account.go similarity index 85% rename from pkg/cloud/services/ec2/account.go rename to pkg/cloud/services/network/account.go index 65496acfb3..fc1f40c9cf 100644 --- a/pkg/cloud/services/ec2/account.go +++ b/pkg/cloud/services/network/account.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package ec2 +package network import ( "sort" @@ -26,11 +26,11 @@ import ( ) func (s *Service) getAvailableZones() ([]string, error) { - out, err := s.scope.EC2.DescribeAvailabilityZones(&ec2.DescribeAvailabilityZonesInput{ + out, err := s.EC2Client.DescribeAvailabilityZones(&ec2.DescribeAvailabilityZonesInput{ Filters: []*ec2.Filter{filter.EC2.Available()}, }) if err != nil { - record.Eventf(s.scope.AWSCluster, "FailedDescribeAvailableZone", "Failed getting available zones: %v", err) + record.Eventf(s.scope.InfraCluster(), "FailedDescribeAvailableZone", "Failed getting available zones: %v", err) return nil, errors.Wrap(err, "failed to describe availability zones") } diff --git a/pkg/cloud/services/ec2/eips.go b/pkg/cloud/services/network/eips.go similarity index 79% rename from pkg/cloud/services/ec2/eips.go rename to pkg/cloud/services/network/eips.go index 7935f84ed8..feb07c9370 100644 --- a/pkg/cloud/services/ec2/eips.go +++ b/pkg/cloud/services/network/eips.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package ec2 +package network import ( "fmt" @@ -33,7 +33,7 @@ import ( func (s *Service) getOrAllocateAddresses(num int, role string) (eips []string, err error) { out, err := s.describeAddresses(role) if err != nil { - record.Eventf(s.scope.AWSCluster, "FailedDescribeAddresses", "Failed to query addresses for role %q: %v", role, err) + record.Eventf(s.scope.InfraCluster(), "FailedDescribeAddresses", "Failed to query addresses for role %q: %v", role, err) return nil, errors.Wrap(err, "failed to query addresses") } @@ -55,17 +55,17 @@ func (s *Service) getOrAllocateAddresses(num int, role string) (eips []string, e } func (s *Service) allocateAddress(role string) (string, error) { - out, err := s.scope.EC2.AllocateAddress(&ec2.AllocateAddressInput{ + out, err := s.EC2Client.AllocateAddress(&ec2.AllocateAddressInput{ Domain: aws.String("vpc"), }) if err != nil { - record.Warnf(s.scope.AWSCluster, "FailedAllocateEIP", "Failed to allocate Elastic IP for %q: %v", role, err) + record.Warnf(s.scope.InfraCluster(), "FailedAllocateEIP", "Failed to allocate Elastic IP for %q: %v", role, err) return "", errors.Wrap(err, "failed to allocate Elastic IP") } if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) { if err := tags.Apply(&tags.ApplyParams{ - EC2Client: s.scope.EC2, + EC2Client: s.EC2Client, BuildParams: infrav1.BuildParams{ ClusterName: s.scope.Name(), ResourceID: *out.AllocationId, @@ -79,7 +79,7 @@ func (s *Service) allocateAddress(role string) (string, error) { } return true, nil }, awserrors.EIPNotFound); err != nil { - record.Eventf(s.scope.AWSCluster, "FailedAllocateAddress", "Failed to tag elastic IP %q: %v", aws.StringValue(out.AllocationId), err) + record.Eventf(s.scope.InfraCluster(), "FailedAllocateAddress", "Failed to tag elastic IP %q: %v", aws.StringValue(out.AllocationId), err) return "", errors.Wrapf(err, "failed to tag Elastic IP %q", aws.StringValue(out.AllocationId)) } @@ -92,14 +92,14 @@ func (s *Service) describeAddresses(role string) (*ec2.DescribeAddressesOutput, x = append(x, filter.EC2.ProviderRole(role)) } - return s.scope.EC2.DescribeAddresses(&ec2.DescribeAddressesInput{ + return s.EC2Client.DescribeAddresses(&ec2.DescribeAddressesInput{ Filters: x, }) } func (s *Service) disassociateAddress(ip *ec2.Address) error { err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) { - _, err := s.scope.EC2.DisassociateAddress(&ec2.DisassociateAddressInput{ + _, err := s.EC2Client.DisassociateAddress(&ec2.DisassociateAddressInput{ AssociationId: ip.AssociationId, }) if err != nil { @@ -111,14 +111,14 @@ func (s *Service) disassociateAddress(ip *ec2.Address) error { return true, nil }, awserrors.AuthFailure) if err != nil { - record.Warnf(s.scope.AWSCluster, "FailedDisassociateEIP", "Failed to disassociate Elastic IP %q: %v", *ip.AllocationId, err) + record.Warnf(s.scope.InfraCluster(), "FailedDisassociateEIP", "Failed to disassociate Elastic IP %q: %v", *ip.AllocationId, err) return errors.Wrapf(err, "failed to disassociate Elastic IP %q", *ip.AllocationId) } return nil } func (s *Service) releaseAddresses() error { - out, err := s.scope.EC2.DescribeAddresses(&ec2.DescribeAddressesInput{ + out, err := s.EC2Client.DescribeAddresses(&ec2.DescribeAddressesInput{ Filters: []*ec2.Filter{filter.EC2.Cluster(s.scope.Name())}, }) if err != nil { @@ -128,17 +128,17 @@ func (s *Service) releaseAddresses() error { for i := range out.Addresses { ip := out.Addresses[i] if ip.AssociationId != nil { - _, err := s.scope.EC2.DisassociateAddress(&ec2.DisassociateAddressInput{ + _, err := s.EC2Client.DisassociateAddress(&ec2.DisassociateAddressInput{ AssociationId: ip.AssociationId, }) if err != nil { - record.Warnf(s.scope.AWSCluster, "FailedDisassociateEIP", "Failed to disassociate Elastic IP %q: %v", *ip.AllocationId, err) + record.Warnf(s.scope.InfraCluster(), "FailedDisassociateEIP", "Failed to disassociate Elastic IP %q: %v", *ip.AllocationId, err) return errors.Errorf("failed to disassociate Elastic IP %q with allocation ID %q: Still associated with association ID %q", *ip.PublicIp, *ip.AllocationId, *ip.AssociationId) } } err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) { - _, err := s.scope.EC2.ReleaseAddress(&ec2.ReleaseAddressInput{AllocationId: ip.AllocationId}) + _, err := s.EC2Client.ReleaseAddress(&ec2.ReleaseAddressInput{AllocationId: ip.AllocationId}) if err != nil { if ip.AssociationId != nil { if s.disassociateAddress(ip) != nil { @@ -151,7 +151,7 @@ func (s *Service) releaseAddresses() error { return true, nil }, awserrors.AuthFailure, awserrors.InUseIPAddress) if err != nil { - record.Warnf(s.scope.AWSCluster, "FailedReleaseEIP", "Failed to disassociate Elastic IP %q: %v", *ip.AllocationId, err) + record.Warnf(s.scope.InfraCluster(), "FailedReleaseEIP", "Failed to disassociate Elastic IP %q: %v", *ip.AllocationId, err) return errors.Wrapf(err, "failed to release ElasticIP %q", *ip.AllocationId) } diff --git a/pkg/cloud/services/ec2/gateways.go b/pkg/cloud/services/network/gateways.go similarity index 71% rename from pkg/cloud/services/ec2/gateways.go rename to pkg/cloud/services/network/gateways.go index cfee92207f..d24c6c75dd 100644 --- a/pkg/cloud/services/ec2/gateways.go +++ b/pkg/cloud/services/network/gateways.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package ec2 +package network import ( "fmt" @@ -61,17 +61,17 @@ func (s *Service) reconcileInternetGateways() error { // Make sure tags are up to date. if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) { if err := tags.Ensure(converters.TagsToMap(gateway.Tags), &tags.ApplyParams{ - EC2Client: s.scope.EC2, + EC2Client: s.EC2Client, BuildParams: s.getGatewayTagParams(*gateway.InternetGatewayId), }); err != nil { return false, err } return true, nil }, awserrors.InternetGatewayNotFound); err != nil { - record.Warnf(s.scope.AWSCluster, "FailedTagInternetGateway", "Failed to tag managed Internet Gateway %q: %v", gateway.InternetGatewayId, err) + record.Warnf(s.scope.InfraCluster(), "FailedTagInternetGateway", "Failed to tag managed Internet Gateway %q: %v", gateway.InternetGatewayId, err) return errors.Wrapf(err, "failed to tag internet gateway %q", *gateway.InternetGatewayId) } - conditions.MarkTrue(s.scope.AWSCluster, infrav1.InternetGatewayReadyCondition) + conditions.MarkTrue(s.scope.InfraCluster(), infrav1.InternetGatewayReadyCondition) return nil } @@ -94,24 +94,24 @@ func (s *Service) deleteInternetGateways() error { VpcId: aws.String(s.scope.VPC().ID), } - if _, err := s.scope.EC2.DetachInternetGateway(detachReq); err != nil { - record.Warnf(s.scope.AWSCluster, "FailedDetachInternetGateway", "Failed to detach Internet Gateway %q from VPC %q: %v", *ig.InternetGatewayId, s.scope.VPC().ID, err) + if _, err := s.EC2Client.DetachInternetGateway(detachReq); err != nil { + record.Warnf(s.scope.InfraCluster(), "FailedDetachInternetGateway", "Failed to detach Internet Gateway %q from VPC %q: %v", *ig.InternetGatewayId, s.scope.VPC().ID, err) return errors.Wrapf(err, "failed to detach internet gateway %q", *ig.InternetGatewayId) } - record.Eventf(s.scope.AWSCluster, "SuccessfulDetachInternetGateway", "Detached Internet Gateway %q from VPC %q", *ig.InternetGatewayId, s.scope.VPC().ID) + record.Eventf(s.scope.InfraCluster(), "SuccessfulDetachInternetGateway", "Detached Internet Gateway %q from VPC %q", *ig.InternetGatewayId, s.scope.VPC().ID) s.scope.Info("Detached internet gateway from VPC", "internet-gateway-id", *ig.InternetGatewayId, "vpc-id", s.scope.VPC().ID) deleteReq := &ec2.DeleteInternetGatewayInput{ InternetGatewayId: ig.InternetGatewayId, } - if _, err = s.scope.EC2.DeleteInternetGateway(deleteReq); err != nil { - record.Warnf(s.scope.AWSCluster, "FailedDeleteInternetGateway", "Failed to delete Internet Gateway %q previously attached to VPC %q: %v", *ig.InternetGatewayId, s.scope.VPC().ID, err) + if _, err = s.EC2Client.DeleteInternetGateway(deleteReq); err != nil { + record.Warnf(s.scope.InfraCluster(), "FailedDeleteInternetGateway", "Failed to delete Internet Gateway %q previously attached to VPC %q: %v", *ig.InternetGatewayId, s.scope.VPC().ID, err) return errors.Wrapf(err, "failed to delete internet gateway %q", *ig.InternetGatewayId) } - record.Eventf(s.scope.AWSCluster, "SuccessfulDeleteInternetGateway", "Deleted Internet Gateway %q previously attached to VPC %q", *ig.InternetGatewayId, s.scope.VPC().ID) + record.Eventf(s.scope.InfraCluster(), "SuccessfulDeleteInternetGateway", "Deleted Internet Gateway %q previously attached to VPC %q", *ig.InternetGatewayId, s.scope.VPC().ID) s.scope.Info("Deleted internet gateway in VPC", "internet-gateway-id", *ig.InternetGatewayId, "vpc-id", s.scope.VPC().ID) } @@ -119,20 +119,20 @@ func (s *Service) deleteInternetGateways() error { } func (s *Service) createInternetGateway() (*ec2.InternetGateway, error) { - ig, err := s.scope.EC2.CreateInternetGateway(&ec2.CreateInternetGatewayInput{ + ig, err := s.EC2Client.CreateInternetGateway(&ec2.CreateInternetGatewayInput{ TagSpecifications: []*ec2.TagSpecification{ tags.BuildParamsToTagSpecification(ec2.ResourceTypeInternetGateway, s.getGatewayTagParams(temporaryResourceID)), }, }) if err != nil { - record.Warnf(s.scope.AWSCluster, "FailedCreateInternetGateway", "Failed to create new managed Internet Gateway: %v", err) + record.Warnf(s.scope.InfraCluster(), "FailedCreateInternetGateway", "Failed to create new managed Internet Gateway: %v", err) return nil, errors.Wrap(err, "failed to create internet gateway") } - record.Eventf(s.scope.AWSCluster, "SuccessfulCreateInternetGateway", "Created new managed Internet Gateway %q", *ig.InternetGateway.InternetGatewayId) + record.Eventf(s.scope.InfraCluster(), "SuccessfulCreateInternetGateway", "Created new managed Internet Gateway %q", *ig.InternetGateway.InternetGatewayId) s.scope.Info("Created internet gateway for VPC", "vpc-id", s.scope.VPC().ID) if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) { - if _, err := s.scope.EC2.AttachInternetGateway(&ec2.AttachInternetGatewayInput{ + if _, err := s.EC2Client.AttachInternetGateway(&ec2.AttachInternetGatewayInput{ InternetGatewayId: ig.InternetGateway.InternetGatewayId, VpcId: aws.String(s.scope.VPC().ID), }); err != nil { @@ -140,23 +140,23 @@ func (s *Service) createInternetGateway() (*ec2.InternetGateway, error) { } return true, nil }, awserrors.InternetGatewayNotFound); err != nil { - record.Warnf(s.scope.AWSCluster, "FailedAttachInternetGateway", "Failed to attach managed Internet Gateway %q to vpc %q: %v", *ig.InternetGateway.InternetGatewayId, s.scope.VPC().ID, err) + record.Warnf(s.scope.InfraCluster(), "FailedAttachInternetGateway", "Failed to attach managed Internet Gateway %q to vpc %q: %v", *ig.InternetGateway.InternetGatewayId, s.scope.VPC().ID, err) return nil, errors.Wrapf(err, "failed to attach internet gateway %q to vpc %q", *ig.InternetGateway.InternetGatewayId, s.scope.VPC().ID) } - record.Eventf(s.scope.AWSCluster, "SuccessfulAttachInternetGateway", "Internet Gateway %q attached to VPC %q", *ig.InternetGateway.InternetGatewayId, s.scope.VPC().ID) + record.Eventf(s.scope.InfraCluster(), "SuccessfulAttachInternetGateway", "Internet Gateway %q attached to VPC %q", *ig.InternetGateway.InternetGatewayId, s.scope.VPC().ID) s.scope.Info("attached internet gateway to VPC", "internet-gateway-id", *ig.InternetGateway.InternetGatewayId, "vpc-id", s.scope.VPC().ID) return ig.InternetGateway, nil } func (s *Service) describeVpcInternetGateways() ([]*ec2.InternetGateway, error) { - out, err := s.scope.EC2.DescribeInternetGateways(&ec2.DescribeInternetGatewaysInput{ + out, err := s.EC2Client.DescribeInternetGateways(&ec2.DescribeInternetGatewaysInput{ Filters: []*ec2.Filter{ filter.EC2.VPCAttachment(s.scope.VPC().ID), }, }) if err != nil { - record.Eventf(s.scope.AWSCluster, "FailedDescribeInternetGateway", "Failed to describe internet gateways in vpc %q: %v", s.scope.VPC().ID, err) + record.Eventf(s.scope.InfraCluster(), "FailedDescribeInternetGateway", "Failed to describe internet gateways in vpc %q: %v", s.scope.VPC().ID, err) return nil, errors.Wrapf(err, "failed to describe internet gateways in vpc %q", s.scope.VPC().ID) } diff --git a/pkg/cloud/services/ec2/gateways_test.go b/pkg/cloud/services/network/gateways_test.go similarity index 94% rename from pkg/cloud/services/ec2/gateways_test.go rename to pkg/cloud/services/network/gateways_test.go index 65d3450e29..b0c0b3f95a 100644 --- a/pkg/cloud/services/ec2/gateways_test.go +++ b/pkg/cloud/services/network/gateways_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package ec2 +package network import ( "testing" @@ -26,7 +26,6 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-aws/api/v1alpha3" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/scope" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/ec2/mock_ec2iface" - "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/elb/mock_elbiface" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" ) @@ -116,16 +115,11 @@ func TestReconcileInternetGateways(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { ec2Mock := mock_ec2iface.NewMockEC2API(mockCtrl) - elbMock := mock_elbiface.NewMockELBAPI(mockCtrl) scope, err := scope.NewClusterScope(scope.ClusterScopeParams{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{Name: "test-cluster"}, }, - AWSClients: scope.AWSClients{ - EC2: ec2Mock, - ELB: elbMock, - }, AWSCluster: &infrav1.AWSCluster{ Spec: infrav1.AWSClusterSpec{ NetworkSpec: *tc.input, @@ -139,6 +133,8 @@ func TestReconcileInternetGateways(t *testing.T) { tc.expect(ec2Mock.EXPECT()) s := NewService(scope) + s.EC2Client = ec2Mock + if err := s.reconcileInternetGateways(); err != nil { t.Fatalf("got an unexpected error: %v", err) } diff --git a/pkg/cloud/services/ec2/natgateways.go b/pkg/cloud/services/network/natgateways.go similarity index 85% rename from pkg/cloud/services/ec2/natgateways.go rename to pkg/cloud/services/network/natgateways.go index cd4b6fdad3..1bd13acacb 100644 --- a/pkg/cloud/services/ec2/natgateways.go +++ b/pkg/cloud/services/network/natgateways.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package ec2 +package network import ( "fmt" @@ -45,7 +45,7 @@ func (s *Service) reconcileNatGateways() error { if len(s.scope.Subnets().FilterPrivate()) == 0 { s.scope.V(2).Info("No private subnets available, skipping NAT gateways") conditions.MarkFalse( - s.scope.AWSCluster, + s.scope.InfraCluster(), infrav1.NatGatewaysReadyCondition, infrav1.NatGatewaysReconciliationFailedReason, clusterv1.ConditionSeverityWarning, @@ -54,7 +54,7 @@ func (s *Service) reconcileNatGateways() error { } else if len(s.scope.Subnets().FilterPublic()) == 0 { s.scope.V(2).Info("No public subnets available. Cannot create NAT gateways for private subnets, this might be a configuration error.") conditions.MarkFalse( - s.scope.AWSCluster, + s.scope.InfraCluster(), infrav1.NatGatewaysReadyCondition, infrav1.NatGatewaysReconciliationFailedReason, clusterv1.ConditionSeverityWarning, @@ -78,14 +78,14 @@ func (s *Service) reconcileNatGateways() error { // Make sure tags are up to date. if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) { if err := tags.Ensure(converters.TagsToMap(ngw.Tags), &tags.ApplyParams{ - EC2Client: s.scope.EC2, + EC2Client: s.EC2Client, BuildParams: s.getNatGatewayTagParams(*ngw.NatGatewayId), }); err != nil { return false, err } return true, nil }, awserrors.ResourceNotFound); err != nil { - record.Warnf(s.scope.AWSCluster, "FailedTagNATGateway", "Failed to tag managed NAT Gateway %q: %v", *ngw.NatGatewayId, err) + record.Warnf(s.scope.InfraCluster(), "FailedTagNATGateway", "Failed to tag managed NAT Gateway %q: %v", *ngw.NatGatewayId, err) return errors.Wrapf(err, "failed to tag nat gateway %q", *ngw.NatGatewayId) } @@ -98,8 +98,8 @@ func (s *Service) reconcileNatGateways() error { // Batch the creation of NAT gateways if len(subnetIDs) > 0 { // set NatGatewayCreationStarted if the condition has never been set before - if !conditions.Has(s.scope.AWSCluster, infrav1.NatGatewaysReadyCondition) { - conditions.MarkFalse(s.scope.AWSCluster, infrav1.NatGatewaysReadyCondition, infrav1.NatGatewaysCreationStartedReason, clusterv1.ConditionSeverityInfo, "") + if !conditions.Has(s.scope.InfraCluster(), infrav1.NatGatewaysReadyCondition) { + conditions.MarkFalse(s.scope.InfraCluster(), infrav1.NatGatewaysReadyCondition, infrav1.NatGatewaysCreationStartedReason, clusterv1.ConditionSeverityInfo, "") if err := s.scope.PatchObject(); err != nil { return errors.Wrap(err, "failed to patch conditions") } @@ -114,7 +114,7 @@ func (s *Service) reconcileNatGateways() error { if err != nil { return err } - conditions.MarkTrue(s.scope.AWSCluster, infrav1.NatGatewaysReadyCondition) + conditions.MarkTrue(s.scope.InfraCluster(), infrav1.NatGatewaysReadyCondition) } return nil @@ -165,7 +165,7 @@ func (s *Service) describeNatGatewaysBySubnet() (map[string]*ec2.NatGateway, err gateways := make(map[string]*ec2.NatGateway) - err := s.scope.EC2.DescribeNatGatewaysPages(describeNatGatewayInput, + err := s.EC2Client.DescribeNatGatewaysPages(describeNatGatewayInput, func(page *ec2.DescribeNatGatewaysOutput, lastPage bool) bool { for _, r := range page.NatGateways { gateways[*r.SubnetId] = r @@ -173,7 +173,7 @@ func (s *Service) describeNatGatewaysBySubnet() (map[string]*ec2.NatGateway, err return !lastPage }) if err != nil { - record.Eventf(s.scope.AWSCluster, "FailedDescribeNATGateways", "Failed to describe NAT gateways with VPC ID %q: %v", s.scope.VPC().ID, err) + record.Eventf(s.scope.InfraCluster(), "FailedDescribeNATGateways", "Failed to describe NAT gateways with VPC ID %q: %v", s.scope.VPC().ID, err) return nil, errors.Wrapf(err, "failed to describe NAT gateways with VPC ID %q", s.scope.VPC().ID) } @@ -226,7 +226,7 @@ func (s *Service) createNatGateway(subnetID, ip string) (*ec2.NatGateway, error) var err error if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) { - if out, err = s.scope.EC2.CreateNatGateway(&ec2.CreateNatGatewayInput{ + if out, err = s.EC2Client.CreateNatGateway(&ec2.CreateNatGatewayInput{ SubnetId: aws.String(subnetID), AllocationId: aws.String(ip), TagSpecifications: []*ec2.TagSpecification{tags.BuildParamsToTagSpecification(ec2.ResourceTypeNatgateway, s.getNatGatewayTagParams(temporaryResourceID))}, @@ -235,13 +235,13 @@ func (s *Service) createNatGateway(subnetID, ip string) (*ec2.NatGateway, error) } return true, nil }, awserrors.InvalidSubnet); err != nil { - record.Warnf(s.scope.AWSCluster, "FailedCreateNATGateway", "Failed to create new NAT Gateway: %v", err) + record.Warnf(s.scope.InfraCluster(), "FailedCreateNATGateway", "Failed to create new NAT Gateway: %v", err) return nil, errors.Wrapf(err, "failed to create NAT gateway for subnet ID %q", subnetID) } - record.Eventf(s.scope.AWSCluster, "SuccessfulCreateNATGateway", "Created new NAT Gateway %q", *out.NatGateway.NatGatewayId) + record.Eventf(s.scope.InfraCluster(), "SuccessfulCreateNATGateway", "Created new NAT Gateway %q", *out.NatGateway.NatGatewayId) wReq := &ec2.DescribeNatGatewaysInput{NatGatewayIds: []*string{out.NatGateway.NatGatewayId}} - if err := s.scope.EC2.WaitUntilNatGatewayAvailable(wReq); err != nil { + if err := s.EC2Client.WaitUntilNatGatewayAvailable(wReq); err != nil { return nil, errors.Wrapf(err, "failed to wait for nat gateway %q in subnet %q", *out.NatGateway.NatGatewayId, subnetID) } @@ -250,14 +250,14 @@ func (s *Service) createNatGateway(subnetID, ip string) (*ec2.NatGateway, error) } func (s *Service) deleteNatGateway(id string) error { - _, err := s.scope.EC2.DeleteNatGateway(&ec2.DeleteNatGatewayInput{ + _, err := s.EC2Client.DeleteNatGateway(&ec2.DeleteNatGatewayInput{ NatGatewayId: aws.String(id), }) if err != nil { - record.Warnf(s.scope.AWSCluster, "FailedDeleteNATGateway", "Failed to delete NAT Gateway %q previously attached to VPC %q: %v", id, s.scope.VPC().ID, err) + record.Warnf(s.scope.InfraCluster(), "FailedDeleteNATGateway", "Failed to delete NAT Gateway %q previously attached to VPC %q: %v", id, s.scope.VPC().ID, err) return errors.Wrapf(err, "failed to delete nat gateway %q", id) } - record.Eventf(s.scope.AWSCluster, "SuccessfulDeleteNATGateway", "Deleted NAT Gateway %q previously attached to VPC %q", id, s.scope.VPC().ID) + record.Eventf(s.scope.InfraCluster(), "SuccessfulDeleteNATGateway", "Deleted NAT Gateway %q previously attached to VPC %q", id, s.scope.VPC().ID) s.scope.Info("Deleted NAT gateway in VPC", "nat-gateway-id", id, "vpc-id", s.scope.VPC().ID) describeInput := &ec2.DescribeNatGatewaysInput{ @@ -265,7 +265,7 @@ func (s *Service) deleteNatGateway(id string) error { } if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (done bool, err error) { - out, err := s.scope.EC2.DescribeNatGateways(describeInput) + out, err := s.EC2Client.DescribeNatGateways(describeInput) if err != nil { return false, err } diff --git a/pkg/cloud/services/ec2/natgateways_test.go b/pkg/cloud/services/network/natgateways_test.go similarity index 97% rename from pkg/cloud/services/ec2/natgateways_test.go rename to pkg/cloud/services/network/natgateways_test.go index c7208894c0..243591a0bc 100644 --- a/pkg/cloud/services/ec2/natgateways_test.go +++ b/pkg/cloud/services/network/natgateways_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package ec2 +package network import ( "testing" @@ -27,7 +27,6 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-aws/api/v1alpha3" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/scope" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/ec2/mock_ec2iface" - "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/elb/mock_elbiface" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -325,7 +324,6 @@ func TestReconcileNatGateways(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { ec2Mock := mock_ec2iface.NewMockEC2API(mockCtrl) - elbMock := mock_elbiface.NewMockELBAPI(mockCtrl) scheme := runtime.NewScheme() _ = infrav1.AddToScheme(scheme) awsCluster := &infrav1.AWSCluster{ @@ -346,10 +344,6 @@ func TestReconcileNatGateways(t *testing.T) { Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{Name: "test-cluster"}, }, - AWSClients: scope.AWSClients{ - EC2: ec2Mock, - ELB: elbMock, - }, AWSCluster: awsCluster, Client: client, }) @@ -360,6 +354,8 @@ func TestReconcileNatGateways(t *testing.T) { tc.expect(ec2Mock.EXPECT()) s := NewService(clusterScope) + s.EC2Client = ec2Mock + if err := s.reconcileNatGateways(); err != nil { t.Fatalf("got an unexpected error: %v", err) } diff --git a/pkg/cloud/services/ec2/network.go b/pkg/cloud/services/network/network.go similarity index 68% rename from pkg/cloud/services/ec2/network.go rename to pkg/cloud/services/network/network.go index d2d4423242..6a456cee9d 100644 --- a/pkg/cloud/services/ec2/network.go +++ b/pkg/cloud/services/network/network.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package ec2 +package network import ( infrav1 "sigs.k8s.io/cluster-api-provider-aws/api/v1alpha3" @@ -25,42 +25,42 @@ import ( // ReconcileNetwork reconciles the network of the given cluster. func (s *Service) ReconcileNetwork() (err error) { - s.scope.V(2).Info("Reconciling network for cluster", "cluster-name", s.scope.Cluster.Name, "cluster-namespace", s.scope.Cluster.Namespace) + s.scope.V(2).Info("Reconciling network for cluster", "cluster-name", s.scope.Name(), "cluster-namespace", s.scope.Namespace()) // VPC. if err := s.reconcileVPC(); err != nil { - conditions.MarkFalse(s.scope.AWSCluster, infrav1.VpcReadyCondition, infrav1.VpcReconciliationFailedReason, clusterv1.ConditionSeverityError, err.Error()) + conditions.MarkFalse(s.scope.InfraCluster(), infrav1.VpcReadyCondition, infrav1.VpcReconciliationFailedReason, clusterv1.ConditionSeverityError, err.Error()) return err } - conditions.MarkTrue(s.scope.AWSCluster, infrav1.VpcReadyCondition) + conditions.MarkTrue(s.scope.InfraCluster(), infrav1.VpcReadyCondition) // Subnets. if err := s.reconcileSubnets(); err != nil { - conditions.MarkFalse(s.scope.AWSCluster, infrav1.SubnetsReadyCondition, infrav1.SubnetsReconciliationFailedReason, clusterv1.ConditionSeverityError, err.Error()) + conditions.MarkFalse(s.scope.InfraCluster(), infrav1.SubnetsReadyCondition, infrav1.SubnetsReconciliationFailedReason, clusterv1.ConditionSeverityError, err.Error()) return err } // Internet Gateways. if err := s.reconcileInternetGateways(); err != nil { - conditions.MarkFalse(s.scope.AWSCluster, infrav1.InternetGatewayReadyCondition, infrav1.InternetGatewayFailedReason, clusterv1.ConditionSeverityError, err.Error()) + conditions.MarkFalse(s.scope.InfraCluster(), infrav1.InternetGatewayReadyCondition, infrav1.InternetGatewayFailedReason, clusterv1.ConditionSeverityError, err.Error()) return err } // NAT Gateways. if err := s.reconcileNatGateways(); err != nil { - conditions.MarkFalse(s.scope.AWSCluster, infrav1.NatGatewaysReadyCondition, infrav1.NatGatewaysReconciliationFailedReason, clusterv1.ConditionSeverityError, err.Error()) + conditions.MarkFalse(s.scope.InfraCluster(), infrav1.NatGatewaysReadyCondition, infrav1.NatGatewaysReconciliationFailedReason, clusterv1.ConditionSeverityError, err.Error()) return err } // Routing tables. if err := s.reconcileRouteTables(); err != nil { - conditions.MarkFalse(s.scope.AWSCluster, infrav1.RouteTablesReadyCondition, infrav1.RouteTableReconciliationFailedReason, clusterv1.ConditionSeverityError, err.Error()) + conditions.MarkFalse(s.scope.InfraCluster(), infrav1.RouteTablesReadyCondition, infrav1.RouteTableReconciliationFailedReason, clusterv1.ConditionSeverityError, err.Error()) return err } // Security groups. if err := s.reconcileSecurityGroups(); err != nil { - conditions.MarkFalse(s.scope.AWSCluster, infrav1.ClusterSecurityGroupsReadyCondition, infrav1.ClusterSecurityGroupReconciliationFailedReason, clusterv1.ConditionSeverityError, err.Error()) + conditions.MarkFalse(s.scope.InfraCluster(), infrav1.ClusterSecurityGroupsReadyCondition, infrav1.ClusterSecurityGroupReconciliationFailedReason, clusterv1.ConditionSeverityError, err.Error()) return err } diff --git a/pkg/cloud/services/ec2/routetables.go b/pkg/cloud/services/network/routetables.go similarity index 78% rename from pkg/cloud/services/ec2/routetables.go rename to pkg/cloud/services/network/routetables.go index 1d69de5856..4a0e9bfb54 100644 --- a/pkg/cloud/services/ec2/routetables.go +++ b/pkg/cloud/services/network/routetables.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package ec2 +package network import ( "strings" @@ -83,7 +83,7 @@ func (s *Service) reconcileRouteTables() error { ((currentRoute.GatewayId != nil && *currentRoute.GatewayId != *specRoute.GatewayId) || (currentRoute.NatGatewayId != nil && *currentRoute.NatGatewayId != *specRoute.NatGatewayId)) { if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) { - if _, err := s.scope.EC2.ReplaceRoute(&ec2.ReplaceRouteInput{ + if _, err := s.EC2Client.ReplaceRoute(&ec2.ReplaceRouteInput{ RouteTableId: rt.RouteTableId, DestinationCidrBlock: specRoute.DestinationCidrBlock, GatewayId: specRoute.GatewayId, @@ -93,7 +93,7 @@ func (s *Service) reconcileRouteTables() error { } return true, nil }); err != nil { - record.Warnf(s.scope.AWSCluster, "FailedReplaceRoute", "Failed to replace outdated route on managed RouteTable %q: %v", *rt.RouteTableId, err) + record.Warnf(s.scope.InfraCluster(), "FailedReplaceRoute", "Failed to replace outdated route on managed RouteTable %q: %v", *rt.RouteTableId, err) return errors.Wrapf(err, "failed to replace outdated route on route table %q", *rt.RouteTableId) } } @@ -103,14 +103,14 @@ func (s *Service) reconcileRouteTables() error { // Make sure tags are up to date. if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) { if err := tags.Ensure(converters.TagsToMap(rt.Tags), &tags.ApplyParams{ - EC2Client: s.scope.EC2, + EC2Client: s.EC2Client, BuildParams: s.getRouteTableTagParams(*rt.RouteTableId, sn.IsPublic, sn.AvailabilityZone), }); err != nil { return false, err } return true, nil }, awserrors.RouteTableNotFound); err != nil { - record.Warnf(s.scope.AWSCluster, "FailedTagRouteTable", "Failed to tag managed RouteTable %q: %v", *rt.RouteTableId, err) + record.Warnf(s.scope.InfraCluster(), "FailedTagRouteTable", "Failed to tag managed RouteTable %q: %v", *rt.RouteTableId, err) return errors.Wrapf(err, "failed to ensure tags on route table %q", *rt.RouteTableId) } @@ -137,7 +137,7 @@ func (s *Service) reconcileRouteTables() error { s.scope.V(2).Info("Subnet has been associated with route table", "subnet-id", sn.ID, "route-table-id", rt.ID) sn.RouteTableID = aws.String(rt.ID) } - conditions.MarkTrue(s.scope.AWSCluster, infrav1.RouteTablesReadyCondition) + conditions.MarkTrue(s.scope.InfraCluster(), infrav1.RouteTablesReadyCondition) return nil } @@ -183,21 +183,21 @@ func (s *Service) deleteRouteTables() error { continue } - if _, err := s.scope.EC2.DisassociateRouteTable(&ec2.DisassociateRouteTableInput{AssociationId: as.RouteTableAssociationId}); err != nil { - record.Warnf(s.scope.AWSCluster, "FailedDisassociateRouteTable", "Failed to disassociate managed RouteTable %q from Subnet %q: %v", *rt.RouteTableId, *as.SubnetId, err) + if _, err := s.EC2Client.DisassociateRouteTable(&ec2.DisassociateRouteTableInput{AssociationId: as.RouteTableAssociationId}); err != nil { + record.Warnf(s.scope.InfraCluster(), "FailedDisassociateRouteTable", "Failed to disassociate managed RouteTable %q from Subnet %q: %v", *rt.RouteTableId, *as.SubnetId, err) return errors.Wrapf(err, "failed to disassociate route table %q from subnet %q", *rt.RouteTableId, *as.SubnetId) } - record.Eventf(s.scope.AWSCluster, "SuccessfulDisassociateRouteTable", "Disassociated managed RouteTable %q from subnet %q", *rt.RouteTableId, *as.SubnetId) + record.Eventf(s.scope.InfraCluster(), "SuccessfulDisassociateRouteTable", "Disassociated managed RouteTable %q from subnet %q", *rt.RouteTableId, *as.SubnetId) s.scope.Info("Deleted association between route table and subnet", "route-table-id", *rt.RouteTableId, "subnet-id", *as.SubnetId) } - if _, err := s.scope.EC2.DeleteRouteTable(&ec2.DeleteRouteTableInput{RouteTableId: rt.RouteTableId}); err != nil { - record.Warnf(s.scope.AWSCluster, "FailedDeleteRouteTable", "Failed to delete managed RouteTable %q: %v", *rt.RouteTableId, err) + if _, err := s.EC2Client.DeleteRouteTable(&ec2.DeleteRouteTableInput{RouteTableId: rt.RouteTableId}); err != nil { + record.Warnf(s.scope.InfraCluster(), "FailedDeleteRouteTable", "Failed to delete managed RouteTable %q: %v", *rt.RouteTableId, err) return errors.Wrapf(err, "failed to delete route table %q", *rt.RouteTableId) } - record.Eventf(s.scope.AWSCluster, "SuccessfulDeleteRouteTable", "Deleted managed RouteTable %q", *rt.RouteTableId) + record.Eventf(s.scope.InfraCluster(), "SuccessfulDeleteRouteTable", "Deleted managed RouteTable %q", *rt.RouteTableId) s.scope.Info("Deleted route table", "route-table-id", *rt.RouteTableId) } return nil @@ -212,11 +212,11 @@ func (s *Service) describeVpcRouteTables() ([]*ec2.RouteTable, error) { filters = append(filters, filter.EC2.Cluster(s.scope.Name())) } - out, err := s.scope.EC2.DescribeRouteTables(&ec2.DescribeRouteTablesInput{ + out, err := s.EC2Client.DescribeRouteTables(&ec2.DescribeRouteTablesInput{ Filters: filters, }) if err != nil { - record.Eventf(s.scope.AWSCluster, "FailedDescribeVPCRouteTable", "Failed to describe route tables in vpc %q: %v", s.scope.VPC().ID, err) + record.Eventf(s.scope.InfraCluster(), "FailedDescribeVPCRouteTable", "Failed to describe route tables in vpc %q: %v", s.scope.VPC().ID, err) return nil, errors.Wrapf(err, "failed to describe route tables in vpc %q", s.scope.VPC().ID) } @@ -224,33 +224,33 @@ func (s *Service) describeVpcRouteTables() ([]*ec2.RouteTable, error) { } func (s *Service) createRouteTableWithRoutes(routes []*ec2.Route, isPublic bool, zone string) (*infrav1.RouteTable, error) { - out, err := s.scope.EC2.CreateRouteTable(&ec2.CreateRouteTableInput{ + out, err := s.EC2Client.CreateRouteTable(&ec2.CreateRouteTableInput{ VpcId: aws.String(s.scope.VPC().ID), }) if err != nil { - record.Warnf(s.scope.AWSCluster, "FailedCreateRouteTable", "Failed to create managed RouteTable: %v", err) + record.Warnf(s.scope.InfraCluster(), "FailedCreateRouteTable", "Failed to create managed RouteTable: %v", err) return nil, errors.Wrapf(err, "failed to create route table in vpc %q", s.scope.VPC().ID) } - record.Eventf(s.scope.AWSCluster, "SuccessfulCreateRouteTable", "Created managed RouteTable %q", *out.RouteTable.RouteTableId) + record.Eventf(s.scope.InfraCluster(), "SuccessfulCreateRouteTable", "Created managed RouteTable %q", *out.RouteTable.RouteTableId) if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) { if err := tags.Apply(&tags.ApplyParams{ - EC2Client: s.scope.EC2, + EC2Client: s.EC2Client, BuildParams: s.getRouteTableTagParams(*out.RouteTable.RouteTableId, isPublic, zone), }); err != nil { return false, err } return true, nil }, awserrors.RouteTableNotFound); err != nil { - record.Warnf(s.scope.AWSCluster, "FailedTagRouteTable", "Failed to tag managed RouteTable %q: %v", *out.RouteTable.RouteTableId, err) + record.Warnf(s.scope.InfraCluster(), "FailedTagRouteTable", "Failed to tag managed RouteTable %q: %v", *out.RouteTable.RouteTableId, err) return nil, errors.Wrapf(err, "failed to tag route table %q", *out.RouteTable.RouteTableId) } - record.Eventf(s.scope.AWSCluster, "SuccessfulTagRouteTable", "Tagged managed RouteTable %q", *out.RouteTable.RouteTableId) + record.Eventf(s.scope.InfraCluster(), "SuccessfulTagRouteTable", "Tagged managed RouteTable %q", *out.RouteTable.RouteTableId) for i := range routes { route := routes[i] if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) { - if _, err := s.scope.EC2.CreateRoute(&ec2.CreateRouteInput{ + if _, err := s.EC2Client.CreateRoute(&ec2.CreateRouteInput{ RouteTableId: out.RouteTable.RouteTableId, DestinationCidrBlock: route.DestinationCidrBlock, DestinationIpv6CidrBlock: route.DestinationIpv6CidrBlock, @@ -266,10 +266,10 @@ func (s *Service) createRouteTableWithRoutes(routes []*ec2.Route, isPublic bool, return true, nil }, awserrors.RouteTableNotFound, awserrors.NATGatewayNotFound, awserrors.GatewayNotFound); err != nil { // TODO(vincepri): cleanup the route table if this fails. - record.Warnf(s.scope.AWSCluster, "FailedCreateRoute", "Failed to create route %s for RouteTable %q: %v", route.GoString(), *out.RouteTable.RouteTableId, err) + record.Warnf(s.scope.InfraCluster(), "FailedCreateRoute", "Failed to create route %s for RouteTable %q: %v", route.GoString(), *out.RouteTable.RouteTableId, err) return nil, errors.Wrapf(err, "failed to create route in route table %q: %s", *out.RouteTable.RouteTableId, route.GoString()) } - record.Eventf(s.scope.AWSCluster, "SuccessfulCreateRoute", "Created route %s for RouteTable %q", route.GoString(), *out.RouteTable.RouteTableId) + record.Eventf(s.scope.InfraCluster(), "SuccessfulCreateRoute", "Created route %s for RouteTable %q", route.GoString(), *out.RouteTable.RouteTableId) } return &infrav1.RouteTable{ @@ -278,16 +278,16 @@ func (s *Service) createRouteTableWithRoutes(routes []*ec2.Route, isPublic bool, } func (s *Service) associateRouteTable(rt *infrav1.RouteTable, subnetID string) error { - _, err := s.scope.EC2.AssociateRouteTable(&ec2.AssociateRouteTableInput{ + _, err := s.EC2Client.AssociateRouteTable(&ec2.AssociateRouteTableInput{ RouteTableId: aws.String(rt.ID), SubnetId: aws.String(subnetID), }) if err != nil { - record.Warnf(s.scope.AWSCluster, "FailedAssociateRouteTable", "Failed to associate managed RouteTable %q with Subnet %q: %v", rt.ID, subnetID, err) + record.Warnf(s.scope.InfraCluster(), "FailedAssociateRouteTable", "Failed to associate managed RouteTable %q with Subnet %q: %v", rt.ID, subnetID, err) return errors.Wrapf(err, "failed to associate route table %q to subnet %q", rt.ID, subnetID) } - record.Eventf(s.scope.AWSCluster, "SuccessfulAssociateRouteTable", "Associated managed RouteTable %q with subnet %q", rt.ID, subnetID) + record.Eventf(s.scope.InfraCluster(), "SuccessfulAssociateRouteTable", "Associated managed RouteTable %q with subnet %q", rt.ID, subnetID) return nil } diff --git a/pkg/cloud/services/ec2/routetables_test.go b/pkg/cloud/services/network/routetables_test.go similarity index 97% rename from pkg/cloud/services/ec2/routetables_test.go rename to pkg/cloud/services/network/routetables_test.go index b58ceda658..c3a9ababd8 100644 --- a/pkg/cloud/services/ec2/routetables_test.go +++ b/pkg/cloud/services/network/routetables_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package ec2 +package network import ( "strings" @@ -28,7 +28,6 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-aws/api/v1alpha3" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/scope" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/ec2/mock_ec2iface" - "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/elb/mock_elbiface" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" ) @@ -244,16 +243,11 @@ func TestReconcileRouteTables(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { ec2Mock := mock_ec2iface.NewMockEC2API(mockCtrl) - elbMock := mock_elbiface.NewMockELBAPI(mockCtrl) scope, err := scope.NewClusterScope(scope.ClusterScopeParams{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{Name: "test-cluster"}, }, - AWSClients: scope.AWSClients{ - EC2: ec2Mock, - ELB: elbMock, - }, AWSCluster: &infrav1.AWSCluster{ Spec: infrav1.AWSClusterSpec{ NetworkSpec: *tc.input, @@ -267,6 +261,8 @@ func TestReconcileRouteTables(t *testing.T) { tc.expect(ec2Mock.EXPECT()) s := NewService(scope) + s.EC2Client = ec2Mock + if err := s.reconcileRouteTables(); err != nil && tc.err != nil { if !strings.Contains(err.Error(), tc.err.Error()) { t.Fatalf("was expecting error to look like '%v', but got '%v'", tc.err, err) diff --git a/pkg/cloud/services/ec2/securitygroups.go b/pkg/cloud/services/network/securitygroups.go similarity index 88% rename from pkg/cloud/services/ec2/securitygroups.go rename to pkg/cloud/services/network/securitygroups.go index 7ff3ffe7cf..dd1d52a63a 100644 --- a/pkg/cloud/services/ec2/securitygroups.go +++ b/pkg/cloud/services/network/securitygroups.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package ec2 +package network import ( "fmt" @@ -94,7 +94,7 @@ func (s *Service) reconcileSecurityGroups() error { // Make sure tags are up to date. if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) { if err := tags.Ensure(existing.Tags, &tags.ApplyParams{ - EC2Client: s.scope.EC2, + EC2Client: s.EC2Client, BuildParams: s.getSecurityGroupTagParams(existing.Name, existing.ID, role), }); err != nil { return false, err @@ -148,7 +148,7 @@ func (s *Service) reconcileSecurityGroups() error { s.scope.V(2).Info("Authorized ingress rules in security group", "authorized-ingress-rules", toAuthorize, "security-group-id", sg.ID) } } - conditions.MarkTrue(s.scope.AWSCluster, infrav1.ClusterSecurityGroupsReadyCondition) + conditions.MarkTrue(s.scope.InfraCluster(), infrav1.ClusterSecurityGroupsReadyCondition) return nil } @@ -195,12 +195,12 @@ func (s *Service) deleteSecurityGroup(sg *infrav1.SecurityGroup, typ string) err GroupId: aws.String(sg.ID), } - if _, err := s.scope.EC2.DeleteSecurityGroup(input); awserrors.IsIgnorableSecurityGroupError(err) != nil { - record.Warnf(s.scope.AWSCluster, "FailedDeleteSecurityGroup", "Failed to delete %s SecurityGroup %q: %v", typ, sg.ID, err) + if _, err := s.EC2Client.DeleteSecurityGroup(input); awserrors.IsIgnorableSecurityGroupError(err) != nil { + record.Warnf(s.scope.InfraCluster(), "FailedDeleteSecurityGroup", "Failed to delete %s SecurityGroup %q: %v", typ, sg.ID, err) return errors.Wrapf(err, "failed to delete security group %q", sg.ID) } - record.Eventf(s.scope.AWSCluster, "SuccessfulDeleteSecurityGroup", "Deleted %s SecurityGroup %q", typ, sg.ID) + record.Eventf(s.scope.InfraCluster(), "SuccessfulDeleteSecurityGroup", "Deleted %s SecurityGroup %q", typ, sg.ID) s.scope.V(2).Info("Deleted security group", "security-group-id", sg.ID, "kind", typ) return nil @@ -216,7 +216,7 @@ func (s *Service) describeClusterOwnedSecurityGroups() ([]infrav1.SecurityGroup, groups := []infrav1.SecurityGroup{} - err := s.scope.EC2.DescribeSecurityGroupsPages(input, func(out *ec2.DescribeSecurityGroupsOutput, last bool) bool { + err := s.EC2Client.DescribeSecurityGroupsPages(input, func(out *ec2.DescribeSecurityGroupsOutput, last bool) bool { for _, group := range out.SecurityGroups { if group != nil { groups = append(groups, makeInfraSecurityGroup(group)) @@ -238,7 +238,7 @@ func (s *Service) describeSecurityGroupsByName() (map[string]infrav1.SecurityGro }, } - out, err := s.scope.EC2.DescribeSecurityGroups(input) + out, err := s.EC2Client.DescribeSecurityGroups(input) if err != nil { return nil, errors.Wrapf(err, "failed to describe security groups in vpc %q", s.scope.VPC().ID) } @@ -267,7 +267,7 @@ func makeInfraSecurityGroup(ec2sg *ec2.SecurityGroup) infrav1.SecurityGroup { func (s *Service) createSecurityGroup(role infrav1.SecurityGroupRole, input *ec2.SecurityGroup) error { sgTags := s.getSecurityGroupTagParams(aws.StringValue(input.GroupName), temporaryResourceID, role) - out, err := s.scope.EC2.CreateSecurityGroup(&ec2.CreateSecurityGroupInput{ + out, err := s.EC2Client.CreateSecurityGroup(&ec2.CreateSecurityGroupInput{ VpcId: input.VpcId, GroupName: input.GroupName, Description: aws.String(fmt.Sprintf("Kubernetes cluster %s: %s", s.scope.Name(), role)), @@ -276,11 +276,11 @@ func (s *Service) createSecurityGroup(role infrav1.SecurityGroupRole, input *ec2 }, }) if err != nil { - record.Warnf(s.scope.AWSCluster, "FailedCreateSecurityGroup", "Failed to create managed SecurityGroup for Role %q: %v", role, err) + record.Warnf(s.scope.InfraCluster(), "FailedCreateSecurityGroup", "Failed to create managed SecurityGroup for Role %q: %v", role, err) return errors.Wrapf(err, "failed to create security group %q in vpc %q", role, aws.StringValue(input.VpcId)) } - record.Eventf(s.scope.AWSCluster, "SuccessfulCreateSecurityGroup", "Created managed SecurityGroup %q for Role %q", aws.StringValue(out.GroupId), role) + record.Eventf(s.scope.InfraCluster(), "SuccessfulCreateSecurityGroup", "Created managed SecurityGroup %q for Role %q", aws.StringValue(out.GroupId), role) // Set the group id. input.GroupId = out.GroupId @@ -294,12 +294,12 @@ func (s *Service) authorizeSecurityGroupIngressRules(id string, rules infrav1.In input.IpPermissions = append(input.IpPermissions, ingressRuleToSDKType(rule)) } - if _, err := s.scope.EC2.AuthorizeSecurityGroupIngress(input); err != nil { - record.Warnf(s.scope.AWSCluster, "FailedAuthorizeSecurityGroupIngressRules", "Failed to authorize security group ingress rules %v for SecurityGroup %q: %v", rules, id, err) + if _, err := s.EC2Client.AuthorizeSecurityGroupIngress(input); err != nil { + record.Warnf(s.scope.InfraCluster(), "FailedAuthorizeSecurityGroupIngressRules", "Failed to authorize security group ingress rules %v for SecurityGroup %q: %v", rules, id, err) return errors.Wrapf(err, "failed to authorize security group %q ingress rules: %v", id, rules) } - record.Eventf(s.scope.AWSCluster, "SuccessfulAuthorizeSecurityGroupIngressRules", "Authorized security group ingress rules %v for SecurityGroup %q", rules, id) + record.Eventf(s.scope.InfraCluster(), "SuccessfulAuthorizeSecurityGroupIngressRules", "Authorized security group ingress rules %v for SecurityGroup %q", rules, id) return nil } @@ -309,19 +309,19 @@ func (s *Service) revokeSecurityGroupIngressRules(id string, rules infrav1.Ingre input.IpPermissions = append(input.IpPermissions, ingressRuleToSDKType(rule)) } - if _, err := s.scope.EC2.RevokeSecurityGroupIngress(input); err != nil { - record.Warnf(s.scope.AWSCluster, "FailedRevokeSecurityGroupIngressRules", "Failed to revoke security group ingress rules %v for SecurityGroup %q: %v", rules, id, err) + if _, err := s.EC2Client.RevokeSecurityGroupIngress(input); err != nil { + record.Warnf(s.scope.InfraCluster(), "FailedRevokeSecurityGroupIngressRules", "Failed to revoke security group ingress rules %v for SecurityGroup %q: %v", rules, id, err) return errors.Wrapf(err, "failed to revoke security group %q ingress rules: %v", id, rules) } - record.Eventf(s.scope.AWSCluster, "SuccessfulRevokeSecurityGroupIngressRules", "Revoked security group ingress rules %v for SecurityGroup %q", rules, id) + record.Eventf(s.scope.InfraCluster(), "SuccessfulRevokeSecurityGroupIngressRules", "Revoked security group ingress rules %v for SecurityGroup %q", rules, id) return nil } func (s *Service) revokeAllSecurityGroupIngressRules(id string) error { describeInput := &ec2.DescribeSecurityGroupsInput{GroupIds: []*string{aws.String(id)}} - securityGroups, err := s.scope.EC2.DescribeSecurityGroups(describeInput) + securityGroups, err := s.EC2Client.DescribeSecurityGroups(describeInput) if err != nil { return errors.Wrapf(err, "failed to query security group %q", id) } @@ -332,11 +332,11 @@ func (s *Service) revokeAllSecurityGroupIngressRules(id string) error { GroupId: aws.String(id), IpPermissions: sg.IpPermissions, } - if _, err := s.scope.EC2.RevokeSecurityGroupIngress(revokeInput); err != nil { - record.Warnf(s.scope.AWSCluster, "FailedRevokeSecurityGroupIngressRules", "Failed to revoke all security group ingress rules for SecurityGroup %q: %v", *sg.GroupId, err) + if _, err := s.EC2Client.RevokeSecurityGroupIngress(revokeInput); err != nil { + record.Warnf(s.scope.InfraCluster(), "FailedRevokeSecurityGroupIngressRules", "Failed to revoke all security group ingress rules for SecurityGroup %q: %v", *sg.GroupId, err) return errors.Wrapf(err, "failed to revoke security group %q ingress rules", id) } - record.Eventf(s.scope.AWSCluster, "SuccessfulRevokeSecurityGroupIngressRules", "Revoked all security group ingress rules for SecurityGroup %q", *sg.GroupId) + record.Eventf(s.scope.InfraCluster(), "SuccessfulRevokeSecurityGroupIngressRules", "Revoked all security group ingress rules for SecurityGroup %q", *sg.GroupId) } } @@ -377,7 +377,7 @@ func (s *Service) getSecurityGroupIngressRules(role infrav1.SecurityGroupRole) ( Protocol: infrav1.SecurityGroupProtocolTCP, FromPort: 22, ToPort: 22, - CidrBlocks: s.scope.AWSCluster.Spec.Bastion.AllowedCIDRBlocks, + CidrBlocks: s.scope.Bastion().AllowedCIDRBlocks, }, }, nil case infrav1.SecurityGroupControlPlane: diff --git a/pkg/cloud/services/ec2/securitygroups_test.go b/pkg/cloud/services/network/securitygroups_test.go similarity index 97% rename from pkg/cloud/services/ec2/securitygroups_test.go rename to pkg/cloud/services/network/securitygroups_test.go index 9d2992f13c..45082d6fe5 100644 --- a/pkg/cloud/services/ec2/securitygroups_test.go +++ b/pkg/cloud/services/network/securitygroups_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package ec2 +package network import ( "strings" @@ -28,7 +28,6 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-aws/api/v1alpha3" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/scope" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/ec2/mock_ec2iface" - "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/elb/mock_elbiface" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" ) @@ -242,16 +241,11 @@ func TestReconcileSecurityGroups(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { ec2Mock := mock_ec2iface.NewMockEC2API(mockCtrl) - elbMock := mock_elbiface.NewMockELBAPI(mockCtrl) scope, err := scope.NewClusterScope(scope.ClusterScopeParams{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{Name: "test-cluster"}, }, - AWSClients: scope.AWSClients{ - EC2: ec2Mock, - ELB: elbMock, - }, AWSCluster: &infrav1.AWSCluster{ Spec: infrav1.AWSClusterSpec{ NetworkSpec: *tc.input, @@ -265,6 +259,8 @@ func TestReconcileSecurityGroups(t *testing.T) { tc.expect(ec2Mock.EXPECT()) s := NewService(scope) + s.EC2Client = ec2Mock + if err := s.reconcileSecurityGroups(); err != nil && tc.err != nil { if !strings.Contains(err.Error(), tc.err.Error()) { t.Fatalf("was expecting error to look like '%v', but got '%v'", tc.err, err) diff --git a/pkg/cloud/services/network/service.go b/pkg/cloud/services/network/service.go new file mode 100644 index 0000000000..57ee2a2f5e --- /dev/null +++ b/pkg/cloud/services/network/service.go @@ -0,0 +1,66 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package network + +import ( + "github.com/aws/aws-sdk-go/service/ec2/ec2iface" + + infrav1 "sigs.k8s.io/cluster-api-provider-aws/api/v1alpha3" + "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud" + "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/scope" +) + +const ( + temporaryResourceID = "temporary-resource-id" +) + +// Scope is scope for use with the network service +type Scope interface { + cloud.ClusterScoper + + // Network returns the cluster network object. + Network() *infrav1.Network + // VPC returns the cluster VPC. + VPC() *infrav1.VPCSpec + // Subnets returns the cluster subnets. + Subnets() infrav1.Subnets + // SetSubnets updates the clusters subnets. + SetSubnets(subnets infrav1.Subnets) + // CNIIngressRules returns the CNI spec ingress rules. + CNIIngressRules() infrav1.CNIIngressRules + // SecurityGroups returns the cluster security groups as a map, it creates the map if empty. + SecurityGroups() map[infrav1.SecurityGroupRole]infrav1.SecurityGroup + + // Bastion returns the bastion details for the cluster. + Bastion() *infrav1.Bastion +} + +// Service holds a collection of interfaces. +// The interfaces are broken down like this to group functions together. +// One alternative is to have a large list of functions from the ec2 client. +type Service struct { + scope Scope + EC2Client ec2iface.EC2API +} + +// NewService returns a new service given the ec2 api client. +func NewService(networkScope Scope) *Service { + return &Service{ + scope: networkScope, + EC2Client: scope.NewEC2Client(networkScope, networkScope, networkScope.InfraCluster()), + } +} diff --git a/pkg/cloud/services/ec2/subnets.go b/pkg/cloud/services/network/subnets.go similarity index 85% rename from pkg/cloud/services/ec2/subnets.go rename to pkg/cloud/services/network/subnets.go index e22fea40b8..fc6d656fe2 100644 --- a/pkg/cloud/services/ec2/subnets.go +++ b/pkg/cloud/services/network/subnets.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package ec2 +package network import ( "fmt" @@ -47,7 +47,7 @@ func (s *Service) reconcileSubnets() error { subnets := s.scope.Subnets() defer func() { - s.scope.AWSCluster.Spec.NetworkSpec.Subnets = subnets + s.scope.SetSubnets(subnets) }() // Describe subnets in the vpc. @@ -62,14 +62,14 @@ func (s *Service) reconcileSubnets() error { if unmanagedVPC { // If we have a unmanaged VPC then subnets must be specified errMsg := "no subnets specified, you must specify the subnets when using an umanaged vpc" - record.Warnf(s.scope.AWSCluster, "FailedNoSubnets", errMsg) + record.Warnf(s.scope.InfraCluster(), "FailedNoSubnets", errMsg) return errors.New(errMsg) } // If we a managed VPC and have no subnets then create subnets. There will be 1 public and 1 private subnet // for each az in a region up to a maximum of 3 azs subnets, err = s.getDefaultSubnets() if err != nil { - record.Warnf(s.scope.AWSCluster, "FailedDefaultSubnets", "Failed getting default subnets: %v", err) + record.Warnf(s.scope.InfraCluster(), "FailedDefaultSubnets", "Failed getting default subnets: %v", err) return errors.Wrap(err, "failed getting default subnets") } } @@ -82,14 +82,14 @@ func (s *Service) reconcileSubnets() error { // Make sure tags are up to date if we have a managed VPC. if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) { if err := tags.Ensure(existingSubnet.Tags, &tags.ApplyParams{ - EC2Client: s.scope.EC2, + EC2Client: s.EC2Client, BuildParams: s.getSubnetTagParams(existingSubnet.ID, existingSubnet.IsPublic, existingSubnet.AvailabilityZone, subnetTags), }); err != nil { return false, err } return true, nil }, awserrors.SubnetNotFound); err != nil { - record.Warnf(s.scope.AWSCluster, "FailedTagSubnet", "Failed tagging managed Subnet %q: %v", existingSubnet.ID, err) + record.Warnf(s.scope.InfraCluster(), "FailedTagSubnet", "Failed tagging managed Subnet %q: %v", existingSubnet.ID, err) return errors.Wrapf(err, "failed to ensure tags on subnet %q", existingSubnet.ID) } } @@ -99,18 +99,18 @@ func (s *Service) reconcileSubnets() error { existingSubnet.DeepCopyInto(sub) } else if unmanagedVPC { // If there is no existing subnet and we have an umanaged vpc report an error - record.Warnf(s.scope.AWSCluster, "FailedMatchSubnet", "Using unmanaged VPC and failed to find existing subnet for specified subnet id %d, cidr %q", sub.ID, sub.CidrBlock) + record.Warnf(s.scope.InfraCluster(), "FailedMatchSubnet", "Using unmanaged VPC and failed to find existing subnet for specified subnet id %d, cidr %q", sub.ID, sub.CidrBlock) return errors.New(fmt.Sprintf("usign unmanaged vpc and subnet %s (cidr %s) specified but it doesn't exist in vpc %s", sub.ID, sub.CidrBlock, s.scope.VPC().ID)) } } // Check that we need at least 1 private and 1 public subnet after we have updated the metadata if len(subnets.FilterPrivate()) < 1 { - record.Warnf(s.scope.AWSCluster, "FailedNoPrivateSubnet", "Expected at least 1 private subnet but got 0") + record.Warnf(s.scope.InfraCluster(), "FailedNoPrivateSubnet", "Expected at least 1 private subnet but got 0") return errors.New("expected at least 1 private subnet but got 0") } if len(subnets.FilterPublic()) < 1 { - record.Warnf(s.scope.AWSCluster, "FailedNoPublicSubnet", "Expected at least 1 public subnet but got 0") + record.Warnf(s.scope.InfraCluster(), "FailedNoPublicSubnet", "Expected at least 1 public subnet but got 0") return errors.New("expected at least 1 public subnet but got 0") } @@ -130,7 +130,7 @@ func (s *Service) reconcileSubnets() error { } s.scope.V(2).Info("Subnets available", "subnets", subnets) - conditions.MarkTrue(s.scope.AWSCluster, infrav1.SubnetsReadyCondition) + conditions.MarkTrue(s.scope.InfraCluster(), infrav1.SubnetsReadyCondition) return nil } @@ -227,9 +227,9 @@ func (s *Service) describeVpcSubnets() (infrav1.Subnets, error) { input.Filters = append(input.Filters, filter.EC2.VPC(s.scope.VPC().ID)) } - out, err := s.scope.EC2.DescribeSubnets(input) + out, err := s.EC2Client.DescribeSubnets(input) if err != nil { - record.Eventf(s.scope.AWSCluster, "FailedDescribeSubnet", "Failed to describe subnets in vpc %q: %v", s.scope.VPC().ID, err) + record.Eventf(s.scope.InfraCluster(), "FailedDescribeSubnet", "Failed to describe subnets in vpc %q: %v", s.scope.VPC().ID, err) return nil, errors.Wrapf(err, "failed to describe subnets in vpc %q", s.scope.VPC().ID) } @@ -285,7 +285,7 @@ func (s *Service) describeVpcSubnets() (infrav1.Subnets, error) { } func (s *Service) createSubnet(sn *infrav1.SubnetSpec) (*infrav1.SubnetSpec, error) { - out, err := s.scope.EC2.CreateSubnet(&ec2.CreateSubnetInput{ + out, err := s.EC2Client.CreateSubnet(&ec2.CreateSubnetInput{ VpcId: aws.String(s.scope.VPC().ID), CidrBlock: aws.String(sn.CidrBlock), AvailabilityZone: aws.String(sn.AvailabilityZone), @@ -297,14 +297,14 @@ func (s *Service) createSubnet(sn *infrav1.SubnetSpec) (*infrav1.SubnetSpec, err }, }) if err != nil { - record.Warnf(s.scope.AWSCluster, "FailedCreateSubnet", "Failed creating new managed Subnet %v", err) + record.Warnf(s.scope.InfraCluster(), "FailedCreateSubnet", "Failed creating new managed Subnet %v", err) return nil, errors.Wrap(err, "failed to create subnet") } - record.Eventf(s.scope.AWSCluster, "SuccessfulCreateSubnet", "Created new managed Subnet %q", *out.Subnet.SubnetId) + record.Eventf(s.scope.InfraCluster(), "SuccessfulCreateSubnet", "Created new managed Subnet %q", *out.Subnet.SubnetId) wReq := &ec2.DescribeSubnetsInput{SubnetIds: []*string{out.Subnet.SubnetId}} - if err := s.scope.EC2.WaitUntilSubnetAvailable(wReq); err != nil { + if err := s.EC2Client.WaitUntilSubnetAvailable(wReq); err != nil { return nil, errors.Wrapf(err, "failed to wait for subnet %q", *out.Subnet.SubnetId) } @@ -317,15 +317,15 @@ func (s *Service) createSubnet(sn *infrav1.SubnetSpec) (*infrav1.SubnetSpec, err } if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) { - if _, err := s.scope.EC2.ModifySubnetAttribute(attReq); err != nil { + if _, err := s.EC2Client.ModifySubnetAttribute(attReq); err != nil { return false, err } return true, nil }, awserrors.SubnetNotFound); err != nil { - record.Warnf(s.scope.AWSCluster, "FailedModifySubnetAttributes", "Failed modifying managed Subnet %q attributes: %v", *out.Subnet.SubnetId, err) + record.Warnf(s.scope.InfraCluster(), "FailedModifySubnetAttributes", "Failed modifying managed Subnet %q attributes: %v", *out.Subnet.SubnetId, err) return nil, errors.Wrapf(err, "failed to set subnet %q attributes", *out.Subnet.SubnetId) } - record.Eventf(s.scope.AWSCluster, "SuccessfulModifySubnetAttributes", "Modified managed Subnet %q attributes", *out.Subnet.SubnetId) + record.Eventf(s.scope.InfraCluster(), "SuccessfulModifySubnetAttributes", "Modified managed Subnet %q attributes", *out.Subnet.SubnetId) } s.scope.V(2).Info("Created new subnet in VPC with cidr and availability zone ", @@ -343,16 +343,16 @@ func (s *Service) createSubnet(sn *infrav1.SubnetSpec) (*infrav1.SubnetSpec, err } func (s *Service) deleteSubnet(id string) error { - _, err := s.scope.EC2.DeleteSubnet(&ec2.DeleteSubnetInput{ + _, err := s.EC2Client.DeleteSubnet(&ec2.DeleteSubnetInput{ SubnetId: aws.String(id), }) if err != nil { - record.Warnf(s.scope.AWSCluster, "FailedDeleteSubnet", "Failed to delete managed Subnet %q: %v", id, err) + record.Warnf(s.scope.InfraCluster(), "FailedDeleteSubnet", "Failed to delete managed Subnet %q: %v", id, err) return errors.Wrapf(err, "failed to delete subnet %q", id) } s.scope.V(2).Info("Deleted subnet in vpc", "subnet-id", id, "vpc-id", s.scope.VPC().ID) - record.Eventf(s.scope.AWSCluster, "SuccessfulDeleteSubnet", "Deleted managed Subnet %q", id) + record.Eventf(s.scope.InfraCluster(), "SuccessfulDeleteSubnet", "Deleted managed Subnet %q", id) return nil } diff --git a/pkg/cloud/services/ec2/subnets_test.go b/pkg/cloud/services/network/subnets_test.go similarity index 99% rename from pkg/cloud/services/ec2/subnets_test.go rename to pkg/cloud/services/network/subnets_test.go index 5e2905fad6..5a5d400462 100644 --- a/pkg/cloud/services/ec2/subnets_test.go +++ b/pkg/cloud/services/network/subnets_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package ec2 +package network import ( "encoding/json" @@ -30,7 +30,6 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-aws/api/v1alpha3" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/scope" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/ec2/mock_ec2iface" - "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/elb/mock_elbiface" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" ) @@ -1385,16 +1384,11 @@ func TestReconcileSubnets(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() ec2Mock := mock_ec2iface.NewMockEC2API(mockCtrl) - elbMock := mock_elbiface.NewMockELBAPI(mockCtrl) scope, err := scope.NewClusterScope(scope.ClusterScopeParams{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{Name: "test-cluster"}, }, - AWSClients: scope.AWSClients{ - EC2: ec2Mock, - ELB: elbMock, - }, AWSCluster: &infrav1.AWSCluster{ Spec: infrav1.AWSClusterSpec{ NetworkSpec: *tc.input, @@ -1408,6 +1402,7 @@ func TestReconcileSubnets(t *testing.T) { tc.expect(ec2Mock.EXPECT()) s := NewService(scope) + s.EC2Client = ec2Mock err = s.reconcileSubnets() if tc.errorExpected && err == nil { @@ -1572,16 +1567,11 @@ func TestDiscoverSubnets(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() ec2Mock := mock_ec2iface.NewMockEC2API(mockCtrl) - elbMock := mock_elbiface.NewMockELBAPI(mockCtrl) scope, err := scope.NewClusterScope(scope.ClusterScopeParams{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{Name: "test-cluster"}, }, - AWSClients: scope.AWSClients{ - EC2: ec2Mock, - ELB: elbMock, - }, AWSCluster: &infrav1.AWSCluster{ Spec: infrav1.AWSClusterSpec{ NetworkSpec: *tc.input, @@ -1595,11 +1585,13 @@ func TestDiscoverSubnets(t *testing.T) { tc.mocks(ec2Mock.EXPECT()) s := NewService(scope) + s.EC2Client = ec2Mock + if err := s.reconcileSubnets(); err != nil { t.Fatalf("got an unexpected error: %v", err) } - subnets := s.scope.AWSCluster.Spec.NetworkSpec.Subnets + subnets := s.scope.Subnets() out := make(map[string]*infrav1.SubnetSpec) for _, sn := range subnets { out[sn.ID] = sn diff --git a/pkg/cloud/services/ec2/vpc.go b/pkg/cloud/services/network/vpc.go similarity index 85% rename from pkg/cloud/services/ec2/vpc.go rename to pkg/cloud/services/network/vpc.go index 15ee799415..b9bce0edd0 100644 --- a/pkg/cloud/services/ec2/vpc.go +++ b/pkg/cloud/services/network/vpc.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package ec2 +package network import ( "fmt" @@ -45,8 +45,8 @@ func (s *Service) reconcileVPC() error { vpc, err := s.describeVPC() if awserrors.IsNotFound(err) { // nolint:nestif // Create a new managed vpc. - if !conditions.Has(s.scope.AWSCluster, infrav1.VpcReadyCondition) { - conditions.MarkFalse(s.scope.AWSCluster, infrav1.VpcReadyCondition, infrav1.VpcCreationStartedReason, clusterv1.ConditionSeverityInfo, "") + if !conditions.Has(s.scope.InfraCluster(), infrav1.VpcReadyCondition) { + conditions.MarkFalse(s.scope.InfraCluster(), infrav1.VpcReadyCondition, infrav1.VpcCreationStartedReason, clusterv1.ConditionSeverityInfo, "") if err := s.scope.PatchObject(); err != nil { return errors.Wrap(err, "failed to patch conditions") } @@ -82,14 +82,14 @@ func (s *Service) reconcileVPC() error { // Make sure attributes are configured if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) { if err := tags.Ensure(vpc.Tags, &tags.ApplyParams{ - EC2Client: s.scope.EC2, + EC2Client: s.EC2Client, BuildParams: s.getVPCTagParams(vpc.ID), }); err != nil { return false, err } return true, nil }, awserrors.VPCNotFound); err != nil { - record.Warnf(s.scope.AWSCluster, "FailedTagVPC", "Failed to tag managed VPC %q: %v", vpc.ID, err) + record.Warnf(s.scope.InfraCluster(), "FailedTagVPC", "Failed to tag managed VPC %q: %v", vpc.ID, err) return errors.Wrapf(err, "failed to tag vpc %q", vpc.ID) } @@ -119,7 +119,7 @@ func (s *Service) ensureManagedVPCAttributes(vpc *infrav1.VPCSpec) error { VpcId: aws.String(vpc.ID), Attribute: aws.String("enableDnsHostnames"), } - vpcAttr, err := s.scope.EC2.DescribeVpcAttribute(descAttrInput) + vpcAttr, err := s.EC2Client.DescribeVpcAttribute(descAttrInput) if err != nil { errs = append(errs, errors.Wrap(err, "failed to describe enableDnsHostnames vpc attribute")) } else if !aws.BoolValue(vpcAttr.EnableDnsHostnames.Value) { @@ -127,7 +127,7 @@ func (s *Service) ensureManagedVPCAttributes(vpc *infrav1.VPCSpec) error { VpcId: aws.String(vpc.ID), EnableDnsHostnames: &ec2.AttributeBooleanValue{Value: aws.Bool(true)}, } - if _, err := s.scope.EC2.ModifyVpcAttribute(attrInput); err != nil { + if _, err := s.EC2Client.ModifyVpcAttribute(attrInput); err != nil { errs = append(errs, errors.Wrap(err, "failed to set enableDnsHostnames vpc attribute")) } else { updated = true @@ -138,7 +138,7 @@ func (s *Service) ensureManagedVPCAttributes(vpc *infrav1.VPCSpec) error { VpcId: aws.String(vpc.ID), Attribute: aws.String("enableDnsSupport"), } - vpcAttr, err = s.scope.EC2.DescribeVpcAttribute(descAttrInput) + vpcAttr, err = s.EC2Client.DescribeVpcAttribute(descAttrInput) if err != nil { errs = append(errs, errors.Wrap(err, "failed to describe enableDnsSupport vpc attribute")) } else if !aws.BoolValue(vpcAttr.EnableDnsSupport.Value) { @@ -146,7 +146,7 @@ func (s *Service) ensureManagedVPCAttributes(vpc *infrav1.VPCSpec) error { VpcId: aws.String(vpc.ID), EnableDnsSupport: &ec2.AttributeBooleanValue{Value: aws.Bool(true)}, } - if _, err := s.scope.EC2.ModifyVpcAttribute(attrInput); err != nil { + if _, err := s.EC2Client.ModifyVpcAttribute(attrInput); err != nil { errs = append(errs, errors.Wrap(err, "failed to set enableDnsSupport vpc attribute")) } else { updated = true @@ -154,12 +154,12 @@ func (s *Service) ensureManagedVPCAttributes(vpc *infrav1.VPCSpec) error { } if len(errs) > 0 { - record.Warnf(s.scope.AWSCluster, "FailedSetVPCAttributes", "Failed to set managed VPC attributes for %q: %v", vpc.ID, err) + record.Warnf(s.scope.InfraCluster(), "FailedSetVPCAttributes", "Failed to set managed VPC attributes for %q: %v", vpc.ID, err) return kerrors.NewAggregate(errs) } if updated { - record.Eventf(s.scope.AWSCluster, "SuccessfulSetVPCAttributes", "Set managed VPC attributes for %q", vpc.ID) + record.Eventf(s.scope.InfraCluster(), "SuccessfulSetVPCAttributes", "Set managed VPC attributes for %q", vpc.ID) } return nil @@ -181,13 +181,13 @@ func (s *Service) createVPC() (*infrav1.VPCSpec, error) { }, } - out, err := s.scope.EC2.CreateVpc(input) + out, err := s.EC2Client.CreateVpc(input) if err != nil { - record.Warnf(s.scope.AWSCluster, "FailedCreateVPC", "Failed to create new managed VPC: %v", err) + record.Warnf(s.scope.InfraCluster(), "FailedCreateVPC", "Failed to create new managed VPC: %v", err) return nil, errors.Wrap(err, "failed to create vpc") } - record.Eventf(s.scope.AWSCluster, "SuccessfulCreateVPC", "Created new managed VPC %q", *out.Vpc.VpcId) + record.Eventf(s.scope.InfraCluster(), "SuccessfulCreateVPC", "Created new managed VPC %q", *out.Vpc.VpcId) s.scope.V(2).Info("Created new VPC with cidr", "vpc-id", *out.Vpc.VpcId, "cidr-block", *out.Vpc.CidrBlock) // TODO: we should attempt to record the VPC ID as soon as possible by setting s.scope.VPC().ID @@ -195,7 +195,7 @@ func (s *Service) createVPC() (*infrav1.VPCSpec, error) { // need to be updated to accommodate for the recording of the VPC ID prior to the tagging. wReq := &ec2.DescribeVpcsInput{VpcIds: []*string{out.Vpc.VpcId}} - if err := s.scope.EC2.WaitUntilVpcAvailable(wReq); err != nil { + if err := s.EC2Client.WaitUntilVpcAvailable(wReq); err != nil { return nil, errors.Wrapf(err, "failed to wait for vpc %q", *out.Vpc.VpcId) } @@ -218,18 +218,18 @@ func (s *Service) deleteVPC() error { VpcId: aws.String(vpc.ID), } - if _, err := s.scope.EC2.DeleteVpc(input); err != nil { + if _, err := s.EC2Client.DeleteVpc(input); err != nil { // Ignore if it's already deleted if code, ok := awserrors.Code(err); ok && code == awserrors.VPCNotFound { s.scope.V(4).Info("Skipping VPC deletion, VPC not found") return nil } - record.Warnf(s.scope.AWSCluster, "FailedDeleteVPC", "Failed to delete managed VPC %q: %v", vpc.ID, err) + record.Warnf(s.scope.InfraCluster(), "FailedDeleteVPC", "Failed to delete managed VPC %q: %v", vpc.ID, err) return errors.Wrapf(err, "failed to delete vpc %q", vpc.ID) } s.scope.V(2).Info("Deleted VPC", "vpc-id", vpc.ID) - record.Eventf(s.scope.AWSCluster, "SuccessfulDeleteVPC", "Deleted managed VPC %q", vpc.ID) + record.Eventf(s.scope.InfraCluster(), "SuccessfulDeleteVPC", "Deleted managed VPC %q", vpc.ID) return nil } @@ -247,7 +247,7 @@ func (s *Service) describeVPC() (*infrav1.VPCSpec, error) { input.VpcIds = []*string{aws.String(s.scope.VPC().ID)} } - out, err := s.scope.EC2.DescribeVpcs(input) + out, err := s.EC2Client.DescribeVpcs(input) if err != nil { if awserrors.IsNotFound(err) { return nil, err diff --git a/pkg/cloud/services/ec2/vpc_test.go b/pkg/cloud/services/network/vpc_test.go similarity index 97% rename from pkg/cloud/services/ec2/vpc_test.go rename to pkg/cloud/services/network/vpc_test.go index 53ba46657e..b28b0cb8cf 100644 --- a/pkg/cloud/services/ec2/vpc_test.go +++ b/pkg/cloud/services/network/vpc_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package ec2 +package network import ( "reflect" @@ -29,7 +29,6 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-aws/api/v1alpha3" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/scope" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/ec2/mock_ec2iface" - "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/elb/mock_elbiface" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -196,7 +195,6 @@ func TestReconcileVPC(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { ec2Mock := mock_ec2iface.NewMockEC2API(mockCtrl) - elbMock := mock_elbiface.NewMockELBAPI(mockCtrl) scheme := runtime.NewScheme() _ = infrav1.AddToScheme(scheme) @@ -213,10 +211,6 @@ func TestReconcileVPC(t *testing.T) { Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{Name: "test-cluster"}, }, - AWSClients: scope.AWSClients{ - EC2: ec2Mock, - ELB: elbMock, - }, AWSCluster: awsCluster, Client: client, }) @@ -227,6 +221,8 @@ func TestReconcileVPC(t *testing.T) { tc.expect(ec2Mock.EXPECT()) s := NewService(clusterScope) + s.EC2Client = ec2Mock + if err := s.reconcileVPC(); err != nil { t.Fatalf("got an unexpected error: %v", err) } diff --git a/pkg/cloud/services/secretsmanager/secret.go b/pkg/cloud/services/secretsmanager/secret.go index dfbb4425db..e71fd01323 100644 --- a/pkg/cloud/services/secretsmanager/secret.go +++ b/pkg/cloud/services/secretsmanager/secret.go @@ -84,7 +84,7 @@ func (s *Service) Create(m *scope.MachineScope, data []byte) (string, int32, err // retryableCreateSecret is a function to be passed into a waiter. In a separate function for ease of reading func (s *Service) retryableCreateSecret(name string, chunk []byte, tags infrav1.Tags) (bool, error) { - _, err := s.scope.SecretsManager.CreateSecret(&secretsmanager.CreateSecretInput{ + _, err := s.SecretsManagerClient.CreateSecret(&secretsmanager.CreateSecretInput{ Name: aws.String(name), SecretBinary: chunk, Tags: converters.MapToSecretsManagerTags(tags), @@ -101,7 +101,7 @@ func (s *Service) retryableCreateSecret(name string, chunk []byte, tags infrav1. // forceDeleteSecretEntry deletes a single secret, ignoring if it is absent func (s *Service) forceDeleteSecretEntry(name string) error { - _, err := s.scope.SecretsManager.DeleteSecret(&secretsmanager.DeleteSecretInput{ + _, err := s.SecretsManagerClient.DeleteSecret(&secretsmanager.DeleteSecretInput{ SecretId: aws.String(name), ForceDeleteWithoutRecovery: aws.Bool(true), }) diff --git a/pkg/cloud/services/secretsmanager/service.go b/pkg/cloud/services/secretsmanager/service.go index ad46ff7914..9a3d716bca 100644 --- a/pkg/cloud/services/secretsmanager/service.go +++ b/pkg/cloud/services/secretsmanager/service.go @@ -17,6 +17,9 @@ limitations under the License. package secretsmanager import ( + "github.com/aws/aws-sdk-go/service/secretsmanager/secretsmanageriface" + + "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/scope" ) @@ -24,12 +27,14 @@ import ( // The interfaces are broken down like this to group functions together. // One alternative is to have a large list of functions from the ec2 client. type Service struct { - scope *scope.ClusterScope + scope cloud.ClusterScoper + SecretsManagerClient secretsmanageriface.SecretsManagerAPI } // NewService returns a new service given the api clients. -func NewService(scope *scope.ClusterScope) *Service { +func NewService(secretsScope cloud.ClusterScoper) *Service { return &Service{ - scope: scope, + scope: secretsScope, + SecretsManagerClient: scope.NewSecretsManagerClient(secretsScope, secretsScope, secretsScope.InfraCluster()), } }