From 045dcab8d05b62c16c6c703ccece2819ee0c88ec Mon Sep 17 00:00:00 2001 From: Nick Huanca <1903525+endzyme@users.noreply.github.com> Date: Tue, 3 Jan 2023 09:57:37 -0700 Subject: [PATCH] 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 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. --- 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 1ff11cc0..7461ab47 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 00a7b30c..0e1835f7 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 bf824667..8386a1a3 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