Fixes bug #326 and adds the option to configure mail #445

jeffrey-e · 2018-04-24T13:41:16Z

Fixes #326
Added code to allow the configuration of x-pack notifications through mail.
Apparently a syntax error in parenthesis is also fixed, but can't remember I did that.

elasticmachine · 2018-04-24T13:41:19Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

itsmed · 2018-04-24T19:12:22Z

jenkins please test this

itsmed · 2018-04-25T16:34:14Z

jenkins test this please

itsmed · 2018-04-26T05:39:24Z

Thanks for this PR @gekkeharry13. The build is currently failing with

17:00:56        TASK [elasticsearch : Copy Configuration File] *********************************
17:00:56        fatal: [localhost]: FAILED! => {"changed": false, "msg": "AnsibleUndefinedVariable: 'path_repo' is undefined"}

which is a variable you've added. You'll need to add a default value in defaults/main.yml so this works for those who aren't using a custom setting.

The es_mail_config option is awesome! Could you update the README so that people know it exists and how to use it?

Updated configuration template with if statements for optional features

jeffrey-e · 2018-04-26T13:03:14Z

Good points! Added the documentation and made path_repo an if statement so it is optional (as it is not always used)
Also added an if statement to authentication settings in the E-mail config.

itsmed · 2018-04-26T14:56:18Z

jenkins test this please

itsmed · 2018-05-02T03:21:24Z

This is looking great @gekkeharry13. Could you add a default value for path_repo? Then I think it will be all set.

jeffrey-e · 2018-05-02T05:22:59Z

Hi itsmed, the path.repo value is not a default one (https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-snapshots.html). I think that this value should always be set manually as you must be aware of where your snapshots are stored. If not you could corrupt your filesystem if it runs out of space.

itsmed · 2018-05-02T22:12:30Z

@gekkeharry13 sorry, I can't read, or type :)

path_repo an if statement so it is optional

You addressed the concern I meant to bring up, so jenkins magic time.

jenkins test this

itsmed · 2018-05-03T00:31:47Z

Hey @gekkeharry13 we're still seeing build failures with

fatal: [localhost]: FAILED! => {"changed": false, "msg": "AnsibleUndefinedVariable: 'es_path_repo' is undefined"}

jeffrey-e · 2018-05-03T04:45:27Z

hmm sorry about that. Still getting used to ansible and jinja. Had to use "{% if es_path_repo is defined %}" instead of {% if es_path_repo %}. I have now verified it on my local machine, there it worked without the variable available. So hopefully jenkins likes the new master....

itsmed · 2018-05-03T04:50:49Z

no worries, my apologies. I got distracted and didn't finish the suggestion. Great to hear you have it working locally, let's run the build!

jenkins test this

itsmed · 2018-05-03T05:07:39Z

@gekkeharry13 this is looking close. We'll need to set a default value for action_auto_create_index

From the most recent build:

fatal: [localhost]: FAILED! => {"changed": false, "msg": "AnsibleUndefinedVariable: 'action_auto_create_index' is undefined"}

jeffrey-e · 2018-05-03T05:39:17Z

Hi itsmed, I have adjusted the name to match the standard (es_action_auto_create_index). As far as I can see here "https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-index_.html". The default value should be true, so added that to the defaults/main.yml

Added documentation for es_action_auto_create_index and fixed the order

jeffrey-e · 2018-05-03T14:31:11Z

@itsmed Was testing a bit with all settings and came across the fact that it es_action_auto_create_index should also be able to be set as an array with allowed indices, so adjusted the code accordingly. It was a bit of a hassle working it out, but it should work properly now!

jeffrey-e · 2018-05-03T14:58:01Z

hmm woopsie... My git skills are lacking pretty bad... Just pushed a set of commits to master while I was just trying to sync my local repo with the current master. I will try to figure out how to reverse things

jeffrey-e · 2018-05-03T15:25:25Z

maybe you can also review the code and provide feedback on if you like it to be pushed to your repo? I can imagine that SSL configuration might be a valuable addition, but for now it might be a bit too specific to my environment. If you can help me fix that I can make it available to you guys

itsmed · 2018-05-04T19:16:05Z

Let's see what the tests say first, then regarding the SSL configuration we'll review :)
jenkins test this

itsmed · 2018-05-04T23:10:14Z

I think for simplicity's sake @gekkeharry13 you should git revert to this commit and add the change you described in this comment so we can get this PR set, and open a second PR with the SSL changes you propose. The second round of changes are causing the build to fail at this time. What do you think?

jeffrey-e · 2018-05-05T06:07:17Z

Okee, I think we are back were we were... My history is a mess now, but ah well. PS is there a way to get in touch privately. I also am building Logstash and Kibana templates, which I am willing to share

itsmed · 2018-05-07T16:39:30Z

PS is there a way to get in touch privately. I also am building Logstash and Kibana templates, which I am willing to share

If you can send a link I'd be happy to help evaluate the kibana/logstash playbooks. In general we prefer to keep communication public to allow others to participate in the conversation, and it encourages accountability.

itsmed · 2018-05-07T16:39:47Z

jenkins test this

jeffrey-e · 2018-05-07T20:00:11Z

Kibana is located here: https://github.com/gekkeharry13/ansible-kibana. Logstash will be uploaded shortly after. They are currently a bit focused on my personal build, but when that part is over I will make them as generic as possible

itsmed · 2018-05-07T23:26:10Z

It looks like is defined got removed from the es_path_repo if statement during the revert @gekkeharry13 .
You had it set properly in this commit, could you add that back in? I think the other if/elif/else you have around action.auto_correct_index looks correct still.

jeffrey-e · 2018-05-08T05:08:34Z

it is back now, missed that part I guess

itsmed · 2018-05-08T15:38:33Z

jenkins test this

elasticmachine · 2018-05-22T18:15:20Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

itsmed · 2018-05-22T18:15:30Z

Sorry for the delay here @gekkeharry13. A change was made that may help ensure that the tests aren't transient failures, so I'm going to use the "update branch" button and re-run the tests to see if that helps. The last check had failed with fatal: [localhost]: FAILED! => {"changed": false, "msg": "AnsibleUndefinedVariable: 'action_auto_create_index' is undefined"} again.

itsmed · 2018-05-22T18:15:58Z

jenkins test this

Default value is es_action_auto_create_index: true

jeffrey-e · 2018-05-22T18:29:51Z

Awh man, I am sorry I messed up my repo this bad.... I managed to work in a seperate branch with my new changes now, so it no longer interferes. I have added es_action_auto_create_index: true back to my defaults/main.yml. I checked the repo and there are only references to "es_action_auto_create_index" not to "action_auto_create_index"

itsmed · 2018-05-22T18:38:05Z

No worries at all! Your contributions are much appreciated :) If you want to push the new changes to this branch ( and rebase against master to get the most recent changes ) so we can run the tests that would be great! If it's easier to push a new branch and open a new PR, ping me there and we can get that one tested and close this PR, as you prefer :)

itsmed · 2018-05-22T22:29:44Z

jenkins test this

itsmed · 2018-05-22T22:39:04Z

I think we're super close now @gekkeharry13 This time the build failed because es_mail_config is undefined. Looks like you're checking for account in the dict here:
{% if es_mail_config['account'] is defined %}

22:32:37 fatal: [localhost]: FAILED! => {"changed": false, "msg": "AnsibleUndefinedVariable: 'es_mail_config' is undefined"}

first check is es_mail_config is defined

jeffrey-e · 2018-05-23T04:19:15Z

I have updated the es_mail_config statement again :)

itsmed · 2018-05-23T05:04:15Z

jenkins test this

itsmed

LGTM, thanks so much @gekkeharry13 for iterating on this!

itsmed · 2018-04-26T05:38:47Z

templates/elasticsearch.yml.j2

@@ -23,6 +23,10 @@ path.data: {{ data_dirs | array_to_str }}

 path.logs: {{ log_dir }}

+path.repo: {{ path_repo }}


path_repo will need a default value set in default/main.yml

itsmed · 2018-04-26T05:38:51Z

templates/elasticsearch.yml.j2

@@ -23,6 +23,10 @@ path.data: {{ data_dirs | array_to_str }}

 path.logs: {{ log_dir }}

+path.repo: {{ path_repo }}
+
+action.auto_create_index: {{ action_auto_create_index }}


I anticipate that this will need a default value set as well

itsmed · 2018-05-22T22:40:06Z

templates/elasticsearch.yml.j2

@@ -44,3 +56,21 @@ xpack.ml.enabled: false
 xpack.graph.enabled: false
 {% endif %}
 {% endif %}
+
+{% if es_mail_config['account'] is defined %}


The most recent failure "AnsibleUndefinedVariable: 'es_mail_config' is undefined"
So I think we'll need to check for both es_mail_config and es_mail_config['account'].
Does that seem like the right way to go about it to you @gekkeharry13 ?

jeffrey-e · 2018-05-23T08:06:13Z

So everything is fine now? I am currently actively developing the following Ansible repo's:

They are currently working for (as I do not have much customisation to paths and such), if you like we can discuss if there is a way to provide them in a decent fashion to Elastic in order for other customers to use.

itsmed · 2018-05-25T17:41:48Z

Yes the PR has been merged. We do have plans to provide playbooks for our other products, so thanks for offering these up @gekkeharry13! We'll take a look at your playbooks here and assess how to move forward.

jeffrey-e · 2018-05-25T18:32:58Z

Okay great! Let me know if you need something from me!

Added changes to add required functionality and fix a bug

34f0813

Added documentation for added features

7aebcae

Updated configuration template with if statements for optional features

Merge branch 'master' into master

6cf11ef

Update elasticsearch.yml.j2

7640a03

jeffrey-e added 2 commits May 3, 2018 07:23

Added es_action_auto_create_index: true

4caefef

Adjust variable name to match standard

11a1e4b

jeffrey-e added 2 commits May 3, 2018 16:27

Fixed documentation

96f3a87

Added documentation for es_action_auto_create_index and fixed the order

Added support for providing an array with indices

da527ec

jeffrey-e added 2 commits May 3, 2018 16:45

Added more x-pack config options

cc28b61

Sync with HEAD

4791195

Fixed documentation commit issue

a3ea8e0

jeffrey-e added 5 commits May 5, 2018 08:00

Removing ssl configuration

7967317

Commit README as well

9ba1082

Merging went wrong

01fd5ce

test

e880cdf

Merged change

8cfa0cf

Added path repo is defined statement again

d72ce3c

Merge branch 'master' into master

31ee9af

Readded es_action_auto_create_index

c79fd2e

Default value is es_action_auto_create_index: true

Fixed es_mail_config again

cf4da01

first check is es_mail_config is defined

itsmed approved these changes May 23, 2018

View reviewed changes

itsmed merged commit a70d259 into elastic:master May 23, 2018

		@@ -23,6 +23,10 @@ path.data: {{ data_dirs \| array_to_str }}

		path.logs: {{ log_dir }}

		path.repo: {{ path_repo }}

Fixes bug #326 and adds the option to configure mail #445

Fixes bug #326 and adds the option to configure mail #445

Conversation

jeffrey-e commented Apr 24, 2018

elasticmachine commented Apr 24, 2018

itsmed commented Apr 24, 2018

itsmed commented Apr 25, 2018

itsmed commented Apr 26, 2018 • edited Loading

jeffrey-e commented Apr 26, 2018

itsmed commented Apr 26, 2018

itsmed commented May 2, 2018

jeffrey-e commented May 2, 2018

itsmed commented May 2, 2018

itsmed commented May 3, 2018

jeffrey-e commented May 3, 2018

itsmed commented May 3, 2018

itsmed commented May 3, 2018

jeffrey-e commented May 3, 2018

jeffrey-e commented May 3, 2018

jeffrey-e commented May 3, 2018

jeffrey-e commented May 3, 2018 • edited Loading

itsmed commented May 4, 2018

itsmed commented May 4, 2018

jeffrey-e commented May 5, 2018

itsmed commented May 7, 2018

itsmed commented May 7, 2018

jeffrey-e commented May 7, 2018

itsmed commented May 7, 2018

jeffrey-e commented May 8, 2018

itsmed commented May 8, 2018

elasticmachine commented May 22, 2018

itsmed commented May 22, 2018

itsmed commented May 22, 2018

jeffrey-e commented May 22, 2018

itsmed commented May 22, 2018

itsmed commented May 22, 2018

itsmed commented May 22, 2018

jeffrey-e commented May 23, 2018

itsmed commented May 23, 2018

itsmed left a comment

Choose a reason for hiding this comment

itsmed Apr 26, 2018

Choose a reason for hiding this comment

itsmed Apr 26, 2018

Choose a reason for hiding this comment

itsmed May 22, 2018

Choose a reason for hiding this comment

jeffrey-e commented May 23, 2018

itsmed commented May 25, 2018

jeffrey-e commented May 25, 2018

itsmed commented Apr 26, 2018 •

edited

Loading

jeffrey-e commented May 3, 2018 •

edited

Loading