Skip to content
This repository has been archived by the owner on Jun 24, 2022. It is now read-only.

Use default config files #637

Merged
merged 11 commits into from
Dec 5, 2019
Merged

Conversation

jmlrt
Copy link
Member

@jmlrt jmlrt commented Nov 26, 2019

Lot of ES config files templates provided by this role weren't up to date with files provided by standard elasticsearch 6.x and 7.x packages.

This PR fix that by using standard config file provided by elasticsearch package when possible (log4j2.properties) or updating templates to reflect new versions provided by elasticsearch packages (/etc/sysconfig/elasticsearch, /etc/default/elasticsearch and jvm.options).

This is also removing some options deprecated since 6.0.

Also allow to fully override these files by providing custom files.

Related to #631, #628, #598, #577 and #400 and https://discuss.elastic.co/t/es-6-8-vs-7-4-memory-issues/202849

The log4j configuration template provided by this ansible role doesn't reflect current log4j configuration in elasticsearch 6.x and 7.x.
Also this template doesn't contain any variable so everything is hardcoded inside.
As we don't need to override any value inside, let's use standard elasticsearch log4j configuration file provided by elasticsearch package and stop overriding it with this role.
This option doesn't seem to exists anymore in elasticsearch. It's not present in current versions of system configuration provided by recent elasticsearch packages and there is no code using it inside elasticsearch.
@jmlrt jmlrt requested review from Crazybus and a team November 26, 2019 22:51
@jmlrt jmlrt self-assigned this Nov 26, 2019
@jmlrt
Copy link
Member Author

jmlrt commented Nov 27, 2019

jenkins test this please

…kage

This is removing lot of options deprecated since 6.0 like ES_JVM_OPTIONS, CONF_DIR, DATA_DIR and LOG_DIR (https://www.elastic.co/guide/en/elasticsearch/reference/6.0/breaking_60_packaging_changes.html)
The jvm options template provided by this ansible role doesn't reflect current log4j configuration in elasticsearch 6.x and 7.x. This commit update it to default version provided in 6.x and 7.x elasticsearch standard packages.
@jmlrt jmlrt force-pushed the use-default-config-files branch 2 times, most recently from f72510d to 06009f5 Compare November 27, 2019 11:17
@jmlrt
Copy link
Member Author

jmlrt commented Nov 27, 2019

Ubuntu 14.04 failures aren't related => created test-kitchen/kitchen-docker#353 for that

@jmlrt
Copy link
Member Author

jmlrt commented Nov 27, 2019

I have a really strange permissions issue with this PR:

  • permissions on master branch:
kitchen@3d4e7e477739:~$ ls -ld /etc/elasticsearch/
drwxr-s--- 2 root elasticsearch 4096 Nov 27 16:41 /etc/elasticsearch/
kitchen@3d4e7e477739:~$ ls -ld /var/lib/elasticsearch/
drwxr-s--- 3 elasticsearch elasticsearch 4096 Nov 27 16:41 /var/lib/elasticsearch/
kitchen@3d4e7e477739:~$ ls -ld /var/log/elasticsearch/
drwxr-s--- 2 elasticsearch elasticsearch 4096 Nov 27 16:41 /var/log/elasticsearch/
  • permissions on this PR:
[kitchen@093070b94dce ~]$ ls -ld /etc/elasticsearch/
d-wSrwxrwT 2 root elasticsearch 4096 Nov 27 16:05 /etc/elasticsearch/
[kitchen@093070b94dce ~]$ ls -ld /var/lib/elasticsearch/
d-wSrwxrwT 2 elasticsearch elasticsearch 4096 Oct 28 20:56 /var/lib/elasticsearch/
[kitchen@093070b94dce ~]$ ls -ld /var/log/elasticsearch/
d-wSrwxrwT 2 elasticsearch elasticsearch 4096 Oct 28 20:56 /var/log/elasticsearch/

I don't see any change which could give these strange permissions...

@Conky5
Copy link
Contributor

Conky5 commented Nov 27, 2019

@jmlrt maybe try putting all the mode values in quotes, the 2750 might be causing this funkiness https://docs.ansible.com/ansible/latest/modules/file_module.html#file-module

You must either add a leading zero so that Ansible's YAML parser knows it is an octal number (like 0644 or 01777) or quote it (like '644' or '1777') so Ansible receives a string and can do its own conversion from string into number.

From Ansible doc (https://docs.ansible.com/ansible/latest/modules/template_module.html#template-module)
For those used to /usr/bin/chmod remember that modes are actually octal numbers. You must either add a leading zero so that Ansible's YAML parser knows it is an octal number (like 0644 or 01777) or quote it (like '644' or '1777') so Ansible receives a string and can do its own conversion from string into number.
Giving Ansible a number without following one of these rules will end up with a decimal number which will have unexpected results.
@jmlrt
Copy link
Member Author

jmlrt commented Nov 28, 2019

Finally tests are OK (except ubuntu-1404 due to #637 (comment))! @Conky5 @fatmcgav @Crazybus This is ready for review.

tasks/elasticsearch-config.yml Outdated Show resolved Hide resolved
tasks/elasticsearch-config.yml Outdated Show resolved Hide resolved
templates/jvm.options.j2 Show resolved Hide resolved
templates/log4j2.properties.j2 Show resolved Hide resolved
@jmlrt jmlrt requested a review from Crazybus December 2, 2019 15:21
Copy link
Contributor

@Crazybus Crazybus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Contributor

@Conky5 Conky5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants