You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/01/21 18:38:26 UTC
[GitHub] [lucene-solr-operator] thelabdude opened a new pull request #193: Support custom log4j2 config from user-provided ConfigMap
thelabdude opened a new pull request #193:
URL: https://github.com/apache/lucene-solr-operator/pull/193
Fixes: https://github.com/apache/lucene-solr-operator/issues/192
SolrCloud controller now looks for the presence of a `log4j2.xml` key in the `instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap`
The only tricky aspect of this PR is we don't save the md5 in an annotation if the provided Log4j2 config includes `monitorInterval=` as we don't need to trigger a restart of the Solr pods as Log4j2 will reload the config at runtime. If the config does not include `monitorInterval`, then we save the md5 hash of the XML in an annotation on the pod spec to trigger a restart as we do for solr.xml
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr-operator] HoustonPutman commented on a change in pull request #193: Support custom log4j2 config from user-provided ConfigMap
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on a change in pull request #193:
URL: https://github.com/apache/lucene-solr-operator/pull/193#discussion_r566232749
##########
File path: controllers/solrcloud_controller.go
##########
@@ -182,44 +182,61 @@ func (r *SolrCloudReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
}
}
- // Generate ConfigMap unless the user supplied a custom ConfigMap for solr.xml ... but the provided ConfigMap
- // might be for the Prometheus exporter, so we only care if they provide a solr.xml in the CM
- solrXmlConfigMapName := instance.ConfigMapName()
- solrXmlMd5 := ""
+ // Generate ConfigMap unless the user supplied a custom ConfigMap for solr.xml
+ configMapInfo := make(map[string]string)
if instance.Spec.CustomSolrKubeOptions.ConfigMapOptions != nil && instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap != "" {
+ providedConfigMapName := instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap
foundConfigMap := &corev1.ConfigMap{}
- nn := types.NamespacedName{Name: instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap, Namespace: instance.Namespace}
+ nn := types.NamespacedName{Name: providedConfigMapName, Namespace: instance.Namespace}
err = r.Get(context.TODO(), nn, foundConfigMap)
if err != nil {
return requeueOrNot, err // if they passed a providedConfigMap name, then it must exist
}
- // ConfigMap doesn't have to have a solr.xml, but if it does, then it needs to be valid!
if foundConfigMap.Data != nil {
- solrXml, ok := foundConfigMap.Data["solr.xml"]
- if ok {
+ logXml, hasLogXml := foundConfigMap.Data[util.LogXmlFile]
+ solrXml, hasSolrXml := foundConfigMap.Data[util.SolrXmlFile]
+
+ // if there's a user-provided config, it must have one of the expected keys
+ if !hasLogXml && !hasSolrXml {
+ return requeueOrNot, fmt.Errorf("User provided ConfigMap %s must have one of 'solr.xml' and/or 'log4j2.xml'",
+ providedConfigMapName)
+ }
+
+ if hasSolrXml {
+ // make sure the user-provided solr.xml is valid
if !strings.Contains(solrXml, "${hostPort:") {
return requeueOrNot,
fmt.Errorf("Custom solr.xml in ConfigMap %s must contain a placeholder for the 'hostPort' variable, such as <int name=\"hostPort\">${hostPort:80}</int>",
- instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap)
+ providedConfigMapName)
}
// stored in the pod spec annotations on the statefulset so that we get a restart when solr.xml changes
- solrXmlMd5 = fmt.Sprintf("%x", md5.Sum([]byte(solrXml)))
- solrXmlConfigMapName = foundConfigMap.Name
- } else {
- return requeueOrNot, fmt.Errorf("Required 'solr.xml' key not found in provided ConfigMap %s",
- instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap)
+ configMapInfo[util.SolrXmlMd5Annotation] = fmt.Sprintf("%x", md5.Sum([]byte(solrXml)))
+ configMapInfo[util.SolrXmlFile] = foundConfigMap.Name
+ }
+
+ if hasLogXml {
+ if !strings.Contains(logXml, "monitorInterval=") {
+ // stored in the pod spec annotations on the statefulset so that we get a restart when the log config changes
+ configMapInfo[util.LogXmlMd5Annotation] = fmt.Sprintf("%x", md5.Sum([]byte(logXml)))
+ } // else log4j will automatically refresh for us, so no restart needed
+ configMapInfo[util.LogXmlFile] = foundConfigMap.Name
}
+
} else {
- return requeueOrNot, fmt.Errorf("Provided ConfigMap %s has no data",
- instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap)
+ return requeueOrNot, fmt.Errorf("Provided ConfigMap %s has no data", providedConfigMapName)
}
- } else {
+ }
+
+ if configMapInfo[util.SolrXmlFile] == "" {
Review comment:
Yeah, that sounds great. Just wanted to make sure I understood correctly.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr-operator] thelabdude commented on a change in pull request #193: Support custom log4j2 config from user-provided ConfigMap
Posted by GitBox <gi...@apache.org>.
thelabdude commented on a change in pull request #193:
URL: https://github.com/apache/lucene-solr-operator/pull/193#discussion_r566235249
##########
File path: controllers/solrcloud_controller.go
##########
@@ -182,44 +182,61 @@ func (r *SolrCloudReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
}
}
- // Generate ConfigMap unless the user supplied a custom ConfigMap for solr.xml ... but the provided ConfigMap
- // might be for the Prometheus exporter, so we only care if they provide a solr.xml in the CM
- solrXmlConfigMapName := instance.ConfigMapName()
- solrXmlMd5 := ""
+ // Generate ConfigMap unless the user supplied a custom ConfigMap for solr.xml
+ configMapInfo := make(map[string]string)
if instance.Spec.CustomSolrKubeOptions.ConfigMapOptions != nil && instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap != "" {
+ providedConfigMapName := instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap
foundConfigMap := &corev1.ConfigMap{}
- nn := types.NamespacedName{Name: instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap, Namespace: instance.Namespace}
+ nn := types.NamespacedName{Name: providedConfigMapName, Namespace: instance.Namespace}
err = r.Get(context.TODO(), nn, foundConfigMap)
if err != nil {
return requeueOrNot, err // if they passed a providedConfigMap name, then it must exist
}
- // ConfigMap doesn't have to have a solr.xml, but if it does, then it needs to be valid!
if foundConfigMap.Data != nil {
- solrXml, ok := foundConfigMap.Data["solr.xml"]
- if ok {
+ logXml, hasLogXml := foundConfigMap.Data[util.LogXmlFile]
+ solrXml, hasSolrXml := foundConfigMap.Data[util.SolrXmlFile]
+
+ // if there's a user-provided config, it must have one of the expected keys
+ if !hasLogXml && !hasSolrXml {
+ return requeueOrNot, fmt.Errorf("User provided ConfigMap %s must have one of 'solr.xml' and/or 'log4j2.xml'",
+ providedConfigMapName)
+ }
+
+ if hasSolrXml {
+ // make sure the user-provided solr.xml is valid
if !strings.Contains(solrXml, "${hostPort:") {
return requeueOrNot,
fmt.Errorf("Custom solr.xml in ConfigMap %s must contain a placeholder for the 'hostPort' variable, such as <int name=\"hostPort\">${hostPort:80}</int>",
- instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap)
+ providedConfigMapName)
}
// stored in the pod spec annotations on the statefulset so that we get a restart when solr.xml changes
- solrXmlMd5 = fmt.Sprintf("%x", md5.Sum([]byte(solrXml)))
- solrXmlConfigMapName = foundConfigMap.Name
- } else {
- return requeueOrNot, fmt.Errorf("Required 'solr.xml' key not found in provided ConfigMap %s",
- instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap)
+ configMapInfo[util.SolrXmlMd5Annotation] = fmt.Sprintf("%x", md5.Sum([]byte(solrXml)))
+ configMapInfo[util.SolrXmlFile] = foundConfigMap.Name
+ }
+
+ if hasLogXml {
+ if !strings.Contains(logXml, "monitorInterval=") {
+ // stored in the pod spec annotations on the statefulset so that we get a restart when the log config changes
+ configMapInfo[util.LogXmlMd5Annotation] = fmt.Sprintf("%x", md5.Sum([]byte(logXml)))
+ } // else log4j will automatically refresh for us, so no restart needed
+ configMapInfo[util.LogXmlFile] = foundConfigMap.Name
}
+
} else {
- return requeueOrNot, fmt.Errorf("Provided ConfigMap %s has no data",
- instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap)
+ return requeueOrNot, fmt.Errorf("Provided ConfigMap %s has no data", providedConfigMapName)
}
- } else {
+ }
+
+ if configMapInfo[util.SolrXmlFile] == "" {
Review comment:
ok cool ... I switched to keeping all the user-provided ConfigMap info in a single map arg to `GenerateStatefulSet` vs. having multiple function args just to keep things tidy.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr-operator] thelabdude commented on a change in pull request #193: Support custom log4j2 config from user-provided ConfigMap
Posted by GitBox <gi...@apache.org>.
thelabdude commented on a change in pull request #193:
URL: https://github.com/apache/lucene-solr-operator/pull/193#discussion_r566236753
##########
File path: controllers/suite_test.go
##########
@@ -76,6 +78,9 @@ func SetupTestReconcile(inner reconcile.Reconciler) (reconcile.Reconciler, chan
requests := make(chan reconcile.Request)
fn := reconcile.Func(func(req reconcile.Request) (reconcile.Result, error) {
result, err := inner.Reconcile(req)
+ if err != nil {
+ fmt.Printf("\n\nReconcile Error: %s\n\n", err)
Review comment:
this is a bit ugly but so useful in tracking down errors during reconcile that are returned like this:
```
if !strings.Contains(solrXml, "${hostPort:") {
return requeueOrNot,
fmt.Errorf("Custom solr.xml in ConfigMap %s must contain a placeholder for the 'hostPort' variable, such as <int name=\"hostPort\">${hostPort:80}</int>",
providedConfigMapName)
}
```
W/o this change, the error isn't reported and the test just fails with reconcile timeouts. I think it's a useful addition.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr-operator] thelabdude commented on a change in pull request #193: Support custom log4j2 config from user-provided ConfigMap
Posted by GitBox <gi...@apache.org>.
thelabdude commented on a change in pull request #193:
URL: https://github.com/apache/lucene-solr-operator/pull/193#discussion_r564072280
##########
File path: controllers/util/solr_util.go
##########
@@ -327,13 +330,42 @@ func GenerateStatefulSet(solrCloud *solr.SolrCloud, solrCloudStatus *solr.SolrCl
envVars = append(envVars, customPodOptions.EnvVariables...)
}
+ // Did the user provide a custom log config?
+ if configMapInfo[LogXmlFile] != "" {
+
+ if configMapInfo[LogXmlMd5Annotation] != "" {
+ if podAnnotations == nil {
+ podAnnotations = make(map[string]string, 1)
+ }
+ podAnnotations[LogXmlMd5Annotation] = configMapInfo[LogXmlMd5Annotation]
+ }
+
+ // cannot use /var/solr as a mountPath, so mount the custom log config in a sub-dir
+ volName := "log4j2-xml"
+ mountPath := fmt.Sprintf("/var/solr/%s-log-config", solrCloud.Name)
+ log4jPropsEnvVarPath := fmt.Sprintf("%s/%s", mountPath, LogXmlFile)
+
+ solrVolumes = append(solrVolumes, corev1.Volume{
Review comment:
This is a good catch! K8s allows it and results in a structure like the following in the STS:
```
volumes:
- configMap:
defaultMode: 420
items:
- key: solr.xml
path: solr.xml
name: dev-custom-solr-xml
name: solr-xml
- configMap:
defaultMode: 420
items:
- key: log4j2.xml
path: log4j2.xml
name: dev-custom-solr-xml
name: log4j2-xml
```
But we certainly should use a single volume with multiple `items` as you suggest ;-) Will fix it up and add a test for both keys being provided.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr-operator] thelabdude commented on a change in pull request #193: Support custom log4j2 config from user-provided ConfigMap
Posted by GitBox <gi...@apache.org>.
thelabdude commented on a change in pull request #193:
URL: https://github.com/apache/lucene-solr-operator/pull/193#discussion_r566184234
##########
File path: controllers/solrcloud_controller.go
##########
@@ -182,44 +182,61 @@ func (r *SolrCloudReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
}
}
- // Generate ConfigMap unless the user supplied a custom ConfigMap for solr.xml ... but the provided ConfigMap
- // might be for the Prometheus exporter, so we only care if they provide a solr.xml in the CM
- solrXmlConfigMapName := instance.ConfigMapName()
- solrXmlMd5 := ""
+ // Generate ConfigMap unless the user supplied a custom ConfigMap for solr.xml
+ configMapInfo := make(map[string]string)
if instance.Spec.CustomSolrKubeOptions.ConfigMapOptions != nil && instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap != "" {
+ providedConfigMapName := instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap
foundConfigMap := &corev1.ConfigMap{}
- nn := types.NamespacedName{Name: instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap, Namespace: instance.Namespace}
+ nn := types.NamespacedName{Name: providedConfigMapName, Namespace: instance.Namespace}
err = r.Get(context.TODO(), nn, foundConfigMap)
if err != nil {
return requeueOrNot, err // if they passed a providedConfigMap name, then it must exist
}
- // ConfigMap doesn't have to have a solr.xml, but if it does, then it needs to be valid!
if foundConfigMap.Data != nil {
- solrXml, ok := foundConfigMap.Data["solr.xml"]
- if ok {
+ logXml, hasLogXml := foundConfigMap.Data[util.LogXmlFile]
+ solrXml, hasSolrXml := foundConfigMap.Data[util.SolrXmlFile]
+
+ // if there's a user-provided config, it must have one of the expected keys
+ if !hasLogXml && !hasSolrXml {
+ return requeueOrNot, fmt.Errorf("User provided ConfigMap %s must have one of 'solr.xml' and/or 'log4j2.xml'",
+ providedConfigMapName)
+ }
+
+ if hasSolrXml {
+ // make sure the user-provided solr.xml is valid
if !strings.Contains(solrXml, "${hostPort:") {
return requeueOrNot,
fmt.Errorf("Custom solr.xml in ConfigMap %s must contain a placeholder for the 'hostPort' variable, such as <int name=\"hostPort\">${hostPort:80}</int>",
- instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap)
+ providedConfigMapName)
}
// stored in the pod spec annotations on the statefulset so that we get a restart when solr.xml changes
- solrXmlMd5 = fmt.Sprintf("%x", md5.Sum([]byte(solrXml)))
- solrXmlConfigMapName = foundConfigMap.Name
- } else {
- return requeueOrNot, fmt.Errorf("Required 'solr.xml' key not found in provided ConfigMap %s",
- instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap)
+ configMapInfo[util.SolrXmlMd5Annotation] = fmt.Sprintf("%x", md5.Sum([]byte(solrXml)))
+ configMapInfo[util.SolrXmlFile] = foundConfigMap.Name
+ }
+
+ if hasLogXml {
+ if !strings.Contains(logXml, "monitorInterval=") {
+ // stored in the pod spec annotations on the statefulset so that we get a restart when the log config changes
+ configMapInfo[util.LogXmlMd5Annotation] = fmt.Sprintf("%x", md5.Sum([]byte(logXml)))
+ } // else log4j will automatically refresh for us, so no restart needed
+ configMapInfo[util.LogXmlFile] = foundConfigMap.Name
}
+
} else {
- return requeueOrNot, fmt.Errorf("Provided ConfigMap %s has no data",
- instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap)
+ return requeueOrNot, fmt.Errorf("Provided ConfigMap %s has no data", providedConfigMapName)
}
- } else {
+ }
+
+ if configMapInfo[util.SolrXmlFile] == "" {
Review comment:
If the user-provided ConfigMap only contains a key for the log4j2.xml, then the operator will create the default ConfigMap for solr.xml, resulting in this structure in the STS for a SolrCloud named `dev`:
```
volumes:
- configMap:
defaultMode: 420
items:
- key: solr.xml
path: solr.xml
name: dev-solrcloud-configmap
name: solr-xml
```
The `if configMapInfo[util.SolrXmlFile] == "" {` is just how we know there wasn't a `solr.xml` in the user-provided ConfigMap at that point. Does that sound right?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr-operator] thelabdude commented on a change in pull request #193: Support custom log4j2 config from user-provided ConfigMap
Posted by GitBox <gi...@apache.org>.
thelabdude commented on a change in pull request #193:
URL: https://github.com/apache/lucene-solr-operator/pull/193#discussion_r564072280
##########
File path: controllers/util/solr_util.go
##########
@@ -327,13 +330,42 @@ func GenerateStatefulSet(solrCloud *solr.SolrCloud, solrCloudStatus *solr.SolrCl
envVars = append(envVars, customPodOptions.EnvVariables...)
}
+ // Did the user provide a custom log config?
+ if configMapInfo[LogXmlFile] != "" {
+
+ if configMapInfo[LogXmlMd5Annotation] != "" {
+ if podAnnotations == nil {
+ podAnnotations = make(map[string]string, 1)
+ }
+ podAnnotations[LogXmlMd5Annotation] = configMapInfo[LogXmlMd5Annotation]
+ }
+
+ // cannot use /var/solr as a mountPath, so mount the custom log config in a sub-dir
+ volName := "log4j2-xml"
+ mountPath := fmt.Sprintf("/var/solr/%s-log-config", solrCloud.Name)
+ log4jPropsEnvVarPath := fmt.Sprintf("%s/%s", mountPath, LogXmlFile)
+
+ solrVolumes = append(solrVolumes, corev1.Volume{
Review comment:
This is a good catch! K8s allows it and results in a structure like the following in the STS:
```
volumes:
- configMap:
defaultMode: 420
items:
- key: solr.xml
path: solr.xml
name: dev-custom-solr-xml
name: solr-xml
- configMap:
defaultMode: 420
items:
- key: log4j2.xml
path: log4j2.xml
name: dev-custom-solr-xml
name: log4j2-xml
```
But we certainly should use a single volume with multiple `items` as you suggest ;-) Will fix it up and add a test for both keys being provided.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr-operator] thelabdude commented on a change in pull request #193: Support custom log4j2 config from user-provided ConfigMap
Posted by GitBox <gi...@apache.org>.
thelabdude commented on a change in pull request #193:
URL: https://github.com/apache/lucene-solr-operator/pull/193#discussion_r566340100
##########
File path: controllers/solrcloud_controller.go
##########
@@ -182,44 +182,61 @@ func (r *SolrCloudReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
}
}
- // Generate ConfigMap unless the user supplied a custom ConfigMap for solr.xml ... but the provided ConfigMap
- // might be for the Prometheus exporter, so we only care if they provide a solr.xml in the CM
- solrXmlConfigMapName := instance.ConfigMapName()
- solrXmlMd5 := ""
+ // Generate ConfigMap unless the user supplied a custom ConfigMap for solr.xml
+ configMapInfo := make(map[string]string)
if instance.Spec.CustomSolrKubeOptions.ConfigMapOptions != nil && instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap != "" {
+ providedConfigMapName := instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap
foundConfigMap := &corev1.ConfigMap{}
- nn := types.NamespacedName{Name: instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap, Namespace: instance.Namespace}
+ nn := types.NamespacedName{Name: providedConfigMapName, Namespace: instance.Namespace}
err = r.Get(context.TODO(), nn, foundConfigMap)
if err != nil {
return requeueOrNot, err // if they passed a providedConfigMap name, then it must exist
}
- // ConfigMap doesn't have to have a solr.xml, but if it does, then it needs to be valid!
if foundConfigMap.Data != nil {
- solrXml, ok := foundConfigMap.Data["solr.xml"]
- if ok {
+ logXml, hasLogXml := foundConfigMap.Data[util.LogXmlFile]
+ solrXml, hasSolrXml := foundConfigMap.Data[util.SolrXmlFile]
+
+ // if there's a user-provided config, it must have one of the expected keys
+ if !hasLogXml && !hasSolrXml {
+ return requeueOrNot, fmt.Errorf("User provided ConfigMap %s must have one of 'solr.xml' and/or 'log4j2.xml'",
Review comment:
Can you point me to an example elsewhere in the code for what you're looking for here?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr-operator] thelabdude merged pull request #193: Support custom log4j2 config from user-provided ConfigMap
Posted by GitBox <gi...@apache.org>.
thelabdude merged pull request #193:
URL: https://github.com/apache/lucene-solr-operator/pull/193
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr-operator] HoustonPutman commented on a change in pull request #193: Support custom log4j2 config from user-provided ConfigMap
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on a change in pull request #193:
URL: https://github.com/apache/lucene-solr-operator/pull/193#discussion_r564044525
##########
File path: controllers/solrcloud_controller.go
##########
@@ -182,44 +182,61 @@ func (r *SolrCloudReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
}
}
- // Generate ConfigMap unless the user supplied a custom ConfigMap for solr.xml ... but the provided ConfigMap
- // might be for the Prometheus exporter, so we only care if they provide a solr.xml in the CM
- solrXmlConfigMapName := instance.ConfigMapName()
- solrXmlMd5 := ""
+ // Generate ConfigMap unless the user supplied a custom ConfigMap for solr.xml
+ configMapInfo := make(map[string]string)
if instance.Spec.CustomSolrKubeOptions.ConfigMapOptions != nil && instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap != "" {
+ providedConfigMapName := instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap
foundConfigMap := &corev1.ConfigMap{}
- nn := types.NamespacedName{Name: instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap, Namespace: instance.Namespace}
+ nn := types.NamespacedName{Name: providedConfigMapName, Namespace: instance.Namespace}
err = r.Get(context.TODO(), nn, foundConfigMap)
if err != nil {
return requeueOrNot, err // if they passed a providedConfigMap name, then it must exist
}
- // ConfigMap doesn't have to have a solr.xml, but if it does, then it needs to be valid!
if foundConfigMap.Data != nil {
- solrXml, ok := foundConfigMap.Data["solr.xml"]
- if ok {
+ logXml, hasLogXml := foundConfigMap.Data[util.LogXmlFile]
+ solrXml, hasSolrXml := foundConfigMap.Data[util.SolrXmlFile]
+
+ // if there's a user-provided config, it must have one of the expected keys
+ if !hasLogXml && !hasSolrXml {
+ return requeueOrNot, fmt.Errorf("User provided ConfigMap %s must have one of 'solr.xml' and/or 'log4j2.xml'",
+ providedConfigMapName)
+ }
+
+ if hasSolrXml {
+ // make sure the user-provided solr.xml is valid
if !strings.Contains(solrXml, "${hostPort:") {
return requeueOrNot,
fmt.Errorf("Custom solr.xml in ConfigMap %s must contain a placeholder for the 'hostPort' variable, such as <int name=\"hostPort\">${hostPort:80}</int>",
- instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap)
+ providedConfigMapName)
}
// stored in the pod spec annotations on the statefulset so that we get a restart when solr.xml changes
- solrXmlMd5 = fmt.Sprintf("%x", md5.Sum([]byte(solrXml)))
- solrXmlConfigMapName = foundConfigMap.Name
- } else {
- return requeueOrNot, fmt.Errorf("Required 'solr.xml' key not found in provided ConfigMap %s",
- instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap)
+ configMapInfo[util.SolrXmlMd5Annotation] = fmt.Sprintf("%x", md5.Sum([]byte(solrXml)))
+ configMapInfo[util.SolrXmlFile] = foundConfigMap.Name
+ }
+
+ if hasLogXml {
+ if !strings.Contains(logXml, "monitorInterval=") {
+ // stored in the pod spec annotations on the statefulset so that we get a restart when the log config changes
+ configMapInfo[util.LogXmlMd5Annotation] = fmt.Sprintf("%x", md5.Sum([]byte(logXml)))
+ } // else log4j will automatically refresh for us, so no restart needed
+ configMapInfo[util.LogXmlFile] = foundConfigMap.Name
}
+
} else {
- return requeueOrNot, fmt.Errorf("Provided ConfigMap %s has no data",
- instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap)
+ return requeueOrNot, fmt.Errorf("Provided ConfigMap %s has no data", providedConfigMapName)
}
- } else {
+ }
+
+ if configMapInfo[util.SolrXmlFile] == "" {
Review comment:
So if a user passes a custom configMap that just contains a log4j.xml, the operator will still create a configMap for the xml, correct?
##########
File path: controllers/util/solr_util.go
##########
@@ -327,13 +330,42 @@ func GenerateStatefulSet(solrCloud *solr.SolrCloud, solrCloudStatus *solr.SolrCl
envVars = append(envVars, customPodOptions.EnvVariables...)
}
+ // Did the user provide a custom log config?
+ if configMapInfo[LogXmlFile] != "" {
+
+ if configMapInfo[LogXmlMd5Annotation] != "" {
+ if podAnnotations == nil {
+ podAnnotations = make(map[string]string, 1)
+ }
+ podAnnotations[LogXmlMd5Annotation] = configMapInfo[LogXmlMd5Annotation]
+ }
+
+ // cannot use /var/solr as a mountPath, so mount the custom log config in a sub-dir
+ volName := "log4j2-xml"
+ mountPath := fmt.Sprintf("/var/solr/%s-log-config", solrCloud.Name)
+ log4jPropsEnvVarPath := fmt.Sprintf("%s/%s", mountPath, LogXmlFile)
+
+ solrVolumes = append(solrVolumes, corev1.Volume{
Review comment:
So if the custom configMap contains both a Solr.xml and a log4j.xml, then this will add 2 volumes, using the same configMap, correct?
Is this supported in Kube? I think there might be an issue. But if you've tested it, then maybe it's ok to go.
It would be pretty easy to make sure that the volume is only added once, and it has the correct `items`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr-operator] HoustonPutman commented on a change in pull request #193: Support custom log4j2 config from user-provided ConfigMap
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on a change in pull request #193:
URL: https://github.com/apache/lucene-solr-operator/pull/193#discussion_r566367646
##########
File path: controllers/solrcloud_controller.go
##########
@@ -182,44 +182,61 @@ func (r *SolrCloudReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
}
}
- // Generate ConfigMap unless the user supplied a custom ConfigMap for solr.xml ... but the provided ConfigMap
- // might be for the Prometheus exporter, so we only care if they provide a solr.xml in the CM
- solrXmlConfigMapName := instance.ConfigMapName()
- solrXmlMd5 := ""
+ // Generate ConfigMap unless the user supplied a custom ConfigMap for solr.xml
+ configMapInfo := make(map[string]string)
if instance.Spec.CustomSolrKubeOptions.ConfigMapOptions != nil && instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap != "" {
+ providedConfigMapName := instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap
foundConfigMap := &corev1.ConfigMap{}
- nn := types.NamespacedName{Name: instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap, Namespace: instance.Namespace}
+ nn := types.NamespacedName{Name: providedConfigMapName, Namespace: instance.Namespace}
err = r.Get(context.TODO(), nn, foundConfigMap)
if err != nil {
return requeueOrNot, err // if they passed a providedConfigMap name, then it must exist
}
- // ConfigMap doesn't have to have a solr.xml, but if it does, then it needs to be valid!
if foundConfigMap.Data != nil {
- solrXml, ok := foundConfigMap.Data["solr.xml"]
- if ok {
+ logXml, hasLogXml := foundConfigMap.Data[util.LogXmlFile]
+ solrXml, hasSolrXml := foundConfigMap.Data[util.SolrXmlFile]
+
+ // if there's a user-provided config, it must have one of the expected keys
+ if !hasLogXml && !hasSolrXml {
+ return requeueOrNot, fmt.Errorf("User provided ConfigMap %s must have one of 'solr.xml' and/or 'log4j2.xml'",
Review comment:
https://github.com/apache/lucene-solr-operator/blob/main/controllers/solrcloud_controller.go#L330
Just a simple thing for us to look for when implementing events.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr-operator] HoustonPutman commented on a change in pull request #193: Support custom log4j2 config from user-provided ConfigMap
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on a change in pull request #193:
URL: https://github.com/apache/lucene-solr-operator/pull/193#discussion_r564044525
##########
File path: controllers/solrcloud_controller.go
##########
@@ -182,44 +182,61 @@ func (r *SolrCloudReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
}
}
- // Generate ConfigMap unless the user supplied a custom ConfigMap for solr.xml ... but the provided ConfigMap
- // might be for the Prometheus exporter, so we only care if they provide a solr.xml in the CM
- solrXmlConfigMapName := instance.ConfigMapName()
- solrXmlMd5 := ""
+ // Generate ConfigMap unless the user supplied a custom ConfigMap for solr.xml
+ configMapInfo := make(map[string]string)
if instance.Spec.CustomSolrKubeOptions.ConfigMapOptions != nil && instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap != "" {
+ providedConfigMapName := instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap
foundConfigMap := &corev1.ConfigMap{}
- nn := types.NamespacedName{Name: instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap, Namespace: instance.Namespace}
+ nn := types.NamespacedName{Name: providedConfigMapName, Namespace: instance.Namespace}
err = r.Get(context.TODO(), nn, foundConfigMap)
if err != nil {
return requeueOrNot, err // if they passed a providedConfigMap name, then it must exist
}
- // ConfigMap doesn't have to have a solr.xml, but if it does, then it needs to be valid!
if foundConfigMap.Data != nil {
- solrXml, ok := foundConfigMap.Data["solr.xml"]
- if ok {
+ logXml, hasLogXml := foundConfigMap.Data[util.LogXmlFile]
+ solrXml, hasSolrXml := foundConfigMap.Data[util.SolrXmlFile]
+
+ // if there's a user-provided config, it must have one of the expected keys
+ if !hasLogXml && !hasSolrXml {
+ return requeueOrNot, fmt.Errorf("User provided ConfigMap %s must have one of 'solr.xml' and/or 'log4j2.xml'",
+ providedConfigMapName)
+ }
+
+ if hasSolrXml {
+ // make sure the user-provided solr.xml is valid
if !strings.Contains(solrXml, "${hostPort:") {
return requeueOrNot,
fmt.Errorf("Custom solr.xml in ConfigMap %s must contain a placeholder for the 'hostPort' variable, such as <int name=\"hostPort\">${hostPort:80}</int>",
- instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap)
+ providedConfigMapName)
}
// stored in the pod spec annotations on the statefulset so that we get a restart when solr.xml changes
- solrXmlMd5 = fmt.Sprintf("%x", md5.Sum([]byte(solrXml)))
- solrXmlConfigMapName = foundConfigMap.Name
- } else {
- return requeueOrNot, fmt.Errorf("Required 'solr.xml' key not found in provided ConfigMap %s",
- instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap)
+ configMapInfo[util.SolrXmlMd5Annotation] = fmt.Sprintf("%x", md5.Sum([]byte(solrXml)))
+ configMapInfo[util.SolrXmlFile] = foundConfigMap.Name
+ }
+
+ if hasLogXml {
+ if !strings.Contains(logXml, "monitorInterval=") {
+ // stored in the pod spec annotations on the statefulset so that we get a restart when the log config changes
+ configMapInfo[util.LogXmlMd5Annotation] = fmt.Sprintf("%x", md5.Sum([]byte(logXml)))
+ } // else log4j will automatically refresh for us, so no restart needed
+ configMapInfo[util.LogXmlFile] = foundConfigMap.Name
}
+
} else {
- return requeueOrNot, fmt.Errorf("Provided ConfigMap %s has no data",
- instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap)
+ return requeueOrNot, fmt.Errorf("Provided ConfigMap %s has no data", providedConfigMapName)
}
- } else {
+ }
+
+ if configMapInfo[util.SolrXmlFile] == "" {
Review comment:
So if a user passes a custom configMap that just contains a log4j.xml, the operator will still create a configMap for the xml, correct?
##########
File path: controllers/util/solr_util.go
##########
@@ -327,13 +330,42 @@ func GenerateStatefulSet(solrCloud *solr.SolrCloud, solrCloudStatus *solr.SolrCl
envVars = append(envVars, customPodOptions.EnvVariables...)
}
+ // Did the user provide a custom log config?
+ if configMapInfo[LogXmlFile] != "" {
+
+ if configMapInfo[LogXmlMd5Annotation] != "" {
+ if podAnnotations == nil {
+ podAnnotations = make(map[string]string, 1)
+ }
+ podAnnotations[LogXmlMd5Annotation] = configMapInfo[LogXmlMd5Annotation]
+ }
+
+ // cannot use /var/solr as a mountPath, so mount the custom log config in a sub-dir
+ volName := "log4j2-xml"
+ mountPath := fmt.Sprintf("/var/solr/%s-log-config", solrCloud.Name)
+ log4jPropsEnvVarPath := fmt.Sprintf("%s/%s", mountPath, LogXmlFile)
+
+ solrVolumes = append(solrVolumes, corev1.Volume{
Review comment:
So if the custom configMap contains both a Solr.xml and a log4j.xml, then this will add 2 volumes, using the same configMap, correct?
Is this supported in Kube? I think there might be an issue. But if you've tested it, then maybe it's ok to go.
It would be pretty easy to make sure that the volume is only added once, and it has the correct `items`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr-operator] HoustonPutman commented on a change in pull request #193: Support custom log4j2 config from user-provided ConfigMap
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on a change in pull request #193:
URL: https://github.com/apache/lucene-solr-operator/pull/193#discussion_r566324592
##########
File path: docs/solr-cloud/solr-cloud-crd.md
##########
@@ -127,4 +127,118 @@ the solr-operator can manage Zookeeper ensemble(s) for you.
Using the [zookeeper-operator](https://github.com/pravega/zookeeper-operator), a new Zookeeper ensemble can be spun up for
each solrCloud that has this option specified.
-The startup parameter `zookeeper-operator` must be provided on startup of the solr-operator for this parameter to be available.
\ No newline at end of file
+The startup parameter `zookeeper-operator` must be provided on startup of the solr-operator for this parameter to be available.
+
+## Override Built-in Solr Configuration Files
+
+The Solr operator deploys well-configured SolrCloud instances with minimal input required from human operators.
+As such, the operator installs various configuration files automatically, including `solr.xml` for node-level settings and `log4j2.xml` for logging.
+However, there may come a time when you need to override the built-in configuration files with custom settings.
+
+In general, users can provide custom config files by providing a ConfigMap in the same namespace as the SolrCloud instance;
+all custom config files should be stored in the same user-provided ConfigMap under different keys.
+Point your SolrCloud definition to a user-provided ConfigMap using the following structure:
+```
+spec:
+ ...
+ customSolrKubeOptions:
+ configMapOptions:
+ providedConfigMap: <Custom-ConfigMap-Here>
+```
+
+### Custom solr.xml
+
+Solr pods load node-level configuration settings from `/var/solr/data/solr.xml`.
+This important configuration file gets created by the `cp-solr-xml` initContainer which bootstraps the `solr.home` directory on each pod before starting the main container.
+The default `solr.xml` is mounted into the `cp-solr-xml` initContainer from a ConfigMap named `<INSTANCE>-solrcloud-configmap` (where `<INSTANCE>` is the name of your SolrCloud instance) created by the Solr operator.
+
+_Note: The data in the default ConfigMap is not editable! Any changes to the `solr.xml` in the default ConfigMap created by the operator will be overwritten during the next reconcile cycle._
+
+Many of the specific values in `solr.xml` can be set using Java system properties; for instance, the following setting controls the read timeout for the HTTP client used by Solr's `HttpShardHandlerFactory`:
+```
+<int name="socketTimeout">${socketTimeout:600000}</int>
+```
+The `${socketTimeout:600000}` syntax means pull the value from a Java system property named `socketTimeout` with default `600000` if not set.
+
+You can set Java system properties using the `solrOpts` string in your SolrCloud definition, such as:
+```
+spec:
+ solrOpts: -DsocketTimeout=300000
+```
+This same approach works for a number of settings in `solrconfig.xml` as well.
+
+However, if you need to customize `solr.xml` beyond what can be accomplished with Java system properties,
+then you need to supply your own `solr.xml` in a ConfigMap in the same namespace where you deploy your SolrCloud instance.
+Provide your custom XML in the ConfigMap using `solr.xml` as the key as shown in the example below:
+```
+---
+kind: ConfigMap
+apiVersion: v1
+metadata:
+ name: custom-solr-xml
+data:
+ solr.xml: |
+ <?xml version="1.0" encoding="UTF-8" ?>
+ <solr>
+ ... CUSTOM CONFIG HERE ...
+ </solr>
+```
+
+You can get the default `solr.xml` from a Solr pod as a starting point for creating a custom config using `kubectl cp` as shown in the example below:
+```
+SOLR_POD_ID=$(kubectl get pod -l technology=solr-cloud --no-headers -o custom-columns=":metadata.name" | head -1)
+kubectl cp $SOLR_POD_ID:/var/solr/data/solr.xml ./custom-solr.xml
+```
+This copies the default config from the first Solr pod found in the namespace and names it `custom-solr.xml`. Customize the settings in `custom-solr.xml` as needed and then create a ConfigMap using YAML.
+
+_Note: Using `kubectl create configmap --from-file` scrambles the XML formatting, so we recommend defining the configmap YAML as shown above to keep the XML formatted properly._
+
+Point your SolrCloud instance at the custom ConfigMap using:
+```
+ customSolrKubeOptions:
+ configMapOptions:
+ providedConfigMap: custom-solr-xml
+```
+
+#### Changes to Custom Config Trigger Rolling Restarts
+
+The Solr operator stores the MD5 hash of your custom XML in the StatefulSet's pod spec annotations (`spec.template.metadata.annotations`). To see the current annotations for your Solr pods, you can do:
+```
+kubectl annotate pod -l technology=solr-cloud --list=true
+```
+If the custom `solr.xml` changes in the user-provided ConfigMap, then the operator triggers a rolling restart of Solr pods to apply the updated configuration settings automatically.
+
+To summarize, if you need to customize `solr.xml`, provide your own version in a ConfigMap and changes made to the XML in the ConfigMap are automatically applied to your Solr pods.
+
+### Custom Log Configuration
+
+As with `solr.xml`, the operator configures Solr with a default Log4j2 configuration file: `-Dlog4j.configurationFile=/var/solr/log4j2.xml`.
Review comment:
Unlike with `solr.xml`, the operator uses the default log4j2 configuration file that ships with Solr.
##########
File path: docs/solr-cloud/solr-cloud-crd.md
##########
@@ -127,4 +127,118 @@ the solr-operator can manage Zookeeper ensemble(s) for you.
Using the [zookeeper-operator](https://github.com/pravega/zookeeper-operator), a new Zookeeper ensemble can be spun up for
each solrCloud that has this option specified.
-The startup parameter `zookeeper-operator` must be provided on startup of the solr-operator for this parameter to be available.
\ No newline at end of file
+The startup parameter `zookeeper-operator` must be provided on startup of the solr-operator for this parameter to be available.
+
+## Override Built-in Solr Configuration Files
+
+The Solr operator deploys well-configured SolrCloud instances with minimal input required from human operators.
+As such, the operator installs various configuration files automatically, including `solr.xml` for node-level settings and `log4j2.xml` for logging.
+However, there may come a time when you need to override the built-in configuration files with custom settings.
+
+In general, users can provide custom config files by providing a ConfigMap in the same namespace as the SolrCloud instance;
+all custom config files should be stored in the same user-provided ConfigMap under different keys.
+Point your SolrCloud definition to a user-provided ConfigMap using the following structure:
+```
+spec:
+ ...
+ customSolrKubeOptions:
+ configMapOptions:
+ providedConfigMap: <Custom-ConfigMap-Here>
+```
+
+### Custom solr.xml
+
+Solr pods load node-level configuration settings from `/var/solr/data/solr.xml`.
+This important configuration file gets created by the `cp-solr-xml` initContainer which bootstraps the `solr.home` directory on each pod before starting the main container.
+The default `solr.xml` is mounted into the `cp-solr-xml` initContainer from a ConfigMap named `<INSTANCE>-solrcloud-configmap` (where `<INSTANCE>` is the name of your SolrCloud instance) created by the Solr operator.
+
+_Note: The data in the default ConfigMap is not editable! Any changes to the `solr.xml` in the default ConfigMap created by the operator will be overwritten during the next reconcile cycle._
+
+Many of the specific values in `solr.xml` can be set using Java system properties; for instance, the following setting controls the read timeout for the HTTP client used by Solr's `HttpShardHandlerFactory`:
+```
+<int name="socketTimeout">${socketTimeout:600000}</int>
+```
+The `${socketTimeout:600000}` syntax means pull the value from a Java system property named `socketTimeout` with default `600000` if not set.
+
+You can set Java system properties using the `solrOpts` string in your SolrCloud definition, such as:
+```
+spec:
+ solrOpts: -DsocketTimeout=300000
+```
+This same approach works for a number of settings in `solrconfig.xml` as well.
+
+However, if you need to customize `solr.xml` beyond what can be accomplished with Java system properties,
+then you need to supply your own `solr.xml` in a ConfigMap in the same namespace where you deploy your SolrCloud instance.
+Provide your custom XML in the ConfigMap using `solr.xml` as the key as shown in the example below:
+```
+---
+kind: ConfigMap
+apiVersion: v1
+metadata:
+ name: custom-solr-xml
+data:
+ solr.xml: |
+ <?xml version="1.0" encoding="UTF-8" ?>
+ <solr>
+ ... CUSTOM CONFIG HERE ...
+ </solr>
+```
Review comment:
I would put a bolded statement here stating the need for a `<hostPort>${hostPort}</hostPort>` under the <solr><solrcloud> section.
##########
File path: docs/solr-cloud/solr-cloud-crd.md
##########
@@ -127,4 +127,118 @@ the solr-operator can manage Zookeeper ensemble(s) for you.
Using the [zookeeper-operator](https://github.com/pravega/zookeeper-operator), a new Zookeeper ensemble can be spun up for
each solrCloud that has this option specified.
-The startup parameter `zookeeper-operator` must be provided on startup of the solr-operator for this parameter to be available.
\ No newline at end of file
+The startup parameter `zookeeper-operator` must be provided on startup of the solr-operator for this parameter to be available.
+
+## Override Built-in Solr Configuration Files
+
+The Solr operator deploys well-configured SolrCloud instances with minimal input required from human operators.
+As such, the operator installs various configuration files automatically, including `solr.xml` for node-level settings and `log4j2.xml` for logging.
+However, there may come a time when you need to override the built-in configuration files with custom settings.
+
+In general, users can provide custom config files by providing a ConfigMap in the same namespace as the SolrCloud instance;
+all custom config files should be stored in the same user-provided ConfigMap under different keys.
+Point your SolrCloud definition to a user-provided ConfigMap using the following structure:
+```
+spec:
+ ...
+ customSolrKubeOptions:
+ configMapOptions:
+ providedConfigMap: <Custom-ConfigMap-Here>
+```
+
+### Custom solr.xml
+
+Solr pods load node-level configuration settings from `/var/solr/data/solr.xml`.
+This important configuration file gets created by the `cp-solr-xml` initContainer which bootstraps the `solr.home` directory on each pod before starting the main container.
+The default `solr.xml` is mounted into the `cp-solr-xml` initContainer from a ConfigMap named `<INSTANCE>-solrcloud-configmap` (where `<INSTANCE>` is the name of your SolrCloud instance) created by the Solr operator.
+
+_Note: The data in the default ConfigMap is not editable! Any changes to the `solr.xml` in the default ConfigMap created by the operator will be overwritten during the next reconcile cycle._
+
+Many of the specific values in `solr.xml` can be set using Java system properties; for instance, the following setting controls the read timeout for the HTTP client used by Solr's `HttpShardHandlerFactory`:
+```
+<int name="socketTimeout">${socketTimeout:600000}</int>
+```
+The `${socketTimeout:600000}` syntax means pull the value from a Java system property named `socketTimeout` with default `600000` if not set.
+
+You can set Java system properties using the `solrOpts` string in your SolrCloud definition, such as:
+```
+spec:
+ solrOpts: -DsocketTimeout=300000
+```
+This same approach works for a number of settings in `solrconfig.xml` as well.
+
+However, if you need to customize `solr.xml` beyond what can be accomplished with Java system properties,
+then you need to supply your own `solr.xml` in a ConfigMap in the same namespace where you deploy your SolrCloud instance.
+Provide your custom XML in the ConfigMap using `solr.xml` as the key as shown in the example below:
+```
+---
+kind: ConfigMap
+apiVersion: v1
+metadata:
+ name: custom-solr-xml
+data:
+ solr.xml: |
+ <?xml version="1.0" encoding="UTF-8" ?>
+ <solr>
+ ... CUSTOM CONFIG HERE ...
+ </solr>
+```
Review comment:
Also say that your statefulset and solr pods will not be created if this is missing from the XML.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr-operator] thelabdude commented on a change in pull request #193: Support custom log4j2 config from user-provided ConfigMap
Posted by GitBox <gi...@apache.org>.
thelabdude commented on a change in pull request #193:
URL: https://github.com/apache/lucene-solr-operator/pull/193#discussion_r566235409
##########
File path: controllers/util/solr_util.go
##########
@@ -327,13 +330,42 @@ func GenerateStatefulSet(solrCloud *solr.SolrCloud, solrCloudStatus *solr.SolrCl
envVars = append(envVars, customPodOptions.EnvVariables...)
}
+ // Did the user provide a custom log config?
+ if configMapInfo[LogXmlFile] != "" {
+
+ if configMapInfo[LogXmlMd5Annotation] != "" {
+ if podAnnotations == nil {
+ podAnnotations = make(map[string]string, 1)
+ }
+ podAnnotations[LogXmlMd5Annotation] = configMapInfo[LogXmlMd5Annotation]
+ }
+
+ // cannot use /var/solr as a mountPath, so mount the custom log config in a sub-dir
+ volName := "log4j2-xml"
+ mountPath := fmt.Sprintf("/var/solr/%s-log-config", solrCloud.Name)
+ log4jPropsEnvVarPath := fmt.Sprintf("%s/%s", mountPath, LogXmlFile)
+
+ solrVolumes = append(solrVolumes, corev1.Volume{
Review comment:
Latest commit addresses this issue.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr-operator] thelabdude commented on pull request #193: Support custom log4j2 config from user-provided ConfigMap
Posted by GitBox <gi...@apache.org>.
thelabdude commented on pull request #193:
URL: https://github.com/apache/lucene-solr-operator/pull/193#issuecomment-769283945
Now with docs! ;-)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr-operator] HoustonPutman commented on a change in pull request #193: Support custom log4j2 config from user-provided ConfigMap
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on a change in pull request #193:
URL: https://github.com/apache/lucene-solr-operator/pull/193#discussion_r566277962
##########
File path: controllers/solrcloud_controller.go
##########
@@ -182,44 +182,61 @@ func (r *SolrCloudReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
}
}
- // Generate ConfigMap unless the user supplied a custom ConfigMap for solr.xml ... but the provided ConfigMap
- // might be for the Prometheus exporter, so we only care if they provide a solr.xml in the CM
- solrXmlConfigMapName := instance.ConfigMapName()
- solrXmlMd5 := ""
+ // Generate ConfigMap unless the user supplied a custom ConfigMap for solr.xml
+ configMapInfo := make(map[string]string)
if instance.Spec.CustomSolrKubeOptions.ConfigMapOptions != nil && instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap != "" {
+ providedConfigMapName := instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap
foundConfigMap := &corev1.ConfigMap{}
- nn := types.NamespacedName{Name: instance.Spec.CustomSolrKubeOptions.ConfigMapOptions.ProvidedConfigMap, Namespace: instance.Namespace}
+ nn := types.NamespacedName{Name: providedConfigMapName, Namespace: instance.Namespace}
err = r.Get(context.TODO(), nn, foundConfigMap)
if err != nil {
return requeueOrNot, err // if they passed a providedConfigMap name, then it must exist
}
- // ConfigMap doesn't have to have a solr.xml, but if it does, then it needs to be valid!
if foundConfigMap.Data != nil {
- solrXml, ok := foundConfigMap.Data["solr.xml"]
- if ok {
+ logXml, hasLogXml := foundConfigMap.Data[util.LogXmlFile]
+ solrXml, hasSolrXml := foundConfigMap.Data[util.SolrXmlFile]
+
+ // if there's a user-provided config, it must have one of the expected keys
+ if !hasLogXml && !hasSolrXml {
+ return requeueOrNot, fmt.Errorf("User provided ConfigMap %s must have one of 'solr.xml' and/or 'log4j2.xml'",
Review comment:
Please add a TODO comment to create a controller event for this error.
##########
File path: controllers/solrcloud_controller_test.go
##########
@@ -759,18 +754,246 @@ func TestCloudWithCustomSolrXmlConfigMapReconcile(t *testing.T) {
// update the embedded solr.xml to trigger a pod rolling restart
foundConfigMap = &corev1.ConfigMap{}
err = testClient.Get(context.TODO(), types.NamespacedName{Name: testCustomSolrXmlConfigMap, Namespace: instance.Namespace}, foundConfigMap)
- updateSolrXml := foundConfigMap.Data["solr.xml"]
- foundConfigMap.Data["solr.xml"] = strings.Replace(updateSolrXml, "${zkClientTimeout:30000}", "${zkClientTimeout:15000}", 1)
+ updateSolrXml := foundConfigMap.Data[util.SolrXmlFile]
+ foundConfigMap.Data[util.SolrXmlFile] = strings.Replace(updateSolrXml, "${zkClientTimeout:30000}", "${zkClientTimeout:15000}", 1)
err = testClient.Update(context.TODO(), foundConfigMap)
g.Expect(err).NotTo(gomega.HaveOccurred())
g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
+ g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
// Check the annotation on the pod template to make sure a rolling restart will take place
- updateSolrXml = foundConfigMap.Data["solr.xml"]
+ updateSolrXml = foundConfigMap.Data[util.SolrXmlFile]
updateSolrXmlMd5 := fmt.Sprintf("%x", md5.Sum([]byte(updateSolrXml)))
time.Sleep(time.Millisecond * 250)
err = testClient.Get(context.TODO(), cloudSsKey, stateful)
g.Expect(err).NotTo(gomega.HaveOccurred())
assert.Equal(t, updateSolrXmlMd5, stateful.Spec.Template.Annotations[util.SolrXmlMd5Annotation], "Custom solr.xml MD5 annotation should be updated on the pod template.")
}
+
+func TestCloudWithUserProvidedLogConfigMapReconcile(t *testing.T) {
+ UseZkCRD(true)
+ g := gomega.NewGomegaWithT(t)
+
+ testCustomLogXmlConfigMap := "my-custom-log4j2-xml"
+
+ instance := &solr.SolrCloud{
+ ObjectMeta: metav1.ObjectMeta{Name: expectedCloudRequest.Name, Namespace: expectedCloudRequest.Namespace},
+ Spec: solr.SolrCloudSpec{
+ CustomSolrKubeOptions: solr.CustomSolrKubeOptions{
+ ConfigMapOptions: &solr.ConfigMapOptions{
+ ProvidedConfigMap: testCustomLogXmlConfigMap,
+ },
+ },
+ },
+ }
+
+ // Setup the Manager and Controller. Wrap the Controller Reconcile function so it writes each request to a
+ // channel when it is finished.
+ mgr, err := manager.New(testCfg, manager.Options{})
+ g.Expect(err).NotTo(gomega.HaveOccurred())
+ testClient = mgr.GetClient()
+
+ solrCloudReconciler := &SolrCloudReconciler{
+ Client: testClient,
+ Log: ctrl.Log.WithName("controllers").WithName("SolrCloud"),
+ }
+ newRec, requests := SetupTestReconcile(solrCloudReconciler)
+ g.Expect(solrCloudReconciler.SetupWithManagerAndReconciler(mgr, newRec)).NotTo(gomega.HaveOccurred())
+
+ stopMgr, mgrStopped := StartTestManager(mgr, g)
+
+ defer func() {
+ close(stopMgr)
+ mgrStopped.Wait()
+ }()
+
+ cleanupTest(g, instance.Namespace)
+
+ // Create the SolrCloud object and expect the Reconcile and StatefulSet to be created
+ err = testClient.Create(context.TODO(), instance)
+ g.Expect(err).NotTo(gomega.HaveOccurred())
+ defer testClient.Delete(context.TODO(), instance)
+
+ // reconcile will have failed b/c the provided ConfigMap doesn't exist ...
+ g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
+
+ userProvidedConfigMap := &corev1.ConfigMap{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: testCustomLogXmlConfigMap,
+ Namespace: instance.Namespace,
+ },
+ Data: map[string]string{
+ util.LogXmlFile: "<Configuration/>",
+ },
+ }
+ err = testClient.Create(context.TODO(), userProvidedConfigMap)
+ g.Expect(err).NotTo(gomega.HaveOccurred())
+ defer testClient.Delete(context.TODO(), userProvidedConfigMap)
+
+ g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
+
+ g.Eventually(func() error { return testClient.Get(context.TODO(), expectedCloudRequest.NamespacedName, instance) }, timeout).Should(gomega.Succeed())
+ emptyRequests(requests)
+
+ stateful := &appsv1.StatefulSet{}
+ g.Eventually(func() error { return testClient.Get(context.TODO(), cloudSsKey, stateful) }, timeout).Should(gomega.Succeed())
+ assert.NotEmpty(t, stateful.Spec.Template.Annotations[util.LogXmlMd5Annotation], "Custom log XML MD5 annotation should be set on the pod template!")
+
+ assert.NotNil(t, stateful.Spec.Template.Spec.Volumes)
+ var logXmlVol *corev1.Volume = nil
+ for _, vol := range stateful.Spec.Template.Spec.Volumes {
+ if vol.Name == "log4j2-xml" {
+ logXmlVol = &vol
+ break
+ }
+ }
+ assert.NotNil(t, logXmlVol, "Didn't find custom log4j2-xml volume in sts config!")
+ assert.NotNil(t, logXmlVol.VolumeSource.ConfigMap, "log4j2-xml Volume should have a ConfigMap source")
+ assert.Equal(t, logXmlVol.VolumeSource.ConfigMap.Name, testCustomLogXmlConfigMap, "log4j2-xml Volume should have a ConfigMap source")
+
+ var logXmlVolMount *corev1.VolumeMount = nil
+ for _, mount := range stateful.Spec.Template.Spec.Containers[0].VolumeMounts {
+ if mount.Name == "log4j2-xml" {
+ logXmlVolMount = &mount
+ break
+ }
+ }
+ assert.NotNil(t, logXmlVolMount, "Didn't find the log4j2-xml Volume mount")
+ expectedMountPath := fmt.Sprintf("/var/solr/%s", testCustomLogXmlConfigMap)
+ assert.Equal(t, expectedMountPath, logXmlVolMount.MountPath)
+
+ solrXmlConfigName := fmt.Sprintf("%s-solrcloud-configmap", instance.GetName())
+ foundConfigMap := &corev1.ConfigMap{}
+ err = testClient.Get(context.TODO(), types.NamespacedName{Name: solrXmlConfigName, Namespace: instance.Namespace}, foundConfigMap)
+ g.Expect(err).NotTo(gomega.HaveOccurred(), "Built-in solr.xml ConfigMap should still exist! ")
+
+ emptyRequests(requests)
+ // update the user-provided log XML to trigger a pod rolling restart
+ updatedXml := "<Configuration>Updated!</Configuration>"
+ foundConfigMap = &corev1.ConfigMap{}
+ err = testClient.Get(context.TODO(), types.NamespacedName{Name: testCustomLogXmlConfigMap, Namespace: instance.Namespace}, foundConfigMap)
+ foundConfigMap.Data[util.LogXmlFile] = updatedXml
+ err = testClient.Update(context.TODO(), foundConfigMap)
+ g.Expect(err).NotTo(gomega.HaveOccurred())
+ // capture all reconcile requests
+ g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
+ g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
+ g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
+
+ // Check the annotation on the pod template to make sure a rolling restart will take place
+ updateLogXmlMd5 := fmt.Sprintf("%x", md5.Sum([]byte(updatedXml)))
+ time.Sleep(time.Millisecond * 250)
+ err = testClient.Get(context.TODO(), cloudSsKey, stateful)
+ g.Expect(err).NotTo(gomega.HaveOccurred())
+ assert.Equal(t, updateLogXmlMd5, stateful.Spec.Template.Annotations[util.LogXmlMd5Annotation], "Custom log XML MD5 annotation should be updated on the pod template.")
Review comment:
Should you test that the log4j location env var is set?
##########
File path: controllers/solrcloud_controller_test.go
##########
@@ -759,18 +754,246 @@ func TestCloudWithCustomSolrXmlConfigMapReconcile(t *testing.T) {
// update the embedded solr.xml to trigger a pod rolling restart
foundConfigMap = &corev1.ConfigMap{}
err = testClient.Get(context.TODO(), types.NamespacedName{Name: testCustomSolrXmlConfigMap, Namespace: instance.Namespace}, foundConfigMap)
- updateSolrXml := foundConfigMap.Data["solr.xml"]
- foundConfigMap.Data["solr.xml"] = strings.Replace(updateSolrXml, "${zkClientTimeout:30000}", "${zkClientTimeout:15000}", 1)
+ updateSolrXml := foundConfigMap.Data[util.SolrXmlFile]
+ foundConfigMap.Data[util.SolrXmlFile] = strings.Replace(updateSolrXml, "${zkClientTimeout:30000}", "${zkClientTimeout:15000}", 1)
err = testClient.Update(context.TODO(), foundConfigMap)
g.Expect(err).NotTo(gomega.HaveOccurred())
g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
+ g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
// Check the annotation on the pod template to make sure a rolling restart will take place
- updateSolrXml = foundConfigMap.Data["solr.xml"]
+ updateSolrXml = foundConfigMap.Data[util.SolrXmlFile]
updateSolrXmlMd5 := fmt.Sprintf("%x", md5.Sum([]byte(updateSolrXml)))
time.Sleep(time.Millisecond * 250)
err = testClient.Get(context.TODO(), cloudSsKey, stateful)
g.Expect(err).NotTo(gomega.HaveOccurred())
assert.Equal(t, updateSolrXmlMd5, stateful.Spec.Template.Annotations[util.SolrXmlMd5Annotation], "Custom solr.xml MD5 annotation should be updated on the pod template.")
}
+
+func TestCloudWithUserProvidedLogConfigMapReconcile(t *testing.T) {
+ UseZkCRD(true)
+ g := gomega.NewGomegaWithT(t)
+
+ testCustomLogXmlConfigMap := "my-custom-log4j2-xml"
+
+ instance := &solr.SolrCloud{
+ ObjectMeta: metav1.ObjectMeta{Name: expectedCloudRequest.Name, Namespace: expectedCloudRequest.Namespace},
+ Spec: solr.SolrCloudSpec{
+ CustomSolrKubeOptions: solr.CustomSolrKubeOptions{
+ ConfigMapOptions: &solr.ConfigMapOptions{
+ ProvidedConfigMap: testCustomLogXmlConfigMap,
+ },
+ },
+ },
+ }
+
+ // Setup the Manager and Controller. Wrap the Controller Reconcile function so it writes each request to a
+ // channel when it is finished.
+ mgr, err := manager.New(testCfg, manager.Options{})
+ g.Expect(err).NotTo(gomega.HaveOccurred())
+ testClient = mgr.GetClient()
+
+ solrCloudReconciler := &SolrCloudReconciler{
+ Client: testClient,
+ Log: ctrl.Log.WithName("controllers").WithName("SolrCloud"),
+ }
+ newRec, requests := SetupTestReconcile(solrCloudReconciler)
+ g.Expect(solrCloudReconciler.SetupWithManagerAndReconciler(mgr, newRec)).NotTo(gomega.HaveOccurred())
+
+ stopMgr, mgrStopped := StartTestManager(mgr, g)
+
+ defer func() {
+ close(stopMgr)
+ mgrStopped.Wait()
+ }()
+
+ cleanupTest(g, instance.Namespace)
+
+ // Create the SolrCloud object and expect the Reconcile and StatefulSet to be created
+ err = testClient.Create(context.TODO(), instance)
+ g.Expect(err).NotTo(gomega.HaveOccurred())
+ defer testClient.Delete(context.TODO(), instance)
+
+ // reconcile will have failed b/c the provided ConfigMap doesn't exist ...
+ g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
+
+ userProvidedConfigMap := &corev1.ConfigMap{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: testCustomLogXmlConfigMap,
+ Namespace: instance.Namespace,
+ },
+ Data: map[string]string{
+ util.LogXmlFile: "<Configuration/>",
+ },
+ }
+ err = testClient.Create(context.TODO(), userProvidedConfigMap)
+ g.Expect(err).NotTo(gomega.HaveOccurred())
+ defer testClient.Delete(context.TODO(), userProvidedConfigMap)
+
+ g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
+
+ g.Eventually(func() error { return testClient.Get(context.TODO(), expectedCloudRequest.NamespacedName, instance) }, timeout).Should(gomega.Succeed())
+ emptyRequests(requests)
+
+ stateful := &appsv1.StatefulSet{}
+ g.Eventually(func() error { return testClient.Get(context.TODO(), cloudSsKey, stateful) }, timeout).Should(gomega.Succeed())
+ assert.NotEmpty(t, stateful.Spec.Template.Annotations[util.LogXmlMd5Annotation], "Custom log XML MD5 annotation should be set on the pod template!")
+
+ assert.NotNil(t, stateful.Spec.Template.Spec.Volumes)
+ var logXmlVol *corev1.Volume = nil
+ for _, vol := range stateful.Spec.Template.Spec.Volumes {
+ if vol.Name == "log4j2-xml" {
+ logXmlVol = &vol
+ break
+ }
+ }
+ assert.NotNil(t, logXmlVol, "Didn't find custom log4j2-xml volume in sts config!")
+ assert.NotNil(t, logXmlVol.VolumeSource.ConfigMap, "log4j2-xml Volume should have a ConfigMap source")
+ assert.Equal(t, logXmlVol.VolumeSource.ConfigMap.Name, testCustomLogXmlConfigMap, "log4j2-xml Volume should have a ConfigMap source")
+
+ var logXmlVolMount *corev1.VolumeMount = nil
+ for _, mount := range stateful.Spec.Template.Spec.Containers[0].VolumeMounts {
+ if mount.Name == "log4j2-xml" {
+ logXmlVolMount = &mount
+ break
+ }
+ }
+ assert.NotNil(t, logXmlVolMount, "Didn't find the log4j2-xml Volume mount")
+ expectedMountPath := fmt.Sprintf("/var/solr/%s", testCustomLogXmlConfigMap)
+ assert.Equal(t, expectedMountPath, logXmlVolMount.MountPath)
Review comment:
Add a error message saying that's the mountPath is incorrect.
##########
File path: controllers/solrcloud_controller_test.go
##########
@@ -759,18 +754,246 @@ func TestCloudWithCustomSolrXmlConfigMapReconcile(t *testing.T) {
// update the embedded solr.xml to trigger a pod rolling restart
foundConfigMap = &corev1.ConfigMap{}
err = testClient.Get(context.TODO(), types.NamespacedName{Name: testCustomSolrXmlConfigMap, Namespace: instance.Namespace}, foundConfigMap)
- updateSolrXml := foundConfigMap.Data["solr.xml"]
- foundConfigMap.Data["solr.xml"] = strings.Replace(updateSolrXml, "${zkClientTimeout:30000}", "${zkClientTimeout:15000}", 1)
+ updateSolrXml := foundConfigMap.Data[util.SolrXmlFile]
+ foundConfigMap.Data[util.SolrXmlFile] = strings.Replace(updateSolrXml, "${zkClientTimeout:30000}", "${zkClientTimeout:15000}", 1)
err = testClient.Update(context.TODO(), foundConfigMap)
g.Expect(err).NotTo(gomega.HaveOccurred())
g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
+ g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
// Check the annotation on the pod template to make sure a rolling restart will take place
- updateSolrXml = foundConfigMap.Data["solr.xml"]
+ updateSolrXml = foundConfigMap.Data[util.SolrXmlFile]
updateSolrXmlMd5 := fmt.Sprintf("%x", md5.Sum([]byte(updateSolrXml)))
time.Sleep(time.Millisecond * 250)
err = testClient.Get(context.TODO(), cloudSsKey, stateful)
g.Expect(err).NotTo(gomega.HaveOccurred())
assert.Equal(t, updateSolrXmlMd5, stateful.Spec.Template.Annotations[util.SolrXmlMd5Annotation], "Custom solr.xml MD5 annotation should be updated on the pod template.")
}
+
+func TestCloudWithUserProvidedLogConfigMapReconcile(t *testing.T) {
+ UseZkCRD(true)
+ g := gomega.NewGomegaWithT(t)
+
+ testCustomLogXmlConfigMap := "my-custom-log4j2-xml"
+
+ instance := &solr.SolrCloud{
+ ObjectMeta: metav1.ObjectMeta{Name: expectedCloudRequest.Name, Namespace: expectedCloudRequest.Namespace},
+ Spec: solr.SolrCloudSpec{
+ CustomSolrKubeOptions: solr.CustomSolrKubeOptions{
+ ConfigMapOptions: &solr.ConfigMapOptions{
+ ProvidedConfigMap: testCustomLogXmlConfigMap,
+ },
+ },
+ },
+ }
+
+ // Setup the Manager and Controller. Wrap the Controller Reconcile function so it writes each request to a
+ // channel when it is finished.
+ mgr, err := manager.New(testCfg, manager.Options{})
+ g.Expect(err).NotTo(gomega.HaveOccurred())
+ testClient = mgr.GetClient()
+
+ solrCloudReconciler := &SolrCloudReconciler{
+ Client: testClient,
+ Log: ctrl.Log.WithName("controllers").WithName("SolrCloud"),
+ }
+ newRec, requests := SetupTestReconcile(solrCloudReconciler)
+ g.Expect(solrCloudReconciler.SetupWithManagerAndReconciler(mgr, newRec)).NotTo(gomega.HaveOccurred())
+
+ stopMgr, mgrStopped := StartTestManager(mgr, g)
+
+ defer func() {
+ close(stopMgr)
+ mgrStopped.Wait()
+ }()
+
+ cleanupTest(g, instance.Namespace)
+
+ // Create the SolrCloud object and expect the Reconcile and StatefulSet to be created
+ err = testClient.Create(context.TODO(), instance)
+ g.Expect(err).NotTo(gomega.HaveOccurred())
+ defer testClient.Delete(context.TODO(), instance)
+
+ // reconcile will have failed b/c the provided ConfigMap doesn't exist ...
+ g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
+
+ userProvidedConfigMap := &corev1.ConfigMap{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: testCustomLogXmlConfigMap,
+ Namespace: instance.Namespace,
+ },
+ Data: map[string]string{
+ util.LogXmlFile: "<Configuration/>",
+ },
+ }
+ err = testClient.Create(context.TODO(), userProvidedConfigMap)
+ g.Expect(err).NotTo(gomega.HaveOccurred())
+ defer testClient.Delete(context.TODO(), userProvidedConfigMap)
+
+ g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
+
+ g.Eventually(func() error { return testClient.Get(context.TODO(), expectedCloudRequest.NamespacedName, instance) }, timeout).Should(gomega.Succeed())
+ emptyRequests(requests)
+
+ stateful := &appsv1.StatefulSet{}
+ g.Eventually(func() error { return testClient.Get(context.TODO(), cloudSsKey, stateful) }, timeout).Should(gomega.Succeed())
+ assert.NotEmpty(t, stateful.Spec.Template.Annotations[util.LogXmlMd5Annotation], "Custom log XML MD5 annotation should be set on the pod template!")
+
+ assert.NotNil(t, stateful.Spec.Template.Spec.Volumes)
+ var logXmlVol *corev1.Volume = nil
+ for _, vol := range stateful.Spec.Template.Spec.Volumes {
+ if vol.Name == "log4j2-xml" {
+ logXmlVol = &vol
+ break
+ }
+ }
+ assert.NotNil(t, logXmlVol, "Didn't find custom log4j2-xml volume in sts config!")
+ assert.NotNil(t, logXmlVol.VolumeSource.ConfigMap, "log4j2-xml Volume should have a ConfigMap source")
+ assert.Equal(t, logXmlVol.VolumeSource.ConfigMap.Name, testCustomLogXmlConfigMap, "log4j2-xml Volume should have a ConfigMap source")
+
+ var logXmlVolMount *corev1.VolumeMount = nil
+ for _, mount := range stateful.Spec.Template.Spec.Containers[0].VolumeMounts {
+ if mount.Name == "log4j2-xml" {
+ logXmlVolMount = &mount
+ break
+ }
+ }
+ assert.NotNil(t, logXmlVolMount, "Didn't find the log4j2-xml Volume mount")
+ expectedMountPath := fmt.Sprintf("/var/solr/%s", testCustomLogXmlConfigMap)
+ assert.Equal(t, expectedMountPath, logXmlVolMount.MountPath)
+
+ solrXmlConfigName := fmt.Sprintf("%s-solrcloud-configmap", instance.GetName())
+ foundConfigMap := &corev1.ConfigMap{}
+ err = testClient.Get(context.TODO(), types.NamespacedName{Name: solrXmlConfigName, Namespace: instance.Namespace}, foundConfigMap)
+ g.Expect(err).NotTo(gomega.HaveOccurred(), "Built-in solr.xml ConfigMap should still exist! ")
+
+ emptyRequests(requests)
+ // update the user-provided log XML to trigger a pod rolling restart
+ updatedXml := "<Configuration>Updated!</Configuration>"
+ foundConfigMap = &corev1.ConfigMap{}
+ err = testClient.Get(context.TODO(), types.NamespacedName{Name: testCustomLogXmlConfigMap, Namespace: instance.Namespace}, foundConfigMap)
+ foundConfigMap.Data[util.LogXmlFile] = updatedXml
+ err = testClient.Update(context.TODO(), foundConfigMap)
+ g.Expect(err).NotTo(gomega.HaveOccurred())
+ // capture all reconcile requests
+ g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
+ g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
+ g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest)))
+
+ // Check the annotation on the pod template to make sure a rolling restart will take place
+ updateLogXmlMd5 := fmt.Sprintf("%x", md5.Sum([]byte(updatedXml)))
+ time.Sleep(time.Millisecond * 250)
+ err = testClient.Get(context.TODO(), cloudSsKey, stateful)
+ g.Expect(err).NotTo(gomega.HaveOccurred())
+ assert.Equal(t, updateLogXmlMd5, stateful.Spec.Template.Annotations[util.LogXmlMd5Annotation], "Custom log XML MD5 annotation should be updated on the pod template.")
+}
+
+func TestCloudWithUserProvidedSolrXmlAndLogConfigReconcile(t *testing.T) {
+ UseZkCRD(true)
+ g := gomega.NewGomegaWithT(t)
+
+ testCustomConfigMap := "my-custom-config-xml"
+
+ instance := &solr.SolrCloud{
+ ObjectMeta: metav1.ObjectMeta{Name: expectedCloudRequest.Name, Namespace: expectedCloudRequest.Namespace},
+ Spec: solr.SolrCloudSpec{
+ CustomSolrKubeOptions: solr.CustomSolrKubeOptions{
+ ConfigMapOptions: &solr.ConfigMapOptions{
+ ProvidedConfigMap: testCustomConfigMap,
+ },
+ },
+ },
+ }
+
+ // Setup the Manager and Controller. Wrap the Controller Reconcile function so it writes each request to a
+ // channel when it is finished.
+ mgr, err := manager.New(testCfg, manager.Options{})
+ g.Expect(err).NotTo(gomega.HaveOccurred())
+ testClient = mgr.GetClient()
+
+ solrCloudReconciler := &SolrCloudReconciler{
+ Client: testClient,
+ Log: ctrl.Log.WithName("controllers").WithName("SolrCloud"),
+ }
+ newRec, requests := SetupTestReconcile(solrCloudReconciler)
+ g.Expect(solrCloudReconciler.SetupWithManagerAndReconciler(mgr, newRec)).NotTo(gomega.HaveOccurred())
+
+ stopMgr, mgrStopped := StartTestManager(mgr, g)
+
+ defer func() {
+ close(stopMgr)
+ mgrStopped.Wait()
+ }()
+
+ cleanupTest(g, instance.Namespace)
+
+ // Create the user-provided ConfigMap first to streamline reconcile,
+ // other tests cover creating it after the reconcile loop starts on the SolrCloud
+ userProvidedConfigMap := &corev1.ConfigMap{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: testCustomConfigMap,
+ Namespace: instance.Namespace,
+ },
+ Data: map[string]string{
+ util.LogXmlFile: "<Configuration/>",
Review comment:
Should there be a test where the log4J file is auto-updated? (Thus no need for an annotation)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org