Add configurable revisionHistoryLimit to Druid node spec

apis/druid/v1alpha1/druid_types.go

@@ -451,6 +451,14 @@ type DruidNodeSpec struct {
 	// +optional
 	MaxUnavailable *int32 `json:"maxUnavailable,omitempty"`
 
+	// RevisionHistoryLimit is the number of old ReplicaSets (for Deployment
+	// node types) or ControllerRevisions (for StatefulSet node types) to retain
+	// to allow rollback. Bounding it lets Kubernetes garbage-collect superseded
+	// pod-template revisions that otherwise keep references to old container
+	// image tags. If nil, the Kubernetes default of 10 is used.
+	// +optional
+	RevisionHistoryLimit *int32 `json:"revisionHistoryLimit,omitempty"`
+
 	// UpdateStrategy
 	// +optional
 	UpdateStrategy *appsv1.StatefulSetUpdateStrategy `json:"updateStrategy,omitempty"`

chart/crds/druid.apache.org_druids.yaml

@@ -5638,6 +5638,15 @@ spec:
                             More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
                           type: object
                       type: object
+                    revisionHistoryLimit:
+                      description: |-
+                        RevisionHistoryLimit is the number of old ReplicaSets (for Deployment
+                        node types) or ControllerRevisions (for StatefulSet node types) to retain
+                        to allow rollback. Bounding it lets Kubernetes garbage-collect superseded
+                        pod-template revisions that otherwise keep references to old container
+                        image tags. If nil, the Kubernetes default of 10 is used.
+                      format: int32
+                      type: integer
                     runtime.properties:
                       description: RuntimeProperties Additional runtime configuration
                         for the specific workload.

config/crd/bases/druid.apache.org_druids.yaml

@@ -5638,6 +5638,15 @@ spec:
                             More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
                           type: object
                       type: object
+                    revisionHistoryLimit:
+                      description: |-
+                        RevisionHistoryLimit is the number of old ReplicaSets (for Deployment
+                        node types) or ControllerRevisions (for StatefulSet node types) to retain
+                        to allow rollback. Bounding it lets Kubernetes garbage-collect superseded
+                        pod-template revisions that otherwise keep references to old container
+                        image tags. If nil, the Kubernetes default of 10 is used.
+                      format: int32
+                      type: integer
                     runtime.properties:
                       description: RuntimeProperties Additional runtime configuration
                         for the specific workload.

controllers/druid/handler.go

@@ -1070,6 +1070,7 @@ func makeStatefulSetSpec(nodeSpec *v1alpha1.DruidNodeSpec, m *v1alpha1.Druid, ls
 			MatchLabels: ls,
 		},
 		Replicas:             &nodeSpec.Replicas,
+		RevisionHistoryLimit: nodeSpec.RevisionHistoryLimit,
 		PodManagementPolicy:  appsv1.PodManagementPolicyType(firstNonEmptyStr(firstNonEmptyStr(string(nodeSpec.PodManagementPolicy), string(m.Spec.PodManagementPolicy)), string(appsv1.ParallelPodManagement))),
 		UpdateStrategy:       *updateStrategy,
 		Template:             makePodTemplate(nodeSpec, m, ls, nodeSpecificUniqueString, configMapSHA),
@@ -1086,8 +1087,9 @@ func makeDeploymentSpec(nodeSpec *v1alpha1.DruidNodeSpec, m *v1alpha1.Druid, ls
 		Selector: &metav1.LabelSelector{
 			MatchLabels: ls,
 		},
-		Replicas: &nodeSpec.Replicas,
-		Template: makePodTemplate(nodeSpec, m, ls, nodeSpecificUniqueString, configMapSHA),
+		Replicas:             &nodeSpec.Replicas,
+		RevisionHistoryLimit: nodeSpec.RevisionHistoryLimit,
+		Template:             makePodTemplate(nodeSpec, m, ls, nodeSpecificUniqueString, configMapSHA),
 		Strategy: appsv1.DeploymentStrategy{
 			Type:          "RollingUpdate",
 			RollingUpdate: getRollingUpdateStrategy(nodeSpec),

controllers/druid/handler_test.go

@@ -113,6 +113,54 @@ var _ = Describe("Test handler", func() {
 			Expect(actual).Should(Equal(expected))
 		})
 
+		It("should set revisionHistoryLimit on the statefulset when specified on the node spec", func() {
+			By("By setting revisionHistoryLimit on the node spec")
+			clusterSpec, err := readDruidClusterSpecFromFile("testdata/druid-test-cr.yaml")
+			Expect(err).Should(BeNil())
+
+			nodeSpecUniqueStr := makeNodeSpecificUniqueString(clusterSpec, "brokers")
+			nodeSpec := clusterSpec.Spec.Nodes["brokers"]
+			limit := int32(2)
+			nodeSpec.RevisionHistoryLimit = &limit
+
+			actual, _ := makeStatefulSet(&nodeSpec, clusterSpec, makeLabelsForNodeSpec(&nodeSpec, clusterSpec, clusterSpec.Name, nodeSpecUniqueStr), nodeSpecUniqueStr, "blah", nodeSpecUniqueStr)
+
+			Expect(actual.Spec.RevisionHistoryLimit).ShouldNot(BeNil())
+			Expect(*actual.Spec.RevisionHistoryLimit).Should(Equal(int32(2)))
+		})
+
+		It("should set revisionHistoryLimit on the deployment when specified on the node spec", func() {
+			By("By setting revisionHistoryLimit on the node spec")
+			clusterSpec, err := readDruidClusterSpecFromFile("testdata/druid-test-cr.yaml")
+			Expect(err).Should(BeNil())
+
+			nodeSpecUniqueStr := makeNodeSpecificUniqueString(clusterSpec, "brokers")
+			nodeSpec := clusterSpec.Spec.Nodes["brokers"]
+			limit := int32(2)
+			nodeSpec.RevisionHistoryLimit = &limit
+
+			actual, _ := makeDeployment(&nodeSpec, clusterSpec, makeLabelsForNodeSpec(&nodeSpec, clusterSpec, clusterSpec.Name, nodeSpecUniqueStr), nodeSpecUniqueStr, "blah", nodeSpecUniqueStr)
+
+			Expect(actual.Spec.RevisionHistoryLimit).ShouldNot(BeNil())
+			Expect(*actual.Spec.RevisionHistoryLimit).Should(Equal(int32(2)))
+		})
+
+		It("should leave revisionHistoryLimit unset (Kubernetes default) when not specified on the node spec", func() {
+			By("By not setting revisionHistoryLimit on the node spec")
+			clusterSpec, err := readDruidClusterSpecFromFile("testdata/druid-test-cr.yaml")
+			Expect(err).Should(BeNil())
+
+			nodeSpecUniqueStr := makeNodeSpecificUniqueString(clusterSpec, "brokers")
+			nodeSpec := clusterSpec.Spec.Nodes["brokers"]
+			Expect(nodeSpec.RevisionHistoryLimit).Should(BeNil())
+
+			sts, _ := makeStatefulSet(&nodeSpec, clusterSpec, makeLabelsForNodeSpec(&nodeSpec, clusterSpec, clusterSpec.Name, nodeSpecUniqueStr), nodeSpecUniqueStr, "blah", nodeSpecUniqueStr)
+			deploy, _ := makeDeployment(&nodeSpec, clusterSpec, makeLabelsForNodeSpec(&nodeSpec, clusterSpec, clusterSpec.Name, nodeSpecUniqueStr), nodeSpecUniqueStr, "blah", nodeSpecUniqueStr)
+
+			Expect(sts.Spec.RevisionHistoryLimit).Should(BeNil())
+			Expect(deploy.Spec.RevisionHistoryLimit).Should(BeNil())
+		})
+
 		It("should make PDB for broker", func() {
 			By("By making PDB for broker")
 			filePath := "testdata/druid-test-cr.yaml"

examples/tiny-cluster.yaml

@@ -374,6 +374,12 @@ spec:
       druid.port: 8088
       nodeConfigMountPath: "/opt/druid/conf/druid/cluster/query/router"
       replicas: 1
+      # Optional: cap the controller's retained revision history. For Deployment
+      # node types (like this router) it bounds old ReplicaSets, so superseded
+      # pod templates / image tags are garbage-collected instead of lingering;
+      # for StatefulSet node types it bounds ControllerRevisions. Omit to keep
+      # the Kubernetes default of 10.
+      revisionHistoryLimit: 2
       runtime.properties: |
         druid.service=druid/router