(MODULES-2971) Add java_home to all operating systems #184
Conversation
This patch adds the java_home variable to all supported operating systemd, and gives the user the option to set it themselves. It also updates config.pp to ensure that the JAVA_HOME variable is set to the desired java_home.
| @@ -9,6 +9,11 @@ | |||
| unless => "test /etc/alternatives/java -ef '${java::use_java_alternative_path}'", | |||
| } | |||
| } | |||
| if $java::use_java_home != undef { | |||
| file { '/etc/environment': | |||
There was a problem hiding this comment.
This file is used for environment variables of all processes, so should not be directly managed by this module as any other lines would be erased or it would cause puppet resource conflicts.
Is there any other possible way to solve this? Or do we just have to leave JAVA_HOME settings up to the user to implement?
There was a problem hiding this comment.
Yeah I wasn't really sure which way to go here - as it is the $java_home variable is being set, but unless I'm mistaken I don't think it's actually ever used. Is there no way for two different classes/modules to touch the same file without creating a resource conflict? (Like using line_in_file in ansible or something)
There was a problem hiding this comment.
could you use an exec resource to echo JAVA_HOME to /etc/environment instead?
This commit changes the way the JAVA_HOME variable is added to /etc/environment from using a file resource to using an exec resource. This way we can avoid resource conflicts with the file. Signed-off-by: Nate Potter <ntpttr@gmail.com>
|
The tests are passing now - would it be possible to remove the tests-fail lable? |
| user => 'root', | ||
| command => "echo JAVA_HOME=${$java::use_java_home} >> /etc/environment", | ||
| unless => "grep -Fx \"JAVA_HOME=${$java::use_java_home}\" \"/etc/environment\"", | ||
| } |
There was a problem hiding this comment.
Hmm, I think this is exactly what file_line from stdlib is for: https://github.com/puppetlabs/puppetlabs-stdlib#file_line
This commit updates the setting of JAVA_HOME in /etc/environment to be done with file_line rather than exec.
|
Not sure why the facter test is hanging just on job 403.3, on my machine with ruby 2.3.1 and puppet 4 all the tests are passing. |
|
The hanging is something we're seeing across the board. We think it's caused by mocha 1.5? |
| file_line { 'java-home-environment': | ||
| path => '/etc/environment', | ||
| line => "JAVA_HOME=${$java::use_java_home}", | ||
| match => 'JAVA_HOME=', |
There was a problem hiding this comment.
I think this would say "look for a match of JAVA_HOME= and if found then assume it is updated. This would cause file_line to update the environment once and never again.
There was a problem hiding this comment.
In the file_line documentation, it looks like 'replace' defaults to true, which would mean if a match is found the current implementation would be to replace it with the updated java home for the environment.
There was a problem hiding this comment.
Ah, yes. It looks for that line to replace instead of adding the new one, but looks for line exactly already.
This patch adds the java_home variable to all supported
operating systemd, and gives the user the option to
set it themselves. It also updates config.pp to
ensure that the JAVA_HOME variable is set to
the desired java_home.