From dfc0824ed80ed139debb6d8ae433731d668c6f9c Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Tue, 25 Jul 2023 13:39:03 -0400 Subject: [PATCH 01/22] Add koperator/api changes for KRaft support --- api/go.mod | 4 + api/go.sum | 7 + api/util/util.go | 10 + api/util/util_test.go | 10 + api/v1beta1/kafkacluster_types.go | 65 +++++- api/v1beta1/kafkacluster_types_test.go | 194 ++++++++++++++++++ api/v1beta1/zz_generated.deepcopy.go | 5 + charts/kafka-operator/templates/crds.yaml | 26 ++- .../kafka.banzaicloud.io_kafkaclusters.yaml | 26 ++- 9 files changed, 338 insertions(+), 9 deletions(-) diff --git a/api/go.mod b/api/go.mod index 55a13bb52..b8c959c26 100644 --- a/api/go.mod +++ b/api/go.mod @@ -7,6 +7,7 @@ require ( github.com/banzaicloud/istio-client-go v0.0.17 github.com/cert-manager/cert-manager v1.11.2 github.com/imdario/mergo v0.3.13 + github.com/stretchr/testify v1.8.1 golang.org/x/exp v0.0.0-20230713183714-613f0c0eb8a1 gotest.tools v2.2.0+incompatible k8s.io/api v0.26.4 @@ -15,6 +16,7 @@ require ( ) require ( + github.com/davecgh/go-spew v1.1.1 // indirect github.com/go-logr/logr v1.2.3 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/google/go-cmp v0.5.9 // indirect @@ -23,12 +25,14 @@ require ( github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect go.uber.org/atomic v1.9.0 // indirect go.uber.org/multierr v1.6.0 // indirect golang.org/x/net v0.7.0 // indirect golang.org/x/text v0.7.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/klog/v2 v2.80.1 // indirect k8s.io/utils v0.0.0-20221128185143-99ec85e7a448 // indirect sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 // indirect diff --git a/api/go.sum b/api/go.sum index bc91b36aa..33435c842 100644 --- a/api/go.sum +++ b/api/go.sum @@ -47,8 +47,13 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= +github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= @@ -100,8 +105,10 @@ gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo= gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw= k8s.io/api v0.26.4 h1:qSG2PmtcD23BkYiWfoYAcak870eF/hE7NNYBYavTT94= diff --git a/api/util/util.go b/api/util/util.go index c2b0e3b6c..42ed1693f 100644 --- a/api/util/util.go +++ b/api/util/util.go @@ -39,3 +39,13 @@ func MergeLabels(l ...map[string]string) map[string]string { func LabelsForKafka(name string) map[string]string { return map[string]string{"app": "kafka", "kafka_cr": name} } + +// StringSliceContains returns true if list contains s +func StringSliceContains(list []string, s string) bool { + for _, v := range list { + if v == s { + return true + } + } + return false +} diff --git a/api/util/util_test.go b/api/util/util_test.go index a2fa1a8ab..133bd3dd7 100644 --- a/api/util/util_test.go +++ b/api/util/util_test.go @@ -35,3 +35,13 @@ func TestMergeLabels(t *testing.T) { t.Error("Expected:", expected, "Got:", merged) } } + +func TestStringSliceContains(t *testing.T) { + slice := []string{"1", "2", "3"} + if !StringSliceContains(slice, "1") { + t.Error("Expected slice contains 1, got false") + } + if StringSliceContains(slice, "4") { + t.Error("Expected slice not contains 4, got true") + } +} diff --git a/api/v1beta1/kafkacluster_types.go b/api/v1beta1/kafkacluster_types.go index a44d358bc..d8951f035 100644 --- a/api/v1beta1/kafkacluster_types.go +++ b/api/v1beta1/kafkacluster_types.go @@ -19,7 +19,6 @@ import ( "strings" "emperror.dev/errors" - "github.com/imdario/mergo" "github.com/banzaicloud/istio-client-go/pkg/networking/v1beta1" @@ -62,19 +61,33 @@ const ( // DefaultKafkaImage is the default Kafka image used when users don't specify it in KafkaClusterSpec.ClusterImage DefaultKafkaImage = "ghcr.io/banzaicloud/kafka:2.13-3.4.1" + + // controllerNodeProcessRole represents the node is a controller node + controllerNodeProcessRole = "controller" + // brokerNodeProcessRole represents the node is a broker node + brokerNodeProcessRole = "broker" ) // KafkaClusterSpec defines the desired state of KafkaCluster type KafkaClusterSpec struct { + // kRaft is used to decide where the Kafka cluster is under KRaft mode or ZooKeeper mode. + // This is default to be true; if set to false, the Kafka cluster is in ZooKeeper mode. + // +kubebuilder:default=true + // +optional + KRaftMode bool `json:"kRaft"` HeadlessServiceEnabled bool `json:"headlessServiceEnabled"` ListenersConfig ListenersConfig `json:"listenersConfig"` // Custom ports to expose in the container. Example use case: a custom kafka distribution, that includes an integrated metrics api endpoint AdditionalPorts []corev1.ContainerPort `json:"additionalPorts,omitempty"` // ZKAddresses specifies the ZooKeeper connection string // in the form hostname:port where host and port are the host and port of a ZooKeeper server. - ZKAddresses []string `json:"zkAddresses"` + // This is not used under KRaft mode. + // +optional + ZKAddresses []string `json:"zkAddresses,omitempty"` // ZKPath specifies the ZooKeeper chroot path as part // of its ZooKeeper connection string which puts its data under some path in the global ZooKeeper namespace. + // This is not used under KRaft mode. + // +optional ZKPath string `json:"zkPath,omitempty"` RackAwareness *RackAwareness `json:"rackAwareness,omitempty"` ClusterImage string `json:"clusterImage,omitempty"` @@ -126,6 +139,8 @@ type KafkaClusterStatus struct { RollingUpgrade RollingUpgradeStatus `json:"rollingUpgradeStatus,omitempty"` AlertCount int `json:"alertCount"` ListenerStatuses ListenerStatuses `json:"listenerStatuses,omitempty"` + // ClusterID is a based64-encoded random UUID generated to run the Kafka cluster in KRaft mode + ClusterID string `json:"clusterID,omitempty"` } // RollingUpgradeStatus defines status of rolling upgrade @@ -161,11 +176,12 @@ type DisruptionBudgetWithStrategy struct { DisruptionBudget `json:",inline"` // The strategy to be used, either minAvailable or maxUnavailable // +kubebuilder:validation:Enum=minAvailable;maxUnavailable - Stategy string `json:"strategy,omitempty"` + Strategy string `json:"strategy,omitempty"` } // Broker defines the broker basic configuration type Broker struct { + // id maps to "node.id" configuration in KRaft mode, and it maps to "broker.id" configuration in ZooKeeper mode. // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=65535 // +kubebuilder:validation:ExclusiveMaximum=true @@ -173,6 +189,11 @@ type Broker struct { BrokerConfigGroup string `json:"brokerConfigGroup,omitempty"` ReadOnlyConfig string `json:"readOnlyConfig,omitempty"` BrokerConfig *BrokerConfig `json:"brokerConfig,omitempty"` + // processRoles defines the role(s) for this particular broker node: broker, controller, or both. + // This must be set in KRaft mode. + // +kubebuilder:validation:MaxItems=2 + // +optional + Roles []string `json:"processRoles,omitempty"` } // BrokerConfig defines the broker configuration @@ -859,6 +880,19 @@ func (bConfig *BrokerConfig) GetTerminationGracePeriod() int64 { return *bConfig.TerminationGracePeriod } +// GetStorageMountPaths returns a string with comma-separated storage mount paths that the broker uses +func (bConfig *BrokerConfig) GetStorageMountPaths() string { + var mountPaths string + for i, sc := range bConfig.StorageConfigs { + if i != len(bConfig.StorageConfigs)-1 { + mountPaths += sc.MountPath + "," + } else { + mountPaths += sc.MountPath + } + } + return mountPaths +} + // GetNodeSelector returns the node selector for cruise control func (cConfig *CruiseControlConfig) GetNodeSelector() map[string]string { return cConfig.NodeSelector @@ -1137,6 +1171,31 @@ func (b *Broker) GetBrokerConfig(kafkaClusterSpec KafkaClusterSpec) (*BrokerConf return bConfig, nil } +// IsBrokerNode returns true when the broker is a broker node +func (b *Broker) IsBrokerNode() bool { + return util.StringSliceContains(b.Roles, brokerNodeProcessRole) +} + +// IsControllerNode returns true when the broker is a controller node +func (b *Broker) IsControllerNode() bool { + return util.StringSliceContains(b.Roles, controllerNodeProcessRole) +} + +// IsBrokerOnlyNode returns true when the broker is a broker-only node +func (b *Broker) IsBrokerOnlyNode() bool { + return b.IsBrokerNode() && !b.IsControllerNode() +} + +// IsControllerOnlyNode returns true when the broker is a controller-only node +func (b *Broker) IsControllerOnlyNode() bool { + return b.IsControllerNode() && !b.IsBrokerNode() +} + +// IsCombinedNode returns true when the broker is a broker + controller node +func (b *Broker) IsCombinedNode() bool { + return b.IsBrokerNode() && b.IsControllerNode() +} + func mergeEnvs(kafkaClusterSpec KafkaClusterSpec, groupConfig, bConfig *BrokerConfig) []corev1.EnvVar { var envs []corev1.EnvVar envs = append(envs, kafkaClusterSpec.Envs...) diff --git a/api/v1beta1/kafkacluster_types_test.go b/api/v1beta1/kafkacluster_types_test.go index 72e8e7b17..ee6ffdf5d 100644 --- a/api/v1beta1/kafkacluster_types_test.go +++ b/api/v1beta1/kafkacluster_types_test.go @@ -19,6 +19,7 @@ import ( "strconv" "testing" + "github.com/stretchr/testify/require" "gotest.tools/assert" corev1 "k8s.io/api/core/v1" @@ -458,3 +459,196 @@ func TestGetBrokerLabels(t *testing.T) { t.Error("Expected:", expected, "Got:", result) } } + +func TestGetStorageMountPaths(t *testing.T) { + testCases := []struct { + testName string + brokerConfig *BrokerConfig + expectedMountPaths string + }{ + { + testName: "BrokerConfig has no StorageConfigs", + brokerConfig: &BrokerConfig{}, + expectedMountPaths: "", + }, + { + testName: "BrokerConfig has one storage configuration under StorageConfigs", + brokerConfig: &BrokerConfig{ + StorageConfigs: []StorageConfig{ + { + MountPath: "test-log-1", + }, + }, + }, + expectedMountPaths: "test-log-1", + }, + { + testName: "BrokerConfig has multiple storage configuration under StorageConfigs", + brokerConfig: &BrokerConfig{ + StorageConfigs: []StorageConfig{ + { + MountPath: "test-log-1", + }, + { + MountPath: "test-log-2", + }, + { + MountPath: "test-log-3", + }, + { + MountPath: "test-log-4", + }, + { + MountPath: "test-log-5", + }, + }, + }, + expectedMountPaths: "test-log-1,test-log-2,test-log-3,test-log-4,test-log-5", + }, + } + + for _, test := range testCases { + t.Run(test.testName, func(t *testing.T) { + gotMountPaths := test.brokerConfig.GetStorageMountPaths() + require.Equal(t, gotMountPaths, test.expectedMountPaths) + }) + } +} + +func TestIsBrokerOnlyNode(t *testing.T) { + testCases := []struct { + testName string + broker Broker + isBrokerOnly bool + }{ + { + testName: "the broker is a broker-only node", + broker: Broker{ + Id: 0, + Roles: []string{"broker"}, + }, + isBrokerOnly: true, + }, + { + testName: "the broker is a controller-only node", + broker: Broker{ + Id: 0, + Roles: []string{"controller"}, + }, + isBrokerOnly: false, + }, + { + testName: "the broker is a combined node", + broker: Broker{ + Id: 0, + Roles: []string{"controller", "broker"}, + }, + isBrokerOnly: false, + }, + { + testName: "the broker has no process roles defined", + broker: Broker{ + Id: 0, + }, + isBrokerOnly: false, + }, + } + + for _, test := range testCases { + t.Run(test.testName, func(t *testing.T) { + require.Equal(t, test.broker.IsBrokerOnlyNode(), test.isBrokerOnly) + }) + } +} + +func TestIsControllerOnlyNode(t *testing.T) { + testCases := []struct { + testName string + broker Broker + isControllerOnly bool + }{ + { + testName: "the broker is a controller-only node", + broker: Broker{ + Id: 0, + Roles: []string{"controller"}, + }, + isControllerOnly: true, + }, + { + testName: "the broker is a broker-only node", + broker: Broker{ + Id: 0, + Roles: []string{"broker"}, + }, + isControllerOnly: false, + }, + { + testName: "the broker is a combined node", + broker: Broker{ + Id: 0, + Roles: []string{"broker", "controller"}, + }, + isControllerOnly: false, + }, + { + testName: "the broker has no process roles defined", + broker: Broker{ + Id: 0, + }, + isControllerOnly: false, + }, + } + + for _, test := range testCases { + t.Run(test.testName, func(t *testing.T) { + require.Equal(t, test.broker.IsControllerOnlyNode(), test.isControllerOnly) + }) + } +} + +func TestIsCombinedNode(t *testing.T) { + testCases := []struct { + testName string + broker Broker + isCombined bool + }{ + { + testName: "the broker is a broker-only node", + broker: Broker{ + Id: 0, + Roles: []string{"broker"}, + }, + isCombined: false, + }, + { + testName: "the broker is a controller-only node", + broker: Broker{ + Id: 0, + Roles: []string{"controller"}, + }, + isCombined: false, + }, + { + testName: "the broker is a combined node", + broker: Broker{ + Id: 0, + Roles: []string{"broker", "controller"}, + }, + isCombined: true, + }, + { + testName: "the broker has no process roles defined", + broker: Broker{ + Id: 0, + }, + isCombined: false, + }, + } + + for _, test := range testCases { + t.Run(test.testName, func(t *testing.T) { + require.Equal(t, test.broker.IsCombinedNode(), test.isCombined) + }) + } +} diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index a3e79c9ca..0463d6e2f 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -51,6 +51,11 @@ func (in *Broker) DeepCopyInto(out *Broker) { *out = new(BrokerConfig) (*in).DeepCopyInto(*out) } + if in.Roles != nil { + in, out := &in.Roles, &out.Roles + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Broker. diff --git a/charts/kafka-operator/templates/crds.yaml b/charts/kafka-operator/templates/crds.yaml index 9368b8117..8d12b9950 100644 --- a/charts/kafka-operator/templates/crds.yaml +++ b/charts/kafka-operator/templates/crds.yaml @@ -12823,11 +12823,21 @@ spec: brokerConfigGroup: type: string id: + description: id maps to "node.id" configuration in KRaft mode, + and it maps to "broker.id" configuration in ZooKeeper mode. exclusiveMaximum: true format: int32 maximum: 65535 minimum: 0 type: integer + processRoles: + description: 'processRoles defines the role(s) for this particular + broker node: broker, controller, or both. This must be set + in KRaft mode.' + items: + type: string + maxItems: 2 + type: array readOnlyConfig: type: string required: @@ -19056,6 +19066,12 @@ spec: type: string type: object type: object + kRaft: + default: true + description: kRaft is used to decide where the Kafka cluster is under + KRaft mode or ZooKeeper mode. This is default to be true; if set + to false, the Kafka cluster is in ZooKeeper mode. + type: boolean kubernetesClusterDomain: type: string listenersConfig: @@ -21703,14 +21719,15 @@ spec: zkAddresses: description: ZKAddresses specifies the ZooKeeper connection string in the form hostname:port where host and port are the host and port - of a ZooKeeper server. + of a ZooKeeper server. This is not used under KRaft mode. items: type: string type: array zkPath: description: ZKPath specifies the ZooKeeper chroot path as part of its ZooKeeper connection string which puts its data under some path - in the global ZooKeeper namespace. + in the global ZooKeeper namespace. This is not used under KRaft + mode. type: string required: - brokers @@ -21719,7 +21736,6 @@ spec: - listenersConfig - oneBrokerPerNode - rollingUpgradeConfig - - zkAddresses type: object status: description: KafkaClusterStatus defines the observed state of KafkaCluster @@ -21813,6 +21829,10 @@ spec: - rackAwarenessState type: object type: object + clusterID: + description: ClusterID is a based64-encoded random UUID generated + to run the Kafka cluster in KRaft mode + type: string cruiseControlTopicStatus: description: CruiseControlTopicStatus holds info about the CC topic status diff --git a/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml b/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml index b09d32ff4..330866a3e 100644 --- a/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml +++ b/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml @@ -12660,11 +12660,21 @@ spec: brokerConfigGroup: type: string id: + description: id maps to "node.id" configuration in KRaft mode, + and it maps to "broker.id" configuration in ZooKeeper mode. exclusiveMaximum: true format: int32 maximum: 65535 minimum: 0 type: integer + processRoles: + description: 'processRoles defines the role(s) for this particular + broker node: broker, controller, or both. This must be set + in KRaft mode.' + items: + type: string + maxItems: 2 + type: array readOnlyConfig: type: string required: @@ -18893,6 +18903,12 @@ spec: type: string type: object type: object + kRaft: + default: true + description: kRaft is used to decide where the Kafka cluster is under + KRaft mode or ZooKeeper mode. This is default to be true; if set + to false, the Kafka cluster is in ZooKeeper mode. + type: boolean kubernetesClusterDomain: type: string listenersConfig: @@ -21540,14 +21556,15 @@ spec: zkAddresses: description: ZKAddresses specifies the ZooKeeper connection string in the form hostname:port where host and port are the host and port - of a ZooKeeper server. + of a ZooKeeper server. This is not used under KRaft mode. items: type: string type: array zkPath: description: ZKPath specifies the ZooKeeper chroot path as part of its ZooKeeper connection string which puts its data under some path - in the global ZooKeeper namespace. + in the global ZooKeeper namespace. This is not used under KRaft + mode. type: string required: - brokers @@ -21556,7 +21573,6 @@ spec: - listenersConfig - oneBrokerPerNode - rollingUpgradeConfig - - zkAddresses type: object status: description: KafkaClusterStatus defines the observed state of KafkaCluster @@ -21650,6 +21666,10 @@ spec: - rackAwarenessState type: object type: object + clusterID: + description: ClusterID is a based64-encoded random UUID generated + to run the Kafka cluster in KRaft mode + type: string cruiseControlTopicStatus: description: CruiseControlTopicStatus holds info about the CC topic status From 54af7053ae40fae45bc2a6806b350b5c90436a53 Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Thu, 27 Jul 2023 09:32:23 -0400 Subject: [PATCH 02/22] Move processRoles under brokerConfig --- api/v1beta1/kafkacluster_types.go | 60 ++++++++-------- api/v1beta1/kafkacluster_types_test.go | 69 ++++++++++++------- api/v1beta1/zz_generated.deepcopy.go | 10 +-- charts/kafka-operator/templates/crds.yaml | 24 ++++--- .../kafka.banzaicloud.io_kafkaclusters.yaml | 24 ++++--- 5 files changed, 113 insertions(+), 74 deletions(-) diff --git a/api/v1beta1/kafkacluster_types.go b/api/v1beta1/kafkacluster_types.go index d8951f035..7d3ea1542 100644 --- a/api/v1beta1/kafkacluster_types.go +++ b/api/v1beta1/kafkacluster_types.go @@ -189,15 +189,15 @@ type Broker struct { BrokerConfigGroup string `json:"brokerConfigGroup,omitempty"` ReadOnlyConfig string `json:"readOnlyConfig,omitempty"` BrokerConfig *BrokerConfig `json:"brokerConfig,omitempty"` - // processRoles defines the role(s) for this particular broker node: broker, controller, or both. - // This must be set in KRaft mode. - // +kubebuilder:validation:MaxItems=2 - // +optional - Roles []string `json:"processRoles,omitempty"` } // BrokerConfig defines the broker configuration type BrokerConfig struct { + // processRoles defines the role(s) for this particular Kafka node: "broker", "controller", or both. + // This must be set in KRaft mode. + // +kubebuilder:validation:MaxItems=2 + // +optional + Roles []string `json:"processRoles,omitempty"` Image string `json:"image,omitempty"` MetricsReporterImage string `json:"metricsReporterImage,omitempty"` Config string `json:"config,omitempty"` @@ -1026,6 +1026,31 @@ func (cConfig *CruiseControlConfig) GetResources() *corev1.ResourceRequirements } } +// IsBrokerNode returns true when the broker is a broker node +func (bConfig *BrokerConfig) IsBrokerNode() bool { + return util.StringSliceContains(bConfig.Roles, brokerNodeProcessRole) +} + +// IsControllerNode returns true when the broker is a controller node +func (bConfig *BrokerConfig) IsControllerNode() bool { + return util.StringSliceContains(bConfig.Roles, controllerNodeProcessRole) +} + +// IsBrokerOnlyNode returns true when the broker is a broker-only node +func (bConfig *BrokerConfig) IsBrokerOnlyNode() bool { + return bConfig.IsBrokerNode() && !bConfig.IsControllerNode() +} + +// IsControllerOnlyNode returns true when the broker is a controller-only node +func (bConfig *BrokerConfig) IsControllerOnlyNode() bool { + return bConfig.IsControllerNode() && !bConfig.IsBrokerNode() +} + +// IsCombinedNode returns true when the broker is a broker + controller node +func (bConfig *BrokerConfig) IsCombinedNode() bool { + return bConfig.IsBrokerNode() && bConfig.IsControllerNode() +} + // GetResources returns the broker specific Kubernetes resource func (bConfig *BrokerConfig) GetResources() *corev1.ResourceRequirements { if bConfig.Resources != nil { @@ -1171,31 +1196,6 @@ func (b *Broker) GetBrokerConfig(kafkaClusterSpec KafkaClusterSpec) (*BrokerConf return bConfig, nil } -// IsBrokerNode returns true when the broker is a broker node -func (b *Broker) IsBrokerNode() bool { - return util.StringSliceContains(b.Roles, brokerNodeProcessRole) -} - -// IsControllerNode returns true when the broker is a controller node -func (b *Broker) IsControllerNode() bool { - return util.StringSliceContains(b.Roles, controllerNodeProcessRole) -} - -// IsBrokerOnlyNode returns true when the broker is a broker-only node -func (b *Broker) IsBrokerOnlyNode() bool { - return b.IsBrokerNode() && !b.IsControllerNode() -} - -// IsControllerOnlyNode returns true when the broker is a controller-only node -func (b *Broker) IsControllerOnlyNode() bool { - return b.IsControllerNode() && !b.IsBrokerNode() -} - -// IsCombinedNode returns true when the broker is a broker + controller node -func (b *Broker) IsCombinedNode() bool { - return b.IsBrokerNode() && b.IsControllerNode() -} - func mergeEnvs(kafkaClusterSpec KafkaClusterSpec, groupConfig, bConfig *BrokerConfig) []corev1.EnvVar { var envs []corev1.EnvVar envs = append(envs, kafkaClusterSpec.Envs...) diff --git a/api/v1beta1/kafkacluster_types_test.go b/api/v1beta1/kafkacluster_types_test.go index ee6ffdf5d..5e695b565 100644 --- a/api/v1beta1/kafkacluster_types_test.go +++ b/api/v1beta1/kafkacluster_types_test.go @@ -524,31 +524,38 @@ func TestIsBrokerOnlyNode(t *testing.T) { { testName: "the broker is a broker-only node", broker: Broker{ - Id: 0, - Roles: []string{"broker"}, + Id: 0, + BrokerConfig: &BrokerConfig{ + Roles: []string{"broker"}, + }, }, isBrokerOnly: true, }, { testName: "the broker is a controller-only node", broker: Broker{ - Id: 0, - Roles: []string{"controller"}, + Id: 0, + BrokerConfig: &BrokerConfig{ + Roles: []string{"controller"}, + }, }, isBrokerOnly: false, }, { testName: "the broker is a combined node", broker: Broker{ - Id: 0, - Roles: []string{"controller", "broker"}, + Id: 0, + BrokerConfig: &BrokerConfig{ + Roles: []string{"controller", "broker"}, + }, }, isBrokerOnly: false, }, { testName: "the broker has no process roles defined", broker: Broker{ - Id: 0, + Id: 0, + BrokerConfig: &BrokerConfig{}, }, isBrokerOnly: false, }, @@ -556,7 +563,7 @@ func TestIsBrokerOnlyNode(t *testing.T) { for _, test := range testCases { t.Run(test.testName, func(t *testing.T) { - require.Equal(t, test.broker.IsBrokerOnlyNode(), test.isBrokerOnly) + require.Equal(t, test.broker.BrokerConfig.IsBrokerOnlyNode(), test.isBrokerOnly) }) } } @@ -570,24 +577,30 @@ func TestIsControllerOnlyNode(t *testing.T) { { testName: "the broker is a controller-only node", broker: Broker{ - Id: 0, - Roles: []string{"controller"}, + Id: 0, + BrokerConfig: &BrokerConfig{ + Roles: []string{"controller"}, + }, }, isControllerOnly: true, }, { testName: "the broker is a broker-only node", broker: Broker{ - Id: 0, - Roles: []string{"broker"}, + Id: 0, + BrokerConfig: &BrokerConfig{ + Roles: []string{"broker"}, + }, }, isControllerOnly: false, }, { testName: "the broker is a combined node", broker: Broker{ - Id: 0, - Roles: []string{"broker", "controller"}, + Id: 0, + BrokerConfig: &BrokerConfig{ + Roles: []string{"broker", "controller"}, + }, }, isControllerOnly: false, }, @@ -595,6 +608,9 @@ func TestIsControllerOnlyNode(t *testing.T) { testName: "the broker has no process roles defined", broker: Broker{ Id: 0, + // every broker is expected to have the broker configurations set through either "brokerConfig" or "brokerConfigGroup" + // and this part is tested in other places, therefore setting "brokerConfig" to be empty for this unit test + BrokerConfig: &BrokerConfig{}, }, isControllerOnly: false, }, @@ -602,7 +618,7 @@ func TestIsControllerOnlyNode(t *testing.T) { for _, test := range testCases { t.Run(test.testName, func(t *testing.T) { - require.Equal(t, test.broker.IsControllerOnlyNode(), test.isControllerOnly) + require.Equal(t, test.broker.BrokerConfig.IsControllerOnlyNode(), test.isControllerOnly) }) } } @@ -616,31 +632,38 @@ func TestIsCombinedNode(t *testing.T) { { testName: "the broker is a broker-only node", broker: Broker{ - Id: 0, - Roles: []string{"broker"}, + Id: 0, + BrokerConfig: &BrokerConfig{ + Roles: []string{"broker"}, + }, }, isCombined: false, }, { testName: "the broker is a controller-only node", broker: Broker{ - Id: 0, - Roles: []string{"controller"}, + Id: 0, + BrokerConfig: &BrokerConfig{ + Roles: []string{"controller"}, + }, }, isCombined: false, }, { testName: "the broker is a combined node", broker: Broker{ - Id: 0, - Roles: []string{"broker", "controller"}, + Id: 0, + BrokerConfig: &BrokerConfig{ + Roles: []string{"broker", "controller"}, + }, }, isCombined: true, }, { testName: "the broker has no process roles defined", broker: Broker{ - Id: 0, + Id: 0, + BrokerConfig: &BrokerConfig{}, }, isCombined: false, }, @@ -648,7 +671,7 @@ func TestIsCombinedNode(t *testing.T) { for _, test := range testCases { t.Run(test.testName, func(t *testing.T) { - require.Equal(t, test.broker.IsCombinedNode(), test.isCombined) + require.Equal(t, test.broker.BrokerConfig.IsCombinedNode(), test.isCombined) }) } } diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 0463d6e2f..83372b0e7 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -51,11 +51,6 @@ func (in *Broker) DeepCopyInto(out *Broker) { *out = new(BrokerConfig) (*in).DeepCopyInto(*out) } - if in.Roles != nil { - in, out := &in.Roles, &out.Roles - *out = make([]string, len(*in)) - copy(*out, *in) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Broker. @@ -71,6 +66,11 @@ func (in *Broker) DeepCopy() *Broker { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BrokerConfig) DeepCopyInto(out *BrokerConfig) { *out = *in + if in.Roles != nil { + in, out := &in.Roles, &out.Roles + *out = make([]string, len(*in)) + copy(*out, *in) + } if in.StorageConfigs != nil { in, out := &in.StorageConfigs, &out.StorageConfigs *out = make([]StorageConfig, len(*in)) diff --git a/charts/kafka-operator/templates/crds.yaml b/charts/kafka-operator/templates/crds.yaml index 8d12b9950..7f328b7c1 100644 --- a/charts/kafka-operator/templates/crds.yaml +++ b/charts/kafka-operator/templates/crds.yaml @@ -4164,6 +4164,14 @@ spec: If not specified, the broker pods' priority is default to zero. type: string + processRoles: + description: 'processRoles defines the role(s) for this particular + Kafka node: "broker", "controller", or both. This must be + set in KRaft mode.' + items: + type: string + maxItems: 2 + type: array resourceRequirements: description: ResourceRequirements describes the compute resource requirements. @@ -10477,6 +10485,14 @@ spec: If not specified, the broker pods' priority is default to zero. type: string + processRoles: + description: 'processRoles defines the role(s) for this + particular Kafka node: "broker", "controller", or both. + This must be set in KRaft mode.' + items: + type: string + maxItems: 2 + type: array resourceRequirements: description: ResourceRequirements describes the compute resource requirements. @@ -12830,14 +12846,6 @@ spec: maximum: 65535 minimum: 0 type: integer - processRoles: - description: 'processRoles defines the role(s) for this particular - broker node: broker, controller, or both. This must be set - in KRaft mode.' - items: - type: string - maxItems: 2 - type: array readOnlyConfig: type: string required: diff --git a/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml b/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml index 330866a3e..55d999324 100644 --- a/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml +++ b/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml @@ -4001,6 +4001,14 @@ spec: If not specified, the broker pods' priority is default to zero. type: string + processRoles: + description: 'processRoles defines the role(s) for this particular + Kafka node: "broker", "controller", or both. This must be + set in KRaft mode.' + items: + type: string + maxItems: 2 + type: array resourceRequirements: description: ResourceRequirements describes the compute resource requirements. @@ -10314,6 +10322,14 @@ spec: If not specified, the broker pods' priority is default to zero. type: string + processRoles: + description: 'processRoles defines the role(s) for this + particular Kafka node: "broker", "controller", or both. + This must be set in KRaft mode.' + items: + type: string + maxItems: 2 + type: array resourceRequirements: description: ResourceRequirements describes the compute resource requirements. @@ -12667,14 +12683,6 @@ spec: maximum: 65535 minimum: 0 type: integer - processRoles: - description: 'processRoles defines the role(s) for this particular - broker node: broker, controller, or both. This must be set - in KRaft mode.' - items: - type: string - maxItems: 2 - type: array readOnlyConfig: type: string required: From dbf32003e81e014c2614ab5bf8c4b7a3f828f7fd Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Thu, 27 Jul 2023 13:34:08 -0400 Subject: [PATCH 03/22] Update comments for ZK-relevant configurations in kafkacluster_types.go --- api/v1beta1/kafkacluster_types.go | 5 +++-- charts/kafka-operator/templates/crds.yaml | 8 +++++--- config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml | 8 +++++--- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/api/v1beta1/kafkacluster_types.go b/api/v1beta1/kafkacluster_types.go index 7d3ea1542..a2054072d 100644 --- a/api/v1beta1/kafkacluster_types.go +++ b/api/v1beta1/kafkacluster_types.go @@ -81,12 +81,13 @@ type KafkaClusterSpec struct { AdditionalPorts []corev1.ContainerPort `json:"additionalPorts,omitempty"` // ZKAddresses specifies the ZooKeeper connection string // in the form hostname:port where host and port are the host and port of a ZooKeeper server. - // This is not used under KRaft mode. + // Under ZooKeeper mode, this is a must-have configuration. + // And if set under KRaft mode, Koperator ignores this configuration. // +optional ZKAddresses []string `json:"zkAddresses,omitempty"` // ZKPath specifies the ZooKeeper chroot path as part // of its ZooKeeper connection string which puts its data under some path in the global ZooKeeper namespace. - // This is not used under KRaft mode. + // If set under KRaft mode, Koperator ignores this configuration. // +optional ZKPath string `json:"zkPath,omitempty"` RackAwareness *RackAwareness `json:"rackAwareness,omitempty"` diff --git a/charts/kafka-operator/templates/crds.yaml b/charts/kafka-operator/templates/crds.yaml index 7f328b7c1..f33c2a65e 100644 --- a/charts/kafka-operator/templates/crds.yaml +++ b/charts/kafka-operator/templates/crds.yaml @@ -21727,15 +21727,17 @@ spec: zkAddresses: description: ZKAddresses specifies the ZooKeeper connection string in the form hostname:port where host and port are the host and port - of a ZooKeeper server. This is not used under KRaft mode. + of a ZooKeeper server. Under ZooKeeper mode, this is a must-have + configuration. And if set under KRaft mode, Koperator ignores this + configuration. items: type: string type: array zkPath: description: ZKPath specifies the ZooKeeper chroot path as part of its ZooKeeper connection string which puts its data under some path - in the global ZooKeeper namespace. This is not used under KRaft - mode. + in the global ZooKeeper namespace. If set under KRaft mode, Koperator + ignores this configuration. type: string required: - brokers diff --git a/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml b/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml index 55d999324..9fe66759d 100644 --- a/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml +++ b/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml @@ -21564,15 +21564,17 @@ spec: zkAddresses: description: ZKAddresses specifies the ZooKeeper connection string in the form hostname:port where host and port are the host and port - of a ZooKeeper server. This is not used under KRaft mode. + of a ZooKeeper server. Under ZooKeeper mode, this is a must-have + configuration. And if set under KRaft mode, Koperator ignores this + configuration. items: type: string type: array zkPath: description: ZKPath specifies the ZooKeeper chroot path as part of its ZooKeeper connection string which puts its data under some path - in the global ZooKeeper namespace. This is not used under KRaft - mode. + in the global ZooKeeper namespace. If set under KRaft mode, Koperator + ignores this configuration. type: string required: - brokers From 219d99802b9d9dcda413e1de0a5f7b5789bebbb4 Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Thu, 20 Jul 2023 17:40:16 -0400 Subject: [PATCH 04/22] Rebase origin/kraft-api --- api/v1beta1/kafkacluster_types.go | 8 ++++++++ charts/kafka-operator/templates/crds.yaml | 8 ++++++++ config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml | 8 ++++++++ go.mod | 2 +- 4 files changed, 25 insertions(+), 1 deletion(-) diff --git a/api/v1beta1/kafkacluster_types.go b/api/v1beta1/kafkacluster_types.go index a2054072d..7e5ff92ce 100644 --- a/api/v1beta1/kafkacluster_types.go +++ b/api/v1beta1/kafkacluster_types.go @@ -74,7 +74,11 @@ type KafkaClusterSpec struct { // This is default to be true; if set to false, the Kafka cluster is in ZooKeeper mode. // +kubebuilder:default=true // +optional +<<<<<<< HEAD KRaftMode bool `json:"kRaft"` +======= + KRaftMode bool `json:"kRaft,omitempty"` +>>>>>>> 199eb1e6 (Modify KafkaCluster API to support running in KRaft) HeadlessServiceEnabled bool `json:"headlessServiceEnabled"` ListenersConfig ListenersConfig `json:"listenersConfig"` // Custom ports to expose in the container. Example use case: a custom kafka distribution, that includes an integrated metrics api endpoint @@ -801,6 +805,10 @@ func (kSpec *KafkaClusterSpec) GetClusterMetricsReporterImage() string { return kSpec.CruiseControlConfig.GetCCImage() } +func (kSpec *KafkaClusterSpec) KraftMode() bool { + return kSpec.KRaftMode +} + func (cTaskSpec *CruiseControlTaskSpec) GetDurationMinutes() float64 { if cTaskSpec.RetryDurationMinutes == 0 { return 5 diff --git a/charts/kafka-operator/templates/crds.yaml b/charts/kafka-operator/templates/crds.yaml index f33c2a65e..d71a88524 100644 --- a/charts/kafka-operator/templates/crds.yaml +++ b/charts/kafka-operator/templates/crds.yaml @@ -12838,6 +12838,14 @@ spec: type: object brokerConfigGroup: type: string + brokerRoles: + description: 'brokerRoles defines the role(s) for this particular + broker node: regular broker, controller, or both. This must + be set in KRaft mode.' + items: + type: string + maxItems: 2 + type: array id: description: id maps to "node.id" configuration in KRaft mode, and it maps to "broker.id" configuration in ZooKeeper mode. diff --git a/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml b/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml index 9fe66759d..5dbfe2653 100644 --- a/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml +++ b/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml @@ -12675,6 +12675,14 @@ spec: type: object brokerConfigGroup: type: string + brokerRoles: + description: 'brokerRoles defines the role(s) for this particular + broker node: regular broker, controller, or both. This must + be set in KRaft mode.' + items: + type: string + maxItems: 2 + type: array id: description: id maps to "node.id" configuration in KRaft mode, and it maps to "broker.id" configuration in ZooKeeper mode. diff --git a/go.mod b/go.mod index 00cbd7668..be37eca21 100644 --- a/go.mod +++ b/go.mod @@ -73,7 +73,7 @@ require ( github.com/google/gnostic v0.6.9 // indirect github.com/google/go-cmp v0.5.9 github.com/google/gofuzz v1.2.0 // indirect - github.com/google/uuid v1.3.0 // indirect + github.com/google/uuid v1.3.0 github.com/hashicorp/errwrap v1.1.0 // indirect github.com/hashicorp/go-multierror v1.1.1 // indirect github.com/hashicorp/go-uuid v1.0.3 // indirect From 314ea41da58d0b86475e6de9b7d656591032dbd2 Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Thu, 20 Jul 2023 17:41:25 -0400 Subject: [PATCH 05/22] Support running Kafka cluster in KRaft mode --- pkg/resources/cruisecontrol/configmap.go | 19 +- pkg/resources/envoy/poddisruptionbudget.go | 4 +- pkg/resources/kafka/configmap.go | 287 ++++++++++----- pkg/resources/kafka/configmap_test.go | 3 +- pkg/resources/kafka/kafka.go | 17 +- pkg/resources/kafka/pod.go | 16 +- pkg/resources/kafka/util.go | 85 +++++ pkg/resources/kafka/util_test.go | 342 ++++++++++++++++++ pkg/resources/kafka/wait-for-envoy-sidecar.sh | 6 + pkg/util/kafka/const.go | 25 +- pkg/util/kafka/template.go | 2 + 11 files changed, 697 insertions(+), 109 deletions(-) create mode 100644 pkg/resources/kafka/util.go create mode 100644 pkg/resources/kafka/util_test.go diff --git a/pkg/resources/cruisecontrol/configmap.go b/pkg/resources/cruisecontrol/configmap.go index 63e7eeac9..bdade79e1 100644 --- a/pkg/resources/cruisecontrol/configmap.go +++ b/pkg/resources/cruisecontrol/configmap.go @@ -60,10 +60,21 @@ func (r *Reconciler) configMap(clientPass string, capacityConfig string, log log log.Error(err, fmt.Sprintf("setting '%s' in Cruise Control configuration failed", kafkautils.KafkaConfigBoostrapServers), "config", bootstrapServers) } - // Add Zookeeper configuration - zkConnect := zookeeperutils.PrepareConnectionAddress(r.KafkaCluster.Spec.ZKAddresses, r.KafkaCluster.Spec.GetZkPath()) - if err = ccConfig.Set(kafkautils.KafkaConfigZooKeeperConnect, zkConnect); err != nil { - log.Error(err, fmt.Sprintf("setting '%s' in Cruise Control configuration failed", kafkautils.KafkaConfigZooKeeperConnect), "config", zkConnect) + if !r.KafkaCluster.Spec.KraftMode() { + // Add Zookeeper configuration when we are in Zookeeper mode only + zkConnect := zookeeperutils.PrepareConnectionAddress(r.KafkaCluster.Spec.ZKAddresses, r.KafkaCluster.Spec.GetZkPath()) + if err = ccConfig.Set(kafkautils.KafkaConfigZooKeeperConnect, zkConnect); err != nil { + log.Error(err, fmt.Sprintf("setting '%s' in Cruise Control configuration failed", kafkautils.KafkaConfigZooKeeperConnect), "config", zkConnect) + } + } else { + // Set configurations to have Cruise Control to run without Zookeeper + if err = ccConfig.Set(kafkautils.CruiseControlConfigTopicConfigProviderClass, kafkautils.CruiseControlConfigTopicConfigProviderClassVal); err != nil { + log.Error(err, fmt.Sprintf("setting '%s' in Cruise Control configuration failed", kafkautils.CruiseControlConfigTopicConfigProviderClass), "config", kafkautils.CruiseControlConfigTopicConfigProviderClassVal) + } + + if err = ccConfig.Set(kafkautils.CruiseControlConfigKafkaBrokerFailureDetectionEnable, kafkautils.CruiseControlConfigKafkaBrokerFailureDetectionEnableVal); err != nil { + log.Error(err, fmt.Sprintf("setting '%s' in Cruise Control configuration failed", kafkautils.CruiseControlConfigKafkaBrokerFailureDetectionEnable), "config", kafkautils.CruiseControlConfigKafkaBrokerFailureDetectionEnableVal) + } } // Add SSL configuration diff --git a/pkg/resources/envoy/poddisruptionbudget.go b/pkg/resources/envoy/poddisruptionbudget.go index 52cb5a431..c9a99a405 100644 --- a/pkg/resources/envoy/poddisruptionbudget.go +++ b/pkg/resources/envoy/poddisruptionbudget.go @@ -50,14 +50,14 @@ func (r *Reconciler) podDisruptionBudget(log logr.Logger, extListener v1beta1.Ex var spec policyv1.PodDisruptionBudgetSpec var matchLabels = labelsForEnvoyIngress(r.KafkaCluster.GetName(), eListenerLabelName) - if pdbConfig.Stategy == MIN_AVAILABLE { + if pdbConfig.Strategy == MIN_AVAILABLE { spec = policyv1.PodDisruptionBudgetSpec{ MinAvailable: &budget, Selector: &metav1.LabelSelector{ MatchLabels: matchLabels, }, } - } else if pdbConfig.Stategy == MAX_UNAVAILABLE { + } else if pdbConfig.Strategy == MAX_UNAVAILABLE { spec = policyv1.PodDisruptionBudgetSpec{ Selector: &metav1.LabelSelector{ MatchLabels: matchLabels, diff --git a/pkg/resources/kafka/configmap.go b/pkg/resources/kafka/configmap.go index 33b4fdffa..3f1a6e2c6 100644 --- a/pkg/resources/kafka/configmap.go +++ b/pkg/resources/kafka/configmap.go @@ -21,6 +21,7 @@ import ( "strings" "emperror.dev/errors" + zookeeperutils "github.com/banzaicloud/koperator/pkg/util/zookeeper" "github.com/go-logr/logr" apierrors "k8s.io/apimachinery/pkg/api/errors" "sigs.k8s.io/controller-runtime/pkg/client" @@ -34,45 +35,76 @@ import ( "github.com/banzaicloud/koperator/pkg/resources/templates" "github.com/banzaicloud/koperator/pkg/util" kafkautils "github.com/banzaicloud/koperator/pkg/util/kafka" - zookeeperutils "github.com/banzaicloud/koperator/pkg/util/zookeeper" properties "github.com/banzaicloud/koperator/properties/pkg" ) -func (r *Reconciler) getConfigProperties(bConfig *v1beta1.BrokerConfig, id int32, +func (r *Reconciler) getConfigProperties(bConfig *v1beta1.BrokerConfig, broker v1beta1.Broker, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses map[string]v1beta1.ListenerStatusList, serverPasses map[string]string, clientPass string, superUsers []string, log logr.Logger) *properties.Properties { config := properties.NewProperties() - // Add listener configuration - listenerConf := generateListenerSpecificConfig(&r.KafkaCluster.Spec.ListenersConfig, serverPasses, log) - config.Merge(listenerConf) + bootstrapServers, err := kafkautils.GetBootstrapServersService(r.KafkaCluster) + if err != nil { + log.Error(err, "getting Kafka bootstrap servers for Cruise Control failed") + } - // Add listener configuration - advertisedListenerConf := generateAdvertisedListenerConfig(id, r.KafkaCluster.Spec.ListenersConfig, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses) - if len(advertisedListenerConf) > 0 { - if err := config.Set(kafkautils.KafkaConfigAdvertisedListeners, advertisedListenerConf); err != nil { - log.Error(err, fmt.Sprintf("setting '%s' in broker configuration resulted an error", kafkautils.KafkaConfigAdvertisedListeners)) - } + // Cruise Control metrics reporter configuration + configCCMetricsReporter(r.KafkaCluster, config, clientPass, bootstrapServers, log) + + // Kafka Broker configurations + if r.KafkaCluster.Spec.KraftMode() { + configureBrokerKRaftMode(broker, r.KafkaCluster, config, bootstrapServers, serverPasses, extListenerStatuses, + intListenerStatuses, controllerIntListenerStatuses, log) + } else { + configureBrokerZKMode(broker.Id, r.KafkaCluster, config, serverPasses, extListenerStatuses, + intListenerStatuses, controllerIntListenerStatuses, log) } - // Add control plane listener - cclConf := generateControlPlaneListener(r.KafkaCluster.Spec.ListenersConfig.InternalListeners) - if cclConf != "" { - if err := config.Set(kafkautils.KafkaConfigControlPlaneListener, cclConf); err != nil { - log.Error(err, fmt.Sprintf("setting '%s' parameter in broker configuration resulted an error", kafkautils.KafkaConfigControlPlaneListener)) + // This logic prevents the removal of the mountPath from the broker configmap + brokerConfigMapName := fmt.Sprintf(brokerConfigTemplate+"-%d", r.KafkaCluster.Name, broker.Id) + var brokerConfigMapOld v1.ConfigMap + err = r.Client.Get(context.Background(), client.ObjectKey{Name: brokerConfigMapName, Namespace: r.KafkaCluster.GetNamespace()}, &brokerConfigMapOld) + if err != nil && !apierrors.IsNotFound(err) { + log.Error(err, "getting broker configmap from the Kubernetes API server resulted an error") + } + + mountPathsOld, err := getMountPathsFromBrokerConfigMap(&brokerConfigMapOld) + if err != nil { + log.Error(err, "could not get mountPaths from broker configmap", v1beta1.BrokerIdLabelKey, broker.Id) + } + + mountPathsNew := generateStorageConfig(bConfig.StorageConfigs) + mountPathsMerged, isMountPathRemoved := mergeMountPaths(mountPathsOld, mountPathsNew) + + if isMountPathRemoved { + log.Error(errors.New("removed storage is found in the KafkaCluster CR"), + "removing storage from broker is not supported", v1beta1.BrokerIdLabelKey, broker.Id, "mountPaths", + mountPathsOld, "mountPaths in kafkaCluster CR ", mountPathsNew) + } + + if len(mountPathsMerged) != 0 { + if err := config.Set(kafkautils.KafkaConfigBrokerLogDirectory, strings.Join(mountPathsMerged, ",")); err != nil { + log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.KafkaConfigBrokerLogDirectory)) } } - // Add Zookeeper configuration - if err := config.Set(kafkautils.KafkaConfigZooKeeperConnect, zookeeperutils.PrepareConnectionAddress(r.KafkaCluster.Spec.ZKAddresses, r.KafkaCluster.Spec.GetZkPath())); err != nil { - log.Error(err, fmt.Sprintf("setting '%s' parameter in broker configuration resulted an error", kafkautils.KafkaConfigZooKeeperConnect)) + // Add superuser configuration + su := strings.Join(generateSuperUsers(superUsers), ";") + if su != "" { + if err := config.Set(kafkautils.KafkaConfigSuperUsers, su); err != nil { + log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.KafkaConfigSuperUsers)) + } } + return config +} +func configCCMetricsReporter(kafkaCluster *v1beta1.KafkaCluster, config *properties.Properties, clientPass, bootstrapServers string, log logr.Logger) { // Add Cruise Control Metrics Reporter SSL configuration - if util.IsSSLEnabledForInternalCommunication(r.KafkaCluster.Spec.ListenersConfig.InternalListeners) { - if !r.KafkaCluster.Spec.IsClientSSLSecretPresent() { + if util.IsSSLEnabledForInternalCommunication(kafkaCluster.Spec.ListenersConfig.InternalListeners) { + if !kafkaCluster.Spec.IsClientSSLSecretPresent() { log.Error(errors.New("cruise control metrics reporter needs ssl but client certificate hasn't specified"), "") } + keyStoreLoc := clientKeystorePath + "/" + v1alpha1.TLSJKSKeyStore trustStoreLoc := clientKeystorePath + "/" + v1alpha1.TLSJKSTrustStore @@ -92,58 +124,114 @@ func (r *Reconciler) getConfigProperties(bConfig *v1beta1.BrokerConfig, id int32 } // Add Cruise Control Metrics Reporter configuration - if err := config.Set(kafkautils.CruiseControlConfigMetricsReporters, "com.linkedin.kafka.cruisecontrol.metricsreporter.CruiseControlMetricsReporter"); err != nil { - log.Error(err, fmt.Sprintf("setting '%s' in broker configuration resulted an error", kafkautils.CruiseControlConfigMetricsReporters)) - } - bootstrapServers, err := kafkautils.GetBootstrapServersService(r.KafkaCluster) - if err != nil { - log.Error(err, "getting Kafka bootstrap servers for Cruise Control failed") + if err := config.Set(kafkautils.CruiseControlConfigMetricsReporters, kafkautils.CruiseControlConfigMetricsReportersVal); err != nil { + log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.CruiseControlConfigMetricsReporters)) } if err := config.Set(kafkautils.CruiseControlConfigMetricsReportersBootstrapServers, bootstrapServers); err != nil { - log.Error(err, fmt.Sprintf("setting '%s' in broker configuration resulted an error", kafkautils.CruiseControlConfigMetricsReportersBootstrapServers)) + log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.CruiseControlConfigMetricsReportersBootstrapServers)) } if err := config.Set(kafkautils.CruiseControlConfigMetricsReporterK8sMode, true); err != nil { - log.Error(err, fmt.Sprintf("setting '%s' in broker configuration resulted an error", kafkautils.CruiseControlConfigMetricsReporterK8sMode)) + log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.CruiseControlConfigMetricsReporterK8sMode)) } +} - // Kafka Broker configuration - if err := config.Set(kafkautils.KafkaConfigBrokerId, id); err != nil { - log.Error(err, fmt.Sprintf("setting '%s' in broker configuration resulted an error", kafkautils.KafkaConfigBrokerId)) +func configureBrokerKRaftMode(broker v1beta1.Broker, kafkaCluster *v1beta1.KafkaCluster, config *properties.Properties, + bootstrapServers string, serverPasses map[string]string, extListenerStatuses, intListenerStatuses, + controllerIntListenerStatuses map[string]v1beta1.ListenerStatusList, log logr.Logger) { + if err := config.Set(kafkautils.KafkaConfigNodeID, broker.Id); err != nil { + log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.KafkaConfigNodeID)) } - // This logic prevents the removal of the mountPath from the broker configmap - brokerConfigMapName := fmt.Sprintf(brokerConfigTemplate+"-%d", r.KafkaCluster.Name, id) - var brokerConfigMapOld v1.ConfigMap - err = r.Client.Get(context.Background(), client.ObjectKey{Name: brokerConfigMapName, Namespace: r.KafkaCluster.GetNamespace()}, &brokerConfigMapOld) - if err != nil && !apierrors.IsNotFound(err) { - log.Error(err, "getting broker configmap from the Kubernetes API server resulted an error") + if err := config.Set(kafkautils.KafkaConfigProcessRoles, broker.Roles); err != nil { + log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.KafkaConfigProcessRoles)) } - mountPathsOld, err := getMountPathsFromBrokerConfigMap(&brokerConfigMapOld) - if err != nil { - log.Error(err, "could not get mountPaths from broker configmap", v1beta1.BrokerIdLabelKey, id) + if err := config.Set(kafkautils.KafkaConfigControllerQuorumVoters, generateQuorumVoters(kafkaCluster.Spec.Brokers, controllerIntListenerStatuses)); err != nil { + log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.KafkaConfigControllerQuorumVoters)) } - mountPathsNew := generateStorageConfig(bConfig.StorageConfigs) - mountPathsMerged, isMountPathRemoved := mergeMountPaths(mountPathsOld, mountPathsNew) - if isMountPathRemoved { - log.Error(errors.New("removed storage is found in the KafkaCluster CR"), "removing storage from broker is not supported", v1beta1.BrokerIdLabelKey, id, "mountPaths", mountPathsOld, "mountPaths in kafkaCluster CR ", mountPathsNew) + controllerListenerName := generateControlPlaneListener(kafkaCluster.Spec.ListenersConfig.InternalListeners) + if controllerListenerName != "" { + if err := config.Set(kafkautils.KafkaConfigControllerListenerName, controllerListenerName); err != nil { + log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.KafkaConfigControllerListenerName)) + } } - if len(mountPathsMerged) != 0 { - if err := config.Set(kafkautils.KafkaConfigBrokerLogDirectory, strings.Join(mountPathsMerged, ",")); err != nil { - log.Error(err, fmt.Sprintf("setting '%s' in broker configuration resulted an error", kafkautils.KafkaConfigBrokerLogDirectory)) + // Add listener configuration + listenerConf, listenerConfig := generateListenerSpecificConfig(&kafkaCluster.Spec.ListenersConfig, serverPasses, log) + config.Merge(listenerConf) + + var advertisedListenerConf []string + + // expose controller listener in the listeners configuration only if this broker is a controller + // also expose only controller listener as the advertised.listeners configuration when this broker is a controller + if !isControllerNode(broker.Roles) { + advertisedListenerConf = generateAdvertisedListenerConfig(broker.Id, kafkaCluster.Spec.ListenersConfig, + extListenerStatuses, intListenerStatuses, nil) + if err := config.Set(kafkautils.KafkaConfigAdvertisedListeners, advertisedListenerConf); err != nil { + log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.KafkaConfigAdvertisedListeners)) } } - // Add superuser configuration - su := strings.Join(generateSuperUsers(superUsers), ";") - if su != "" { - if err := config.Set(kafkautils.KafkaConfigSuperUsers, su); err != nil { - log.Error(err, fmt.Sprintf("setting '%s' in broker configuration resulted an error", kafkautils.KafkaConfigSuperUsers)) + // "listeners" configuration can only contain controller configuration when the node is a controller-only node + if isControllerNodeOnly(broker.Roles) { + for _, listener := range listenerConfig { + if listener[:len(controllerListenerName)] == strings.ToUpper(controllerListenerName) { + if err := config.Set(kafkautils.KafkaConfigListeners, listener); err != nil { + log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.KafkaConfigListeners)) + } + break + } + } + } + + // "listeners" configuration cannot contain controller configuration when the node is a broker-only node + var nonControllerListener []string + if isBrokerNodeOnly(broker.Roles) { + for _, listener := range listenerConfig { + if listener[:len(controllerListenerName)] != strings.ToUpper(controllerListenerName) { + nonControllerListener = append(nonControllerListener, listener) + } + } + if err := config.Set(kafkautils.KafkaConfigListeners, nonControllerListener); err != nil { + log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.KafkaConfigListeners)) } } - return config +} + +func configureBrokerZKMode(brokerID int32, kafkaCluster *v1beta1.KafkaCluster, config *properties.Properties, + serverPasses map[string]string, extListenerStatuses, intListenerStatuses, + controllerIntListenerStatuses map[string]v1beta1.ListenerStatusList, log logr.Logger) { + if err := config.Set(kafkautils.KafkaConfigBrokerID, brokerID); err != nil { + log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.KafkaConfigBrokerID)) + } + + // Add listener configuration + listenerConf, _ := generateListenerSpecificConfig(&kafkaCluster.Spec.ListenersConfig, serverPasses, log) + config.Merge(listenerConf) + + // Add advertised listener configuration + advertisedListenerConf := generateAdvertisedListenerConfig(brokerID, kafkaCluster.Spec.ListenersConfig, + extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses) + if len(advertisedListenerConf) > 0 { + if err := config.Set(kafkautils.KafkaConfigAdvertisedListeners, advertisedListenerConf); err != nil { + log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.KafkaConfigAdvertisedListeners)) + } + } + + // Add control plane listener + cclConf := generateControlPlaneListener(kafkaCluster.Spec.ListenersConfig.InternalListeners) + if cclConf != "" { + if err := config.Set(kafkautils.KafkaConfigControlPlaneListener, cclConf); err != nil { + log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.KafkaConfigControlPlaneListener)) + } + } + + // Add Zookeeper configuration + if err := config.Set(kafkautils.KafkaConfigZooKeeperConnect, zookeeperutils.PrepareConnectionAddress( + kafkaCluster.Spec.ZKAddresses, kafkaCluster.Spec.GetZkPath())); err != nil { + log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.KafkaConfigZooKeeperConnect)) + } } // mergeMountPaths is merges the new mountPaths with the old. @@ -179,19 +267,19 @@ func generateSuperUsers(users []string) (suStrings []string) { return } -func (r *Reconciler) configMap(id int32, brokerConfig *v1beta1.BrokerConfig, extListenerStatuses, +func (r *Reconciler) configMap(broker v1beta1.Broker, brokerConfig *v1beta1.BrokerConfig, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses map[string]v1beta1.ListenerStatusList, serverPasses map[string]string, clientPass string, superUsers []string, log logr.Logger) *corev1.ConfigMap { brokerConf := &corev1.ConfigMap{ ObjectMeta: templates.ObjectMeta( - fmt.Sprintf(brokerConfigTemplate+"-%d", r.KafkaCluster.Name, id), + fmt.Sprintf(brokerConfigTemplate+"-%d", r.KafkaCluster.Name, broker.Id), apiutil.MergeLabels( apiutil.LabelsForKafka(r.KafkaCluster.Name), - map[string]string{v1beta1.BrokerIdLabelKey: fmt.Sprintf("%d", id)}, + map[string]string{v1beta1.BrokerIdLabelKey: fmt.Sprintf("%d", broker.Id)}, ), r.KafkaCluster, ), - Data: map[string]string{kafkautils.ConfigPropertyName: r.generateBrokerConfig(id, brokerConfig, extListenerStatuses, + Data: map[string]string{kafkautils.ConfigPropertyName: r.generateBrokerConfig(broker, brokerConfig, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses, serverPasses, clientPass, superUsers, log)}, } if brokerConfig.Log4jConfig != "" { @@ -264,15 +352,45 @@ func generateControlPlaneListener(iListeners []v1beta1.InternalListenerConfig) s return controlPlaneListener } -func generateListenerSpecificConfig(l *v1beta1.ListenersConfig, serverPasses map[string]string, log logr.Logger) *properties.Properties { +func generateListenerSpecificConfig(l *v1beta1.ListenersConfig, serverPasses map[string]string, log logr.Logger) (*properties.Properties, []string) { + config := properties.NewProperties() + + interBrokerListenerName, securityProtocolMapConfig, listenerConfig, internalListenerSSLConfig, externalListenerSSLConfig := getListenerSpecificConfig(l, serverPasses, log) + + for k, v := range internalListenerSSLConfig { + if err := config.Set(k, v); err != nil { + log.Error(err, fmt.Sprintf("setting '%s' parameter in broker configuration resulted an error", k)) + } + } + + for k, v := range externalListenerSSLConfig { + if err := config.Set(k, v); err != nil { + log.Error(err, fmt.Sprintf("setting '%s' parameter in broker configuration resulted an error", k)) + } + } + + if err := config.Set(kafkautils.KafkaConfigListenerSecurityProtocolMap, securityProtocolMapConfig); err != nil { + log.Error(err, fmt.Sprintf("setting '%s' parameter in broker configuration resulted an error", kafkautils.KafkaConfigListenerSecurityProtocolMap)) + } + if err := config.Set(kafkautils.KafkaConfigInterBrokerListenerName, interBrokerListenerName); err != nil { + log.Error(err, fmt.Sprintf("setting '%s' parameter in broker configuration resulted an error", kafkautils.KafkaConfigInterBrokerListenerName)) + } + if err := config.Set(kafkautils.KafkaConfigListeners, listenerConfig); err != nil { + log.Error(err, fmt.Sprintf("setting '%s' parameter in broker configuration resulted an error", kafkautils.KafkaConfigListeners)) + } + + return config, listenerConfig +} + +func getListenerSpecificConfig(l *v1beta1.ListenersConfig, serverPasses map[string]string, log logr.Logger) (string, []string, []string, map[string]string, map[string]string) { var ( interBrokerListenerName string securityProtocolMapConfig []string listenerConfig []string + internalListenerSSLConfig map[string]string + externalListenerSSLConfig map[string]string ) - config := properties.NewProperties() - for _, iListener := range l.InternalListeners { if iListener.UsedForInnerBrokerCommunication { if interBrokerListenerName == "" { @@ -285,9 +403,10 @@ func generateListenerSpecificConfig(l *v1beta1.ListenersConfig, serverPasses map upperedListenerName := strings.ToUpper(iListener.Name) securityProtocolMapConfig = append(securityProtocolMapConfig, fmt.Sprintf("%s:%s", upperedListenerName, upperedListenerType)) listenerConfig = append(listenerConfig, fmt.Sprintf("%s://:%d", upperedListenerName, iListener.ContainerPort)) + // Add internal listeners SSL configuration if iListener.Type == v1beta1.SecurityProtocolSSL { - generateListenerSSLConfig(config, iListener.Name, iListener.SSLClientAuth, serverPasses[iListener.Name], log) + internalListenerSSLConfig = generateListenerSSLConfig(iListener.Name, iListener.SSLClientAuth, serverPasses[iListener.Name]) } } @@ -298,22 +417,14 @@ func generateListenerSpecificConfig(l *v1beta1.ListenersConfig, serverPasses map listenerConfig = append(listenerConfig, fmt.Sprintf("%s://:%d", upperedListenerName, eListener.ContainerPort)) // Add external listeners SSL configuration if eListener.Type == v1beta1.SecurityProtocolSSL { - generateListenerSSLConfig(config, eListener.Name, eListener.SSLClientAuth, serverPasses[eListener.Name], log) + externalListenerSSLConfig = generateListenerSSLConfig(eListener.Name, eListener.SSLClientAuth, serverPasses[eListener.Name]) } } - if err := config.Set(kafkautils.KafkaConfigListenerSecurityProtocolMap, securityProtocolMapConfig); err != nil { - log.Error(err, fmt.Sprintf("setting '%s' parameter in broker configuration resulted an error", kafkautils.KafkaConfigListenerSecurityProtocolMap)) - } - if err := config.Set(kafkautils.KafkaConfigInterBrokerListenerName, interBrokerListenerName); err != nil { - log.Error(err, fmt.Sprintf("setting '%s' parameter in broker configuration resulted an error", kafkautils.KafkaConfigInterBrokerListenerName)) - } - if err := config.Set(kafkautils.KafkaConfigListeners, listenerConfig); err != nil { - log.Error(err, fmt.Sprintf("setting '%s' parameter in broker configuration resulted an error", kafkautils.KafkaConfigListeners)) - } - return config + + return interBrokerListenerName, securityProtocolMapConfig, listenerConfig, internalListenerSSLConfig, externalListenerSSLConfig } -func generateListenerSSLConfig(config *properties.Properties, name string, sslClientAuth v1beta1.SSLClientAuthentication, password string, log logr.Logger) { +func generateListenerSSLConfig(name string, sslClientAuth v1beta1.SSLClientAuthentication, password string) map[string]string { var listenerSSLConfig map[string]string namedKeystorePath := fmt.Sprintf(listenerServerKeyStorePathTemplate, serverKeystorePath, name) keyStoreType := "JKS" @@ -337,11 +448,7 @@ func generateListenerSSLConfig(config *properties.Properties, name string, sslCl listenerSSLConfig[fmt.Sprintf("%s.%s.%s", kafkautils.KafkaConfigListenerName, name, kafkautils.KafkaConfigSSLClientAuth)] = string(sslClientAuth) } - for k, v := range listenerSSLConfig { - if err := config.Set(k, v); err != nil { - log.Error(err, fmt.Sprintf("setting '%s' parameter in broker configuration resulted an error", k)) - } - } + return listenerSSLConfig } // mergeSuperUsersPropertyValue merges the target and source super.users property value, and returns it as string. @@ -382,13 +489,14 @@ func mergeSuperUsersPropertyValue(source *properties.Properties, target *propert return "" } -func (r Reconciler) generateBrokerConfig(id int32, brokerConfig *v1beta1.BrokerConfig, extListenerStatuses, +func (r Reconciler) generateBrokerConfig(broker v1beta1.Broker, brokerConfig *v1beta1.BrokerConfig, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses map[string]v1beta1.ListenerStatusList, serverPasses map[string]string, clientPass string, superUsers []string, log logr.Logger) string { - finalBrokerConfig := getBrokerReadOnlyConfig(id, r.KafkaCluster, log) + finalBrokerConfig := getBrokerReadOnlyConfig(broker, r.KafkaCluster, log) // Get operator generated configuration - opGenConf := r.getConfigProperties(brokerConfig, id, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses, serverPasses, clientPass, superUsers, log) + opGenConf := r.getConfigProperties(brokerConfig, broker, extListenerStatuses, intListenerStatuses, + controllerIntListenerStatuses, serverPasses, clientPass, superUsers, log) // Merge operator generated configuration to the final one if opGenConf != nil { @@ -408,7 +516,7 @@ func (r Reconciler) generateBrokerConfig(id int32, brokerConfig *v1beta1.BrokerC } // TODO move this into api in the future (adamantal) -func getBrokerReadOnlyConfig(id int32, kafkaCluster *v1beta1.KafkaCluster, log logr.Logger) *properties.Properties { +func getBrokerReadOnlyConfig(broker v1beta1.Broker, kafkaCluster *v1beta1.KafkaCluster, log logr.Logger) *properties.Properties { // Parse cluster-wide readonly configuration finalBrokerConfig, err := properties.NewFromString(kafkaCluster.Spec.ReadOnlyConfig) if err != nil { @@ -416,16 +524,9 @@ func getBrokerReadOnlyConfig(id int32, kafkaCluster *v1beta1.KafkaCluster, log l } // Parse readonly broker configuration - var parsedReadOnlyBrokerConfig *properties.Properties - // Find configuration for broker with id - for _, broker := range kafkaCluster.Spec.Brokers { - if broker.Id == id { - parsedReadOnlyBrokerConfig, err = properties.NewFromString(broker.ReadOnlyConfig) - if err != nil { - log.Error(err, fmt.Sprintf("failed to parse readonly broker configuration for broker with id: %d", id)) - } - break - } + parsedReadOnlyBrokerConfig, err := properties.NewFromString(broker.ReadOnlyConfig) + if err != nil { + log.Error(err, fmt.Sprintf("failed to parse readonly broker configuration for broker with id: %d", broker.Id)) } // Merge cluster-wide configuration into broker-level configuration diff --git a/pkg/resources/kafka/configmap_test.go b/pkg/resources/kafka/configmap_test.go index 615a8d6e8..67975db32 100644 --- a/pkg/resources/kafka/configmap_test.go +++ b/pkg/resources/kafka/configmap_test.go @@ -683,7 +683,8 @@ zookeeper.connect=example.zk:2181/`, superUsers = []string{"CN=kafka-headless.kafka.svc.cluster.local"} } - generatedConfig := r.generateBrokerConfig(0, r.KafkaCluster.Spec.Brokers[0].BrokerConfig, map[string]v1beta1.ListenerStatusList{}, map[string]v1beta1.ListenerStatusList{}, controllerListenerStatus, serverPasses, clientPass, superUsers, logr.Discard()) + generatedConfig := r.generateBrokerConfig(r.KafkaCluster.Spec.Brokers[0], r.KafkaCluster.Spec.Brokers[0].BrokerConfig, map[string]v1beta1.ListenerStatusList{}, + map[string]v1beta1.ListenerStatusList{}, controllerListenerStatus, serverPasses, clientPass, superUsers, logr.Discard()) generated, err := properties.NewFromString(generatedConfig) if err != nil { diff --git a/pkg/resources/kafka/kafka.go b/pkg/resources/kafka/kafka.go index 81d91a7ff..be4f63863 100644 --- a/pkg/resources/kafka/kafka.go +++ b/pkg/resources/kafka/kafka.go @@ -308,6 +308,19 @@ func (r *Reconciler) Reconcile(log logr.Logger) error { reorderedBrokers := reorderBrokers(runningBrokers, boundPersistentVolumeClaims, r.KafkaCluster.Spec.Brokers, r.KafkaCluster.Status.BrokersState, controllerID, log) allBrokerDynamicConfigSucceeded := true + + // all broker nodes under the same Kafka cluster must use the same cluster UUID + if r.KafkaCluster.Status.ClusterID == "" { + r.KafkaCluster.Status.ClusterID = generateRandomClusterID() + err = r.Client.Status().Update(ctx, r.KafkaCluster) + if apierrors.IsNotFound(err) { + err = r.Client.Update(ctx, r.KafkaCluster) + } + if err != nil { + return errors.WrapIf(err, "could not update cluster UUID status") + } + } + for _, broker := range reorderedBrokers { brokerConfig, err := broker.GetBrokerConfig(r.KafkaCluster.Spec) if err != nil { @@ -316,14 +329,14 @@ func (r *Reconciler) Reconcile(log logr.Logger) error { var configMap *corev1.ConfigMap if r.KafkaCluster.Spec.RackAwareness == nil { - configMap = r.configMap(broker.Id, brokerConfig, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses, serverPasses, clientPass, superUsers, log) + configMap = r.configMap(broker, brokerConfig, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses, serverPasses, clientPass, superUsers, log) err := k8sutil.Reconcile(log, r.Client, configMap, r.KafkaCluster) if err != nil { return errors.WrapIfWithDetails(err, "failed to reconcile resource", "resource", configMap.GetObjectKind().GroupVersionKind()) } } else if brokerState, ok := r.KafkaCluster.Status.BrokersState[strconv.Itoa(int(broker.Id))]; ok { if brokerState.RackAwarenessState != "" { - configMap = r.configMap(broker.Id, brokerConfig, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses, serverPasses, clientPass, superUsers, log) + configMap = r.configMap(broker, brokerConfig, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses, serverPasses, clientPass, superUsers, log) err := k8sutil.Reconcile(log, r.Client, configMap, r.KafkaCluster) if err != nil { return errors.WrapIfWithDetails(err, "failed to reconcile resource", "resource", configMap.GetObjectKind().GroupVersionKind()) diff --git a/pkg/resources/kafka/pod.go b/pkg/resources/kafka/pod.go index 227cfdbc6..0e56a4022 100644 --- a/pkg/resources/kafka/pod.go +++ b/pkg/resources/kafka/pod.go @@ -43,6 +43,7 @@ var ( func (r *Reconciler) pod(id int32, brokerConfig *v1beta1.BrokerConfig, pvcs []corev1.PersistentVolumeClaim, log logr.Logger) runtime.Object { var kafkaBrokerContainerPorts []corev1.ContainerPort + const kafkaContainerName = "kafka" for _, eListener := range r.KafkaCluster.Spec.ListenersConfig.ExternalListeners { if _, ok := r.KafkaCluster.Status.ListenerStatuses.ExternalListeners[eListener.Name]; !ok { @@ -100,7 +101,7 @@ func (r *Reconciler) pod(id int32, brokerConfig *v1beta1.BrokerConfig, pvcs []co Affinity: getAffinity(brokerConfig, r.KafkaCluster), Containers: append([]corev1.Container{ { - Name: "kafka", + Name: kafkaContainerName, Image: util.GetBrokerImage(brokerConfig, r.KafkaCluster.Spec.GetClusterImage()), Lifecycle: &corev1.Lifecycle{ PreStop: &corev1.LifecycleHandler{ @@ -167,6 +168,19 @@ fi`}, pod.Spec.Subdomain = fmt.Sprintf(kafkautils.HeadlessServiceTemplate, r.KafkaCluster.Name) } + // in KRaft mode, all broker nodes within the same Kafka cluster need to use the same cluster UUID to format the storage + if r.KafkaCluster.Spec.KraftMode() { + for i, container := range pod.Spec.Containers { + if container.Name == kafkaContainerName { + pod.Spec.Containers[i].Env = append(pod.Spec.Containers[i].Env, corev1.EnvVar{ + Name: "CLUSTER_ID", + Value: r.KafkaCluster.Status.ClusterID, + }) + break + } + } + } + return pod } diff --git a/pkg/resources/kafka/util.go b/pkg/resources/kafka/util.go new file mode 100644 index 000000000..6ae9ee3f7 --- /dev/null +++ b/pkg/resources/kafka/util.go @@ -0,0 +1,85 @@ +// Copyright © 2023 Cisco Systems, Inc. and/or its affiliates +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package kafka + +import ( + "encoding/base64" + "fmt" + "sort" + + "github.com/banzaicloud/koperator/api/v1beta1" + "github.com/banzaicloud/koperator/pkg/util" + "github.com/google/uuid" +) + +// generateQuorumVoters generates the quorum voters in the format of brokerID@nodeAddress:listenerPort +// The generated quorum voters are guaranteed in ascending order by broker IDs to ensure same quorum voters configurations are returned +// regardless of the order of brokers and controllerListenerStatuses are passed in - this is needed to avoid triggering +// unnecessary rolling upgrade operations +func generateQuorumVoters(brokers []v1beta1.Broker, controllerListenerStatuses map[string]v1beta1.ListenerStatusList) []string { + var ( + quorumVoters []string + brokerIDs []int32 + ) + idToListenerAddrMap := make(map[int32]string) + + // find the controller nodes and their corresponding listener addresses + for _, b := range brokers { + if isControllerNode(b.Roles) { + for _, controllerListenerStatus := range controllerListenerStatuses { + for _, status := range controllerListenerStatus { + if status.Name == fmt.Sprintf("broker-%d", b.Id) { + idToListenerAddrMap[b.Id] = status.Address + brokerIDs = append(brokerIDs, b.Id) + break + } + } + } + } + } + + sort.Slice(brokerIDs, func(i, j int) bool { + return brokerIDs[i] < brokerIDs[j] + }) + + for _, brokerId := range brokerIDs { + quorumVoters = append(quorumVoters, fmt.Sprintf("%d@%s", brokerId, idToListenerAddrMap[brokerId])) + } + + return quorumVoters +} + +func isBrokerNodeOnly(roles []string) bool { + return isBrokerNode(roles) && !isControllerNode(roles) +} + +func isControllerNodeOnly(roles []string) bool { + return isControllerNode(roles) && !isBrokerNode(roles) +} + +func isBrokerNode(roles []string) bool { + return util.StringSliceContains(roles, "broker") +} + +func isControllerNode(roles []string) bool { + return util.StringSliceContains(roles, "controller") +} + +// generateRandomClusterID() generates a based64-encoded random UUID with 16 bytes as the cluster ID +// it uses URL based64 encoding since that's what Kafka expects +func generateRandomClusterID() string { + randomUUID := uuid.New() + return base64.URLEncoding.EncodeToString(randomUUID[:]) +} diff --git a/pkg/resources/kafka/util_test.go b/pkg/resources/kafka/util_test.go new file mode 100644 index 000000000..064ec95ce --- /dev/null +++ b/pkg/resources/kafka/util_test.go @@ -0,0 +1,342 @@ +// Copyright © 2023 Cisco Systems, Inc. and/or its affiliates +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package kafka + +import ( + "encoding/base64" + "reflect" + "testing" + + "github.com/banzaicloud/koperator/api/v1beta1" +) + +func TestGenerateClusterID(t *testing.T) { + // one random cluster ID serves for the entire Kafka cluster, therefore testing 100000 cluster IDs should be enough + numOfIDs := 100000 + test := make(map[string]bool, numOfIDs) + for i := 0; i < numOfIDs; i++ { + clusterID := generateRandomClusterID() + _, err := base64.URLEncoding.DecodeString(clusterID) + if err != nil { + t.Errorf("expected nil error, got: %v", err) + } + + if test[clusterID] { + t.Error("expected random cluster ID that does not collide with previous ones") + } + + // mark the map to note that this cluster ID has been generated + test[clusterID] = true + } +} + +func TestGenerateQuorumVoters(t *testing.T) { + + tests := []struct { + testName string + brokers []v1beta1.Broker + listenersStatuses map[string]v1beta1.ListenerStatusList + expectedQuorumVoters []string + }{ + { + testName: "brokers with ascending order by IDs; controller listener statuses has the same order as brokers", + brokers: []v1beta1.Broker{ + { + Id: int32(0), + Roles: []string{"broker"}, + }, + { + Id: int32(10), + Roles: []string{"broker"}, + }, + { + Id: int32(20), + Roles: []string{"broker"}, + }, + { + Id: int32(30), + Roles: []string{"broker"}, + }, + { + Id: int32(40), + Roles: []string{"controller"}, + }, + { + Id: int32(50), + Roles: []string{"controller"}, + }, + { + Id: int32(60), + Roles: []string{"controller"}, + }, + }, + listenersStatuses: map[string]v1beta1.ListenerStatusList{ + "test-listener": { + { + Name: "broker-0", + Address: "fakeKafka-0.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-10", + Address: "fakeKafka-10.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-20", + Address: "fakeKafka-20.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-30", + Address: "fakeKafka-30.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-40", + Address: "fakeKafka-40.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-50", + Address: "fakeKafka-50.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-60", + Address: "fakeKafka-60.fakeKafka-headless.default.svc.cluster.local:29093", + }, + }, + }, + expectedQuorumVoters: []string{ + "40@fakeKafka-40.fakeKafka-headless.default.svc.cluster.local:29093", + "50@fakeKafka-50.fakeKafka-headless.default.svc.cluster.local:29093", + "60@fakeKafka-60.fakeKafka-headless.default.svc.cluster.local:29093"}, + }, + { + testName: "brokers with decreasing order by IDs; controller listener statuses has the same order as brokers", + brokers: []v1beta1.Broker{ + { + Id: int32(60), + Roles: []string{"broker"}, + }, + { + Id: int32(50), + Roles: []string{"broker"}, + }, + { + Id: int32(40), + Roles: []string{"broker"}, + }, + { + Id: int32(30), + Roles: []string{"broker"}, + }, + { + Id: int32(20), + Roles: []string{"controller"}, + }, + { + Id: int32(10), + Roles: []string{"controller"}, + }, + { + Id: int32(0), + Roles: []string{"controller"}, + }, + }, + listenersStatuses: map[string]v1beta1.ListenerStatusList{ + "test-listener": { + { + Name: "broker-60", + Address: "fakeKafka-60.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-50", + Address: "fakeKafka-50.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-40", + Address: "fakeKafka-40.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-30", + Address: "fakeKafka-30.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-20", + Address: "fakeKafka-20.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-10", + Address: "fakeKafka-10.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-0", + Address: "fakeKafka-0.fakeKafka-headless.default.svc.cluster.local:29093", + }, + }, + }, + expectedQuorumVoters: []string{ + "0@fakeKafka-0.fakeKafka-headless.default.svc.cluster.local:29093", + "10@fakeKafka-10.fakeKafka-headless.default.svc.cluster.local:29093", + "20@fakeKafka-20.fakeKafka-headless.default.svc.cluster.local:29093"}, + }, + { + testName: "brokers with ascending order by IDs; controller listener statuses has the opposite order as brokers", + brokers: []v1beta1.Broker{ + { + Id: int32(0), + Roles: []string{"broker"}, + }, + { + Id: int32(10), + Roles: []string{"broker"}, + }, + { + Id: int32(20), + Roles: []string{"broker"}, + }, + { + Id: int32(30), + Roles: []string{"broker"}, + }, + { + Id: int32(40), + Roles: []string{"controller"}, + }, + { + Id: int32(50), + Roles: []string{"controller"}, + }, + { + Id: int32(60), + Roles: []string{"controller"}, + }, + }, + listenersStatuses: map[string]v1beta1.ListenerStatusList{ + "test-listener": { + { + Name: "broker-60", + Address: "fakeKafka-60.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-50", + Address: "fakeKafka-50.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-40", + Address: "fakeKafka-40.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-30", + Address: "fakeKafka-30.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-20", + Address: "fakeKafka-20.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-10", + Address: "fakeKafka-10.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-0", + Address: "fakeKafka-0.fakeKafka-headless.default.svc.cluster.local:29093", + }, + }, + }, + expectedQuorumVoters: []string{ + "40@fakeKafka-40.fakeKafka-headless.default.svc.cluster.local:29093", + "50@fakeKafka-50.fakeKafka-headless.default.svc.cluster.local:29093", + "60@fakeKafka-60.fakeKafka-headless.default.svc.cluster.local:29093"}, + }, + { + testName: "brokers and controller listener statuses with random order", + brokers: []v1beta1.Broker{ + { + Id: int32(100), + Roles: []string{"broker", "controller"}, + }, + { + Id: int32(50), + Roles: []string{"broker"}, + }, + { + Id: int32(80), + Roles: []string{"controller"}, + }, + { + Id: int32(30), + Roles: []string{"broker"}, + }, + { + Id: int32(90), + Roles: []string{"controller"}, + }, + { + Id: int32(40), + Roles: []string{"broker"}, + }, + { + Id: int32(60), + Roles: []string{"controller"}, + }, + }, + listenersStatuses: map[string]v1beta1.ListenerStatusList{ + "test-listener": { + { + Name: "broker-30", + Address: "fakeKafka-30.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-50", + Address: "fakeKafka-50.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-60", + Address: "fakeKafka-60.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-100", + Address: "fakeKafka-100.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-80", + Address: "fakeKafka-80.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-90", + Address: "fakeKafka-90.fakeKafka-headless.default.svc.cluster.local:29093", + }, + { + Name: "broker-40", + Address: "fakeKafka-40.fakeKafka-headless.default.svc.cluster.local:29093", + }, + }, + }, + expectedQuorumVoters: []string{ + "60@fakeKafka-60.fakeKafka-headless.default.svc.cluster.local:29093", + "80@fakeKafka-80.fakeKafka-headless.default.svc.cluster.local:29093", + "90@fakeKafka-90.fakeKafka-headless.default.svc.cluster.local:29093", + "100@fakeKafka-100.fakeKafka-headless.default.svc.cluster.local:29093", + }, + }, + } + + for _, test := range tests { + t.Run(test.testName, func(t *testing.T) { + gotQuorumVoters := generateQuorumVoters(test.brokers, test.listenersStatuses) + if !reflect.DeepEqual(gotQuorumVoters, test.expectedQuorumVoters) { + t.Error("Expected:", test.expectedQuorumVoters, "Got:", gotQuorumVoters) + } + }) + } + +} diff --git a/pkg/resources/kafka/wait-for-envoy-sidecar.sh b/pkg/resources/kafka/wait-for-envoy-sidecar.sh index 48fdb03d5..1c5e8b61c 100644 --- a/pkg/resources/kafka/wait-for-envoy-sidecar.sh +++ b/pkg/resources/kafka/wait-for-envoy-sidecar.sh @@ -28,5 +28,11 @@ if [[ -n "$ENVOY_SIDECAR_STATUS" ]]; then done fi touch /var/run/wait/do-not-exit-yet + +if [[ -n "$CLUSTER_ID" ]]; then +echo "Formatting storage directories in KRaft mode with cluster ID $CLUSTER_ID" +/opt/kafka/bin/kafka-storage.sh format --cluster-id "$CLUSTER_ID" -c /config/broker-config +fi + /opt/kafka/bin/kafka-server-start.sh /config/broker-config rm /var/run/wait/do-not-exit-yet diff --git a/pkg/util/kafka/const.go b/pkg/util/kafka/const.go index e286db10e..980fcc45c 100644 --- a/pkg/util/kafka/const.go +++ b/pkg/util/kafka/const.go @@ -21,11 +21,18 @@ const ConfigPropertyName = "broker-config" const ( KafkaConfigSuperUsers = "super.users" - KafkaConfigBoostrapServers = "bootstrap.servers" - KafkaConfigZooKeeperConnect = "zookeeper.connect" - KafkaConfigBrokerId = "broker.id" + KafkaConfigBoostrapServers = "bootstrap.servers" + KafkaConfigZooKeeperConnect = "zookeeper.connect" + // KafkaConfigBrokerID is used in ZooKeeper mode + KafkaConfigBrokerID = "broker.id" KafkaConfigBrokerLogDirectory = "log.dirs" + // Configuration keys for KRaft + KafkaConfigNodeID = "node.id" + KafkaConfigProcessRoles = "process.roles" + KafkaConfigControllerQuorumVoters = "controller.quorum.voters" + KafkaConfigControllerListenerName = "controller.listener.names" + KafkaConfigListeners = "listeners" KafkaConfigListenerName = "listener.name" KafkaConfigListenerSecurityProtocolMap = "listener.security.protocol.map" @@ -45,7 +52,13 @@ const ( // used for Cruise Control configurations const ( - CruiseControlConfigMetricsReporters = "metric.reporters" - CruiseControlConfigMetricsReportersBootstrapServers = "cruise.control.metrics.reporter.bootstrap.servers" - CruiseControlConfigMetricsReporterK8sMode = "cruise.control.metrics.reporter.kubernetes.mode" + CruiseControlConfigMetricsReporters = "metric.reporters" + CruiseControlConfigMetricsReportersBootstrapServers = "cruise.control.metrics.reporter.bootstrap.servers" + CruiseControlConfigMetricsReporterK8sMode = "cruise.control.metrics.reporter.kubernetes.mode" + CruiseControlConfigTopicConfigProviderClass = "topic.config.provider.class" + CruiseControlConfigKafkaBrokerFailureDetectionEnable = "kafka.broker.failure.detection.enable" + + CruiseControlConfigMetricsReportersVal = "com.linkedin.kafka.cruisecontrol.metricsreporter.CruiseControlMetricsReporter" + CruiseControlConfigTopicConfigProviderClassVal = "com.linkedin.kafka.cruisecontrol.config.KafkaAdminTopicConfigProvider" + CruiseControlConfigKafkaBrokerFailureDetectionEnableVal = "true" ) diff --git a/pkg/util/kafka/template.go b/pkg/util/kafka/template.go index 32e140c6a..8ccda0523 100644 --- a/pkg/util/kafka/template.go +++ b/pkg/util/kafka/template.go @@ -21,4 +21,6 @@ const ( HeadlessServiceTemplate = "%s-headless" // NodePortServiceTemplate template for Kafka nodeport service NodePortServiceTemplate = "%s-%d-%s" + + BrokerConfigErrorMsgTemplate = "setting '%s' in broker configuration resulted in an error" ) From 7613f06c335e5ff3c06e41f7b10890722504b2e2 Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Thu, 20 Jul 2023 23:01:25 -0400 Subject: [PATCH 06/22] brokerRoles -> processRoles to match upstream Kafka naming --- charts/kafka-operator/templates/crds.yaml | 8 -------- config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml | 8 -------- 2 files changed, 16 deletions(-) diff --git a/charts/kafka-operator/templates/crds.yaml b/charts/kafka-operator/templates/crds.yaml index d71a88524..f33c2a65e 100644 --- a/charts/kafka-operator/templates/crds.yaml +++ b/charts/kafka-operator/templates/crds.yaml @@ -12838,14 +12838,6 @@ spec: type: object brokerConfigGroup: type: string - brokerRoles: - description: 'brokerRoles defines the role(s) for this particular - broker node: regular broker, controller, or both. This must - be set in KRaft mode.' - items: - type: string - maxItems: 2 - type: array id: description: id maps to "node.id" configuration in KRaft mode, and it maps to "broker.id" configuration in ZooKeeper mode. diff --git a/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml b/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml index 5dbfe2653..9fe66759d 100644 --- a/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml +++ b/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml @@ -12675,14 +12675,6 @@ spec: type: object brokerConfigGroup: type: string - brokerRoles: - description: 'brokerRoles defines the role(s) for this particular - broker node: regular broker, controller, or both. This must - be set in KRaft mode.' - items: - type: string - maxItems: 2 - type: array id: description: id maps to "node.id" configuration in KRaft mode, and it maps to "broker.id" configuration in ZooKeeper mode. From aca2396ccba240d22703b25feb19f3f4320242ce Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Fri, 21 Jul 2023 15:11:47 -0400 Subject: [PATCH 07/22] Update pod start-up process so pods can be restarted during rolling upgrade triggered by controller addition/removal --- pkg/resources/kafka/pod.go | 22 ++++++++++++---- pkg/resources/kafka/util_test.go | 1 - pkg/resources/kafka/wait-for-envoy-sidecar.sh | 25 ++++++++++++++++--- 3 files changed, 39 insertions(+), 9 deletions(-) diff --git a/pkg/resources/kafka/pod.go b/pkg/resources/kafka/pod.go index 0e56a4022..274190556 100644 --- a/pkg/resources/kafka/pod.go +++ b/pkg/resources/kafka/pod.go @@ -168,14 +168,26 @@ fi`}, pod.Spec.Subdomain = fmt.Sprintf(kafkautils.HeadlessServiceTemplate, r.KafkaCluster.Name) } - // in KRaft mode, all broker nodes within the same Kafka cluster need to use the same cluster UUID to format the storage if r.KafkaCluster.Spec.KraftMode() { for i, container := range pod.Spec.Containers { if container.Name == kafkaContainerName { - pod.Spec.Containers[i].Env = append(pod.Spec.Containers[i].Env, corev1.EnvVar{ - Name: "CLUSTER_ID", - Value: r.KafkaCluster.Status.ClusterID, - }) + // in KRaft mode, all broker nodes within the same Kafka cluster need to use the same cluster ID to format the storage + pod.Spec.Containers[i].Env = append(pod.Spec.Containers[i].Env, + corev1.EnvVar{ + Name: "CLUSTER_ID", + Value: r.KafkaCluster.Status.ClusterID, + }, + ) + // see how this env var is used in wait-for-envoy-sidecars.sh + storageMountPaths := brokerConfig.GetStorageMountPaths() + if storageMountPaths != "" { + pod.Spec.Containers[i].Env = append(pod.Spec.Containers[i].Env, + corev1.EnvVar{ + Name: "LOG_DIRS", + Value: brokerConfig.GetStorageMountPaths(), + }, + ) + } break } } diff --git a/pkg/resources/kafka/util_test.go b/pkg/resources/kafka/util_test.go index 064ec95ce..b380eac3c 100644 --- a/pkg/resources/kafka/util_test.go +++ b/pkg/resources/kafka/util_test.go @@ -338,5 +338,4 @@ func TestGenerateQuorumVoters(t *testing.T) { } }) } - } diff --git a/pkg/resources/kafka/wait-for-envoy-sidecar.sh b/pkg/resources/kafka/wait-for-envoy-sidecar.sh index 1c5e8b61c..5a8832f20 100644 --- a/pkg/resources/kafka/wait-for-envoy-sidecar.sh +++ b/pkg/resources/kafka/wait-for-envoy-sidecar.sh @@ -29,9 +29,28 @@ if [[ -n "$ENVOY_SIDECAR_STATUS" ]]; then fi touch /var/run/wait/do-not-exit-yet -if [[ -n "$CLUSTER_ID" ]]; then -echo "Formatting storage directories in KRaft mode with cluster ID $CLUSTER_ID" -/opt/kafka/bin/kafka-storage.sh format --cluster-id "$CLUSTER_ID" -c /config/broker-config +# A few necessary steps if we are in KRaft mode +if [[ -n "${CLUSTER_ID}" ]]; then + # If the storage is already formatted (e.g. broker restarts), the kafka-storage.sh will skip formatting for that storage + # thus we can safely run the storage format command regardless if the storage has been formatted or not + echo "Formatting KRaft storage with cluster ID ${CLUSTER_ID}" + /opt/kafka/bin/kafka-storage.sh format --cluster-id "${CLUSTER_ID}" -c /config/broker-config + + # Adding or removing controller nodes to the Kafka cluster would trigger cluster rolling upgrade so all the nodes in the cluster are aware of the newly added/removed controllers. + # When this happens, Kafka's local quorum state file would be outdated since it is static and the Kafka server can't be started with conflicting controllers info (compared to info stored in ConfigMap), + # so we need to wipe out the local state files before starting the server so the information about the controller nodes is up-to-date with what is stored in ConfigMap + # (Note: although we don't know if the server start-up is due to scaling up/down of the controller nodes, it is not harmful to remove the quorum state file before the server start-up process + # because the server will re-create the quorum state file after it starts up successfully) + if [[ -n "${LOG_DIRS}" ]]; then + IFS=',' read -ra LOGS <<< "${LOG_DIRS}" + for LOG in "${LOGS[@]}"; do + QUORUM_STATE_FILE="${LOG}/kafka/__cluster_metadata-0/quorum-state" + if [ -f "${QUORUM_STATE_FILE}" ]; then + echo "Removing quorum-state file from \"${QUORUM_STATE_FILE}\"" + rm -f "${QUORUM_STATE_FILE}" + fi + done + fi fi /opt/kafka/bin/kafka-server-start.sh /config/broker-config From c5a481964fd7830e915310323d9bda8503b1678d Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Sun, 23 Jul 2023 16:34:02 -0400 Subject: [PATCH 08/22] Rebase from origin/kraft-api --- api/v1beta1/kafkacluster_types.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/api/v1beta1/kafkacluster_types.go b/api/v1beta1/kafkacluster_types.go index 7e5ff92ce..7fd381d1e 100644 --- a/api/v1beta1/kafkacluster_types.go +++ b/api/v1beta1/kafkacluster_types.go @@ -74,11 +74,7 @@ type KafkaClusterSpec struct { // This is default to be true; if set to false, the Kafka cluster is in ZooKeeper mode. // +kubebuilder:default=true // +optional -<<<<<<< HEAD KRaftMode bool `json:"kRaft"` -======= - KRaftMode bool `json:"kRaft,omitempty"` ->>>>>>> 199eb1e6 (Modify KafkaCluster API to support running in KRaft) HeadlessServiceEnabled bool `json:"headlessServiceEnabled"` ListenersConfig ListenersConfig `json:"listenersConfig"` // Custom ports to expose in the container. Example use case: a custom kafka distribution, that includes an integrated metrics api endpoint From 5d1b77741ff357a50a7a2ebf717a33bd6162cfe5 Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Sun, 23 Jul 2023 16:36:16 -0400 Subject: [PATCH 09/22] Update exsiting integration tests and func signatures --- controllers/tests/clusterregistry/common_test.go | 1 + controllers/tests/common_test.go | 1 + pkg/resources/kafka/configmap.go | 10 +++------- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/controllers/tests/clusterregistry/common_test.go b/controllers/tests/clusterregistry/common_test.go index 2487c6f22..2505964a7 100644 --- a/controllers/tests/clusterregistry/common_test.go +++ b/controllers/tests/clusterregistry/common_test.go @@ -36,6 +36,7 @@ func createMinimalKafkaClusterCR(name, namespace string) *v1beta1.KafkaCluster { Annotations: map[string]string{}, }, Spec: v1beta1.KafkaClusterSpec{ + KRaftMode: false, ListenersConfig: v1beta1.ListenersConfig{ ExternalListeners: []v1beta1.ExternalListenerConfig{ { diff --git a/controllers/tests/common_test.go b/controllers/tests/common_test.go index c164a2498..aa5a3fbec 100644 --- a/controllers/tests/common_test.go +++ b/controllers/tests/common_test.go @@ -40,6 +40,7 @@ func createMinimalKafkaClusterCR(name, namespace string) *v1beta1.KafkaCluster { Annotations: map[string]string{}, }, Spec: v1beta1.KafkaClusterSpec{ + KRaftMode: false, ListenersConfig: v1beta1.ListenersConfig{ ExternalListeners: []v1beta1.ExternalListenerConfig{ { diff --git a/pkg/resources/kafka/configmap.go b/pkg/resources/kafka/configmap.go index 3f1a6e2c6..4710d29ba 100644 --- a/pkg/resources/kafka/configmap.go +++ b/pkg/resources/kafka/configmap.go @@ -53,11 +53,9 @@ func (r *Reconciler) getConfigProperties(bConfig *v1beta1.BrokerConfig, broker v // Kafka Broker configurations if r.KafkaCluster.Spec.KraftMode() { - configureBrokerKRaftMode(broker, r.KafkaCluster, config, bootstrapServers, serverPasses, extListenerStatuses, - intListenerStatuses, controllerIntListenerStatuses, log) + configureBrokerKRaftMode(broker, r.KafkaCluster, config, serverPasses, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses, log) } else { - configureBrokerZKMode(broker.Id, r.KafkaCluster, config, serverPasses, extListenerStatuses, - intListenerStatuses, controllerIntListenerStatuses, log) + configureBrokerZKMode(broker.Id, r.KafkaCluster, config, serverPasses, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses, log) } // This logic prevents the removal of the mountPath from the broker configmap @@ -135,9 +133,7 @@ func configCCMetricsReporter(kafkaCluster *v1beta1.KafkaCluster, config *propert } } -func configureBrokerKRaftMode(broker v1beta1.Broker, kafkaCluster *v1beta1.KafkaCluster, config *properties.Properties, - bootstrapServers string, serverPasses map[string]string, extListenerStatuses, intListenerStatuses, - controllerIntListenerStatuses map[string]v1beta1.ListenerStatusList, log logr.Logger) { +func configureBrokerKRaftMode(broker v1beta1.Broker, kafkaCluster *v1beta1.KafkaCluster, config *properties.Properties, serverPasses map[string]string, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses map[string]v1beta1.ListenerStatusList, log logr.Logger) { if err := config.Set(kafkautils.KafkaConfigNodeID, broker.Id); err != nil { log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.KafkaConfigNodeID)) } From aa4f5f5f868cc4e948d59ad17f9f0c2c363e28ce Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Mon, 24 Jul 2023 09:56:02 -0400 Subject: [PATCH 10/22] Fix broker configurations; add unit tests for broker configurations under KRaft mode --- pkg/resources/kafka/configmap.go | 16 +- pkg/resources/kafka/configmap_test.go | 325 +++++++++++++++++++++++++- 2 files changed, 326 insertions(+), 15 deletions(-) diff --git a/pkg/resources/kafka/configmap.go b/pkg/resources/kafka/configmap.go index 4710d29ba..da2c45980 100644 --- a/pkg/resources/kafka/configmap.go +++ b/pkg/resources/kafka/configmap.go @@ -158,10 +158,8 @@ func configureBrokerKRaftMode(broker v1beta1.Broker, kafkaCluster *v1beta1.Kafka config.Merge(listenerConf) var advertisedListenerConf []string - - // expose controller listener in the listeners configuration only if this broker is a controller - // also expose only controller listener as the advertised.listeners configuration when this broker is a controller - if !isControllerNode(broker.Roles) { + // only expose "advertised.listeners" when the node serves as a regular broker or a combined node + if isBrokerNode(broker.Roles) { advertisedListenerConf = generateAdvertisedListenerConfig(broker.Id, kafkaCluster.Spec.ListenersConfig, extListenerStatuses, intListenerStatuses, nil) if err := config.Set(kafkautils.KafkaConfigAdvertisedListeners, advertisedListenerConf); err != nil { @@ -169,8 +167,8 @@ func configureBrokerKRaftMode(broker v1beta1.Broker, kafkaCluster *v1beta1.Kafka } } - // "listeners" configuration can only contain controller configuration when the node is a controller-only node if isControllerNodeOnly(broker.Roles) { + // "listeners" configuration can only contain controller configuration when the node is a controller-only node for _, listener := range listenerConfig { if listener[:len(controllerListenerName)] == strings.ToUpper(controllerListenerName) { if err := config.Set(kafkautils.KafkaConfigListeners, listener); err != nil { @@ -179,11 +177,9 @@ func configureBrokerKRaftMode(broker v1beta1.Broker, kafkaCluster *v1beta1.Kafka break } } - } - - // "listeners" configuration cannot contain controller configuration when the node is a broker-only node - var nonControllerListener []string - if isBrokerNodeOnly(broker.Roles) { + } else if isBrokerNodeOnly(broker.Roles) { + // "listeners" configuration cannot contain controller configuration when the node is a broker-only node + var nonControllerListener []string for _, listener := range listenerConfig { if listener[:len(controllerListenerName)] != strings.ToUpper(controllerListenerName) { nonControllerListener = append(nonControllerListener, listener) diff --git a/pkg/resources/kafka/configmap_test.go b/pkg/resources/kafka/configmap_test.go index 67975db32..325b21427 100644 --- a/pkg/resources/kafka/configmap_test.go +++ b/pkg/resources/kafka/configmap_test.go @@ -21,6 +21,7 @@ import ( "github.com/go-logr/logr" "github.com/golang/mock/gomock" + "github.com/stretchr/testify/require" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -120,9 +121,7 @@ func TestMergeMountPaths(t *testing.T) { if !reflect.DeepEqual(mergedMountPaths, test.expectedMergedMountPath) { t.Errorf("testName: %s, expected: %s, got: %s", test.testName, test.expectedMergedMountPath, mergedMountPaths) } - if isRemoved != test.expectedRemoved { - t.Errorf("testName: %s, expectedRemoved: %v, got: %v", test.testName, test.expectedRemoved, isRemoved) - } + require.Equal(t, test.expectedRemoved, isRemoved) } } @@ -696,8 +695,324 @@ zookeeper.connect=example.zk:2181/`, t.Fatalf("failed parsing expected configuration as Properties: %s", expected) } - if !expected.Equal(generated) { - t.Errorf("the expected config is:\n%s\nreceived:\n%s\n", test.expectedConfig, generatedConfig) + require.Equal(t, expected, generated) + }) + } +} + +// TestGenerateBrokerConfigKRaftMode serves as an aggregated test on top of TestGenerateBrokerConfig to verify basic broker configurations under KRaft mode +// Note: most of the test cases under TestGenerateBrokerConfig are not replicated here since running KRaft mode doesn't affect things like SSL and storage configurations +func TestGenerateBrokerConfigKRaftMode(t *testing.T) { + testCases := []struct { + testName string + brokers []v1beta1.Broker + listenersConfig v1beta1.ListenersConfig + internalListenerStatuses map[string]v1beta1.ListenerStatusList + controllerListenerStatus map[string]v1beta1.ListenerStatusList + expectedBrokerConfigs []string + }{ + { + testName: "a Kafka cluster with a mix of broker-only and controller-only nodes; broker-only nodes with multiple mount paths", + brokers: []v1beta1.Broker{ + { + Id: 0, + Roles: []string{"broker"}, + BrokerConfig: &v1beta1.BrokerConfig{ + StorageConfigs: []v1beta1.StorageConfig{ + { + MountPath: "/test-kafka-logs", + }, + { + MountPath: "/test-kafka-logs-0", + }, + }, + }, + }, + { + Id: 500, + Roles: []string{"controller"}, + BrokerConfig: &v1beta1.BrokerConfig{ + StorageConfigs: []v1beta1.StorageConfig{ + { + MountPath: "/test-kafka-logs", + }, + }, + }, + }, + { + Id: 200, + Roles: []string{"broker"}, + BrokerConfig: &v1beta1.BrokerConfig{ + StorageConfigs: []v1beta1.StorageConfig{ + { + MountPath: "/test-kafka-logs", + }, + }, + }, + }, + }, + listenersConfig: v1beta1.ListenersConfig{ + InternalListeners: []v1beta1.InternalListenerConfig{ + { + CommonListenerSpec: v1beta1.CommonListenerSpec{ + Type: v1beta1.SecurityProtocol("PLAINTEXT"), + Name: "internal", + ContainerPort: 9092, + }, + UsedForInnerBrokerCommunication: true, + }, + { + CommonListenerSpec: v1beta1.CommonListenerSpec{ + Type: v1beta1.SecurityProtocol("PLAINTEXT"), + Name: "controller", + ContainerPort: 9093, + }, + UsedForControllerCommunication: true, + }, + }, + }, + internalListenerStatuses: map[string]v1beta1.ListenerStatusList{ + "internal": { + { + Name: "broker-0", + Address: "kafka-0.kafka.svc.cluster.local:9092", + }, + { + Name: "broker-500", + Address: "kafka-500.kafka.svc.cluster.local:9092", + }, + { + Name: "broker-200", + Address: "kafka-200.kafka.svc.cluster.local:9092", + }, + }, + }, + controllerListenerStatus: map[string]v1beta1.ListenerStatusList{ + "controller": { + { + Name: "broker-0", + Address: "kafka-0.kafka.svc.cluster.local:9093", + }, + { + Name: "broker-500", + Address: "kafka-500.kafka.svc.cluster.local:9093", + }, + { + Name: "broker-200", + Address: "kafka-200.kafka.svc.cluster.local:9093", + }, + }, + }, + expectedBrokerConfigs: []string{ + `advertised.listeners=INTERNAL://kafka-0.kafka.svc.cluster.local:9092 +controller.listener.names=CONTROLLER +controller.quorum.voters=500@kafka-500.kafka.svc.cluster.local:9093 +cruise.control.metrics.reporter.bootstrap.servers=kafka-all-broker.kafka.svc.cluster.local:9092 +cruise.control.metrics.reporter.kubernetes.mode=true +inter.broker.listener.name=INTERNAL +listener.security.protocol.map=INTERNAL:PLAINTEXT,CONTROLLER:PLAINTEXT +listeners=INTERNAL://:9092 +log.dirs=/test-kafka-logs/kafka,/test-kafka-logs-0/kafka +metric.reporters=com.linkedin.kafka.cruisecontrol.metricsreporter.CruiseControlMetricsReporter +node.id=0 +process.roles=broker +`, + `controller.listener.names=CONTROLLER +controller.quorum.voters=500@kafka-500.kafka.svc.cluster.local:9093 +cruise.control.metrics.reporter.bootstrap.servers=kafka-all-broker.kafka.svc.cluster.local:9092 +cruise.control.metrics.reporter.kubernetes.mode=true +inter.broker.listener.name=INTERNAL +listener.security.protocol.map=INTERNAL:PLAINTEXT,CONTROLLER:PLAINTEXT +listeners=CONTROLLER://:9093 +log.dirs=/test-kafka-logs/kafka +metric.reporters=com.linkedin.kafka.cruisecontrol.metricsreporter.CruiseControlMetricsReporter +node.id=500 +process.roles=controller +`, + `advertised.listeners=INTERNAL://kafka-200.kafka.svc.cluster.local:9092 +controller.listener.names=CONTROLLER +controller.quorum.voters=500@kafka-500.kafka.svc.cluster.local:9093 +cruise.control.metrics.reporter.bootstrap.servers=kafka-all-broker.kafka.svc.cluster.local:9092 +cruise.control.metrics.reporter.kubernetes.mode=true +inter.broker.listener.name=INTERNAL +listener.security.protocol.map=INTERNAL:PLAINTEXT,CONTROLLER:PLAINTEXT +listeners=INTERNAL://:9092 +log.dirs=/test-kafka-logs/kafka +metric.reporters=com.linkedin.kafka.cruisecontrol.metricsreporter.CruiseControlMetricsReporter +node.id=200 +process.roles=broker +`}, + }, + { + testName: "a Kafka cluster with a mix of broker-only, controller-only, and combined roles; controller nodes with multiple mount paths", + brokers: []v1beta1.Broker{ + { + Id: 0, + Roles: []string{"broker"}, + BrokerConfig: &v1beta1.BrokerConfig{ + StorageConfigs: []v1beta1.StorageConfig{ + { + MountPath: "/test-kafka-logs", + }, + }, + }, + }, + { + Id: 50, + Roles: []string{"controller"}, + BrokerConfig: &v1beta1.BrokerConfig{ + StorageConfigs: []v1beta1.StorageConfig{ + { + MountPath: "/test-kafka-logs", + }, + { + MountPath: "/test-kafka-logs-50", + }, + }, + }, + }, + { + Id: 100, + Roles: []string{"broker", "controller"}, + BrokerConfig: &v1beta1.BrokerConfig{ + StorageConfigs: []v1beta1.StorageConfig{ + { + MountPath: "/test-kafka-logs", + }, + { + MountPath: "/test-kafka-logs-50", + }, + { + MountPath: "/test-kafka-logs-100", + }, + }, + }, + }, + }, + listenersConfig: v1beta1.ListenersConfig{ + InternalListeners: []v1beta1.InternalListenerConfig{ + { + CommonListenerSpec: v1beta1.CommonListenerSpec{ + Type: v1beta1.SecurityProtocol("PLAINTEXT"), + Name: "internal", + ContainerPort: 9092, + }, + UsedForInnerBrokerCommunication: true, + }, + { + CommonListenerSpec: v1beta1.CommonListenerSpec{ + Type: v1beta1.SecurityProtocol("PLAINTEXT"), + Name: "controller", + ContainerPort: 9093, + }, + UsedForControllerCommunication: true, + }, + }, + }, + internalListenerStatuses: map[string]v1beta1.ListenerStatusList{ + "internal": { + { + Name: "broker-0", + Address: "kafka-0.kafka.svc.cluster.local:9092", + }, + { + Name: "broker-50", + Address: "kafka-50.kafka.svc.cluster.local:9092", + }, + { + Name: "broker-100", + Address: "kafka-100.kafka.svc.cluster.local:9092", + }, + }, + }, + controllerListenerStatus: map[string]v1beta1.ListenerStatusList{ + "controller": { + { + Name: "broker-0", + Address: "kafka-0.kafka.svc.cluster.local:9093", + }, + { + Name: "broker-50", + Address: "kafka-50.kafka.svc.cluster.local:9093", + }, + { + Name: "broker-100", + Address: "kafka-100.kafka.svc.cluster.local:9093", + }, + }, + }, + expectedBrokerConfigs: []string{ + `advertised.listeners=INTERNAL://kafka-0.kafka.svc.cluster.local:9092 +controller.listener.names=CONTROLLER +controller.quorum.voters=50@kafka-50.kafka.svc.cluster.local:9093,100@kafka-100.kafka.svc.cluster.local:9093 +cruise.control.metrics.reporter.bootstrap.servers=kafka-all-broker.kafka.svc.cluster.local:9092 +cruise.control.metrics.reporter.kubernetes.mode=true +inter.broker.listener.name=INTERNAL +listener.security.protocol.map=INTERNAL:PLAINTEXT,CONTROLLER:PLAINTEXT +listeners=INTERNAL://:9092 +log.dirs=/test-kafka-logs/kafka +metric.reporters=com.linkedin.kafka.cruisecontrol.metricsreporter.CruiseControlMetricsReporter +node.id=0 +process.roles=broker +`, + `controller.listener.names=CONTROLLER +controller.quorum.voters=50@kafka-50.kafka.svc.cluster.local:9093,100@kafka-100.kafka.svc.cluster.local:9093 +cruise.control.metrics.reporter.bootstrap.servers=kafka-all-broker.kafka.svc.cluster.local:9092 +cruise.control.metrics.reporter.kubernetes.mode=true +inter.broker.listener.name=INTERNAL +listener.security.protocol.map=INTERNAL:PLAINTEXT,CONTROLLER:PLAINTEXT +listeners=CONTROLLER://:9093 +log.dirs=/test-kafka-logs/kafka,/test-kafka-logs-50/kafka +metric.reporters=com.linkedin.kafka.cruisecontrol.metricsreporter.CruiseControlMetricsReporter +node.id=50 +process.roles=controller +`, + `advertised.listeners=INTERNAL://kafka-100.kafka.svc.cluster.local:9092 +controller.listener.names=CONTROLLER +controller.quorum.voters=50@kafka-50.kafka.svc.cluster.local:9093,100@kafka-100.kafka.svc.cluster.local:9093 +cruise.control.metrics.reporter.bootstrap.servers=kafka-all-broker.kafka.svc.cluster.local:9092 +cruise.control.metrics.reporter.kubernetes.mode=true +inter.broker.listener.name=INTERNAL +listener.security.protocol.map=INTERNAL:PLAINTEXT,CONTROLLER:PLAINTEXT +listeners=INTERNAL://:9092,CONTROLLER://:9093 +log.dirs=/test-kafka-logs/kafka,/test-kafka-logs-50/kafka,/test-kafka-logs-100/kafka +metric.reporters=com.linkedin.kafka.cruisecontrol.metricsreporter.CruiseControlMetricsReporter +node.id=100 +process.roles=broker,controller +`}, + }, + } + + t.Parallel() + mockClient := mocks.NewMockClient(gomock.NewController(t)) + mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() + + for _, test := range testCases { + test := test + + t.Run(test.testName, func(t *testing.T) { + r := Reconciler{ + Reconciler: resources.Reconciler{ + Client: mockClient, + KafkaCluster: &v1beta1.KafkaCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kafka", + Namespace: "kafka", + }, + Spec: v1beta1.KafkaClusterSpec{ + KRaftMode: true, + ListenersConfig: test.listenersConfig, + Brokers: test.brokers, + }, + }, + }, + } + + for i, b := range test.brokers { + generatedConfig := r.generateBrokerConfig(b, b.BrokerConfig, map[string]v1beta1.ListenerStatusList{}, + test.internalListenerStatuses, test.controllerListenerStatus, nil, "", nil, logr.Discard()) + + require.Equal(t, test.expectedBrokerConfigs[i], generatedConfig) } }) } From 7431c9370c4709876a74379c1b7cd7ccf8794be6 Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Mon, 24 Jul 2023 21:20:02 -0400 Subject: [PATCH 11/22] Remove unnecessary method from koperator/api --- api/v1beta1/kafkacluster_types.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/api/v1beta1/kafkacluster_types.go b/api/v1beta1/kafkacluster_types.go index 7fd381d1e..a2054072d 100644 --- a/api/v1beta1/kafkacluster_types.go +++ b/api/v1beta1/kafkacluster_types.go @@ -801,10 +801,6 @@ func (kSpec *KafkaClusterSpec) GetClusterMetricsReporterImage() string { return kSpec.CruiseControlConfig.GetCCImage() } -func (kSpec *KafkaClusterSpec) KraftMode() bool { - return kSpec.KRaftMode -} - func (cTaskSpec *CruiseControlTaskSpec) GetDurationMinutes() float64 { if cTaskSpec.RetryDurationMinutes == 0 { return 5 From 2e3be0682b4a671c2a70d9382e2b976d655689ec Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Mon, 24 Jul 2023 21:24:11 -0400 Subject: [PATCH 12/22] Extend integration tests to cover KRaft mode --- ...kacluster_controller_cruisecontrol_test.go | 16 +- ...ontroller_externallistenerbindings_test.go | 82 +++++++- .../kafkacluster_controller_kafka_test.go | 175 ++++++++++++---- .../tests/kafkacluster_controller_test.go | 189 +++++++++++++++++- pkg/jmxextractor/mock_extractor.go | 2 +- pkg/resources/cruisecontrol/configmap.go | 15 +- pkg/resources/kafka/configmap.go | 2 +- pkg/resources/kafka/pod.go | 2 +- 8 files changed, 416 insertions(+), 67 deletions(-) diff --git a/controllers/tests/kafkacluster_controller_cruisecontrol_test.go b/controllers/tests/kafkacluster_controller_cruisecontrol_test.go index 325bce427..e7be74eb2 100644 --- a/controllers/tests/kafkacluster_controller_cruisecontrol_test.go +++ b/controllers/tests/kafkacluster_controller_cruisecontrol_test.go @@ -97,7 +97,7 @@ func expectCruiseControlService(ctx context.Context, kafkaCluster *v1beta1.Kafka TargetPort: intstr.FromInt(9020), }, )) - Expect(service.Spec.Selector).To(HaveKeyWithValue(v1beta1.KafkaCRLabelKey, "kafkacluster-1")) + Expect(service.Spec.Selector).To(HaveKeyWithValue(v1beta1.KafkaCRLabelKey, kafkaCluster.Name)) Expect(service.Spec.Selector).To(HaveKeyWithValue(v1beta1.AppLabelKey, "cruisecontrol")) } @@ -113,10 +113,20 @@ func expectCruiseControlConfigMap(ctx context.Context, kafkaCluster *v1beta1.Kaf Expect(configMap.Labels).To(HaveKeyWithValue(v1beta1.AppLabelKey, "cruisecontrol")) Expect(configMap.Labels).To(HaveKeyWithValue(v1beta1.KafkaCRLabelKey, kafkaCluster.Name)) - Expect(configMap.Data).To(HaveKeyWithValue("cruisecontrol.properties", fmt.Sprintf(`bootstrap.servers=%s-all-broker.%s.%s:29092 + var expectedCCProperties string + if kafkaCluster.Spec.KRaftMode { + expectedCCProperties = fmt.Sprintf(`bootstrap.servers=%s-all-broker.%s.%s:29092 +kafka.broker.failure.detection.enable=true +some.config=value +topic.config.provider.class=com.linkedin.kafka.cruisecontrol.config.KafkaAdminTopicConfigProvider +`, kafkaCluster.Name, kafkaCluster.Namespace, "svc.cluster.local") + } else { + expectedCCProperties = fmt.Sprintf(`bootstrap.servers=%s-all-broker.%s.%s:29092 some.config=value zookeeper.connect=/ -`, kafkaCluster.Name, kafkaCluster.Namespace, "svc.cluster.local"))) +`, kafkaCluster.Name, kafkaCluster.Namespace, "svc.cluster.local") + } + Expect(configMap.Data).To(HaveKeyWithValue("cruisecontrol.properties", expectedCCProperties)) Expect(configMap.Data).To(HaveKeyWithValue("capacity.json", `{ "brokerCapacities": [ { diff --git a/controllers/tests/kafkacluster_controller_externallistenerbindings_test.go b/controllers/tests/kafkacluster_controller_externallistenerbindings_test.go index 8778db1b0..d6647746e 100644 --- a/controllers/tests/kafkacluster_controller_externallistenerbindings_test.go +++ b/controllers/tests/kafkacluster_controller_externallistenerbindings_test.go @@ -49,12 +49,49 @@ func expectDefaultBrokerSettingsForExternalListenerBinding(ctx context.Context, brokerConfig, err := properties.NewFromString(configMap.Data["broker-config"]) Expect(err).NotTo(HaveOccurred()) advertisedListener, found := brokerConfig.Get("advertised.listeners") - Expect(found).To(BeTrue()) - Expect(advertisedListener.Value()).To(Equal(fmt.Sprintf("CONTROLLER://kafkacluster-%d-%d.kafka-%d.svc.cluster.local:29093,INTERNAL://kafkacluster-%d-%d.kafka-%d.svc.cluster.local:29092,TEST://external.az1.host.com:%d", - randomGenTestNumber, broker.Id, randomGenTestNumber, randomGenTestNumber, broker.Id, randomGenTestNumber, 19090+broker.Id))) + expectedAdvertisedListenersFound := true + var expectedAdvertisedListener string + if kafkaCluster.Spec.KRaftMode { + switch broker.Id { + case 0: + // broker-only node + expectedAdvertisedListener = fmt.Sprintf("INTERNAL://kafkacluster-kraft-%d-%d.kafka-%d.svc.cluster.local:29092,TEST://external.az1.host.com:%d", + randomGenTestNumber, broker.Id, randomGenTestNumber, 19090+broker.Id) + case 1: + // controller-only node + expectedAdvertisedListenersFound = false + expectedAdvertisedListener = "" + case 2: + // combined node + expectedAdvertisedListener = fmt.Sprintf("INTERNAL://kafkacluster-kraft-%d-%d.kafka-%d.svc.cluster.local:29092,TEST://external.az1.host.com:%d", + randomGenTestNumber, broker.Id, randomGenTestNumber, 19090+broker.Id) + } + } else { + expectedAdvertisedListener = fmt.Sprintf("CONTROLLER://kafkacluster-%d-%d.kafka-%d.svc.cluster.local:29093,"+ + "INTERNAL://kafkacluster-%d-%d.kafka-%d.svc.cluster.local:29092,TEST://external.az1.host.com:%d", + randomGenTestNumber, broker.Id, randomGenTestNumber, randomGenTestNumber, broker.Id, randomGenTestNumber, 19090+broker.Id) + } + Expect(found).To(Equal(expectedAdvertisedListenersFound)) + Expect(advertisedListener.Value()).To(Equal(expectedAdvertisedListener)) + listeners, found := brokerConfig.Get("listeners") Expect(found).To(BeTrue()) - Expect(listeners.Value()).To(Equal("INTERNAL://:29092,CONTROLLER://:29093,TEST://:9094")) + + var expectedListeners string + if kafkaCluster.Spec.KRaftMode { + switch broker.Id { + case 0: + expectedListeners = "INTERNAL://:29092,TEST://:9094" + case 1: + expectedListeners = "CONTROLLER://:29093" + case 2: + expectedListeners = "INTERNAL://:29092,CONTROLLER://:29093,TEST://:9094" + } + } else { + expectedListeners = "INTERNAL://:29092,CONTROLLER://:29093,TEST://:9094" + } + Expect(listeners.Value()).To(Equal(expectedListeners)) + listenerSecMap, found := brokerConfig.Get(kafkautils.KafkaConfigListenerSecurityProtocolMap) Expect(found).To(BeTrue()) Expect(listenerSecMap.Value()).To(Equal("INTERNAL:PLAINTEXT,CONTROLLER:PLAINTEXT,TEST:PLAINTEXT")) @@ -112,8 +149,16 @@ func expectBrokerConfigmapForAz1ExternalListener(ctx context.Context, kafkaClust Expect(err).NotTo(HaveOccurred()) advertisedListener, found := brokerConfig.Get("advertised.listeners") Expect(found).To(BeTrue()) - Expect(advertisedListener.Value()).To(Equal(fmt.Sprintf("CONTROLLER://kafkacluster-%d-%d.kafkaconfigtest-%d.svc.cluster.local:29093,INTERNAL://kafkacluster-%d-%d.kafkaconfigtest-%d.svc.cluster.local:29092,TEST://external.az1.host.com:%d", - randomGenTestNumber, 0, randomGenTestNumber, randomGenTestNumber, 0, randomGenTestNumber, 19090))) + var expectedAdvertisedListener string + if kafkaCluster.Spec.KRaftMode { + // broker-0 is a broker-only node + expectedAdvertisedListener = fmt.Sprintf("INTERNAL://kafkacluster-kraft-%d-%d.kafkaconfigtest-%d.svc.cluster.local:29092,TEST://external.az1.host.com:%d", + randomGenTestNumber, 0, randomGenTestNumber, 19090) + } else { + expectedAdvertisedListener = fmt.Sprintf("CONTROLLER://kafkacluster-%d-%d.kafkaconfigtest-%d.svc.cluster.local:29093,INTERNAL://kafkacluster-%d-%d.kafkaconfigtest-%d.svc.cluster.local:29092,TEST://external.az1.host.com:%d", + randomGenTestNumber, 0, randomGenTestNumber, randomGenTestNumber, 0, randomGenTestNumber, 19090) + } + Expect(advertisedListener.Value()).To(Equal(expectedAdvertisedListener)) } func expectBrokerConfigmapForAz2ExternalListener(ctx context.Context, kafkaCluster *v1beta1.KafkaCluster, randomGenTestNumber uint64) { @@ -128,9 +173,18 @@ func expectBrokerConfigmapForAz2ExternalListener(ctx context.Context, kafkaClust brokerConfig, err := properties.NewFromString(configMap.Data["broker-config"]) Expect(err).NotTo(HaveOccurred()) advertisedListener, found := brokerConfig.Get("advertised.listeners") - Expect(found).To(BeTrue()) - Expect(advertisedListener.Value()).To(Equal(fmt.Sprintf("CONTROLLER://kafkacluster-%d-%d.kafkaconfigtest-%d.svc.cluster.local:29093,INTERNAL://kafkacluster-%d-%d.kafkaconfigtest-%d.svc.cluster.local:29092,TEST://external.az2.host.com:%d", - randomGenTestNumber, 1, randomGenTestNumber, randomGenTestNumber, 1, randomGenTestNumber, 19091))) + var ( + expectedAdvertisedListener string + expectedFound bool + ) + // for the Kafka cluster under KRaft mode, broker-1 is a controller-only node and therefore "advertised.listeners" is not available + if !kafkaCluster.Spec.KRaftMode { + expectedAdvertisedListener = fmt.Sprintf("CONTROLLER://kafkacluster-%d-%d.kafkaconfigtest-%d.svc.cluster.local:29093,INTERNAL://kafkacluster-%d-%d.kafkaconfigtest-%d.svc.cluster.local:29092,TEST://external.az2.host.com:%d", + randomGenTestNumber, 1, randomGenTestNumber, randomGenTestNumber, 1, randomGenTestNumber, 19091) + expectedFound = true + } + Expect(found).To(Equal(expectedFound)) + Expect(advertisedListener.Value()).To(Equal(expectedAdvertisedListener)) configMap = corev1.ConfigMap{} Eventually(ctx, func() error { @@ -144,6 +198,12 @@ func expectBrokerConfigmapForAz2ExternalListener(ctx context.Context, kafkaClust Expect(err).NotTo(HaveOccurred()) advertisedListener, found = brokerConfig.Get("advertised.listeners") Expect(found).To(BeTrue()) - Expect(advertisedListener.Value()).To(Equal(fmt.Sprintf("CONTROLLER://kafkacluster-%d-%d.kafkaconfigtest-%d.svc.cluster.local:29093,INTERNAL://kafkacluster-%d-%d.kafkaconfigtest-%d.svc.cluster.local:29092,TEST://external.az2.host.com:%d", - randomGenTestNumber, 2, randomGenTestNumber, randomGenTestNumber, 2, randomGenTestNumber, 19092))) + if kafkaCluster.Spec.KRaftMode { + expectedAdvertisedListener = fmt.Sprintf("INTERNAL://kafkacluster-kraft-%d-%d.kafkaconfigtest-%d.svc.cluster.local:29092,TEST://external.az2.host.com:%d", + randomGenTestNumber, 2, randomGenTestNumber, 19092) + } else { + expectedAdvertisedListener = fmt.Sprintf("CONTROLLER://kafkacluster-%d-%d.kafkaconfigtest-%d.svc.cluster.local:29093,INTERNAL://kafkacluster-%d-%d.kafkaconfigtest-%d.svc.cluster.local:29092,TEST://external.az2.host.com:%d", + randomGenTestNumber, 2, randomGenTestNumber, randomGenTestNumber, 2, randomGenTestNumber, 19092) + } + Expect(advertisedListener.Value()).To(Equal(expectedAdvertisedListener)) } diff --git a/controllers/tests/kafkacluster_controller_kafka_test.go b/controllers/tests/kafkacluster_controller_kafka_test.go index 5d74b2734..8ce75e97e 100644 --- a/controllers/tests/kafkacluster_controller_kafka_test.go +++ b/controllers/tests/kafkacluster_controller_kafka_test.go @@ -162,7 +162,59 @@ func expectKafkaBrokerConfigmap(ctx context.Context, kafkaCluster *v1beta1.Kafka Expect(configMap.Labels).To(HaveKeyWithValue(v1beta1.KafkaCRLabelKey, kafkaCluster.Name)) Expect(configMap.Labels).To(HaveKeyWithValue(v1beta1.BrokerIdLabelKey, strconv.Itoa(int(broker.Id)))) - Expect(configMap.Data).To(HaveKeyWithValue("broker-config", fmt.Sprintf(`advertised.listeners=CONTROLLER://kafkacluster-%d-%d.kafka-%d.svc.cluster.local:29093,INTERNAL://kafkacluster-%d-%d.kafka-%d.svc.cluster.local:29092,TEST://test.host.com:%d + var expectedBrokerConfig string + if kafkaCluster.Spec.KRaftMode { + switch broker.Id { + case 0: + // broker-only node + expectedBrokerConfig = fmt.Sprintf(`advertised.listeners=INTERNAL://%s-%d.%s.svc.cluster.local:29092,TEST://test.host.com:%d +controller.listener.names=CONTROLLER +controller.quorum.voters=1@%s-1.%s.svc.cluster.local:29093,2@%s-2.%s.svc.cluster.local:29093 +cruise.control.metrics.reporter.bootstrap.servers=%s-all-broker.%s.svc.cluster.local:29092 +cruise.control.metrics.reporter.kubernetes.mode=true +inter.broker.listener.name=INTERNAL +listener.security.protocol.map=INTERNAL:PLAINTEXT,CONTROLLER:PLAINTEXT,TEST:PLAINTEXT +listeners=INTERNAL://:29092,TEST://:9094 +log.dirs=/kafka-logs/kafka,/ephemeral-dir1/kafka +metric.reporters=com.linkedin.kafka.cruisecontrol.metricsreporter.CruiseControlMetricsReporter +node.id=%d +process.roles=broker +`, kafkaCluster.Name, broker.Id, kafkaCluster.Namespace, 19090+broker.Id, kafkaCluster.Name, kafkaCluster.Namespace, kafkaCluster.Name, kafkaCluster.Namespace, kafkaCluster.Name, + kafkaCluster.Namespace, broker.Id) + case 1: + // controller-only node + expectedBrokerConfig = fmt.Sprintf(`controller.listener.names=CONTROLLER +controller.quorum.voters=1@%s-1.%s.svc.cluster.local:29093,2@%s-2.%s.svc.cluster.local:29093 +cruise.control.metrics.reporter.bootstrap.servers=%s-all-broker.%s.svc.cluster.local:29092 +cruise.control.metrics.reporter.kubernetes.mode=true +inter.broker.listener.name=INTERNAL +listener.security.protocol.map=INTERNAL:PLAINTEXT,CONTROLLER:PLAINTEXT,TEST:PLAINTEXT +listeners=CONTROLLER://:29093 +log.dirs=/kafka-logs/kafka,/ephemeral-dir1/kafka +metric.reporters=com.linkedin.kafka.cruisecontrol.metricsreporter.CruiseControlMetricsReporter +node.id=%d +process.roles=controller +`, kafkaCluster.Name, kafkaCluster.Namespace, kafkaCluster.Name, kafkaCluster.Namespace, kafkaCluster.Name, + kafkaCluster.Namespace, broker.Id) + case 2: + // combined node + expectedBrokerConfig = fmt.Sprintf(`advertised.listeners=INTERNAL://%s-%d.%s.svc.cluster.local:29092,TEST://test.host.com:%d +controller.listener.names=CONTROLLER +controller.quorum.voters=1@%s-1.%s.svc.cluster.local:29093,2@%s-2.%s.svc.cluster.local:29093 +cruise.control.metrics.reporter.bootstrap.servers=%s-all-broker.%s.svc.cluster.local:29092 +cruise.control.metrics.reporter.kubernetes.mode=true +inter.broker.listener.name=INTERNAL +listener.security.protocol.map=INTERNAL:PLAINTEXT,CONTROLLER:PLAINTEXT,TEST:PLAINTEXT +listeners=INTERNAL://:29092,CONTROLLER://:29093,TEST://:9094 +log.dirs=/kafka-logs/kafka,/ephemeral-dir1/kafka +metric.reporters=com.linkedin.kafka.cruisecontrol.metricsreporter.CruiseControlMetricsReporter +node.id=%d +process.roles=controller,broker +`, kafkaCluster.Name, broker.Id, kafkaCluster.Namespace, 19090+broker.Id, kafkaCluster.Name, kafkaCluster.Namespace, kafkaCluster.Name, kafkaCluster.Namespace, kafkaCluster.Name, + kafkaCluster.Namespace, broker.Id) + } + } else { + expectedBrokerConfig = fmt.Sprintf(`advertised.listeners=CONTROLLER://kafkacluster-%d-%d.kafka-%d.svc.cluster.local:29093,INTERNAL://kafkacluster-%d-%d.kafka-%d.svc.cluster.local:29092,TEST://test.host.com:%d broker.id=%d control.plane.listener.name=CONTROLLER cruise.control.metrics.reporter.bootstrap.servers=kafkacluster-1-all-broker.kafka-1.svc.cluster.local:29092 @@ -173,7 +225,9 @@ listeners=INTERNAL://:29092,CONTROLLER://:29093,TEST://:9094 log.dirs=/kafka-logs/kafka,/ephemeral-dir1/kafka metric.reporters=com.linkedin.kafka.cruisecontrol.metricsreporter.CruiseControlMetricsReporter zookeeper.connect=/ -`, randomGenTestNumber, broker.Id, randomGenTestNumber, randomGenTestNumber, broker.Id, randomGenTestNumber, 19090+broker.Id, broker.Id))) +`, randomGenTestNumber, broker.Id, randomGenTestNumber, randomGenTestNumber, broker.Id, randomGenTestNumber, 19090+broker.Id, broker.Id) + } + Expect(configMap.Data).To(HaveKeyWithValue("broker-config", expectedBrokerConfig)) // assert log4j? } @@ -255,38 +309,85 @@ func expectKafkaBrokerPod(ctx context.Context, kafkaCluster *v1beta1.KafkaCluste Expect(container.Lifecycle).NotTo(BeNil()) Expect(container.Lifecycle.PreStop).NotTo(BeNil()) getEnvName := func(c corev1.EnvVar) string { return c.Name } - Expect(container.Env).To(ConsistOf( - // the exact value is not interesting - WithTransform(getEnvName, Equal("KAFKA_OPTS")), - WithTransform(getEnvName, Equal("KAFKA_JVM_PERFORMANCE_OPTS")), - - // the exact value should be checked - corev1.EnvVar{ - Name: "ENVOY_SIDECAR_STATUS", - ValueFrom: &corev1.EnvVarSource{ - FieldRef: &corev1.ObjectFieldSelector{ - APIVersion: "v1", - FieldPath: `metadata.annotations['sidecar.istio.io/status']`, + + // when passing a slice to ConsistOf(), the slice needs to be the only argument, which is not applicable here, + // therefore, we need an if-else clause where each condition must have its own ConsistOf() + if kafkaCluster.Spec.KRaftMode { + Expect(container.Env).To(ConsistOf( + // the exact value is not interesting + WithTransform(getEnvName, Equal("KAFKA_OPTS")), + WithTransform(getEnvName, Equal("KAFKA_JVM_PERFORMANCE_OPTS")), + + // the exact value should be checked + corev1.EnvVar{ + Name: "ENVOY_SIDECAR_STATUS", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: `metadata.annotations['sidecar.istio.io/status']`, + }, }, }, - }, - corev1.EnvVar{ - Name: "KAFKA_HEAP_OPTS", - Value: "-Xmx2G -Xms2G", - }, - corev1.EnvVar{ - Name: "ENVVAR1", - Value: "VALUE1 VALUE2", - }, - corev1.EnvVar{ - Name: "ENVVAR2", - Value: "VALUE2", - }, - corev1.EnvVar{ - Name: "CLASSPATH", - Value: "/opt/kafka/libs/extensions/*:/test/class/path", - }, - )) + corev1.EnvVar{ + Name: "KAFKA_HEAP_OPTS", + Value: "-Xmx2G -Xms2G", + }, + corev1.EnvVar{ + Name: "ENVVAR1", + Value: "VALUE1 VALUE2", + }, + corev1.EnvVar{ + Name: "ENVVAR2", + Value: "VALUE2", + }, + corev1.EnvVar{ + Name: "CLASSPATH", + Value: "/opt/kafka/libs/extensions/*:/test/class/path", + }, + corev1.EnvVar{ + Name: "CLUSTER_ID", + Value: kafkaCluster.Status.ClusterID, + }, + corev1.EnvVar{ + Name: "LOG_DIRS", + Value: "/kafka-logs,/ephemeral-dir1", + }, + )) + } else { + Expect(container.Env).To(ConsistOf( + // the exact value is not interesting + WithTransform(getEnvName, Equal("KAFKA_OPTS")), + WithTransform(getEnvName, Equal("KAFKA_JVM_PERFORMANCE_OPTS")), + + // the exact value should be checked + corev1.EnvVar{ + Name: "ENVOY_SIDECAR_STATUS", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: `metadata.annotations['sidecar.istio.io/status']`, + }, + }, + }, + corev1.EnvVar{ + Name: "KAFKA_HEAP_OPTS", + Value: "-Xmx2G -Xms2G", + }, + corev1.EnvVar{ + Name: "ENVVAR1", + Value: "VALUE1 VALUE2", + }, + corev1.EnvVar{ + Name: "ENVVAR2", + Value: "VALUE2", + }, + corev1.EnvVar{ + Name: "CLASSPATH", + Value: "/opt/kafka/libs/extensions/*:/test/class/path", + }, + )) + } + Expect(container.VolumeMounts).To(HaveLen(9)) Expect(container.VolumeMounts[0]).To(Equal(corev1.VolumeMount{ Name: "a-test-volume", @@ -423,19 +524,19 @@ func expectKafkaCRStatus(ctx context.Context, kafkaCluster *v1beta1.KafkaCluster "internal": { { Name: "any-broker", - Address: fmt.Sprintf("%s-all-broker.kafka-1.svc.cluster.local:29092", kafkaCluster.Name), + Address: fmt.Sprintf("%s-all-broker.%s.svc.cluster.local:29092", kafkaCluster.Name, kafkaCluster.Namespace), }, { Name: "broker-0", - Address: fmt.Sprintf("%s-0.kafka-1.svc.cluster.local:29092", kafkaCluster.Name), + Address: fmt.Sprintf("%s-0.%s.svc.cluster.local:29092", kafkaCluster.Name, kafkaCluster.Namespace), }, { Name: "broker-1", - Address: fmt.Sprintf("%s-1.kafka-1.svc.cluster.local:29092", kafkaCluster.Name), + Address: fmt.Sprintf("%s-1.%s.svc.cluster.local:29092", kafkaCluster.Name, kafkaCluster.Namespace), }, { Name: "broker-2", - Address: fmt.Sprintf("%s-2.kafka-1.svc.cluster.local:29092", kafkaCluster.Name), + Address: fmt.Sprintf("%s-2.%s.svc.cluster.local:29092", kafkaCluster.Name, kafkaCluster.Namespace), }, }, }, @@ -461,7 +562,7 @@ func expectKafkaCRStatus(ctx context.Context, kafkaCluster *v1beta1.KafkaCluster }, })) for _, brokerState := range kafkaCluster.Status.BrokersState { - Expect(brokerState.Version).To(Equal("2.7.0")) + Expect(brokerState.Version).To(Equal("3.4.1")) Expect(brokerState.Image).To(Equal(kafkaCluster.Spec.GetClusterImage())) } } diff --git a/controllers/tests/kafkacluster_controller_test.go b/controllers/tests/kafkacluster_controller_test.go index 3a68c5b06..58b2a354f 100644 --- a/controllers/tests/kafkacluster_controller_test.go +++ b/controllers/tests/kafkacluster_controller_test.go @@ -39,6 +39,10 @@ var _ = Describe("KafkaCluster", func() { kafkaCluster *v1beta1.KafkaCluster loadBalancerServiceName string externalListenerHostName string + + kafkaClusterKRaft *v1beta1.KafkaCluster + loadBalancerServiceNameKRaft string + externalListenerHostNameKRaft string ) BeforeEach(func() { @@ -158,6 +162,12 @@ var _ = Describe("KafkaCluster", func() { Value: "VALUE1", }, } + + // use the kafkaCluster above as a blueprint for creating a KafkaCluster under KRaft mode + kafkaClusterKRaft = kafkaCluster.DeepCopy() + kafkaClusterKRaft.Spec.KRaftMode = true + kafkaClusterKRaft.Spec.Brokers = createMinimalKRaftBrokers() + kafkaClusterKRaft.Name = fmt.Sprintf("kafkacluster-kraft-%d", count) }) JustBeforeEach(func(ctx SpecContext) { @@ -181,11 +191,30 @@ var _ = Describe("KafkaCluster", func() { envoyLBService.Status.LoadBalancer.Ingress = []corev1.LoadBalancerIngress{{ Hostname: externalListenerHostName, }} - err = k8sClient.Status().Update(ctx, envoyLBService) Expect(err).NotTo(HaveOccurred()) waitForClusterRunningState(ctx, kafkaCluster, namespace) + + /* same tests for KRaft mode */ + By("creating kafka cluster object under KRaft mode " + kafkaClusterKRaft.Name + " in namespace " + namespace) + err = k8sClient.Create(ctx, kafkaClusterKRaft) + Expect(err).NotTo(HaveOccurred()) + + envoyLBServiceKRaft := &corev1.Service{} + Eventually(ctx, func() error { + return k8sClient.Get(ctx, types.NamespacedName{ + Name: loadBalancerServiceNameKRaft, + Namespace: namespace, + }, envoyLBServiceKRaft) + }, 5*time.Second, 100*time.Millisecond).Should(Succeed()) + + envoyLBServiceKRaft.Status.LoadBalancer.Ingress = []corev1.LoadBalancerIngress{{ + Hostname: externalListenerHostNameKRaft, + }} + err = k8sClient.Status().Update(ctx, envoyLBServiceKRaft) + Expect(err).NotTo(HaveOccurred()) + waitForClusterRunningState(ctx, kafkaClusterKRaft, namespace) }) JustAfterEach(func(ctx SpecContext) { @@ -195,12 +224,19 @@ var _ = Describe("KafkaCluster", func() { err := k8sClient.Delete(ctx, kafkaCluster) Expect(err).NotTo(HaveOccurred()) + By("deleting Kafka cluster object under KRaft mode " + kafkaClusterKRaft.Name + " in namespace " + kafkaClusterKRaft.Namespace) + err = k8sClient.Delete(ctx, kafkaClusterKRaft) + Expect(err).NotTo(HaveOccurred()) + kafkaCluster = nil }) When("using default configuration", func() { BeforeEach(func() { loadBalancerServiceName = fmt.Sprintf("envoy-loadbalancer-test-%s", kafkaCluster.Name) externalListenerHostName = "test.host.com" + + loadBalancerServiceNameKRaft = fmt.Sprintf("envoy-loadbalancer-test-%s", kafkaClusterKRaft.Name) + externalListenerHostNameKRaft = "test.host.com" }) It("should reconciles objects properly", func(ctx SpecContext) { expectEnvoy(ctx, kafkaCluster, []string{"test"}) @@ -209,6 +245,14 @@ var _ = Describe("KafkaCluster", func() { expectKafka(ctx, kafkaCluster, count) expectCruiseControl(ctx, kafkaCluster) }) + + It("should reconciles objects properly for Kafka cluster un KRaft mode", func(ctx SpecContext) { + expectEnvoy(ctx, kafkaClusterKRaft, []string{"test"}) + expectKafkaMonitoring(ctx, kafkaClusterKRaft) + expectCruiseControlMonitoring(ctx, kafkaClusterKRaft) + expectKafka(ctx, kafkaClusterKRaft, count) + expectCruiseControl(ctx, kafkaClusterKRaft) + }) }) When("configuring one ingress envoy controller config inside the external listener without bindings", func() { BeforeEach(func() { @@ -224,10 +268,27 @@ var _ = Describe("KafkaCluster", func() { kafkaCluster.Spec.ListenersConfig.ExternalListeners[0] = testExternalListener loadBalancerServiceName = fmt.Sprintf("envoy-loadbalancer-test-az1-%s", kafkaCluster.Name) externalListenerHostName = "external.az1.host.com" + + // use the kafkaCluster above as a blueprint for creating a KafkaCluster under KRaft mode + testExternalListenerKRaft := kafkaClusterKRaft.Spec.ListenersConfig.ExternalListeners[0] + testExternalListenerKRaft.Config = &v1beta1.Config{ + DefaultIngressConfig: "az1", + IngressConfig: map[string]v1beta1.IngressConfig{ + "az1": {EnvoyConfig: &v1beta1.EnvoyConfig{ + Annotations: map[string]string{"zone": "az1"}, + }}, + }, + } + kafkaClusterKRaft.Spec.ListenersConfig.ExternalListeners[0] = testExternalListenerKRaft + loadBalancerServiceNameKRaft = fmt.Sprintf("envoy-loadbalancer-test-az1-%s", kafkaClusterKRaft.Name) + externalListenerHostNameKRaft = "external.az1.host.com" }) It("should reconcile object properly", func(ctx SpecContext) { expectDefaultBrokerSettingsForExternalListenerBinding(ctx, kafkaCluster, count) expectEnvoy(ctx, kafkaCluster, []string{"test-az1"}) + + expectDefaultBrokerSettingsForExternalListenerBinding(ctx, kafkaClusterKRaft, count) + expectEnvoy(ctx, kafkaClusterKRaft, []string{"test-az1"}) }) }) When("configuring two ingress envoy controller config inside the external listener without bindings", func() { @@ -249,10 +310,32 @@ var _ = Describe("KafkaCluster", func() { kafkaCluster.Spec.ListenersConfig.ExternalListeners[0] = testExternalListener loadBalancerServiceName = fmt.Sprintf("envoy-loadbalancer-test-az1-%s", kafkaCluster.Name) externalListenerHostName = "external.az1.host.com" + + // KRaft + testExternalListenerKRaft := kafkaClusterKRaft.Spec.ListenersConfig.ExternalListeners[0] + testExternalListenerKRaft.Config = &v1beta1.Config{ + DefaultIngressConfig: "az1", + IngressConfig: map[string]v1beta1.IngressConfig{ + "az1": {EnvoyConfig: &v1beta1.EnvoyConfig{ + Annotations: map[string]string{"zone": "az1"}, + }, + }, + "az2": {EnvoyConfig: &v1beta1.EnvoyConfig{ + Annotations: map[string]string{"zone": "az2"}, + }, + }, + }, + } + kafkaClusterKRaft.Spec.ListenersConfig.ExternalListeners[0] = testExternalListenerKRaft + loadBalancerServiceNameKRaft = fmt.Sprintf("envoy-loadbalancer-test-az1-%s", kafkaClusterKRaft.Name) + externalListenerHostNameKRaft = "external.az1.host.com" }) It("should reconcile object properly", func(ctx SpecContext) { expectDefaultBrokerSettingsForExternalListenerBinding(ctx, kafkaCluster, count) expectEnvoy(ctx, kafkaCluster, []string{"test-az1"}) + + expectDefaultBrokerSettingsForExternalListenerBinding(ctx, kafkaClusterKRaft, count) + expectEnvoy(ctx, kafkaClusterKRaft, []string{"test-az1"}) }) }) When("configuring two ingress envoy controller config inside the external listener without using the default", func() { @@ -277,19 +360,43 @@ var _ = Describe("KafkaCluster", func() { kafkaCluster.Spec.BrokerConfigGroups["default"] = defaultBrokerConfig loadBalancerServiceName = fmt.Sprintf("envoy-loadbalancer-test-az2-%s", kafkaCluster.Name) externalListenerHostName = "external.az2.host.com" + + // KRaft + testExternalListenerKRaft := kafkaClusterKRaft.Spec.ListenersConfig.ExternalListeners[0] + testExternalListenerKRaft.Config = &v1beta1.Config{ + DefaultIngressConfig: "az1", + IngressConfig: map[string]v1beta1.IngressConfig{ + "az1": {EnvoyConfig: &v1beta1.EnvoyConfig{ + Annotations: map[string]string{"zone": "az1"}, + }, + }, + "az2": {EnvoyConfig: &v1beta1.EnvoyConfig{ + Annotations: map[string]string{"zone": "az2"}, + }, + }, + }, + } + kafkaClusterKRaft.Spec.ListenersConfig.ExternalListeners[0] = testExternalListenerKRaft + defaultBrokerConfigKRaft := kafkaClusterKRaft.Spec.BrokerConfigGroups["default"] + defaultBrokerConfigKRaft.BrokerIngressMapping = []string{"az2"} + kafkaClusterKRaft.Spec.BrokerConfigGroups["default"] = defaultBrokerConfigKRaft + loadBalancerServiceNameKRaft = fmt.Sprintf("envoy-loadbalancer-test-az2-%s", kafkaClusterKRaft.Name) + externalListenerHostNameKRaft = "external.az2.host.com" }) It("should reconcile object properly", func(ctx SpecContext) { expectEnvoy(ctx, kafkaCluster, []string{"test-az2"}) + expectEnvoy(ctx, kafkaClusterKRaft, []string{"test-az2"}) }) }) }) var _ = Describe("KafkaCluster with two config external listener", func() { var ( - count uint64 = 0 - namespace string - namespaceObj *corev1.Namespace - kafkaCluster *v1beta1.KafkaCluster + count uint64 = 0 + namespace string + namespaceObj *corev1.Namespace + kafkaCluster *v1beta1.KafkaCluster + kafkaClusterKRaft *v1beta1.KafkaCluster ) BeforeEach(func() { @@ -320,6 +427,12 @@ var _ = Describe("KafkaCluster with two config external listener", func() { }, } kafkaCluster.Spec.ListenersConfig.ExternalListeners[0] = testExternalListener + + // use the kafkaCluster above as a blueprint for creating a KafkaCluster under KRaft mode + kafkaClusterKRaft = kafkaCluster.DeepCopy() + kafkaClusterKRaft.Spec.KRaftMode = true + kafkaClusterKRaft.Spec.Brokers = createMinimalKRaftBrokers() + kafkaClusterKRaft.Name = fmt.Sprintf("kafkacluster-kraft-%d", count) }) JustBeforeEach(func(ctx SpecContext) { By("creating namespace " + namespace) @@ -364,18 +477,63 @@ var _ = Describe("KafkaCluster with two config external listener", func() { Expect(err).NotTo(HaveOccurred()) waitForClusterRunningState(ctx, kafkaCluster, namespace) + + /* same tests for KRaft mode */ + + By("creating kafka cluster object in KRaft mode " + kafkaClusterKRaft.Name + " in namespace " + namespace) + err = k8sClient.Create(ctx, kafkaClusterKRaft) + Expect(err).NotTo(HaveOccurred()) + + envoyLBServiceKRaft := &corev1.Service{} + Eventually(ctx, func() error { + return k8sClient.Get(ctx, types.NamespacedName{ + Name: fmt.Sprintf("envoy-loadbalancer-test-az1-%s", kafkaClusterKRaft.Name), + Namespace: namespace, + }, envoyLBServiceKRaft) + }, 5*time.Second, 100*time.Millisecond).Should(Succeed()) + + envoyLBServiceKRaft.Status.LoadBalancer.Ingress = []corev1.LoadBalancerIngress{{ + Hostname: "external.az1.host.com", + }} + err = k8sClient.Status().Update(ctx, envoyLBServiceKRaft) + Expect(err).NotTo(HaveOccurred()) + + envoyLBServiceKRaft = &corev1.Service{} + Eventually(ctx, func() error { + return k8sClient.Get(ctx, types.NamespacedName{ + Name: fmt.Sprintf("envoy-loadbalancer-test-az2-%s", kafkaClusterKRaft.Name), + Namespace: namespace, + }, envoyLBServiceKRaft) + }, 5*time.Second, 100*time.Millisecond).Should(Succeed()) + + envoyLBServiceKRaft.Status.LoadBalancer.Ingress = []corev1.LoadBalancerIngress{{ + Hostname: "external.az2.host.com", + }} + err = k8sClient.Status().Update(ctx, envoyLBServiceKRaft) + Expect(err).NotTo(HaveOccurred()) + + waitForClusterRunningState(ctx, kafkaClusterKRaft, namespace) }) When("configuring two ingress envoy controller config inside the external listener using both as bindings", func() { BeforeEach(func() { kafkaCluster.Spec.Brokers[0].BrokerConfig = &v1beta1.BrokerConfig{BrokerIngressMapping: []string{"az1"}} kafkaCluster.Spec.Brokers[1].BrokerConfig = &v1beta1.BrokerConfig{BrokerIngressMapping: []string{"az2"}} + + /* same tests for KafkaCluster in KRaft mode */ + kafkaClusterKRaft.Spec.Brokers[0].BrokerConfig = &v1beta1.BrokerConfig{BrokerIngressMapping: []string{"az1"}} + kafkaClusterKRaft.Spec.Brokers[1].BrokerConfig = &v1beta1.BrokerConfig{BrokerIngressMapping: []string{"az2"}} }) It("should reconcile object properly", func(ctx SpecContext) { expectEnvoyWithConfigAz1(ctx, kafkaCluster) expectEnvoyWithConfigAz2(ctx, kafkaCluster) expectBrokerConfigmapForAz1ExternalListener(ctx, kafkaCluster, count) expectBrokerConfigmapForAz2ExternalListener(ctx, kafkaCluster, count) + + expectEnvoyWithConfigAz1(ctx, kafkaClusterKRaft) + expectEnvoyWithConfigAz2(ctx, kafkaClusterKRaft) + expectBrokerConfigmapForAz1ExternalListener(ctx, kafkaClusterKRaft, count) + expectBrokerConfigmapForAz2ExternalListener(ctx, kafkaClusterKRaft, count) }) }) }) @@ -395,7 +553,6 @@ func expectKafkaMonitoring(ctx context.Context, kafkaCluster *v1beta1.KafkaClust func expectCruiseControlMonitoring(ctx context.Context, kafkaCluster *v1beta1.KafkaCluster) { configMap := corev1.ConfigMap{} configMapName := fmt.Sprintf("%s-cc-jmx-exporter", kafkaCluster.Name) - // logf.Log.Info("name", "name", configMapName) Eventually(ctx, func() error { err := k8sClient.Get(ctx, types.NamespacedName{Name: configMapName, Namespace: kafkaCluster.Namespace}, &configMap) return err @@ -404,3 +561,23 @@ func expectCruiseControlMonitoring(ctx context.Context, kafkaCluster *v1beta1.Ka Expect(configMap.Labels).To(And(HaveKeyWithValue(v1beta1.AppLabelKey, "cruisecontrol-jmx"), HaveKeyWithValue(v1beta1.KafkaCRLabelKey, kafkaCluster.Name))) Expect(configMap.Data).To(HaveKeyWithValue("config.yaml", kafkaCluster.Spec.MonitoringConfig.CCJMXExporterConfig)) } + +func createMinimalKRaftBrokers() []v1beta1.Broker { + return []v1beta1.Broker{ + { + Id: int32(0), + Roles: []string{"broker"}, + BrokerConfigGroup: defaultBrokerConfigGroup, + }, + { + Id: int32(1), + Roles: []string{"controller"}, + BrokerConfigGroup: defaultBrokerConfigGroup, + }, + { + Id: int32(2), + Roles: []string{"controller", "broker"}, + BrokerConfigGroup: defaultBrokerConfigGroup, + }, + } +} diff --git a/pkg/jmxextractor/mock_extractor.go b/pkg/jmxextractor/mock_extractor.go index 79c8c2b81..f8c56ac13 100644 --- a/pkg/jmxextractor/mock_extractor.go +++ b/pkg/jmxextractor/mock_extractor.go @@ -20,5 +20,5 @@ type mockJmxExtractor struct{} func (exp *mockJmxExtractor) ExtractDockerImageAndVersion(brokerId int32, brokerConfig *v1beta1.BrokerConfig, clusterImage string, headlessServiceEnabled bool) (*v1beta1.KafkaVersion, error) { - return &v1beta1.KafkaVersion{Image: clusterImage, Version: "2.7.0"}, nil + return &v1beta1.KafkaVersion{Image: clusterImage, Version: "3.4.1"}, nil } diff --git a/pkg/resources/cruisecontrol/configmap.go b/pkg/resources/cruisecontrol/configmap.go index bdade79e1..e845ef086 100644 --- a/pkg/resources/cruisecontrol/configmap.go +++ b/pkg/resources/cruisecontrol/configmap.go @@ -60,13 +60,7 @@ func (r *Reconciler) configMap(clientPass string, capacityConfig string, log log log.Error(err, fmt.Sprintf("setting '%s' in Cruise Control configuration failed", kafkautils.KafkaConfigBoostrapServers), "config", bootstrapServers) } - if !r.KafkaCluster.Spec.KraftMode() { - // Add Zookeeper configuration when we are in Zookeeper mode only - zkConnect := zookeeperutils.PrepareConnectionAddress(r.KafkaCluster.Spec.ZKAddresses, r.KafkaCluster.Spec.GetZkPath()) - if err = ccConfig.Set(kafkautils.KafkaConfigZooKeeperConnect, zkConnect); err != nil { - log.Error(err, fmt.Sprintf("setting '%s' in Cruise Control configuration failed", kafkautils.KafkaConfigZooKeeperConnect), "config", zkConnect) - } - } else { + if r.KafkaCluster.Spec.KRaftMode { // Set configurations to have Cruise Control to run without Zookeeper if err = ccConfig.Set(kafkautils.CruiseControlConfigTopicConfigProviderClass, kafkautils.CruiseControlConfigTopicConfigProviderClassVal); err != nil { log.Error(err, fmt.Sprintf("setting '%s' in Cruise Control configuration failed", kafkautils.CruiseControlConfigTopicConfigProviderClass), "config", kafkautils.CruiseControlConfigTopicConfigProviderClassVal) @@ -75,6 +69,13 @@ func (r *Reconciler) configMap(clientPass string, capacityConfig string, log log if err = ccConfig.Set(kafkautils.CruiseControlConfigKafkaBrokerFailureDetectionEnable, kafkautils.CruiseControlConfigKafkaBrokerFailureDetectionEnableVal); err != nil { log.Error(err, fmt.Sprintf("setting '%s' in Cruise Control configuration failed", kafkautils.CruiseControlConfigKafkaBrokerFailureDetectionEnable), "config", kafkautils.CruiseControlConfigKafkaBrokerFailureDetectionEnableVal) } + + } else { + // Add Zookeeper configuration when we are in Zookeeper mode only + zkConnect := zookeeperutils.PrepareConnectionAddress(r.KafkaCluster.Spec.ZKAddresses, r.KafkaCluster.Spec.GetZkPath()) + if err = ccConfig.Set(kafkautils.KafkaConfigZooKeeperConnect, zkConnect); err != nil { + log.Error(err, fmt.Sprintf("setting '%s' in Cruise Control configuration failed", kafkautils.KafkaConfigZooKeeperConnect), "config", zkConnect) + } } // Add SSL configuration diff --git a/pkg/resources/kafka/configmap.go b/pkg/resources/kafka/configmap.go index da2c45980..8354a72fd 100644 --- a/pkg/resources/kafka/configmap.go +++ b/pkg/resources/kafka/configmap.go @@ -52,7 +52,7 @@ func (r *Reconciler) getConfigProperties(bConfig *v1beta1.BrokerConfig, broker v configCCMetricsReporter(r.KafkaCluster, config, clientPass, bootstrapServers, log) // Kafka Broker configurations - if r.KafkaCluster.Spec.KraftMode() { + if r.KafkaCluster.Spec.KRaftMode { configureBrokerKRaftMode(broker, r.KafkaCluster, config, serverPasses, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses, log) } else { configureBrokerZKMode(broker.Id, r.KafkaCluster, config, serverPasses, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses, log) diff --git a/pkg/resources/kafka/pod.go b/pkg/resources/kafka/pod.go index 274190556..b38151e71 100644 --- a/pkg/resources/kafka/pod.go +++ b/pkg/resources/kafka/pod.go @@ -168,7 +168,7 @@ fi`}, pod.Spec.Subdomain = fmt.Sprintf(kafkautils.HeadlessServiceTemplate, r.KafkaCluster.Name) } - if r.KafkaCluster.Spec.KraftMode() { + if r.KafkaCluster.Spec.KRaftMode { for i, container := range pod.Spec.Containers { if container.Name == kafkaContainerName { // in KRaft mode, all broker nodes within the same Kafka cluster need to use the same cluster ID to format the storage From 78e63ee3b9bf909f41c2cb36e73b018481cc279e Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Mon, 24 Jul 2023 21:33:05 -0400 Subject: [PATCH 13/22] make lint-fix --- controllers/tests/kafkacluster_controller_test.go | 10 ++++++---- pkg/resources/cruisecontrol/configmap.go | 1 - pkg/resources/kafka/configmap.go | 3 ++- pkg/resources/kafka/util.go | 3 ++- pkg/resources/kafka/util_test.go | 1 - 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/controllers/tests/kafkacluster_controller_test.go b/controllers/tests/kafkacluster_controller_test.go index 58b2a354f..363298855 100644 --- a/controllers/tests/kafkacluster_controller_test.go +++ b/controllers/tests/kafkacluster_controller_test.go @@ -45,6 +45,8 @@ var _ = Describe("KafkaCluster", func() { externalListenerHostNameKRaft string ) + const lbIngressHostName = "external.az1.host.com" + BeforeEach(func() { atomic.AddUint64(&count, 1) @@ -267,7 +269,7 @@ var _ = Describe("KafkaCluster", func() { } kafkaCluster.Spec.ListenersConfig.ExternalListeners[0] = testExternalListener loadBalancerServiceName = fmt.Sprintf("envoy-loadbalancer-test-az1-%s", kafkaCluster.Name) - externalListenerHostName = "external.az1.host.com" + externalListenerHostName = lbIngressHostName // use the kafkaCluster above as a blueprint for creating a KafkaCluster under KRaft mode testExternalListenerKRaft := kafkaClusterKRaft.Spec.ListenersConfig.ExternalListeners[0] @@ -281,7 +283,7 @@ var _ = Describe("KafkaCluster", func() { } kafkaClusterKRaft.Spec.ListenersConfig.ExternalListeners[0] = testExternalListenerKRaft loadBalancerServiceNameKRaft = fmt.Sprintf("envoy-loadbalancer-test-az1-%s", kafkaClusterKRaft.Name) - externalListenerHostNameKRaft = "external.az1.host.com" + externalListenerHostNameKRaft = lbIngressHostName }) It("should reconcile object properly", func(ctx SpecContext) { expectDefaultBrokerSettingsForExternalListenerBinding(ctx, kafkaCluster, count) @@ -309,7 +311,7 @@ var _ = Describe("KafkaCluster", func() { } kafkaCluster.Spec.ListenersConfig.ExternalListeners[0] = testExternalListener loadBalancerServiceName = fmt.Sprintf("envoy-loadbalancer-test-az1-%s", kafkaCluster.Name) - externalListenerHostName = "external.az1.host.com" + externalListenerHostName = lbIngressHostName // KRaft testExternalListenerKRaft := kafkaClusterKRaft.Spec.ListenersConfig.ExternalListeners[0] @@ -328,7 +330,7 @@ var _ = Describe("KafkaCluster", func() { } kafkaClusterKRaft.Spec.ListenersConfig.ExternalListeners[0] = testExternalListenerKRaft loadBalancerServiceNameKRaft = fmt.Sprintf("envoy-loadbalancer-test-az1-%s", kafkaClusterKRaft.Name) - externalListenerHostNameKRaft = "external.az1.host.com" + externalListenerHostNameKRaft = lbIngressHostName }) It("should reconcile object properly", func(ctx SpecContext) { expectDefaultBrokerSettingsForExternalListenerBinding(ctx, kafkaCluster, count) diff --git a/pkg/resources/cruisecontrol/configmap.go b/pkg/resources/cruisecontrol/configmap.go index e845ef086..0fe745d18 100644 --- a/pkg/resources/cruisecontrol/configmap.go +++ b/pkg/resources/cruisecontrol/configmap.go @@ -69,7 +69,6 @@ func (r *Reconciler) configMap(clientPass string, capacityConfig string, log log if err = ccConfig.Set(kafkautils.CruiseControlConfigKafkaBrokerFailureDetectionEnable, kafkautils.CruiseControlConfigKafkaBrokerFailureDetectionEnableVal); err != nil { log.Error(err, fmt.Sprintf("setting '%s' in Cruise Control configuration failed", kafkautils.CruiseControlConfigKafkaBrokerFailureDetectionEnable), "config", kafkautils.CruiseControlConfigKafkaBrokerFailureDetectionEnableVal) } - } else { // Add Zookeeper configuration when we are in Zookeeper mode only zkConnect := zookeeperutils.PrepareConnectionAddress(r.KafkaCluster.Spec.ZKAddresses, r.KafkaCluster.Spec.GetZkPath()) diff --git a/pkg/resources/kafka/configmap.go b/pkg/resources/kafka/configmap.go index 8354a72fd..8d3169a64 100644 --- a/pkg/resources/kafka/configmap.go +++ b/pkg/resources/kafka/configmap.go @@ -21,11 +21,12 @@ import ( "strings" "emperror.dev/errors" - zookeeperutils "github.com/banzaicloud/koperator/pkg/util/zookeeper" "github.com/go-logr/logr" apierrors "k8s.io/apimachinery/pkg/api/errors" "sigs.k8s.io/controller-runtime/pkg/client" + zookeeperutils "github.com/banzaicloud/koperator/pkg/util/zookeeper" + corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1" diff --git a/pkg/resources/kafka/util.go b/pkg/resources/kafka/util.go index 6ae9ee3f7..3bb6088e2 100644 --- a/pkg/resources/kafka/util.go +++ b/pkg/resources/kafka/util.go @@ -19,9 +19,10 @@ import ( "fmt" "sort" + "github.com/google/uuid" + "github.com/banzaicloud/koperator/api/v1beta1" "github.com/banzaicloud/koperator/pkg/util" - "github.com/google/uuid" ) // generateQuorumVoters generates the quorum voters in the format of brokerID@nodeAddress:listenerPort diff --git a/pkg/resources/kafka/util_test.go b/pkg/resources/kafka/util_test.go index b380eac3c..b29a6c780 100644 --- a/pkg/resources/kafka/util_test.go +++ b/pkg/resources/kafka/util_test.go @@ -43,7 +43,6 @@ func TestGenerateClusterID(t *testing.T) { } func TestGenerateQuorumVoters(t *testing.T) { - tests := []struct { testName string brokers []v1beta1.Broker From 3b5aff9d649f7331e436144a24bcad18386f80d0 Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Mon, 24 Jul 2023 22:14:36 -0400 Subject: [PATCH 14/22] Update static KafkaCluster yamls; add check for kraft mode before setting ClusterID in status --- .../banzaicloud_v1beta1_kafkacluster.yaml | 4 + .../kafkacluster_with_external_ssl.yaml | 1 + ...acluster_with_external_ssl_customcert.yaml | 1 + .../samples/kafkacluster_with_ssl_groups.yaml | 1 + ...fkacluster_with_ssl_groups_customcert.yaml | 1 + ...fkacluster_with_ssl_hybrid_customcert.yaml | 1 + config/samples/kafkacluster_without_ssl.yaml | 1 + .../kafkacluster_without_ssl_groups.yaml | 1 + .../kraft/simplekafkacluster_kraft.yaml | 282 ++++++++++++++++++ ...implekafkacluster-with-brokerbindings.yaml | 1 + ...lekafkacluster-with-nodeport-external.yaml | 1 + config/samples/simplekafkacluster.yaml | 1 + .../samples/simplekafkacluster_affinity.yaml | 1 + .../samples/simplekafkacluster_ebs_csi.yaml | 1 + config/samples/simplekafkacluster_ssl.yaml | 1 + ...fkacluster_with_k8s_provided_nodeport.yaml | 1 + .../samples/simplekafkacluster_with_sasl.yaml | 1 + docs/benchmarks/infrastructure/kafka.yaml | 1 + .../infrastructure/kafka_202001_3brokers.yaml | 1 + pkg/resources/kafka/kafka.go | 2 +- 20 files changed, 304 insertions(+), 1 deletion(-) create mode 100644 config/samples/kraft/simplekafkacluster_kraft.yaml diff --git a/config/samples/banzaicloud_v1beta1_kafkacluster.yaml b/config/samples/banzaicloud_v1beta1_kafkacluster.yaml index 47aefbd0a..a4841250d 100644 --- a/config/samples/banzaicloud_v1beta1_kafkacluster.yaml +++ b/config/samples/banzaicloud_v1beta1_kafkacluster.yaml @@ -5,6 +5,8 @@ metadata: controller-tools.k8s.io: "1.0" name: kafka spec: + # specify if the cluster is in KRaft mode or not, default is true + kRaft: false # Specify if the cluster should use headlessService for Kafka or individual services # using service/broker may come in handy in case of service mesh headlessServiceEnabled: true @@ -16,10 +18,12 @@ spec: # Specify the usable ingress controller, only envoy and istioingress supported can be left blank ingressController: "envoy" # Specify the zookeeper addresses where the Kafka should store it's metadata + # (this should not be set in KRaft mode) zkAddresses: - "zookeeper-client.zookeeper:2181" # Specify the zookeeper path where the Kafka related metadatas should be placed # By default it is bound to "/" and can be left blank + # (this should not be set in KRaft mode) zkPath: "/kafka" # rackAwareness add support for Kafka rack aware feature rackAwareness: diff --git a/config/samples/kafkacluster_with_external_ssl.yaml b/config/samples/kafkacluster_with_external_ssl.yaml index 7a9958101..d00eb827c 100644 --- a/config/samples/kafkacluster_with_external_ssl.yaml +++ b/config/samples/kafkacluster_with_external_ssl.yaml @@ -5,6 +5,7 @@ metadata: controller-tools.k8s.io: "1.0" name: kafka spec: + kRaft: false headlessServiceEnabled: true zkAddresses: - "zookeeper-server-client.zookeeper:2181" diff --git a/config/samples/kafkacluster_with_external_ssl_customcert.yaml b/config/samples/kafkacluster_with_external_ssl_customcert.yaml index 1d6c7abed..f3ef04c64 100644 --- a/config/samples/kafkacluster_with_external_ssl_customcert.yaml +++ b/config/samples/kafkacluster_with_external_ssl_customcert.yaml @@ -5,6 +5,7 @@ metadata: controller-tools.k8s.io: "1.0" name: kafka spec: + kRaft: false headlessServiceEnabled: true zkAddresses: - "zookeeper-server-client.zookeeper:2181" diff --git a/config/samples/kafkacluster_with_ssl_groups.yaml b/config/samples/kafkacluster_with_ssl_groups.yaml index 5e8bc1685..ead5e23a1 100644 --- a/config/samples/kafkacluster_with_ssl_groups.yaml +++ b/config/samples/kafkacluster_with_ssl_groups.yaml @@ -5,6 +5,7 @@ metadata: controller-tools.k8s.io: "1.0" name: kafka spec: + kRaft: false headlessServiceEnabled: true zkAddresses: - "zookeeper-server-client.zookeeper:2181" diff --git a/config/samples/kafkacluster_with_ssl_groups_customcert.yaml b/config/samples/kafkacluster_with_ssl_groups_customcert.yaml index c3e31d67e..19da5857f 100644 --- a/config/samples/kafkacluster_with_ssl_groups_customcert.yaml +++ b/config/samples/kafkacluster_with_ssl_groups_customcert.yaml @@ -5,6 +5,7 @@ metadata: controller-tools.k8s.io: "1.0" name: kafka spec: + kRaft: false headlessServiceEnabled: true zkAddresses: - "zookeeper-server-client.zookeeper:2181" diff --git a/config/samples/kafkacluster_with_ssl_hybrid_customcert.yaml b/config/samples/kafkacluster_with_ssl_hybrid_customcert.yaml index f2faf5fac..7dd18ee69 100644 --- a/config/samples/kafkacluster_with_ssl_hybrid_customcert.yaml +++ b/config/samples/kafkacluster_with_ssl_hybrid_customcert.yaml @@ -5,6 +5,7 @@ metadata: controller-tools.k8s.io: "1.0" name: kafka spec: + kRaft: false headlessServiceEnabled: true zkAddresses: - "zookeeper-server-client.zookeeper:2181" diff --git a/config/samples/kafkacluster_without_ssl.yaml b/config/samples/kafkacluster_without_ssl.yaml index 13216fa2b..7b652007f 100644 --- a/config/samples/kafkacluster_without_ssl.yaml +++ b/config/samples/kafkacluster_without_ssl.yaml @@ -5,6 +5,7 @@ metadata: controller-tools.k8s.io: "1.0" name: kafka spec: + kRaft: false headlessServiceEnabled: true zkAddresses: - "zookeeper-server-client.zookeeper:2181" diff --git a/config/samples/kafkacluster_without_ssl_groups.yaml b/config/samples/kafkacluster_without_ssl_groups.yaml index 6ae9e451c..9cd0507c3 100644 --- a/config/samples/kafkacluster_without_ssl_groups.yaml +++ b/config/samples/kafkacluster_without_ssl_groups.yaml @@ -5,6 +5,7 @@ metadata: controller-tools.k8s.io: "1.0" name: kafka spec: + kRaft: false headlessServiceEnabled: true zkAddresses: - "zookeeper-server-client.zookeeper:2181" diff --git a/config/samples/kraft/simplekafkacluster_kraft.yaml b/config/samples/kraft/simplekafkacluster_kraft.yaml new file mode 100644 index 000000000..43b81c378 --- /dev/null +++ b/config/samples/kraft/simplekafkacluster_kraft.yaml @@ -0,0 +1,282 @@ +apiVersion: kafka.banzaicloud.io/v1beta1 +kind: KafkaCluster +metadata: + labels: + controller-tools.k8s.io: "1.0" + name: kafka +spec: + kRaft: true + monitoringConfig: + jmxImage: "ghcr.io/banzaicloud/jmx-javaagent:0.16.1" + headlessServiceEnabled: true + propagateLabels: false + oneBrokerPerNode: false + clusterImage: "ghcr.io/banzaicloud/kafka:2.13-3.4.1" + readOnlyConfig: | + auto.create.topics.enable=false + cruise.control.metrics.topic.auto.create=true + cruise.control.metrics.topic.num.partitions=1 + cruise.control.metrics.topic.replication.factor=2 + brokerConfigGroups: + default: + storageConfigs: + - mountPath: "/kafka-logs" + pvcSpec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 10Gi + - mountPath: "/kafka-logs-2" + pvcSpec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 10Gi + brokerAnnotations: + prometheus.io/scrape: "true" + prometheus.io/port: "9020" + brokers: + - id: 0 + brokerConfigGroup: "default" + processRoles: + - broker + - id: 1 + brokerConfigGroup: "default" + processRoles: + - broker + - id: 2 + brokerConfigGroup: "default" + processRoles: + - broker + - id: 3 + brokerConfigGroup: "default" + processRoles: + - controller + - broker + - id: 4 + brokerConfigGroup: "default" + processRoles: + - controller + - id: 5 + brokerConfigGroup: "default" + processRoles: + - controller + rollingUpgradeConfig: + failureThreshold: 1 + listenersConfig: + internalListeners: + - type: "plaintext" + name: "internal" + containerPort: 29092 + usedForInnerBrokerCommunication: true + - type: "plaintext" + name: "controller" + containerPort: 29093 + usedForInnerBrokerCommunication: false + usedForControllerCommunication: true + cruiseControlConfig: + # podSecurityContext: + # runAsNonRoot: false + # securityContext: + # privileged: true + cruiseControlTaskSpec: + RetryDurationMinutes: 5 + topicConfig: + partitions: 12 + replicationFactor: 3 +# resourceRequirements: +# requests: +# cpu: 500m +# memory: 1Gi +# limits: +# cpu: 500m +# memory: 1Gi +# image: "ghcr.io/banzaicloud/cruise-control:2.5.86" + config: | + # Copyright 2017 LinkedIn Corp. Licensed under the BSD 2-Clause License (the "License"). See License in the project root for license information. + # + # This is an example property file for Kafka Cruise Control. See KafkaCruiseControlConfig for more details. + # Configuration for the metadata client. + # ======================================= + # The maximum interval in milliseconds between two metadata refreshes. + #metadata.max.age.ms=300000 + # Client id for the Cruise Control. It is used for the metadata client. + #client.id=kafka-cruise-control + # The size of TCP send buffer bytes for the metadata client. + #send.buffer.bytes=131072 + # The size of TCP receive buffer size for the metadata client. + #receive.buffer.bytes=131072 + # The time to wait before disconnect an idle TCP connection. + #connections.max.idle.ms=540000 + # The time to wait before reconnect to a given host. + #reconnect.backoff.ms=50 + # The time to wait for a response from a host after sending a request. + #request.timeout.ms=30000 + # Configurations for the load monitor + # ======================================= + # The number of metric fetcher thread to fetch metrics for the Kafka cluster + num.metric.fetchers=1 + # The metric sampler class + metric.sampler.class=com.linkedin.kafka.cruisecontrol.monitor.sampling.CruiseControlMetricsReporterSampler + # Configurations for CruiseControlMetricsReporterSampler + metric.reporter.topic.pattern=__CruiseControlMetrics + # The sample store class name + sample.store.class=com.linkedin.kafka.cruisecontrol.monitor.sampling.KafkaSampleStore + # The config for the Kafka sample store to save the partition metric samples + partition.metric.sample.store.topic=__KafkaCruiseControlPartitionMetricSamples + # The config for the Kafka sample store to save the model training samples + broker.metric.sample.store.topic=__KafkaCruiseControlModelTrainingSamples + # The replication factor of Kafka metric sample store topic + sample.store.topic.replication.factor=2 + # The config for the number of Kafka sample store consumer threads + num.sample.loading.threads=8 + # The partition assignor class for the metric samplers + metric.sampler.partition.assignor.class=com.linkedin.kafka.cruisecontrol.monitor.sampling.DefaultMetricSamplerPartitionAssignor + # The metric sampling interval in milliseconds + metric.sampling.interval.ms=120000 + metric.anomaly.detection.interval.ms=180000 + # The partition metrics window size in milliseconds + partition.metrics.window.ms=300000 + # The number of partition metric windows to keep in memory + num.partition.metrics.windows=1 + # The minimum partition metric samples required for a partition in each window + min.samples.per.partition.metrics.window=1 + # The broker metrics window size in milliseconds + broker.metrics.window.ms=300000 + # The number of broker metric windows to keep in memory + num.broker.metrics.windows=20 + # The minimum broker metric samples required for a partition in each window + min.samples.per.broker.metrics.window=1 + # The configuration for the BrokerCapacityConfigFileResolver (supports JBOD and non-JBOD broker capacities) + capacity.config.file=config/capacity.json + #capacity.config.file=config/capacityJBOD.json + # Configurations for the analyzer + # ======================================= + # The list of goals to optimize the Kafka cluster for with pre-computed proposals + default.goals=com.linkedin.kafka.cruisecontrol.analyzer.goals.ReplicaCapacityGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.DiskCapacityGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkInboundCapacityGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkOutboundCapacityGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.CpuCapacityGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.ReplicaDistributionGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.PotentialNwOutGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.DiskUsageDistributionGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkInboundUsageDistributionGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkOutboundUsageDistributionGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.CpuUsageDistributionGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.TopicReplicaDistributionGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.LeaderBytesInDistributionGoal + # The list of supported goals + goals=com.linkedin.kafka.cruisecontrol.analyzer.goals.ReplicaCapacityGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.DiskCapacityGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkInboundCapacityGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkOutboundCapacityGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.CpuCapacityGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.ReplicaDistributionGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.PotentialNwOutGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.DiskUsageDistributionGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkInboundUsageDistributionGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkOutboundUsageDistributionGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.CpuUsageDistributionGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.TopicReplicaDistributionGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.LeaderBytesInDistributionGoal,com.linkedin.kafka.cruisecontrol.analyzer.kafkaassigner.KafkaAssignerDiskUsageDistributionGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.PreferredLeaderElectionGoal + # The list of supported hard goals + hard.goals=com.linkedin.kafka.cruisecontrol.analyzer.goals.ReplicaCapacityGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.DiskCapacityGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkInboundCapacityGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkOutboundCapacityGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.CpuCapacityGoal + # The minimum percentage of well monitored partitions out of all the partitions + min.monitored.partition.percentage=0.95 + # The balance threshold for CPU + cpu.balance.threshold=1.1 + # The balance threshold for disk + disk.balance.threshold=1.1 + # The balance threshold for network inbound utilization + network.inbound.balance.threshold=1.1 + # The balance threshold for network outbound utilization + network.outbound.balance.threshold=1.1 + # The balance threshold for the replica count + replica.count.balance.threshold=1.1 + # The capacity threshold for CPU in percentage + cpu.capacity.threshold=0.8 + # The capacity threshold for disk in percentage + disk.capacity.threshold=0.8 + # The capacity threshold for network inbound utilization in percentage + network.inbound.capacity.threshold=0.8 + # The capacity threshold for network outbound utilization in percentage + network.outbound.capacity.threshold=0.8 + # The threshold to define the cluster to be in a low CPU utilization state + cpu.low.utilization.threshold=0.0 + # The threshold to define the cluster to be in a low disk utilization state + disk.low.utilization.threshold=0.0 + # The threshold to define the cluster to be in a low network inbound utilization state + network.inbound.low.utilization.threshold=0.0 + # The threshold to define the cluster to be in a low disk utilization state + network.outbound.low.utilization.threshold=0.0 + # The metric anomaly percentile upper threshold + metric.anomaly.percentile.upper.threshold=90.0 + # The metric anomaly percentile lower threshold + metric.anomaly.percentile.lower.threshold=10.0 + # How often should the cached proposal be expired and recalculated if necessary + proposal.expiration.ms=60000 + # The maximum number of replicas that can reside on a broker at any given time. + max.replicas.per.broker=10000 + # The number of threads to use for proposal candidate precomputing. + num.proposal.precompute.threads=1 + # the topics that should be excluded from the partition movement. + #topics.excluded.from.partition.movement + # Configurations for the executor + # ======================================= + # The max number of partitions to move in/out on a given broker at a given time. + num.concurrent.partition.movements.per.broker=10 + # The interval between two execution progress checks. + execution.progress.check.interval.ms=10000 + # Configurations for anomaly detector + # ======================================= + # The goal violation notifier class + anomaly.notifier.class=com.linkedin.kafka.cruisecontrol.detector.notifier.SelfHealingNotifier + # The metric anomaly finder class + metric.anomaly.finder.class=com.linkedin.kafka.cruisecontrol.detector.KafkaMetricAnomalyFinder + # The anomaly detection interval + anomaly.detection.interval.ms=10000 + # The goal violation to detect. + anomaly.detection.goals=com.linkedin.kafka.cruisecontrol.analyzer.goals.ReplicaCapacityGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.DiskCapacityGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkInboundCapacityGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkOutboundCapacityGoal,com.linkedin.kafka.cruisecontrol.analyzer.goals.CpuCapacityGoal + # The interested metrics for metric anomaly analyzer. + metric.anomaly.analyzer.metrics=BROKER_PRODUCE_LOCAL_TIME_MS_MAX,BROKER_PRODUCE_LOCAL_TIME_MS_MEAN,BROKER_CONSUMER_FETCH_LOCAL_TIME_MS_MAX,BROKER_CONSUMER_FETCH_LOCAL_TIME_MS_MEAN,BROKER_FOLLOWER_FETCH_LOCAL_TIME_MS_MAX,BROKER_FOLLOWER_FETCH_LOCAL_TIME_MS_MEAN,BROKER_LOG_FLUSH_TIME_MS_MAX,BROKER_LOG_FLUSH_TIME_MS_MEAN + ## Adjust accordingly if your metrics reporter is an older version and does not produce these metrics. + #metric.anomaly.analyzer.metrics=BROKER_PRODUCE_LOCAL_TIME_MS_50TH,BROKER_PRODUCE_LOCAL_TIME_MS_999TH,BROKER_CONSUMER_FETCH_LOCAL_TIME_MS_50TH,BROKER_CONSUMER_FETCH_LOCAL_TIME_MS_999TH,BROKER_FOLLOWER_FETCH_LOCAL_TIME_MS_50TH,BROKER_FOLLOWER_FETCH_LOCAL_TIME_MS_999TH,BROKER_LOG_FLUSH_TIME_MS_50TH,BROKER_LOG_FLUSH_TIME_MS_999TH + # The cluster configurations for the KafkaTopicConfigProvider + cluster.configs.file=config/clusterConfigs.json + # The maximum time in milliseconds to store the response and access details of a completed user task. + completed.user.task.retention.time.ms=21600000 + # The maximum time in milliseconds to retain the demotion history of brokers. + demotion.history.retention.time.ms=86400000 + # The maximum number of completed user tasks for which the response and access details will be cached. + max.cached.completed.user.tasks=500 + # The maximum number of user tasks for concurrently running in async endpoints across all users. + max.active.user.tasks=25 + # Enable self healing for all anomaly detectors, unless the particular anomaly detector is explicitly disabled + self.healing.enabled=true + # Enable self healing for broker failure detector + #self.healing.broker.failure.enabled=true + # Enable self healing for goal violation detector + #self.healing.goal.violation.enabled=true + # Enable self healing for metric anomaly detector + #self.healing.metric.anomaly.enabled=true + # configurations for the webserver + # ================================ + # HTTP listen port + webserver.http.port=9090 + # HTTP listen address + webserver.http.address=0.0.0.0 + # Whether CORS support is enabled for API or not + webserver.http.cors.enabled=false + # Value for Access-Control-Allow-Origin + webserver.http.cors.origin=http://localhost:8080/ + # Value for Access-Control-Request-Method + webserver.http.cors.allowmethods=OPTIONS,GET,POST + # Headers that should be exposed to the Browser (Webapp) + # This is a special header that is used by the + # User Tasks subsystem and should be explicitly + # Enabled when CORS mode is used as part of the + # Admin Interface + webserver.http.cors.exposeheaders=User-Task-ID + # REST API default prefix + # (dont forget the ending *) + webserver.api.urlprefix=/kafkacruisecontrol/* + # Location where the Cruise Control frontend is deployed + webserver.ui.diskpath=./cruise-control-ui/dist/ + # URL path prefix for UI + # (dont forget the ending *) + webserver.ui.urlprefix=/* + # Time After which request is converted to Async + webserver.request.maxBlockTimeMs=10000 + # Default Session Expiry Period + webserver.session.maxExpiryTimeMs=60000 + # Session cookie path + webserver.session.path=/ + # Server Access Logs + webserver.accesslog.enabled=true + # Location of HTTP Request Logs + webserver.accesslog.path=access.log + # HTTP Request Log retention days + webserver.accesslog.retention.days=14 + clusterConfig: | + { + "min.insync.replicas": 3 + } diff --git a/config/samples/simplekafkacluster-with-brokerbindings.yaml b/config/samples/simplekafkacluster-with-brokerbindings.yaml index e02417f5c..907af8bcb 100644 --- a/config/samples/simplekafkacluster-with-brokerbindings.yaml +++ b/config/samples/simplekafkacluster-with-brokerbindings.yaml @@ -5,6 +5,7 @@ metadata: controller-tools.k8s.io: "1.0" name: kafka spec: + kRaft: false headlessServiceEnabled: true zkAddresses: - "zookeeper-server-client.zookeeper:2181" diff --git a/config/samples/simplekafkacluster-with-nodeport-external.yaml b/config/samples/simplekafkacluster-with-nodeport-external.yaml index de151c096..57b692395 100644 --- a/config/samples/simplekafkacluster-with-nodeport-external.yaml +++ b/config/samples/simplekafkacluster-with-nodeport-external.yaml @@ -5,6 +5,7 @@ metadata: controller-tools.k8s.io: "1.0" name: kafka spec: + kRaft: false headlessServiceEnabled: false zkAddresses: - "zookeeper-server-client.zookeeper:2181" diff --git a/config/samples/simplekafkacluster.yaml b/config/samples/simplekafkacluster.yaml index 63d057216..cd2bc510e 100644 --- a/config/samples/simplekafkacluster.yaml +++ b/config/samples/simplekafkacluster.yaml @@ -5,6 +5,7 @@ metadata: controller-tools.k8s.io: "1.0" name: kafka spec: + kRaft: false monitoringConfig: jmxImage: "ghcr.io/banzaicloud/jmx-javaagent:0.16.1" headlessServiceEnabled: true diff --git a/config/samples/simplekafkacluster_affinity.yaml b/config/samples/simplekafkacluster_affinity.yaml index 3e2fc6368..2de14aa01 100644 --- a/config/samples/simplekafkacluster_affinity.yaml +++ b/config/samples/simplekafkacluster_affinity.yaml @@ -5,6 +5,7 @@ metadata: controller-tools.k8s.io: "1.0" name: kafka spec: + kRaft: false headlessServiceEnabled: true zkAddresses: - "zookeeper-server-client.zookeeper:2181" diff --git a/config/samples/simplekafkacluster_ebs_csi.yaml b/config/samples/simplekafkacluster_ebs_csi.yaml index d2550845e..c8f2ff228 100644 --- a/config/samples/simplekafkacluster_ebs_csi.yaml +++ b/config/samples/simplekafkacluster_ebs_csi.yaml @@ -5,6 +5,7 @@ metadata: controller-tools.k8s.io: "1.0" name: kafka spec: + kRaft: false headlessServiceEnabled: false rackAwareness: labels: diff --git a/config/samples/simplekafkacluster_ssl.yaml b/config/samples/simplekafkacluster_ssl.yaml index 625b697ec..59741fac4 100644 --- a/config/samples/simplekafkacluster_ssl.yaml +++ b/config/samples/simplekafkacluster_ssl.yaml @@ -5,6 +5,7 @@ metadata: controller-tools.k8s.io: "1.0" name: kafka spec: + kRaft: false headlessServiceEnabled: true zkAddresses: - "zookeeper-server-client.zookeeper:2181" diff --git a/config/samples/simplekafkacluster_with_k8s_provided_nodeport.yaml b/config/samples/simplekafkacluster_with_k8s_provided_nodeport.yaml index 7f796c360..278f29d87 100644 --- a/config/samples/simplekafkacluster_with_k8s_provided_nodeport.yaml +++ b/config/samples/simplekafkacluster_with_k8s_provided_nodeport.yaml @@ -5,6 +5,7 @@ metadata: controller-tools.k8s.io: "1.0" name: kafka spec: + kRaft: false headlessServiceEnabled: true zkAddresses: - "zookeeper-server-client.zookeeper:2181" diff --git a/config/samples/simplekafkacluster_with_sasl.yaml b/config/samples/simplekafkacluster_with_sasl.yaml index b944f1c46..09208444d 100644 --- a/config/samples/simplekafkacluster_with_sasl.yaml +++ b/config/samples/simplekafkacluster_with_sasl.yaml @@ -5,6 +5,7 @@ metadata: controller-tools.k8s.io: "1.0" name: kafka spec: + kRaft: false headlessServiceEnabled: true zkAddresses: - "zookeeper-server-client.zookeeper:2181" diff --git a/docs/benchmarks/infrastructure/kafka.yaml b/docs/benchmarks/infrastructure/kafka.yaml index 80da2139e..23a436428 100644 --- a/docs/benchmarks/infrastructure/kafka.yaml +++ b/docs/benchmarks/infrastructure/kafka.yaml @@ -5,6 +5,7 @@ metadata: controller-tools.k8s.io: "1.0" name: kafka spec: + kRaft: false headlessServiceEnabled: false zkAddresses: - "zookeeper-server-client.zookeeper:2181" diff --git a/docs/benchmarks/infrastructure/kafka_202001_3brokers.yaml b/docs/benchmarks/infrastructure/kafka_202001_3brokers.yaml index 24118802a..dc165ee7c 100644 --- a/docs/benchmarks/infrastructure/kafka_202001_3brokers.yaml +++ b/docs/benchmarks/infrastructure/kafka_202001_3brokers.yaml @@ -5,6 +5,7 @@ metadata: controller-tools.k8s.io: "1.0" name: kafka spec: + kRaft: false monitoringConfig: jmxImage: "banzaicloud/jmx-javaagent:0.13.0" pathToJar: "/opt/jmx_exporter/jmx_prometheus_javaagent-0.13.0.jar" diff --git a/pkg/resources/kafka/kafka.go b/pkg/resources/kafka/kafka.go index be4f63863..a255a0b12 100644 --- a/pkg/resources/kafka/kafka.go +++ b/pkg/resources/kafka/kafka.go @@ -310,7 +310,7 @@ func (r *Reconciler) Reconcile(log logr.Logger) error { allBrokerDynamicConfigSucceeded := true // all broker nodes under the same Kafka cluster must use the same cluster UUID - if r.KafkaCluster.Status.ClusterID == "" { + if r.KafkaCluster.Spec.KRaftMode && r.KafkaCluster.Status.ClusterID == "" { r.KafkaCluster.Status.ClusterID = generateRandomClusterID() err = r.Client.Status().Update(ctx, r.KafkaCluster) if apierrors.IsNotFound(err) { From f9aedacd2638585826e1444a2d62c6ac74a98e01 Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Tue, 25 Jul 2023 12:34:15 -0400 Subject: [PATCH 15/22] Rebase from origin/koperator-api --- api/v1beta1/kafkacluster_types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/v1beta1/kafkacluster_types.go b/api/v1beta1/kafkacluster_types.go index a2054072d..2ea78f370 100644 --- a/api/v1beta1/kafkacluster_types.go +++ b/api/v1beta1/kafkacluster_types.go @@ -560,7 +560,7 @@ type IngressServiceSettings struct { ServiceAnnotations map[string]string `json:"serviceAnnotations,omitempty"` // externalTrafficPolicy denotes if this Service desires to route external // traffic to node-local or cluster-wide endpoints. "Local" preserves the - // client source IP and avoids a second hop for LoadBalancer and Nodeport + // client source IP and avoids a second hop for LoadBalancer anapi/v1beta1/kafkacluster_types_test.god Nodeport // type services, but risks potentially imbalanced traffic spreading. // "Cluster" obscures the client source IP and may cause a second hop to // another node, but should have good overall load-spreading. From 58048358a78331d2c6305ab0b0e38b303f0a8f8c Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Tue, 25 Jul 2023 12:35:28 -0400 Subject: [PATCH 16/22] Use util functions that got moved to the koperator/api module --- .../kraft/simplekafkacluster_kraft.yaml | 2 +- controllers/kafkacluster_controller.go | 10 ++++++---- controllers/kafkatopic_controller.go | 6 ++++-- controllers/kafkauser_controller.go | 6 ++++-- pkg/resources/kafka/configmap.go | 6 +++--- pkg/resources/kafka/util.go | 19 +----------------- pkg/util/kafka/common.go | 9 +++++---- pkg/util/util.go | 20 ++++++------------- pkg/util/util_test.go | 10 ---------- 9 files changed, 30 insertions(+), 58 deletions(-) diff --git a/config/samples/kraft/simplekafkacluster_kraft.yaml b/config/samples/kraft/simplekafkacluster_kraft.yaml index 43b81c378..a93b65ad7 100644 --- a/config/samples/kraft/simplekafkacluster_kraft.yaml +++ b/config/samples/kraft/simplekafkacluster_kraft.yaml @@ -93,7 +93,7 @@ spec: # limits: # cpu: 500m # memory: 1Gi -# image: "ghcr.io/banzaicloud/cruise-control:2.5.86" +# image: "ghcr.io/banzaicloud/cruise-control:2.5.123" config: | # Copyright 2017 LinkedIn Corp. Licensed under the BSD 2-Clause License (the "License"). See License in the project root for license information. # diff --git a/controllers/kafkacluster_controller.go b/controllers/kafkacluster_controller.go index 4043145d8..377d56fb6 100644 --- a/controllers/kafkacluster_controller.go +++ b/controllers/kafkacluster_controller.go @@ -32,6 +32,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/predicate" + apiutil "github.com/banzaicloud/koperator/api/util" + "github.com/banzaicloud/k8s-objectmatcher/patch" "github.com/banzaicloud/koperator/api/v1alpha1" @@ -200,7 +202,7 @@ func (r *KafkaClusterReconciler) checkFinalizers(ctx context.Context, cluster *v log.Info("KafkaCluster is marked for deletion, checking for children") // If the main finalizer is gone then we've already finished up - if !util.StringSliceContains(cluster.GetFinalizers(), clusterFinalizer) { + if !apiutil.StringSliceContains(cluster.GetFinalizers(), clusterFinalizer) { return reconciled() } @@ -224,7 +226,7 @@ func (r *KafkaClusterReconciler) checkFinalizers(ctx context.Context, cluster *v // If we haven't deleted all kafkatopics yet, iterate namespaces and delete all kafkatopics // with the matching label. - if util.StringSliceContains(cluster.GetFinalizers(), clusterTopicsFinalizer) { + if apiutil.StringSliceContains(cluster.GetFinalizers(), clusterTopicsFinalizer) { log.Info(fmt.Sprintf("Sending delete kafkatopics request to all namespaces for cluster %s/%s", cluster.Namespace, cluster.Name)) for _, ns := range namespaces { if err := r.Client.DeleteAllOf( @@ -268,7 +270,7 @@ func (r *KafkaClusterReconciler) checkFinalizers(ctx context.Context, cluster *v // If we haven't deleted all kafkausers yet, iterate namespaces and delete all kafkausers // with the matching label. - if util.StringSliceContains(cluster.GetFinalizers(), clusterUsersFinalizer) { + if apiutil.StringSliceContains(cluster.GetFinalizers(), clusterUsersFinalizer) { log.Info(fmt.Sprintf("Sending delete kafkausers request to all namespaces for cluster %s/%s", cluster.Namespace, cluster.Name)) for _, ns := range namespaces { if err := r.Client.DeleteAllOf( @@ -329,7 +331,7 @@ func topicListToStrSlice(list v1alpha1.KafkaTopicList) []string { func (r *KafkaClusterReconciler) ensureFinalizers(ctx context.Context, cluster *v1beta1.KafkaCluster) (updated *v1beta1.KafkaCluster, err error) { finalizers := []string{clusterFinalizer, clusterTopicsFinalizer, clusterUsersFinalizer} for _, finalizer := range finalizers { - if util.StringSliceContains(cluster.GetFinalizers(), finalizer) { + if apiutil.StringSliceContains(cluster.GetFinalizers(), finalizer) { continue } cluster.SetFinalizers(append(cluster.GetFinalizers(), finalizer)) diff --git a/controllers/kafkatopic_controller.go b/controllers/kafkatopic_controller.go index 496c36fbe..3878cd824 100644 --- a/controllers/kafkatopic_controller.go +++ b/controllers/kafkatopic_controller.go @@ -30,6 +30,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/reconcile" + apiutil "github.com/banzaicloud/koperator/api/util" + "github.com/banzaicloud/koperator/api/v1alpha1" "github.com/banzaicloud/koperator/api/v1beta1" "github.com/banzaicloud/koperator/pkg/k8sutil" @@ -180,7 +182,7 @@ func (r *KafkaTopicReconciler) Reconcile(ctx context.Context, request reconcile. } // ensure a finalizer for cleanup on deletion - if !util.StringSliceContains(instance.GetFinalizers(), topicFinalizer) { + if !apiutil.StringSliceContains(instance.GetFinalizers(), topicFinalizer) { reqLogger.Info("Adding Finalizer for the KafkaTopic") instance.SetFinalizers(append(instance.GetFinalizers(), topicFinalizer)) if instance, err = r.updateAndFetchLatest(ctx, instance); err != nil { @@ -224,7 +226,7 @@ func (r *KafkaTopicReconciler) checkFinalizers(ctx context.Context, broker kafka reqLogger := logr.FromContextOrDiscard(ctx) reqLogger.Info("Kafka topic is marked for deletion") var err error - if util.StringSliceContains(topic.GetFinalizers(), topicFinalizer) { + if apiutil.StringSliceContains(topic.GetFinalizers(), topicFinalizer) { // Remove topic from Kafka cluster when it is managed by Koperator if isTopicManagedByKoperator(topic) { if err = r.finalizeKafkaTopic(reqLogger, broker, topic); err != nil { diff --git a/controllers/kafkauser_controller.go b/controllers/kafkauser_controller.go index 35fc5b460..fe861240e 100644 --- a/controllers/kafkauser_controller.go +++ b/controllers/kafkauser_controller.go @@ -23,6 +23,8 @@ import ( "emperror.dev/errors" + apiutil "github.com/banzaicloud/koperator/api/util" + "github.com/banzaicloud/k8s-objectmatcher/patch" certv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" @@ -294,7 +296,7 @@ func (r *KafkaUserReconciler) Reconcile(ctx context.Context, request reconcile.R } // ensure a finalizer for cleanup on deletion - if !util.StringSliceContains(instance.GetFinalizers(), userFinalizer) { + if !apiutil.StringSliceContains(instance.GetFinalizers(), userFinalizer) { r.addFinalizer(reqLogger, instance) if instance, err = r.updateAndFetchLatest(ctx, instance); err != nil { return requeueWithError(reqLogger, "failed to update kafkauser with finalizer", err) @@ -338,7 +340,7 @@ func (r *KafkaUserReconciler) checkFinalizers(ctx context.Context, cluster *v1be reqLogger := logr.FromContextOrDiscard(ctx) // run finalizers var err error - if util.StringSliceContains(instance.GetFinalizers(), userFinalizer) { + if apiutil.StringSliceContains(instance.GetFinalizers(), userFinalizer) { if len(instance.Spec.TopicGrants) > 0 { for _, topicGrant := range instance.Spec.TopicGrants { if err = r.finalizeKafkaUserACLs(reqLogger, cluster, user, topicGrant.PatternType); err != nil { diff --git a/pkg/resources/kafka/configmap.go b/pkg/resources/kafka/configmap.go index 8d3169a64..8827eb5e4 100644 --- a/pkg/resources/kafka/configmap.go +++ b/pkg/resources/kafka/configmap.go @@ -160,7 +160,7 @@ func configureBrokerKRaftMode(broker v1beta1.Broker, kafkaCluster *v1beta1.Kafka var advertisedListenerConf []string // only expose "advertised.listeners" when the node serves as a regular broker or a combined node - if isBrokerNode(broker.Roles) { + if broker.IsBrokerNode() { advertisedListenerConf = generateAdvertisedListenerConfig(broker.Id, kafkaCluster.Spec.ListenersConfig, extListenerStatuses, intListenerStatuses, nil) if err := config.Set(kafkautils.KafkaConfigAdvertisedListeners, advertisedListenerConf); err != nil { @@ -168,7 +168,7 @@ func configureBrokerKRaftMode(broker v1beta1.Broker, kafkaCluster *v1beta1.Kafka } } - if isControllerNodeOnly(broker.Roles) { + if broker.IsControllerOnlyNode() { // "listeners" configuration can only contain controller configuration when the node is a controller-only node for _, listener := range listenerConfig { if listener[:len(controllerListenerName)] == strings.ToUpper(controllerListenerName) { @@ -178,7 +178,7 @@ func configureBrokerKRaftMode(broker v1beta1.Broker, kafkaCluster *v1beta1.Kafka break } } - } else if isBrokerNodeOnly(broker.Roles) { + } else if broker.IsBrokerOnlyNode() { // "listeners" configuration cannot contain controller configuration when the node is a broker-only node var nonControllerListener []string for _, listener := range listenerConfig { diff --git a/pkg/resources/kafka/util.go b/pkg/resources/kafka/util.go index 3bb6088e2..b8f697b38 100644 --- a/pkg/resources/kafka/util.go +++ b/pkg/resources/kafka/util.go @@ -22,7 +22,6 @@ import ( "github.com/google/uuid" "github.com/banzaicloud/koperator/api/v1beta1" - "github.com/banzaicloud/koperator/pkg/util" ) // generateQuorumVoters generates the quorum voters in the format of brokerID@nodeAddress:listenerPort @@ -38,7 +37,7 @@ func generateQuorumVoters(brokers []v1beta1.Broker, controllerListenerStatuses m // find the controller nodes and their corresponding listener addresses for _, b := range brokers { - if isControllerNode(b.Roles) { + if b.IsControllerNode() { for _, controllerListenerStatus := range controllerListenerStatuses { for _, status := range controllerListenerStatus { if status.Name == fmt.Sprintf("broker-%d", b.Id) { @@ -62,22 +61,6 @@ func generateQuorumVoters(brokers []v1beta1.Broker, controllerListenerStatuses m return quorumVoters } -func isBrokerNodeOnly(roles []string) bool { - return isBrokerNode(roles) && !isControllerNode(roles) -} - -func isControllerNodeOnly(roles []string) bool { - return isControllerNode(roles) && !isBrokerNode(roles) -} - -func isBrokerNode(roles []string) bool { - return util.StringSliceContains(roles, "broker") -} - -func isControllerNode(roles []string) bool { - return util.StringSliceContains(roles, "controller") -} - // generateRandomClusterID() generates a based64-encoded random UUID with 16 bytes as the cluster ID // it uses URL based64 encoding since that's what Kafka expects func generateRandomClusterID() string { diff --git a/pkg/util/kafka/common.go b/pkg/util/kafka/common.go index 52ff1a97a..647647f6d 100644 --- a/pkg/util/kafka/common.go +++ b/pkg/util/kafka/common.go @@ -21,11 +21,12 @@ import ( "emperror.dev/errors" "github.com/go-logr/logr" + apiutil "github.com/banzaicloud/koperator/api/util" + "github.com/banzaicloud/koperator/api/v1beta1" properties "github.com/banzaicloud/koperator/properties/pkg" "github.com/banzaicloud/koperator/api/v1alpha1" - "github.com/banzaicloud/koperator/pkg/util" ) // PerBrokerConfigs configurations will not trigger rolling upgrade when updated @@ -64,7 +65,7 @@ func GrantsToACLStrings(dn string, grants []v1alpha1.UserTopicGrant) []string { } patternType := strings.ToUpper(string(x.PatternType)) cmn := fmt.Sprintf(commonACLString, dn, patternType, x.TopicName) - if !util.StringSliceContains(acls, cmn) { + if !apiutil.StringSliceContains(acls, cmn) { acls = append(acls, cmn) } switch x.AccessType { @@ -72,7 +73,7 @@ func GrantsToACLStrings(dn string, grants []v1alpha1.UserTopicGrant) []string { readACL := fmt.Sprintf(readACLString, dn, patternType, x.TopicName) readGroupACL := fmt.Sprintf(readGroupACLString, dn) for _, y := range []string{readACL, readGroupACL} { - if !util.StringSliceContains(acls, y) { + if !apiutil.StringSliceContains(acls, y) { acls = append(acls, y) } } @@ -80,7 +81,7 @@ func GrantsToACLStrings(dn string, grants []v1alpha1.UserTopicGrant) []string { createACL := fmt.Sprintf(createACLString, dn, patternType, x.TopicName) writeACL := fmt.Sprintf(writeACLString, dn, patternType, x.TopicName) for _, y := range []string{createACL, writeACL} { - if !util.StringSliceContains(acls, y) { + if !apiutil.StringSliceContains(acls, y) { acls = append(acls, y) } } diff --git a/pkg/util/util.go b/pkg/util/util.go index 97a60b1c0..fad600a20 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -48,6 +48,8 @@ import ( clientCtrl "sigs.k8s.io/controller-runtime/pkg/client" k8s_zap "sigs.k8s.io/controller-runtime/pkg/log/zap" + apiutil "github.com/banzaicloud/koperator/api/util" + "github.com/banzaicloud/koperator/api/v1alpha1" "github.com/banzaicloud/koperator/api/v1beta1" "github.com/banzaicloud/koperator/pkg/errorfactory" @@ -151,16 +153,6 @@ func ConvertPropertiesToMapStringPointer(pp *properties.Properties) map[string]* return result } -// StringSliceContains returns true if list contains s -func StringSliceContains(list []string, s string) bool { - for _, v := range list { - if v == s { - return true - } - } - return false -} - // StringSliceRemove will remove s from list func StringSliceRemove(list []string, s string) []string { for i, v := range list { @@ -219,12 +211,12 @@ func IsIngressConfigInUse(iConfigName, defaultConfigName string, cluster *v1beta return false } if len(brokerConfig.BrokerIngressMapping) == 0 && iConfigName == defaultConfigName || - StringSliceContains(brokerConfig.BrokerIngressMapping, iConfigName) { + apiutil.StringSliceContains(brokerConfig.BrokerIngressMapping, iConfigName) { return true } } for _, status := range cluster.Status.BrokersState { - if StringSliceContains(status.ExternalListenerConfigNames, iConfigName) { + if apiutil.StringSliceContains(status.ExternalListenerConfigNames, iConfigName) { return true } } @@ -245,12 +237,12 @@ func ShouldIncludeBroker(brokerConfig *v1beta1.BrokerConfig, status v1beta1.Kafk defaultIngressConfigName, ingressConfigName string) bool { if brokerConfig != nil { if len(brokerConfig.BrokerIngressMapping) == 0 && (ingressConfigName == defaultIngressConfigName || defaultIngressConfigName == "") || - StringSliceContains(brokerConfig.BrokerIngressMapping, ingressConfigName) { + apiutil.StringSliceContains(brokerConfig.BrokerIngressMapping, ingressConfigName) { return true } } if brokerState, ok := status.BrokersState[strconv.Itoa(brokerID)]; ok { - if StringSliceContains(brokerState.ExternalListenerConfigNames, ingressConfigName) { + if apiutil.StringSliceContains(brokerState.ExternalListenerConfigNames, ingressConfigName) { return true } } diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index 8add02c4a..c091cca4a 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -120,16 +120,6 @@ func TestIsSSLEnabledForInternalCommunication(t *testing.T) { } } -func TestStringSliceContains(t *testing.T) { - slice := []string{"1", "2", "3"} - if !StringSliceContains(slice, "1") { - t.Error("Expected slice contains 1, got false") - } - if StringSliceContains(slice, "4") { - t.Error("Expected slice not contains 4, got true") - } -} - func TestStringSliceRemove(t *testing.T) { slice := []string{"1", "2", "3"} removed := StringSliceRemove(slice, "3") From ca1642227f1aa8fbee0a50a50f8b7d3efa6aa1a3 Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Tue, 25 Jul 2023 13:59:37 -0400 Subject: [PATCH 17/22] Remove unineteded changes during rebase --- api/v1beta1/kafkacluster_types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/v1beta1/kafkacluster_types.go b/api/v1beta1/kafkacluster_types.go index 2ea78f370..a2054072d 100644 --- a/api/v1beta1/kafkacluster_types.go +++ b/api/v1beta1/kafkacluster_types.go @@ -560,7 +560,7 @@ type IngressServiceSettings struct { ServiceAnnotations map[string]string `json:"serviceAnnotations,omitempty"` // externalTrafficPolicy denotes if this Service desires to route external // traffic to node-local or cluster-wide endpoints. "Local" preserves the - // client source IP and avoids a second hop for LoadBalancer anapi/v1beta1/kafkacluster_types_test.god Nodeport + // client source IP and avoids a second hop for LoadBalancer and Nodeport // type services, but risks potentially imbalanced traffic spreading. // "Cluster" obscures the client source IP and may cause a second hop to // another node, but should have good overall load-spreading. From afd567b14e939e97a214260471066086a3c707c6 Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Tue, 25 Jul 2023 15:15:06 -0400 Subject: [PATCH 18/22] Do not take active controller identity into consideration when reorder the brokers --- pkg/resources/kafka/kafka.go | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/pkg/resources/kafka/kafka.go b/pkg/resources/kafka/kafka.go index a255a0b12..d5d9930df 100644 --- a/pkg/resources/kafka/kafka.go +++ b/pkg/resources/kafka/kafka.go @@ -287,11 +287,6 @@ func (r *Reconciler) Reconcile(log logr.Logger) error { runningBrokers[brokerID] = struct{}{} } - controllerID, err := r.determineControllerId() - if err != nil { - log.Error(err, "could not find controller broker") - } - var pvcList corev1.PersistentVolumeClaimList err = r.Client.List(ctx, &pvcList, client.ListOption(client.InNamespace(r.KafkaCluster.Namespace)), client.ListOption(matchingLabels)) if err != nil { @@ -306,9 +301,6 @@ func (r *Reconciler) Reconcile(log logr.Logger) error { } } - reorderedBrokers := reorderBrokers(runningBrokers, boundPersistentVolumeClaims, r.KafkaCluster.Spec.Brokers, r.KafkaCluster.Status.BrokersState, controllerID, log) - allBrokerDynamicConfigSucceeded := true - // all broker nodes under the same Kafka cluster must use the same cluster UUID if r.KafkaCluster.Spec.KRaftMode && r.KafkaCluster.Status.ClusterID == "" { r.KafkaCluster.Status.ClusterID = generateRandomClusterID() @@ -317,10 +309,29 @@ func (r *Reconciler) Reconcile(log logr.Logger) error { err = r.Client.Update(ctx, r.KafkaCluster) } if err != nil { - return errors.WrapIf(err, "could not update cluster UUID status") + return errors.NewWithDetails("could not update ClusterID status", + "component", componentName, + "clusterName", r.KafkaCluster.Name, + "clusterNamespace", r.KafkaCluster.Namespace) } } + controllerID, err := r.determineControllerId() + if err != nil { + log.Error(err, "could not find controller broker") + } + + // In KRaft mode: + // 1. there is no way for admin client to know which node is the active controller, controllerID obtained above is just a broker ID of a random active broker (this is intentional by Kafka) + // 2. the follower controllers replicate the data that is written to the active controller and serves as hot standbys if the active controller fails. + // Because the controllers now all track the latest state, controller fail-over will not require a lengthy reloading time to have all the state to transfer to the new controller + // Therefore, by setting the controllerID to be -1 to not take the controller identity into consideration when reordering the brokers + if r.KafkaCluster.Spec.KRaftMode { + controllerID = -1 + } + reorderedBrokers := reorderBrokers(runningBrokers, boundPersistentVolumeClaims, r.KafkaCluster.Spec.Brokers, r.KafkaCluster.Status.BrokersState, controllerID, log) + + allBrokerDynamicConfigSucceeded := true for _, broker := range reorderedBrokers { brokerConfig, err := broker.GetBrokerConfig(r.KafkaCluster.Spec) if err != nil { @@ -1287,6 +1298,8 @@ func (r *Reconciler) getK8sNodeIP(nodeName string, nodeAddressType string) (stri } // determineControllerId returns the ID of the controller broker of the current cluster +// In KRaft mode, controller accesses are isolated from admin client (see KIP-590 for mode details), +// therefore, the KRaft metadata caches intentionally choose a random broker node to report as the controller func (r *Reconciler) determineControllerId() (int32, error) { kClient, close, err := r.kafkaClientProvider.NewFromCluster(r.Client, r.KafkaCluster) if err != nil { From bbc0307b4c35d1c078a865ca4c37b8d7a982cf93 Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Thu, 27 Jul 2023 12:54:50 -0400 Subject: [PATCH 19/22] Update implementation to accomomdate the latest KafkaCluster API change --- .../kraft/simplekafkacluster_kraft.yaml | 35 ++-- .../tests/kafkacluster_controller_test.go | 25 ++- pkg/resources/kafka/configmap.go | 41 ++-- pkg/resources/kafka/configmap_test.go | 33 ++-- pkg/resources/kafka/kafka.go | 55 +++--- pkg/resources/kafka/util.go | 13 +- pkg/resources/kafka/util_test.go | 179 ++++++++++++------ 7 files changed, 236 insertions(+), 145 deletions(-) diff --git a/config/samples/kraft/simplekafkacluster_kraft.yaml b/config/samples/kraft/simplekafkacluster_kraft.yaml index a93b65ad7..c508b96ec 100644 --- a/config/samples/kraft/simplekafkacluster_kraft.yaml +++ b/config/samples/kraft/simplekafkacluster_kraft.yaml @@ -27,7 +27,11 @@ spec: resources: requests: storage: 10Gi - - mountPath: "/kafka-logs-2" + broker: + processRoles: + - broker + storageConfigs: + - mountPath: "/kafka-logs-broker" pvcSpec: accessModes: - ReadWriteOnce @@ -39,30 +43,27 @@ spec: prometheus.io/port: "9020" brokers: - id: 0 - brokerConfigGroup: "default" - processRoles: - - broker + brokerConfigGroup: "broker" - id: 1 - brokerConfigGroup: "default" - processRoles: - - broker + brokerConfigGroup: "broker" - id: 2 - brokerConfigGroup: "default" - processRoles: - - broker + brokerConfigGroup: "broker" - id: 3 brokerConfigGroup: "default" - processRoles: - - controller - - broker + brokerConfig: + processRoles: + - controller + - broker - id: 4 brokerConfigGroup: "default" - processRoles: - - controller + brokerConfig: + processRoles: + - controller - id: 5 brokerConfigGroup: "default" - processRoles: - - controller + brokerConfig: + processRoles: + - controller rollingUpgradeConfig: failureThreshold: 1 listenersConfig: diff --git a/controllers/tests/kafkacluster_controller_test.go b/controllers/tests/kafkacluster_controller_test.go index 363298855..c257ca02c 100644 --- a/controllers/tests/kafkacluster_controller_test.go +++ b/controllers/tests/kafkacluster_controller_test.go @@ -523,8 +523,8 @@ var _ = Describe("KafkaCluster with two config external listener", func() { kafkaCluster.Spec.Brokers[1].BrokerConfig = &v1beta1.BrokerConfig{BrokerIngressMapping: []string{"az2"}} /* same tests for KafkaCluster in KRaft mode */ - kafkaClusterKRaft.Spec.Brokers[0].BrokerConfig = &v1beta1.BrokerConfig{BrokerIngressMapping: []string{"az1"}} - kafkaClusterKRaft.Spec.Brokers[1].BrokerConfig = &v1beta1.BrokerConfig{BrokerIngressMapping: []string{"az2"}} + kafkaClusterKRaft.Spec.Brokers[0].BrokerConfig.BrokerIngressMapping = []string{"az1"} + kafkaClusterKRaft.Spec.Brokers[1].BrokerConfig.BrokerIngressMapping = []string{"az2"} }) It("should reconcile object properly", func(ctx SpecContext) { expectEnvoyWithConfigAz1(ctx, kafkaCluster) @@ -567,19 +567,24 @@ func expectCruiseControlMonitoring(ctx context.Context, kafkaCluster *v1beta1.Ka func createMinimalKRaftBrokers() []v1beta1.Broker { return []v1beta1.Broker{ { - Id: int32(0), - Roles: []string{"broker"}, + Id: int32(0), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker"}, + }, BrokerConfigGroup: defaultBrokerConfigGroup, }, { - Id: int32(1), - Roles: []string{"controller"}, + Id: int32(1), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"controller"}, + }, BrokerConfigGroup: defaultBrokerConfigGroup, }, { - Id: int32(2), - Roles: []string{"controller", "broker"}, - BrokerConfigGroup: defaultBrokerConfigGroup, - }, + Id: int32(2), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"controller", "broker"}, + }, + BrokerConfigGroup: defaultBrokerConfigGroup}, } } diff --git a/pkg/resources/kafka/configmap.go b/pkg/resources/kafka/configmap.go index 8827eb5e4..2418bdfd5 100644 --- a/pkg/resources/kafka/configmap.go +++ b/pkg/resources/kafka/configmap.go @@ -39,7 +39,7 @@ import ( properties "github.com/banzaicloud/koperator/properties/pkg" ) -func (r *Reconciler) getConfigProperties(bConfig *v1beta1.BrokerConfig, broker v1beta1.Broker, +func (r *Reconciler) getConfigProperties(bConfig *v1beta1.BrokerConfig, id int32, quorumVoters []string, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses map[string]v1beta1.ListenerStatusList, serverPasses map[string]string, clientPass string, superUsers []string, log logr.Logger) *properties.Properties { config := properties.NewProperties() @@ -54,13 +54,13 @@ func (r *Reconciler) getConfigProperties(bConfig *v1beta1.BrokerConfig, broker v // Kafka Broker configurations if r.KafkaCluster.Spec.KRaftMode { - configureBrokerKRaftMode(broker, r.KafkaCluster, config, serverPasses, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses, log) + configureBrokerKRaftMode(bConfig, id, r.KafkaCluster, config, quorumVoters, serverPasses, extListenerStatuses, intListenerStatuses, log) } else { - configureBrokerZKMode(broker.Id, r.KafkaCluster, config, serverPasses, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses, log) + configureBrokerZKMode(id, r.KafkaCluster, config, serverPasses, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses, log) } // This logic prevents the removal of the mountPath from the broker configmap - brokerConfigMapName := fmt.Sprintf(brokerConfigTemplate+"-%d", r.KafkaCluster.Name, broker.Id) + brokerConfigMapName := fmt.Sprintf(brokerConfigTemplate+"-%d", r.KafkaCluster.Name, id) var brokerConfigMapOld v1.ConfigMap err = r.Client.Get(context.Background(), client.ObjectKey{Name: brokerConfigMapName, Namespace: r.KafkaCluster.GetNamespace()}, &brokerConfigMapOld) if err != nil && !apierrors.IsNotFound(err) { @@ -69,7 +69,7 @@ func (r *Reconciler) getConfigProperties(bConfig *v1beta1.BrokerConfig, broker v mountPathsOld, err := getMountPathsFromBrokerConfigMap(&brokerConfigMapOld) if err != nil { - log.Error(err, "could not get mountPaths from broker configmap", v1beta1.BrokerIdLabelKey, broker.Id) + log.Error(err, "could not get mountPaths from broker configmap", v1beta1.BrokerIdLabelKey, id) } mountPathsNew := generateStorageConfig(bConfig.StorageConfigs) @@ -77,7 +77,7 @@ func (r *Reconciler) getConfigProperties(bConfig *v1beta1.BrokerConfig, broker v if isMountPathRemoved { log.Error(errors.New("removed storage is found in the KafkaCluster CR"), - "removing storage from broker is not supported", v1beta1.BrokerIdLabelKey, broker.Id, "mountPaths", + "removing storage from broker is not supported", v1beta1.BrokerIdLabelKey, id, "mountPaths", mountPathsOld, "mountPaths in kafkaCluster CR ", mountPathsNew) } @@ -134,16 +134,17 @@ func configCCMetricsReporter(kafkaCluster *v1beta1.KafkaCluster, config *propert } } -func configureBrokerKRaftMode(broker v1beta1.Broker, kafkaCluster *v1beta1.KafkaCluster, config *properties.Properties, serverPasses map[string]string, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses map[string]v1beta1.ListenerStatusList, log logr.Logger) { - if err := config.Set(kafkautils.KafkaConfigNodeID, broker.Id); err != nil { +func configureBrokerKRaftMode(bConfig *v1beta1.BrokerConfig, brokerID int32, kafkaCluster *v1beta1.KafkaCluster, config *properties.Properties, + quorumVoters []string, serverPasses map[string]string, extListenerStatuses, intListenerStatuses map[string]v1beta1.ListenerStatusList, log logr.Logger) { + if err := config.Set(kafkautils.KafkaConfigNodeID, brokerID); err != nil { log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.KafkaConfigNodeID)) } - if err := config.Set(kafkautils.KafkaConfigProcessRoles, broker.Roles); err != nil { + if err := config.Set(kafkautils.KafkaConfigProcessRoles, bConfig.Roles); err != nil { log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.KafkaConfigProcessRoles)) } - if err := config.Set(kafkautils.KafkaConfigControllerQuorumVoters, generateQuorumVoters(kafkaCluster.Spec.Brokers, controllerIntListenerStatuses)); err != nil { + if err := config.Set(kafkautils.KafkaConfigControllerQuorumVoters, quorumVoters); err != nil { log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.KafkaConfigControllerQuorumVoters)) } @@ -160,15 +161,15 @@ func configureBrokerKRaftMode(broker v1beta1.Broker, kafkaCluster *v1beta1.Kafka var advertisedListenerConf []string // only expose "advertised.listeners" when the node serves as a regular broker or a combined node - if broker.IsBrokerNode() { - advertisedListenerConf = generateAdvertisedListenerConfig(broker.Id, kafkaCluster.Spec.ListenersConfig, + if bConfig.IsBrokerNode() { + advertisedListenerConf = generateAdvertisedListenerConfig(brokerID, kafkaCluster.Spec.ListenersConfig, extListenerStatuses, intListenerStatuses, nil) if err := config.Set(kafkautils.KafkaConfigAdvertisedListeners, advertisedListenerConf); err != nil { log.Error(err, fmt.Sprintf(kafkautils.BrokerConfigErrorMsgTemplate, kafkautils.KafkaConfigAdvertisedListeners)) } } - if broker.IsControllerOnlyNode() { + if bConfig.IsControllerOnlyNode() { // "listeners" configuration can only contain controller configuration when the node is a controller-only node for _, listener := range listenerConfig { if listener[:len(controllerListenerName)] == strings.ToUpper(controllerListenerName) { @@ -178,7 +179,7 @@ func configureBrokerKRaftMode(broker v1beta1.Broker, kafkaCluster *v1beta1.Kafka break } } - } else if broker.IsBrokerOnlyNode() { + } else if bConfig.IsBrokerOnlyNode() { // "listeners" configuration cannot contain controller configuration when the node is a broker-only node var nonControllerListener []string for _, listener := range listenerConfig { @@ -260,8 +261,8 @@ func generateSuperUsers(users []string) (suStrings []string) { return } -func (r *Reconciler) configMap(broker v1beta1.Broker, brokerConfig *v1beta1.BrokerConfig, extListenerStatuses, - intListenerStatuses, controllerIntListenerStatuses map[string]v1beta1.ListenerStatusList, +func (r *Reconciler) configMap(broker v1beta1.Broker, brokerConfig *v1beta1.BrokerConfig, quorumVoters []string, + extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses map[string]v1beta1.ListenerStatusList, serverPasses map[string]string, clientPass string, superUsers []string, log logr.Logger) *corev1.ConfigMap { brokerConf := &corev1.ConfigMap{ ObjectMeta: templates.ObjectMeta( @@ -272,7 +273,7 @@ func (r *Reconciler) configMap(broker v1beta1.Broker, brokerConfig *v1beta1.Brok ), r.KafkaCluster, ), - Data: map[string]string{kafkautils.ConfigPropertyName: r.generateBrokerConfig(broker, brokerConfig, extListenerStatuses, + Data: map[string]string{kafkautils.ConfigPropertyName: r.generateBrokerConfig(broker, brokerConfig, quorumVoters, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses, serverPasses, clientPass, superUsers, log)}, } if brokerConfig.Log4jConfig != "" { @@ -482,13 +483,13 @@ func mergeSuperUsersPropertyValue(source *properties.Properties, target *propert return "" } -func (r Reconciler) generateBrokerConfig(broker v1beta1.Broker, brokerConfig *v1beta1.BrokerConfig, extListenerStatuses, - intListenerStatuses, controllerIntListenerStatuses map[string]v1beta1.ListenerStatusList, +func (r Reconciler) generateBrokerConfig(broker v1beta1.Broker, brokerConfig *v1beta1.BrokerConfig, quorumVoters []string, + extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses map[string]v1beta1.ListenerStatusList, serverPasses map[string]string, clientPass string, superUsers []string, log logr.Logger) string { finalBrokerConfig := getBrokerReadOnlyConfig(broker, r.KafkaCluster, log) // Get operator generated configuration - opGenConf := r.getConfigProperties(brokerConfig, broker, extListenerStatuses, intListenerStatuses, + opGenConf := r.getConfigProperties(brokerConfig, broker.Id, quorumVoters, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses, serverPasses, clientPass, superUsers, log) // Merge operator generated configuration to the final one diff --git a/pkg/resources/kafka/configmap_test.go b/pkg/resources/kafka/configmap_test.go index 325b21427..8054946b1 100644 --- a/pkg/resources/kafka/configmap_test.go +++ b/pkg/resources/kafka/configmap_test.go @@ -682,7 +682,7 @@ zookeeper.connect=example.zk:2181/`, superUsers = []string{"CN=kafka-headless.kafka.svc.cluster.local"} } - generatedConfig := r.generateBrokerConfig(r.KafkaCluster.Spec.Brokers[0], r.KafkaCluster.Spec.Brokers[0].BrokerConfig, map[string]v1beta1.ListenerStatusList{}, + generatedConfig := r.generateBrokerConfig(r.KafkaCluster.Spec.Brokers[0], r.KafkaCluster.Spec.Brokers[0].BrokerConfig, nil, map[string]v1beta1.ListenerStatusList{}, map[string]v1beta1.ListenerStatusList{}, controllerListenerStatus, serverPasses, clientPass, superUsers, logr.Discard()) generated, err := properties.NewFromString(generatedConfig) @@ -715,9 +715,9 @@ func TestGenerateBrokerConfigKRaftMode(t *testing.T) { testName: "a Kafka cluster with a mix of broker-only and controller-only nodes; broker-only nodes with multiple mount paths", brokers: []v1beta1.Broker{ { - Id: 0, - Roles: []string{"broker"}, + Id: 0, BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker"}, StorageConfigs: []v1beta1.StorageConfig{ { MountPath: "/test-kafka-logs", @@ -729,9 +729,9 @@ func TestGenerateBrokerConfigKRaftMode(t *testing.T) { }, }, { - Id: 500, - Roles: []string{"controller"}, + Id: 500, BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"controller"}, StorageConfigs: []v1beta1.StorageConfig{ { MountPath: "/test-kafka-logs", @@ -740,9 +740,9 @@ func TestGenerateBrokerConfigKRaftMode(t *testing.T) { }, }, { - Id: 200, - Roles: []string{"broker"}, + Id: 200, BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker"}, StorageConfigs: []v1beta1.StorageConfig{ { MountPath: "/test-kafka-logs", @@ -847,9 +847,9 @@ process.roles=broker testName: "a Kafka cluster with a mix of broker-only, controller-only, and combined roles; controller nodes with multiple mount paths", brokers: []v1beta1.Broker{ { - Id: 0, - Roles: []string{"broker"}, + Id: 0, BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker"}, StorageConfigs: []v1beta1.StorageConfig{ { MountPath: "/test-kafka-logs", @@ -858,9 +858,9 @@ process.roles=broker }, }, { - Id: 50, - Roles: []string{"controller"}, + Id: 50, BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"controller"}, StorageConfigs: []v1beta1.StorageConfig{ { MountPath: "/test-kafka-logs", @@ -872,9 +872,9 @@ process.roles=broker }, }, { - Id: 100, - Roles: []string{"broker", "controller"}, + Id: 100, BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker", "controller"}, StorageConfigs: []v1beta1.StorageConfig{ { MountPath: "/test-kafka-logs", @@ -1009,7 +1009,12 @@ process.roles=broker,controller } for i, b := range test.brokers { - generatedConfig := r.generateBrokerConfig(b, b.BrokerConfig, map[string]v1beta1.ListenerStatusList{}, + quorumVoters, err := generateQuorumVoters(r.KafkaCluster, test.controllerListenerStatus) + if err != nil { + t.Error(err) + } + + generatedConfig := r.generateBrokerConfig(b, b.BrokerConfig, quorumVoters, map[string]v1beta1.ListenerStatusList{}, test.internalListenerStatuses, test.controllerListenerStatus, nil, "", nil, logr.Discard()) require.Equal(t, test.expectedBrokerConfigs[i], generatedConfig) diff --git a/pkg/resources/kafka/kafka.go b/pkg/resources/kafka/kafka.go index d5d9930df..ff8d1bbd6 100644 --- a/pkg/resources/kafka/kafka.go +++ b/pkg/resources/kafka/kafka.go @@ -301,34 +301,45 @@ func (r *Reconciler) Reconcile(log logr.Logger) error { } } - // all broker nodes under the same Kafka cluster must use the same cluster UUID - if r.KafkaCluster.Spec.KRaftMode && r.KafkaCluster.Status.ClusterID == "" { - r.KafkaCluster.Status.ClusterID = generateRandomClusterID() - err = r.Client.Status().Update(ctx, r.KafkaCluster) - if apierrors.IsNotFound(err) { - err = r.Client.Update(ctx, r.KafkaCluster) - } - if err != nil { - return errors.NewWithDetails("could not update ClusterID status", - "component", componentName, - "clusterName", r.KafkaCluster.Name, - "clusterNamespace", r.KafkaCluster.Namespace) - } - } - controllerID, err := r.determineControllerId() if err != nil { log.Error(err, "could not find controller broker") } - // In KRaft mode: - // 1. there is no way for admin client to know which node is the active controller, controllerID obtained above is just a broker ID of a random active broker (this is intentional by Kafka) - // 2. the follower controllers replicate the data that is written to the active controller and serves as hot standbys if the active controller fails. - // Because the controllers now all track the latest state, controller fail-over will not require a lengthy reloading time to have all the state to transfer to the new controller - // Therefore, by setting the controllerID to be -1 to not take the controller identity into consideration when reordering the brokers + var quorumVoters []string if r.KafkaCluster.Spec.KRaftMode { + // all broker nodes under the same Kafka cluster must use the same cluster UUID + if r.KafkaCluster.Status.ClusterID == "" { + r.KafkaCluster.Status.ClusterID = generateRandomClusterID() + err = r.Client.Status().Update(ctx, r.KafkaCluster) + if apierrors.IsNotFound(err) { + err = r.Client.Update(ctx, r.KafkaCluster) + } + if err != nil { + return errors.NewWithDetails("could not update ClusterID status", + "component", componentName, + "clusterName", r.KafkaCluster.Name, + "clusterNamespace", r.KafkaCluster.Namespace) + } + } + + // In KRaft mode: + // 1. there is no way for admin client to know which node is the active controller, controllerID obtained above is just a broker ID of a random active broker (this is intentional by Kafka) + // 2. the follower controllers replicate the data that is written to the active controller and serves as hot standbys if the active controller fails. + // Because the controllers now all track the latest state, controller fail-over will not require a lengthy reloading time to have all the state to transfer to the new controller + // Therefore, by setting the controllerID to be -1 to not take the controller identity into consideration when reordering the brokers controllerID = -1 + + quorumVoters, err = generateQuorumVoters(r.KafkaCluster, controllerIntListenerStatuses) + if err != nil { + return errors.WrapIfWithDetails(err, + "failed to generate quorum voters configuration", + "component", componentName, + "clusterName", r.KafkaCluster.GetName(), + "clusterNamespace", r.KafkaCluster.GetNamespace()) + } } + reorderedBrokers := reorderBrokers(runningBrokers, boundPersistentVolumeClaims, r.KafkaCluster.Spec.Brokers, r.KafkaCluster.Status.BrokersState, controllerID, log) allBrokerDynamicConfigSucceeded := true @@ -340,14 +351,14 @@ func (r *Reconciler) Reconcile(log logr.Logger) error { var configMap *corev1.ConfigMap if r.KafkaCluster.Spec.RackAwareness == nil { - configMap = r.configMap(broker, brokerConfig, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses, serverPasses, clientPass, superUsers, log) + configMap = r.configMap(broker, brokerConfig, quorumVoters, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses, serverPasses, clientPass, superUsers, log) err := k8sutil.Reconcile(log, r.Client, configMap, r.KafkaCluster) if err != nil { return errors.WrapIfWithDetails(err, "failed to reconcile resource", "resource", configMap.GetObjectKind().GroupVersionKind()) } } else if brokerState, ok := r.KafkaCluster.Status.BrokersState[strconv.Itoa(int(broker.Id))]; ok { if brokerState.RackAwarenessState != "" { - configMap = r.configMap(broker, brokerConfig, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses, serverPasses, clientPass, superUsers, log) + configMap = r.configMap(broker, brokerConfig, quorumVoters, extListenerStatuses, intListenerStatuses, controllerIntListenerStatuses, serverPasses, clientPass, superUsers, log) err := k8sutil.Reconcile(log, r.Client, configMap, r.KafkaCluster) if err != nil { return errors.WrapIfWithDetails(err, "failed to reconcile resource", "resource", configMap.GetObjectKind().GroupVersionKind()) diff --git a/pkg/resources/kafka/util.go b/pkg/resources/kafka/util.go index b8f697b38..c02e43bb7 100644 --- a/pkg/resources/kafka/util.go +++ b/pkg/resources/kafka/util.go @@ -28,7 +28,7 @@ import ( // The generated quorum voters are guaranteed in ascending order by broker IDs to ensure same quorum voters configurations are returned // regardless of the order of brokers and controllerListenerStatuses are passed in - this is needed to avoid triggering // unnecessary rolling upgrade operations -func generateQuorumVoters(brokers []v1beta1.Broker, controllerListenerStatuses map[string]v1beta1.ListenerStatusList) []string { +func generateQuorumVoters(kafkaCluster *v1beta1.KafkaCluster, controllerListenerStatuses map[string]v1beta1.ListenerStatusList) ([]string, error) { var ( quorumVoters []string brokerIDs []int32 @@ -36,8 +36,13 @@ func generateQuorumVoters(brokers []v1beta1.Broker, controllerListenerStatuses m idToListenerAddrMap := make(map[int32]string) // find the controller nodes and their corresponding listener addresses - for _, b := range brokers { - if b.IsControllerNode() { + for _, b := range kafkaCluster.Spec.Brokers { + brokerConfig, err := b.GetBrokerConfig(kafkaCluster.Spec) + if err != nil { + return nil, err + } + + if brokerConfig.IsControllerNode() { for _, controllerListenerStatus := range controllerListenerStatuses { for _, status := range controllerListenerStatus { if status.Name == fmt.Sprintf("broker-%d", b.Id) { @@ -58,7 +63,7 @@ func generateQuorumVoters(brokers []v1beta1.Broker, controllerListenerStatuses m quorumVoters = append(quorumVoters, fmt.Sprintf("%d@%s", brokerId, idToListenerAddrMap[brokerId])) } - return quorumVoters + return quorumVoters, nil } // generateRandomClusterID() generates a based64-encoded random UUID with 16 bytes as the cluster ID diff --git a/pkg/resources/kafka/util_test.go b/pkg/resources/kafka/util_test.go index b29a6c780..3476e69ff 100644 --- a/pkg/resources/kafka/util_test.go +++ b/pkg/resources/kafka/util_test.go @@ -42,7 +42,10 @@ func TestGenerateClusterID(t *testing.T) { } } +//nolint:funlen func TestGenerateQuorumVoters(t *testing.T) { + kafkaCluster := &v1beta1.KafkaCluster{} + tests := []struct { testName string brokers []v1beta1.Broker @@ -53,32 +56,46 @@ func TestGenerateQuorumVoters(t *testing.T) { testName: "brokers with ascending order by IDs; controller listener statuses has the same order as brokers", brokers: []v1beta1.Broker{ { - Id: int32(0), - Roles: []string{"broker"}, + Id: int32(0), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker"}, + }, }, { - Id: int32(10), - Roles: []string{"broker"}, + Id: int32(10), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker"}, + }, }, { - Id: int32(20), - Roles: []string{"broker"}, + Id: int32(20), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker"}, + }, }, { - Id: int32(30), - Roles: []string{"broker"}, + Id: int32(30), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker"}, + }, }, { - Id: int32(40), - Roles: []string{"controller"}, + Id: int32(40), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"controller"}, + }, }, { - Id: int32(50), - Roles: []string{"controller"}, + Id: int32(50), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"controller"}, + }, }, { - Id: int32(60), - Roles: []string{"controller"}, + Id: int32(60), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"controller"}, + }, }, }, listenersStatuses: map[string]v1beta1.ListenerStatusList{ @@ -119,35 +136,49 @@ func TestGenerateQuorumVoters(t *testing.T) { "60@fakeKafka-60.fakeKafka-headless.default.svc.cluster.local:29093"}, }, { - testName: "brokers with decreasing order by IDs; controller listener statuses has the same order as brokers", + testName: "brokers with descending order by IDs; controller listener statuses has the same order as brokers", brokers: []v1beta1.Broker{ { - Id: int32(60), - Roles: []string{"broker"}, + Id: int32(60), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker"}, + }, }, { - Id: int32(50), - Roles: []string{"broker"}, + Id: int32(50), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker"}, + }, }, { - Id: int32(40), - Roles: []string{"broker"}, + Id: int32(40), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker"}, + }, }, { - Id: int32(30), - Roles: []string{"broker"}, + Id: int32(30), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker"}, + }, }, { - Id: int32(20), - Roles: []string{"controller"}, + Id: int32(20), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"controller"}, + }, }, { - Id: int32(10), - Roles: []string{"controller"}, + Id: int32(10), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"controller"}, + }, }, { - Id: int32(0), - Roles: []string{"controller"}, + Id: int32(0), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"controller"}, + }, }, }, listenersStatuses: map[string]v1beta1.ListenerStatusList{ @@ -191,32 +222,46 @@ func TestGenerateQuorumVoters(t *testing.T) { testName: "brokers with ascending order by IDs; controller listener statuses has the opposite order as brokers", brokers: []v1beta1.Broker{ { - Id: int32(0), - Roles: []string{"broker"}, + Id: int32(0), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker"}, + }, }, { - Id: int32(10), - Roles: []string{"broker"}, + Id: int32(10), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker"}, + }, }, { - Id: int32(20), - Roles: []string{"broker"}, + Id: int32(20), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker"}, + }, }, { - Id: int32(30), - Roles: []string{"broker"}, + Id: int32(30), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker"}, + }, }, { - Id: int32(40), - Roles: []string{"controller"}, + Id: int32(40), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"controller"}, + }, }, { - Id: int32(50), - Roles: []string{"controller"}, + Id: int32(50), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"controller"}, + }, }, { - Id: int32(60), - Roles: []string{"controller"}, + Id: int32(60), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"controller"}, + }, }, }, listenersStatuses: map[string]v1beta1.ListenerStatusList{ @@ -260,32 +305,46 @@ func TestGenerateQuorumVoters(t *testing.T) { testName: "brokers and controller listener statuses with random order", brokers: []v1beta1.Broker{ { - Id: int32(100), - Roles: []string{"broker", "controller"}, + Id: int32(100), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker", "controller"}, + }, }, { - Id: int32(50), - Roles: []string{"broker"}, + Id: int32(50), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker"}, + }, }, { - Id: int32(80), - Roles: []string{"controller"}, + Id: int32(80), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"controller"}, + }, }, { - Id: int32(30), - Roles: []string{"broker"}, + Id: int32(30), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker"}, + }, }, { - Id: int32(90), - Roles: []string{"controller"}, + Id: int32(90), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"controller"}, + }, }, { - Id: int32(40), - Roles: []string{"broker"}, + Id: int32(40), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"broker"}, + }, }, { - Id: int32(60), - Roles: []string{"controller"}, + Id: int32(60), + BrokerConfig: &v1beta1.BrokerConfig{ + Roles: []string{"controller"}, + }, }, }, listenersStatuses: map[string]v1beta1.ListenerStatusList{ @@ -331,7 +390,11 @@ func TestGenerateQuorumVoters(t *testing.T) { for _, test := range tests { t.Run(test.testName, func(t *testing.T) { - gotQuorumVoters := generateQuorumVoters(test.brokers, test.listenersStatuses) + kafkaCluster.Spec.Brokers = test.brokers + gotQuorumVoters, err := generateQuorumVoters(kafkaCluster, test.listenersStatuses) + if err != nil { + t.Error(err) + } if !reflect.DeepEqual(gotQuorumVoters, test.expectedQuorumVoters) { t.Error("Expected:", test.expectedQuorumVoters, "Got:", gotQuorumVoters) } From 455ef3f559f2b3ac3471f1f6d854230f3a54a294 Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Thu, 27 Jul 2023 13:37:31 -0400 Subject: [PATCH 20/22] Make comments about ZK-relevant configurations more clear --- config/samples/banzaicloud_v1beta1_kafkacluster.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/samples/banzaicloud_v1beta1_kafkacluster.yaml b/config/samples/banzaicloud_v1beta1_kafkacluster.yaml index a4841250d..286934a1f 100644 --- a/config/samples/banzaicloud_v1beta1_kafkacluster.yaml +++ b/config/samples/banzaicloud_v1beta1_kafkacluster.yaml @@ -18,12 +18,12 @@ spec: # Specify the usable ingress controller, only envoy and istioingress supported can be left blank ingressController: "envoy" # Specify the zookeeper addresses where the Kafka should store it's metadata - # (this should not be set in KRaft mode) + # This configuration has no impact if the KafkaCluster is under KRaft mode zkAddresses: - "zookeeper-client.zookeeper:2181" # Specify the zookeeper path where the Kafka related metadatas should be placed # By default it is bound to "/" and can be left blank - # (this should not be set in KRaft mode) + # This configuration has no impact if the KafkaCluster is under KRaft mode zkPath: "/kafka" # rackAwareness add support for Kafka rack aware feature rackAwareness: From 6b3d61600edaa685dde5c253e1df3ac7c23ca93a Mon Sep 17 00:00:00 2001 From: Lucian Ilie <96051211+ctrlaltluc@users.noreply.github.com> Date: Thu, 27 Jul 2023 21:25:17 +0300 Subject: [PATCH 21/22] Add ConcurrentBrokerRestartCountPerRack to RollingUpgradeConfig (#1002) * Add ConcurrentBrokerRestartCountPerRack to RollingUpgradeConfig --- api/v1beta1/kafkacluster_types.go | 12 ++++++++++++ charts/kafka-operator/templates/crds.yaml | 17 +++++++++++++++++ .../kafka.banzaicloud.io_kafkaclusters.yaml | 17 +++++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/api/v1beta1/kafkacluster_types.go b/api/v1beta1/kafkacluster_types.go index a2054072d..b5ea2fa26 100644 --- a/api/v1beta1/kafkacluster_types.go +++ b/api/v1beta1/kafkacluster_types.go @@ -159,6 +159,18 @@ type RollingUpgradeConfig struct { // distinct broker replicas with either offline replicas or out of sync replicas and the number of alerts triggered by // alerts with 'rollingupgrade' FailureThreshold int `json:"failureThreshold"` + + // ConcurrentBrokerRestartCountPerRack controls how many brokers can be restarted in parallel during a rolling upgrade. If + // it is set to a value greater than 1, the operator will restart up to that amount of brokers in parallel, if the + // brokers are within the same rack (as specified by "broker.rack" in broker read-only configs). Since using Kafka broker + // racks spreads out the replicas, we know that restarting multiple brokers in the same rack will not cause more than + // 1/Nth of the replicas of a topic-partition to be unavailable at the same time, where N is the number of racks used. + // This is a safe way to speed up the rolling upgrade. Note that for the rack distribution explained above, Cruise Control + // requires `com.linkedin.kafka.cruisecontrol.analyzer.goals.RackAwareDistributionGoal` to be configured. Default value is 1. + // +kubebuilder:validation:Minimum=1 + // +kubebuilder:default=1 + // +optional + ConcurrentBrokerRestartCountPerRack int `json:"concurrentBrokerRestartCountPerRack,omitempty"` } // DisruptionBudget defines the configuration for PodDisruptionBudget where the workload is managed by the kafka-operator diff --git a/charts/kafka-operator/templates/crds.yaml b/charts/kafka-operator/templates/crds.yaml index f33c2a65e..29e9fc202 100644 --- a/charts/kafka-operator/templates/crds.yaml +++ b/charts/kafka-operator/templates/crds.yaml @@ -21713,6 +21713,23 @@ spec: description: RollingUpgradeConfig defines the desired config of the RollingUpgrade properties: + concurrentBrokerRestartCountPerRack: + default: 1 + description: ConcurrentBrokerRestartCountPerRack controls how + many brokers can be restarted in parallel during a rolling upgrade. + If it is set to a value greater than 1, the operator will restart + up to that amount of brokers in parallel, if the brokers are + within the same rack (as specified by "broker.rack" in broker + read-only configs). Since using Kafka broker racks spreads out + the replicas, we know that restarting multiple brokers in the + same rack will not cause more than 1/Nth of the replicas of + a topic-partition to be unavailable at the same time, where + N is the number of racks used. This is a safe way to speed up + the rolling upgrade. Note that for the rack distribution explained + above, Cruise Control requires `com.linkedin.kafka.cruisecontrol.analyzer.goals.RackAwareDistributionGoal` + to be configured. Default value is 1. + minimum: 1 + type: integer failureThreshold: description: FailureThreshold controls how many failures the cluster can tolerate during a rolling upgrade. Once the number of failures diff --git a/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml b/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml index 9fe66759d..4e890d50a 100644 --- a/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml +++ b/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml @@ -21550,6 +21550,23 @@ spec: description: RollingUpgradeConfig defines the desired config of the RollingUpgrade properties: + concurrentBrokerRestartCountPerRack: + default: 1 + description: ConcurrentBrokerRestartCountPerRack controls how + many brokers can be restarted in parallel during a rolling upgrade. + If it is set to a value greater than 1, the operator will restart + up to that amount of brokers in parallel, if the brokers are + within the same rack (as specified by "broker.rack" in broker + read-only configs). Since using Kafka broker racks spreads out + the replicas, we know that restarting multiple brokers in the + same rack will not cause more than 1/Nth of the replicas of + a topic-partition to be unavailable at the same time, where + N is the number of racks used. This is a safe way to speed up + the rolling upgrade. Note that for the rack distribution explained + above, Cruise Control requires `com.linkedin.kafka.cruisecontrol.analyzer.goals.RackAwareDistributionGoal` + to be configured. Default value is 1. + minimum: 1 + type: integer failureThreshold: description: FailureThreshold controls how many failures the cluster can tolerate during a rolling upgrade. Once the number of failures From 0a90251e7c7dafb59375b7c40adf2f38ee185a47 Mon Sep 17 00:00:00 2001 From: Darren Lau Date: Thu, 27 Jul 2023 20:40:38 -0400 Subject: [PATCH 22/22] Small refactoring --- api/v1beta1/kafkacluster_types.go | 4 ++-- charts/kafka-operator/templates/crds.yaml | 10 ++++++---- .../crds/kafka.banzaicloud.io_kafkaclusters.yaml | 10 ++++++---- pkg/resources/kafka/kafka.go | 14 +++++++------- 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/api/v1beta1/kafkacluster_types.go b/api/v1beta1/kafkacluster_types.go index b5ea2fa26..2f630f26d 100644 --- a/api/v1beta1/kafkacluster_types.go +++ b/api/v1beta1/kafkacluster_types.go @@ -140,7 +140,7 @@ type KafkaClusterStatus struct { RollingUpgrade RollingUpgradeStatus `json:"rollingUpgradeStatus,omitempty"` AlertCount int `json:"alertCount"` ListenerStatuses ListenerStatuses `json:"listenerStatuses,omitempty"` - // ClusterID is a based64-encoded random UUID generated to run the Kafka cluster in KRaft mode + // ClusterID is a base64-encoded random UUID generated by Koperator to run the Kafka cluster in KRaft mode ClusterID string `json:"clusterID,omitempty"` } @@ -207,7 +207,7 @@ type Broker struct { // BrokerConfig defines the broker configuration type BrokerConfig struct { // processRoles defines the role(s) for this particular Kafka node: "broker", "controller", or both. - // This must be set in KRaft mode. + // This must be set in KRaft mode. If set in ZooKeeper mode, Koperator ignores this configuration. // +kubebuilder:validation:MaxItems=2 // +optional Roles []string `json:"processRoles,omitempty"` diff --git a/charts/kafka-operator/templates/crds.yaml b/charts/kafka-operator/templates/crds.yaml index 29e9fc202..ecaa28c70 100644 --- a/charts/kafka-operator/templates/crds.yaml +++ b/charts/kafka-operator/templates/crds.yaml @@ -4167,7 +4167,8 @@ spec: processRoles: description: 'processRoles defines the role(s) for this particular Kafka node: "broker", "controller", or both. This must be - set in KRaft mode.' + set in KRaft mode. If set in ZooKeeper mode, Koperator ignores + this configuration.' items: type: string maxItems: 2 @@ -10488,7 +10489,8 @@ spec: processRoles: description: 'processRoles defines the role(s) for this particular Kafka node: "broker", "controller", or both. - This must be set in KRaft mode.' + This must be set in KRaft mode. If set in ZooKeeper mode, + Koperator ignores this configuration.' items: type: string maxItems: 2 @@ -21857,8 +21859,8 @@ spec: type: object type: object clusterID: - description: ClusterID is a based64-encoded random UUID generated - to run the Kafka cluster in KRaft mode + description: ClusterID is a base64-encoded random UUID generated by + Koperator to run the Kafka cluster in KRaft mode type: string cruiseControlTopicStatus: description: CruiseControlTopicStatus holds info about the CC topic diff --git a/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml b/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml index 4e890d50a..3bcfa0bf6 100644 --- a/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml +++ b/config/base/crds/kafka.banzaicloud.io_kafkaclusters.yaml @@ -4004,7 +4004,8 @@ spec: processRoles: description: 'processRoles defines the role(s) for this particular Kafka node: "broker", "controller", or both. This must be - set in KRaft mode.' + set in KRaft mode. If set in ZooKeeper mode, Koperator ignores + this configuration.' items: type: string maxItems: 2 @@ -10325,7 +10326,8 @@ spec: processRoles: description: 'processRoles defines the role(s) for this particular Kafka node: "broker", "controller", or both. - This must be set in KRaft mode.' + This must be set in KRaft mode. If set in ZooKeeper mode, + Koperator ignores this configuration.' items: type: string maxItems: 2 @@ -21694,8 +21696,8 @@ spec: type: object type: object clusterID: - description: ClusterID is a based64-encoded random UUID generated - to run the Kafka cluster in KRaft mode + description: ClusterID is a base64-encoded random UUID generated by + Koperator to run the Kafka cluster in KRaft mode type: string cruiseControlTopicStatus: description: CruiseControlTopicStatus holds info about the CC topic diff --git a/pkg/resources/kafka/kafka.go b/pkg/resources/kafka/kafka.go index ff8d1bbd6..5445417b1 100644 --- a/pkg/resources/kafka/kafka.go +++ b/pkg/resources/kafka/kafka.go @@ -323,13 +323,6 @@ func (r *Reconciler) Reconcile(log logr.Logger) error { } } - // In KRaft mode: - // 1. there is no way for admin client to know which node is the active controller, controllerID obtained above is just a broker ID of a random active broker (this is intentional by Kafka) - // 2. the follower controllers replicate the data that is written to the active controller and serves as hot standbys if the active controller fails. - // Because the controllers now all track the latest state, controller fail-over will not require a lengthy reloading time to have all the state to transfer to the new controller - // Therefore, by setting the controllerID to be -1 to not take the controller identity into consideration when reordering the brokers - controllerID = -1 - quorumVoters, err = generateQuorumVoters(r.KafkaCluster, controllerIntListenerStatuses) if err != nil { return errors.WrapIfWithDetails(err, @@ -338,6 +331,13 @@ func (r *Reconciler) Reconcile(log logr.Logger) error { "clusterName", r.KafkaCluster.GetName(), "clusterNamespace", r.KafkaCluster.GetNamespace()) } + + // In KRaft mode: + // 1. there is no way for admin client to know which node is the active controller, controllerID obtained above is just a broker ID of a random active broker (this is intentional by Kafka) + // 2. the follower controllers replicate the data that is written to the active controller and serves as hot standbys if the active controller fails. + // Because the controllers now all track the latest state, controller fail-over will not require a lengthy reloading time to have all the state to transfer to the new controller + // Therefore, by setting the controllerID to be -1 to not take the controller identity into consideration when reordering the brokers + controllerID = -1 } reorderedBrokers := reorderBrokers(runningBrokers, boundPersistentVolumeClaims, r.KafkaCluster.Spec.Brokers, r.KafkaCluster.Status.BrokersState, controllerID, log)