Description of problem: Default type for `oc patch` is `strategic` but it fails for CRD as described in bug 1619869. As a user I would like to write `oc patch` without specifying type in most use cases. Thus I think default patch type can be "merge" when run against a CRD. `oc` can figure out this is a custom resource and use "merge" as a default. In case I have specified --type=strategic, then command should fail though. Version-Release number of selected component (if applicable): oc v3.11.28 How reproducible: always Steps to Reproduce: 1. oc patch prometheusrule/prometheus-k8s-rules -n openshift-monitoring --type=strategic -p '{"metadata": {"annotations": {"cr_patch": "yep"}}}' Actual results: > Error from server (UnsupportedMediaType): the body of the request was in an unknown format - accepted media types include: application/json-patch+json, application/merge-patch+json Expected results: > patched
Figuring out whether a command is dealing with a specific resource type goes against our design of having "generic" commands. Although strategic patch types are not currently supported for CRDs, they might one day be supported. I would be against modifying the behavior of the patch command based on this temporary edge-case.
It is more and more common use case I think. In any case, can't we then modify server so that strategic patch on CRDs simply results in merge patch?
> In any case, can't we then modify server so that strategic patch on CRDs simply results in merge patch? I think this would make sense. Adding Stefan for his thoughts here.
The semantics of the available patch types is different. We cannot just switch to another one for CRDs on the server. The patch has a MIME type and that's what we follow when patching. Kubectl tells the server about the MIME type. It would violate the API specification to ignore that. kubectl does not know and should not know whether a resource is backed by a CRD. With the same argument that the merge type makes a behaviour difference, the user of kubectl has to consider carefully which one to choose. Otherwise he gets inconsistent results. I don't think we can or should take this burden from the user. Strategic patch is considered as a failure by many (CRDs will not get suppport for it) and one might argue that it was a bad choice to set that as default. It's unfortunate that CRDs do not support it. We might want a better error message suggesting to switch to merge patch.
Stefan, `strategic` patch works mostly like `merge` patch. So on the server level it makes sense to me for resources that do not have `strategic` patch support to just do merge patch (as an internal "strategy"). I'm not sure I manage to explain it. i.e. instead of server returning that there is no strategy and fail, server can just use the "merge" strategy. Does what I'm saying make sense? This would give convenience for users now as well allow strategies to be added going forward for whatever resources. e.g. even though we do not anticipate it, some day CRDs may have the metadata needed for merge patch. Then nothing will need to change on the client side, just on server side things will work better.
Changing the default from one value to another is a breaking change. So you cannot do that later-on. The switch from strategic – which fails not – to something else might be ok, technically. But in general, I don't like such an automatism of choosing different semantics depending on some heuristic. Kubectl and the API should work consistently for all resources. Making incompatible exception for certain resources is the wrong direction. Why not just make the error message better and require to pass the right patch format to kubectl? It's easy to do, clear for the user and consistent.
I meant on the server side. I understand why ot doesn't make much sense for such logic on client.