redhat-cop / openshift-applier

Used to apply OpenShift objects to an OpenShift Cluster
Apache License 2.0
102 stars 61 forks source link

Support `patch` as an action #88

Closed etsauer closed 5 years ago

etsauer commented 5 years ago

I've been seeing more and more use cases in the field where having proper oc patch support in applier would be really valuable. We frequently run into scenarios where we need to apply small changes to resources that are not fully managed by applier. Things like:

In this case, it would be useful to be able to pass to applier a minimal resource definition that just identifies a resource by name, e.g.

kind: Route
apiVersion: v1
metadata
  name: myroute

and a file defining the patch

metadata:
  annotations:
    openshift.io/random-flag-for-filtering-purposes: hello

The challenge is that currently, oc patch does not support passing a patch as a file, only as a JSON string. So applier would need to provide a new parameter in which to provide the path to the patch file, and then convert the file contents into a JSON string to pass to oc.

This functionality could provide a more generic and more native way of handling things like applying labels to resources, which currently is provided by a separate post hook role that lives in another repo.

@oybed @logandonley @pcarney8

oybed commented 5 years ago

My suggestion would be to just support this by having the proper action set, and make the openshift-applier process any entries with action: patch to source the file and convert to a json string. Hence, an example inventory could look like the following:

openshift_cluster_content:
- object: "my-patch-request"
  content:
  - name: "patching-my-route"
    file: "/path/to/my/file/route_patch.yml"
    action: patch

The file parameter for other actions can currently be either a local file or an URL (the oc command processes either). Hence, the support for patch will also need to do the same, but it will require the openshift-applier to do the proper processing to fetch the file from local or remote source.

etsauer commented 5 years ago

@oybed the challenge is that with patch, you need to provide two arguments: the name (or file) specifying the object to be patched, and then the patch string. We don't really have a second parameter to pass currently.

etsauer commented 5 years ago

One option could be to overload params and params_from_vars and when action == patch, convert the file or embedded yaml to a json string, and pass that in.

oybed commented 5 years ago

@etsauer yeah, good and valid points. One thing I would caution us about is limiting to patching just one object at time - i.e.: I can already see the need/request to allow the same patch to be applied to multiple objects, so probably would want to make the "second parameter" (or wherever the the target object is coming from) be a list of objects.

Since all of these options are using a custom definition, couldn't we just define the content of the "file" and make it define both the list of objects to be patched and the patch content? I.e.: based on your example in the write-up above, it could be something like:

> cat my-applier-patch-file.yml
---
target_objects:
- kind: Route
  apiVersion: v1
  metadata
    name: myroute
- kind: Route
  apiVersion: v1
  metadata
    name: myroute2

patch_content:
  metadata:
    annotations:
      openshift.io/random-flag-for-filtering-purposes: hello
etsauer commented 5 years ago

@oybed yeah, I like it.

oybed commented 5 years ago

PR merged - closing issue. (please re-open if needed)