kubevirt/hyperconverged-cluster-operator

Annotation kubevirt.kubevirt.io/jsonpatch can only patch one resource

nonamef opened this issue · 2 comments

What happened:
Only one resource is patched when multiple resources are defined with the annotation kubevirt.kubevirt.io/jsonpatch. I have verified each patch will work on its own.

What you expected to happen:
All resource defined to be patched.

How to reproduce it (as minimally and precisely as possible):

kubevirt.kubevirt.io/jsonpatch: |-
      [
        {
          "op": "add",
          "path": "/spec/customizeComponents/patches",
          "value": [{
              "patch": "[{\"op\":\"add\",\"path\":\"/spec/template/metadata/annotations\",\"value\":{\"prometheus.io\/path\":\"\/metrics\",\"prometheus.io\/port\":\"8443\",\"prometheus.io\/scheme\":\"https\",\"prometheus.io\/scrape\":\"true\"}}]",
              "resourceName": "virt-controller",
              "resourceType": "Deployment",
              "type": "json"
          }]
        },
        {
          "op": "add",
          "path": "/spec/customizeComponents/patches",
          "value": [{
              "patch": "[{\"op\":\"add\",\"path\":\"/spec/template/metadata/annotations\",\"value\":{\"prometheus.io\/path\":\"\/metrics\",\"prometheus.io\/port\":\"8443\",\"prometheus.io\/scheme\":\"https\",\"prometheus.io\/scrape\":\"true\"}}]",
              "resourceName": "virt-handler",
              "resourceType": "Daemonset",
              "type": "json"
          }]
        }
      ] 

Additional context:
Only the last value in the array is applied.

Environment:

  • KubeVirt version (use virtctl version): 1.12.0
  • Kubernetes version (use kubectl version): 1.29
  • Cloud provider or hardware configuration: EKS

Hi. I'm afraid the problem is in the jsonpatch itself. Using add operation twice, on the same path, will override the existing value.

You have two options here: add these two patches as an array of objects, in a single patch option:

[
  {
    "op": "add",
    "path": "/spec/customizeComponents/patches",
    "value": [
      {
        "patch": "[{\"op\":\"add\",\"path\":\"/spec/template/metadata/annotations\",\"value\":{\"prometheus.io\/path\":\"\/metrics\",\"prometheus.io\/port\":\"8443\",\"prometheus.io\/scheme\":\"https\",\"prometheus.io\/scrape\":\"true\"}}]",
        "resourceName": "virt-controller",
        "resourceType": "Deployment",
        "type": "json"
      },
      {
        "patch": "[{\"op\":\"add\",\"path\":\"/spec/template/metadata/annotations\",\"value\":{\"prometheus.io\/path\":\"\/metrics\",\"prometheus.io\/port\":\"8443\",\"prometheus.io\/scheme\":\"https\",\"prometheus.io\/scrape\":\"true\"}}]",
        "resourceName": "virt-handler",
        "resourceType": "Daemonset",
        "type": "json"
      }
    ]
  }
]

PROS:

  • simpler solution.
  • Will work even if the patches field does not exist yet.

CONS:

  • it will override everything in /spec/customizeComponents/patches

The other option is to use the /spec/customizeComponents/patches/- (with /- suffix), to add two single options - not two arrays, to the existing array.

[
  {
    "op": "add",
    "path": "/spec/customizeComponents/patches/-",
    "value": {
      "patch": "[{\"op\":\"add\",\"path\":\"/spec/template/metadata/annotations\",\"value\":{\"prometheus.io\/path\":\"\/metrics\",\"prometheus.io\/port\":\"8443\",\"prometheus.io\/scheme\":\"https\",\"prometheus.io\/scrape\":\"true\"}}]",
      "resourceName": "virt-controller",
      "resourceType": "Deployment",
      "type": "json"
    }
  },
  {
    "op": "add",
    "path": "/spec/customizeComponents/patches/-",
    "value": {
      "patch": "[{\"op\":\"add\",\"path\":\"/spec/template/metadata/annotations\",\"value\":{\"prometheus.io\/path\":\"\/metrics\",\"prometheus.io\/port\":\"8443\",\"prometheus.io\/scheme\":\"https\",\"prometheus.io\/scrape\":\"true\"}}]",
      "resourceName": "virt-handler",
      "resourceType": "Daemonset",
      "type": "json"
    }
  }
]

PROS:

  • it won't override what's already in the array

CONS:

  • it won't override what's already in the array, including identical objects that are already there
  • The patches array must be pre-exists

=== EDIT ===
Since the patches field is missing by default, I would go with the first option.

closing as no-bug.