diff --git a/pkg/cloud/aws/actuators/machine/actuator.go b/pkg/cloud/aws/actuators/machine/actuator.go index 986a31c78a..9c5673abcf 100644 --- a/pkg/cloud/aws/actuators/machine/actuator.go +++ b/pkg/cloud/aws/actuators/machine/actuator.go @@ -301,20 +301,21 @@ func (a *Actuator) Update(cluster *clusterv1.Cluster, machine *clusterv1.Machine glog.Errorf("attempted to update machine but no instances found") return fmt.Errorf("attempted to update machine but no instances found") } - newestInstance, terminateInstances := SortInstances(instances) - // In very unusual circumstances, there could be more than one machine running matching this - // machine name and cluster ID. In this scenario we will keep the newest, and delete all others. glog.Info("instance found") + // In very unusual circumstances, there could be more than one machine running matching this + // machine name and cluster ID. In this scenario we will keep the newest, and delete all others. + sortInstances(instances) if len(instances) > 1 { - err = TerminateInstances(client, terminateInstances) + err = TerminateInstances(client, instances[1:]) if err != nil { return err } - } + newestInstance := instances[0] + err = a.UpdateLoadBalancers(client, machineProviderConfig, newestInstance) if err != nil { glog.Errorf("error updating load balancers: %v", err) diff --git a/pkg/cloud/aws/actuators/machine/instances.go b/pkg/cloud/aws/actuators/machine/instances.go index c9d63850b0..cd0d92c526 100644 --- a/pkg/cloud/aws/actuators/machine/instances.go +++ b/pkg/cloud/aws/actuators/machine/instances.go @@ -3,6 +3,7 @@ package machine import ( "encoding/base64" "fmt" + "sort" "time" "github.com/golang/glog" @@ -262,3 +263,34 @@ func launchInstance(machine *clusterv1.Machine, machineProviderConfig *providerc return runResult.Instances[0], nil } + +type instanceList []*ec2.Instance + +func (il instanceList) Len() int { + return len(il) +} + +func (il instanceList) Swap(i, j int) { + il[i], il[j] = il[j], il[i] +} + +func (il instanceList) Less(i, j int) bool { + if il[i].LaunchTime == nil && il[j].LaunchTime == nil { + return false + } + if il[i].LaunchTime != nil && il[j].LaunchTime == nil { + return false + } + if il[i].LaunchTime == nil && il[j].LaunchTime != nil { + return true + } + return (*il[i].LaunchTime).After(*il[j].LaunchTime) +} + +// sortInstances will sort a list of instance based on an instace launch time +// from the newest to the oldest. +// This function should only be called with running instances, not those which are stopped or +// terminated. +func sortInstances(instances []*ec2.Instance) { + sort.Sort(instanceList(instances)) +} diff --git a/pkg/cloud/aws/actuators/machine/utils.go b/pkg/cloud/aws/actuators/machine/utils.go index ea47e466a9..7d6a00e0d2 100644 --- a/pkg/cloud/aws/actuators/machine/utils.go +++ b/pkg/cloud/aws/actuators/machine/utils.go @@ -35,50 +35,6 @@ import ( "github.com/ghodss/yaml" ) -// SortInstances will examine the given slice of instances and return the current active instance for -// the machine, as well as a slice of all other instances which the caller may want to terminate. The -// active instance is calculated as the most recently launched instance. -// This function should only be called with running instances, not those which are stopped or -// terminated. -func SortInstances(instances []*ec2.Instance) (*ec2.Instance, []*ec2.Instance) { - if len(instances) == 0 { - return nil, []*ec2.Instance{} - } - var newestInstance *ec2.Instance - inactiveInstances := make([]*ec2.Instance, 0, len(instances)-1) - for _, i := range instances { - if newestInstance == nil { - newestInstance = i - continue - } - tempInstance := chooseNewest(newestInstance, i) - if *tempInstance.InstanceId != *newestInstance.InstanceId { - inactiveInstances = append(inactiveInstances, newestInstance) - } else { - inactiveInstances = append(inactiveInstances, i) - } - newestInstance = tempInstance - } - return newestInstance, inactiveInstances -} - -func chooseNewest(instance1, instance2 *ec2.Instance) *ec2.Instance { - if instance1.LaunchTime == nil && instance2.LaunchTime == nil { - // No idea what to do here, should not be possible, just return the first. - return instance1 - } - if instance1.LaunchTime != nil && instance2.LaunchTime == nil { - return instance1 - } - if instance1.LaunchTime == nil && instance2.LaunchTime != nil { - return instance2 - } - if (*instance1.LaunchTime).After(*instance2.LaunchTime) { - return instance1 - } - return instance2 -} - // GetRunningInstance returns the AWS instance for a given machine. If multiple instances match our machine, // the most recently launched will be returned. If no instance exists, an error will be returned. func GetRunningInstance(machine *clusterv1.Machine, client awsclient.Client) (*ec2.Instance, error) { @@ -90,8 +46,8 @@ func GetRunningInstance(machine *clusterv1.Machine, client awsclient.Client) (*e return nil, fmt.Errorf("no instance found for machine: %s", machine.Name) } - instance, _ := SortInstances(instances) - return instance, nil + sortInstances(instances) + return instances[0], nil } // GetRunningInstances returns all running instances that have a tag matching our machine name,