You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ho...@apache.org on 2022/07/07 14:49:29 UTC

[solr-operator] branch main updated: Use the Solr image as the default Prom Exp image (#454)

This is an automated email from the ASF dual-hosted git repository.

houston pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr-operator.git


The following commit(s) were added to refs/heads/main by this push:
     new e36b843  Use the Solr image as the default Prom Exp image (#454)
e36b843 is described below

commit e36b8439845c43582de02a2d629c43fe22fb22b4
Author: Houston Putman <ho...@apache.org>
AuthorDate: Thu Jul 7 10:49:24 2022 -0400

    Use the Solr image as the default Prom Exp image (#454)
---
 api/v1beta1/solrprometheusexporter_types.go        | 12 +++++--
 controllers/controller_utils_test.go               |  1 +
 controllers/solrprometheusexporter_controller.go   | 10 +++---
 .../solrprometheusexporter_controller_test.go      | 42 ++++++++++++++++++++--
 controllers/util/prometheus_exporter_util.go       | 15 +++++---
 docs/solr-prometheus-exporter/README.md            |  3 ++
 docs/upgrade-notes.md                              |  9 +++--
 hack/release/smoke_test/test_cluster.sh            |  8 ++---
 helm/solr-operator/Chart.yaml                      |  9 ++++-
 9 files changed, 88 insertions(+), 21 deletions(-)

diff --git a/api/v1beta1/solrprometheusexporter_types.go b/api/v1beta1/solrprometheusexporter_types.go
index 5779548..ad5de71 100644
--- a/api/v1beta1/solrprometheusexporter_types.go
+++ b/api/v1beta1/solrprometheusexporter_types.go
@@ -80,9 +80,17 @@ func (ps *SolrPrometheusExporterSpec) withDefaults(namespace string) (changed bo
 	changed = ps.SolrReference.withDefaults(namespace) || changed
 
 	if ps.Image == nil {
-		ps.Image = &ContainerImage{}
+		// Only instantiate the Image variable if the Solr reference is a SolrCloud resource.
+		// If so, then the image will be defaulted to the same image that the Solr reference uses.
+		if ps.SolrReference.Cloud == nil || ps.SolrReference.Cloud.Name == "" {
+			ps.Image = &ContainerImage{}
+		}
+	}
+
+	// Do not change this to an else, as the Image might be instantiated in the if
+	if ps.Image != nil {
+		changed = ps.Image.withDefaults(DefaultSolrRepo, DefaultSolrVersion, DefaultPullPolicy) || changed
 	}
-	changed = ps.Image.withDefaults(DefaultSolrRepo, DefaultSolrVersion, DefaultPullPolicy) || changed
 
 	if ps.NumThreads == 0 {
 		ps.NumThreads = 1
diff --git a/controllers/controller_utils_test.go b/controllers/controller_utils_test.go
index d7e7299..cd707c1 100644
--- a/controllers/controller_utils_test.go
+++ b/controllers/controller_utils_test.go
@@ -808,6 +808,7 @@ var (
 	}
 	testPriorityClass              = "p4"
 	testImagePullSecretName        = "MAIN_SECRET"
+	testImagePullSecretName2       = "ANOTHER_SECRET"
 	testAdditionalImagePullSecrets = []corev1.LocalObjectReference{
 		{Name: "ADDITIONAL_SECRET_1"},
 		{Name: "ADDITIONAL_SECRET_2"},
diff --git a/controllers/solrprometheusexporter_controller.go b/controllers/solrprometheusexporter_controller.go
index ae8f53a..9adc3f4 100644
--- a/controllers/solrprometheusexporter_controller.go
+++ b/controllers/solrprometheusexporter_controller.go
@@ -178,7 +178,8 @@ func (r *SolrPrometheusExporterReconciler) Reconcile(ctx context.Context, req ct
 
 	// Get the ZkConnectionString to connect to
 	solrConnectionInfo := util.SolrConnectionInfo{}
-	if solrConnectionInfo, err = getSolrConnectionInfo(ctx, r, prometheusExporter); err != nil {
+	var solrCloudImage *solrv1beta1.ContainerImage
+	if solrConnectionInfo, solrCloudImage, err = getSolrConnectionInfo(ctx, r, prometheusExporter); err != nil {
 		return requeueOrNot, err
 	}
 
@@ -207,7 +208,7 @@ func (r *SolrPrometheusExporterReconciler) Reconcile(ctx context.Context, req ct
 		basicAuthMd5 = fmt.Sprintf("%x", md5.Sum([]byte(creds)))
 	}
 
-	deploy := util.GenerateSolrPrometheusExporterDeployment(prometheusExporter, solrConnectionInfo, configXmlMd5, tls, basicAuthMd5)
+	deploy := util.GenerateSolrPrometheusExporterDeployment(prometheusExporter, solrConnectionInfo, solrCloudImage, configXmlMd5, tls, basicAuthMd5)
 
 	ready := false
 	// Check if the Metrics Deployment already exists
@@ -269,7 +270,7 @@ func (r *SolrPrometheusExporterReconciler) Reconcile(ctx context.Context, req ct
 	return requeueOrNot, err
 }
 
-func getSolrConnectionInfo(ctx context.Context, r *SolrPrometheusExporterReconciler, prometheusExporter *solrv1beta1.SolrPrometheusExporter) (solrConnectionInfo util.SolrConnectionInfo, err error) {
+func getSolrConnectionInfo(ctx context.Context, r *SolrPrometheusExporterReconciler, prometheusExporter *solrv1beta1.SolrPrometheusExporter) (solrConnectionInfo util.SolrConnectionInfo, solrCloudImage *solrv1beta1.ContainerImage, err error) {
 	solrConnectionInfo = util.SolrConnectionInfo{}
 
 	if prometheusExporter.Spec.SolrReference.Standalone != nil {
@@ -288,10 +289,11 @@ func getSolrConnectionInfo(ctx context.Context, r *SolrPrometheusExporterReconci
 			err = r.Get(ctx, types.NamespacedName{Name: prometheusExporter.Spec.SolrReference.Cloud.Name, Namespace: solrNamespace}, solrCloud)
 			if err == nil {
 				solrConnectionInfo.CloudZkConnnectionInfo = &solrCloud.Status.ZookeeperConnectionInfo
+				solrCloudImage = solrCloud.Spec.SolrImage
 			}
 		}
 	}
-	return solrConnectionInfo, err
+	return
 }
 
 // reconcileTLSConfig Reconciles the various options for configuring TLS for the exporter
diff --git a/controllers/solrprometheusexporter_controller_test.go b/controllers/solrprometheusexporter_controller_test.go
index 3c63b13..c20f751 100644
--- a/controllers/solrprometheusexporter_controller_test.go
+++ b/controllers/solrprometheusexporter_controller_test.go
@@ -52,6 +52,7 @@ var _ = FDescribe("SolrPrometheusExporter controller - General", func() {
 		ctx context.Context
 
 		solrPrometheusExporter *solrv1beta1.SolrPrometheusExporter
+		solrRef                *solrv1beta1.SolrCloud
 	)
 
 	BeforeEach(func() {
@@ -282,7 +283,6 @@ var _ = FDescribe("SolrPrometheusExporter controller - General", func() {
 		solrName := "test-solr"
 		testZkCnxString := "host-from-solr:2181"
 		testZKChroot := "/this/path"
-		var solrRef *solrv1beta1.SolrCloud
 		BeforeEach(func() {
 			solrPrometheusExporter.Spec = solrv1beta1.SolrPrometheusExporterSpec{
 				SolrReference: solrv1beta1.SolrReference{
@@ -303,6 +303,11 @@ var _ = FDescribe("SolrPrometheusExporter controller - General", func() {
 					Namespace: "default",
 				},
 				Spec: solrv1beta1.SolrCloudSpec{
+					SolrImage: &solrv1beta1.ContainerImage{
+						Tag:             "should-be-the-same",
+						PullPolicy:      corev1.PullAlways,
+						ImagePullSecret: testImagePullSecretName2,
+					},
 					ZookeeperRef: &solrv1beta1.ZookeeperRef{
 						ConnectionInfo: &solrv1beta1.ZookeeperConnectionInfo{
 							InternalConnectionString: testZkCnxString,
@@ -334,6 +339,9 @@ var _ = FDescribe("SolrPrometheusExporter controller - General", func() {
 				}
 				g.Expect(found.Spec.Template.Spec.Containers[0].Args).To(Equal(expectedArgs), "Incorrect arguments for the SolrPrometheusExporter container")
 				g.Expect(found.Spec.Template.Spec.Containers[0].Command).To(Equal([]string{util.DefaultPrometheusExporterEntrypoint}), "Incorrect command for the SolrPrometheusExporter container")
+				g.Expect(found.Spec.Template.Spec.Containers[0].Image).To(Equal(solrv1beta1.DefaultSolrRepo+":should-be-the-same"), "Incorrect image, should be pulled from the SolrCloud")
+				g.Expect(found.Spec.Template.Spec.Containers[0].ImagePullPolicy).To(Equal(corev1.PullAlways), "Incorrect imagePullPolicy, should be pulled from the SolrCloud")
+				g.Expect(found.Spec.Template.Spec.ImagePullSecrets).To(ConsistOf(corev1.LocalObjectReference{Name: testImagePullSecretName2}), "Incorrect imagePullSecrets, should be pulled from the SolrCloud")
 
 				// Env Variable Tests
 				expectedEnvVars := map[string]string{
@@ -417,7 +425,7 @@ var _ = FDescribe("SolrPrometheusExporter controller - General", func() {
 		})
 	})
 
-	FContext("Updating a user-provided ConfigMap", func() {
+	FContext("Updating a user-provided ConfigMap | Use explicitly defined image", func() {
 		testZkCnxString := "host-from-solr:2181"
 		testZKChroot := "/this/path"
 		withUserProvidedConfigMapName := "custom-exporter-config"
@@ -436,7 +444,32 @@ var _ = FDescribe("SolrPrometheusExporter controller - General", func() {
 						ProvidedConfigMap: withUserProvidedConfigMapName,
 					},
 				},
+				Image: &solrv1beta1.ContainerImage{
+					Repository: "test/repo",
+				},
 			}
+
+			solrRef = &solrv1beta1.SolrCloud{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-solr",
+					Namespace: "default",
+				},
+				Spec: solrv1beta1.SolrCloudSpec{
+					SolrImage: &solrv1beta1.ContainerImage{
+						Tag:             "should-be-the-same",
+						PullPolicy:      corev1.PullAlways,
+						ImagePullSecret: testImagePullSecretName2,
+					},
+					ZookeeperRef: &solrv1beta1.ZookeeperRef{
+						ConnectionInfo: &solrv1beta1.ZookeeperConnectionInfo{
+							InternalConnectionString: testZkCnxString,
+							ChRoot:                   testZKChroot,
+						},
+					},
+				},
+			}
+			Expect(k8sClient.Create(ctx, solrRef)).To(Succeed(), "Creating test SolrCloud for Prometheus Exporter to connect to")
+
 		})
 		FIt("has the correct resources", func() {
 			By("testing the SolrPrometheusExporter Deployment is not created without an existing configMap")
@@ -478,6 +511,11 @@ var _ = FDescribe("SolrPrometheusExporter controller - General", func() {
 					util.PrometheusExporterConfigXmlMd5Annotation: fmt.Sprintf("%x", md5.Sum([]byte(updatedConfigXml))),
 				}
 				g.Expect(found.Spec.Template.ObjectMeta.Annotations).To(Equal(expectedAnnotations), "Incorrect pod annotations after updating configMap")
+
+				g.Expect(found.Spec.Template.Spec.Containers[0].Image).To(Equal("test/repo:"+solrv1beta1.DefaultSolrVersion), "The prometheus exporter container has the wrong image, should be using the explictly defined information, not copying from the SolrCloud")
+				g.Expect(found.Spec.Template.Spec.Containers[0].ImagePullPolicy).To(Equal(corev1.PullIfNotPresent), "The prometheus exporter container has the wrong imagePullPolicy, should not be copying from the SolrCloud")
+
+				g.Expect(found.Spec.Template.Spec.ImagePullSecrets).To(BeEmpty(), "The prometheus exporter should not have any imagePullSecrets, since it's not copying image information from the SolrCloud")
 			})
 		})
 	})
diff --git a/controllers/util/prometheus_exporter_util.go b/controllers/util/prometheus_exporter_util.go
index 3fb2029..e35da50 100644
--- a/controllers/util/prometheus_exporter_util.go
+++ b/controllers/util/prometheus_exporter_util.go
@@ -48,7 +48,7 @@ type SolrConnectionInfo struct {
 
 // GenerateSolrPrometheusExporterDeployment returns a new appsv1.Deployment pointer generated for the SolrCloud Prometheus Exporter instance
 // solrPrometheusExporter: SolrPrometheusExporter instance
-func GenerateSolrPrometheusExporterDeployment(solrPrometheusExporter *solr.SolrPrometheusExporter, solrConnectionInfo SolrConnectionInfo, configXmlMd5 string, tls *TLSCerts, basicAuthMd5 string) *appsv1.Deployment {
+func GenerateSolrPrometheusExporterDeployment(solrPrometheusExporter *solr.SolrPrometheusExporter, solrConnectionInfo SolrConnectionInfo, solrCloudImage *solr.ContainerImage, configXmlMd5 string, tls *TLSCerts, basicAuthMd5 string) *appsv1.Deployment {
 	gracePeriodTerm := int64(10)
 	singleReplica := int32(1)
 	fsGroup := int64(SolrMetricsPort)
@@ -175,11 +175,16 @@ func GenerateSolrPrometheusExporterDeployment(solrPrometheusExporter *solr.SolrP
 		envVars = append(envVars, corev1.EnvVar{Name: "JAVA_OPTS", Value: strings.Join(allJavaOpts, " ")})
 	}
 
+	containerImage := solrPrometheusExporter.Spec.Image
+	if containerImage == nil {
+		containerImage = solrCloudImage
+	}
+
 	containers := []corev1.Container{
 		{
 			Name:            "solr-prometheus-exporter",
-			Image:           solrPrometheusExporter.Spec.Image.ToImageName(),
-			ImagePullPolicy: solrPrometheusExporter.Spec.Image.PullPolicy,
+			Image:           containerImage.ToImageName(),
+			ImagePullPolicy: containerImage.PullPolicy,
 			Ports:           []corev1.ContainerPort{{ContainerPort: SolrMetricsPort, Name: SolrMetricsPortName, Protocol: corev1.ProtocolTCP}},
 			VolumeMounts:    volumeMounts,
 			Command:         []string{entrypoint},
@@ -261,10 +266,10 @@ func GenerateSolrPrometheusExporterDeployment(solrPrometheusExporter *solr.SolrP
 		},
 	}
 
-	if solrPrometheusExporter.Spec.Image.ImagePullSecret != "" {
+	if containerImage.ImagePullSecret != "" {
 		imagePullSecrets = append(
 			imagePullSecrets,
-			corev1.LocalObjectReference{Name: solrPrometheusExporter.Spec.Image.ImagePullSecret},
+			corev1.LocalObjectReference{Name: containerImage.ImagePullSecret},
 		)
 	}
 
diff --git a/docs/solr-prometheus-exporter/README.md b/docs/solr-prometheus-exporter/README.md
index c607c51..f223904 100644
--- a/docs/solr-prometheus-exporter/README.md
+++ b/docs/solr-prometheus-exporter/README.md
@@ -43,6 +43,9 @@ This name can be provided at: `SolrPrometheusExporter.spec.solrRef.cloud.name`
 - Provide explicit Zookeeper Connection info for the prometheus exporter to use.  
   This info can be provided at: `SolrPrometheusExporter.spec.solrRef.cloud.zkConnectionInfo`, with keys `internalConnectionString` and `chroot`
 
+If `SolrPrometheusExporter.spec.solrRef.cloud.name` is used and no image information is passed via `SolrPrometheusExporter.spec.image.*` options, then the Prometheus Exporter will use the same image as the SolrCloud it is listening to.
+If any `SolrPrometheusExporter.spec.image.*` option is provided, then the Prometheus Exporter will use its own image.
+
 #### ACLs
 _Since v0.2.7_
 
diff --git a/docs/upgrade-notes.md b/docs/upgrade-notes.md
index aa1dd33..756ac45 100644
--- a/docs/upgrade-notes.md
+++ b/docs/upgrade-notes.md
@@ -122,11 +122,16 @@ _Note that the Helm chart version does not contain a `v` prefix, which the downl
   In this release `additionalDomains` is still accepted, but all values will automatically be added to `additionalDomainNames` and the field will be set to `nil` by the operator.
   The `additionalDomains` option will be removed in a future version.
 
+- `SolrPrometheusExporter` resources without any image specifications (`SolrPrometheusExporter.Spec.image.*`) will use the referenced `SolrCloud` image, if the reference is by `name`, not `zkConnectionString`.
+  If any `SolrPrometheusExporter.Spec.image.*` option is provided, then those values will be defaulted by the Solr Operator and the `SolrCloud` image will not be used.
+  When upgrading from `v0.5.*` to `v0.6.0`, only new `SolrPrometheusExporter` resources will use this new feature.
+  To enable it on existing resources, update the resources and remove the `SolrPrometheusExporter.Spec.image` section.
+
 - CRD options deprecated in `v0.5.0` have been removed.
   This includes field `SolrCloud.spec.dataStorage.backupRestoreOptions`, `SolrBackup.spec.persistence` and `SolrBackup.status.persistenceStatus`.
-  Upgrading to `v0.5.0` will remove these options on existing and new SolrCloud and SolrBackup resources.
+  Upgrading to `v0.5.*` will remove these options on existing and new SolrCloud and SolrBackup resources.
   However, once the Solr CRDs are upgraded to `v0.6.0`, you will no longer be able to submit resources with the options listed above.
-  Please migrate your systems to use the new options while running `v0.5.0`, before upgrading to `v0.6.0`. 
+  Please migrate your systems to use the new options while running `v0.5.*`, before upgrading to `v0.6.0`. 
 
 ### v0.5.0
 - Due to the deprecation and removal of `networking.k8s.io/v1beta1` in Kubernetes v1.22, `networking.k8s.io/v1` will be used for Ingresses.
diff --git a/hack/release/smoke_test/test_cluster.sh b/hack/release/smoke_test/test_cluster.sh
index 274e654..2aec793 100755
--- a/hack/release/smoke_test/test_cluster.sh
+++ b/hack/release/smoke_test/test_cluster.sh
@@ -77,7 +77,7 @@ if [[ -z "${KUBERNETES_VERSION:-}" ]]; then
   KUBERNETES_VERSION="v1.21.2"
 fi
 if [[ -z "${SOLR_IMAGE:-}" ]]; then
-  SOLR_IMAGE="${SOLR_VERSION:-8.11}"
+  SOLR_IMAGE="${SOLR_VERSION:-9.0}"
 fi
 if [[ "${SOLR_IMAGE}" != *":"* ]]; then
   SOLR_IMAGE="solr:${SOLR_IMAGE}"
@@ -218,8 +218,6 @@ spec:
     cloud:
       name: "example"
   numThreads: 4
-  image:
-    tag: "8.11"
 EOF
 
 printf "\nWait for the Solr Prometheus Exporter to be ready\n"
@@ -285,8 +283,8 @@ helm upgrade --kube-context "${KUBE_CONTEXT}" ${VERIFY_OR_NOT} example "${SOLR_H
 printf '\nWait for the rolling restart to begin.\n\n'
 grep -q "3              [[:digit:]]       [[:digit:]]            0" <(exec kubectl get solrcloud example -w); kill $!
 
-printf '\nWait for all 3 Solr nodes to become ready.\n\n'
-grep -q "3              3       3            3" <(exec kubectl get solrcloud example -w); kill $!
+printf '\nWait 5 minutes for all 3 Solr nodes to become ready.\n\n'
+grep -q "3              3       3            3" <(exec kubectl get solrcloud example -w --request-timeout 300); kill $!
 
 # Need a new port-forward, since the last one will have broken due to all pods restarting
 kubectl port-forward service/example-solrcloud-common 28983:80 || true &
diff --git a/helm/solr-operator/Chart.yaml b/helm/solr-operator/Chart.yaml
index bbd45ef..908018d 100644
--- a/helm/solr-operator/Chart.yaml
+++ b/helm/solr-operator/Chart.yaml
@@ -130,12 +130,19 @@ annotations:
         - name: Github PR
           url: https://github.com/apache/solr-operator/pull/450
     - kind: added
-      description: SolrCloud and SolrPrometheusExporter Services now have the appropriate (http or https) appProtocol set.
+      description: SolrCloud and SolrPrometheusExporter Services now have the appropriate (http or https) appProtocol set. This fixes integration with Istio.
       links:
         - name: Github Issue
           url: https://github.com/apache/solr-operator/issues/427
         - name: Github PR
           url: https://github.com/apache/solr-operator/pull/453
+    - kind: changed
+      description: SolrPrometheusExporters now default to using the references SolrCloud image if none is provided. Note, the entire PrometheusExporter image specification must be empty for it to use the SolrCloud image. This does not effect existing resources, only newly created resources.
+      links:
+        - name: Github Issue
+          url: https://github.com/apache/solr-operator/issues/386
+        - name: Github PR
+          url: https://github.com/apache/solr-operator/pull/454
   artifacthub.io/images: |
     - name: solr-operator
       image: apache/solr-operator:v0.6.0-prerelease