Add Broker reconciler #31

pierDipi · 2020-07-08T08:50:48Z

Proposed Changes

Add Broker reconciler
Add a base reconciler for both Broker and Trigger reconcilers.

slinkydeveloper

Maybe I lost it in the process, but before triggering the update with UpdateDispatcherPodsAnnotation do you assert that the data plane config map actually changed?

slinkydeveloper · 2020-07-09T10:44:39Z

control-plane/config/100-kafka-broker-configmap.yaml

+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: kafka-broker-config


Can we reuse config-kafka config map? (the same we use in eventing-contrib for kafka channel, we also have code to parse it https://github.com/knative/eventing-contrib/blob/master/kafka/channel/pkg/utils/util.go#L53). If it misses something, let's add stuff to it (the defaults on topic configs seems useful for kafka channel too)

control-plane/config/500-controller.yaml

control-plane/pkg/reconciler/base/reconciler.go

control-plane/pkg/reconciler/broker/broker.go

slinkydeveloper · 2020-07-09T11:09:07Z

Ok I pushed reviewed while i was still reviewing it, another round coming

slinkydeveloper

Second pass

control-plane/pkg/reconciler/broker/broker.go

control-plane/pkg/reconciler/broker/controller.go

pierDipi · 2020-07-09T13:36:31Z

Maybe I lost it in the process, but before triggering the update with UpdateDispatcherPodsAnnotation do you assert that the data plane config map actually changed?

I'm not keeping track of the previous state (should I?), anyway on the data plane we are idempotent, so nothing breaks.

pierDipi · 2020-07-09T13:55:03Z

Ok, I left unresolved configurations related comments that we can reconcile with eventing-contrib later.

slinkydeveloper · 2020-07-10T07:32:41Z

I'm not keeping track of the previous state (should I?), anyway on the data plane we are idempotent, so nothing breaks.

Sounds good

control-plane/pkg/reconciler/base/reconciler.go

control-plane/pkg/reconciler/broker/broker.go

control-plane/pkg/reconciler/broker/broker_lifecycle.go

control-plane/pkg/reconciler/broker/config.go

control-plane/config/500-controller.yaml

pierDipi · 2020-07-10T09:56:44Z

sonatype breaks every day 😭

https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative-sandbox_eventing-kafka-broker/31/pull-knative-sandbox-eventing-kafka-broker-integration-tests/1281523450262851584#1:build-log.txt%3A1387

https://testgrid.knative.dev/eventing-kafka-broker#continuous

/retest

slinkydeveloper · 2020-07-10T09:57:30Z

@pierDipi I feel you ahahah

pierDipi · 2020-07-10T10:15:53Z

again? 😭

https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative-sandbox_eventing-kafka-broker/31/pull-knative-sandbox-eventing-kafka-broker-integration-tests/1281527853501911040#1:build-log.txt%3A1387

/retest

pierDipi · 2020-07-10T10:41:06Z

Now, it seems to be happy.

Signed-off-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>

pierDipi · 2020-07-10T10:46:41Z

rebased following: #35

knative-metrics-robot · 2020-07-10T10:48:35Z

The following is the coverage report on the affected files.
Say /test pull-knative-sandbox-eventing-kafka-broker-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
control-plane/pkg/reconciler/broker/broker.go	60.0%	88.5%	28.5
control-plane/pkg/reconciler/broker/broker_lifecycle.go	Do not exist	61.1%
control-plane/pkg/reconciler/broker/config.go	Do not exist	100.0%
control-plane/pkg/reconciler/broker/controller.go	77.8%	84.6%	6.8
control-plane/pkg/reconciler/broker/topic.go	Do not exist	100.0%

slinkydeveloper · 2020-07-10T11:02:20Z

/lgtm
/approve

knative-prow-robot · 2020-07-10T11:02:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pierDipi, slinkydeveloper

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

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

[release-v1.0] Reorder scripts

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

knative-prow-robot requested review from matzew and slinkydeveloper July 8, 2020 08:50

knative-prow-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 8, 2020

pierDipi force-pushed the control-plane branch 3 times, most recently from 0df75ca to 9bba2f5 Compare July 8, 2020 12:36

pierDipi changed the title ~~Add Broker reconciler~~ [WIP] Add Broker reconciler Jul 8, 2020

knative-prow-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 8, 2020

pierDipi force-pushed the control-plane branch from 9bba2f5 to 2fc701a Compare July 8, 2020 15:03

pierDipi changed the title ~~[WIP] Add Broker reconciler~~ Add Broker reconciler Jul 8, 2020

knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 8, 2020

slinkydeveloper reviewed Jul 9, 2020

View reviewed changes

pierDipi mentioned this pull request Jul 10, 2020

Change topic prefix from knative- to knative-broker- #32

Closed

slinkydeveloper reviewed Jul 10, 2020

View reviewed changes

pierDipi requested a review from slinkydeveloper July 10, 2020 09:39

slinkydeveloper mentioned this pull request Jul 10, 2020

Auto-update dependencies #34

Closed

slinkydeveloper mentioned this pull request Jul 10, 2020

Bump Go from 1.13 to 1.14 #21

Closed

pierDipi added 3 commits July 10, 2020 12:45

Add Broker reconciler

d9d7c37

Signed-off-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>

Rename data plane config from FORMAT to DATA_PLANE_CONFIG_FORMAT

ccea485

Signed-off-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>

Lowercase constant for data plane config format

e4b3ac6

Signed-off-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>

pierDipi added 10 commits July 10, 2020 12:45

Rename data plane config references

80c0c45

Signed-off-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>

Move topic methods to topic.go and check topic existence

3a274c1

Signed-off-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>

Logging messages start with uppercase letter

316b29f

Signed-off-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>

Move config map update handler to reconciler code

2733d83

Signed-off-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>

Rename p to pod

c4ce400

Signed-off-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>

Use configmap utilities to parse configurations

75bf95a

Signed-off-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>

Add error handling

d910a34

Signed-off-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>

Change to Data plane config map

7b57d1f

Signed-off-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>

Simply cluster admin recreation

09048ea

Signed-off-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>

Uppercased reasons

7c3cc05

Signed-off-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>

pierDipi force-pushed the control-plane branch from 97ffa41 to 7c3cc05 Compare July 10, 2020 10:45

knative-prow-robot assigned slinkydeveloper Jul 10, 2020

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

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

knative-prow-robot merged commit 57a7788 into knative-extensions:master Jul 10, 2020

pierDipi deleted the control-plane branch April 14, 2021 21:15

matzew added a commit to matzew/eventing-kafka-broker that referenced this pull request Nov 12, 2021

Merge pull request knative-extensions#31 from matzew/reorder_scripts

858dda0

[release-v1.0] Reorder scripts

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Broker reconciler #31

Add Broker reconciler #31

pierDipi commented Jul 8, 2020 •

edited

Loading

slinkydeveloper left a comment

slinkydeveloper Jul 9, 2020

slinkydeveloper commented Jul 9, 2020

slinkydeveloper left a comment

pierDipi commented Jul 9, 2020

pierDipi commented Jul 9, 2020

slinkydeveloper commented Jul 10, 2020

pierDipi commented Jul 10, 2020

slinkydeveloper commented Jul 10, 2020

pierDipi commented Jul 10, 2020

pierDipi commented Jul 10, 2020

pierDipi commented Jul 10, 2020

knative-metrics-robot commented Jul 10, 2020

slinkydeveloper commented Jul 10, 2020

knative-prow-robot commented Jul 10, 2020

Add Broker reconciler #31

Add Broker reconciler #31

Conversation

pierDipi commented Jul 8, 2020 • edited Loading

Proposed Changes

slinkydeveloper left a comment

Choose a reason for hiding this comment

slinkydeveloper Jul 9, 2020

Choose a reason for hiding this comment

slinkydeveloper commented Jul 9, 2020

slinkydeveloper left a comment

Choose a reason for hiding this comment

pierDipi commented Jul 9, 2020

pierDipi commented Jul 9, 2020

slinkydeveloper commented Jul 10, 2020

pierDipi commented Jul 10, 2020

slinkydeveloper commented Jul 10, 2020

pierDipi commented Jul 10, 2020

pierDipi commented Jul 10, 2020

pierDipi commented Jul 10, 2020

knative-metrics-robot commented Jul 10, 2020

slinkydeveloper commented Jul 10, 2020

knative-prow-robot commented Jul 10, 2020

pierDipi commented Jul 8, 2020 •

edited

Loading