kubernetes-sigs
🐛 Inc `Generation` on `fakeClient` `Update`/`Patch`
Closes #1857.
This applies only to custom resources!
The behaviour of the API server regarding generation is polymorphic meaning that any resource type may behave differently. Custom resources have a uniform behaviour (increment on non-metadata change), and so this patch adds logic to fakeClient to handle this specific use case.
Hi @danaradg. Thanks for your PR.
I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
I understand the commands that are listed here.
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.
Tagging @FillZpp, @alvaroaleman as well as they corresponded with me on #1857.
Generally LG, could you please add test cases for structured, unstructured and metadata objects?
Generally LG, could you please add test cases for structured, unstructured and metadata objects?
Can you clarify? The Update method receives a runtime.Object and converts it to unstructured behind the scenes to check equality.
I don't understand the test cases that you want me to add.
Isn't structured object the same as what I'm already testing (a dummy custom resource)?
Unstructured objects are not runtime.Objects. I could make an object that implements both, but from the tested code's viewpoint it would be the same as passing any other runtime.Object.
What does a metadata object mean? metav1.ObjectMeta? It doesn't implement runtime.Object.
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: danaradg
Once this PR has been reviewed and has the lgtm label, please assign droot for approval by writing /assign @droot in a comment. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Hey @FillZpp. Stull waiting for an answer about the tests. Would very much like to merge the PR.
Can you clarify? The Update method receives a runtime.Object and converts it to unstructured behind the scenes to check equality.
Oh, I mean the runtime.Object could be a typed CR object or an unstructured CR object. The later one should also have a test case.
Newbie question: How do I get the approved label? Is there an approver that goes over CLs with the lgtm label or do I need to contact one?
This is a breaking change, existing tests might start failing now /retitle :warning: Inc Generation on fakeClient Update/Patch
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: danaradg
Once this PR has been reviewed and has the lgtm label, please ask for approval from fillzpp by writing /assign @fillzpp in a comment. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
PR needs rebase.
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.
Hi,
I just found that issue. Personally I think it would be very beneficial to have this working for custom resources, as it's not obvious, that this behavior is different in fake client and in the real life.
What's the status of it? What should happen to move this forward? Is there anything I can do to help?
Thanks!
I'll close this PR given no recent activity.
@konkit If you're still interested, feel free to open a new PR to continue this work
/close
@sbueringer: Closed this PR.
In response to this:
I'll close this PR given no recent activity.
@konkit If you're still interested, feel free to open a new PR to continue this work
/close
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.