Add sugar namespace and trigger reconcilers to main eventing controller #6027

xtreme-sameer-vohra · 2022-01-03T01:58:40Z

TODO List

Proposed Changes

At least 80% unit test coverage
E2E tests for any new behavior
Docs PR for any user-facing impact

Release Note

The sugar reconciler has been integrated into the base eventing controller and is now controlled by two LabelSelector fields.
`namespace-sugar-selector` and `trigger-sugar-selector` fields in the `config-sugar` ConfigMap in `knative-eventing` Namespace allow you to use a Kubernetes LabelSelector to choose which
 namespaces or triggers respectively should have a Broker provisioned.

To migrate existing usage of the sugar controller, do the following:

1. Set the namespace-sugar-selector to the value:
    matchExpressions:
    - key: "eventing.knative.dev/injection"
      operator: "In"
      values: ["enabled"]

2. Set the trigger-sugar-selector to the value:
    matchExpressions:
    - key: "eventing.knative.dev/injection"
      operator: "In"
      values: ["enabled"]

3. Remove the Deployment defined by the eventing-sugar-controller.yaml resources
   in the previous release.

Docs

knative-prow-robot · 2022-01-03T01:58:41Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

xtreme-sameer-vohra · 2022-01-11T21:07:49Z

/retest

Testing out new retesting of failed github actions

config/400-config-sugar.yaml

knative-prow-robot · 2022-02-06T02:24:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: xtreme-sameer-vohra

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~~ [xtreme-sameer-vohra]

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

codecov · 2022-02-07T02:16:04Z

Codecov Report

Merging #6027 (40cf2b5) into main (f9498e8) will decrease coverage by 0.14%.
The diff coverage is 62.29%.

@@            Coverage Diff             @@
##             main    #6027      +/-   ##
==========================================
- Coverage   82.13%   81.98%   -0.15%     
==========================================
  Files         231      231              
  Lines        7774     7811      +37     
==========================================
+ Hits         6385     6404      +19     
- Misses        938      956      +18     
  Partials      451      451

Impacted Files	Coverage Δ
pkg/apis/sugar/store.go	`14.28% <14.28%> (ø)`
pkg/reconciler/sugar/namespace/namespace.go	`71.42% <77.77%> (-1.91%)`	⬇️
pkg/reconciler/sugar/trigger/trigger.go	`70.00% <77.77%> (-1.43%)`	⬇️
pkg/apis/sugar/sugar.go	`93.75% <93.75%> (ø)`
pkg/reconciler/sugar/namespace/controller.go	`80.76% <100.00%> (+1.60%)`	⬆️
pkg/reconciler/sugar/trigger/controller.go	`58.62% <100.00%> (+3.06%)`	⬆️
...ciler/apiserversource/resources/receive_adapter.go	`93.87% <0.00%> (ø)`
...er/inmemorychannel/controller/resources/service.go	`100.00% <0.00%> (ø)`
pkg/reconciler/broker/trigger/trigger.go	`84.31% <0.00%> (+0.98%)`	⬆️
pkg/reconciler/broker/trigger/controller.go	`94.23% <0.00%> (+2.33%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f9498e8...40cf2b5. Read the comment docs.

xtreme-sameer-vohra · 2022-03-03T01:26:11Z

/retest

xtreme-sameer-vohra · 2022-03-12T17:30:41Z

@matzew Added you as a reviewer since you have context in this area :)

knative-metrics-robot · 2022-03-12T17:32:29Z

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

File	Old Coverage	New Coverage	Delta
pkg/reconciler/sugar/namespace/controller.go	84.6%	86.7%	2.1
pkg/reconciler/sugar/namespace/namespace.go	84.6%	83.3%	-1.3
pkg/reconciler/sugar/trigger/controller.go	55.0%	59.1%	4.1
pkg/reconciler/sugar/trigger/trigger.go	83.3%	82.4%	-1.0

xtreme-sameer-vohra · 2022-03-24T14:00:27Z

The checks were all passing 12 days ago but are getting stale and failing now 😥

xtreme-sameer-vohra · 2022-03-24T14:00:35Z

/retest

xtreme-sameer-vohra · 2022-03-29T13:37:37Z

/retest

odacremolbap

Sweet, most of my comments are nuances and can be discarded upon your consideration.

cmd/controller/main.go

DEVELOPMENT.md

pkg/apis/sugar/store.go

pkg/apis/sugar/sugar.go

config/core/configmaps/sugar.yaml

- Main eventing-controller includes namespace & trigger sugar reconcilers - Add sugar.yaml configmap to toggle sugar controller behaviour - Add "namespace-sugar-selector" to to allow configuring of sugar behaviour for namespaces - Add "trigger-sugar-selector" to allow configuring of sugar behaviour for triggers - Wire config map into namespace & trigger reconcillers - Update namespace & trigger unit tests

- remove sugar_controller go & yamls - remove "BROKER_INJECTION_DEFAULT" reference - remove sugar filters & labels logic which isn't required anymore

- remove references to sugar that aren't needed

- no longer need to install sugar controller - install test/config/sugar to controller sugar behaviour for testing

- remove broken references to sugar injection key - update tests to reference test/config/sugar.yaml

- sugar controller needs to be enabled for e2e tests that depend on it. This is done in e2e-common.sh for CI but must be done manually for local setups.

- ran ./hack/update-checksum.sh for sugar.yaml

- move configmap properties under _example key to be consistent with other configmaps

- Add config-sugar map to Config Validator Webhook

- NamespaceSugarSelector -> NamespaceSelector - namespace-sugar-selector -> namespace-selector - TriggerSugarSelector -> TriggerSelector - trigger-sugar-selector -> trigger-selector

odacremolbap · 2022-04-03T18:33:04Z

/lgtm
/approve

knative-prow · 2022-04-03T18:33:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: odacremolbap, xtreme-sameer-vohra

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~~ [odacremolbap,xtreme-sameer-vohra]

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

knative-prow-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 3, 2022

lionelvillard reviewed Jan 18, 2022

View reviewed changes

config/400-config-sugar.yaml Outdated Show resolved Hide resolved

xtreme-sameer-vohra force-pushed the 5958-integrate-sugar branch from 4c35842 to 75ff1f1 Compare February 6, 2022 02:24

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

xtreme-sameer-vohra force-pushed the 5958-integrate-sugar branch from 75ff1f1 to 5e13bdb Compare February 6, 2022 02:56

xtreme-sameer-vohra force-pushed the 5958-integrate-sugar branch from c647336 to dca4707 Compare February 7, 2022 02:24

xtreme-sameer-vohra mentioned this pull request Feb 7, 2022

Combine Sugar Controller into main eventing controller #5958

Closed

xtreme-sameer-vohra force-pushed the 5958-integrate-sugar branch from dca4707 to 4365ea0 Compare February 21, 2022 19:44

knative-prow-robot added the area/test-and-release Test infrastructure, tests or release label Feb 21, 2022

xtreme-sameer-vohra force-pushed the 5958-integrate-sugar branch 9 times, most recently from 5635771 to c9d66a2 Compare February 22, 2022 00:26

xtreme-sameer-vohra changed the title ~~WIP Add sugar namespace and trigger reconcilers to main eventing controller~~ Add sugar namespace and trigger reconcilers to main eventing controller Feb 22, 2022

xtreme-sameer-vohra marked this pull request as ready for review February 22, 2022 00:27

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 Feb 22, 2022

devguyio mentioned this pull request Mar 9, 2022

WIP: move the sugar controller to the main controller #6248

Closed

5 tasks

xtreme-sameer-vohra force-pushed the 5958-integrate-sugar branch from e44d676 to c75ff0e Compare March 12, 2022 17:26

xtreme-sameer-vohra requested a review from matzew March 12, 2022 17:30

xtreme-sameer-vohra requested a review from lionelvillard March 12, 2022 17:30

odacremolbap reviewed Mar 30, 2022

View reviewed changes

xtreme-sameer-vohra force-pushed the 5958-integrate-sugar branch 2 times, most recently from c8e51d7 to 0127894 Compare April 1, 2022 00:28

xtreme-sameer-vohra and others added 10 commits March 31, 2022 19:45

Remove standalone sugar controller

e964b45

- remove sugar_controller go & yamls - remove "BROKER_INJECTION_DEFAULT" reference - remove sugar filters & labels logic which isn't required anymore

Update hack/generate-yamls.sh

ea9e125

- remove references to sugar that aren't needed

Update test/e2e-common.sh sugar references

f3dbddd

- no longer need to install sugar controller - install test/config/sugar to controller sugar behaviour for testing

Update e2e tests that depend on sugar controller

712f13e

- remove broken references to sugar injection key - update tests to reference test/config/sugar.yaml

Update DEVELOPMENT.md sugar references

7474b2d

- sugar controller needs to be enabled for e2e tests that depend on it. This is done in e2e-common.sh for CI but must be done manually for local setups.

Update config/core/configmaps/sugar.yaml

ef2f305

- ran ./hack/update-checksum.sh for sugar.yaml

Update config/core/configmaps/sugar.yaml

ce2e32e

- move configmap properties under _example key to be consistent with other configmaps

Update cmd/webhook/main.go

7c6a92a

- Add config-sugar map to Config Validator Webhook

Rename sugar properties

40cf2b5

- NamespaceSugarSelector -> NamespaceSelector - namespace-sugar-selector -> namespace-selector - TriggerSugarSelector -> TriggerSelector - trigger-sugar-selector -> trigger-selector

xtreme-sameer-vohra force-pushed the 5958-integrate-sugar branch from 0127894 to 40cf2b5 Compare April 1, 2022 00:45

knative-prow bot assigned odacremolbap Apr 3, 2022

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Apr 3, 2022

knative-prow bot merged commit 06f57ca into knative:main Apr 3, 2022

xtreme-sameer-vohra mentioned this pull request Apr 4, 2022

Eventing Sugar Controller Integration knative/docs#4896

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add sugar namespace and trigger reconcilers to main eventing controller #6027

Add sugar namespace and trigger reconcilers to main eventing controller #6027

xtreme-sameer-vohra commented Jan 3, 2022 •

edited

Loading

knative-prow-robot commented Jan 3, 2022

xtreme-sameer-vohra commented Jan 11, 2022

knative-prow-robot commented Feb 6, 2022

codecov bot commented Feb 7, 2022 •

edited

Loading

xtreme-sameer-vohra commented Mar 3, 2022

xtreme-sameer-vohra commented Mar 12, 2022

knative-metrics-robot commented Mar 12, 2022

xtreme-sameer-vohra commented Mar 24, 2022

xtreme-sameer-vohra commented Mar 24, 2022

xtreme-sameer-vohra commented Mar 29, 2022

odacremolbap left a comment

odacremolbap commented Apr 3, 2022

knative-prow bot commented Apr 3, 2022

Add sugar namespace and trigger reconcilers to main eventing controller #6027

Add sugar namespace and trigger reconcilers to main eventing controller #6027

Conversation

xtreme-sameer-vohra commented Jan 3, 2022 • edited Loading

Proposed Changes

knative-prow-robot commented Jan 3, 2022

xtreme-sameer-vohra commented Jan 11, 2022

knative-prow-robot commented Feb 6, 2022

codecov bot commented Feb 7, 2022 • edited Loading

Codecov Report

xtreme-sameer-vohra commented Mar 3, 2022

xtreme-sameer-vohra commented Mar 12, 2022

knative-metrics-robot commented Mar 12, 2022

xtreme-sameer-vohra commented Mar 24, 2022

xtreme-sameer-vohra commented Mar 24, 2022

xtreme-sameer-vohra commented Mar 29, 2022

odacremolbap left a comment

Choose a reason for hiding this comment

odacremolbap commented Apr 3, 2022

knative-prow bot commented Apr 3, 2022

xtreme-sameer-vohra commented Jan 3, 2022 •

edited

Loading

codecov bot commented Feb 7, 2022 •

edited

Loading