From 9849db7fdf4db5e7d0290405a3a7d7b0c3555561 Mon Sep 17 00:00:00 2001 From: Adam Sunderland Date: Thu, 14 May 2015 18:53:47 -0500 Subject: [PATCH] Fixing Many Issues --- pkg/cloudprovider/aws/aws.go | 118 +++++++++++++---------- pkg/cloudprovider/aws/aws_test.go | 152 +++++++++++++++++++++--------- 2 files changed, 177 insertions(+), 93 deletions(-) diff --git a/pkg/cloudprovider/aws/aws.go b/pkg/cloudprovider/aws/aws.go index f2b4c0260b..4d7b2d03e1 100644 --- a/pkg/cloudprovider/aws/aws.go +++ b/pkg/cloudprovider/aws/aws.go @@ -31,6 +31,7 @@ import ( "code.google.com/p/gcfg" "github.com/awslabs/aws-sdk-go/aws" + "github.com/awslabs/aws-sdk-go/aws/credentials" "github.com/awslabs/aws-sdk-go/service/ec2" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" @@ -46,16 +47,16 @@ type EC2 interface { Instances(instanceIds []string, filter *ec2InstanceFilter) (instances []*ec2.Instance, err error) // Attach a volume to an instance - AttachVolume(volumeID string, instanceId string, mountDevice string) (resp *ec2.AttachVolumeResp, err error) + AttachVolume(volumeID, instanceId, mountDevice string) (resp *ec2.VolumeAttachment, err error) // Detach a volume from whatever instance it is attached to // TODO: We should specify the InstanceID and the Device, for safety - DetachVolume(volumeID string) (resp *ec2.SimpleResp, err error) + DetachVolume(volumeID, instanceId, mountDevice string) (resp *ec2.VolumeAttachment, err error) // Lists volumes - Volumes(volumeIDs []string, filter *ec2.Filter) (resp *ec2.VolumesResp, err error) + Volumes(volumeIDs []string, filter *ec2.Filter) (resp *ec2.DescribeVolumesOutput, err error) // Create an EBS volume - CreateVolume(request *ec2.CreateVolume) (resp *ec2.CreateVolumeResp, err error) + CreateVolume(request *ec2.CreateVolumeInput) (resp *ec2.Volume, err error) // Delete an EBS volume - DeleteVolume(volumeID string) (resp *ec2.SimpleResp, err error) + DeleteVolume(volumeID string) (resp *ec2.DeleteVolumeOutput, err error) } // Abstraction over the AWS metadata service @@ -123,6 +124,14 @@ type awsSdkEC2 struct { ec2 *ec2.EC2 } +func stringPointerArray(orig []string) []*string { + n := make([]*string, len(orig)) + for i, s := range orig { + n[i] = &s + } + return n +} + // Implementation of EC2.Instances func (self *awsSdkEC2) Instances(instanceIds []string, filter *ec2InstanceFilter) (resp []*ec2.Instance, err error) { var filters []*ec2.Filter @@ -142,7 +151,7 @@ func (self *awsSdkEC2) Instances(instanceIds []string, filter *ec2InstanceFilter for { res, err := self.ec2.DescribeInstances(&ec2.DescribeInstancesInput{ - InstanceIDs: instanceIds, + InstanceIDs: stringPointerArray(instanceIds), Filters: filters, NextToken: &nextToken, }) @@ -175,7 +184,7 @@ var metadataClient = http.Client{ // Implements AWSMetadata.GetMetaData func (self *awsSdkMetadata) GetMetaData(key string) ([]byte, error) { // TODO Get an implementation of this merged into aws-sdk-go - url := "http://169.254.169.254/latest/meta-data" + path + url := "http://169.254.169.254/latest/meta-data/" + key res, err := metadataClient.Get(url) if err != nil { @@ -196,26 +205,41 @@ func (self *awsSdkMetadata) GetMetaData(key string) ([]byte, error) { return []byte(body), nil } -type AuthFunc func() (creds aws.CredentialsProvider, err error) +type AuthFunc func() (creds *credentials.Credentials) -func (s *awsSdkEC2) AttachVolume(volumeID string, instanceId string, device string) (resp *ec2.AttachVolumeResp, err error) { - return s.ec2.AttachVolume(volumeID, instanceId, device) +func (s *awsSdkEC2) AttachVolume(volumeID, instanceId, device string) (resp *ec2.VolumeAttachment, err error) { + + request := ec2.AttachVolumeInput{ + Device: &device, + InstanceID: &instanceId, + VolumeID: &volumeID, + } + return s.ec2.AttachVolume(&request) } -func (s *awsSdkEC2) DetachVolume(volumeID string) (resp *ec2.SimpleResp, err error) { - return s.ec2.DetachVolume(volumeID) +func (s *awsSdkEC2) DetachVolume(volumeID, instanceId, device string) (resp *ec2.VolumeAttachment, err error) { + request := ec2.DetachVolumeInput{ + Device: &device, + InstanceID: &instanceId, + VolumeID: &volumeID, + } + return s.ec2.DetachVolume(&request) } -func (s *awsSdkEC2) Volumes(volumeIDs []string, filter *ec2.Filter) (resp *ec2.VolumesResp, err error) { - return s.ec2.Volumes(volumeIDs, filter) +func (s *awsSdkEC2) Volumes(volumeIDs []string, filter *ec2.Filter) (resp *ec2.DescribeVolumesOutput, err error) { + request := ec2.DescribeVolumesInput{ + VolumeIDs: stringPointerArray(volumeIDs), + } + return s.ec2.DescribeVolumes(&request) } -func (s *awsSdkEC2) CreateVolume(request *ec2.CreateVolume) (resp *ec2.CreateVolumeResp, err error) { +func (s *awsSdkEC2) CreateVolume(request *ec2.CreateVolumeInput) (resp *ec2.Volume, err error) { return s.ec2.CreateVolume(request) } -func (s *awsSdkEC2) DeleteVolume(volumeID string) (resp *ec2.SimpleResp, err error) { - return s.ec2.DeleteVolume(volumeID) +func (s *awsSdkEC2) DeleteVolume(volumeID string) (resp *ec2.DeleteVolumeOutput, err error) { + request := ec2.DeleteVolumeInput{VolumeID: &volumeID} + return s.ec2.DeleteVolume(&request) } func init() { @@ -225,8 +249,8 @@ func init() { }) } -func getAuth() (creds aws.CredentialsProvider) { - return aws.DetectCreds("", "", "") +func getAuth() (creds *credentials.Credentials) { + return credentials.NewStaticCredentials("", "", "") } // readAWSCloudConfig reads an instance of AWSCloudConfig from config reader. @@ -339,11 +363,11 @@ func (aws *AWSCloud) NodeAddresses(name string) ([]api.NodeAddress, error) { addresses := []api.NodeAddress{} - if *instance.PrivateIpAddress != "" { - ipAddress := *instance.PrivateIpAddress + if *instance.PrivateIPAddress != "" { + ipAddress := *instance.PrivateIPAddress ip := net.ParseIP(ipAddress) if ip == nil { - return nil, fmt.Errorf("EC2 instance had invalid private address: %s (%s)", *instance.InstanceId, ipAddress) + return nil, fmt.Errorf("EC2 instance had invalid private address: %s (%s)", *instance.InstanceID, ipAddress) } addresses = append(addresses, api.NodeAddress{Type: api.NodeInternalIP, Address: ip.String()}) @@ -352,11 +376,11 @@ func (aws *AWSCloud) NodeAddresses(name string) ([]api.NodeAddress, error) { } // TODO: Other IP addresses (multiple ips)? - if *instance.PublicIpAddress != "" { - ipAddress := *instance.PublicIpAddress + if *instance.PublicIPAddress != "" { + ipAddress := *instance.PublicIPAddress ip := net.ParseIP(ipAddress) if ip == nil { - return nil, fmt.Errorf("EC2 instance had invalid public address: %s (%s)", *instance.InstanceId, ipAddress) + return nil, fmt.Errorf("EC2 instance had invalid public address: %s (%s)", *instance.InstanceID, ipAddress) } addresses = append(addresses, api.NodeAddress{Type: api.NodeExternalIP, Address: ip.String()}) } @@ -370,7 +394,7 @@ func (aws *AWSCloud) ExternalID(name string) (string, error) { if err != nil { return "", err } - return inst.InstanceId, nil + return *inst.InstanceID, nil } // Return the instances matching the relevant private dns name. @@ -411,7 +435,8 @@ func (aws *AWSCloud) getInstancesByDnsName(name string) (*ec2.Instance, error) { // Check if the instance is alive (running or pending) // We typically ignore instances that are not alive func isAlive(instance *ec2.Instance) bool { - switch *instance.State.Name { + state := *instance.State + switch *state.Name { case "shutting-down", "terminated", "stopping", "stopped": return false case "pending", "running": @@ -693,23 +718,17 @@ func (self *awsInstance) getInstanceType() *awsInstanceType { // Gets the full information about this instance from the EC2 API func (self *awsInstance) getInfo() (*ec2.Instance, error) { - resp, err := self.ec2.Instances([]string{self.awsID}, nil) + instances, err := self.ec2.Instances([]string{self.awsID}, nil) if err != nil { return nil, fmt.Errorf("error querying ec2 for instance info: %v", err) } - if len(resp.Reservations) == 0 { - return nil, fmt.Errorf("no reservations found for instance: %s", self.awsID) - } - if len(resp.Reservations) > 1 { - return nil, fmt.Errorf("multiple reservations found for instance: %s", self.awsID) - } - if len(resp.Reservations[0].Instances) == 0 { + if len(instances) == 0 { return nil, fmt.Errorf("no instances found for instance: %s", self.awsID) } - if len(resp.Reservations[0].Instances) > 1 { + if len(instances) > 1 { return nil, fmt.Errorf("multiple instances found for instance: %s", self.awsID) } - return &resp.Reservations[0].Instances[0], nil + return instances[0], nil } // Assigns an unused mount device for the specified volume. @@ -733,8 +752,8 @@ func (self *awsInstance) assignMountDevice(volumeID string) (mountDevice string, return "", false, err } deviceMappings := map[string]string{} - for _, blockDevice := range info.BlockDevices { - deviceMappings[blockDevice.DeviceName] = blockDevice.VolumeId + for _, blockDevice := range info.BlockDeviceMappings { + deviceMappings[*blockDevice.DeviceName] = *blockDevice.EBS.VolumeID } self.deviceMappings = deviceMappings } @@ -840,7 +859,7 @@ func (self *awsDisk) getInfo() (*ec2.Volume, error) { if len(resp.Volumes) > 1 { return nil, fmt.Errorf("multiple volumes found for volume: %s", self.awsID) } - return &resp.Volumes[0], nil + return resp.Volumes[0], nil } func (self *awsDisk) waitForAttachmentStatus(status string) error { @@ -861,7 +880,7 @@ func (self *awsDisk) waitForAttachmentStatus(status string) error { if attachmentStatus != "" { glog.Warning("Found multiple attachments: ", info) } - attachmentStatus = attachment.Status + attachmentStatus = *attachment.State } if attachmentStatus == "" { attachmentStatus = "detached" @@ -931,7 +950,7 @@ func (aws *AWSCloud) AttachDisk(instanceName string, diskName string, readOnly b return "", fmt.Errorf("Error finding instance: %v", err) } - awsInstance = newAWSInstance(aws.ec2, instance.InstanceId) + awsInstance = newAWSInstance(aws.ec2, *instance.InstanceID) } if readOnly { @@ -985,7 +1004,7 @@ func (aws *AWSCloud) DetachDisk(instanceName string, diskName string) error { } // TODO: We should specify the InstanceID and the Device, for safety - response, err := aws.ec2.DetachVolume(disk.awsID) + response, err := aws.ec2.DetachVolume(disk.awsID, instanceName, diskName) if err != nil { return fmt.Errorf("error detaching EBS volume: %v", err) } @@ -1002,18 +1021,19 @@ func (aws *AWSCloud) DetachDisk(instanceName string, diskName string) error { // Implements Volumes.CreateVolume func (aws *AWSCloud) CreateVolume(volumeOptions *VolumeOptions) (string, error) { - request := &ec2.CreateVolume{} - request.AvailZone = aws.availabilityZone - request.Size = (int64(volumeOptions.CapacityMB) + 1023) / 1024 + request := &ec2.CreateVolumeInput{} + request.AvailabilityZone = &aws.availabilityZone + volSize := (int64(volumeOptions.CapacityMB) + 1023) / 1024 + request.Size = &volSize response, err := aws.ec2.CreateVolume(request) if err != nil { return "", err } - az := response.AvailZone - awsID := response.VolumeId + az := response.AvailabilityZone + awsID := response.VolumeID - volumeName := "aws://" + az + "/" + awsID + volumeName := "aws://" + *az + "/" + *awsID return volumeName, nil } diff --git a/pkg/cloudprovider/aws/aws_test.go b/pkg/cloudprovider/aws/aws_test.go index 05732a09e6..3621d4613f 100644 --- a/pkg/cloudprovider/aws/aws_test.go +++ b/pkg/cloudprovider/aws/aws_test.go @@ -23,6 +23,7 @@ import ( "testing" "github.com/awslabs/aws-sdk-go/aws" + "github.com/awslabs/aws-sdk-go/aws/credentials" "github.com/awslabs/aws-sdk-go/service/ec2" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" @@ -96,8 +97,8 @@ func TestReadAWSCloudConfig(t *testing.T) { } func TestNewAWSCloud(t *testing.T) { - fakeAuthFunc := func() (creds aws.CredentialProvider, err error) { - return aws.DetectCreds("", "", "") + fakeAuthFunc := func() (creds *credentials.Credentials) { + return credentials.NewStaticCredentials("", "", "") } tests := []struct { @@ -173,13 +174,13 @@ func contains(haystack []string, needle string) bool { return false } -func (self *FakeEC2) Instances(instanceIds []string, filter *ec2InstanceFilter) (resp *ec2.InstancesResp, err error) { +func (self *FakeEC2) Instances(instanceIds []string, filter *ec2InstanceFilter) (instances []*ec2.Instance, err error) { matches := []*ec2.Instance{} for _, instance := range self.instances { if filter != nil && !filter.Matches(instance) { continue } - if instanceIds != nil && !contains(instanceIds, instance.InstanceId) { + if instanceIds != nil && !contains(instanceIds, *instance.InstanceID) { continue } matches = append(matches, instance) @@ -203,23 +204,23 @@ func (self *FakeMetadata) GetMetaData(key string) ([]byte, error) { } } -func (ec2 *FakeEC2) AttachVolume(volumeID string, instanceId string, mountDevice string) (resp *ec2.AttachVolumeResp, err error) { +func (ec2 *FakeEC2) AttachVolume(volumeID, instanceId, mountDevice string) (resp *ec2.VolumeAttachment, err error) { panic("Not implemented") } -func (ec2 *FakeEC2) DetachVolume(volumeID string) (resp *ec2.SimpleResp, err error) { +func (ec2 *FakeEC2) DetachVolume(volumeID, instanceId, mountDevice string) (resp *ec2.VolumeAttachment, err error) { panic("Not implemented") } -func (ec2 *FakeEC2) Volumes(volumeIDs []string, filter *ec2.Filter) (resp *ec2.VolumesResp, err error) { +func (ec2 *FakeEC2) Volumes(volumeIDs []string, filter *ec2.Filter) (resp *ec2.DescribeVolumesOutput, err error) { panic("Not implemented") } -func (ec2 *FakeEC2) CreateVolume(request *ec2.CreateVolume) (resp *ec2.CreateVolumeResp, err error) { +func (ec2 *FakeEC2) CreateVolume(request *ec2.CreateVolumeInput) (resp *ec2.Volume, err error) { panic("Not implemented") } -func (ec2 *FakeEC2) DeleteVolume(volumeID string) (resp *ec2.SimpleResp, err error) { +func (ec2 *FakeEC2) DeleteVolume(volumeID string) (resp *ec2.DeleteVolumeOutput, err error) { panic("Not implemented") } @@ -243,32 +244,61 @@ func mockAvailabilityZone(region string, availabilityZone string) *AWSCloud { } func TestList(t *testing.T) { - instances := make([]*ec2.Instance, 4) - instances[0].Tags = []ec2.Tag{{ + // TODO this setup is not very clean and could probably be improved + var instance0 ec2.Instance + var instance1 ec2.Instance + var instance2 ec2.Instance + var instance3 ec2.Instance + + //0 + tag0 := ec2.Tag{ Key: aws.String("Name"), Value: aws.String("foo"), - }} - instances[0].PrivateDNSName = aws.String("instance1") - instances[0].State.Name = aws.String("running") - instances[1].Tags = []ec2.Tag{{ + } + instance0.Tags = []*ec2.Tag{&tag0} + instance0.PrivateDNSName = aws.String("instance1") + state0 := ec2.InstanceState{ + Name: aws.String("running"), + } + instance0.State = &state0 + + //1 + tag1 := ec2.Tag{ Key: aws.String("Name"), Value: aws.String("bar"), - }} - instances[1].PrivateDNSName = aws.String("instance2") - instances[1].State.Name = aws.String("running") - instances[2].Tags = []ec2.Tag{{ + } + instance1.Tags = []*ec2.Tag{&tag1} + instance1.PrivateDNSName = aws.String("instance2") + state1 := ec2.InstanceState{ + Name: aws.String("running"), + } + instance1.State = &state1 + + //2 + tag2 := ec2.Tag{ Key: aws.String("Name"), Value: aws.String("baz"), - }} - instances[2].PrivateDNSName = aws.String("instance3") - instances[2].State.Name = aws.String("running") - instances[3].Tags = []ec2.Tag{{ + } + instance2.Tags = []*ec2.Tag{&tag2} + instance2.PrivateDNSName = aws.String("instance3") + state2 := ec2.InstanceState{ + Name: aws.String("running"), + } + instance2.State = &state2 + + //3 + tag3 := ec2.Tag{ Key: aws.String("Name"), Value: aws.String("quux"), - }} - instances[3].PrivateDNSName = aws.String("instance4") - instances[3].State.Name = aws.String("running") + } + instance3.Tags = []*ec2.Tag{&tag3} + instance3.PrivateDNSName = aws.String("instance4") + state3 := ec2.InstanceState{ + Name: aws.String("running"), + } + instance3.State = &state3 + instances := []*ec2.Instance{&instance0, &instance1, &instance2, &instance3} aws := mockInstancesResp(instances) table := []struct { @@ -303,14 +333,29 @@ func testHasNodeAddress(t *testing.T, addrs []api.NodeAddress, addressType api.N func TestNodeAddresses(t *testing.T) { // Note these instances have the same name // (we test that this produces an error) - instances := make([]*ec2.Instance, 2) - instances[0].PrivateDNSName = aws.String("instance1") - instances[0].PrivateIpAddress = aws.String("192.168.0.1") - instances[0].PublicIpAddress = aws.String("1.2.3.4") - instances[0].State.Name = aws.String("running") - instances[1].PrivateDNSName = aws.String("instance1") - instances[1].PrivateIpAddress = aws.String("192.168.0.2") - instances[1].State.Name = aws.String("running") + var instance0 ec2.Instance + var instance1 ec2.Instance + + //0 + instance0.PrivateDNSName = aws.String("instance1") + instance0.PrivateIPAddress = aws.String("192.168.0.1") + instance0.PublicIPAddress = aws.String("1.2.3.4") + instance0.InstanceType = aws.String("c3.large") + state0 := ec2.InstanceState{ + Name: aws.String("running"), + } + instance0.State = &state0 + + //1 + instance1.PrivateDNSName = aws.String("instance2") + instance1.PrivateIPAddress = aws.String("192.168.0.2") + instance1.InstanceType = aws.String("c3.large") + state1 := ec2.InstanceState{ + Name: aws.String("running"), + } + instance1.State = &state1 + + instances := []*ec2.Instance{&instance0, &instance1} aws1 := mockInstancesResp([]*ec2.Instance{}) _, err1 := aws1.NodeAddresses("instance") @@ -356,16 +401,35 @@ func TestGetRegion(t *testing.T) { } func TestGetResources(t *testing.T) { - instances := make([]*ec2.Instance, 3) - instances[0].PrivateDNSName = aws.String("m3.medium") - instances[0].InstanceType = aws.String("m3.medium") - instances[0].State.Name = aws.String("running") - instances[1].PrivateDNSName = aws.String("r3.8xlarge") - instances[1].InstanceType = aws.String("r3.8xlarge") - instances[1].State.Name = aws.String("running") - instances[2].PrivateDNSName = aws.String("unknown.type") - instances[2].InstanceType = aws.String("unknown.type") - instances[2].State.Name = aws.String("running") + var instance0 ec2.Instance + var instance1 ec2.Instance + var instance2 ec2.Instance + + //0 + instance0.PrivateDNSName = aws.String("m3.medium") + instance0.InstanceType = aws.String("m3.medium") + state0 := ec2.InstanceState{ + Name: aws.String("running"), + } + instance0.State = &state0 + + //1 + instance1.PrivateDNSName = aws.String("r3.8xlarge") + instance1.InstanceType = aws.String("r3.8xlarge") + state1 := ec2.InstanceState{ + Name: aws.String("running"), + } + instance1.State = &state1 + + //2 + instance2.PrivateDNSName = aws.String("unknown.type") + instance2.InstanceType = aws.String("unknown.type") + state2 := ec2.InstanceState{ + Name: aws.String("running"), + } + instance2.State = &state2 + + instances := []*ec2.Instance{&instance0, &instance1, &instance2} aws1 := mockInstancesResp(instances)