Create the in-memory-channel ClusterProvisioner by adamharwayne · Pull Request #484 · knative/eventing

adamharwayne · 2018-10-01T22:01:43Z

Note that this PR is dependent on #435, which adds all the sidecar code. Any comments about the sidecar should be made on that PR instead.

Fixes #439, #440

Proposed Changes

Create a new Stub Bus for the new eventing.knative.dev types.
- Separate the Controller and Dispatcher into two distinct Deployments to allow easier horizontal scaling.
  - They communicate via a ConfigMap that the Controller writes and the Dispatcher reads.

Release Note

The new Stub Bus can be used by following the instructions in config/buses/stub2

n3wscott · 2018-10-01T22:07:11Z

I would call this in-memory channel

adamharwayne · 2018-10-02T19:31:43Z

I would call this in-memory channel

Done.

scothis

not done reviewing, but wanted to add comments so far

scothis · 2018-10-03T05:05:40Z

+		return nil
+	}
+
+	err := r.createDispatcherService(ctx, cp)


why dynamically create a service that is effectively a singleton when it could easily be part of the provisioner config yaml instead?

A few revisions back, this controller would work on multiple in-memory ClusterProvisioners (e.g. in-memory-channel-foo, in-memory-channel-bar). In that world it made more sense to create the Service dynamically.

The current revision dropped that ability, so this is vestigial. I'm trying to decide if I want to support multiple provisioners, thoughts? If we end up not wanting to support multiple provisioners this will likely get removed.

I vote remove this controller in favor of installing a service on deployment/install of the provisioner.

I used this commit. And when applying the altered in-memory-channel.yaml, I got:

error: error validating "STDIN": error validating data: ValidationError(Service.metadata.ownerReferences[0]): missing required field "uid" in io.k8s.apimachinery.pkg.apis.meta.v1.OwnerReference; if you choose to ignore these errors, turn validation off with --validate=false 2018/10/11 11:17:28 error executing "kubectl apply": exit status 1

If we want to make the K8s Service unowned, then we can switch to doing it via YAML. I prefer to have the owner reference and thus have the Controller make the K8s Service. Thoughts?

Ping @scothis and @n3wscott

…n PR comments.

grantr · 2018-10-05T19:01:28Z

/cc @n3wscott

n3wscott

Quite a bit of cleanup work to do and I got lost on the multi-channel and swappable handlers. maybe you can give more details on what those are?

n3wscott · 2018-10-11T03:46:34Z

omgomgomgomg I can't wait to see this rebased 💃

adamharwayne · 2018-10-16T16:47:26Z

PTAL, I think I have addressed all the comments.

knative-metrics-robot · 2018-10-18T19:28:46Z

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/controller/eventing/inmemory/channel/controller.go	Do not exist	0.0%
pkg/controller/eventing/inmemory/channel/reconcile.go	Do not exist	98.6%
pkg/controller/eventing/inmemory/clusterprovisioner/controller.go	Do not exist	0.0%
pkg/controller/eventing/inmemory/clusterprovisioner/reconcile.go	Do not exist	100.0%
pkg/controller/eventing/inmemory/controller/main.go	Do not exist	0.0%
pkg/sidecar/configmap/parse.go	100.0%	88.9%	-11.1

vaikas · 2018-10-18T19:54:48Z

/lgtm
/approve

knative-prow-robot · 2018-10-18T19:54:55Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adamharwayne, 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~~ [vaikas-google]

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

n3wscott · 2018-10-18T19:54:58Z

/lgtm

Harwayne added 5 commits September 27, 2018 17:28

Start on the new Stub bus controller and provisioner.

8df08c4

Start on unit tests.

e2c2f1f

Fulfill Sinkable and Subscribable.

b632f71

Merge updates from fanout.

eb2c458

Merge branch 'master' into split-stub

13a912b

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 Oct 1, 2018

knative-prow-robot requested review from grantr and inlined October 1, 2018 22:01

knative-prow-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 1, 2018

adamharwayne mentioned this pull request Oct 2, 2018

Multiple Subscriptions overwrite each other (eventing.knative.dev/v1alpha1) #488

Closed

Rename to in-memory bus.

e87adf0

Add unit tests for the ClusterProvisioner Controller.

a9bb764

scothis reviewed Oct 3, 2018

View reviewed changes

Harwayne added 8 commits October 3, 2018 14:11

Add unit tests for the Channel reconciler.

10034f6

Rename in-memory-bus-provisioner to in-memory-channel, as requested i…

906e8f9

…n PR comments.

Add a README.md.

437d2df

Typo.

8dbaeee

Use a service account with specific RBAC permissions.

9c1654f

hack/update-deps.sh

c1d3082

Merge branch 'master' into split-stub

99bc3e4

isio -> istio

9e014e9

knative-prow-robot requested a review from n3wscott October 5, 2018 19:01

adamharwayne changed the title ~~[WIP] Create the eventing.knative.dev Stub Bus~~ Create the in-memory-channel ClusterProvisioner Oct 5, 2018

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 Oct 5, 2018

n3wscott reviewed Oct 8, 2018

View reviewed changes

n3wscott mentioned this pull request Oct 8, 2018

Create a Sidecar for fanning out HTTP requests #435

Merged

neosab reviewed Oct 9, 2018

View reviewed changes

Comment thread pkg/controller/eventing/inmemory/clusterprovisioner/reconcile.go Outdated

Harwayne added 3 commits October 9, 2018 15:36

Respond to PR comments.

4a8f264

ConfigMap create ClusterRole cannot be bound by resourceName.

af00150

Watch K8s Services and VirtualServices.

1d73bba

Merge branch 'master' into split-stub

fd0efd4

adamharwayne force-pushed the split-stub branch from e010d6d to fd0efd4 Compare October 11, 2018 17:29

Harwayne added 7 commits October 11, 2018 11:00

Re-add SerializeConfig (accidentally removed during last merge).

a3aaf5a

goimports

0a75ae7

Collapse if conditions.

e955a14

Only reconcile Channels.

c79aa43

Merge branch 'master' into split-stub

73f94b9

Manipulate Status only via methods on the Status object.

d32d44d

Don't rely on the default values for sidecar flags.

486a2ba

neosab mentioned this pull request Oct 15, 2018

Adds Kafka Channel Provisioner Controllers #468

Merged

adamharwayne mentioned this pull request Oct 15, 2018

[WIP] In-memory Channels without Istio sidecar #524

Closed

Merge branch 'master' into split-stub

b2438e1

knative-prow-robot assigned vaikas Oct 18, 2018

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 18, 2018

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 18, 2018

knative-prow-robot assigned n3wscott Oct 18, 2018

knative-prow-robot merged commit 15b778c into knative:master Oct 18, 2018

evankanderson mentioned this pull request Oct 23, 2018

Round 1: In-Memory Channel Controller #440

Closed

n3wscott mentioned this pull request Oct 24, 2018

adding n3wscott to approvers. #465

Merged

Harwayne deleted the split-stub branch January 29, 2019 22:47

aliok pushed a commit to aliok/eventing that referenced this pull request Mar 23, 2020

🤖 CI was here...🔥 (knative#484)

a7dad99

Conversation

adamharwayne commented Oct 1, 2018

Proposed Changes

Uh oh!

n3wscott commented Oct 1, 2018

Uh oh!

adamharwayne commented Oct 2, 2018

Uh oh!

scothis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

scothis Oct 3, 2018

Choose a reason for hiding this comment

Uh oh!

adamharwayne Oct 3, 2018

Choose a reason for hiding this comment

Uh oh!

n3wscott Oct 8, 2018

Choose a reason for hiding this comment

Uh oh!

adamharwayne Oct 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamharwayne Oct 12, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

grantr commented Oct 5, 2018

Uh oh!

n3wscott left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

n3wscott commented Oct 11, 2018

Uh oh!

adamharwayne commented Oct 16, 2018

Uh oh!

knative-metrics-robot commented Oct 18, 2018

Uh oh!

vaikas commented Oct 18, 2018

Uh oh!

knative-prow-robot commented Oct 18, 2018

Uh oh!

n3wscott commented Oct 18, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

adamharwayne Oct 11, 2018 •

edited

Loading