Consider extending the crate with merge functionality

[soenkeliebau]

Consider extending the crate with merge functionality

[Arnavion]

Yes, IIUC @dpc 's use case wasn't directly related to merge patches, just a custom merge-like thing they were doing locally, so they didn't implement it to follow the Kubernetes strategy specifically. But also IIUC their use case would be unaffected if we did implement the strategy for Vec<T> in accordance with Kubernetes. ie we can repurpose the trait rather than making a whole new one.

So it seems the generated impl incorporate the x-kubernetes-patch-merge-key and x-kubernetes-patch-strategy properties of the fields, and DeepMerge::merge_from would need to take them as a parameter.

[Arnavion]

d3718bf ?

[nightkr]

Oh wow, managed to miss that completely. Will have to investigate!

[nightkr]

Hm, it looks like this doesn't implement strategic merges, right? So merging the following two documents:

apiVersion: v1
kind: Pod
spec:
  containers:
  - name: nginx
    image: nginx
---
apiVersion: v1
kind: Pod
spec:
  containers:
  - name: nginx
    args: ["-t"]

Would give you the following document:

apiVersion: v1
kind: Pod
spec:
  containers:
  - name: nginx
    image: nginx
  - name: nginx
    args: ["-t"]

But containers is actually an ordered map, so the correct behaviour would have been to merge the two container definitions with the same name, like so:

apiVersion: v1
kind: Pod
spec:
  containers:
  - name: nginx
    image: nginx
    args: ["-t"]

[nightkr]

In fact, the documented concat behaviour doesn't match any of K8s' merge strategies (see x-kubernetes-map-type and x-kubernetes-list-type).

[nightkr]

Either that or you could use the strategy to replace the DeepMerge::merge_from call.

[lfrancke]

Thank you both for your work on this!