From d3df0ea840bb44b0df900f1364c8fde88fa742f8 Mon Sep 17 00:00:00 2001 From: Chuck Ha Date: Thu, 20 Sep 2018 11:07:04 -0400 Subject: [PATCH] Adds simple Exists implementation to machine actuator (#67) * Adds simple Exists implementation to machine actuator Remove the machine actuator test for now. Trying to test it will be possible, but it will be a *ton* of work and right now it's not worth the effort. Exports the ec2 service's EC2 field. Signed-off-by: Chuck Ha * Update deps Signed-off-by: Chuck Ha --- cloud/aws/actuators/machine/actuator.go | 25 ++++- cloud/aws/actuators/machine/actuator_test.go | 109 ------------------- cloud/aws/services/ec2/gateways.go | 6 +- cloud/aws/services/ec2/instances.go | 21 +++- cloud/aws/services/ec2/instances_test.go | 82 +++++++++++++- cloud/aws/services/ec2/routetables.go | 6 +- cloud/aws/services/ec2/service.go | 4 +- cloud/aws/services/ec2/subnets.go | 10 +- cloud/aws/services/ec2/utils.go | 4 +- cloud/aws/services/ec2/vpc.go | 8 +- 10 files changed, 138 insertions(+), 137 deletions(-) delete mode 100644 cloud/aws/actuators/machine/actuator_test.go diff --git a/cloud/aws/actuators/machine/actuator.go b/cloud/aws/actuators/machine/actuator.go index 0d335aa0ed..4fca06cbc4 100644 --- a/cloud/aws/actuators/machine/actuator.go +++ b/cloud/aws/actuators/machine/actuator.go @@ -13,6 +13,7 @@ package machine +// should not need to import the ec2 sdk here import ( "fmt" @@ -145,10 +146,28 @@ func (a *Actuator) Update(cluster *clusterv1.Cluster, machine *clusterv1.Machine return fmt.Errorf("TODO: Not yet implemented") } -// Exists test for the existance of a machine and is invoked by the Machine Controller +// Exists test for the existence of a machine and is invoked by the Machine Controller func (a *Actuator) Exists(cluster *clusterv1.Cluster, machine *clusterv1.Machine) (bool, error) { - glog.Infof("Checking if machine %v for cluster %v exists.", machine.Name, cluster.Name) - return false, fmt.Errorf("TODO: Not yet implemented") + glog.Info("Checking if machine %v for cluster %v exists.", machine.Name, cluster.Name) + status, err := a.machineProviderStatus(machine) + if err != nil { + return false, err + } + + instance, err := a.ec2.InstanceIfExists(status.InstanceID) + if err != nil { + return false, err + } + if instance == nil { + return false, nil + } + // TODO update status here + switch instance.State { + case ec2svc.InstanceStateRunning, ec2svc.InstanceStatePending: + return true, nil + default: + return false, nil + } } func (a *Actuator) machineProviderConfig(providerConfig clusterv1.ProviderConfig) (*v1alpha1.AWSMachineProviderConfig, error) { diff --git a/cloud/aws/actuators/machine/actuator_test.go b/cloud/aws/actuators/machine/actuator_test.go deleted file mode 100644 index 9d749acf54..0000000000 --- a/cloud/aws/actuators/machine/actuator_test.go +++ /dev/null @@ -1,109 +0,0 @@ -// Copyright © 2018 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 machine_test - -import ( - "errors" - "testing" - - "sigs.k8s.io/cluster-api-provider-aws/cloud/aws/actuators/machine" - "sigs.k8s.io/cluster-api-provider-aws/cloud/aws/providerconfig/v1alpha1" - ec2svc "sigs.k8s.io/cluster-api-provider-aws/cloud/aws/services/ec2" - clusterv1 "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1" -) - -type ec2 struct{} - -func (e *ec2) CreateInstance(machine *clusterv1.Machine) (*ec2svc.Instance, error) { - return &ec2svc.Instance{ - ID: "abc", - }, nil -} -func (e *ec2) InstanceIfExists(id *string) (*ec2svc.Instance, error) { - if id == nil { - return nil, nil - } - if *id == "abc" { - return &ec2svc.Instance{ - ID: "abc", - }, nil - } - return nil, nil -} - -func (e *ec2) TerminateInstance(instanceID *string) error { - if instanceID == nil { - return errors.New("didn't receive an instanceID") - } - - return nil -} - -type machines struct{} - -func (m *machines) UpdateMachineStatus(machine *clusterv1.Machine) (*clusterv1.Machine, error) { - return machine, nil -} - -func TestCreate(t *testing.T) { - codec, err := v1alpha1.NewCodec() - if err != nil { - t.Fatalf("failed to create a codec: %v", err) - } - ap := machine.ActuatorParams{ - Codec: codec, - MachinesService: &machines{}, - EC2Service: &ec2{}, - } - actuator, err := machine.NewActuator(ap) - if err != nil { - t.Fatalf("failed to create an actuator: %v", err) - } - - if err := actuator.Create(&clusterv1.Cluster{}, &clusterv1.Machine{}); err != nil { - t.Fatalf("failed to create machine: %v", err) - } -} - -func TestDelete(t *testing.T) { - codec, err := v1alpha1.NewCodec() - if err != nil { - t.Fatalf("failed to create a codec: %v", err) - } - - ap := machine.ActuatorParams{ - Codec: codec, - MachinesService: &machines{}, - EC2Service: &ec2{}, - } - - actuator, err := machine.NewActuator(ap) - if err != nil { - t.Fatalf("failed to create an actuator: %v", err) - } - - // Get some empty cluster and machine structs. - testCluster := &clusterv1.Cluster{} - testMachine := &clusterv1.Machine{} - - // Create a machine that we can delete. - if err := actuator.Create(testCluster, testMachine); err != nil { - t.Fatalf("failed to create machine: %v", err) - } - - // Delete the machine. - if err := actuator.Delete(testCluster, testMachine); err != nil { - t.Fatalf("failed to delete machine: %v", err) - } -} diff --git a/cloud/aws/services/ec2/gateways.go b/cloud/aws/services/ec2/gateways.go index a0a1e461a4..9200a27ae2 100644 --- a/cloud/aws/services/ec2/gateways.go +++ b/cloud/aws/services/ec2/gateways.go @@ -37,12 +37,12 @@ func (s *Service) reconcileInternetGateways(in *v1alpha1.Network) error { } func (s *Service) createInternetGateway(vpc *v1alpha1.VPC) (*ec2.InternetGateway, error) { - ig, err := s.ec2.CreateInternetGateway(&ec2.CreateInternetGatewayInput{}) + ig, err := s.EC2.CreateInternetGateway(&ec2.CreateInternetGatewayInput{}) if err != nil { return nil, errors.Wrap(err, "failed to create internet gateway") } - _, err = s.ec2.AttachInternetGateway(&ec2.AttachInternetGatewayInput{ + _, err = s.EC2.AttachInternetGateway(&ec2.AttachInternetGatewayInput{ InternetGatewayId: ig.InternetGateway.InternetGatewayId, VpcId: aws.String(vpc.ID), }) @@ -55,7 +55,7 @@ func (s *Service) createInternetGateway(vpc *v1alpha1.VPC) (*ec2.InternetGateway } func (s *Service) describeVpcInternetGateways(vpc *v1alpha1.VPC) ([]*ec2.InternetGateway, error) { - out, err := s.ec2.DescribeInternetGateways(&ec2.DescribeInternetGatewaysInput{ + out, err := s.EC2.DescribeInternetGateways(&ec2.DescribeInternetGatewaysInput{ Filters: []*ec2.Filter{ { Name: aws.String("attachment.vpc-id"), diff --git a/cloud/aws/services/ec2/instances.go b/cloud/aws/services/ec2/instances.go index 8d52f1416f..1be2008ccd 100644 --- a/cloud/aws/services/ec2/instances.go +++ b/cloud/aws/services/ec2/instances.go @@ -14,6 +14,8 @@ package ec2 import ( + "fmt" + "github.com/aws/aws-sdk-go/service/ec2" "github.com/pkg/errors" clusterv1 "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1" @@ -25,6 +27,12 @@ const ( // InstanceStateTerminated indicates the instance has been terminated InstanceStateTerminated = ec2.InstanceStateNameTerminated + + // InstanceStateRunning indicates the instance is running + InstanceStateRunning = ec2.InstanceStateNameRunning + + // InstanceStatePending indicates the instance is pending + InstanceStatePending = ec2.InstanceStateNamePending ) // Instance is an internal representation of an AWS instance. @@ -41,10 +49,13 @@ func (s *Service) InstanceIfExists(instanceID *string) (*Instance, error) { input := &ec2.DescribeInstancesInput{ InstanceIds: []*string{instanceID}, } + out, err := s.EC2.DescribeInstances(input) - out, err := s.ec2.DescribeInstances(input) - if err != nil { - return nil, errors.Wrapf(err, "failed to describe instances") + switch { + case IsNotFound(err): + return nil, nil + case err != nil: + return nil, fmt.Errorf("failed to describe instances: %v", err) } if len(out.Reservations) > 0 && len(out.Reservations[0].Instances) > 0 { @@ -61,7 +72,7 @@ func (s *Service) InstanceIfExists(instanceID *string) (*Instance, error) { func (s *Service) CreateInstance(machine *clusterv1.Machine) (*Instance, error) { input := &ec2.RunInstancesInput{} - reservation, err := s.ec2.RunInstances(input) + reservation, err := s.EC2.RunInstances(input) if err != nil { return nil, errors.Wrapf(err, "failed to run instances") } @@ -85,7 +96,7 @@ func (s *Service) TerminateInstance(instanceID *string) error { }, } - _, err := s.ec2.TerminateInstances(input) + _, err := s.EC2.TerminateInstances(input) if err != nil { return err } diff --git a/cloud/aws/services/ec2/instances_test.go b/cloud/aws/services/ec2/instances_test.go index 9e12654c07..67cc4962fb 100644 --- a/cloud/aws/services/ec2/instances_test.go +++ b/cloud/aws/services/ec2/instances_test.go @@ -20,8 +20,11 @@ import ( "github.com/aws/aws-sdk-go/service/ec2" "github.com/golang/mock/gomock" "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/runtime" + ec2svc "sigs.k8s.io/cluster-api-provider-aws/cloud/aws/services/ec2" "sigs.k8s.io/cluster-api-provider-aws/cloud/aws/services/ec2/mock_ec2iface" + clusterv1 "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1" ) func TestInstanceIfExists(t *testing.T) { @@ -42,7 +45,7 @@ func TestInstanceIfExists(t *testing.T) { DescribeInstances(gomock.Eq(&ec2.DescribeInstancesInput{ InstanceIds: []*string{aws.String("hello")}, })). - Return(&ec2.DescribeInstancesOutput{}, nil) + Return(nil, ec2svc.NewNotFound(errors.New("not found"))) }, check: func(instance *ec2svc.Instance, err error) { if err != nil { @@ -92,6 +95,20 @@ func TestInstanceIfExists(t *testing.T) { } }, }, + { + name: "error describing instances", + instanceID: "one", + expect: func(m *mock_ec2iface.MockEC2API) { + m.EXPECT(). + DescribeInstances(&ec2.DescribeInstancesInput{InstanceIds: []*string{aws.String("one")}}). + Return(nil, errors.New("some unknown error")) + }, + check: func(i *ec2svc.Instance, err error) { + if err == nil { + t.Fatalf("expected an error but got none.") + } + }, + }, } for _, tc := range testCases { @@ -161,3 +178,66 @@ func TestTerminateInstance(t *testing.T) { }) } } + +func TestCreateInstance(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + testcases := []struct { + name string + machine clusterv1.Machine + expect func(m *mock_ec2iface.MockEC2API) + check func(instance *ec2svc.Instance, err error) + }{ + { + name: "simple", + machine: clusterv1.Machine{ + Spec: clusterv1.MachineSpec{ + ProviderConfig: clusterv1.ProviderConfig{ + Value: &runtime.RawExtension{ + Raw: []byte(`apiVersion: "cluster.k8s.io/v1alpha1" +kind: Machine +metadata: + generateName: aws-controlplane- + labels: + set: controlplane +spec: + versions: + kubelet: v1.11.2 + controlPlane: v1.11.2`), + }, + }, + }, + }, + expect: func(m *mock_ec2iface.MockEC2API) { + m.EXPECT(). + RunInstances(&ec2.RunInstancesInput{}). + Return(&ec2.Reservation{ + Instances: []*ec2.Instance{ + &ec2.Instance{ + State: &ec2.InstanceState{ + Name: aws.String(ec2.InstanceStateNamePending), + }, + InstanceId: aws.String("two"), + }, + }, + }, nil) + }, + check: func(instance *ec2svc.Instance, err error) { + if err != nil { + t.Fatalf("did not expect error: %v", err) + } + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + ec2Mock := mock_ec2iface.NewMockEC2API(mockCtrl) + tc.expect(ec2Mock) + s := ec2svc.NewService(ec2Mock) + instance, err := s.CreateInstance(&tc.machine) + tc.check(instance, err) + }) + } +} diff --git a/cloud/aws/services/ec2/routetables.go b/cloud/aws/services/ec2/routetables.go index ef9898e47d..c7a4d267db 100644 --- a/cloud/aws/services/ec2/routetables.go +++ b/cloud/aws/services/ec2/routetables.go @@ -26,7 +26,7 @@ func (s *Service) reconcileRouteTables(in *v1alpha1.Network) error { } func (s *Service) describeVpcRouteTables(vpcID string) ([]*ec2.RouteTable, error) { - out, err := s.ec2.DescribeRouteTables(&ec2.DescribeRouteTablesInput{ + out, err := s.EC2.DescribeRouteTables(&ec2.DescribeRouteTablesInput{ Filters: []*ec2.Filter{ { Name: aws.String("vpc-id"), @@ -43,7 +43,7 @@ func (s *Service) describeVpcRouteTables(vpcID string) ([]*ec2.RouteTable, error } func (s *Service) createRouteTable(rt *v1alpha1.RouteTable, vpc *v1alpha1.VPC) (*v1alpha1.RouteTable, error) { - out, err := s.ec2.CreateRouteTable(&ec2.CreateRouteTableInput{ + out, err := s.EC2.CreateRouteTable(&ec2.CreateRouteTableInput{ VpcId: aws.String(vpc.ID), }) @@ -57,7 +57,7 @@ func (s *Service) createRouteTable(rt *v1alpha1.RouteTable, vpc *v1alpha1.VPC) ( } func (s *Service) associateRouteTable(rt *v1alpha1.RouteTable, subnetID string) error { - _, err := s.ec2.AssociateRouteTable(&ec2.AssociateRouteTableInput{ + _, err := s.EC2.AssociateRouteTable(&ec2.AssociateRouteTableInput{ RouteTableId: aws.String(rt.ID), SubnetId: aws.String(subnetID), }) diff --git a/cloud/aws/services/ec2/service.go b/cloud/aws/services/ec2/service.go index 5aa7e9f7c9..419c658d83 100644 --- a/cloud/aws/services/ec2/service.go +++ b/cloud/aws/services/ec2/service.go @@ -21,12 +21,12 @@ 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 { - ec2 ec2iface.EC2API + EC2 ec2iface.EC2API } // NewService returns a new service given the ec2 api client. func NewService(i ec2iface.EC2API) *Service { return &Service{ - ec2: i, + EC2: i, } } diff --git a/cloud/aws/services/ec2/subnets.go b/cloud/aws/services/ec2/subnets.go index c1dfe02852..fcf13a3d00 100644 --- a/cloud/aws/services/ec2/subnets.go +++ b/cloud/aws/services/ec2/subnets.go @@ -101,7 +101,7 @@ func (s *Service) reconcileSubnets(subnets v1alpha1.Subnets, vpc *v1alpha1.VPC) } func (s *Service) describeVpcSubnets(vpcID string) (v1alpha1.Subnets, error) { - out, err := s.ec2.DescribeSubnets(&ec2.DescribeSubnetsInput{ + out, err := s.EC2.DescribeSubnets(&ec2.DescribeSubnetsInput{ Filters: []*ec2.Filter{ { Name: aws.String("vpc-id"), @@ -129,7 +129,7 @@ func (s *Service) describeVpcSubnets(vpcID string) (v1alpha1.Subnets, error) { } func (s *Service) createSubnet(sn *v1alpha1.Subnet) (*v1alpha1.Subnet, error) { - out, err := s.ec2.CreateSubnet(&ec2.CreateSubnetInput{ + out, err := s.EC2.CreateSubnet(&ec2.CreateSubnetInput{ VpcId: aws.String(sn.VpcID), CidrBlock: aws.String(sn.CidrBlock), AvailabilityZone: aws.String(sn.AvailabilityZone), @@ -140,7 +140,7 @@ func (s *Service) createSubnet(sn *v1alpha1.Subnet) (*v1alpha1.Subnet, error) { } wReq := &ec2.DescribeSubnetsInput{SubnetIds: []*string{out.Subnet.SubnetId}} - if err := s.ec2.WaitUntilSubnetAvailable(wReq); err != nil { + if err := s.EC2.WaitUntilSubnetAvailable(wReq); err != nil { return nil, errors.Wrapf(err, "failed to wait for subnet %q", *out.Subnet.SubnetId) } @@ -151,7 +151,7 @@ func (s *Service) createSubnet(sn *v1alpha1.Subnet) (*v1alpha1.Subnet, error) { }, } - if _, err := s.ec2.ModifySubnetAttribute(attReq); err != nil { + if _, err := s.EC2.ModifySubnetAttribute(attReq); err != nil { return nil, errors.Wrapf(err, "failed to set subnet %q attributes", *out.Subnet.SubnetId) } } @@ -166,7 +166,7 @@ func (s *Service) createSubnet(sn *v1alpha1.Subnet) (*v1alpha1.Subnet, error) { } func (s *Service) deleteSubnet(sn *v1alpha1.Subnet) error { - _, err := s.ec2.DeleteSubnet(&ec2.DeleteSubnetInput{ + _, err := s.EC2.DeleteSubnet(&ec2.DeleteSubnetInput{ SubnetId: aws.String(sn.ID), }) diff --git a/cloud/aws/services/ec2/utils.go b/cloud/aws/services/ec2/utils.go index f8367b79ea..42082455d9 100644 --- a/cloud/aws/services/ec2/utils.go +++ b/cloud/aws/services/ec2/utils.go @@ -24,7 +24,7 @@ const ( ) func (s *Service) getRegion() string { - switch x := s.ec2.(type) { + switch x := s.EC2.(type) { case *ec2.EC2: return *x.Config.Region default: @@ -33,7 +33,7 @@ func (s *Service) getRegion() string { } func (s *Service) getAvailableZones() ([]string, error) { - out, err := s.ec2.DescribeAvailabilityZones(&ec2.DescribeAvailabilityZonesInput{ + out, err := s.EC2.DescribeAvailabilityZones(&ec2.DescribeAvailabilityZonesInput{ Filters: []*ec2.Filter{ { Name: aws.String("state"), diff --git a/cloud/aws/services/ec2/vpc.go b/cloud/aws/services/ec2/vpc.go index 642ae2f56b..f7264f94f0 100644 --- a/cloud/aws/services/ec2/vpc.go +++ b/cloud/aws/services/ec2/vpc.go @@ -53,13 +53,13 @@ func (s *Service) createVPC(v *v1alpha1.VPC) (*v1alpha1.VPC, error) { CidrBlock: aws.String(v.CidrBlock), } - out, err := s.ec2.CreateVpc(input) + out, err := s.EC2.CreateVpc(input) if err != nil { return nil, errors.Wrap(err, "failed to create vpc") } wReq := &ec2.DescribeVpcsInput{VpcIds: []*string{out.Vpc.VpcId}} - if err := s.ec2.WaitUntilVpcAvailable(wReq); err != nil { + if err := s.EC2.WaitUntilVpcAvailable(wReq); err != nil { return nil, errors.Wrapf(err, "failed to wait for vpc %q", *out.Vpc.VpcId) } @@ -76,7 +76,7 @@ func (s *Service) deleteVPC(v *v1alpha1.VPC) error { VpcId: aws.String(v.ID), } - _, err := s.ec2.DeleteVpc(input) + _, err := s.EC2.DeleteVpc(input) if err != nil { return errors.Wrapf(err, "failed to delete vpc %q", v.ID) } @@ -93,7 +93,7 @@ func (s *Service) describeVPC(id string) (*v1alpha1.VPC, error) { VpcIds: []*string{aws.String(id)}, } - out, err := s.ec2.DescribeVpcs(input) + out, err := s.EC2.DescribeVpcs(input) if err != nil { return nil, err }