Patch passes on dry-run and fails for real

[thockin]

Running a relatively recent build (a few weeks old, maybe a month).

$ k patch svc hostnames -p '{"metatdata":{"annotations":{"new":"value"}}}' --dry-run
service/hostnames patched (dry run)

$ k patch svc hostnames -p '{"metatdata":{"annotations":{"new":"value"}}}'
service/hostnames not patched

Running with --v=10 ends like this:

I0813 10:13:16.280688  251075 request.go:897] Request Body: {"metatdata":{"annotations":{"new":"value"}}}
I0813 10:13:16.280786  251075 round_trippers.go:386] curl -k -v -XPATCH  -H "Accept: application/json" -H "Content-Type: application/strategic-merge-patch+json" -H "User-Agent: kubectl/v1.12.0 (linux/amd64) kubernetes/1d2939e" -H "Authorization: Bearer mUOKBgOtYRhd92TzZMVcDJscuBzcwGs2" 'https://35.232.5.185/api/v1/namespaces/default/services/hostnames'
I0813 10:13:16.322213  251075 round_trippers.go:405] PATCH https://35.232.5.185/api/v1/namespaces/default/services/hostnames 200 OK in 41 milliseconds
I0813 10:13:16.322248  251075 round_trippers.go:411] Response Headers:
I0813 10:13:16.322257  251075 round_trippers.go:414]     Date: Mon, 13 Aug 2018 17:13:16 GMT
I0813 10:13:16.322265  251075 round_trippers.go:414]     Content-Type: application/json
I0813 10:13:16.322272  251075 round_trippers.go:414]     Content-Length: 573
I0813 10:13:16.322311  251075 request.go:897] Response Body: {"kind":"Service","apiVersion":"v1","metadata":{"name":"hostnames","namespace":"default","selfLink":"/api/v1/namespaces/default/services/hostnames","uid":"4e3b9ed9-7bc7-11e8-ad42-42010af00002","resourceVersion":"10119120","creationTimestamp":"2018-06-29T18:07:37Z","labels":{"run":"hostnames"},"annotations":{"old":"value"}},"spec":{"ports":[{"protocol":"TCP","port":80,"targetPort":9376,"nodePort":30828}],"selector":{"run":"hostnames"},"clusterIP":"10.0.212.228","type":"NodePort","sessionAffinity":"None","externalTrafficPolicy":"Cluster"},"status":{"loadBalancer":{}}}
service/hostnames not patched
F0813 10:13:16.322550  251075 helpers.go:119] 

[liggitt]

The "not patched" text is printed if the resulting object is the same as the initial object.

Since your patch had a typo (metatdata != metadata), the patch didn't affect any fields in the object, and the resulting object was the same as the starting object.

There are a couple things there:

• saying "not patched" when a patch was submitted/accepted is misleading when it really means "the patch was a no-op"
• the server doesn't currently detect/report field typos

[thockin]

Maybe the real issue is that dry run said it was patched...

…

On Mon, Aug 13, 2018, 1:02 PM Jordan Liggitt ***@***.***> wrote: The "not patched" text is printed if the resulting object is the same as the initial object. Since your patch had a typo (metatdata != metadata), the patch didn't affect any fields in the object, and the resulting object was the same as the starting object. There are a couple things there: - saying "not patched" when a patch was submitted/accepted is misleading when it really means "the patch was a no-op" - the server doesn't currently detect/report field typos — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#67344 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFVgVJzZHQ5Cgrfvzn6PH_LbPyhrZxK4ks5uQdtcgaJpZM4V65op> .

[liggitt]

yeah... in dry-run "patched" means "a patch was (would have been) submitted to the server". in non-dry-run "patched" means "a patch was submitted to the server and changed the object"

[thockin]

Can we do better? Without running apiserver code locally, it can not be 100%, but ... Otherwise let's close this (and thanks for seeing my typo :)

…

On Mon, Aug 13, 2018 at 1:33 PM Jordan Liggitt ***@***.***> wrote: yeah... in dry-run "patched" means "a patch was (would have been) submitted to the server". in non-dry-run "patched" means "a patch was submitted to the server and changed the object" — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#67344 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFVgVFBP4I83sbcC0JUCqgb-Mq4eP_T6ks5uQeKlgaJpZM4V65op> .

[liggitt]

client-side patch is pretty dumb (in a good way)... just passes the input patch through to the server. I'd rather not expand attempts at client-side application.

[liggitt]

I'd keep this open to improve the message in the "patch submitted but didn't change the object" case