Bug 1466671

Summary:	oc patch always returns "patched" even if it doesn't do anything
Product:	OpenShift Container Platform	Reporter:	Eduardo Minguez <eminguez>
Component:	oc	Assignee:	Fabiano Franz <ffranz>
Status:	CLOSED ERRATA	QA Contact:	Xingxing Xia <xxia>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	3.5.0	CC:	aos-bugs, ffranz, jokerman, mmccomas, myllynen, sdodson
Target Milestone:	---
Target Release:	3.7.0
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:		Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2017-11-28 21:59:33 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Eduardo Minguez 2017-06-30 08:20:00 UTC

Description of problem: oc patch always return "x patched" even if it doesn't change anything.


Version-Release number of selected component (if applicable):
oc v3.5.5.26
kubernetes v1.5.2+43a9be4
features: Basic-Auth GSSAPI Kerberos SPNEGO

Server https://emingueztst.eastus2.cloudapp.azure.com:8443
openshift v3.5.5.26
kubernetes v1.5.2+43a9be4

How reproducible: Create new project, new app, try to patch it and it always return 0 and "patched"
It happens too when trying to patch anything wrong.


Steps to Reproduce:
1. oc new-project test
2. oc new-app kubernetes/guestbook
3. oc get dc guestbook -o json | jq '.spec.template.metadata.annotations'
{
  "openshift.io/generated-by": "OpenShiftNewApp"
}
4. oc patch dc guestbook -p '{"spec":{"template":{"metadata":{"annotations":{"mylabel":"myvalue"}}}}}'
"guestbook" patched
5. oc get dc guestbook -o json | jq '.spec.template.metadata.annotations'
{
  "mylabel": "myvalue",
  "openshift.io/generated-by": "OpenShiftNewApp"
}
6. oc patch dc guestbook -p '{"spec":{"template":{"metadata":{"annotations":{"mylabel":"myvalue"}}}}}'
"guestbook" patched
7. echo $?
0
8. oc get dc guestbook -o json | jq '.spec.template.metadata.annotations'
{
  "mylabel": "myvalue",
  "openshift.io/generated-by": "OpenShiftNewApp"
}

Something wrong:
$ oc get dc guestbook -o json | jq '.spec.template.spec.containers[].imagePullPolicy'
"Always"
$ oc get dc guestbook -o yaml | grep -i always
        imagePullPolicy: Always
      restartPolicy: Always
Now, try to patch in some other level (it should be containers[].template...)
$ oc patch dc guestbook -p '{"spec":{"template":{"spec":{"imagePullPolicy":"Always"}}}}'
"guestbook" patched
$ echo $?
0
$ oc get dc guestbook -o yaml | grep -i always
        imagePullPolicy: Always
      restartPolicy: Always

Actual results:
It always returns 0 and patched

Expected results: If the patch is not applied, it should return an error code and some message about why it wasn't been able to patch it


Additional info:

Comment 1 Xingxing Xia 2017-06-30 10:47:08 UTC

1. Please check if patched field's name/level is correct.
See https://bugzilla.redhat.com/show_bug.cgi?id=1409225#c3

2.
> $ oc patch dc guestbook -p '{"spec":{"template":{"spec":{"imagePullPolicy":"Always"}}}}'
> "guestbook" patched
If user knows about how to use the merge key (said below), seems user will not be confused about this :)
For "containers" field which is array, should use "a merge key" in oc patch.
See `kubectl patch -h` which explains more detailed help info (e.g. about usage of merge key) than `oc patch -h`. (The oc help info issue is tracked in bug 1352453)

Comment 2 Eduardo Minguez 2017-06-30 11:26:03 UTC

(In reply to Xingxing Xia from comment #1)
> 1. Please check if patched field's name/level is correct.
> See https://bugzilla.redhat.com/show_bug.cgi?id=1409225#c3
> 
> 2.
> > $ oc patch dc guestbook -p '{"spec":{"template":{"spec":{"imagePullPolicy":"Always"}}}}'
> > "guestbook" patched
> If user knows about how to use the merge key (said below), seems user will
> not be confused about this :)
> For "containers" field which is array, should use "a merge key" in oc patch.
> See `kubectl patch -h` which explains more detailed help info (e.g. about
> usage of merge key) than `oc patch -h`. (The oc help info issue is tracked
> in bug 1352453)

This was intentional. The problem is not the missing containers[] or whatever, the thing is if I create a non valid patch (as the one I've provided), it should say "patch not valid" or something similar, so, this works:

$ oc patch dc guestbook -p '{"icantypewhatever":"yes"}'

But it shouldn't.

Comment 3 Fabiano Franz 2017-06-30 18:07:23 UTC

*** Bug 1466664 has been marked as a duplicate of this bug. ***

Comment 4 Fabiano Franz 2017-06-30 18:54:18 UTC

Fixed in 3.6.

Comment 5 Xingxing Xia 2017-07-05 10:08:54 UTC

Thank you
Checked in v3.6.133, now it shows:
$ oc patch dc database -p '{"spec":{"template":{"spec":{"imagePullPolicy":"Always"}}}}'
deploymentconfig "database" not patched

$ oc patch dc database -p '{"icantypewhatever":"yes"}'
deploymentconfig "database" not patched

This fixes original issue reported, so moving to VERIFIED (QE is checking many Modified bugs that if they're verifiable. Because fixed, moving to Verified. If have extra concern, pls correct me, thx)

But it is better to have additional prompt such as "please check if the patch field's name/level is correct", this may help user self troubleshoot question such as:
once ever a non-UI QE patches with singular "container" and wondered why not taking effect:
'{"spec":{"template":{"spec":"container":...
Then the guy asked me and I told should use plural "containers"

WDYT?

Comment 9 errata-xmlrpc 2017-11-28 21:59:33 UTC

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2017:3188