[feature request] Sidecarset Pod Metadata support Array merge.

[jzeng4]

What would you like to be added: Currently, pod annotation injection supports the followings:

• Retain
• Overwrite
• MergePatchJson

For MergePatchJson, it only allows to merge non-array annotations. For array annotations, it will did overwrite.

What we need is to merge the annotation array if the annotations have the same key. For example:

• sidecar1 annotation: test/applications: [{"product":"foo1"}, {"product":"bar1"}]
• sidecar2 annotation: test/applications: [{"product":"foo2"}, {"product":"bar2"}]
• pod annotation: test/applications: [{"product":"foo3"}, {"product":"bar3"}]
• Result: pod annotation: test/applications: [{"product":"foo1"}, {"product":"bar1"},{"product":"foo2"}, {"product":"bar2"},{"product":"foo2"}, {"product":"bar2"}]

Why is this needed:

• This is more flexible to support array use cases.
• MergePatchJson indicates the "merge" operation. However, use json array in the annotation will actually do the overwrite operations. It's very confusing.

[jzeng4]

If this feature makes sense, we can commit a PR for this feature.

[jzeng4]

@FillZpp could you prioritize this ticket? thank you!

[furykerry]

@jzeng4 maybe your merge behavior can be satisfied with json patch. it may look like this. ` apiVersion: apps.kruise.io/v1alpha1 kind: SidecarSet spec: containers: ... patchPodMetadata:

• annotations:
  • linkedin.com/applications: '[ { "op": "add", "path": "/-", "value": "{ "product":"foo2"}"} , "op": "add", "path": "/-", "value": "{ "product":"bar2"} } ]' patchPolicy: PatchJson

`

[zmberg]

@jzeng4 maybe your merge behavior can be satisfied with json patch. it may look like this. ` apiVersion: apps.kruise.io/v1alpha1 kind: SidecarSet spec: containers: ... patchPodMetadata:

• annotations:

  • linkedin.com/applications: '[ { "op": "add", "path": "/-", "value": "{ "product":"foo2"}"} , "op": "add", "path": "/-", "value": "{ "product":"bar2"} } ]' patchPolicy: PatchJson

`

@furykerry @jzeng4 Currently sidecarSet controller will call the merge json function when upgrade sidecar in-place, so the above PatchJson will result in repeated element.

If PatchJson is only effective in webhook, the method is ok. So we can skip the jsonPatch in controller. Related Docs:

1. https://datatracker.ietf.org/doc/html/rfc6902
2. https://github.com/evanphx/json-patch

Related Code: https://github.com/openkruise/kruise/blob/master/pkg/control/sidecarcontrol/util.go#L451

@jzeng4 Can you submit the feature?

[jzeng4]

sure. will work on that.

[furykerry]

sure. will work on that.

plz submit a KEP doc to describe the user case and other technical detail before diving into implementation, my concern includes:

1. if multiple sidecarsets include patch metadata fields, what is the expected behavior, how to ensure the patch sequence
2. if the patch metadata field is changed, does it affects existing pods?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.