[OPENJDK-3053] Aggregating JAVA_OPTS_APPEND

[ciis0]

https://issues.redhat.com/browse/OPENJDK-3053

---

Hi,

we are using these images in kustomize-based kubernetes deployments.

while kustomize can merge ConfigMaps, i.e. "VAL=foo" in a CM in the "base" can be overwritten by "VAL=bar" in an "overlay") it cannot merge variables -- if we have JAVA_OPTS_APPEND (or JAVA_OPTS for that matter) in one place, we can only overwrite it, but not extend it.

e.g. if we have a "base" with JAVA_OPTS_APPEND=-Dmyoption=foo and we would like to add -DanotherOption=bar in the "overlay", that requires to put JAVA_OPTS_APPEND=-Dmyoption=foo -Dmyoption=bar into the overlay.

Would you accept a PR that introduces a way to aggregate JAVA_OPTS_APPEND like the following?

# export syntax for easier toying around in shell
export JAVA_OPTS_APPEND_2_BASE_OPTS="-Done.option=foo -Danother.option=bar"
export JAVA_OPTS_APPEND_1_EARLY_OPTS="-XX:-HeapDumpOnOutOfMemoryError"
export JAVA_OPTS_APPEND_3_CACERTS="-Djavax.net.ssl.trustStore=/media/configmap/cacerts/cacerts -Djavax.net.ssl.trustStorePassword=changedit"
export JAVA_OPTS_APPEND_100_END_OPTS="-XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=/media/pvc/heapdumps"

result (where i would prefer to have this (natural) order, but depending on the complexity no order guarantees might be given):

JAVA_OPTS_APPEND=-XX:-HeapDumpOnOutOfMemoryError -Done.option=foo -Danother.option=bar -Djavax.net.ssl.trustStore=/media/configmap/cacerts/cacerts -Djavax.net.ssl.trustStorePassword=changedit -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=/media/pvc/heapdumps

simple, without natural order:

shopt -s extglob
appends=$(compgen -v JAVA_OPTS_APPEND_)
for append in $appends
do JAVA_OPTS_APPEND="$JAVA_OPTS_APPEND ${!append}"
done

likely the logic should back-off when JAVA_OPTS_APPEND is already set.

advanced, natural order and enforced numeric infix to prevent unexpected ordering (which could also help warning about clashes):

shopt -s extglob
appends=$(compgen -v -X '!JAVA_OPTS_APPEND_+([[:digit:]])_*' | sort -V)
for append in $appends
do JAVA_OPTS_APPEND="$JAVA_OPTS_APPEND ${!append}"
done

additionally to backing off when already set, likely extglob should be set only temporarily to prevent unexpected side-effects in other scripts.

I verified both options with ubi8/openjdk-17-runtime:1.19-4.1715070687 already.

[ciis0]

if [ -z "${JAVA_OPTS_APPEND}" ]; then

    shopt -s extglob
    declare -A seen

    appends=$(compgen -v -X '!JAVA_OPTS_APPEND_+([[:digit:]])_*' | sort -V)
    echo -e "Generating JAVA_OPTS_APPENDS from:\n$appends"

    for append in $appends; do

        JAVA_OPTS_APPEND="$JAVA_OPTS_APPEND ${!append}"

        pat="JAVA_OPTS_APPEND_([0-9]+)_(.*)"
        if [[ "$append" =~ $pat ]]; then
            seen[${BASH_REMATCH[1]}]="${seen[${BASH_REMATCH[1]}]} ${BASH_REMATCH[0]}"
        fi
    done

    JAVA_OPTS_APPEND=$(echo "${JAVA_OPTS_APPEND}" | awk '$1=$1')

    for saw in ${!seen[*]}
    do
        elem=(${seen[$saw]})
        if [ ${#elem[*]} -gt 1 ]; then
            echo "Collision for JAVA_OPTS_APPEND_$saw? ${elem[@]}"
        fi
    done

    echo generated JAVA_OPTS_APPEND=$JAVA_OPTS_APPEND

fi

Generating JAVA_OPTS_APPENDS from:
JAVA_OPTS_APPEND_1_EARLY_OPTS
JAVA_OPTS_APPEND_2_BASE_OPTS
JAVA_OPTS_APPEND_3_CACERTS
JAVA_OPTS_APPEND_100_END_OPTS
generated JAVA_OPTS_APPEND=-XX:-HeapDumpOnOutOfMemoryError -Done.option=foo -Danother.option=bar -Djavax.net.ssl.trustStore=/media/configmap/cacerts/cacerts -Djavax.net.ssl.trustStorePassword=changedit -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=/media/pvc/heapdumps

[jmtd]

I sympathise with the problem, but I'd like to explore some other ways of solving it.

As I understand it, you have <openjdk image> → <base1> → <base2>, and each of <base1> and <base2> want to append JVM options. Is that right?

For some time I've wanted to de-couple some of functional areas of the run script, and this could be an opportunity to do so. What if you could drop a script into a known path to add to the JVM arguments? Something like (simplified)

get_java_options() {
  local opts

  for scriptlet in "$JBOSS_CONTAINER_JAVA_RUN_MODULE/opts.d"/*; do
    opts="$opts $($scriptlet)"
  done

  echo "${opts} ${JAVA_OPTS_APPEND}"
}

PoC https://github.com/jboss-container-images/openjdk/compare/ubi9...jmtd:gh-494-jvm-args#diff-e38c49939e67e17236485b64a7afc257e6b78a92f42e9654aac965f225fb00a4

[ciis0]

For some time I've wanted to de-couple some of functional areas of the run script, and this could be an opportunity to do so. What if you could drop a script into a known path to add to the JVM arguments?

That would be fine, too!

I would drop my script there then. :)

[ciis0]

Any updates on this?

[jmtd]

I haven't returned to it yet, sorry. We've just completed shipping an image update (:1.20) and it was too late to get on the slate for that, but, I think we can get this done for the next release. I've filed a corresponding JIRA so we can get it into consideration and I'm running the test suite against the PoC.

[ciis0]

ok, thank you!

[ciis0]

Mainly curious, what is the time frame for 1.21, some place where I can help regarding this? :)