Skip to content
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

non-optimistic locking updateStatus #3066

Closed
shawkins opened this issue Apr 30, 2021 · 6 comments
Closed

non-optimistic locking updateStatus #3066

shawkins opened this issue Apr 30, 2021 · 6 comments
Milestone

Comments

@shawkins
Copy link
Contributor

updateStatus is implemented as a PUT to /status, which requires the resource to have a resourceVersion specified.

Is there a safe alternative to implementing this with a patch or edit instead?

For example it seems like updateStatus could produce a json patch string and call the new patch(PatchContext patchContext, String patch) method.

This is related to operator-framework/java-operator-sdk#409

Related to this and based upon comments in #2166 #2701 - is there a general issue with the current patch(T item) and edit calls in that they compute the diff based upon the current item compared to the changed item - if there is a resourceVersion specified on the item, shouldn't the diff be generated against that base version instead?

@shawkins
Copy link
Contributor Author

Spawned #3067 to cover the related question on patching behavior.

@manusa
Copy link
Member

manusa commented May 5, 2021

#3078 (comment)

@shawkins
Copy link
Contributor Author

shawkins commented May 8, 2021

Looking at the go client there is both a applyStatus and an updateStatus. Apply is a PATCH, and update is a PUT. I think the resolution here would be to add a similar applyStatus method to fabric8.

For consistency should fabric8 be using both the update and replace terms for a PUT call - that is should it be replaceStatus instead of updateStatus, or update(item) instead of replace(item)?

shawkins added a commit to shawkins/kubernetes-client that referenced this issue May 10, 2021
this consolidated the base patch support logic to a single method, adds
an applyStatus, and refines the patch(base, item) behavior when null
shawkins added a commit to shawkins/kubernetes-client that referenced this issue May 10, 2021
this consolidated the base patch support logic to a single method, adds
an applyStatus, and refines the patch(base, item) behavior when null
@shawkins
Copy link
Contributor Author

With #3070 I'm proposing to add editStatus, patchStatus, and replaceStatus

manusa pushed a commit to shawkins/kubernetes-client that referenced this issue May 12, 2021
this consolidated the base patch support logic to a single method, adds
an applyStatus, and refines the patch(base, item) behavior when null
manusa pushed a commit to shawkins/kubernetes-client that referenced this issue May 13, 2021
this consolidated the base patch support logic to a single method, adds
an applyStatus, and refines the patch(base, item) behavior when null
manusa pushed a commit that referenced this issue May 13, 2021
this consolidated the base patch support logic to a single method, adds
an applyStatus, and refines the patch(base, item) behavior when null
@manusa
Copy link
Member

manusa commented May 13, 2021

I think that #3070 fixes this issue, status should be now modifiable in isolation of the rest of the resource, right?

@shawkins
Copy link
Contributor Author

Yes, this fixed by #3070

@manusa manusa added this to the 5.4.0 milestone May 13, 2021
@manusa manusa closed this as completed May 13, 2021
ddl-audi pushed a commit to ddl-audi/kubernetes-client that referenced this issue May 14, 2021
this consolidated the base patch support logic to a single method, adds
an applyStatus, and refines the patch(base, item) behavior when null
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants