From 7a54ec000ca897b00c64b6371acc25dc1fe1c88b Mon Sep 17 00:00:00 2001 From: Jan Chaloupka Date: Tue, 27 Nov 2018 11:44:20 +0100 Subject: [PATCH] Move functions dealing with instances under instances.go file --- pkg/cloud/aws/actuators/machine/actuator.go | 152 +--------------- .../aws/actuators/machine/actuator_test.go | 77 -------- .../aws/actuators/machine/instaces_test.go | 86 +++++++++ pkg/cloud/aws/actuators/machine/instances.go | 166 ++++++++++++++++++ 4 files changed, 253 insertions(+), 228 deletions(-) create mode 100644 pkg/cloud/aws/actuators/machine/instaces_test.go create mode 100644 pkg/cloud/aws/actuators/machine/instances.go diff --git a/pkg/cloud/aws/actuators/machine/actuator.go b/pkg/cloud/aws/actuators/machine/actuator.go index 025ea70366..3e793e4dfe 100644 --- a/pkg/cloud/aws/actuators/machine/actuator.go +++ b/pkg/cloud/aws/actuators/machine/actuator.go @@ -82,21 +82,6 @@ type codec interface { EncodeProviderStatus(runtime.Object) (*runtime.RawExtension, error) } -// Scan machine tags, and return a deduped tags list -func removeDuplicatedTags(tags []*ec2.Tag) []*ec2.Tag { - m := make(map[string]bool) - result := []*ec2.Tag{} - - // look for duplicates - for _, entry := range tags { - if _, value := m[*entry.Key]; !value { - m[*entry.Key] = true - result = append(result, entry) - } - } - return result -} - // NewActuator returns a new AWS Actuator func NewActuator(params ActuatorParams) (*Actuator, error) { actuator := &Actuator{ @@ -178,141 +163,6 @@ func (a *Actuator) updateMachineProviderConditions(machine *clusterv1.Machine, c return nil } -// removeStoppedMachine removes all instances of a specific machine that are in a stopped state. -func (a *Actuator) removeStoppedMachine(machine *clusterv1.Machine, client awsclient.Client) error { - instances, err := GetStoppedInstances(machine, client) - if err != nil { - glog.Errorf("error getting stopped instances: %v", err) - return fmt.Errorf("error getting stopped instances: %v", err) - } - - if len(instances) == 0 { - glog.Infof("no stopped instances found for machine %v", machine.Name) - return nil - } - - return TerminateInstances(client, instances) -} - -func buildEC2Filters(inputFilters []providerconfigv1.Filter) []*ec2.Filter { - filters := make([]*ec2.Filter, len(inputFilters)) - for i, f := range inputFilters { - values := make([]*string, len(f.Values)) - for j, v := range f.Values { - values[j] = aws.String(v) - } - filters[i] = &ec2.Filter{ - Name: aws.String(f.Name), - Values: values, - } - } - return filters -} - -func getSecurityGroupsIDs(securityGroups []providerconfigv1.AWSResourceReference, client awsclient.Client) ([]*string, error) { - var securityGroupIDs []*string - for _, g := range securityGroups { - // ID has priority - if g.ID != nil { - securityGroupIDs = append(securityGroupIDs, g.ID) - } else if g.Filters != nil { - glog.Info("Describing security groups based on filters") - // Get groups based on filters - describeSecurityGroupsRequest := ec2.DescribeSecurityGroupsInput{ - Filters: buildEC2Filters(g.Filters), - } - describeSecurityGroupsResult, err := client.DescribeSecurityGroups(&describeSecurityGroupsRequest) - if err != nil { - glog.Errorf("error describing security groups: %v", err) - return nil, fmt.Errorf("error describing security groups: %v", err) - } - for _, g := range describeSecurityGroupsResult.SecurityGroups { - groupID := *g.GroupId - securityGroupIDs = append(securityGroupIDs, &groupID) - } - } - } - - if len(securityGroups) == 0 { - glog.Info("No security group found") - } - - return securityGroupIDs, nil -} - -func getSubnetIDs(subnet providerconfigv1.AWSResourceReference, availabilityZone string, client awsclient.Client) ([]*string, error) { - var subnetIDs []*string - // ID has priority - if subnet.ID != nil { - subnetIDs = append(subnetIDs, subnet.ID) - } else { - var filters []providerconfigv1.Filter - if availabilityZone != "" { - filters = append(filters, providerconfigv1.Filter{Name: "availabilityZone", Values: []string{availabilityZone}}) - } - filters = append(filters, subnet.Filters...) - glog.Info("Describing subnets based on filters") - describeSubnetRequest := ec2.DescribeSubnetsInput{ - Filters: buildEC2Filters(filters), - } - describeSubnetResult, err := client.DescribeSubnets(&describeSubnetRequest) - if err != nil { - glog.Errorf("error describing subnetes: %v", err) - return nil, fmt.Errorf("error describing subnets: %v", err) - } - for _, n := range describeSubnetResult.Subnets { - subnetID := *n.SubnetId - subnetIDs = append(subnetIDs, &subnetID) - } - } - if len(subnetIDs) == 0 { - return nil, fmt.Errorf("no subnet IDs were found") - } - return subnetIDs, nil -} - -func getAMI(AMI providerconfigv1.AWSResourceReference, client awsclient.Client) (*string, error) { - if AMI.ID != nil { - amiID := AMI.ID - glog.Infof("Using AMI %s", *amiID) - return amiID, nil - } - if len(AMI.Filters) > 0 { - glog.Info("Describing AMI based on filters") - describeImagesRequest := ec2.DescribeImagesInput{ - Filters: buildEC2Filters(AMI.Filters), - } - describeAMIResult, err := client.DescribeImages(&describeImagesRequest) - if err != nil { - glog.Errorf("error describing AMI: %v", err) - return nil, fmt.Errorf("error describing AMI: %v", err) - } - if len(describeAMIResult.Images) < 1 { - glog.Errorf("no image for given filters not found") - return nil, fmt.Errorf("no image for given filters not found") - } - latestImage := describeAMIResult.Images[0] - latestTime, err := time.Parse(time.RFC3339, *latestImage.CreationDate) - if err != nil { - glog.Errorf("unable to parse time for %q AMI: %v", *latestImage.ImageId, err) - return nil, fmt.Errorf("unable to parse time for %q AMI: %v", *latestImage.ImageId, err) - } - for _, image := range describeAMIResult.Images[1:] { - imageTime, err := time.Parse(time.RFC3339, *image.CreationDate) - if err != nil { - glog.Errorf("unable to parse time for %q AMI: %v", *image.ImageId, err) - return nil, fmt.Errorf("unable to parse time for %q AMI: %v", *image.ImageId, err) - } - if latestTime.Before(imageTime) { - latestImage = image - latestTime = imageTime - } - } - return latestImage.ImageId, nil - } - return nil, fmt.Errorf("AMI ID or AMI filters need to be specified") -} - // CreateMachine starts a new AWS instance as described by the cluster and machine resources func (a *Actuator) CreateMachine(cluster *clusterv1.Cluster, machine *clusterv1.Machine) (*ec2.Instance, error) { machineProviderConfig, err := ProviderConfigFromMachine(machine) @@ -334,7 +184,7 @@ func (a *Actuator) CreateMachine(cluster *clusterv1.Cluster, machine *clusterv1. // We explicitly do NOT want to remove stopped masters. if !IsMaster(machine) { // Prevent having a lot of stopped nodes sitting around. - err = a.removeStoppedMachine(machine, client) + err = removeStoppedMachine(machine, client) if err != nil { glog.Errorf("unable to remove stopped machines: %v", err) return nil, fmt.Errorf("unable to remove stopped nodes: %v", err) diff --git a/pkg/cloud/aws/actuators/machine/actuator_test.go b/pkg/cloud/aws/actuators/machine/actuator_test.go index 498a85df6e..0b880468b3 100644 --- a/pkg/cloud/aws/actuators/machine/actuator_test.go +++ b/pkg/cloud/aws/actuators/machine/actuator_test.go @@ -3,7 +3,6 @@ package machine import ( "errors" "fmt" - "reflect" "strings" "testing" "time" @@ -346,82 +345,6 @@ func mockRegisterInstancesWithLoadBalancer(mockAWSClient *mockaws.MockClient, cr } } -func TestRemoveDuplicatedTags(t *testing.T) { - cases := []struct { - tagList []*ec2.Tag - expected []*ec2.Tag - }{ - { - // empty tags - tagList: []*ec2.Tag{}, - expected: []*ec2.Tag{}, - }, - { - // no duplicate tags - tagList: []*ec2.Tag{ - {Key: aws.String("clusterID"), Value: aws.String("test-ClusterIDValue")}, - }, - expected: []*ec2.Tag{ - {Key: aws.String("clusterID"), Value: aws.String("test-ClusterIDValue")}, - }, - }, - { - // multiple duplicate tags - tagList: []*ec2.Tag{ - {Key: aws.String("clusterID"), Value: aws.String("test-ClusterIDValue")}, - {Key: aws.String("clusterSize"), Value: aws.String("test-ClusterSizeValue")}, - {Key: aws.String("clusterSize"), Value: aws.String("test-ClusterSizeDuplicatedValue")}, - }, - expected: []*ec2.Tag{ - {Key: aws.String("clusterID"), Value: aws.String("test-ClusterIDValue")}, - {Key: aws.String("clusterSize"), Value: aws.String("test-ClusterSizeValue")}, - }, - }, - } - - for i, c := range cases { - actual := removeDuplicatedTags(c.tagList) - if !reflect.DeepEqual(c.expected, actual) { - t.Errorf("test #%d: expected %+v, got %+v", i, c.expected, actual) - } - } -} - -func TestBuildEC2Filters(t *testing.T) { - filter1 := "filter1" - filter2 := "filter2" - value1 := "A" - value2 := "B" - value3 := "C" - - inputFilters := []providerconfigv1.Filter{ - { - Name: filter1, - Values: []string{value1, value2}, - }, - { - Name: filter2, - Values: []string{value3}, - }, - } - - expected := []*ec2.Filter{ - { - Name: &filter1, - Values: []*string{&value1, &value2}, - }, - { - Name: &filter2, - Values: []*string{&value3}, - }, - } - - got := buildEC2Filters(inputFilters) - if !reflect.DeepEqual(expected, got) { - t.Errorf("failed to buildEC2Filters. Expected: %+v, got: %+v", expected, got) - } -} - func TestAvailabiltyZone(t *testing.T) { cases := []struct { name string diff --git a/pkg/cloud/aws/actuators/machine/instaces_test.go b/pkg/cloud/aws/actuators/machine/instaces_test.go new file mode 100644 index 0000000000..acf3343a20 --- /dev/null +++ b/pkg/cloud/aws/actuators/machine/instaces_test.go @@ -0,0 +1,86 @@ +package machine + +import ( + "reflect" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + providerconfigv1 "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsproviderconfig/v1alpha1" +) + +func TestRemoveDuplicatedTags(t *testing.T) { + cases := []struct { + tagList []*ec2.Tag + expected []*ec2.Tag + }{ + { + // empty tags + tagList: []*ec2.Tag{}, + expected: []*ec2.Tag{}, + }, + { + // no duplicate tags + tagList: []*ec2.Tag{ + {Key: aws.String("clusterID"), Value: aws.String("test-ClusterIDValue")}, + }, + expected: []*ec2.Tag{ + {Key: aws.String("clusterID"), Value: aws.String("test-ClusterIDValue")}, + }, + }, + { + // multiple duplicate tags + tagList: []*ec2.Tag{ + {Key: aws.String("clusterID"), Value: aws.String("test-ClusterIDValue")}, + {Key: aws.String("clusterSize"), Value: aws.String("test-ClusterSizeValue")}, + {Key: aws.String("clusterSize"), Value: aws.String("test-ClusterSizeDuplicatedValue")}, + }, + expected: []*ec2.Tag{ + {Key: aws.String("clusterID"), Value: aws.String("test-ClusterIDValue")}, + {Key: aws.String("clusterSize"), Value: aws.String("test-ClusterSizeValue")}, + }, + }, + } + + for i, c := range cases { + actual := removeDuplicatedTags(c.tagList) + if !reflect.DeepEqual(c.expected, actual) { + t.Errorf("test #%d: expected %+v, got %+v", i, c.expected, actual) + } + } +} + +func TestBuildEC2Filters(t *testing.T) { + filter1 := "filter1" + filter2 := "filter2" + value1 := "A" + value2 := "B" + value3 := "C" + + inputFilters := []providerconfigv1.Filter{ + { + Name: filter1, + Values: []string{value1, value2}, + }, + { + Name: filter2, + Values: []string{value3}, + }, + } + + expected := []*ec2.Filter{ + { + Name: &filter1, + Values: []*string{&value1, &value2}, + }, + { + Name: &filter2, + Values: []*string{&value3}, + }, + } + + got := buildEC2Filters(inputFilters) + if !reflect.DeepEqual(expected, got) { + t.Errorf("failed to buildEC2Filters. Expected: %+v, got: %+v", expected, got) + } +} diff --git a/pkg/cloud/aws/actuators/machine/instances.go b/pkg/cloud/aws/actuators/machine/instances.go new file mode 100644 index 0000000000..57cd30b41a --- /dev/null +++ b/pkg/cloud/aws/actuators/machine/instances.go @@ -0,0 +1,166 @@ +package machine + +import ( + "fmt" + "time" + + "github.com/golang/glog" + + providerconfigv1 "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsproviderconfig/v1alpha1" + clusterv1 "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + + awsclient "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/client" +) + +// Scan machine tags, and return a deduped tags list +func removeDuplicatedTags(tags []*ec2.Tag) []*ec2.Tag { + m := make(map[string]bool) + result := []*ec2.Tag{} + + // look for duplicates + for _, entry := range tags { + if _, value := m[*entry.Key]; !value { + m[*entry.Key] = true + result = append(result, entry) + } + } + return result +} + +// removeStoppedMachine removes all instances of a specific machine that are in a stopped state. +func removeStoppedMachine(machine *clusterv1.Machine, client awsclient.Client) error { + instances, err := GetStoppedInstances(machine, client) + if err != nil { + glog.Errorf("error getting stopped instances: %v", err) + return fmt.Errorf("error getting stopped instances: %v", err) + } + + if len(instances) == 0 { + glog.Infof("no stopped instances found for machine %v", machine.Name) + return nil + } + + return TerminateInstances(client, instances) +} + +func buildEC2Filters(inputFilters []providerconfigv1.Filter) []*ec2.Filter { + filters := make([]*ec2.Filter, len(inputFilters)) + for i, f := range inputFilters { + values := make([]*string, len(f.Values)) + for j, v := range f.Values { + values[j] = aws.String(v) + } + filters[i] = &ec2.Filter{ + Name: aws.String(f.Name), + Values: values, + } + } + return filters +} + +func getSecurityGroupsIDs(securityGroups []providerconfigv1.AWSResourceReference, client awsclient.Client) ([]*string, error) { + var securityGroupIDs []*string + for _, g := range securityGroups { + // ID has priority + if g.ID != nil { + securityGroupIDs = append(securityGroupIDs, g.ID) + } else if g.Filters != nil { + glog.Info("Describing security groups based on filters") + // Get groups based on filters + describeSecurityGroupsRequest := ec2.DescribeSecurityGroupsInput{ + Filters: buildEC2Filters(g.Filters), + } + describeSecurityGroupsResult, err := client.DescribeSecurityGroups(&describeSecurityGroupsRequest) + if err != nil { + glog.Errorf("error describing security groups: %v", err) + return nil, fmt.Errorf("error describing security groups: %v", err) + } + for _, g := range describeSecurityGroupsResult.SecurityGroups { + groupID := *g.GroupId + securityGroupIDs = append(securityGroupIDs, &groupID) + } + } + } + + if len(securityGroups) == 0 { + glog.Info("No security group found") + } + + return securityGroupIDs, nil +} + +func getSubnetIDs(subnet providerconfigv1.AWSResourceReference, availabilityZone string, client awsclient.Client) ([]*string, error) { + var subnetIDs []*string + // ID has priority + if subnet.ID != nil { + subnetIDs = append(subnetIDs, subnet.ID) + } else { + var filters []providerconfigv1.Filter + if availabilityZone != "" { + filters = append(filters, providerconfigv1.Filter{Name: "availabilityZone", Values: []string{availabilityZone}}) + } + filters = append(filters, subnet.Filters...) + glog.Info("Describing subnets based on filters") + describeSubnetRequest := ec2.DescribeSubnetsInput{ + Filters: buildEC2Filters(filters), + } + describeSubnetResult, err := client.DescribeSubnets(&describeSubnetRequest) + if err != nil { + glog.Errorf("error describing subnetes: %v", err) + return nil, fmt.Errorf("error describing subnets: %v", err) + } + for _, n := range describeSubnetResult.Subnets { + subnetID := *n.SubnetId + subnetIDs = append(subnetIDs, &subnetID) + } + } + if len(subnetIDs) == 0 { + return nil, fmt.Errorf("no subnet IDs were found") + } + return subnetIDs, nil +} + +func getAMI(AMI providerconfigv1.AWSResourceReference, client awsclient.Client) (*string, error) { + if AMI.ID != nil { + amiID := AMI.ID + glog.Infof("Using AMI %s", *amiID) + return amiID, nil + } + if len(AMI.Filters) > 0 { + glog.Info("Describing AMI based on filters") + describeImagesRequest := ec2.DescribeImagesInput{ + Filters: buildEC2Filters(AMI.Filters), + } + describeAMIResult, err := client.DescribeImages(&describeImagesRequest) + if err != nil { + glog.Errorf("error describing AMI: %v", err) + return nil, fmt.Errorf("error describing AMI: %v", err) + } + if len(describeAMIResult.Images) < 1 { + glog.Errorf("no image for given filters not found") + return nil, fmt.Errorf("no image for given filters not found") + } + latestImage := describeAMIResult.Images[0] + latestTime, err := time.Parse(time.RFC3339, *latestImage.CreationDate) + if err != nil { + glog.Errorf("unable to parse time for %q AMI: %v", *latestImage.ImageId, err) + return nil, fmt.Errorf("unable to parse time for %q AMI: %v", *latestImage.ImageId, err) + } + for _, image := range describeAMIResult.Images[1:] { + imageTime, err := time.Parse(time.RFC3339, *image.CreationDate) + if err != nil { + glog.Errorf("unable to parse time for %q AMI: %v", *image.ImageId, err) + return nil, fmt.Errorf("unable to parse time for %q AMI: %v", *image.ImageId, err) + } + if latestTime.Before(imageTime) { + latestImage = image + latestTime = imageTime + } + } + return latestImage.ImageId, nil + } + return nil, fmt.Errorf("AMI ID or AMI filters need to be specified") +}