Make JVM_EXTRA_ARGS manageable for java 11 compatibility

[diLLec]

Pull Request (PR) description

With java-11-openjdk-11.0.7.10 the option "PrintGCTimeStamps" can't be used as a parameter on the JVM. Therefore this PR adds the capability to remove the parameter from setenv.sh.

This Pull Request (PR) fixes the following issues

Fixes the issue, that PrintGCTimeStamps is hardcoded in setenv.sh.erb template which results in a JVM startup error when java-11-openjdk is used.

[diLLec]

also see: https://opennms.discourse.group/t/migration-of-jvm-settings-from-oracle-jdk-8-to-openjdk-11/365

[diLLec]

Hi @nambrosch - I've added the parameter to the default value and fixed another problem with the mysql_connector. I hope that we can both process them in one PR?

[nambrosch]

Hi @nambrosch - I've added the parameter to the default value and fixed another problem with the mysql_connector. I hope that we can both process them in one PR?

shortly after i wrote that comment, as a "solution" i started passing the flags -XX:+ExplicitGCInvokesConcurrent -XX:+IgnoreUnrecognizedVMOptions -XX:+UseG1GC through java_opts which makes jdk8, jdk11, confluence, and jira all happy. I'm not sure what's better practice - leaving a commented-out line in your class or including the -XX:+IgnoreUnrecognizedVMOptions flag so all parameters can be included without consequence - just offering insight as to what worked for me. I didn't see -Xlog:::time,uptime,level,tags in any documentation, so I'm not sure if it should be included here?

[igalic]

I'm not sure what's better practice - leaving a commented-out line in your class or including the -XX:+IgnoreUnrecognizedVMOptions flag so all parameters can be included without consequence

@nambrosch, the consequence is that you're passing options, expecting them to have an effect, but they don't

i prefer a hard failure to hiding issues.

however, we shouldn't break what's worked so far

[nambrosch]

I'm not sure what's better practice - leaving a commented-out line in your class or including the -XX:+IgnoreUnrecognizedVMOptions flag so all parameters can be included without consequence

@nambrosch, the consequence is that you're passing options, expecting them to have an effect, but they don't

i prefer a hard failure to hiding issues.

however, we shouldn't break what's worked so far

Normally I agree - it's a good tool for managing transitions between JDKs where puppet modules or other start scripts have hard-coded parameters that cause startup to fail.

[igalic]

i think i'd like @SimonHoenscheid's opinion on these changes, as he was working on a very similar patch

[SimonHoenscheid]

i think i'd like @SimonHoenscheid's opinion on these changes, as he was working on a very similar patch

I see two things to make this change easy to maintain and understand:

1: Like I wrote here about supported versions and java dependencies, this would give us the option to depend on puppetlabs-java, therefore we can use these facts to make decisions more easy. 2: switch a complete different template based on version or using two different blocks based on java version would make this easy to understand.

[igalic]

thank you @SimonHoenscheid.

[vox-pupuli-tasks[bot]]

Dear @diLLec, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli Github Bot. I noticed that your pull request has CI failures. Can you please have a look at the failing CI jobs? If you need any help, you can reach out to us on our IRC channel voxpupuli on Freenode or our Slack channel voxpupuli at slack.puppet.com. You can find my sourcecode at voxpupuli/vox-pupuli-tasks

[diLLec]

@bastelfreak @igalic @nambrosch I've have changed the code to whats been discussed in #329 and commented there. All checks are green now. Please review the PR :)

[diLLec]

Can you clean up the commit history? Thanks.

done and tests still green

[kenyon]

I just rebased on current master to get rid of the commits already in master.