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

auroradns: Add support for Health Checks #672

Closed
wants to merge 2 commits into
base: trunk
from

Conversation

Projects
None yet
4 participants
@wido
Contributor

wido commented Jan 5, 2016

AuroraDNS supports Health Checks and based on the state of these
checks records will be served or not.

This way a Round-Robin DNS balancing can be achieved pointing to
only healthy servers/services.

Show outdated Hide outdated libcloud/dns/drivers/auroradns.py
@@ -140,6 +272,14 @@ def list_zones(self):
return zones
def list_records(self, zone):
"""

This comment has been minimized.

@Kami

Kami Jan 5, 2016

Member

There is no need to add docstring for methods which are the same as the ones already defined in the base class - those docstrings are automatically inherited from the parent class methods.

@Kami

Kami Jan 5, 2016

Member

There is no need to add docstring for methods which are the same as the ones already defined in the base class - those docstrings are automatically inherited from the parent class methods.

Show outdated Hide outdated docs/examples/dns/auroradns/health_checks.py
zone = driver.get_zone('auroradns.eu')
health_check = driver.create_healthcheck(zone=zone,

This comment has been minimized.

@Kami

Kami Jan 5, 2016

Member

I assume this should be ex_create_health_check? :)

@Kami

Kami Jan 5, 2016

Member

I assume this should be ex_create_health_check? :)

Show outdated Hide outdated docs/examples/dns/auroradns/enable_disable_record.py
@@ -1,6 +1,5 @@
from libcloud.dns.types import Provider
from libcloud.dns.types import Provider, RecordType

This comment has been minimized.

@Kami

Kami Jan 5, 2016

Member

We tend to use one import per line - this makes searching and grepping a bit easier.

@Kami

Kami Jan 5, 2016

Member

We tend to use one import per line - this makes searching and grepping a bit easier.

@wido

This comment has been minimized.

Show comment
Hide comment
@wido

wido Jan 5, 2016

Contributor

@Kami Thank you for the quick reviews! I just force pushed a new version

Contributor

wido commented Jan 5, 2016

@Kami Thank you for the quick reviews! I just force pushed a new version

:param enabled: (optional) If this healthcheck is enabled to run
:type enabled: ``bool``
:param extra: (optional) Extra attributes (driver specific).

This comment has been minimized.

@jimbobhickville

jimbobhickville Jan 7, 2016

Contributor

extra doesn't seem to be used. Is it needed? Are there other 'health check' drivers that use it and this is just allowing it to be passed in for consistency?

@jimbobhickville

jimbobhickville Jan 7, 2016

Contributor

extra doesn't seem to be used. Is it needed? Are there other 'health check' drivers that use it and this is just allowing it to be passed in for consistency?

This comment has been minimized.

@wido

wido Jan 13, 2016

Contributor

That is correct. It is indeed just there for consistency with the other APIs

A more generic approach for health checks might be nice, I think that Route 53 and Google DNS also support this.

@wido

wido Jan 13, 2016

Contributor

That is correct. It is indeed just there for consistency with the other APIs

A more generic approach for health checks might be nice, I think that Route 53 and Google DNS also support this.

auroradns: Add support for Health Checks
AuroraDNS supports Health Checks and based on the state of these
checks records will be served or not.

This way a Round-Robin DNS balancing can be achieved pointing to
only healthy servers/services.
@wido

This comment has been minimized.

Show comment
Hide comment
@wido

wido Jan 14, 2016

Contributor

I updated the commit with tests for AuroraDNS. They were merged in #679

Tests are passing on my system, waiting for Travis CI :)

Contributor

wido commented Jan 14, 2016

I updated the commit with tests for AuroraDNS. They were merged in #679

Tests are passing on my system, waiting for Travis CI :)

@tonybaloney

This comment has been minimized.

Show comment
Hide comment
@tonybaloney

tonybaloney Jan 15, 2016

Contributor

Thanks @wido

Contributor

tonybaloney commented Jan 15, 2016

Thanks @wido

Show outdated Hide outdated libcloud/dns/drivers/auroradns.py
def __res_to_zone(self, zone):
return Zone(id=zone['id'], domain=zone['name'], type=DEFAULT_ZONE_TYPE,
return Zone(id=zone['id'], domain=zone['name'],
type=DEFAULT_ZONE_TYPE,
ttl=DEFAULT_ZONE_TTL, driver=self,

This comment has been minimized.

@tonybaloney

tonybaloney Jan 15, 2016

Contributor

here too

@tonybaloney

tonybaloney Jan 15, 2016

Contributor

here too

Show outdated Hide outdated libcloud/dns/drivers/auroradns.py
interval=healthcheck['interval'],
port=healthcheck['port'],
enabled=healthcheck['enabled'],
zone=zone, driver=self)

This comment has been minimized.

@tonybaloney

tonybaloney Jan 15, 2016

Contributor

I don't know how this method is used. but you should use self.connection.driver. If there are multiple drivers instantiated weird things would happen with this reference!

@tonybaloney

tonybaloney Jan 15, 2016

Contributor

I don't know how this method is used. but you should use self.connection.driver. If there are multiple drivers instantiated weird things would happen with this reference!

Show outdated Hide outdated libcloud/dns/drivers/auroradns.py
return Record(id=record['id'], name=name,
type=record['type'],
data=record['content'], zone=zone,
driver=self, ttl=record['ttl'],

This comment has been minimized.

@tonybaloney

tonybaloney Jan 15, 2016

Contributor

and here

@tonybaloney

tonybaloney Jan 15, 2016

Contributor

and here

@wido

This comment has been minimized.

Show comment
Hide comment
@wido

wido Jan 15, 2016

Contributor

@tonybaloney Thanks for the feedback! I wasn't aware of that. Fixed that in a new commit.

Contributor

wido commented Jan 15, 2016

@tonybaloney Thanks for the feedback! I wasn't aware of that. Fixed that in a new commit.

@tonybaloney

This comment has been minimized.

Show comment
Hide comment
@tonybaloney

tonybaloney Jan 15, 2016

Contributor

thanks. merging

Contributor

tonybaloney commented Jan 15, 2016

thanks. merging

@asfgit asfgit closed this in 56649ca Jan 15, 2016

asfgit pushed a commit that referenced this pull request Jan 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment