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

fix(helm): fix missing whitespace in JAVA_TOOL_OPTIONS #1759

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

WatcherWhale
Copy link
Contributor

In the JAVA_TOOL_OPTIONS environment variable, the different values get put together without any (white)space.
This could lead to some options not being processed by the JVM.

This PR fixes this by removing the "trim whitespace" character from the affected template.

@thjaeckle thjaeckle added this to the 3.4.0 milestone Oct 2, 2023
@thjaeckle
Copy link
Member

I am not sure if this is now correct to be honest ..
from the build output for the gateway I e.g. see:

JAVA_TOOL_OPTIONS:                          -XX:+ExitOnOutOfMemoryError -XX:+UseContainerSupport -XX:+UseStringDeduplication -Xss512k -XX:MaxMetaspaceSize=256m -XX:+UseG1GC -Djava.net.preferIPv4Stack=true
                                                  -XX:ActiveProcessorCount=2 -XX:MaxRAMPercentage=50 -XX:InitialRAMPercentage=50 -XX:MaxGCPauseMillis=150
                                                  -Dpekko.management.cluster.bootstrap.contact-point-discovery.port-name=management -Dpekko.cluster.failure-detector.threshold=15.0 -Dpekko.cluster.failure-detector.expected-response-after=3s -Dpekko.cluster.failure-detector.acceptable-heartbeat-pause=7s -Dpekko.persistence.journal-plugin-fallback.recovery-event-timeout=30s -Dpekko.persistence.max-concurrent-recoveries=100 -Dpekko.cluster.sharding.updating-state-timeout=20s -Dpekko.cluster.shutdown-after-unsuccessful-join-seed-nodes=120s
                                                  
                                                  -Dlogback.configurationFile=/opt/ditto/logback.xml
                                                  
                                                  
                                                  -Dditto.protocol.blocklist.0=raw-request-uri -Dditto.protocol.blocklist.1=cache-control -Dditto.protocol.blocklist.2=connection -Dditto.protocol.blocklist.3=timeout-access -Dditto.protocol.blocklist.4=accept-encoding -Dditto.protocol.blocklist.5=x-forwarded-scheme -Dditto.protocol.blocklist.6=x-forwarded-port -Dditto.protocol.blocklist.7=x-forwarded-for -Dditto.protocol.blocklist.8=forwarded=for -Dditto.protocol.blocklist.9=sec-fetch-mode -Dditto.protocol.blocklist.10=sec-fetch-site -Dditto.protocol.blocklist.11=authorization -Dditto.protocol.blocklist.12=accept-language -Dditto.protocol.blocklist.13=host -Dditto.protocol.blocklist.14=via -Dditto.protocol.blocklist.15=sec-ch-ua -Dditto.protocol.blocklist.16=sec-ch-ua-mobile -Dditto.protocol.blocklist.17=sec-ch-ua-platform -Dditto.protocol.blocklist.18=sec-fetch-dest -Dditto.protocol.blocklist.19=user-agent

So adding a lot of completely blank lines - which hopefully works - I am however not quite sure/confident.

Best would be of course to have each system property on a separate line to have it also more readable.
Any ideas to enable it in that way, @WatcherWhale ?

@WatcherWhale
Copy link
Contributor Author

First of all thanks for the help!
Yeah, it is not really readable at the moment. I think the root of the original problem is {{- .Values.global.pekkoOptions }} having the trim whitespace character, maybe just reduce this PR to only change that line to {{ .Values.global.pekkoOptions }}?

@thjaeckle
Copy link
Member

@WatcherWhale yes, sounds good to me, let's try that first

Signed-off-by: Mathias Maes <mathias.maes@aloxy.io>
Copy link
Member

@thjaeckle thjaeckle left a comment

Choose a reason for hiding this comment

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

@WatcherWhale looks good now, I would approve! :)
Thanks for the contribution

@thjaeckle thjaeckle merged commit f878de5 into eclipse-ditto:master Oct 5, 2023
9 checks passed
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 this pull request may close these issues.

None yet

2 participants