SOLR-15960 Unified use of system properties and environment variables

[janhoy]

https://issues.apache.org/jira/browse/SOLR-15960

This is a first step, demonstrating how we can skip converting env.vars like SOLR_FOO in bin/solr and then passing them as -Dsolr.foo. Instead we just provide Solr with all the SOLR_* environment variables, and let the new EnvUtils class convert and set the proper system propertis. Our Solr code can then continue reading System.getProperty("solr.foo") as before.

We have some exceptions to the naming convention, and for those there is a properties file to map ENV -> prop key.

Some places in Solr we look for either env.var or sys.prop and pick the one that exists. This is no longer necessary, as you can just read the sys.prop and it will be set if the corresponding ENV was passed to Solr.

This PR gives a few examples of cleanup that can be done, by removing explicit parsing of SOLR_LOG_LEVEL and SOLR_MODULES from start scripts. Tons of other places can also be simplified, but to keep this PR small, this is probably a nice first step, then other can follow in other PRs.

[janhoy]

Tests pass, @epugh can you have a first quick review and tell if this helps your CLI work? After your first feedback I can invite more reviewers.

I feel the principle is well proven by the fact that I removed explicit parsing of SOLR_MODULES variable in bin/solr, and still test_modules.bats pass tests both for -Dsolr.modules and export SOLR_MODULES.

I want to keep the scope of this PR somewhat limited. Can follow up with new PRs that remove cruft from the scripts.

[epugh]

Would then the preferred access mode for getting properties be via EnvUtils.getProp(SolrDispatchFilter.SOLR_DEFAULT_CONFDIR_ATTRIBUTE) pattern? I could imagine in a future ticket getting rid of the System.getPRoperty and adding that to our list of disallowed methods ;-). One more thought, how did you build the list of nonstandard mappings?

[janhoy]

Would then the preferred access mode for getting properties be via EnvUtils.getProp(SolrDispatchFilter.SOLR_DEFAULT_CONFDIR_ATTRIBUTE) pattern? I could imagine in a future ticket getting rid of the System.getPRoperty and adding that to our list of disallowed methods ;-). One more thought, how did you build the list of nonstandard mappings?

Such moves can be considered for sure. The EnvUtils.getProp() method does not bring much to the table, but the other variants such as getPropAsList() etc are helpful, but still a convenience on top of the props. I may have duplicated similar logic from other Solr Util classes, so happy to consolidate if you know of something.

[epugh]

This looks like a great step in the right direction! I don't yet have like a immediate "hey, this totally solved one of the CLI issues", but I bet it helps. Bigger is that in the future, maybe folks can use either system properties or environment variables to configure Solr without it being a battle ;-).

[janhoy]

Thanks for your review @dsmiley and sorry for late response. I updated PR.

[janhoy]

In 28400bb I removed yet 20 lines from bin/solr :)

[janhoy]

@HoustonPutman Thanks to your BATS ssl tests, we caught a problem in this PR. Needed to exclude all SOLR_SSL_* vars from auto conversion to SysProp.

[janhoy]

Disabled locking and tests pass. I seem to recall that the GlobalCircuitBreaker tests would break. I’ll try some beasting over Christmas..

[janhoy]

Could not make the CB tests fail with beasting. So I fully removed all locking code. We'll soon discover if there are issues.

[dsmiley]

Thanks for getting back to something simpler :-)

[janhoy]

Thanks for the extra review. Will merge to main tomorrow.

[epugh]

I went to check if Ref Guide was updated, and sure enough it was!