Repository: knative-sandbox/eventing-kafka
Pull request #231 (2020-11-25 14:49:38)

Go with the overall default of the leader election
Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>

## Proposed Changes

- with the global default of the `config-leader-election`, it is still possible to do some fine-tuning, via the `CONFIG_LEADERELECTION_NAME` env-var on the deployments (e.g. controllers/dispatchers)

<!--
If this change has user-visible impact, follow the instructions below.
Examples include:

- 🎁 Add new feature
- 🐛 Fix bug
- 🧽 Update or clean up current behavior
- 🗑️ Remove feature or internal logic

Otherwise delete the rest of this template.
-->

**Release Note**

<!--
🗒️ If this change has user-visible impact, write a release note in the block
below. Include the string "action required" if additional action is required of
users switching to the new release, for example in case of a breaking change.

Write as if you are speaking to users, not other Knative contributors. If this
change has no user-visible impact, no release-note is needed.
-->

```release-note
KafkaSource and KafkaChannel will be default use the `config-leader-election` CM for configs
```

**Docs**

<!--
📖 If this change has user-visible impact, link to an issue or PR in
https://github.com/knative/docs.
-->


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

This pull-request has been approved by: *<a href="https://github.com/knative-sandbox/eventing-kafka/pull/231#" title="Author self-approved">matzew</a>*

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

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

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

- ~~[OWNERS](https://github.com/knative-sandbox/eventing-kafka/blob/master/OWNERS)~~ [matzew]

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":[]} -->
matzew: /hold
matzew: @travis-minke-sap @antoineco how do you feel about this ? 

That way, we can still leverage a global `config-leader-election`, while via the ENV_VAR of the deployment we have still the chance to set it to a different ConfigMap 
codecov[bot]: # [Codecov](https://codecov.io/gh/knative-sandbox/eventing-kafka/pull/231?src=pr&el=h1) Report
> Merging [#231](https://codecov.io/gh/knative-sandbox/eventing-kafka/pull/231?src=pr&el=desc) (8a8bfdf) into [master](https://codecov.io/gh/knative-sandbox/eventing-kafka/commit/b20084393d9bbe519157218d928b43100e955f16?el=desc) (b200843) will **increase** coverage by `0.04%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/knative-sandbox/eventing-kafka/pull/231/graphs/tree.svg?width=650&height=150&src=pr&token=W765CT93QC)](https://codecov.io/gh/knative-sandbox/eventing-kafka/pull/231?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #231      +/-   ##
==========================================
+ Coverage   75.73%   75.78%   +0.04%     
==========================================
  Files         112      112              
  Lines        4274     4274              
==========================================
+ Hits         3237     3239       +2     
+ Misses        842      841       -1     
+ Partials      195      194       -1     
```


| [Impacted Files](https://codecov.io/gh/knative-sandbox/eventing-kafka/pull/231?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ated/reconciler/controller/resources/dispatcher.go](https://codecov.io/gh/knative-sandbox/eventing-kafka/pull/231/diff?src=pr&el=tree#diff-cGtnL2NoYW5uZWwvY29uc29saWRhdGVkL3JlY29uY2lsZXIvY29udHJvbGxlci9yZXNvdXJjZXMvZGlzcGF0Y2hlci5nbw==) | `100.00% <100.00%> (ø)` | |
| [...tributed/common/kafka/admin/eventhubcache/cache.go](https://codecov.io/gh/knative-sandbox/eventing-kafka/pull/231/diff?src=pr&el=tree#diff-cGtnL2NoYW5uZWwvZGlzdHJpYnV0ZWQvY29tbW9uL2thZmthL2FkbWluL2V2ZW50aHViY2FjaGUvY2FjaGUuZ28=) | `67.46% <0.00%> (+2.40%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative-sandbox/eventing-kafka/pull/231?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative-sandbox/eventing-kafka/pull/231?src=pr&el=footer). Last update [b200843...8a8bfdf](https://codecov.io/gh/knative-sandbox/eventing-kafka/pull/231?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

matzew: @antoineco I think we even should _REMOVE_ the duplicated config for "default" values, all togehter :-) 

https://github.com/knative/eventing/issues/4593


lionelvillard: note that leader election has been disabled for the dispatcher
antoineco: 


> 
> 
> @antoineco I think we even should _REMOVE_ the duplicated config for "default" values, all togehter :-)
> 
> [knative/eventing#4593](https://github.com/knative/eventing/issues/4593)

@matzew but then it's a tad harder for users to discover what can be overridden, and expects them to know the `pkg` repo pretty well. I wouldn't set every possible env var in every controller of course, but maybe we can agree on keeping logging + obs + tracing + leader election for discoverability?
knative-prow-robot: @matzew: PR needs rebase.

<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.
</details>
matzew: @antoineco I updated 
matzew: /unhold
knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative-sandbox/eventing-kafka/pull/231#pullrequestreview-539095921" title="LGTM">antoineco</a>*, *<a href="https://github.com/knative-sandbox/eventing-kafka/pull/231#" title="Author self-approved">matzew</a>*

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

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

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

- ~~[OWNERS](https://github.com/knative-sandbox/eventing-kafka/blob/master/OWNERS)~~ [matzew]

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":[]} -->

Patch:
diff --git a/config/channel/consolidated/configmaps/leader-election.yaml b/config/channel/consolidated/configmaps/leader-election.yaml
index 579df8f25..0490eab9a 100644
--- a/config/channel/consolidated/configmaps/leader-election.yaml
+++ b/config/channel/consolidated/configmaps/leader-election.yaml
@@ -15,7 +15,7 @@
 apiVersion: v1
 kind: ConfigMap
 metadata:
-  name: config-leader-election-kafka
+  name: config-leader-election
   namespace: knative-eventing
 data:
   # An inactive but valid configuration follows; see example.
diff --git a/config/channel/consolidated/deployments/controller.yaml b/config/channel/consolidated/deployments/controller.yaml
index d004d6e81..504902228 100644
--- a/config/channel/consolidated/deployments/controller.yaml
+++ b/config/channel/consolidated/deployments/controller.yaml
@@ -47,7 +47,7 @@ spec:
             fieldRef:
               fieldPath: metadata.name
         - name: CONFIG_LEADERELECTION_NAME
-          value: config-leader-election-kafka
+          value: config-leader-election
         - name: DISPATCHER_IMAGE
           value: ko://knative.dev/eventing-kafka/cmd/channel/consolidated/dispatcher
         ports:
diff --git a/config/channel/consolidated/deployments/dispatcher.yaml b/config/channel/consolidated/deployments/dispatcher.yaml
index a9782c8ed..296edd3df 100644
--- a/config/channel/consolidated/deployments/dispatcher.yaml
+++ b/config/channel/consolidated/deployments/dispatcher.yaml
@@ -49,11 +49,11 @@ spec:
             fieldRef:
               fieldPath: metadata.name
         - name: METRICS_DOMAIN
-          value: "knative.dev/eventing"
+          value: knative.dev/eventing
         - name: CONFIG_LOGGING_NAME
-          value: "config-logging"
+          value: config-logging
         - name: CONFIG_LEADERELECTION_NAME
-          value: "config-leader-election-kafka"
+          value: config-leader-election
         - name: CONTAINER_NAME
           value: dispatcher
         ports:
diff --git a/config/channel/distributed/300-leaderelection-configmap.yaml b/config/channel/distributed/300-leaderelection-configmap.yaml
index aa54dfb4b..b708ec236 100644
--- a/config/channel/distributed/300-leaderelection-configmap.yaml
+++ b/config/channel/distributed/300-leaderelection-configmap.yaml
@@ -15,7 +15,7 @@
 apiVersion: v1
 kind: ConfigMap
 metadata:
-  name: config-leader-election-kafkachannel
+  name: config-leader-election
   namespace: knative-eventing
 data:
   # An inactive but valid configuration follows; see example.
diff --git a/config/channel/distributed/500-controller-deployment.yaml b/config/channel/distributed/500-controller-deployment.yaml
index 1b285bfe1..e30cdef13 100644
--- a/config/channel/distributed/500-controller-deployment.yaml
+++ b/config/channel/distributed/500-controller-deployment.yaml
@@ -52,7 +52,7 @@ spec:
         - name: CONFIG_LOGGING_NAME
           value: config-logging
         - name: CONFIG_LEADERELECTION_NAME
-          value: config-leader-election-kafkachannel
+          value: config-leader-election
         - name: SERVICE_ACCOUNT
           valueFrom:
             fieldRef:
diff --git a/config/source/configmaps/leader-election.yaml b/config/source/configmaps/leader-election.yaml
index d1651b5e1..85dc4b6be 100644
--- a/config/source/configmaps/leader-election.yaml
+++ b/config/source/configmaps/leader-election.yaml
@@ -15,7 +15,7 @@
 apiVersion: v1
 kind: ConfigMap
 metadata:
-  name: config-leader-election-kafka
+  name: config-leader-election
   namespace: knative-eventing
 data:
   # An inactive but valid configuration follows; see example.
diff --git a/config/source/deployments/controller.yaml b/config/source/deployments/controller.yaml
index 233333117..bc74014d6 100644
--- a/config/source/deployments/controller.yaml
+++ b/config/source/deployments/controller.yaml
@@ -43,7 +43,7 @@ spec:
         - name: CONFIG_OBSERVABILITY_NAME
           value: config-observability
         - name: CONFIG_LEADERELECTION_NAME
-          value: config-leader-election-kafka
+          value: config-leader-election
         - name: KAFKA_RA_IMAGE
           value: ko://knative.dev/eventing-kafka/cmd/source/receive_adapter
         volumeMounts:
diff --git a/pkg/channel/consolidated/reconciler/controller/resources/dispatcher.go b/pkg/channel/consolidated/reconciler/controller/resources/dispatcher.go
index cae363d23..63fdfba7e 100644
--- a/pkg/channel/consolidated/reconciler/controller/resources/dispatcher.go
+++ b/pkg/channel/consolidated/reconciler/controller/resources/dispatcher.go
@@ -112,7 +112,7 @@ func makeEnv(args DispatcherArgs) []corev1.EnvVar {
 		Value: "config-logging",
 	}, {
 		Name:  "CONFIG_LEADERELECTION_NAME",
-		Value: "config-leader-election-kafka",
+		Value: "config-leader-election",
 	}}
 
 	if args.DispatcherScope == "namespace" {


Test patch:
diff --git a/pkg/channel/consolidated/reconciler/controller/resources/dispatcher_test.go b/pkg/channel/consolidated/reconciler/controller/resources/dispatcher_test.go
index ee4bbef33..c68cadcbd 100644
--- a/pkg/channel/consolidated/reconciler/controller/resources/dispatcher_test.go
+++ b/pkg/channel/consolidated/reconciler/controller/resources/dispatcher_test.go
@@ -77,7 +77,7 @@ func TestNewDispatcher(t *testing.T) {
 								Value: "config-logging",
 							}, {
 								Name:  "CONFIG_LEADERELECTION_NAME",
-								Value: "config-leader-election-kafka",
+								Value: "config-leader-election",
 							}},
 							Ports: []corev1.ContainerPort{{
 								Name:          "metrics",
@@ -159,7 +159,7 @@ func TestNewNamespaceDispatcher(t *testing.T) {
 								Value: "config-logging",
 							}, {
 								Name:  "CONFIG_LEADERELECTION_NAME",
-								Value: "config-leader-election-kafka",
+								Value: "config-leader-election",
 							}, {
 								Name: "NAMESPACE",
 								ValueFrom: &corev1.EnvVarSource{