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

Adding ELB waiter for healthy instance #716

Merged
merged 3 commits into from
Feb 26, 2015

Conversation

omockler
Copy link
Contributor

Adding a waiter that takes an ELB name and list of instance_ids and returns success when at least one of them reports back as "InService".

@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.12% when pulling 9851def on omockler:elb-healthy-instance-waiter into 496c5fc on aws:master.

@omockler
Copy link
Contributor Author

I was just hoping to wait for one of them to be in the success state, should I change then name? I could also add one for checking if they are all healthy.

@trevorrowe
Copy link
Member

The name is fine. It should match "all" of the requested instances are healthy. You control what all means by omitting the instance if list or providing one or more odd. Does that make sense?

@omockler
Copy link
Contributor Author

Yeah, it makes sense, but it doesn't exactly cover the use case I had in mind.

I'm creating a number of new instances to replace the ones currently attached to an ELB. One instance can handle all of the load coming through by itself, but I am starting additional instances for high availability purposes. I was hoping to start the process of deregistering and terminating the old instances once any number of the new instances become available through the load balancer.

Do you think that this use case could warrant having a waiter for pathAll and pathAny?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 95.12% when pulling ccef218 on omockler:elb-healthy-instance-waiter into 496c5fc on aws:master.

@trevorrowe
Copy link
Member

Maybe my confusion is the intended use of the waiter. Do you intend to wait until any of the instances are "InService" and use this as a condition to indicate the health of the load balancer? If so, maybe its just naming confusion.

My suggestion:

  1. Rename the first waiter from "InstanceHealthly" to "InstanceInService". This makes it very clear what state is being polled for.
  2. Rename the second waiter to "AnyInstanceInService".

Example:

# poll until EVERY instance is in service
elb.wait_until(:instance_in_service)

# polls until both instances are in service
elb.wait_until(:instance_in_service, instances: [{ instance_id: 'id' }, { instance_id: 'id2'}])

# poll until ANY instance is in service
elb.wait_until(:any_instance_in_service)

# polls until any of the two instances are in service
elb.wait_until(:any_instance_in_service, instances: [{ instance_id: 'id' }, { instance_id: 'id2'}])

Does that cover your use cases?

@omockler
Copy link
Contributor Author

Exactly what I was looking for, thanks for the naming suggestion!

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 95.08% when pulling 63e41f9 on omockler:elb-healthy-instance-waiter into 496c5fc on aws:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 95.08% when pulling 63e41f9 on omockler:elb-healthy-instance-waiter into 496c5fc on aws:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 95.11% when pulling 63e41f9 on omockler:elb-healthy-instance-waiter into 496c5fc on aws:master.

trevorrowe added a commit that referenced this pull request Feb 26, 2015
@trevorrowe trevorrowe merged commit 0d054b6 into aws:master Feb 26, 2015
@trevorrowe
Copy link
Member

Thanks for the contribution!

@omockler omockler deleted the elb-healthy-instance-waiter branch February 26, 2015 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants