Skip to content
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

Add ability to filter objects on injection controller promotion #2170

Closed
wants to merge 1 commit into from

Conversation

benmoss
Copy link
Member

@benmoss benmoss commented Jun 24, 2021

Currently we enqueue every object with no way to filter, which causes problems for eventing's source controller which reconciles duck CRDs.

ref: knative/eventing#5543

@google-cla google-cla bot added the cla: yes Indicates the PR's author has signed the CLA. label Jun 24, 2021
@knative-prow-robot knative-prow-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jun 24, 2021
@codecov
Copy link

codecov bot commented Jun 24, 2021

Codecov Report

Merging #2170 (e15d370) into main (a1bc850) will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2170      +/-   ##
==========================================
+ Coverage   65.88%   65.92%   +0.04%     
==========================================
  Files         223      223              
  Lines        9385     9398      +13     
==========================================
+ Hits         6183     6196      +13     
  Misses       2921     2921              
  Partials      281      281              
Impacted Files Coverage Δ
.../injection-gen/generators/reconciler_controller.go 0.00% <ø> (ø)
changeset/commit.go 100.00% <0.00%> (ø)

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 a1bc850...e15d370. Read the comment docs.

Currently we enqueue every object with no way to filter, which causes
problems for eventing's source controller which reconciles duck CRDs.
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: benmoss
To complete the pull request process, please assign mattmoor after the PR has been reviewed.
You can assign the PR to them by writing /assign @mattmoor in a comment when ready.

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

Needs approval from an approver in each of these files:

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

@markusthoemmes
Copy link
Contributor

Great start, but I think it should be somewhat more generic to apply to all sorts of "global" events, i.e. resyncs. This also seems to be quite a bit of overlap with #1909, so maybe get together with @lberk to hash out which way to turn?

Thanks for dealing with this 🎉

@benmoss
Copy link
Member Author

benmoss commented Jun 24, 2021

👍 that PR looks like a better approach, let me follow up with that

@benmoss
Copy link
Member Author

benmoss commented Jul 6, 2021

/open

@benmoss
Copy link
Member Author

benmoss commented Jul 6, 2021

/reopen

@knative-prow-robot
Copy link
Contributor

@benmoss: Failed to re-open PR: state cannot be changed. The filter-promote-func branch was force-pushed or recreated.

In response to this:

/reopen

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Indicates the PR's author has signed the CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants