[FLINK-36646][helm] Add required Java Opts to allow Java 17 based job images

[tomncooper]

What is the purpose of the change

From Java 16 onwards, the project jigsaw java opts are now mandatory. To allow Java 17 based job images, we need to include the java opts from the Flink distribution's base conf.yaml in the operator's helm default flink-conf.yaml.

The options are added under the kubernetes.operator.default-configuration.flink-version.<flink-version>.env.java.default-opts.all config option as these will be combined with any user supplied opts (via env.java.opts.all in the FlinkDeployment CR) and ignored by JDK 8 and 11 based images as we set -XX:IgnoreUnrecognizedVMOptions.

For Flink 1.18, the env.java.default-opts.all option doesn't exist, so I have used env.java.opts.all. However, this means any user adding their own opts will override the ones from the helm config and they will need to include them manually. I have added docs to highlight this.

I have tested these settings with the flink:<flink-version>-scala_2.12-java<java-version> images with Flink [1.18.1, 1.19.1, 1.20.0] and Java [8, 11, 17].

Note: If FLINK-36529 is added, which allows a <flink-version>+ syntax, we can reduce the verbosity of the config file entries and just use v1.19+.env.java.default-opts.all until such time as a future version changes the opts.

Brief change log

Added java opts from the Flink distributions default conf.yaml to the Operators helm default flink-conf.yaml.

Verifying this change

This change is verified by running the test matrix proposed in #910 (cherry picked into this PR branch).

Does this pull request potentially affect one of the following parts:

• Dependencies (does it add or upgrade a dependency): no
• The public API, i.e., is any changes to the CustomResourceDescriptors: no
• Core observer or reconciler logic that is regularly executed: no

Documentation

• Does this pull request introduce a new feature? no

[tomncooper]

One question I had was how to ensure these opts stay in sync with the upstream Flink Distribution's opts? Should we add a script to sync them with a tagged release from the main repo? Or would documenting the need to sync them be enough?

[SamBarker]

The options were added under the env.java.default-opts.all config option as these will be combined with any user supplied env.java.opts.all in the FlinkDeployment CR.

Whats does the upgrade story look like for users who have already manually configured things so to run a java17 image? How does java handle duplicate instances of --add-exports=?

I guess in theory the set of exports needs to evolve with each Flink version right? So does that mean the operator needs to support different sets for different versions of Flink? That might be a more theoretical concern given the use of ALL-UNAMED.

This is mostly just a question from ignorance of how modules work, but if we add ALL-UNAMED by default does that work for users who have properly modularised Flink jobs (is that even possible?)

Once #910 is merged, the full flink/java version matrix can be tested and should pass.

I think we should just merge #910 into this PR, it doesn't really add any value on its own.

[tomncooper]

Thanks @SamBarker, using the env.java.default-opts.all config instead of the env.java.opts.all means that any user supplied opts will be appended. Duplicate opts will not cause an error, the java compiler just ignores them.

Your question about differences in option sets between Flink versions is a good one and needs to be addressed. The module opts haven't changed since they were added, but that doesn't mean they won't and it is conceivable that some export/open opts will be removed in later versions that are required in earlier versions.

We could do some munging of the options in the helm template (selecting them based on the configured flink_version) but again that relies on having a sync'd copy of the opts for each version, so smells wrong.

I think @robobario's suggestion, in the JIRA issue, of the individual Flink distribution setting these is probably the long-term solution. I will have a look at that next, in the meantime I will pull #910 into this to confirm it works for all versions.

I will leave this PR up to see if anyone else has opinions. It is a short term fix until the Java 17 opts issue addressed in the main distribution or the opts change in a future Flink version.

[tomncooper]

While I dig into the Flink distribution java settings, I switched the opts in the default helm config to be version specific. So at least we can have different opts for different versions. But we still need to copy and paste these from the Flink dist's conf.

[gyfora]

As long as the e2es pass, good to go from my side

[SamBarker]

Test failure is
Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make field private final java.lang.Object[] java.util.Arrays$ArrayList.a accessible: module java.base does not "opens java.util" to unnamed module @1ad282e0

When using v18 with jdk 17

[robobario]

I don't see env.java.default-opts.all in the 1.18.1 docs, maybe it was introduced in 1.19?

[tomncooper]

Thanks @SamBarker and good spot @robobario. I have updated 1.18 to use the env.java.opts.all and added documentation to highlight the difference.

@gyfora Hopefully that will get the e2e tests to pass. Are the docs in the appropriate section?

[gyfora]

@tomncooper looks good, could you please organize the commits into 2 distinct commits if you want to keep your work from @SamBarker 's separately so I can merge it that way? Alternatively I can squash the whole thing during merge

[tomncooper]

@gyfora That should be ready to go now.