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 2023/01/03 18:13:26 UTC

[solr-operator] branch main updated: Change Zookeeper Cluster CRD Spec Labels (#514)

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 167337e  Change Zookeeper Cluster CRD Spec Labels (#514)
167337e is described below

commit 167337eab232f5d4cbea77b0c71ac4b2059354b7
Author: Nick Huanca <19...@users.noreply.github.com>
AuthorDate: Tue Jan 3 11:13:20 2023 -0700

    Change Zookeeper Cluster CRD Spec Labels (#514)
    
    Zookeeper cluster CRD spec.pod.labels presently does not function as
    intended. StatefulSet pods receive all their labels from spec.labels.
    This is outlined in https://github.com/pravega/zookeeper-operator/issues/511.
    
    This change makes it so that any labels which are provided in the pod
    labels are merged into the spec.labels of the Zookeeper cluster CRD.
    This also prioritizes any labels provided by the Solr Operator, thus not
    allowing you to override any important labels like 'app' or 'release'
    when using the spec.pod.labels in the SolrCloud CRD.
    
    Co-authored-by: Josh Souza <jo...@codethat.rocks>
---
 controllers/solrcloud_controller_zk_test.go | 19 +++++++++++++++++--
 controllers/util/zk_util.go                 | 17 +++++++++++++++--
 helm/solr-operator/Chart.yaml               |  7 +++++++
 3 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/controllers/solrcloud_controller_zk_test.go b/controllers/solrcloud_controller_zk_test.go
index 1ff11cc..7461ab4 100644
--- a/controllers/solrcloud_controller_zk_test.go
+++ b/controllers/solrcloud_controller_zk_test.go
@@ -19,6 +19,9 @@ package controllers
 
 import (
 	"context"
+	"fmt"
+	"strconv"
+
 	solrv1beta1 "github.com/apache/solr-operator/api/v1beta1"
 	"github.com/apache/solr-operator/controllers/util"
 	zk_crd "github.com/apache/solr-operator/controllers/zk_api"
@@ -29,7 +32,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"strconv"
 )
 
 var _ = FDescribe("SolrCloud controller - Zookeeper", func() {
@@ -281,12 +283,25 @@ var _ = FDescribe("SolrCloud controller - Zookeeper", func() {
 			Expect(zkCluster.Spec.Pod.Resources).To(Equal(testResources), "Incorrect zkCluster resources")
 			Expect(zkCluster.Spec.Pod.Env).To(Equal(extraVars), "Incorrect zkCluster env vars")
 			Expect(zkCluster.Spec.Pod.ServiceAccountName).To(Equal(testServiceAccountName), "Incorrect zkCluster serviceAccountName")
-			Expect(zkCluster.Spec.Pod.Labels).To(Equal(util.MergeLabelsOrAnnotations(testSSLabels, map[string]string{"app": "foo-solrcloud-zookeeper", "release": "foo-solrcloud-zookeeper"})), "Incorrect zkCluster pod labels")
 			Expect(zkCluster.Spec.Pod.Annotations).To(Equal(testSSAnnotations), "Incorrect zkCluster pod annotations")
 			Expect(zkCluster.Spec.Pod.SecurityContext).To(Equal(&testPodSecurityContext), "Incorrect zkCluster pod securityContext")
 			Expect(zkCluster.Spec.Pod.TerminationGracePeriodSeconds).To(Equal(testTerminationGracePeriodSeconds), "Incorrect zkCluster pod terminationGracePeriodSeconds")
 			Expect(zkCluster.Spec.Pod.ImagePullSecrets).To(Equal(append(append(make([]corev1.LocalObjectReference, 0), testAdditionalImagePullSecrets...), corev1.LocalObjectReference{Name: testImagePullSecretName})), "Incorrect zkCluster imagePullSecrets")
 
+			// NOTE: Spec.Pod.Labels doesn't presently function as intended in the
+			// Zookeeper Operator, see https://github.com/pravega/zookeeper-operator/issues/511.
+			// Documentation indicates that Spec.Labels is for passing pod labels.
+			// This test will remain here in case the behavior changes on the
+			// Zookeeper Operator in the future.
+			Expect(zkCluster.Spec.Pod.Labels).To(HaveKeyWithValue("app", "foo-solrcloud-zookeeper"), "Missing 'app' label from zkCluster pod labels")
+			Expect(zkCluster.Spec.Pod.Labels).To(HaveKeyWithValue("release", "foo-solrcloud-zookeeper"), "Missing 'release' label zkCluster pod labels")
+			Expect(zkCluster.Spec.Labels).To(HaveKeyWithValue("app", "foo-solrcloud-zookeeper"), "Missing 'app' label from zkCluster pod labels")
+			Expect(zkCluster.Spec.Labels).To(HaveKeyWithValue("release", "foo-solrcloud-zookeeper"), "Missing 'release' label zkCluster pod labels")
+			for k, v := range testSSLabels {
+				Expect(zkCluster.Spec.Pod.Labels).To(HaveKeyWithValue(k, v), fmt.Sprintf("Missing %s=%s from zkCluster pod labels", k, v))
+				Expect(zkCluster.Spec.Labels).To(HaveKeyWithValue(k, v), fmt.Sprintf("Missing %s=%s from zkCluster labels", k, v))
+			}
+
 			// Check ZK Config Options
 			Expect(zkCluster.Spec.Conf.InitLimit).To(Equal(zkConf.InitLimit), "Incorrect zkCluster Config InitLimit")
 			Expect(zkCluster.Spec.Conf.SyncLimit).To(Equal(zkConf.SyncLimit), "Incorrect zkCluster Config SyncLimit")
diff --git a/controllers/util/zk_util.go b/controllers/util/zk_util.go
index 00a7b30..0e1835f 100644
--- a/controllers/util/zk_util.go
+++ b/controllers/util/zk_util.go
@@ -18,12 +18,13 @@
 package util
 
 import (
+	"strings"
+
 	solrv1beta1 "github.com/apache/solr-operator/api/v1beta1"
 	"github.com/apache/solr-operator/controllers/zk_api"
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"strings"
 )
 
 // GenerateZookeeperCluster returns a new ZookeeperCluster pointer generated for the SolrCloud instance
@@ -124,7 +125,19 @@ func GenerateZookeeperCluster(solrCloud *solrv1beta1.SolrCloud, zkSpec *solrv1be
 	}
 
 	if len(zkSpec.ZookeeperPod.Labels) > 0 {
-		zkCluster.Spec.Pod.Labels = zkSpec.ZookeeperPod.Labels
+		// HACK: Include the pod labels on Spec.Labels due to
+		// https://github.com/pravega/zookeeper-operator/issues/511. See
+		// https://github.com/apache/solr-operator/issues/490 for more details.
+		// Note that the `labels` value should always take precedence over the pod
+		// specific labels.
+		podLabels := MergeLabelsOrAnnotations(labels, zkSpec.ZookeeperPod.Labels)
+
+		zkCluster.Spec.Pod.Labels = podLabels
+
+		// This override is applied to the zkCluster.Spec.Labels due to a bug in
+		// the zookeeper operator:
+		// https://github.com/pravega/zookeeper-operator/issues/511
+		zkCluster.Spec.Labels = podLabels
 	}
 
 	if len(zkSpec.ZookeeperPod.Annotations) > 0 {
diff --git a/helm/solr-operator/Chart.yaml b/helm/solr-operator/Chart.yaml
index bf82466..8386a1a 100644
--- a/helm/solr-operator/Chart.yaml
+++ b/helm/solr-operator/Chart.yaml
@@ -94,6 +94,13 @@ annotations:
       links:
         - name: GitHub PR
           url: https://github.com/apache/solr-operator/pull/509
+    - kind: fixed
+      description: Fix issue where Zookeeper specific labels are not propagated to Zookeeper pods
+      links:
+        - name: GitHub PR
+          url: https://github.com/apache/solr-operator/pull/514
+        - name: GitHub Issue
+          url: https://github.com/apache/solr-operator/issues/490
   artifacthub.io/images: |
     - name: solr-operator
       image: apache/solr-operator:v0.7.0-prerelease