Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v2] removed deprecated brokerList #882

Merged
merged 3 commits into from
Jun 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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