Charm needed: Elastic Search

Saw this article (http://www.elasticsearch.org/tutorials/2012/03/21/deploying-elasticsearch-with-chef-solo.html) and thought, wow this ought to be much easier with Juju.

I'm working on this charm right now with the intent of using it with logstash (for which I will also attempt to write a charm).

I have an initial implementation that seems to be working correctly locally and on ec2 with the elasticsearch aws plugin. I'd like to submit my branch for review. Should I assign myself this bug ?

Hi Luis, thanks for sharing your charm!

Here's my static review to start with:

things to fix:

- charms that are part of the charmstore need to cryptographically verify downloads. Can you please look to see if there're published hashes of the elasticsearch tarball that can be used?

- note that you really don't want the hostname in the rest-relation-changed hook... you can always get the other side's address using `relation-get private-address`.

- what's the rest relation for? Can you add some example usage into the README? Is the intention to eventually proxy requests up to the elasticsearch head?

recommendations / discussion (not necessarily "need to fix"):

- in your install hook, I don't suggest an general "apt-get upgrade"... I think you're better off with updating only the packages you need to update. Was there a particular reason for the upgrade or was it just in the upstream script?

So this is a great idea... I'm excited about where it can go.

I'll leave it "In Progress" for now.

Thanks!

Hi Mark,

Thanks for your review! I'm excited to try and get the charm in shape for
the charm store. Some answers below.

On Thu, Jun 21, 2012 at 1:00 AM, Mark Mims <email address hidden>wrote:

> Hi Luis, thanks for sharing your charm!
>
> Here's my static review to start with:
>
> things to fix:
>
> - charms that are part of the charmstore need to cryptographically
> verify downloads. Can you please look to see if there're published
> hashes of the elasticsearch tarball that can be used?
>

 I have posted a request for hashes here and there is a first comment
already. Please step in the conversation since I'm not sure exactly what
to recommend here.

https://github.com/elasticsearch/elasticsearch/issues/2050<https://github.com/elasticsearch/elasticsearch/issues/2050#issuecomment-6551195>

> - note that you really don't want the hostname in the rest-relation-
> changed hook... you can always get the other side's address using
> `relation-get private-address`.
>

I updated the charm locally and am testing. I'll push to my branch when
done.

>
> - what's the rest relation for? Can you add some example usage into the
> README? Is the intention to eventually proxy requests up to the
> elasticsearch head?
>
>
The rest relation is for querying elasticsearch API:

http://www.elasticsearch.org/guide/reference/api/

This allows querying for search results but also a bunch of administrative
tasks.

I did not define a relation for the elasticsearch transport protocol on
port 9300 simply because I was learning as I went along, but for instance
this is used by logstash when interfacing directly with elastic search.

http://www.elasticsearch.org/guide/reference/modules/transport.html

Looking at that page again it seems there are a bunch of modules (JMX,
memcached, etc...) which would merit defining a relation, so there may be a
lot more work involved in this charm than I set out to implement for my use
case.

>
> recommendations / discussion (not necessarily "need to fix"):
>
> - in your install hook, I don't suggest an general "apt-get upgrade"...
> I think you're better off with updating only the packages you need to
> update. Was there a particular reason for the upgrade or was it just in
> the upstream script?
>
>
Ok no prob, this is just a habit that leaked from our current deployment
system which is just a wrapper around aws cloud-formation and we generally
want each new instance to be up to date. I see that it's out of the scope
of juju to guarantee that especially for an individual charm. I'll fix
that.

> So this is a great idea... I'm excited about where it can go.
>
> I'll leave it "In Progress" for now.
>
> Thanks!
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/822979
>
> Title:
> Charm needed: Elastic Search
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/charms/+bug/822979/+subscriptions
>

Hi Luis!

IMO, downloading the latest version of elasticsearch in the charm isn't the best plan. Its a big piece of software and you, the charm maintainer, may want to take a look at it before you dump it onto all the current users of the charm.

Perhaps you should just embed the tarball in the charm itself. That will make the charm fairly large in bytes, but it will also make the charm more robust to github downtime or upstream filtering.

The cryptographic hashes are still needed though, because otherwise, how do you verify a download of a file from github? I'm surprised they don't allow uploading of detached signatures the way launchpad does.

Hi!

The problem with that approach is that some pieces of software, namely
logstash (which is my use case) really require a specific version of
elasticsearch because they use the transport module and not the REST api.
 So the charm needs this kind of flexibility even though yes its huge.
 There are probably going to be similar problems with logstash itself which
is another huge jar with a bunch of stuff and I don't see any hashes either
(I haven't asked yet). http://logstash.net/ Anyway it looks like the
elasticsearch maintainers are open to working something out for the hashes.
 Any pointers to guidelines from the ubuntu / debian practices I could
suggest ?

Luis

On Mon, Jun 25, 2012 at 10:27 PM, Clint Byrum <email address hidden> wrote:

> Hi Luis!
>
> IMO, downloading the latest version of elasticsearch in the charm isn't
> the best plan. Its a big piece of software and you, the charm
> maintainer, may want to take a look at it before you dump it onto all
> the current users of the charm.
>
> Perhaps you should just embed the tarball in the charm itself. That will
> make the charm fairly large in bytes, but it will also make the charm
> more robust to github downtime or upstream filtering.
>
> The cryptographic hashes are still needed though, because otherwise, how
> do you verify a download of a file from github? I'm surprised they don't
> allow uploading of detached signatures the way launchpad does.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/822979
>
> Title:
> Charm needed: Elastic Search
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/charms/+bug/822979/+subscriptions
>

If the public API's of elasticsearch (the transport module) change from version to version, that should be different charms, and probably versioned interfaces. Part of 'the interface matching that juju does is built around the idea that you can assume a certain protocol will be available on the other side of the relation.

That said, for your charm, you *could* just add a hash verification after the wget's, and have the hash be specified in the service configs the same way the version is. You do not need the hashes from upstream, you can just calculate the hash yourself with 'sha256sum', but you need to make sure the file is actually the one from upstream and not one with trojaned code in it. ;)

    bin/plugin -install elasticsearch/elasticsearch-cloud-aws/1.4.0 > /dev/null

Its also not clear if that command does verification of the software it downloads. Can you please check it and assert how it does in a comment in the script?

Also, you should do anything that does 'config-get` in config-changed, not install. This makes it a little more difficult as you have to move the current one out of the way and install a new one, but means that users can change the version after deploy, which may be important and will most definitely be the expected way things should work with any config options.

I'm going to mark this back to Incomplete. You should be able to satisfy the hash requirement yourself. The config-changed is just a suggestion, its not required for charm store inclusion.

Hi Luis! I was looking at your excellent charm today and I remembered this bug. Any progress since June? Are you still interested in it? Otherwise I think we might just fix the issues and get this in the store ourselves.

Thanks!

Hi Clint,

Oh please go ahead and build on what is there if that is helpful to you. I
do not have the possibility to work on this at the present time since we
are using our own CloudFormation wrapper for logstash / elasticsearch /
kibana.

Luis

On Fri, Oct 19, 2012 at 6:50 AM, Clint Byrum <email address hidden> wrote:

> Hi Luis! I was looking at your excellent charm today and I remembered
> this bug. Any progress since June? Are you still interested in it?
> Otherwise I think we might just fix the issues and get this in the store
> ourselves.
>
> Thanks!
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/822979
>
> Title:
> Charm needed: Elastic Search
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/charms/+bug/822979/+subscriptions
>

I'm working on a logstash / elastic search / kibana set of charms. I've taken this charm and updated it after reading this discussion. You can find what I did at the below code branch. I hope its suitable.

lp:~paulcz/charms/precise/elasticsearch/trunk

let me know if I should create a new bug to track my branch of this ... or if we can just do it from here.

One for each charm would be preferable, that way we can keep the reviews separate instead of in one pile, thanks!

I just got a notification that elasticsearch is now providing sha1 hashes for their downloads:

https://github.com/elasticsearch/elasticsearch/issues/2050#issuecomment-18506146

The downloads are also taking place on SSL:

http://www.elasticsearch.org/download/

I'll close the githhub bug associated with this discussion.

Luis

Hi all,

I'd originally worked on an ansible version of the elasticsearch charm not realising that the charmstore should only have one charm for each service. I'm putting it here for discussion, in case it's something Paul and Luis or others involved in the elasticsearch charm are interested switching to (once parity is achieved), if not, I can just use it as a separate charm outside of the charmstore.

lp:~michael.nelson/charms/precise/elasticsearch/trunk (attached above)

My needs were (for a deployment):
  * Be able to install an elasticsearch deb package without downloading it during the install hook (while still allowing the default behaviour of downloading during the install hook)
  * Provide a generic way of doing the discovery that can work on all providers without extra plugins (ie. using juju to orchestrate via a peer relationship rather than plugins that work on ec2 only).
  * Include both unit-tests and functional (amulet) tests for the charm.
  * Simple to maintain

And I wanted to try out the ansible charm helpers support on a real charm (which was more of an experiment). I'm pretty happy with the result from a maintenance point-of-view - you can understand everything about what the charm does by looking at the one file - the playbook.yaml:

http://bazaar.launchpad.net/~michael.nelson/charms/precise/elasticsearch/trunk/view/head:/playbook.yaml

Test locally with `juju test` (verified on trusty, juju 1.17.3, charm-tools 1.2.8-0ubuntu1~ubuntu13.10.1~ppa1, amulet 1.2.1-0ubuntu1~ubuntu13.10.1~ppa1)

If this is something others are keen to switch to, there are a few things that I'll need to check and followup:
 * Confirm that cluster-name not required for rest interface (why rest interface, and not just website?). The kibana rest-relation-changed doesn't use the cluster name (and could get it from the host if needed), and just uses private-address for the host anyway).
 * Find out why logstash template is included in this elasticsearch charm, and why the template is added in start hook - shouldn't that be part of logstash charm/subordinate?
 * Check whether need to add cluster-relation-changed as it was previously.

I'm happy to work on this until it has feature parity with what people are currently using, but understand if it's not something you want in the charmstore version given the work that's gone into the current charm.

Thanks!

I've just added the cluster/rest provisions to match the existing interface, and verified that I get the same results deploying with kibana with the current charmstore elasticsearch and my branch.

I've also added nrpe-external-master support.

Let me know if there are other things that need to change for this to reach the charmstore.

Greetings Michael,

Thank you for sharing your work on the elasticsearch charm! I'm excited to review this particular release as

a) you provided tests
b) its in ANSIBLE
c) I've written amulet tests around the existing elasticsearch charm.

In short, I'm jazzed about the work you've put in. During the review process I came up with the following notes/concerns:

There is no copyright file - this is a requirement for inclusion to the charm store. Choose a FOSS license and embed it in the file 'copyright'

When running the amulet tests, all test cases failed on the install hook with the following output:
failed: [localhost] => (item=openjdk-7-jre-headless,elasticsearch) => {"cmd": ["DEBIAN_FRONTEND=noninteractive", "DEBIAN_PRIORITY=critical", "/usr/bin/apt-get", "-y", "-o", "Dpkg::Options::=--force-confdef", "-o", "Dpkg::Options::=--force-confold", "install", "openjdk-7-jre-headless", "elasticsearch"], "failed": true, "item": "openjdk-7-jre-headless,elasticsearch", "rc": 2}
msg: [Errno 2] No such file or directory

You've made excellent progress on getting the charm under configuration management, and barring some simple modifications You are extremely close to having this revision accepted into the charm store.

I'm going to move the status of the bug to "incomplete". When you are ready for another peer review move the status to "New" or "Fix Committed" and someone will be along shortly to review your work.

On Tue, Mar 11, 2014 at 5:24 PM, Charles Butler
<email address hidden> wrote:
> Greetings Michael,
>
> Thank you for sharing your work on the elasticsearch charm! I'm excited
> to review this particular release as
>
> a) you provided tests
> b) its in ANSIBLE
> c) I've written amulet tests around the existing elasticsearch charm.
>
> In short, I'm jazzed about the work you've put in. During the review
> process I came up with the following notes/concerns:
>
> There is no copyright file - this is a requirement for inclusion to the
> charm store. Choose a FOSS license and embed it in the file 'copyright'

Will do.

>
> When running the amulet tests, all test cases failed on the install hook with the following output:
> failed: [localhost] => (item=openjdk-7-jre-headless,elasticsearch) => {"cmd": ["DEBIAN_FRONTEND=noninteractive", "DEBIAN_PRIORITY=critical", "/usr/bin/apt-get", "-y", "-o", "Dpkg::Options::=--force-confdef", "-o", "Dpkg::Options::=--force-confold", "install", "openjdk-7-jre-headless", "elasticsearch"], "failed": true, "item": "openjdk-7-jre-headless,elasticsearch", "rc": 2}
> msg: [Errno 2] No such file or directory

Yes, that was bad timing. There was a release in the ansible PPA
*today* which causes that bug -
https://github.com/ansible/ansible/issues/6385 .

I'll wait until the ansible PPA is updated before requesting a review again.

>
> You've made excellent progress on getting the charm under configuration
> management, and barring some simple modifications You are extremely
> close to having this revision accepted into the charm store.
>
> I'm going to move the status of the bug to "incomplete". When you are
> ready for another peer review move the status to "New" or "Fix
> Committed" and someone will be along shortly to review your work.

Thanks Charles, will do.

On Tue, Mar 11, 2014 at 5:46 PM, Michael Nelson
<email address hidden> wrote:
> On Tue, Mar 11, 2014 at 5:24 PM, Charles Butler
> <email address hidden> wrote:
>>
>> There is no copyright file - this is a requirement for inclusion to the
>> charm store. Choose a FOSS license and embed it in the file 'copyright'
>
> Will do.

I've just pushed r16 which simply adds the copyright.

>
>>
>> When running the amulet tests, all test cases failed on the install hook with the following output:
>> failed: [localhost] => (item=openjdk-7-jre-headless,elasticsearch) => {"cmd": ["DEBIAN_FRONTEND=noninteractive", "DEBIAN_PRIORITY=critical", "/usr/bin/apt-get", "-y", "-o", "Dpkg::Options::=--force-confdef", "-o", "Dpkg::Options::=--force-confold", "install", "openjdk-7-jre-headless", "elasticsearch"], "failed": true, "item": "openjdk-7-jre-headless,elasticsearch", "rc": 2}
>> msg: [Errno 2] No such file or directory
>
> Yes, that was bad timing. There was a release in the ansible PPA
> *today* which causes that bug -
> https://github.com/ansible/ansible/issues/6385 .
>
> I'll wait until the ansible PPA is updated before requesting a review again.

The PPA was updated the next day. If I run `juju test` now, I see the
first test passing, but the second and third fail due to install hook
errors on the sentry unit:

http://paste.ubuntu.com/7156564/

If I run the tests individually it times out:

http://paste.ubuntu.com/7156722/

Let me know if it's a known issue or something I'm doing wrong.

All 3 amulet tests are passing.

Thanks for this rewrite! awesome to see ES as deployed via an ansible playbook!

In general, everything looks good. Few little things to fix or consider:

Test running:

  - tests expect python-nose, but `make test` runs nosetests3, the
    executable installed by python3-nose.

  - I had to install nose, mock, and py-yaml before I could get the
    tests to run. To allow for automated testing, installing the deps
    should be a requirement for the makefile test target,

  - To get the tests to run, I simplified command to
    `@PYTHONPATH=./hooks nosetests --nologcapture unit_tests`. I'm
    pretty sure this should work in or out of a virtualenv, provided
    the deps are installed.

metadata.yaml

  Charm proof throws a warning on the maintainer field having more
  than a single name and single email. Support is coming for multiple
  maintainers and it will expect a yaml list of names and emails.

playbook.yml

  I recommend encapsulating your ansible bits into a folder named
  `playbook` and naming your entry yaml file `hooks.yml`. The playbook
  looks great, you could improve the scannability a bit by
  consistently putting the tags at the end of each task declaration.

I'm going ahead and giving this a plus one. Thanks again Micheal!

Hey micheal, was playing a bit and noticed some flakiness around adding peers. Is there some reason no to let elasticsearch do auto-discovery and use the cluster name to avoid unwanted cluster joins?

On Tue, May 6, 2014 at 5:07 PM, Whit Morriss <email address hidden> wrote:
> Hey micheal, was playing a bit and noticed some flakiness around adding
> peers. Is there some reason no to let elasticsearch do auto-discovery
> and use the cluster name to avoid unwanted cluster joins?

Hi Whit, thanks for digging in there.

Can you give me steps to reproduce the flakiness? I'd love to get that sorted.

The elasticsearch autodiscovery is dependent on multicast which isn't
generally available. Different services have their own way of enabling
something similar, but I'd really like to have the charm doing one
thing that works (and is tested) on any service provider, rather than
using one plugin for ec2, something else for HP Cloud etc. Given that
juju is already providing the peer IPs, it seemed like the sane thing
to do.

As above, I'd love to debug and fix the flakiness that you're seeing
(the tests include testing a 3 unit deploy). Thanks!

The cluster problem may relate to bug 1294202.

On Tue, May 6, 2014 at 6:09 PM, Curtis Hovey <email address hidden> wrote:
> The cluster problem may relate to bug 1294202.

Isn't that bug against the current elasticsearch charm (which uses
autodiscovery)? The version I've got here intentionally does not use
autodiscovery, instead using the peer relation (so it can work in any
environment whether multicast is supported or not).

That said, I'd still like to test what happens if another ES unit on
the same network is started with the same cluster name with
autodiscovery on (ie. will it add the units, even though they have
autodiscovery off themselves... I'd guessing so, which is still
dangerous), add firewall rules or similar so that the units can only
talk to peers on 9400. Auto-generating the clustername is a good idea
too, but would still allow malicious users to set the cluster name
intentionally.

@Michael
Bug 1294202 is against the version of elasticsearch run by Canonical IS. The charm was initially the elasticsearch charm I wrote that used my deb. IS forked the charm and now build their own debs.

Hello Michael,

I tried using this branch on our enterprise servers that have a more limited networking (to simulate customer production environments). The install hook failed getting the GPG-KEY from elastsicsearch.org. Would it be possible to edit your code to check for the elasticsearch.tar.gz (or .deb) in the charm first and use that to install?

This is a feature the current version of the elasticsearch charm that enables the charm to install on a limited network environment that could not reach elasticsearch.org.

The unit log file is attached for your reference.

On Sat, May 24, 2014 at 12:04 AM, Matt Bruzek
<email address hidden> wrote:
> Hello Michael,
>
> I tried using this branch on our enterprise servers that have a more
> limited networking (to simulate customer production environments). The
> install hook failed getting the GPG-KEY from elastsicsearch.org. Would
> it be possible to edit your code to check for the elasticsearch.tar.gz
> (or .deb) in the charm first and use that to install?
>
> This is a feature the current version of the elasticsearch charm that
> enables the charm to install on a limited network environment that could
> not reach elasticsearch.org.

Hi Matt,

I can do that, sure. FWIW, this branch was previously supporting
install of a local package, but we removed it when adding support for
a local repository. So assuming you're dealing with a limited network
environment without access to a local repository from which to install
elasticsearch and/or other packages, I'll add it back as an option.

-Michael

On Mon, May 26, 2014 at 9:26 AM, Michael Nelson
<email address hidden> wrote:
> On Sat, May 24, 2014 at 12:04 AM, Matt Bruzek
> <email address hidden> wrote:
>> Hello Michael,
>>
>> I tried using this branch on our enterprise servers that have a more
>> limited networking (to simulate customer production environments). The
>> install hook failed getting the GPG-KEY from elastsicsearch.org. Would
>> it be possible to edit your code to check for the elasticsearch.tar.gz
>> (or .deb) in the charm first and use that to install?
>>
>> This is a feature the current version of the elasticsearch charm that
>> enables the charm to install on a limited network environment that could
>> not reach elasticsearch.org.
>
> Hi Matt,
>
> I can do that, sure. FWIW, this branch was previously supporting
> install of a local package, but we removed it when adding support for
> a local repository. So assuming you're dealing with a limited network
> environment without access to a local repository from which to install
> elasticsearch and/or other packages, I'll add it back as an option.

Just to be explicit, when we're deploying this in our restricted
environment, the options we have set are:

       options:
         cluster-name: whatever-name
         install-ansible-from-ppa: false
         apt-repository: deb http://our.private.repo/ubuntu precise main
         apt-key-url: ""

It might be worthwhile ensuring that if a non-default apt-repository
is set, the apt-key-url is defaults to "" rather than the normal
default at elasticsearch.org.

fwiw, i've been using your es charm as part of a trusty stack, and wanted
to relay a few findings. first i'm not sure what the hold up is on pushing
this forward, the extant elasticsearch in the store is pretty broken for
non-ec2 and is suboptimal by using config to workaround lack of
implementation of relations.

also i realize its inherited (and complete baggage) but its unclear what
the distinction is between rest and cluster, they both seem to be identical
forms of provider side access via http. one of them should get dropped, are
both actually in use. also you've added a website / http relation isn't
conveying port which is a required part of that interface communications.
also missing a charm icon (i'd just grab the one off trunk).

On Mon, May 26, 2014 at 4:36 AM, Michael Nelson
<email address hidden>wrote:

> On Mon, May 26, 2014 at 9:26 AM, Michael Nelson
> <email address hidden> wrote:
> > On Sat, May 24, 2014 at 12:04 AM, Matt Bruzek
> > <email address hidden> wrote:
> >> Hello Michael,
> >>
> >> I tried using this branch on our enterprise servers that have a more
> >> limited networking (to simulate customer production environments). The
> >> install hook failed getting the GPG-KEY from elastsicsearch.org. Would
> >> it be possible to edit your code to check for the elasticsearch.tar.gz
> >> (or .deb) in the charm first and use that to install?
> >>
> >> This is a feature the current version of the elasticsearch charm that
> >> enables the charm to install on a limited network environment that could
> >> not reach elasticsearch.org.
> >
> > Hi Matt,
> >
> > I can do that, sure. FWIW, this branch was previously supporting
> > install of a local package, but we removed it when adding support for
> > a local repository. So assuming you're dealing with a limited network
> > environment without access to a local repository from which to install
> > elasticsearch and/or other packages, I'll add it back as an option.
>
> Just to be explicit, when we're deploying this in our restricted
> environment, the options we have set are:
>
> options:
> cluster-name: whatever-name
> install-ansible-from-ppa: false
> apt-repository: deb http://our.private.repo/ubuntu precise main
> apt-key-url: ""
>
> It might be worthwhile ensuring that if a non-default apt-repository
> is set, the apt-key-url is defaults to "" rather than the normal
> default at elasticsearch.org.
>
> --
> You received this bug notification because you are a member of charmers,
> which is subscribed to the bug report.
> https://bugs.launchpad.net/bugs/822979
>
> Title:
> Charm needed: Elastic Search
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/charms/+bug/822979/+subscriptions
>

On Mon, May 26, 2014 at 2:18 PM, Kapil Thangavelu
<email address hidden> wrote:
> fwiw, i've been using your es charm as part of a trusty stack, and wanted
> to relay a few findings. first i'm not sure what the hold up is on pushing
> this forward, the extant elasticsearch in the store is pretty broken for
> non-ec2 and is suboptimal by using config to workaround lack of
> implementation of relations.

Great, thanks for the feedback Kapil.

>
> also i realize its inherited (and complete baggage) but its unclear what
> the distinction is between rest and cluster, they both seem to be identical
> forms of provider side access via http. one of them should get dropped, are
> both actually in use.

As per the comment in the metadata.yaml, they're there only to provide
backwards compat. for the existing charms that relate using those, and
are as far as I can tell, both otherwise unnecessary.

At least, I'm not using them at all in our deployments, instead using
the standard website interface. Do you think we should instead
standardise on 'rest' (maybe more descriptive, but in the end, also
just an address/port).

> also you've added a website / http relation isn't
> conveying port which is a required part of that interface communications.

Yes, I was a bit unsure what to do there, given that by default
elasticsearch uses a *range*, 9200-9300.
Given that juju is deploying instances for each unit, it seems sane to
just use 9200 instead of a range and publish that through the
interface? Let me know if you're OK with that at:

https://code.launchpad.net/~michael.nelson/charms/precise/elasticsearch/add-port-to-iface/+merge/220969

> also missing a charm icon (i'd just grab the one off trunk).

I've added lp:charms/elasticsearch/icon.svg to the above MP.

If you agree with those changes, I'll merge them in.

Thanks Kapil.

On Mon, May 26, 2014 at 11:17 AM, Michael Nelson
<email address hidden>wrote:

> On Mon, May 26, 2014 at 2:18 PM, Kapil Thangavelu
> <email address hidden> wrote:
> > fwiw, i've been using your es charm as part of a trusty stack, and
> wanted
> > to relay a few findings. first i'm not sure what the hold up is on
> pushing
> > this forward, the extant elasticsearch in the store is pretty broken for
> > non-ec2 and is suboptimal by using config to workaround lack of
> > implementation of relations.
>
> Great, thanks for the feedback Kapil.
>
>
Forgot to say, this i really like the way the ansible charm-helpers stuff
has come together, this charm looks great. I'm hoping to use it in a charm
school re how to write ansible charms, it would be nice to toss it into the
charm-tools ala charm create -t ansible that timS's been working on.

One cool trick with newer juju versions (1.18+) is juju run --service=es
"ansible -c playbooks.yaml" to ensure configuration safety (no env
variables set though).

> >
> > also i realize its inherited (and complete baggage) but its unclear what
> > the distinction is between rest and cluster, they both seem to be
> identical
> > forms of provider side access via http. one of them should get dropped,
> are
> > both actually in use.
>
> As per the comment in the metadata.yaml, they're there only to provide
> backwards compat. for the existing charms that relate using those, and
> are as far as I can tell, both otherwise unnecessary.
>

i'm a bit torn on whether their actually not needed. i've been dealing with
this in my etcd and influxdb charms, and it does feel more correct to label
the http traffic per the expected client consumption usage interface rather
than generically via transport. It makes connecting the pieces in the gui
much saner, and with systems like es (or those others) the software
internally routes to the correct location obviating the need for additional
infrastructure like haproxy that could play (albeit poorly) via the extant
http interface.

iotw. i'd rather drop the website/http interface for connecting to clients
(minus ui admin components), and rely on something like 'elasticsearch' for
a client interface, what i find distasteful in the extant store charm is
the use of two named relations using the same interface with zero
distinction creating needless ambiguity for users. unfortunately ic that
same ambiguity throughout the previous monitoring stack charms, and sad
they got into the store with such conventions.

>
> At least, I'm not using them at all in our deployments, instead using
> the standard website interface. Do you think we should instead
> standardise on 'rest' (maybe more descriptive, but in the end, also
> just an address/port).
>

well the rest name is internal to elasticsearch, i'd be just as happy
renaming them to 'client' but renaming relation names is quite tricky
against extant systems. it requires destroying the relation and re-creating
on upgrade. it would be nice to have something in core that could help here
though its a bit unclear on what exactly, some form of declarative upgrade
mapping on relations perhaps.

>
> > also you've added a website / http ...

On Mon, May 26, 2014 at 6:33 PM, Kapil Thangavelu
<email address hidden> wrote:
> Forgot to say, this i really like the way the ansible charm-helpers stuff
> has come together, this charm looks great. I'm hoping to use it in a charm
> school re how to write ansible charms, it would be nice to toss it into the
> charm-tools ala charm create -t ansible that timS's been working on.

Yes, that would be nice. At the moment I've just got the bootstrap
[1], which could probably be used for the template.

[1] https://github.com/absoludity/charm-bootstrap-ansible

> i'm a bit torn on whether their actually not needed. i've been dealing with
> this in my etcd and influxdb charms, and it does feel more correct to label
> the http traffic per the expected client consumption usage interface rather
> than generically via transport. It makes connecting the pieces in the gui
> much saner, and with systems like es (or those others) the software
> internally routes to the correct location obviating the need for additional
> infrastructure like haproxy that could play (albeit poorly) via the extant
> http interface.
>
> iotw. i'd rather drop the website/http interface for connecting to clients
> (minus ui admin components), and rely on something like 'elasticsearch' for
> a client interface, what i find distasteful in the extant store charm is
> the use of two named relations using the same interface with zero
> distinction creating needless ambiguity for users. unfortunately ic that
> same ambiguity throughout the previous monitoring stack charms, and sad
> they got into the store with such conventions.

So based on those comments, I've removed the 'website' relation and
instead promoted the 'rest' relation as the client interface, so that
it's just the 'cluster' relation that is now creating the ambiguity
(and it's commented). The other option would have been to add an
'elasticsearch' or 'client' relation but then have 3 using the same
interface without distinction.

Let me know if that's not what you want. And btw, dropping the website
interface shouldn't be an issue here as it's not yet in the charm
store.

>
> well the rest name is internal to elasticsearch, i'd be just as happy
> renaming them to 'client' but renaming relation names is quite tricky
> against extant systems.

If you think we should add the 'client' relation now for going
forward, happy to do that, but it'd mean 3 relations doing the same
thing.

> it requires destroying the relation and re-creating
> on upgrade. it would be nice to have something in core that could help here
> though its a bit unclear on what exactly, some form of declarative upgrade
> mapping on relations perhaps.

Right. In our case that's not really an issue... I want to re-deploy
on trusty anyway.

>
> re matt's comment, it would be nice to support simple file inclusion for
> capturing the dependency on this.. local repositories are a fairly advanced
> usage mode in comparison, with much more cognitive overhead. dropping a
> file into a directory on the charm is about as simple as it can get.

Added that too. So if your charm includes a files/elasticsearch.deb,
it'll be installed instead. Here's an example:

http...

Current version looks great except "sets: [cluster-name, host, port]" causes charm proof to fail.

On Tue, May 27, 2014 at 10:45 AM, Michael Nelson <email address hidden>
wrote:

> On Mon, May 26, 2014 at 6:33 PM, Kapil Thangavelu
> <email address hidden> wrote:
> > Forgot to say, this i really like the way the ansible charm-helpers stuff
> > has come together, this charm looks great. I'm hoping to use it in a
> charm
> > school re how to write ansible charms, it would be nice to toss it into
> the
> > charm-tools ala charm create -t ansible that timS's been working on.
>
> Yes, that would be nice. At the moment I've just got the bootstrap
> [1], which could probably be used for the template.
>
> [1] https://github.com/absoludity/charm-bootstrap-ansible
>
> > i'm a bit torn on whether their actually not needed. i've been dealing
> with
> > this in my etcd and influxdb charms, and it does feel more correct to
> label
> > the http traffic per the expected client consumption usage interface
> rather
> > than generically via transport. It makes connecting the pieces in the gui
> > much saner, and with systems like es (or those others) the software
> > internally routes to the correct location obviating the need for
> additional
> > infrastructure like haproxy that could play (albeit poorly) via the
> extant
> > http interface.
> >
> > iotw. i'd rather drop the website/http interface for connecting to
> clients
> > (minus ui admin components), and rely on something like 'elasticsearch'
> for
> > a client interface, what i find distasteful in the extant store charm is
> > the use of two named relations using the same interface with zero
> > distinction creating needless ambiguity for users. unfortunately ic that
> > same ambiguity throughout the previous monitoring stack charms, and sad
> > they got into the store with such conventions.
>
> So based on those comments, I've removed the 'website' relation and
> instead promoted the 'rest' relation as the client interface, so that
> it's just the 'cluster' relation that is now creating the ambiguity
> (and it's commented). The other option would have been to add an
> 'elasticsearch' or 'client' relation but then have 3 using the same
> interface without distinction.
>
> Let me know if that's not what you want. And btw, dropping the website
> interface shouldn't be an issue here as it's not yet in the charm
> store.
>

The big thing i want todo is drop the ambiguity, ie no redundancy on
client interface names ('cluster', 'rest', 'website', 'client').. ideally
we'd just have a single simple name 'client'... 'rest' is fine.

It simplifies the documentation and is compatible with extant charms, just
that they need to add/remove relation if they upgrade (to a new charm of a
different series which seems like a good time for us to fix the bad
practices).

> >
> > well the rest name is internal to elasticsearch, i'd be just as happy
> > renaming them to 'client' but renaming relation names is quite tricky
> > against extant systems.
>
> If you think we should add the 'client' relation now for going
> forward, happy to do that, but it'd mean 3 relations doing the same
> thing.
>

per above, i'd say drop the others. we can do doc fixes in kibana/indexer
to remove using relation names.

>
> > it...

On Fri, Jun 6, 2014 at 12:39 AM, Whit Morriss
<email address hidden> wrote:
> Current version looks great except "sets: [cluster-name, host, port]"
> causes charm proof to fail.

I'm OK with charm proof failing there - it should be updated shouldn't
it? I mean, according to the docs [1] it's useful not just for
documentation, but will likely be used by future versions of juju.

[1] https://juju.ubuntu.com/docs/authors-charm-interfaces.html#documenting-charm-relation-interfaces

On Fri, Jun 6, 2014 at 12:40 AM, Kapil Thangavelu
<email address hidden> wrote:
>>
>> Let me know if that's not what you want. And btw, dropping the website
>> interface shouldn't be an issue here as it's not yet in the charm
>> store.
>>
>
> The big thing i want todo is drop the ambiguity, ie no redundancy on
> client interface names ('cluster', 'rest', 'website', 'client').. ideally
> we'd just have a single simple name 'client'... 'rest' is fine.
>
> It simplifies the documentation and is compatible with extant charms, just
> that they need to add/remove relation if they upgrade (to a new charm of a
> different series which seems like a good time for us to fix the bad
> practices).

Right.

>
>
>> >
>> > well the rest name is internal to elasticsearch, i'd be just as happy
>> > renaming them to 'client' but renaming relation names is quite tricky
>> > against extant systems.
>>
>> If you think we should add the 'client' relation now for going
>> forward, happy to do that, but it'd mean 3 relations doing the same
>> thing.
>>
>
> per above, i'd say drop the others. we can do doc fixes in kibana/indexer
> to remove using relation names.

Done with r21. I'll update one of my deploys today to use 'client' instead.

On Fri, Jun 6, 2014 at 8:28 AM, Michael Nelson
<email address hidden> wrote:
>> per above, i'd say drop the others. we can do doc fixes in kibana/indexer
>> to remove using relation names.
>
> Done with r21. I'll update one of my deploys today to use 'client' instead.

...and after testing with a real deploy, fixes in r22 :-)

problem:
  elasticsearch node discovery doesn't work. (tested on local trusty)

How to recreate :

create five elk nodes as follow:
  juju deploy local:trusty/elasticsearch master
  juju deploy local:trusty/elasticsearch slave
  juju add-unit -n 3 slave

however health check (by below json command) of master node revealed
node discovery has been unsuccessful.

 curl -XGET 'http://10.0.3.120:9200/_cluster/health?pretty=true'
{
  "cluster_name" : "elasticsearch",
  "status" : "green",
  "timed_out" : false,
  "number_of_nodes" : 1, <== should be 5
  "number_of_data_nodes" : 1, <== should be 5
  "active_primary_shards" : 0,
  "active_shards" : 0,
  "relocating_shards" : 0,
  "initializing_shards" : 0,
  "unassigned_shards" : 0
}

Expected discovery :
curl -XGET 'http://10.0.3.114:9200/_cluster/health?pretty=true'
{
  "cluster_name" : "elasticsearch",
  "status" : "green",
  "timed_out" : false,
  "number_of_nodes" : 5,
  "number_of_data_nodes" : 5,
  "active_primary_shards" : 0,
  "active_shards" : 0,
  "relocating_shards" : 0,
  "initializing_shards" : 0,
  "unassigned_shards" : 0
}

correction update:
my above deployment procedure was based on general Big Data deployment scenarios (master/slave). It happens that this charm uses peer scenario. Please update the readme file for Juju
novice like I .

agreed the readme does need an update, its currently referencing relations
interfaces and discovery items that are obsolete.

On Mon, Jun 16, 2014 at 10:57 AM, amir sanjar <email address hidden>
wrote:

> correction update:
> my above deployment procedure was based on general Big Data deployment
> scenarios (master/slave). It happens that this charm uses peer scenario.
> Please update the readme file for Juju
> novice like I .
>
> --
> You received this bug notification because you are a member of charmers,
> which is subscribed to the bug report.
> https://bugs.launchpad.net/bugs/822979
>
> Title:
> Charm needed: Elastic Search
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/charms/+bug/822979/+subscriptions
>

Greetings Michael,

As our followup via Email denoted, the charm has been promulgated as a trusty charm, moved from the original Precise mark. There is a follow up item that we have ot address with backporting the charm changes to precise - however this may impact a number of deployments as it removes several configuration options present in the existing Precise series charm. As this is a first for me, this warrants further discussion but should not hinder the progress of the ElasticSearch charm.

You can find the issue tracker for the ElasticSearch charm here: https://bugs.launchpad.net/charms/+source/elasticsearch/+bugs?field.status:list=NEW

Thank you again for your patience during the review process. We appreciate your dedication and efforts to making a high quality contribution to the juju charm store.

If you have any questions/comments/concerns about the review contact us in #juju on irc.freenode.net or email the mailing list <email address hidden>, or ask a question tagged with "juju" on http://askubuntu.com.