Skip to content

Commit

Permalink
Change Zookeeper Cluster CRD Spec Labels
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
endzyme committed Jan 3, 2023
1 parent c3cda10 commit 045dcab
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 4 deletions.
19 changes: 17 additions & 2 deletions controllers/solrcloud_controller_zk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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() {
Expand Down Expand Up @@ -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")
Expand Down
17 changes: 15 additions & 2 deletions controllers/util/zk_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions helm/solr-operator/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 045dcab

Please sign in to comment.