-
Notifications
You must be signed in to change notification settings - Fork 612
Introduce sugar-controller, the reconciler that deals with annotation and label based eventing enablement #3459
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Produced via:
prettier --write --prose-wrap=always $(find -name '*.md' | grep -v vendor | grep -v .github | grep -v docs/cmd/)
config/sugar/README.md
Outdated
The sugar controller is an optional reconciler that watches for labels | ||
and annotations on certain resources to inject eventing components. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Format markdown:
The sugar controller is an optional reconciler that watches for labels | |
and annotations on certain resources to inject eventing components. | |
The sugar controller is an optional reconciler that watches for labels and | |
annotations on certain resources to inject eventing components. |
config/sugar/README.md
Outdated
kubectl annotate trigger mytrigger eventing.knative.dev/injection=enabled | ||
``` | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Format markdown:
Post deployment defaulting to always inject: | ||
|
||
```shell script | ||
kubectl -n knative-eventing set env deployment/sugar-controller BROKER_INJECTION_DEFAULT=true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Format markdown:
kubectl -n knative-eventing set env deployment/sugar-controller BROKER_INJECTION_DEFAULT=true | |
kubectl -n knative-eventing set env deployment/sugar-controller BROKER_INJECTION_DEFAULT=true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Produced via:
gofmt -s -w $(find -path './vendor' -prune -o -path './third_party' -prune -o -name '*.pb.go' -prune -o -type f -name '*.go' -print)
goimports -w $(find -name '*.go' | grep -v vendor | grep -v third_party | grep -v .pb.go | grep -v wire_gen.go)
This pull request introduces 2 alerts when merging dfc3dfd into 17b2978 - view on LGTM.com new alerts:
|
This pull request introduces 2 alerts when merging 3ed614f into 17b2978 - view on LGTM.com new alerts:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Produced via:
gofmt -s -w $(find -path './vendor' -prune -o -path './third_party' -prune -o -name '*.pb.go' -prune -o -type f -name '*.go' -print)
goimports -w $(find -name '*.go' | grep -v vendor | grep -v third_party | grep -v .pb.go | grep -v wire_gen.go)
This pull request introduces 2 alerts when merging 988b727 into a70e05c - view on LGTM.com new alerts:
|
I assume you're going to create a release artifact also? |
Addresses: #3138 |
Maybe modify the CRD shape to default the trigger Status to 'no b0rk3r': |
/retest |
/lgtm |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: n3wscott, vaikas 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:
Approvers can indicate their approval by writing |
The following jobs failed:
Failed non-flaky tests preventing automatic retry of pull-knative-eventing-integration-tests:
|
whoops! labeled the wrong namespace deployment. |
monitoring (elastic search didn't come up), retrying...
/test pull-knative-eventing-integration-tests |
/lgtm |
…espace. sed rename like the other installs.
The following is the coverage report on the affected files.
|
@n3wscott: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
/lgtm |
The current eventing namespace and trigger reconcilers act on labels and annotations in those components to create eventing components in the given namespace for a given broker. The trigger controller was not acting as expected, it would auto-label the namespace because of a long history of the complications needed to install a broker (service account, roles, and role bindings, plus broker). Today we just need the broker object to be created.
We can enable the interactions with these labels as an optional installable controller that enables "sugar" onto the other Kubernetes resources.
Proposed Changes
Release Note
Docs