Repository: knative-sandbox/eventing-kafka
Pull request #606 (2021-05-07 12:48:18)

[release-0.22] [MTSource] ST Adapter start returns error instead of panic
This is an automated cherry-pick of #505

```release-note

```

knative-prow-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **travis-minke-sap** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @travis-minke-sap` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative-sandbox%2Feventing-kafka).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/knative-sandbox/eventing-kafka/blob/release-0.22/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["travis-minke-sap"]} -->
google-cla[bot]: All (the pull request submitter and all commit authors) CLAs are signed, **but** one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project.  Please have them confirm that by leaving a comment that contains only `@googlebot I consent.` in this pull request.

*Note to project maintainer:* There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent.  In those cases, you can manually confirm consent of the commit author(s), and set the `cla` label to `yes` (if enabled on your project).

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Fknative-sandbox%2Feventing-kafka%2Fpull%2F606) for more info**.

<!-- need_author_consent -->
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-sandbox-eventing-kafka-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[pkg/source/adapter/adapter.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative-sandbox_eventing-kafka/606/pull-knative-sandbox-eventing-kafka-go-coverage/1390649702067736576/artifacts/line-cov.html#file0) | 57.4% | 60.4% | 3.0
[pkg/source/mtadapter/adapter.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative-sandbox_eventing-kafka/606/pull-knative-sandbox-eventing-kafka-go-coverage/1390649702067736576/artifacts/line-cov.html#file1) | 48.3% | 68.4% | 20.1

codecov[bot]: # [Codecov](https://codecov.io/gh/knative-sandbox/eventing-kafka/pull/606?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) Report
> :exclamation: No coverage uploaded for pull request base (`release-0.22@4085d74`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#section-missing-base-commit).
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/knative-sandbox/eventing-kafka/pull/606/graphs/tree.svg?width=650&height=150&src=pr&token=W765CT93QC&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox)](https://codecov.io/gh/knative-sandbox/eventing-kafka/pull/606?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox)

```diff
@@               Coverage Diff               @@
##             release-0.22     #606   +/-   ##
===============================================
  Coverage                ?   73.80%           
===============================================
  Files                   ?      147           
  Lines                   ?     5760           
  Branches                ?        0           
===============================================
  Hits                    ?     4251           
  Misses                  ?     1267           
  Partials                ?      242           
```



------

[Continue to review full report at Codecov](https://codecov.io/gh/knative-sandbox/eventing-kafka/pull/606?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative-sandbox/eventing-kafka/pull/606?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox). Last update [4085d74...69ebaec](https://codecov.io/gh/knative-sandbox/eventing-kafka/pull/606?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox).

knative-prow-robot: @knative-prow-robot: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-sandbox-eventing-kafka-integration-test-mt-source | 69ebaecb2967118d58f9d4521e7a4f0ab22a7da8 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative-sandbox_eventing-kafka/606/pull-knative-sandbox-eventing-kafka-integration-test-mt-source/1390649701971267584) | `/test pull-knative-sandbox-eventing-kafka-integration-test-mt-source`

[Full PR test history](https://gubernator.knative.dev/pr/knative-sandbox_eventing-kafka/606). [Your PR dashboard](https://gubernator.knative.dev/pr/knative-prow-robot).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
lionelvillard: /test pull-knative-sandbox-eventing-kafka-integration-test-mt-source

Patch:
diff --git a/pkg/source/adapter/adapter.go b/pkg/source/adapter/adapter.go
index 6c7dd48c5..7e02f5d92 100644
--- a/pkg/source/adapter/adapter.go
+++ b/pkg/source/adapter/adapter.go
@@ -104,7 +104,7 @@ func (a *Adapter) Start(ctx context.Context) error {
 	consumerGroupFactory := consumer.NewConsumerGroupFactory(addrs, config)
 	group, err := consumerGroupFactory.StartConsumerGroup(a.config.ConsumerGroup, a.config.Topics, a.logger, a)
 	if err != nil {
-		panic(err)
+		return fmt.Errorf("failed to start consumer group: %w", err)
 	}
 	defer func() {
 		err := group.Close()


Test patch:
diff --git a/pkg/source/adapter/adapter_test.go b/pkg/source/adapter/adapter_test.go
index 5b81670ca..2dfc7b382 100644
--- a/pkg/source/adapter/adapter_test.go
+++ b/pkg/source/adapter/adapter_test.go
@@ -28,7 +28,6 @@ import (
 
 	"github.com/Shopify/sarama"
 	"github.com/cloudevents/sdk-go/v2/types"
-	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
 	"knative.dev/eventing/pkg/adapter/v2"
 	"knative.dev/pkg/source"
@@ -441,9 +440,9 @@ func TestAdapter_Start(t *testing.T) { // just increase code coverage
 	_ = os.Setenv("KAFKA_BOOTSTRAP_SERVERS", "my-cluster-kafka-bootstrap.my-kafka-namespace:9092")
 
 	a := NewAdapter(ctx, NewEnvConfig(), nil, nil)
-	require.Panics(t, func() {
-		_ = a.Start(ctx)
-	})
-
+	err := a.Start(ctx)
+	if err == nil {
+		t.Errorf("expected error, but got nil")
+	}
 	cancel()
 }
diff --git a/pkg/source/mtadapter/adapter_test.go b/pkg/source/mtadapter/adapter_test.go
index 492653c5a..9a7cd3f9e 100644
--- a/pkg/source/mtadapter/adapter_test.go
+++ b/pkg/source/mtadapter/adapter_test.go
@@ -33,6 +33,7 @@ import (
 	adaptertest "knative.dev/eventing/pkg/adapter/v2/test"
 	"knative.dev/eventing/pkg/kncloudevents"
 
+	bindingsv1beta1 "knative.dev/eventing-kafka/pkg/apis/bindings/v1beta1"
 	sourcesv1beta1 "knative.dev/eventing-kafka/pkg/apis/sources/v1beta1"
 )
 
@@ -81,6 +82,28 @@ func TestUpdateRemoveSources(t *testing.T) {
 		t.Error(`Expected adapter to contain "test-ns/test-name"`)
 	}
 
+	adapter.Update(ctx, &sourcesv1beta1.KafkaSource{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-name-badBroker",
+			Namespace: "test-ns",
+		},
+		Spec: sourcesv1beta1.KafkaSourceSpec{
+			KafkaAuthSpec: bindingsv1beta1.KafkaAuthSpec{
+				BootstrapServers: []string{"my-cluster-kafka-bootstrap-badBroker.kafka.svc:9092"},
+			},
+		},
+		Status: sourcesv1beta1.KafkaSourceStatus{
+			Placeable: duckv1alpha1.Placeable{
+				Placement: []duckv1alpha1.Placement{
+					{PodName: podName, VReplicas: int32(1)},
+				}},
+		},
+	})
+
+	if _, ok := adapter.sources["test-ns/test-name-badBroker"]; !ok {
+		t.Error(`Expected adapter to contain "test-ns/test-name-badBroker"`)
+	}
+
 	select {
 	case a := <-runningAdapterChan:
 		if !a.running {
@@ -103,6 +126,19 @@ func TestUpdateRemoveSources(t *testing.T) {
 		t.Error(`Expected adapter to not contain "test-ns/test-name"`)
 	}
 
+	adapter.Remove(&sourcesv1beta1.KafkaSource{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-name-badBroker",
+			Namespace: "test-ns",
+		},
+		Spec:   sourcesv1beta1.KafkaSourceSpec{},
+		Status: sourcesv1beta1.KafkaSourceStatus{},
+	})
+
+	if _, ok := adapter.sources["test-ns/test-name-badBroker"]; ok {
+		t.Error(`Expected adapter to not contain "test-ns/test-name-badBroker"`)
+	}
+
 	select {
 	case a := <-stoppingAdapterChan:
 		if a.running {