-
Notifications
You must be signed in to change notification settings - Fork 115
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
fix: triggers use correct consumer group id generated from template #3779
fix: triggers use correct consumer group id generated from template #3779
Conversation
Signed-off-by: Calum Murray <cmurray@redhat.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3779 +/- ##
============================================
+ Coverage 48.39% 52.48% +4.09%
- Complexity 0 877 +877
============================================
Files 243 343 +100
Lines 18044 21466 +3422
Branches 0 288 +288
============================================
+ Hits 8732 11267 +2535
- Misses 8574 9288 +714
- Partials 738 911 +173
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Calum Murray <cmurray@redhat.com>
…eatures cm Signed-off-by: Calum Murray <cmurray@redhat.com>
/cc @maschmid |
/cherry-pick release-1.13 |
@Cali0707: once the present PR merges, I will cherry-pick it on top of release-1.13 in a new PR and assign it to you. In response to this:
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. |
/cherry-pick release-1.12 |
@Cali0707: once the present PR merges, I will cherry-pick it on top of release-1.12 in a new PR and assign it to you. In response to this:
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. |
Signed-off-by: Calum Murray <cmurray@redhat.com>
} | ||
} | ||
|
||
func WithTriggerConsumerGroupIDTemplate(template string) manifest.CfgFn { |
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.
👍
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.
/lgtm
/approve
/retest
Looks like that may have been a real failure /cc @matzew |
43a014a
to
57a7e44
Compare
Signed-off-by: Calum Murray <cmurray@redhat.com>
57a7e44
to
d78a6f7
Compare
…features cm after the test Signed-off-by: Calum Murray <cmurray@redhat.com>
/retest-required |
/retest |
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.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Cali0707, matzew 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 |
bf38772
into
knative-extensions:main
@Cali0707: #3779 failed to apply on top of branch "release-1.13":
In response to this:
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. |
@Cali0707: #3779 failed to apply on top of branch "release-1.12":
In response to this:
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. |
…native-extensions#3779) * add rekt test to catch trigger cg being wrong Signed-off-by: Calum Murray <cmurray@redhat.com> * fix cm key name Signed-off-by: Calum Murray <cmurray@redhat.com> * fix: trigger controller now watches for changes to the config-kafka-features cm Signed-off-by: Calum Murray <cmurray@redhat.com> * fixed unit test Signed-off-by: Calum Murray <cmurray@redhat.com> * also watch cm for namespaced triggers Signed-off-by: Calum Murray <cmurray@redhat.com> * goimports Signed-off-by: Calum Murray <cmurray@redhat.com> * don't use manifest.InstallYamlFS because it deletes the config-kafka-features cm after the test Signed-off-by: Calum Murray <cmurray@redhat.com> --------- Signed-off-by: Calum Murray <cmurray@redhat.com>
… from template (knative-extensions#3779) (#1038) * fix: triggers use correct consumer group id generated from template (knative-extensions#3779) * add rekt test to catch trigger cg being wrong Signed-off-by: Calum Murray <cmurray@redhat.com> * fix cm key name Signed-off-by: Calum Murray <cmurray@redhat.com> * fix: trigger controller now watches for changes to the config-kafka-features cm Signed-off-by: Calum Murray <cmurray@redhat.com> * fixed unit test Signed-off-by: Calum Murray <cmurray@redhat.com> * also watch cm for namespaced triggers Signed-off-by: Calum Murray <cmurray@redhat.com> * goimports Signed-off-by: Calum Murray <cmurray@redhat.com> * don't use manifest.InstallYamlFS because it deletes the config-kafka-features cm after the test Signed-off-by: Calum Murray <cmurray@redhat.com> --------- Signed-off-by: Calum Murray <cmurray@redhat.com> * cleanup: consumergroup template test does not modify configmap now (knative-extensions#3782) * cleanup: consumergroup template test does not modify configmap now Signed-off-by: Calum Murray <cmurray@redhat.com> * account for different possible cm keys Signed-off-by: Calum Murray <cmurray@redhat.com> --------- Signed-off-by: Calum Murray <cmurray@redhat.com> * update configmap in CI to test non default values Signed-off-by: Calum Murray <cmurray@redhat.com> * Update openshift/e2e-common.sh * clone correct SO branch Signed-off-by: Calum Murray <cmurray@redhat.com> --------- Signed-off-by: Calum Murray <cmurray@redhat.com> Co-authored-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>
… from template (knative-extensions#3779) (knative-extensions#1038) * fix: triggers use correct consumer group id generated from template (knative-extensions#3779) * add rekt test to catch trigger cg being wrong Signed-off-by: Calum Murray <cmurray@redhat.com> * fix cm key name Signed-off-by: Calum Murray <cmurray@redhat.com> * fix: trigger controller now watches for changes to the config-kafka-features cm Signed-off-by: Calum Murray <cmurray@redhat.com> * fixed unit test Signed-off-by: Calum Murray <cmurray@redhat.com> * also watch cm for namespaced triggers Signed-off-by: Calum Murray <cmurray@redhat.com> * goimports Signed-off-by: Calum Murray <cmurray@redhat.com> * don't use manifest.InstallYamlFS because it deletes the config-kafka-features cm after the test Signed-off-by: Calum Murray <cmurray@redhat.com> --------- Signed-off-by: Calum Murray <cmurray@redhat.com> * cleanup: consumergroup template test does not modify configmap now (knative-extensions#3782) * cleanup: consumergroup template test does not modify configmap now Signed-off-by: Calum Murray <cmurray@redhat.com> * account for different possible cm keys Signed-off-by: Calum Murray <cmurray@redhat.com> --------- Signed-off-by: Calum Murray <cmurray@redhat.com> * update configmap in CI to test non default values Signed-off-by: Calum Murray <cmurray@redhat.com> * Update openshift/e2e-common.sh * clone correct SO branch Signed-off-by: Calum Murray <cmurray@redhat.com> --------- Signed-off-by: Calum Murray <cmurray@redhat.com> Co-authored-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>
/cherry-pick release-1.13 |
/cherry-pick release-1.12 |
@Cali0707: #3779 failed to apply on top of branch "release-1.13":
In response to this:
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. |
@Cali0707: #3779 failed to apply on top of branch "release-1.12":
In response to this:
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. |
…native-extensions#3779) * add rekt test to catch trigger cg being wrong Signed-off-by: Calum Murray <cmurray@redhat.com> * fix cm key name Signed-off-by: Calum Murray <cmurray@redhat.com> * fix: trigger controller now watches for changes to the config-kafka-features cm Signed-off-by: Calum Murray <cmurray@redhat.com> * fixed unit test Signed-off-by: Calum Murray <cmurray@redhat.com> * also watch cm for namespaced triggers Signed-off-by: Calum Murray <cmurray@redhat.com> * goimports Signed-off-by: Calum Murray <cmurray@redhat.com> * don't use manifest.InstallYamlFS because it deletes the config-kafka-features cm after the test Signed-off-by: Calum Murray <cmurray@redhat.com> --------- Signed-off-by: Calum Murray <cmurray@redhat.com>
…native-extensions#3779) * add rekt test to catch trigger cg being wrong Signed-off-by: Calum Murray <cmurray@redhat.com> * fix cm key name Signed-off-by: Calum Murray <cmurray@redhat.com> * fix: trigger controller now watches for changes to the config-kafka-features cm Signed-off-by: Calum Murray <cmurray@redhat.com> * fixed unit test Signed-off-by: Calum Murray <cmurray@redhat.com> * also watch cm for namespaced triggers Signed-off-by: Calum Murray <cmurray@redhat.com> * goimports Signed-off-by: Calum Murray <cmurray@redhat.com> * don't use manifest.InstallYamlFS because it deletes the config-kafka-features cm after the test Signed-off-by: Calum Murray <cmurray@redhat.com> --------- Signed-off-by: Calum Murray <cmurray@redhat.com>
…from template (#3779) (#3800) * fix: triggers use correct consumer group id generated from template (#3779) * add rekt test to catch trigger cg being wrong Signed-off-by: Calum Murray <cmurray@redhat.com> * fix cm key name Signed-off-by: Calum Murray <cmurray@redhat.com> * fix: trigger controller now watches for changes to the config-kafka-features cm Signed-off-by: Calum Murray <cmurray@redhat.com> * fixed unit test Signed-off-by: Calum Murray <cmurray@redhat.com> * also watch cm for namespaced triggers Signed-off-by: Calum Murray <cmurray@redhat.com> * goimports Signed-off-by: Calum Murray <cmurray@redhat.com> * don't use manifest.InstallYamlFS because it deletes the config-kafka-features cm after the test Signed-off-by: Calum Murray <cmurray@redhat.com> --------- Signed-off-by: Calum Murray <cmurray@redhat.com> * cleanup: consumergroup template test does not modify configmap now (#3782) * cleanup: consumergroup template test does not modify configmap now Signed-off-by: Calum Murray <cmurray@redhat.com> * account for different possible cm keys Signed-off-by: Calum Murray <cmurray@redhat.com> --------- Signed-off-by: Calum Murray <cmurray@redhat.com> --------- Signed-off-by: Calum Murray <cmurray@redhat.com>
…from template (#3779) (#3801) * fix: triggers use correct consumer group id generated from template (#3779) * add rekt test to catch trigger cg being wrong Signed-off-by: Calum Murray <cmurray@redhat.com> * fix cm key name Signed-off-by: Calum Murray <cmurray@redhat.com> * fix: trigger controller now watches for changes to the config-kafka-features cm Signed-off-by: Calum Murray <cmurray@redhat.com> * fixed unit test Signed-off-by: Calum Murray <cmurray@redhat.com> * also watch cm for namespaced triggers Signed-off-by: Calum Murray <cmurray@redhat.com> * goimports Signed-off-by: Calum Murray <cmurray@redhat.com> * don't use manifest.InstallYamlFS because it deletes the config-kafka-features cm after the test Signed-off-by: Calum Murray <cmurray@redhat.com> --------- Signed-off-by: Calum Murray <cmurray@redhat.com> * cleanup: consumergroup template test does not modify configmap now (#3782) * cleanup: consumergroup template test does not modify configmap now Signed-off-by: Calum Murray <cmurray@redhat.com> * account for different possible cm keys Signed-off-by: Calum Murray <cmurray@redhat.com> --------- Signed-off-by: Calum Murray <cmurray@redhat.com> --------- Signed-off-by: Calum Murray <cmurray@redhat.com>
… from template (knative-extensions#3779) (#1038) (#1041) * fix: triggers use correct consumer group id generated from template (knative-extensions#3779) * add rekt test to catch trigger cg being wrong * fix cm key name * fix: trigger controller now watches for changes to the config-kafka-features cm * fixed unit test * also watch cm for namespaced triggers * goimports * don't use manifest.InstallYamlFS because it deletes the config-kafka-features cm after the test --------- * cleanup: consumergroup template test does not modify configmap now (knative-extensions#3782) * cleanup: consumergroup template test does not modify configmap now * account for different possible cm keys --------- * update configmap in CI to test non default values * Update openshift/e2e-common.sh * clone correct SO branch --------- Signed-off-by: Calum Murray <cmurray@redhat.com> Co-authored-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com> Co-authored-by: Pierangelo Di Pilato <pierdipi@redhat.com>
Currently, the triggers will always use a consumer group id generated from the default template. Instead, they should use the most up to date version of the template.
Proposed Changes
Release Note
Docs