Merge lp:~lifeless/juju/bug-945505 into lp:juju

Proposed by Robert Collins on 2012-06-24
Status: Work in progress
Proposed branch: lp:~lifeless/juju/bug-945505
Merge into: lp:juju
Diff against target: 70 lines (+7/-7) 3 files modified
To merge this branch: bzr merge lp:~lifeless/juju/bug-945505
Reviewer Review Type Date Requested Status
Gustavo Niemeyer 2012-06-24 Disapprove on 2012-06-26
Martin Packman Abstain on 2012-06-25
Review via email: mp+111754@code.launchpad.net

Description of the Change

This changes the EC2 Machine provider to supply the public IP rather than the public DNS to the Machine instance. Machines don't have ip and dns, only one contact field, but that is stringifie pretty much everywhere, so putting ip addresses in it works fine.

I think we should probably rename the public and private names to endpoints, or something similar, to avoid confusion, but thats clearly orthogonal to this patch, which merely changes how its populated.

We don't, as far as I can tell, need to do this for internal names, as the inside-openstack dnsmasq instance handles them, its only the public ones, for the client machine, that need to avoid depending on DNS.

To post a comment you must log in.
Martin Packman (gz) wrote :

So, it's a little perverse putting ip_address in an attribute named dns_name but it's what I do with the openstack provider too.

Having this as a flag-day revision rather than continuing with the old behaviour for previous txaws releases I'm a bit wary of, given the lack of a txaws release that actually includes the required change.

Updating juju/machine/__init__.py or at least the comment there would be good.

review: Abstain
Clint Byrum (clint-fewbar) wrote :

Actually txaws 0.2.3 includes ipAddress.

Martin Packman (gz) wrote :

Confusing, is there any record of when 0.2.3 was released or what changes it contains? I'm failing via the normal methods of looking at the release page, tags on the branch, log of when the version was changed... some means other than inspecting the tarball?

Gustavo Niemeyer (niemeyer) wrote :

I'm -1 on changing this as suggested. If a DNS name is available, it should be provided instead of the IP address, and we shouldn't be changing such an API without better reasoning. What's breaking at the moment? Do we have cases where the DNS name does not exist? If so, we could fallback to the IP adddress.

review: Disapprove
Kapil Thangavelu (hazmat) wrote :

this appears to be a common issue with private with private clouds where
the dns given is invalid outside of the cloud network, ie. not pubilcly
accessible dns names. which is problematic even connecting juju client to
the env.

-k

On Tue, Jun 26, 2012 at 1:07 PM, Gustavo Niemeyer <email address hidden>wrote:

> The proposal to merge lp:~lifeless/juju/bug-945505 into lp:juju has been
> updated.
>
> Status: Needs review => Work in progress
>
> For more details, see:
> https://code.launchpad.net/~lifeless/juju/bug-945505/+merge/111754
> --
> https://code.launchpad.net/~lifeless/juju/bug-945505/+merge/111754
> You are subscribed to branch lp:juju.
>

Gustavo Niemeyer (niemeyer) wrote :

I'm instinct is that if the API is reporting a public DNS name that is not public, that isn't an issue with juju. We could argue equally that the public IP may well be private too, and be inaccessible.

I'm happy to understand more about the use case, though. Who can I talk to that is having this issue right now?

Kapil Thangavelu (hazmat) wrote :

the bug reporter, there are a few others who have similiar issues as well..

On Tue, Jun 26, 2012 at 1:24 PM, Gustavo Niemeyer <email address hidden>wrote:

> I'm instinct is that if the API is reporting a public DNS name that is not
> public, that isn't an issue with juju. We could argue equally that the
> public IP may well be private too, and be inaccessible.
>
> I'm happy to understand more about the use case, though. Who can I talk to
> that is having this issue right now?
>
> --
> https://code.launchpad.net/~lifeless/juju/bug-945505/+merge/111754
> You are subscribed to branch lp:juju.
>

Gustavo Niemeyer (niemeyer) wrote :

> the bug reporter, there are a few others who have similiar issues as well..

Okay, so hopefully we'll hear from him.

Robert Collins (lifeless) wrote :

You can talk to me, you can talk to everyone using canonistack. I'd be
delighted to step you through whats going on, including exactly what
private openstack instances hand out and what the challenges are. I
will note that Martin Packman has used IP in the openstack provider
*for exactly this reason*. So if you are -1 on this proposal because
of the way it uses DNS, it would be consistent to be -1 on the
openstack provider for the same reason.

This is whats going on - I thought I detailed it elsewhere already,
but apparently not :)

Openstack hands out DNS entries of the form 'Server-N' (see Ted
Gould's post to canonical-tech, for instance). for *both* its public
and private dns entries.

Thats not resolvable at all, unless you manually expose dnsmasq on
your nova-network controller *and* add that as your primary DNS
server. Which won't work at all when dealing with multi-region
openstack.

It is extensible by writing a custom 'public dns api provider' for
nova, which brokers DNS names, either setting them to be like the ec2
ones (which embed the IP address in them so that amazon can bulk-load
their entire zone at once for a region), or letting you do arbitrary
custom funky stuff like using an LDAP backed DNS server. There is no
sensible-default shipped, or (that I know of) publically available.
Certainly a helper for bind to do amazon's trick would be useful for
folk, but not very useful for people that throw maas at 5 machines
under their desk and bring up openstack on top of that.

The private dns name isn't extensible at all today. And its also just
'server-N' not 'server-N.$customzone'. euca-describe-instances for
canonistack, for instance:
INSTANCE i-00000080 ami-000000bf server-128
server-128 running 0 m1.small
2012-06-27T03:09:18.000Z nova
monitoring-disabled 10.55.63.14 10.55.63.14
instance-store

Note that the only way to tell that this is canonistack2 vs
canonistack1 is the ip address.

This gets worse though (and my patch doesn't help with the next bit -
though its on my list (to let juju ssh $unit work for private
openstack clouds): The machines themselves report hostnames that are
only resolvable within the cloud itself without special configuration:
'server-128.canonistack2' (hostname == server128', but unless you have
specially configured DNS (or for Canonistack users, ~/.ssh.config),
thats not resolvable at all.

Using IP addresses would provide a single point to configure to let
this work, and work with vpns, ssh etc with equal ease. I don't think
the same can be said for dns.

-Rob

Gustavo Niemeyer (niemeyer) wrote :

Thanks Rob.

I'm copy & pasting this to the bug and following up there.

Unmerged revisions

545. By Robert Collins on 2012-06-24

Use the EC2 IP address rather than public DNS name. Openstack clouds do
not have out of the box sane DNS mapping implementations (vs EC2 which
just has the ip address statically mapped in DNS), so the public name
returned by openstack is 'server-N' rather than something sensible like
'10-0-0-3.openstack.domain'. Further complicating matters, folk running
private openstack deploys will often not have local DNS setup at all, and
using the IP address avoids all of that complexity.

This requires txaws rev 134, which is in quantal and will be backported to
precise in an SRU shortly.

Preview Diff

1=== modified file 'juju/providers/ec2/machine.py'
2--- juju/providers/ec2/machine.py 2011-10-11 10:59:04 +0000
3+++ juju/providers/ec2/machine.py 2012-06-24 23:08:21 +0000
4@@ -17,6 +17,6 @@
5 """
6 return EC2ProviderMachine(
7 instance.instance_id,
8- instance.dns_name,
9+ instance.ip_address,
10 instance.private_dns_name,
11 instance.instance_state)
12
13=== modified file 'juju/providers/ec2/tests/test_getmachines.py'
14--- juju/providers/ec2/tests/test_getmachines.py 2011-09-15 18:50:23 +0000
15+++ juju/providers/ec2/tests/test_getmachines.py 2012-06-24 23:08:21 +0000
16@@ -29,7 +29,7 @@
17 """
18 self.ec2.describe_instances()
19 self.mocker.result(succeed([
20- self.get_instance("i-amrunning", dns_name="x1.example.com"),
21+ self.get_instance("i-amrunning", ip_address="10.10.10.10"),
22 self.get_instance("i-amdead", "terminated"),
23 self.get_instance("i-amalien", groups=["other"]),
24 self.get_instance("i-ampending", "pending")]))
25@@ -40,7 +40,7 @@
26
27 def verify(result):
28 (running, pending,) = result
29- self.assert_machine(running, "i-amrunning", "x1.example.com")
30+ self.assert_machine(running, "i-amrunning", "10.10.10.10")
31 self.assert_machine(pending, "i-ampending", "")
32 d.addCallback(verify)
33 return d
34@@ -92,7 +92,7 @@
35 def test_get_some_success(self):
36 self.ec2.describe_instances("i-amrunning", "i-ampending")
37 self.mocker.result(succeed([
38- self.get_instance("i-amrunning", dns_name="x1.example.com"),
39+ self.get_instance("i-amrunning", ip_address="10.10.10.10"),
40 self.get_instance("i-ampending", "pending")]))
41 self.mocker.replay()
42
43@@ -101,7 +101,7 @@
44
45 def verify(result):
46 (running, pending,) = result
47- self.assert_machine(running, "i-amrunning", "x1.example.com")
48+ self.assert_machine(running, "i-amrunning", "10.10.10.10")
49 self.assert_machine(pending, "i-ampending", "")
50 d.addCallback(verify)
51 return d
52
53=== modified file 'juju/providers/ec2/tests/test_machine.py'
54--- juju/providers/ec2/tests/test_machine.py 2011-10-11 14:03:11 +0000
55+++ juju/providers/ec2/tests/test_machine.py 2012-06-24 23:08:21 +0000
56@@ -10,12 +10,12 @@
57 def test_machine_from_instance(self):
58 instance = Instance(
59 "i-foobar", "oscillating",
60- dns_name="public",
61+ ip_address="192.168.254.253",
62 private_dns_name="private")
63
64 machine = machine_from_instance(instance)
65 self.assertTrue(isinstance(machine, EC2ProviderMachine))
66 self.assertEquals(machine.instance_id, "i-foobar")
67- self.assertEquals(machine.dns_name, "public")
68+ self.assertEquals(machine.dns_name, "192.168.254.253")
69 self.assertEquals(machine.private_dns_name, "private")
70 self.assertEquals(machine.state, "oscillating")

Subscribers

People subscribed via source and target branches

to status/vote changes: