-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 returning object after status update #2489
Conversation
@alvaroaleman: GitHub didn't allow me to request PR reviews from the following users: berlin-ab. Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs. 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. |
@alvaroaleman this makes sense and also behaves as expected. |
Very nice. Thanks.
Again, thanks for maintaining this tool that I use heavily daily.
…On Sun, Sep 10, 2023 at 4:02 PM Alvaro Aleman ***@***.***> wrote:
Prior to this, we overwrote the passed in pointer to obj on status
update, which means that the caller can never see the updated object after
the call. This changes makes us instead overwrite the value behind the
pointer, so that it gets propagated back to the caller.
@berlin-ab <https://github.com/berlin-ab> I like this approach more, as
it seems a bit simpler and keeps the changed contained to the place that
causes the need for them. WDYT?
/cc @berlin-ab <https://github.com/berlin-ab> @troy0820
<https://github.com/troy0820>
Superseeds #2486
<#2486>
Fixes #2487
<#2487>
------------------------------
You can view, comment on, or merge this pull request online at:
#2489
Commit Summary
- 8f0fe71
<8f0fe71>
Draft: Test that an object is updatable after updating its status.
- c527239
<c527239>
Draft: ensure we udpate the new obj's accessor's ResourceVersion after
doing the deep copy
- 8ac8d8e
<8ac8d8e>
Rework code to pass object back on status update
File Changes
(3 files
<https://github.com/kubernetes-sigs/controller-runtime/pull/2489/files>)
- *M* pkg/client/fake/client.go
<https://github.com/kubernetes-sigs/controller-runtime/pull/2489/files#diff-20ecedbf30721c01c33fb67d911da11c277e29990497a600d20cb0ec7215affd>
(4)
- *M* pkg/client/fake/client_test.go
<https://github.com/kubernetes-sigs/controller-runtime/pull/2489/files#diff-b461e25818820a3ee1743111fb4376995e7184c87e18f7c033799eb5ac09f908>
(78)
- *M* pkg/client/interfaces.go
<https://github.com/kubernetes-sigs/controller-runtime/pull/2489/files#diff-7b1a875f6fde4cf8525a3eb20e91841bfcf3284928dc64b47a971777f7d9ff90>
(1)
Patch Links:
- https://github.com/kubernetes-sigs/controller-runtime/pull/2489.patch
- https://github.com/kubernetes-sigs/controller-runtime/pull/2489.diff
—
Reply to this email directly, view it on GitHub
<#2489>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAVLCAX3WPKDL7RZWCWN7TXZYMF7ANCNFSM6AAAAAA4SMJOFU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
/cherrypick release-0.16 |
@alvaroaleman: once the present PR merges, I will cherry-pick it on top of release-0.16 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. |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alvaroaleman, troy0820 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 |
@alvaroaleman: new pull request created: #2490 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. |
Prior to this, we overwrote the passed in pointer to
obj
on status update, which means that the caller can never see the updated object after the call. This changes makes us instead overwrite the value behind the pointer, so that it gets propagated back to the caller.@berlin-ab I like this approach more, as it seems a bit simpler and keeps the changed contained to the place that causes the need for them. WDYT?
/cc @berlin-ab @troy0820
Superseeds #2486
Fixes #2487