search

apache / solr-operator

Official Kubernetes operator for Apache Solr
https://solr.apache.org/operator
Apache License 2.0
243 stars 112 forks source link

Pass exporter args as env vars where possible #639

Closed gerlowskija closed 9 months ago

gerlowskija commented 9 months ago

In recent versions of Solr (>=8.8) the prometheus exporter has an env-var equivalent for each CLI argument taken by its start script.

This commit switches the operator over to providing values using this env-var capability where possible. Hopefully this makes the created resources a little cleaner and easier-to-read for administrators and users.

Closes #638

gerlowskija commented 9 months ago

Two questions I could use a bit of clarity on before this can be merged:

  1. How do we want to handle the potential conflict between user-specified env-vars, and values like .Spec.NumThreads that are converted into an env-var by the operator? Currently in this PR, the user-specified env-var "wins" that conflict, though I'm not sure that's the best behavior.
  2. Does this need a "changes" helm chart entry, or can that be skipped here as this is largely a refactor?
HoustonPutman commented 9 months ago
  1. How do we want to handle the potential conflict between user-specified env-vars, and values like .Spec.NumThreads that are converted into an env-var by the operator? Currently in this PR, the user-specified env-var "wins" that conflict, though I'm not sure that's the best behavior.

I think that user-specified env-vars should win.

  1. Does this need a "changes" helm chart entry, or can that be skipped here as this is largely a refactor?

Yeah I guess it doesn't need it, could go either way, so I don't particularly care!