Add support for leader election #966

pmorie · 2020-02-25T16:05:30Z

Proposed Changes

Add (inactive) leader election configurations for:

Kafka channel
Kafka source
Prometheus source
Github source
Couchdb source
AWS SQS

Release Note

Add support for leader election to:

- Kafka channel
- Kafka source
- Prometheus source
- Github source
- Couchdb source
- AWS SQS

n3wscott · 2020-02-26T15:15:15Z

/approve
/lgtm
/hold
hold for found source = "github.com/pmorie/pkg" for knative/pkg in toml.

knative-prow-robot · 2020-02-26T15:15:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: n3wscott, pmorie

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~~ [n3wscott]

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

matzew · 2020-02-26T17:04:49Z

Is this already in the pkg 0.13 ?

matzew · 2020-02-26T17:17:29Z

Is this already in the pkg 0.13 ?

it is: https://github.com/knative/pkg/commits/release-0.13

matzew · 2020-02-26T17:18:30Z

and since we now have that already used in here:

https://github.com/knative/eventing-contrib/pull/968/files

I think this is not needed?

pmorie · 2020-02-26T20:03:27Z

it is: https://github.com/knative/pkg/commits/release-0.13

Nope, that's just the commit that differentiates the main for the normal controllers and webhooks.

evankanderson

Do you want to update the PR description?

Proposed Changes

This affects only the kafka channel dispatcher, since it uses sharedmain.MainWithContext

evankanderson · 2020-02-27T07:34:41Z

awssqs/config/201-clusterrole.yaml

+  resources:
+  - leases
+  verbs:
+  - get


Why does this use an explicit list, but the couchdb YAML and others use a reference?

I followed the pattern in use in whatever file I was modifying, which follow varying conventions.

evankanderson · 2020-02-27T07:35:40Z

couchdb/source/config/config-leader-election.yaml

+    # leader election is enabled. Valid values are:
+    #
+    # - couchdb-controller
+    enabledComponents: "couchdb-controller"


Do we want to rename the awssqs component awssqs-controller, rather than controller?

Probably sensible

evankanderson · 2020-02-27T07:37:19Z

kafka/channel/config/200-dispatcher-clusterrole.yaml

+      - "coordination.k8s.io"
+    resources:
+      - leases
+    verbs:


Why is this a smaller list of verbs than elsewhere (missing delete)?

Inadvertent, I was tired.

evankanderson · 2020-02-27T07:37:50Z

kafka/channel/config/400-leader-election-config.yaml

+    #
+    # - kafkachannel_dispatcher
+    # - kafkachannel_controller
+    enabledComponents: "kafkachannel_dispatcher,kafkachannel_controller"


Should these have hyphens, to match the changes in the main.go files?

Whoops, I thought I got them all. thanks

matzew · 2020-02-27T07:52:45Z

awssqs/config/config-leader-election.yaml

+    # leader election is enabled. Valid values are:
+    #
+    # - controller
+    enabledComponents: "controller"


should we here, and perhaps in https://github.com/knative/eventing-contrib/blob/master/awssqs/cmd/controller/main.go#L29

make this more like awssqs-controller ?

matzew · 2020-02-27T07:54:59Z

kafka/channel/cmd/channel_dispatcher/main.go

@@ -26,7 +26,7 @@ import (
 	controller "knative.dev/eventing-contrib/kafka/channel/pkg/reconciler/dispatcher"
 )

-const component = "kafkachannel_dispatcher"
+const component = "kafkachannel-dispatcher"


there is references to this _ version in kafka/docs/metrics.md -> mind updateing ?

ahhh, thank you

n3wscott · 2020-02-27T18:45:32Z

/lgtm

n3wscott · 2020-02-27T18:53:46Z

/unhold

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Feb 25, 2020

knative-prow-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Feb 25, 2020

pmorie changed the title ~~DO NOT MERGE: update pkg dep to pmorie fork~~ Add support for leader election Feb 25, 2020

knative-prow-robot requested review from lionelvillard and vaikas February 25, 2020 16:05

mattmoor reviewed Feb 25, 2020

View reviewed changes

knative-prow-robot assigned n3wscott Feb 26, 2020

knative-prow-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. labels Feb 26, 2020

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 26, 2020

pmorie force-pushed the leader-election branch from 3efa2a3 to 5c745b4 Compare February 26, 2020 20:16

knative-prow-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 26, 2020

mattmoor reviewed Feb 26, 2020

View reviewed changes

pmorie force-pushed the leader-election branch from 5c745b4 to d0bcb38 Compare February 26, 2020 20:31

mattmoor reviewed Feb 26, 2020

View reviewed changes

pmorie force-pushed the leader-election branch from d0bcb38 to 748a4ac Compare February 26, 2020 21:41

mattmoor reviewed Feb 26, 2020

View reviewed changes

pmorie force-pushed the leader-election branch from 748a4ac to 0945318 Compare February 26, 2020 21:50

mattmoor reviewed Feb 26, 2020

View reviewed changes

pmorie added 2 commits February 26, 2020 19:03

Add leader election config for kafka channel

7347572

Add leader election config for AWS SQS

a57f880

pmorie added 5 commits February 26, 2020 19:03

Add leader election config for couchdb

98c7600

Add leader election config for kafka source

04c3efb

Add leader election config for github source

d5b9aa4

Add leader election config for prometheus

15e20e7

Update dependencies

4c6202c

pmorie force-pushed the leader-election branch from a5aaa17 to 4c6202c Compare February 27, 2020 00:18

knative-prow-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 27, 2020

evankanderson reviewed Feb 27, 2020

View reviewed changes

matzew reviewed Feb 27, 2020

View reviewed changes

pmorie added 3 commits February 27, 2020 12:09

Address feedback for AWS SQS

04f5a8e

Remove couchdb controller from HA enablement

83cd463

Address feedback on Kafka

aeb3de1

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 27, 2020

knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 27, 2020

knative-prow-robot merged commit 9bdda04 into knative:master Feb 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for leader election #966

Add support for leader election #966

pmorie commented Feb 25, 2020 •

edited

Loading

n3wscott commented Feb 26, 2020

knative-prow-robot commented Feb 26, 2020

matzew commented Feb 26, 2020

matzew commented Feb 26, 2020

matzew commented Feb 26, 2020

pmorie commented Feb 26, 2020

evankanderson left a comment

evankanderson Feb 27, 2020

pmorie Feb 27, 2020

evankanderson Feb 27, 2020

pmorie Feb 27, 2020

evankanderson Feb 27, 2020

pmorie Feb 27, 2020

evankanderson Feb 27, 2020

pmorie Feb 27, 2020

matzew Feb 27, 2020

matzew Feb 27, 2020

pmorie Feb 27, 2020

n3wscott commented Feb 27, 2020

n3wscott commented Feb 27, 2020

Add support for leader election #966

Add support for leader election #966

Conversation

pmorie commented Feb 25, 2020 • edited Loading

Proposed Changes

n3wscott commented Feb 26, 2020

knative-prow-robot commented Feb 26, 2020

matzew commented Feb 26, 2020

matzew commented Feb 26, 2020

matzew commented Feb 26, 2020

pmorie commented Feb 26, 2020

evankanderson left a comment

Choose a reason for hiding this comment

Proposed Changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

n3wscott commented Feb 27, 2020

n3wscott commented Feb 27, 2020

pmorie commented Feb 25, 2020 •

edited

Loading