Skip to content

Commit

Permalink
[v2] removed deprecated brokerList for Kafka scaler (#882)
Browse files Browse the repository at this point in the history
Signed-off-by: Samuel Macko <samuel.macko.sm@gmail.com>
  • Loading branch information
samuelmacko committed Jun 17, 2020
1 parent 804c1b9 commit 3455485
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 30 deletions.
4 changes: 1 addition & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

## Deprecations

- As of v1.3, support for `brokerList` is deprecated for our Kafka topic scaler and will be removed in v2.0 ([#632](https://github.com/kedacore/keda/issues/632))

## History

- [v2.0.0](#v200)
Expand All @@ -19,6 +17,7 @@
### Breaking Changes

- Remove `New()` and `Close()` from the interface of `service ExternalScaler` in `externalscaler.proto`.
- Removed deprecated brokerList for Kafka scaler ([#882](https://github.com/kedacore/keda/pull/882))

## v1.4.1

Expand Down Expand Up @@ -81,7 +80,6 @@ None.
### Improvements

- Make targetQueryValue configurable in postgreSQL scaler ([#643](https://github.com/kedacore/keda/pull/643))
- Added bootstrapServers to deprecate brokerList ([#621](https://github.com/kedacore/keda/pull/621))
- Removed the need for deploymentName label ([#644](https://github.com/kedacore/keda/pull/644))
- Adding Kubernetes recommended labels to resources ([#596](https://github.com/kedacore/keda/pull/596))

Expand Down
12 changes: 2 additions & 10 deletions pkg/scalers/kafka_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,20 +83,12 @@ func NewKafkaScaler(resolvedEnv, metadata, authParams map[string]string) (Scaler
func parseKafkaMetadata(resolvedEnv, metadata, authParams map[string]string) (kafkaMetadata, error) {
meta := kafkaMetadata{}

// brokerList marked as deprecated, bootstrapServers is the new one to use
if metadata["brokerList"] != "" && metadata["bootstrapServers"] != "" {
return meta, errors.New("cannot specify both bootstrapServers and brokerList (deprecated)")
}
if metadata["brokerList"] == "" && metadata["bootstrapServers"] == "" {
return meta, errors.New("no bootstrapServers or brokerList (deprecated) given")
if metadata["bootstrapServers"] == "" {
return meta, errors.New("no bootstrapServers given")
}
if metadata["bootstrapServers"] != "" {
meta.bootstrapServers = strings.Split(metadata["bootstrapServers"], ",")
}
if metadata["brokerList"] != "" {
kafkaLog.V(0).Info("WARNING: usage of brokerList is deprecated. use bootstrapServers instead.")
meta.bootstrapServers = strings.Split(metadata["brokerList"], ",")
}

if metadata["consumerGroup"] == "" {
return meta, errors.New("no consumer group given")
Expand Down
21 changes: 4 additions & 17 deletions pkg/scalers/kafka_scaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ type parseKafkaMetadataTestData struct {

// A complete valid metadata example for reference
var validMetadata = map[string]string{
"brokerList": "broker1:9092,broker2:9092",
"consumerGroup": "my-group",
"topic": "my-topic",
"bootstrapServers": "broker1:9092,broker2:9092",
"consumerGroup": "my-group",
"topic": "my-topic",
}

// A complete valid authParams example for sasl, with username and passwd
Expand All @@ -32,22 +32,9 @@ var validWithAuthParams = map[string]string{
var validWithoutAuthParams = map[string]string{}

var parseKafkaMetadataTestDataset = []parseKafkaMetadataTestData{
// failure, no brokerList (deprecated) or bootstrapServers
// failure, no bootstrapServers
{map[string]string{}, true, 0, nil, "", ""},
// failure, both brokerList (deprecated) and bootstrapServers
{map[string]string{"brokerList": "foobar:9092", "bootstrapServers": "foobar:9092"}, true, 0, nil, "", ""},

// tests with brokerList (deprecated)
// failure, no consumer group
{map[string]string{"brokerList": "foobar:9092"}, true, 1, []string{"foobar:9092"}, "", ""},
// failure, no topic
{map[string]string{"brokerList": "foobar:9092", "consumerGroup": "my-group"}, true, 1, []string{"foobar:9092"}, "my-group", ""},
// success
{map[string]string{"brokerList": "foobar:9092", "consumerGroup": "my-group", "topic": "my-topic"}, false, 1, []string{"foobar:9092"}, "my-group", "my-topic"},
// success, more brokers
{map[string]string{"brokerList": "foo:9092,bar:9092", "consumerGroup": "my-group", "topic": "my-topic"}, false, 2, []string{"foo:9092", "bar:9092"}, "my-group", "my-topic"},

// tests with bootstrapServers
// failure, no consumer group
{map[string]string{"bootstrapServers": "foobar:9092"}, true, 1, []string{"foobar:9092"}, "", ""},
// failure, no topic
Expand Down

0 comments on commit 3455485

Please sign in to comment.