Broker controller new infrastructure by nachocano · Pull Request #1073 · knative/eventing

nachocano · 2019-04-19T22:34:38Z

Contributes to #734

Proposed Changes

Changing Broker controller to match Kn Serving infra

Release Note

NONE

…o broker-controller

vaikas

Just curious about a few things and lots of commented out code :)
Oh, and needs a rebase... :(

# Conflicts: # cmd/controller/main.go # pkg/reconciler/testing/listers.go # pkg/reconciler/v1alpha1/trigger/trigger.go # pkg/reconciler/v1alpha1/trigger/trigger_test.go

…o broker-controller

nachocano · 2019-04-24T19:14:18Z

There is something weird going on with the broker integration test, that I may have messed up. It doesn't report to be ready and it hangs there forever until timeouts.
The weird thing is that it was passing, I made changes only to UTs, and then stopped passing in some of the previous commits.
Will dig in this further...

nachocano · 2019-04-24T19:30:33Z

/cc @Harwayne can you take a look, especially the broker reconciler. I may have mess up something there as it is not becoming ready in the integration test...
It was passing, I added only few UTs and it started failing, and then did some merges (and keeps on failing)...

nachocano · 2019-04-24T19:34:06Z

current error:

I0424 19:26:48.390] --- FAIL: TestMain/TestDefaultBrokerWithManyTriggers-in-memory (241.10s)
I0424 19:26:48.393] e2e.go:459: Creating Namespace: test-default-broker-with-many-triggers-in-memory
I0424 19:26:48.393] broker_trigger_test.go:63: Labeling namespace test-default-broker-with-many-triggers-in-memory
I0424 19:26:48.393] broker_trigger_test.go:69: Namespace test-default-broker-with-many-triggers-in-memory annotated
I0424 19:26:48.393] broker_trigger_test.go:72: Waiting for default broker to be ready
I0424 19:26:48.393] broker_trigger_test.go:76: Error waiting for default broker to become ready: timed out waiting for the condition

n3wscott · 2019-04-24T19:40:56Z

/test pull-knative-eventing-integration-tests

nachocano · 2019-04-24T22:54:53Z

fixed the readiness stuff...

/test pull-knative-eventing-integration-tests

n3wscott · 2019-04-25T15:20:15Z

/approve
I will let Adam lgtm

vaikas · 2019-04-25T16:31:30Z

looks like just needs a rebase?
/approve

knative-prow-robot · 2019-04-25T16:31:40Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: n3wscott, nachocano, vaikas-google

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [n3wscott,vaikas-google]

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

nachocano · 2019-04-25T16:45:17Z

looks like just needs a rebase?

yes, it was a rebase

nachocano · 2019-04-25T16:49:08Z

@n3wscott @vaikas-google @Harwayne
If the test passes, it'd be great if someone can ltgtm it, otherwise I keep on having to rebase all the time.
The only cosmetic thing is a comment from Adam where I'm passing a string with the brokerName to a function instead of a Broker pointer. I guess we can change that back later on if you guys prefer the Broker pointer way.

vaikas · 2019-04-25T16:55:01Z

/lgtm

knative-metrics-robot · 2019-04-25T16:55:41Z

The following is the coverage report on pkg/.
Say /test pull-knative-eventing-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/subscription/subscription.go	Do not exist	80.6%
pkg/reconciler/trigger/trigger.go	Do not exist	78.6%
pkg/reconciler/v1alpha1/broker/broker.go	89.8%	88.1%	-1.7

nachocano added 4 commits April 19, 2019 14:01

first pass to update broker controller

7ee77fe

starting informers properly, listening to own resources

f54fe07

fix and commenting out UTs

68bb41d

space

20d4e7d

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 Apr 19, 2019

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Apr 19, 2019

knative-prow-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 19, 2019

knative-prow-robot requested review from evankanderson and vaikas April 19, 2019 22:34

nachocano added 2 commits April 22, 2019 10:05

adding listers

4f66cf7

Merge remote-tracking branch 'upstream/master' into broker-controller

8bc4c87

n3wscott mentioned this pull request Apr 22, 2019

Unify the controller infrastructures across various Knative components #734

Closed

nachocano added 11 commits April 22, 2019 16:32

updating tests

acf45b7

Merge remote-tracking branch 'upstream/master' into broker-controller

fd0447a

more tests

eac14f9

adding deployment functional

9400105

compiling

4e692f0

more UTs

56d5efa

more UTs

62a813a

Merge branch 'broker-controller' of github.com:nachocano/eventing int…

17d15ea

…o broker-controller

more UTs

592653e

more UTs

8bf83e9

more UTs

53ef443

vaikas reviewed Apr 24, 2019

View reviewed changes

nachocano added 5 commits April 23, 2019 18:55

Merge remote-tracking branch 'upstream/master' into broker-controller

147c800

# Conflicts: # cmd/controller/main.go # pkg/reconciler/testing/listers.go # pkg/reconciler/v1alpha1/trigger/trigger.go # pkg/reconciler/v1alpha1/trigger/trigger_test.go

updating deps

7ecd402

updating UTs

f859c51

updating UTs

ef78bcb

Merge remote-tracking branch 'upstream/master' into broker-controller

b98fac2

nachocano changed the title ~~[WIP] Broker controller new infrastructure~~ Broker controller new infrastructure Apr 24, 2019

n3wscott reviewed Apr 24, 2019

View reviewed changes

Comment thread pkg/reconciler/v1alpha1/broker/broker_test.go Outdated

nachocano added 3 commits April 24, 2019 12:01

update after comment

da6314d

review comments

d95ea3e

Merge branch 'broker-controller' of github.com:nachocano/eventing int…

0b6f0bc

…o broker-controller

n3wscott reviewed Apr 24, 2019

View reviewed changes

Comment thread cmd/controller/main.go

nachocano added 3 commits April 24, 2019 14:40

serviceInformer and namespaceInformer

da24fe0

attempt to fix

4e3ada7

solved

4c8c11a

Harwayne reviewed Apr 24, 2019

View reviewed changes

Updates after code review

6f39fb1

n3wscott reviewed Apr 25, 2019

View reviewed changes

Comment thread pkg/reconciler/subscription/subscription_test.go

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 25, 2019

vaikas reviewed Apr 25, 2019

View reviewed changes

Comment thread pkg/reconciler/trigger/trigger_test.go Outdated

nachocano added 2 commits April 25, 2019 09:40

rebasing

3dcf1a5

Updating gopkg.lock

768afe9

knative-prow-robot assigned vaikas Apr 25, 2019

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 25, 2019

knative-prow-robot merged commit 2a3b8a2 into knative:master Apr 25, 2019

Conversation

nachocano commented Apr 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Uh oh!

vaikas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nachocano commented Apr 24, 2019

Uh oh!

nachocano commented Apr 24, 2019

Uh oh!

nachocano commented Apr 24, 2019

Uh oh!

Uh oh!

n3wscott commented Apr 24, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nachocano commented Apr 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

n3wscott commented Apr 25, 2019

Uh oh!

Uh oh!

vaikas commented Apr 25, 2019

Uh oh!

knative-prow-robot commented Apr 25, 2019

Uh oh!

nachocano commented Apr 25, 2019

Uh oh!

nachocano commented Apr 25, 2019

Uh oh!

vaikas commented Apr 25, 2019

Uh oh!

knative-metrics-robot commented Apr 25, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

nachocano commented Apr 19, 2019 •

edited

Loading

nachocano commented Apr 24, 2019 •

edited

Loading