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

Cannot use deployment rolling restart API with CRUD mockwebserver #2516

Closed
coopstah13 opened this issue Sep 29, 2020 · 4 comments
Closed

Cannot use deployment rolling restart API with CRUD mockwebserver #2516

coopstah13 opened this issue Sep 29, 2020 · 4 comments

Comments

@coopstah13
Copy link

CRUD mockwebserver attempts to perform a JSON Patch (application/json-patch+json) against the request, but the request is using application/strategic-merge-patch+json which has different semantics for the patching.

To reproduce:

  • Start up a mockwebserver in CRUD mode.
  • Create a deployment
  • Restart deployment using below code
        kubernetesClient
            .apps()
            .deployments()
            .inNamespace(namespace)
            .withName(deploymentName)
            .rolling()
            .restart();

Results in this error in the server:

Sep 29, 2020 1:15:23 PM okhttp3.mockwebserver.MockWebServer$3 execute
SEVERE: MockWebServer[32923] connection from /127.0.0.1 crashed
java.lang.NullPointerException
    at io.fabric8.zjsonpatch.JsonPatch.apply(JsonPatch.java:44)
    at io.fabric8.kubernetes.client.server.mock.KubernetesCrudDispatcher.handlePatch(KubernetesCrudDispatcher.java:156)
    at io.fabric8.kubernetes.client.server.mock.KubernetesCrudDispatcher.dispatch(KubernetesCrudDispatcher.java:72)

Not sure what the exact algorithm would be to do this strategic merge, but seems like you would traverse all the nodes in the patch data and apply their key/value pairs to the same traversed node in the source.

@manusa manusa added the bug label Oct 5, 2020
@manusa
Copy link
Member

manusa commented Oct 5, 2020

Hi @coopstah13
We certainly need to provide different implementations for the handlePatch method in the MockWebServer, since we are already using 2 different PATCH strategies application/json-patch+json & application/strategic-merge-patch+json.

I'm not sure how soon we'll be able to tackle this. Maybe you could provide a PR if you have time ;)

@stale
Copy link

stale bot commented Jan 11, 2021

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

@stale
Copy link

stale bot commented May 6, 2021

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

@stale stale bot added status/stale and removed status/stale labels May 6, 2021
@stale
Copy link

stale bot commented Aug 4, 2021

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants