Fix ApplyTask patch behavior

[ANeumann82]

What this PR does / why we need it:
The ApplyTask patch method currently uses a StrategicMerge patch, which by default merges some lists, for example, containers in a Deployment-PodTemplate. This makes it impossible to have a conditional container in an operator.

Added a test harness test.

Fixes #1286

Issue
The Big Problem is that applying new versions of rendered resources from KUDO to the cluster is not as easy as it seems: We generally render full resources from the OperatorVersion templates, but there is no easy way to update existing resources in the cluster:

Patch with StrategicMerge
This is the solution we use at the moment. The issue is, that a strategic merge does not replace certain lists - for example, the list of containers in a PodTemplate is merged, not replaced by default. If a list (or map) is merged instead of replaced depends on the annotations in the resource types:

	// +patchMergeKey=name
	// +patchStrategy=merge
	Containers []Container `json:"containers" patchStrategy:"merge" patchMergeKey:"name" protobuf:"bytes,2,rep,name=containers"`

It is possible to overwrite this behavior for strategic merges: https://www.disasterproject.com/kubernetes-kubectl-patching/
We can add an entry $patch: replace to a list or a map to replace instead of merge. But we can't add these entries to all maps and lists, as this would trigger the replacement of immutable fields which leads to an error by the k8s API. We would have to maintain a list of all places to add these patch-entries.

Replace
We could replace resources instead of patching them. This approach does not work as a lot of resources have immutable fields that can't be updated - for example the ServiceSpec.ClusterIP. As this field is usually filled in by k8s, it's not filled when we generate the resource from the KUDO template. A replace would then complain that ClusterIP will be updated with "" which is not allowed.

It might be possible to Force-Replace. This allows us to update immutable fields by doing a delete/create instead of a replace; the downside is that it would, for example, assign a new ClusterIP and be a disruptive update (https://kubernetes.io/docs/concepts/cluster-administration/manage-deployment/#disruptive-updates)

ServerSideApply
This should theoretically work best. All creations and updates should be done via:

c.Patch(context.TODO(), newObj, client.Apply, client.FieldOwner("KUDO"))

and k8s should figure out what actually changed and apply it. There are some problems on the integration tests (no darwin binaries for kube-apiserver > 15.5), and bugs in the server side apply feature itself. Might be an option for the future.

Client Side Threeway-Diff
This is the only real way to do this at the moment.
First approach was to reuse code from kudectl apply, but that fails for various reasons. kubectl isn't really expected to be used as a library.

I rewrote the code similar to kudectl apply though, it still needs some polishing but it should work for now.

[zen-dog]

Nice work. I left a bunch of nits (mostly around naming) and one question about what happens with existing operators after people upgrade. PTAL

[zen-dog]

I got stuck at this line. Are we backwards compatible after this is merged? In a scenario where someone re-runs a deploy plan for an existing operator (after updating KUDO), all plan resources will be patched, correct? However, there is no LastAppliedConfigAnnotation so it will return nill, nil and the subsequent merges will... fail?

[ANeumann82]

Yeah, this is a problem. I'll try to figure something out about this...

[ANeumann82]

This PR introduces a backwards incompatible change:

When used on an already installed instance of an operator, the deployed resources are missing the new "kudo.dev/last-applied-configuration", which will fail the patch process, as no originalConfiguration can be found for the ThreeWayMerge.

There are multiple options to work around this issue, all of which are a bit problematic:
(1) Bail out on old deployed Instances - This is the current state after this PR
(2) Automatically Delete/Recreate the resource - This is problematic, as we would make a disruptive update without the user explicitly acknowledging or allowing this.
(2a) Allow the user to specify a --force flag when using k kudo update and then use Delete/Recreate. This is detailed in #1335 and probably the most usable approach
(3) Patch the resources as we did before this PR - Not a good solution, as we would leave the deployed instances in weird unknown states, which doesn't solve anything
(4) Calculate the originalConfiguration from the old parameters and metadata of the Instance - This would theoretically work, but is a lot of effort and probably error prone, as there are a lot of edge cases and things to consider.

[zen-dog]

We need to announce/changelog the breaking change. Otherwise, LGTM

[gerred]

We need to think about this one. It's one thing to make breaking changes without some form of automated migration for the operator development side, it's another thing entirely to do something that can cause data loss. Can we introduce some sort of migration to handle this for the future? Or can we at least come up, while noting this, a clear way for any existing users of our operators to preserve their data? Even if we don't have any users now, this is a good practice to establish early.

I understand there's probably no great way to actually perform a migration of existing instances though, and overall the PR itself LGTM.

[ANeumann82]

@gerred I have a proposal to use --force on a parameter update, see #1335 That would allow users to continue using their existing installation, at the downside of having one disruptive update of the operator.

As long as the operator does not manually create a PV, that should not lead to any data loss.

[zen-dog]

it's another thing entirely to do something that can cause data loss.

In the proposed implementation, KUDO manager won't find the needed annotation, three-way-merge will fail and so will the step/plan (a transient ERROR if I'm correct). So no data loss should occur @gerred Human operator, however, will have to recreate the Instance to move on.

[ANeumann82]

@zen-dog Not a transient error anymore. In my last commit I changed a couple of the errors to fatalExecutionError, as they won't recover.
Apart from that you're correct, no data loss (but no change to the deployed resource possible either)

[gerred]

Right, but we're telling them the only way to move forward is to re-create the instance. Deleting the instance will destroy the existing instance. It might not get rid of PVs, but we're now in a manual situation where data loss can occur if I don't manually ensure that my PV is used or backed up/restored, right?

[zen-dog]

True, this is exactly as any prior breaking change situation when instances had to be recreated in the new KUDO version.

[ANeumann82]

@gerred As you already mentioned, this would be a prime example for a migration process. I'll take that chance and prioritise working on KUDO upgrades and try to get a migration for this in.