Change Zookeeper Cluster CRD Spec Labels

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 pravega/zookeeper-operator#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 <joshsouzadev@codethat.rocks>

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")

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 {

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