Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore EC2 instances that are stopped #5064

Merged
merged 1 commit into from
Mar 10, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion cluster/saltbase/salt/kube-controller-manager/default
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
{% if grains.cloud == 'aws' -%}
{% set cloud_provider = "--cloud_provider=aws" -%}
{% set cloud_config = "--cloud_config=/etc/aws.conf" -%}
{% set minion_regexp = "" -%}
{% set machines = "--machines=" + ','.join(salt['mine.get']('roles:kubernetes-pool', 'network.ip_addrs', expr_form='grain').keys()) -%}
{% endif -%}
{% if grains.cloud == 'azure' -%}
Expand Down
49 changes: 41 additions & 8 deletions pkg/cloudprovider/aws/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,20 +187,46 @@ func (aws *AWSCloud) getInstancesByDnsName(name string) (*ec2.Instance, error) {
if err != nil {
return nil, err
}
if len(resp.Reservations) == 0 {
return nil, fmt.Errorf("no reservations found for host: %s", name)
}
if len(resp.Reservations) > 1 {
return nil, fmt.Errorf("multiple reservations found for host: %s", name)

instances := []*ec2.Instance{}
for _, reservation := range resp.Reservations {
for _, instance := range reservation.Instances {
// TODO: Push running logic down into filter?
if !isAlive(&instance) {
continue
}

if instance.PrivateDNSName != name {
// TODO: Should we warn here? - the filter should have caught this
// (this will happen in the tests if they don't fully mock the EC2 API)
continue
}

instances = append(instances, &instance)
}
}
if len(resp.Reservations[0].Instances) == 0 {

if len(instances) == 0 {
return nil, fmt.Errorf("no instances found for host: %s", name)
}
if len(resp.Reservations[0].Instances) > 1 {
if len(instances) > 1 {
return nil, fmt.Errorf("multiple instances found for host: %s", name)
}
return instances[0], nil
}

return &resp.Reservations[0].Instances[0], nil
// 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 {
case "shutting-down", "terminated", "stopping", "stopped":
return false
case "pending", "running":
return true
default:
glog.Errorf("unknown EC2 instance state: %s", instance.State)
return false
}
}

// Return a list of instances matching regex string.
Expand All @@ -226,6 +252,12 @@ func (aws *AWSCloud) getInstancesByRegex(regex string) ([]string, error) {
instances := []string{}
for _, reservation := range resp.Reservations {
for _, instance := range reservation.Instances {
// TODO: Push filtering down into EC2 API filter?
if !isAlive(&instance) {
glog.V(2).Infof("skipping EC2 instance (not alive): %s", instance.InstanceId)
continue
}

for _, tag := range instance.Tags {
if tag.Key == "Name" && re.MatchString(tag.Value) {
instances = append(instances, instance.PrivateDNSName)
Expand All @@ -234,6 +266,7 @@ func (aws *AWSCloud) getInstancesByRegex(regex string) ([]string, error) {
}
}
}
glog.V(2).Infof("Matched EC2 instances: %s", instances)
return instances, nil
}

Expand Down
6 changes: 6 additions & 0 deletions pkg/cloudprovider/aws/aws_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,16 @@ func TestList(t *testing.T) {
instances := make([]ec2.Instance, 4)
instances[0].Tags = []ec2.Tag{{"Name", "foo"}}
instances[0].PrivateDNSName = "instance1"
instances[0].State.Name = "running"
instances[1].Tags = []ec2.Tag{{"Name", "bar"}}
instances[1].PrivateDNSName = "instance2"
instances[1].State.Name = "running"
instances[2].Tags = []ec2.Tag{{"Name", "baz"}}
instances[2].PrivateDNSName = "instance3"
instances[2].State.Name = "running"
instances[3].Tags = []ec2.Tag{{"Name", "quux"}}
instances[3].PrivateDNSName = "instance4"
instances[3].State.Name = "running"

aws := mockInstancesResp(instances)

Expand Down Expand Up @@ -139,8 +143,10 @@ func TestIPAddress(t *testing.T) {
instances := make([]ec2.Instance, 2)
instances[0].PrivateDNSName = "instance1"
instances[0].PrivateIpAddress = "192.168.0.1"
instances[0].State.Name = "running"
instances[1].PrivateDNSName = "instance1"
instances[1].PrivateIpAddress = "192.168.0.2"
instances[1].State.Name = "running"

aws1 := mockInstancesResp([]ec2.Instance{})
_, err1 := aws1.IPAddress("instance")
Expand Down