-
Notifications
You must be signed in to change notification settings - Fork 117
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
Prevent generation only updates #3915
Prevent generation only updates #3915
Conversation
This guards contract ConfigMap updates when only `generation` changes. Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pierDipi 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 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3915 +/- ##
============================================
- Coverage 53.59% 49.24% -4.36%
============================================
Files 344 247 -97
Lines 18346 14925 -3421
Branches 294 0 -294
============================================
- Hits 9833 7350 -2483
+ Misses 7575 6816 -759
+ Partials 938 759 -179
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
BrokerDispatcherPod(env.SystemNamespace, map[string]string{ | ||
base.VolumeGenerationAnnotationKey: "0", | ||
}), |
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.
This is because the test wants to test "no operations" and without an annotation now the control plane will set the annotation on the data plane pod so that the CM and the annotation is fully consistent
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
Putting on hold in case you want some other reviews too Feel free to unhold |
483904b
into
knative-extensions:main
I'm not an expert here but it looks good. |
* Prevent generation only contract updates This guards contract ConfigMap updates when only `generation` changes. Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Refactor to simplify and remove reconciler level change detection Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> --------- Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
* Prevent generation only contract updates This guards contract ConfigMap updates when only `generation` changes. * Refactor to simplify and remove reconciler level change detection --------- Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
This guards and discard contract ConfigMap updates when only
generation
changes.
While this approach is technically slightly slower, the latency is negligible,
and it is more robust and easier to maintain.
Proposed Changes
generation
changes
Release Note
Docs