Bug 1466671 - oc patch always returns "patched" even if it doesn't do anything
Summary: oc patch always returns "patched" even if it doesn't do anything
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: OpenShift Container Platform
Classification: Red Hat
Component: oc
Version: 3.5.0
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: ---
: 3.7.0
Assignee: Fabiano Franz
QA Contact: Xingxing Xia
URL:
Whiteboard:
: 1466664 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-06-30 08:20 UTC by Eduardo Minguez
Modified: 2021-03-03 14:17 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-11-28 21:59:33 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 1267454 0 medium CLOSED It's better to show additional prompt when 'oc patch' non-existing fields of resource 2021-02-22 00:41:40 UTC
Red Hat Product Errata RHSA-2017:3188 0 normal SHIPPED_LIVE Moderate: Red Hat OpenShift Container Platform 3.7 security, bug, and enhancement update 2017-11-29 02:34:54 UTC

Internal Links: 1267454

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


Note You need to log in before you can comment on or make changes to this bug.