Remove applied patch when Patch CR

[LDL-GH]

Hi

I've used the patch operator with great success and am glad to be able to apply patches in a declarative way.

I was however wondering if it would be possible to (maybe optionally) configure it in a way that when the Patch CR is removed the applied patch is undone as well.

This may not be applicable to more advanced patches, but currently we have a few patches that just add some annotations to certain services. It would be nice these annotations would disappear when the Patch CR that added them is removed.

[raffaelespazzoli]

as you have correctly identified there does not seem to be a deterministic way to undo a patch. In terms of declarative configuration what you ask is correct, but I don't know of a way to implement it.

[achetronic]

Hello there,
The right way would be the verbose way at the same time: storing into the Patch CR, the last entire original CR. Honestly, I think this is a dirty way of doing it. I would not implement this but would treat to solve this with better GitOps, for example, remove and reconcile the original CR without applying the Patch CR.

[raffaelespazzoli]

a CR manifest size is limited. This approach would severely limit the number of objects that can be patched by one patch cR. What would the use case be for undoing a patch, besides perfect declarative semantics?

…

On Sun, Apr 10, 2022 at 6:58 AM Alby Hernández ***@***.***> wrote: Hello there, The right way would be the verbose way at the same time: storing into the Patch CR, the last entire original CR. Honestly, I think this is a dirty way of doing it. I would not implement this but would treat to solve this with better GitOps, for example, remove and reconcile the original CR without applying the Patch CR. — Reply to this email directly, view it on GitHub <#14 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABPERXBKZXGVUHZ4QHCQSB3VEKX3NANCNFSM5RRKCMUQ> . You are receiving this because you commented.Message ID: ***@***.***>

-- ciao/bye Raffaele

[raffaelespazzoli]

this issue was mistakenly closed.

[achetronic]

Honestly, I thing it is better not to implement the feature this way commented. Anyway I can not find a better way to do it. What would be your proposal, @LDL-GH?

[LDL-GH]

We currently use the Patch operator for one specific intended purpose, to patch existing openshift objects.
For example to add annotations to the openshift-etcd service (required for dynatrace monitoring) or to patch a custom certificate for ingress.

This usually involves a single object being patched with minimal config, so might not be impacted by the CR manifest size limit. However this is only a small part of what the patch operator is capable of and any implemented solution would better cover the whole scope.

Since objects like proxy/cluster or the ingresscontroller.operator are (to some degree) managed by the cluster itself, I'm not sure what will happen if I'd delete them first to recreate with my custom config added or what would happen during an upgrade?

I personally can't find a better way to implement a possible solution either though.

ref: documentation to replace ingress cert

[achetronic]

We currently use the Patch operator for one specific intended purpose, to patch existing openshift objects.
For example to add annotations to the openshift-etcd service (required for dynatrace monitoring) or to patch a custom certificate for ingress.

This usually involves a single object being patched with minimal config, so might not be impacted by the CR manifest size limit. However this is only a small part of what the patch operator is capable of and any implemented solution would better cover the whole scope.

Since objects like proxy/cluster or the ingresscontroller.operator are (to some degree) managed by the cluster itself, I'm not sure what will happen if I'd delete them first to recreate with my custom config added or what would happen during an upgrade?

I personally can't find a better way to implement a possible solution either though.

ref: documentation to replace ingress cert

I have been thinking about the problem. What about imolementing another CR kinded PatchRevision which stores a copy of the resource before being patches and enough metadata to rollback when needed from a Patch? In the Patch CR could exist a parameter with with the ID of the revision and if the Patch is deleted, it re-apply the PatchRevision it refers to.
What do you think about this approach?

[LDL-GH]

We currently use the Patch operator for one specific intended purpose, to patch existing openshift objects.
For example to add annotations to the openshift-etcd service (required for dynatrace monitoring) or to patch a custom certificate for ingress.
This usually involves a single object being patched with minimal config, so might not be impacted by the CR manifest size limit. However this is only a small part of what the patch operator is capable of and any implemented solution would better cover the whole scope.
Since objects like proxy/cluster or the ingresscontroller.operator are (to some degree) managed by the cluster itself, I'm not sure what will happen if I'd delete them first to recreate with my custom config added or what would happen during an upgrade?
I personally can't find a better way to implement a possible solution either though.
ref: documentation to replace ingress cert

I have been thinking about the problem. What about imolementing another CR kinded PatchRevision which stores a copy of the resource before being patches and enough metadata to rollback when needed from a Patch? In the Patch CR could exist a parameter with with the ID of the revision and if the Patch is deleted, it re-apply the PatchRevision it refers to. What do you think about this approach?

Wouldn't this still be impacted by the CR manifest size when you're patching multiple resources?
The copy of the original resource should also be updated every time the patched resource is updated.
e.g. if a label is added to the resource afterwards I still expect that label to be there if I remove the patch.