chore(deps): bump sarama to v1.41.2 #3348

dnwe · 2023-09-19T08:57:32Z

Proposed Changes

chore(deps): bump sarama to v1.41.2

Note: the module path has changed to github.com/IBM/sarama since ownership transitioned away from Shopify

Additionally use sarama.DefaultVersion as the default config (rather than sarama.MaxVersion) if the user hasn't specified an explicit sarama.config.Version in their config-kafka ConfigMap

Release Note

Migrate to github.com/IBM/sarama and use the latest version for improved protocol support and adherence.
If your target Kafka cluster version is older than 2.1.0 then ensure you set a matching sarama.config.Version in your config-kafka ConfigMap.

knative-prow · 2023-09-19T08:57:40Z

Welcome @dnwe! It looks like this is your first PR to knative-extensions/eventing-kafka-broker 🎉

knative-prow · 2023-09-19T08:57:41Z

Hi @dnwe. Thanks for your PR.

I'm waiting for a knative-extensions member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

pierDipi · 2023-09-19T09:14:10Z

/ok-to-test

pierDipi · 2023-09-19T09:14:20Z

Thank you

codecov · 2023-09-19T09:22:20Z

Codecov Report

Merging #3348 (ec0769b) into main (e7f53ae) will increase coverage by 0.04%.
Report is 1 commits behind head on main.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##               main    #3348      +/-   ##
============================================
+ Coverage     61.61%   61.66%   +0.04%     
- Complexity      768      769       +1     
============================================
  Files           182      182              
  Lines         12331    12331              
  Branches        268      268              
============================================
+ Hits           7598     7604       +6     
+ Misses         4136     4132       -4     
+ Partials        597      595       -2

Flag	Coverage Δ
java-unittests	`71.01% <ø> (+0.09%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
control-plane/pkg/kafka/client.go	`13.04% <0.00%> (ø)`
control-plane/pkg/kafka/consumer_group.go	`58.62% <ø> (ø)`
control-plane/pkg/kafka/consumer_group_lag.go	`62.59% <ø> (ø)`
control-plane/pkg/kafka/topic.go	`80.80% <ø> (ø)`
control-plane/pkg/reconciler/broker/controller.go	`73.68% <ø> (ø)`
...ane/pkg/reconciler/broker/namespaced_controller.go	`0.00% <ø> (ø)`
control-plane/pkg/reconciler/channel/channel.go	`69.10% <ø> (ø)`
control-plane/pkg/reconciler/channel/controller.go	`90.42% <ø> (ø)`
...ntrol-plane/pkg/reconciler/channel/v2/channelv2.go	`70.96% <ø> (ø)`
...ol-plane/pkg/reconciler/channel/v2/controllerv2.go	`92.10% <ø> (ø)`
... and 8 more

... and 1 file with indirect coverage changes

dnwe · 2023-09-19T13:25:47Z

Hmmm looking over these test results:

Error from server (BadRequest): error when creating "test/config/sacura-sink-source/resources/100-source.yaml": admission webhook "validation.webhook.kafka.eventing.knative.dev" denied the request: validation failed: invalid value: : spec.initialOffset

eventing-kafka-broker/control-plane/pkg/apis/sources/v1beta1/kafka_validation.go

Lines 49 to 53 in e087768

    
           switch kss.InitialOffset { 
        
           case OffsetEarliest, OffsetLatest: 
        
           default: 
        
           	errs = errs.Also(apis.ErrInvalidValue(kss.InitialOffset, "initialOffset")) 
        
           }

Not quite sure why this is failing the validation with an empty value, as the defaults func should have initialised it to OffsetLatest

eventing-kafka-broker/control-plane/pkg/apis/sources/v1beta1/kafka_defaults.go

Lines 53 to 55 in e087768

    
           if k.Spec.InitialOffset == "" { 
        
           	k.Spec.InitialOffset = OffsetLatest 
        
           }

dnwe · 2023-09-19T13:26:43Z

In the other tests, strimzi seems to be struggling:

Error from server (InternalError): error when retrieving current configuration of:
Resource: "rbac.authorization.k8s.io/v1, Resource=rolebindings", GroupVersionKind: "rbac.authorization.k8s.io/v1, Kind=RoleBinding"
Name: "strimzi-cluster-operator", Namespace: "kafka"
from server for: "STDIN": an error on the server ("Internal Server Error: "/apis/rbac.authorization.k8s.io/v1/namespaces/kafka/rolebindings/strimzi-cluster-operator": the server is currently unable to handle the request") has prevented the request from succeeding (get rolebindings.rbac.authorization.k8s.io strimzi-cluster-operator)

dnwe · 2023-09-19T13:34:14Z

/retest

Cali0707 · 2023-09-20T16:55:55Z

@dnwe could you run hack/update-codegen.sh? That should fix the build tests

Note: the module path has changed to github.com/IBM/sarama since ownership transitioned away from Shopify Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>

If the user hasn't specified an explicit sarama.config.Version in their config-kafka ConfigMap then use sarama's DefaultVersion rather than setting MaxVersion (which is rarely what the user will want). Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>

dnwe · 2023-09-20T17:21:43Z

/retest-required

dnwe · 2023-09-20T18:10:04Z

/retest-required

dnwe · 2023-09-21T12:32:43Z

/retest-required

matzew · 2023-09-21T14:22:12Z

/retest

matzew · 2023-09-22T05:17:14Z

/retest

matzew · 2023-09-22T06:40:57Z

control-plane/pkg/kafka/client.go

@@ -56,7 +56,7 @@ type NewClientFunc func(addrs []string, config *sarama.Config) (sarama.Client, e
 // GetSaramaConfig returns Kafka Client configuration with the given options applied.
 func GetSaramaConfig(configOptions ...ConfigOption) (*sarama.Config, error) {
 	config := sarama.NewConfig()
-	config.Version = sarama.MaxVersion
+	config.Version = sarama.DefaultVersion


Is this due to newer version?

matzew

/lgtm
/approve

/retest

knative-prow · 2023-09-22T06:42:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dnwe, matzew

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [matzew]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

matzew · 2023-09-22T07:09:24Z

/test channel-integration-tests-sasl-plain

matzew · 2023-09-22T07:16:54Z

/test channel-integration-tests-sasl-plain

knative-prow · 2023-09-22T07:51:59Z

@dnwe: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
reconciler-tests-keda_eventing-kafka-broker_main	`ec0769b`	link	false	`/test reconciler-tests-keda`
reconciler-tests-loom_eventing-kafka-broker_main	`ec0769b`	link	false	`/test reconciler-tests-loom`
reconciler-tests-namespaced-broker-loom_eventing-kafka-broker_main	`ec0769b`	link	false	`/test reconciler-tests-namespaced-broker-loom`

Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

matzew · 2023-09-22T08:12:14Z

/test channel-integration-tests-sasl-plain

matzew · 2023-09-22T09:43:43Z

@dnwe thanks for the help!

knative-prow bot added the area/control-plane label Sep 19, 2023

knative-prow bot requested review from aliok and odacremolbap September 19, 2023 08:57

knative-prow bot added the area/test label Sep 19, 2023

knative-prow bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 19, 2023

knative-prow bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Sep 19, 2023

knative-prow bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 19, 2023

knative-prow bot added the area/data-plane label Sep 19, 2023

dnwe force-pushed the sarama branch 3 times, most recently from 894266e to 3c6faa1 Compare September 19, 2023 12:34

dnwe force-pushed the sarama branch from fea5486 to 6d1bfe8 Compare September 20, 2023 17:15

dnwe added 2 commits September 20, 2023 18:16

chore(deps): bump sarama to v1.41.2

5537bd9

Note: the module path has changed to github.com/IBM/sarama since ownership transitioned away from Shopify Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>

dnwe force-pushed the sarama branch from 6d1bfe8 to ec0769b Compare September 20, 2023 17:16

matzew reviewed Sep 22, 2023

View reviewed changes

matzew approved these changes Sep 22, 2023

View reviewed changes

knative-prow bot assigned matzew Sep 22, 2023

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Sep 22, 2023

knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 22, 2023

knative-prow bot merged commit 5c4461d into knative-extensions:main Sep 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(deps): bump sarama to v1.41.2 #3348

chore(deps): bump sarama to v1.41.2 #3348

dnwe commented Sep 19, 2023

knative-prow bot commented Sep 19, 2023

knative-prow bot commented Sep 19, 2023

pierDipi commented Sep 19, 2023

pierDipi commented Sep 19, 2023

codecov bot commented Sep 19, 2023 •

edited

Loading

dnwe commented Sep 19, 2023

dnwe commented Sep 19, 2023

dnwe commented Sep 19, 2023

Cali0707 commented Sep 20, 2023

dnwe commented Sep 20, 2023

dnwe commented Sep 20, 2023

dnwe commented Sep 21, 2023

matzew commented Sep 21, 2023

matzew commented Sep 22, 2023

matzew Sep 22, 2023

matzew left a comment

knative-prow bot commented Sep 22, 2023

matzew commented Sep 22, 2023

matzew commented Sep 22, 2023

knative-prow bot commented Sep 22, 2023 •

edited

Loading

matzew commented Sep 22, 2023

matzew commented Sep 22, 2023

chore(deps): bump sarama to v1.41.2 #3348

chore(deps): bump sarama to v1.41.2 #3348

Conversation

dnwe commented Sep 19, 2023

Proposed Changes

knative-prow bot commented Sep 19, 2023

knative-prow bot commented Sep 19, 2023

pierDipi commented Sep 19, 2023

pierDipi commented Sep 19, 2023

codecov bot commented Sep 19, 2023 • edited Loading

Codecov Report

dnwe commented Sep 19, 2023

dnwe commented Sep 19, 2023

dnwe commented Sep 19, 2023

Cali0707 commented Sep 20, 2023

dnwe commented Sep 20, 2023

dnwe commented Sep 20, 2023

dnwe commented Sep 21, 2023

matzew commented Sep 21, 2023

matzew commented Sep 22, 2023

matzew Sep 22, 2023

Choose a reason for hiding this comment

matzew left a comment

Choose a reason for hiding this comment

knative-prow bot commented Sep 22, 2023

matzew commented Sep 22, 2023

matzew commented Sep 22, 2023

knative-prow bot commented Sep 22, 2023 • edited Loading

matzew commented Sep 22, 2023

matzew commented Sep 22, 2023

codecov bot commented Sep 19, 2023 •

edited

Loading

knative-prow bot commented Sep 22, 2023 •

edited

Loading