Merge lp:~clint-fewbar/pyjuju/ec2-use-ip-address into lp:pyjuju

Proposed by Clint Byrum
Status: Work in progress
Proposed branch: lp:~clint-fewbar/pyjuju/ec2-use-ip-address
Merge into: lp:pyjuju
Diff against target: 315 lines (+163/-16)
7 files modified
juju/providers/ec2/__init__.py (+2/-1)
juju/providers/ec2/launch.py (+8/-1)
juju/providers/ec2/machine.py (+30/-4)
juju/providers/ec2/tests/common.py (+18/-2)
juju/providers/ec2/tests/test_getmachines.py (+4/-0)
juju/providers/ec2/tests/test_launch.py (+21/-2)
juju/providers/ec2/tests/test_machine.py (+80/-6)
To merge this branch: bzr merge lp:~clint-fewbar/pyjuju/ec2-use-ip-address
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+119021@code.launchpad.net

Description of the change

Use ipAddress as a fallback for dnsName

Use ipAddress as a fallback if dnsName is unresolvable

https://codereview.appspot.com/6441126/

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

This is better than what we have today (per the analysis I provided for use of ips), but it has a significant flaw: many folk have DNS servers (e.g. opendns) that resolve DNS names that don't exist. You'll need a much more complex algorithm to determine 'DNS is broken' than merely fails-to-resolve.

Revision history for this message
Kapil Thangavelu (hazmat) wrote :

I'm confused, i thought we had said this wasn't nesc given the openstack native provider (which uses ip addresses).

Revision history for this message
Clint Byrum (clint-fewbar) wrote :

Its less important for sure. However, it would help for private clouds built on other EC2 implementations like Eucalyptus.

Revision history for this message
Clint Byrum (clint-fewbar) wrote :

I'm less interested in pursuing this approach now that I have tinkered with it. This makes the ec2 provider less robust without good cause. I think a better approach, such as a setting "use-ip" would be more appropriate for those rare private EC2 clouds that won't have resolvable dns. For common private openstack clouds, the native openstack provider makes more sense.

Unmerged revisions

566. By Clint Byrum

skip ip_address tests on txaws that does not support it

565. By Clint Byrum

refactor to do lookup in machine_from_instance

564. By Clint Byrum

add support for older txaws api

563. By Clint Byrum

Use ip address when we cannot lookup the hostname given by the provider

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'juju/providers/ec2/__init__.py'
2--- juju/providers/ec2/__init__.py 2012-07-05 21:49:12 +0000
3+++ juju/providers/ec2/__init__.py 2012-08-09 19:30:25 +0000
4@@ -156,7 +156,8 @@
5 continue
6 if group_name not in instance.reservation.groups:
7 continue
8- machines.append(machine_from_instance(instance))
9+ machine = yield machine_from_instance(instance)
10+ machines.append(machine)
11
12 if instance_ids:
13 # We were asked for a specific list of machines, and if we can't
14
15=== modified file 'juju/providers/ec2/launch.py'
16--- juju/providers/ec2/launch.py 2012-07-05 21:49:12 +0000
17+++ juju/providers/ec2/launch.py 2012-08-09 19:30:25 +0000
18@@ -1,4 +1,7 @@
19 from twisted.internet.defer import inlineCallbacks, returnValue
20+from twisted.names.client import getHostByName
21+from twisted.names.error import (DomainError, AuthoritativeDomainError,
22+ DNSQueryTimeoutError)
23
24 from juju.providers.common.launch import LaunchMachine
25
26@@ -48,7 +51,11 @@
27 availability_zone=availability_zone,
28 user_data=user_data)
29
30- returnValue([machine_from_instance(i) for i in instances])
31+ machines = []
32+ for i in instances:
33+ machine = yield machine_from_instance(i)
34+ machines.append(machine)
35+ returnValue(machines)
36
37 @inlineCallbacks
38 def _ensure_groups(self, machine_id):
39
40=== modified file 'juju/providers/ec2/machine.py'
41--- juju/providers/ec2/machine.py 2011-10-11 10:59:04 +0000
42+++ juju/providers/ec2/machine.py 2012-08-09 19:30:25 +0000
43@@ -1,4 +1,20 @@
44 from juju.machine import ProviderMachine
45+from twisted.internet.defer import inlineCallbacks, returnValue
46+from twisted.names.client import getHostByName
47+from twisted.names.error import DomainError, AuthoritativeDomainError, DNSQueryTimeoutError
48+
49+@inlineCallbacks
50+def _dns_name_ip_fallback(dns_name, ip):
51+ try:
52+ if dns_name:
53+ addr = yield getHostByName(dns_name)
54+ except DomainError:
55+ returnValue(ip)
56+ except AuthoritativeDomainError:
57+ returnValue(ip)
58+ except DNSQueryTimeoutError:
59+ returnValue(ip)
60+ returnValue(dns_name)
61
62
63 class EC2ProviderMachine(ProviderMachine):
64@@ -8,6 +24,7 @@
65 """
66
67
68+@inlineCallbacks
69 def machine_from_instance(instance):
70 """Create an :class:`EC2ProviderMachine` from a txaws :class:`Instance`
71
72@@ -15,8 +32,17 @@
73
74 :return: a matching :class:`EC2ProviderMachine`
75 """
76- return EC2ProviderMachine(
77+ if hasattr(instance, 'ip_address') and instance.ip_address:
78+ dns_name = yield _dns_name_ip_fallback(instance.dns_name, instance.ip_address)
79+ else:
80+ dns_name = instance.dns_name
81+ if hasattr(instance, 'private_ip_address') and instance.private_ip_address:
82+ private_dns_name = yield _dns_name_ip_fallback(instance.private_dns_name, instance.private_ip_address)
83+ else:
84+ private_dns_name = instance.private_dns_name
85+
86+ returnValue(EC2ProviderMachine(
87 instance.instance_id,
88- instance.dns_name,
89- instance.private_dns_name,
90- instance.instance_state)
91+ dns_name,
92+ private_dns_name,
93+ instance.instance_state))
94
95=== modified file 'juju/providers/ec2/tests/common.py'
96--- juju/providers/ec2/tests/common.py 2012-07-05 21:49:12 +0000
97+++ juju/providers/ec2/tests/common.py 2012-08-09 19:30:25 +0000
98@@ -7,8 +7,9 @@
99 from txaws.ec2.client import EC2Client
100 from txaws.ec2.exception import EC2Error
101 from txaws.ec2.model import Instance, Reservation, SecurityGroup
102+from txaws.version import txaws as txaws_version
103
104-from juju.lib.mocker import KWARGS, MATCH
105+from juju.lib.mocker import KWARGS, MATCH, ANY
106 from juju.providers.ec2 import MachineProvider
107 from juju.providers.ec2.machine import EC2ProviderMachine
108
109@@ -52,7 +53,13 @@
110 ["juju-%s" % self.env_name,
111 "juju-%s-%s" % (self.env_name, machine_id)])
112 reservation = Reservation("x", "y", groups=groups)
113- return Instance(instance_id, state, reservation=reservation, **kwargs)
114+ if txaws_version == '0.2':
115+ return Instance(instance_id, state, reservation=reservation,
116+ **kwargs)
117+ else:
118+ return Instance(instance_id, state, reservation=reservation,
119+ ip_address='1.1.1.1', private_ip_address='10.1.1.1',
120+ **kwargs)
121
122 def assert_machine(self, machine, instance_id, dns_name):
123 self.assertTrue(isinstance(machine, EC2ProviderMachine))
124@@ -89,6 +96,15 @@
125 self.mocker.result(self.ec2)
126
127
128+ def _mock_dns_lookups(self):
129+ if txaws_version == '0.2':
130+ return
131+ getHostByName = self.mocker.replace('twisted.names.client.getHostByName')
132+ getHostByName(ANY)
133+ self.mocker.result(succeed('1.1.1.1'))
134+ self.mocker.count(min=0, max=None)
135+
136+
137 class EC2MachineLaunchMixin(object):
138
139 def _mock_launch_utils(self, ami_name="ami-default", get_ami_args=()):
140
141=== modified file 'juju/providers/ec2/tests/test_getmachines.py'
142--- juju/providers/ec2/tests/test_getmachines.py 2011-09-15 18:50:23 +0000
143+++ juju/providers/ec2/tests/test_getmachines.py 2012-08-09 19:30:25 +0000
144@@ -14,6 +14,10 @@
145
146 class GetMachinesTest(EC2TestMixin, TestCase):
147
148+ def setUp(self):
149+ EC2TestMixin.setUp(self)
150+ self._mock_dns_lookups()
151+
152 def assert_not_found(self, d, instance_ids):
153 self.assertFailure(d, MachinesNotFound)
154
155
156=== modified file 'juju/providers/ec2/tests/test_launch.py'
157--- juju/providers/ec2/tests/test_launch.py 2012-07-18 20:04:16 +0000
158+++ juju/providers/ec2/tests/test_launch.py 2012-08-09 19:30:25 +0000
159@@ -3,14 +3,17 @@
160 import yaml
161
162 from twisted.internet.defer import inlineCallbacks, succeed
163+from twisted.names.error import (DomainError, AuthoritativeDomainError,
164+ DNSQueryTimeoutError)
165
166 from txaws.ec2.model import Instance, SecurityGroup
167+from txaws.version import txaws as txaws_version
168
169 from juju.errors import EnvironmentNotFound, ProviderError
170 from juju.providers.ec2.machine import EC2ProviderMachine
171
172 from juju.lib.testing import TestCase
173-from juju.lib.mocker import MATCH
174+from juju.lib.mocker import MATCH, ANY
175
176 from .common import EC2TestMixin, EC2MachineLaunchMixin, get_constraints
177
178@@ -52,6 +55,18 @@
179
180 self.mocker.result(succeed([instance]))
181
182+ self._mock_dns_lookups()
183+
184+
185+ def _mock_dns_lookups(self):
186+ if txaws_version == '0.2':
187+ return
188+ getHostByName = self.mocker.replace('twisted.names.client.getHostByName')
189+ getHostByName(ANY)
190+ self.mocker.result(succeed('1.1.1.1'))
191+ self.mocker.count(min=0, max=None)
192+
193+
194 def test_bad_data(self):
195 self.mocker.replay()
196 d = self.get_provider().start_machine({})
197@@ -159,7 +174,11 @@
198 @inlineCallbacks
199 def test_provider_launch_existing_security_group(self):
200 """Verify that the launch works if the env security group exists"""
201- instance = Instance("i-foobar", "running", dns_name="x1.example.com")
202+ if txaws_version == '0.2':
203+ instance = Instance("i-foobar", "running", dns_name="x1.example.com")
204+ else:
205+ instance = Instance("i-foobar", "running", dns_name="x1.example.com",
206+ ip_address='1.1.1.1',)
207 security_group = SecurityGroup("juju-moon", "some description")
208
209 self.ec2.describe_security_groups()
210
211=== modified file 'juju/providers/ec2/tests/test_machine.py'
212--- juju/providers/ec2/tests/test_machine.py 2011-10-11 14:03:11 +0000
213+++ juju/providers/ec2/tests/test_machine.py 2012-08-09 19:30:25 +0000
214@@ -1,21 +1,95 @@
215 from txaws.ec2.model import Instance
216+from txaws.version import txaws as txaws_version
217+from twisted.internet.defer import inlineCallbacks, succeed
218+from twisted.names.error import DomainError, AuthoritativeDomainError, DNSQueryTimeoutError
219
220 from juju.lib.testing import TestCase
221 from juju.providers.ec2.machine import (
222 EC2ProviderMachine, machine_from_instance)
223
224+def skip_ip_tests():
225+ if txaws_version == '0.2':
226+ return "txaws 0.2 does not support ip_address"
227+
228+
229+def uses_ip_address(f):
230+ f.skip = skip_ip_tests()
231+ return f
232+
233
234 class EC2ProviderMachineTest(TestCase):
235-
236+ def _mock_dns_lookups(self, instance):
237+ if hasattr(instance, 'ip_address'):
238+ getHostByName = self.mocker.replace('twisted.names.client.getHostByName')
239+ if instance.ip_address and instance.dns_name and len(instance.dns_name):
240+ getHostByName(instance.dns_name)
241+ self.mocker.result(succeed('1.1.1.1'))
242+ if instance.private_ip_address and instance.private_dns_name and len(instance.private_dns_name):
243+ getHostByName(instance.private_dns_name)
244+ self.mocker.result(succeed('10.1.1.1'))
245+ self.mocker.replay()
246+
247+
248+ def _mock_failed_dns_lookups(self, instance, error=DomainError):
249+ if hasattr(instance, 'ip_address'):
250+ getHostByName = self.mocker.replace('twisted.names.client.getHostByName')
251+ if instance.ip_address and instance.dns_name and len(instance.dns_name):
252+ getHostByName(instance.dns_name)
253+ self.mocker.throw(error())
254+ if instance.private_ip_address and instance.private_dns_name and len(instance.private_dns_name):
255+ getHostByName(instance.private_dns_name)
256+ self.mocker.throw(error())
257+ self.mocker.replay()
258+
259+
260+ @inlineCallbacks
261 def test_machine_from_instance(self):
262 instance = Instance(
263 "i-foobar", "oscillating",
264 dns_name="public",
265 private_dns_name="private")
266
267- machine = machine_from_instance(instance)
268- self.assertTrue(isinstance(machine, EC2ProviderMachine))
269- self.assertEquals(machine.instance_id, "i-foobar")
270- self.assertEquals(machine.dns_name, "public")
271- self.assertEquals(machine.private_dns_name, "private")
272+ machine = yield machine_from_instance(instance)
273+ self.assertTrue(isinstance(machine, EC2ProviderMachine))
274+ self.assertEquals(machine.instance_id, "i-foobar")
275+ self.assertEquals(machine.dns_name, "public")
276+ self.assertEquals(machine.private_dns_name, "private")
277+ self.assertEquals(machine.state, "oscillating")
278+
279+ @inlineCallbacks
280+ @uses_ip_address
281+ def test_machine_from_instance_with_ip(self):
282+ instance = Instance(
283+ "i-foobar", "oscillating",
284+ dns_name="public",
285+ private_dns_name="private",
286+ ip_address='1.1.1.1',
287+ private_ip_address='10.1.1.1')
288+
289+ self._mock_dns_lookups(instance)
290+
291+ machine = yield machine_from_instance(instance)
292+ self.assertTrue(isinstance(machine, EC2ProviderMachine))
293+ self.assertEquals(machine.instance_id, "i-foobar")
294+ self.assertEquals(machine.dns_name, "public")
295+ self.assertEquals(machine.private_dns_name, "private")
296+ self.assertEquals(machine.state, "oscillating")
297+
298+ @inlineCallbacks
299+ @uses_ip_address
300+ def test_machine_from_instance_with_ip_fail_lookup(self):
301+ instance = Instance(
302+ "i-foobar", "oscillating",
303+ dns_name="public",
304+ private_dns_name="private",
305+ ip_address='1.1.1.1',
306+ private_ip_address='10.1.1.1')
307+
308+ self._mock_failed_dns_lookups(instance)
309+
310+ machine = yield machine_from_instance(instance)
311+ self.assertTrue(isinstance(machine, EC2ProviderMachine))
312+ self.assertEquals(machine.instance_id, "i-foobar")
313+ self.assertEquals(machine.dns_name, "1.1.1.1")
314+ self.assertEquals(machine.private_dns_name, "10.1.1.1")
315 self.assertEquals(machine.state, "oscillating")

Subscribers

People subscribed via source and target branches

to status/vote changes: