Merge lp:~s-matyukevich/charms/trusty/elasticsearch/elasticsearch-dns-bug-fix into lp:~charmers/charms/trusty/elasticsearch/trunk

[Sergey Matyukevich]

Fix bug with elasticsearch charm install on amazon

On amazon when executing realtion-get in private-address parameter we will get DNS name instead of a ip address. This don't work with current implementation of elasticserch charm, because it reques IP address to open port correctly. I add private-ip-address parameter and use it instead of a private-address.

https://codereview.appspot.com/163870043/

[Sergey Matyukevich (s-matyukevich)]

Reviewers: mp+239547_code.launchpad.net,

Message:
Please take a look.

Description:
Fix bug with elasticsearch charm install on amazon

On amazon when executing realtion-get in private-address parameter we
will get DNS name instead of a ip address. This don't work with current
implementation of elasticserch charm, because it reques IP address to
open port correctly. I add private-ip-address parameter and use it
instead of a private-address.

https://code.launchpad.net/~s-matyukevich/charms/trusty/elasticsearch/elasticsearch-dns-bug-fix/+merge/239547

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/163870043/

Affected files (+7, -2 lines):
   A [revision details]
   M hooks/charmhelpers/core/hookenv.py
   M tasks/setup-ufw.yml

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: tasks/setup-ufw.yml
=== modified file 'tasks/setup-ufw.yml'
--- tasks/setup-ufw.yml 2014-07-30 06:35:59 +0000
+++ tasks/setup-ufw.yml 2014-10-24 11:18:33 +0000
@@ -21,7 +21,7 @@
    ufw: state=enabled policy=allow logging=on

  - name: Open the firewall for all clients
- ufw: rule=allow src={{ item.value['private-address'] }} port=9200
proto=tcp
+ ufw: rule=allow src={{ item.value['private-ip-address'] }} port=9200
proto=tcp
    with_dict: relations["client"]["{{ client_relation_id }}"] | default({})
    when: not item.key.startswith(service_name)

Index: hooks/charmhelpers/core/hookenv.py
=== modified file 'hooks/charmhelpers/core/hookenv.py'
--- hooks/charmhelpers/core/hookenv.py 2014-02-06 12:54:59 +0000
+++ hooks/charmhelpers/core/hookenv.py 2014-10-24 11:18:33 +0000
@@ -9,6 +9,7 @@
  import yaml
  import subprocess
  import sys
+inport socket
  import UserDict
  from subprocess import CalledProcessError

@@ -179,7 +180,9 @@
      if unit:
          _args.append(unit)
      try:
- return json.loads(subprocess.check_output(_args))
+ res = json.loads(subprocess.check_output(_args))
+ res['private-ip-address'] =
socket.gethostbyname(res['private-address'])
+ return res
      except ValueError:
          return None
      except CalledProcessError, e:

[lp:~s-matyukevich/charms/trusty/elasticsearch/elasticsearch-dns-bug-fix]

37. By Sergey Matyukevich on 2014-10-24

missprint in import fixed

[Sergey Matyukevich (s-matyukevich)]

Please take a look.

https://codereview.appspot.com/163870043/

[Michael Nelson (michael.nelson)]

Hi Sergey, thanks for finding the issue and a work-around. I've only used the charm with openstack/local myself.

I've just run this locally, and confirm it works as expected.

Two points though:
 1) Although we can modify charmhelpers/core/hookenv.py here, we really need to do this upstream in the charm helpers project - iff there isn't already a way to get the IP. I'd be surprised if we don't have access to the IP in the charmhelpers config on ec2 without socket. If charmhelpers does already have the IP in its config, we'd be best adding it to the context provided to ansible-based charms in hooks/charmhelpers/contrib/templating/contexts.py. And if we were doing that, updating to a newer version of charmhelpers would be good too.
 2) I think although you've fixed the issue for the client-relation-joined, we'll hit the same issue when peers join? (ie. in templates/elasticsearch.yaml is using private-address to list the peers).

Unless you're really keen to have a go at the above, I'll take a look (next week). Let me know what you prefer.

Thanks Sergey!

[Review Queue (review-queue)]

This items has failed automated testing! Results available here http://reports.vapour.ws/charm-tests/charm-bundle-test-1291-results

review: Needs Fixing (automated testing)

[Charles Butler (lazypower)]

Michael,

We had issues with the big data stack using socket to resolve ip's. This had implicit failures on the OrangeBox flavor of deployments.

I dont know that python socket is the way to go here either. I'm currently conferring with Marco about possible alternatives here.

[Charles Butler (lazypower)]

Verified this works on EC2, and HPCloud. Still pending verification this doesn't break OB Deployments - will follow up once I've gotten validation this doesn't break our demo targets.

[Michael Nelson (michael.nelson)]

I've done a separate MP which in the end is the same solution (socket), but fits in with ansible (ie. without having to patch charmhelpers). The branch also updates a few other things that needed doing:

https://code.launchpad.net/~michael.nelson/charms/trusty/elasticsearch/use-new-charmhelpers/+merge/239935

It'd be good to mark this MP as obsolete, assuming Sergey agrees and is happy with the above MP.

[Review Queue (review-queue)]

This items has failed automated testing! Results available here http://reports.vapour.ws/charm-tests/charm-bundle-test-1434-results

review: Needs Fixing (automated testing)

[Sergey Matyukevich (s-matyukevich)]

> I've done a separate MP which in the end is the same solution (socket), but
> fits in with ansible (ie. without having to patch charmhelpers). The branch
> also updates a few other things that needed doing:
>
> https://code.launchpad.net/~michael.nelson/charms/trusty/elasticsearch/use-
> new-charmhelpers/+merge/239935
>
> It'd be good to mark this MP as obsolete, assuming Sergey agrees and is happy
> with the above MP.

I don't find how I can mark my merge proposal as obsolete, but I am totally happy with your solution.

[Jorge Castro (jorge)]

Ok I've set this to rejected, thanks anyway for the contribution Sergey!

Unmerged revisions

37. By Sergey Matyukevich on 2014-10-24

missprint in import fixed

36. By Sergey Matyukevich on 2014-10-24

Bug with elasticsearch install on amazon fixed. Additional parametr private-ip-address, that contain ip addres of a relation, instead of a host name added

=== modified file 'hooks/charmhelpers/core/hookenv.py'
--- hooks/charmhelpers/core/hookenv.py	2014-02-06 12:54:59 +0000
+++ hooks/charmhelpers/core/hookenv.py	2014-10-24 13:02:03 +0000
@@ -9,6 +9,7 @@
 import yaml
 import subprocess
 import sys
+import socket
 import UserDict
 from subprocess import CalledProcessError
 
@@ -179,7 +180,9 @@
     if unit:
         _args.append(unit)
     try:
-        return json.loads(subprocess.check_output(_args))
+        res = json.loads(subprocess.check_output(_args))
+        res['private-ip-address'] = socket.gethostbyname(res['private-address'])
+        return res
     except ValueError:
         return None
     except CalledProcessError, e:

=== modified file 'tasks/setup-ufw.yml'
--- tasks/setup-ufw.yml	2014-07-30 06:35:59 +0000
+++ tasks/setup-ufw.yml	2014-10-24 13:02:03 +0000
@@ -21,7 +21,7 @@
   ufw: state=enabled policy=allow logging=on
 
 - name: Open the firewall for all clients
-  ufw: rule=allow src={{ item.value['private-address'] }} port=9200 proto=tcp
+  ufw: rule=allow src={{ item.value['private-ip-address'] }} port=9200 proto=tcp
   with_dict: relations["client"]["{{ client_relation_id }}"] | default({})
   when: not item.key.startswith(service_name)