Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pid_path is owned by elasticsearch #193

Closed
kamaradclimber opened this issue Mar 10, 2014 · 7 comments
Closed

pid_path is owned by elasticsearch #193

kamaradclimber opened this issue Mar 10, 2014 · 7 comments

Comments

@kamaradclimber
Copy link

since a95a3f7, default pid_path attribute is /usr/local/var/run.
However the default recipe define this as directory resource and specify that user/group owners are elasticsearch.
This directory should belongs to root and allow elasticsearch to write its pid.

@kamaradclimber
Copy link
Author

in https://github.com/elasticsearch/elasticsearch/blob/master/src/deb/init.d/elasticsearch#L152, we can see the pid file being created by the init.d script and chown to elasticsearch.

@karmi would you consider a patch to fix this behavior ?

@karmi
Copy link
Contributor

karmi commented Mar 10, 2014

I believe you mean this line?

The location has been changed because of discussion in #108.

So the expression in default.rb#L40 is problematic.

The problem is that we can't actually blindly chown the node.elasticsearch. pid_path to Elasticsearch. Chowning it to root probably makes some sense, but would like to see some feedback/research around it first...

@kamaradclimber
Copy link
Author

a way to fix this would be to specify the pid_path directory resource without specifying the owner/group. This would let existing value if the directroy already exists or give it to root if created by chef.
Then in the init.d file, we chown pid_file (not pid_path) to give it to elasticsearch user.

the current default of pid_path is a directory used by several applications (not only elasticsearch), giving it to elasticsearch user breaks these other applications.

@karmi
Copy link
Contributor

karmi commented Mar 10, 2014

the current default of pid_path is a directory used by several applications (not only elasticsearch), giving it to elasticsearch user breaks these other applications.

Agreed, this shouldn't happen. Chowning the PID file in the init script is safe.

kamaradclimber added a commit to criteo-forks/elasticsearch that referenced this issue Mar 10, 2014
kamaradclimber added a commit to criteo-forks/elasticsearch that referenced this issue Mar 10, 2014
@kamaradclimber
Copy link
Author

do you need me to change anything in this PR ?

kamaradclimber added a commit to criteo-forks/elasticsearch that referenced this issue May 5, 2014
@karmi
Copy link
Contributor

karmi commented May 5, 2014

@kamaradclimber Sorry, tied up with something else right now, I'll try to process the cookbooks PR soon... Would like to verify it, even though the code change is very simple.

@karmi karmi closed this as completed in 76d918d May 7, 2014
@karmi
Copy link
Contributor

karmi commented May 7, 2014

Hi, thanks for the patch!, merged it in!

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

Successfully merging a pull request may close this issue.

2 participants