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.

Sign up for GitHub

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

Jump to bottom

fix(knative): Integration in error state when knative SinkBinding mode=inclusion #4120

Merged
merged 1 commit into from
Mar 14, 2023
Merged

fix(knative): Integration in error state when knative SinkBinding mode=inclusion #4120

merged 1 commit into from
Mar 14, 2023

Conversation

claudio4j
Copy link
Contributor

#4119

Release Note

knative trait sets `bindings.knative.dev/include=true` label to the namespace when there is a SinkBinding.

christophd
christophd reviewed Mar 13, 2023
View reviewed changes
Copy link
Contributor

@christophd christophd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @claudio4j many thanks! I remember that we have already fixed something regarding namespace labelling for Knative sink binding injection in the past. Do you know why this came back as a regression?

e2e/yaks/common/knative-sinkbinding-http/sinkbinding.feature Show resolved Hide resolved
astefanutti
astefanutti requested changes Mar 13, 2023
View reviewed changes
config/rbac/operator-role.yaml Outdated Show resolved Hide resolved
@claudio4j
Copy link
Contributor Author

I remember that we have already fixed something regarding namespace labelling for Knative sink binding injection in the past. Do you know why this came back as a regression?

It was fixed only on 1.8 downstream at the time.

@claudio4j claudio4j force-pushed the fix_ups_knative_sinkbinding branch from f4b8ac5 to 5e19e46 Compare March 13, 2023 13:13
@claudio4j claudio4j force-pushed the fix_ups_knative_sinkbinding branch from 5e19e46 to 0aa2d75 Compare March 13, 2023 13:23
@claudio4j claudio4j force-pushed the fix_ups_knative_sinkbinding branch from 0aa2d75 to 1e7011a Compare March 13, 2023 13:36
@claudio4j
Copy link
Contributor Author

Can we merge it ?

@claudio4j claudio4j merged commit 82aabd5 into apache:main Mar 14, 2023
This was referenced Mar 14, 2023
Merged
Merged
@claudio4j claudio4j deleted the fix_ups_knative_sinkbinding branch March 16, 2023 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christophd christophd christophd left review comments

@squakez squakez squakez approved these changes

@oscerd oscerd oscerd approved these changes

@astefanutti astefanutti astefanutti approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@claudio4j @christophd @astefanutti @oscerd @squakez