-
Notifications
You must be signed in to change notification settings - Fork 398
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
virtual: Support SSA patch requests for non-existent objects #1854
Conversation
/assign @davidfestal |
Added unit and e2e tests. Also added support for the |
@sttts @davidfestal PTAL. |
// for non-existent objects can still be processed. | ||
if !apierrors.IsNotFound(err) || | ||
!forceAllowCreate && | ||
!(requestInfo != nil && requestInfo.Verb == "patch") { |
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.
What about patch
requests which are not SSA requests ?
Will they try to create it as well ?
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.
For JSON or SMP patch requests, the corresponding patcher implementation returns a errors.NewNotFound
error, so the response code is 404 as expected. The patchers execute as part of the objInfo.UpdatedObject(ctx, oldObj)
call, and their implementations can be found at https://github.com/kubernetes/apiserver/blob/48405445f09218bb8b7a8a81eb21027cfd65761b/pkg/endpoints/handlers/patch.go, with the errors at:
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.
Thanks for the explanation.
Do you think you could add a comment in the code to explain this ? It's not easy to think of or guess when reading.
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 |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sttts 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 |
Summary
This PR switches update operations made to the virtual workspace API server, to calling create operations on the forwarding API server, for non-existent objects. This enables support for server-side apply patch requests in that case.
Related issue(s)
Fixes #1852