janhoy commented on code in PR #827:
URL: https://github.com/apache/solr-operator/pull/827#discussion_r4371387919
##########
docs/modules/upgrade-notes/pages/upgrade-notes.adoc:
##########
@@ -136,6 +137,35 @@ _Note that the Helm chart version does not contain a `v`
prefix, which the downl
=== v0.10.0
+* *Solr 10 support, and Solr image tags must start with the Solr version*
Review Comment:
Should we use some `====` subheaders in here, for better structure. Also,
perhaps some bullets can merge into one?
##########
controllers/solrprometheusexporter_controller_test.go:
##########
@@ -342,7 +342,7 @@ var _ = FDescribe("SolrPrometheusExporter controller -
General", func() {
},
Spec: solrv1beta1.SolrCloudSpec{
SolrImage: &solrv1beta1.ContainerImage{
- Tag:
"should-be-the-same",
+ Tag:
"9.10.0-should-be-the-same",
Review Comment:
I understand why this change was done in the test. But now when bumping the
default solr version to e.g. 9.11.0 and later, there are now three more strings
to update by hand.
Can you instead use e.g. the variable for Default Solr Version here, or just
"9-should-be-the-same", or something that is lower maintenance?
##########
controllers/util/solr_security_util.go:
##########
@@ -497,14 +497,22 @@ func BasicAuthEnvVars(secretName string) []corev1.EnvVar {
func useSecureProbe(solrCloud *solr.SolrCloud, probe *corev1.Probe, mountPath
string) {
// mount the secret in a file so it gets updated; env vars do not see:
//
https://kubernetes.io/docs/concepts/configuration/secret/#environment-variables-are-not-updated-after-a-secret-update
+ isSolr10 := solrCloud.IsSolr10OrLater()
javaToolOptions := make([]string, 0)
+ credentialsArg := ""
if solrCloud.Spec.SolrSecurity != nil &&
solrCloud.Spec.SolrSecurity.ProbesRequireAuth &&
solrCloud.Spec.SolrSecurity.AuthenticationType == solr.Basic {
usernameFile := fmt.Sprintf("%s/%s", mountPath,
corev1.BasicAuthUsernameKey)
passwordFile := fmt.Sprintf("%s/%s", mountPath,
corev1.BasicAuthPasswordKey)
- javaToolOptions = append(javaToolOptions,
- fmt.Sprintf("-Dbasicauth=$(cat %s):$(cat %s)",
usernameFile, passwordFile),
-
"-Dsolr.httpclient.builder.factory=org.apache.solr.client.solrj.impl.PreemptiveBasicAuthClientBuilderFactory",
- )
+ if isSolr10 {
Review Comment:
Instead of each class creating a `isSolr10` variable to check everywhere, it
would be more generic and fewer lines to just do the version comparison logic
in the conditional logic, e.g.
```go
if solrCloud.versionGte(10) { // versionLt(), versionEq()
```
##########
helm/solr/README.md:
##########
@@ -82,7 +82,7 @@ Descriptions on how to use these options can be found in the
[SolrCloud document
| nameOverride | string | `""` | |
| replicas | int | `3` | The number of Solr pods to run in the Solr Cloud. If
you want to use autoScaling, do not set this field. |
| image.repository | string | `"solr"` | The repository of the Solr image |
-| image.tag | string | `"9.10.0"` | The tag/version of Solr to run |
+| image.tag | string | `"9.10.0"` | The tag/version of Solr to run. Must start
with the Solr version, e.g. `9.10.0`, `10.0.0-slim` or `9.10`; tags like
`latest` are not supported by the Solr Operator. |
Review Comment:
Shorten this in the table, the details of the requirements are discussed
elsewhere
```suggestion
| image.tag | string | `"9.10.0"` | The tag/version of Solr to run. Must
start with the Solr version, e.g. `9`, `9.10` or `10.0.0-slim` |
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
