Velero: Create helper function to patch things

[ncdc]

Replace all the individual patchXYZ functions with a single helper

[rosskukulinski]

@ncdc priority? 1.0 milestone? Is this a good issue for new developers?

[ncdc]

@rosskukulinski I'd like to do this eventually, but it's not a strict 1.0 requirement.

[nzoueidi]

@ncdc @rosskukulinski I would like to work on this issue. Do you guys have any example(s) for the patchXYZ functions? I searched in the pkg/ folder and I am seeing many patch funcs.

[ncdc]

@skriss @carlisia

[skriss]

ack, @nzoueidi will provide some direction here as soon as I can!

[carlisia]

I went thru a bunch of the Patch... functions and am not sure which ones could be combined into a helper.

[skriss]

I'm not 100% sure what @ncdc had in mind here, but I'm guessing it was something along these lines:

We have a lot of functions that look like this: https://github.com/heptio/velero/blob/master/pkg/controller/pod_volume_backup_controller.go#L259 that take an API object and a pointer to a function that modifies that object, and they generate and apply the JSON patch to the API.

There's not really anything in this function that's specific to the type being patched, so we could generalize the function with a combination of interfaces and closures, along the lines of this:

// note: Patcher is an interface that we'd define
func genericPatch(obj metav1.Object, modify func() metav1.Object, client Patcher) error {
    orig, err := json.Marshal(obj)
    if err != nil {
        return err
    }

    updated := modify()

    upd, err := json.Marshal(updated)
    if err != nil {
        return err
    }

    patch, err := jsonpatch.CreateMergePatch(orig, upd)
    if err != nil {
        return err
    }

    _, err = patcher.Patch(obj.GetName(), types.MergePatchType, patch)
    return err
}

This would mostly work as-is, except that the type-specific clients don't actually implement the Patcher interface since they return a specific type as their first return parameter rather than an interface{} or a metav1.Object. A wrapper struct might need to be created there, or you could also use a closure instead of an interface there.

[skriss]

Thanks for this request. We are not currently prioritizing this item within our roadmap, and as such will be closing out the issue. Please feel free to reach out as needed in this issue or a new one.