Customize KubeVirt installation

[atanasdinov]

• Extracts all KubeVirt CR fields as customisable values
• Fields and their description extracted from:

adinov@AtanasMac: charts [kubevirt-customization]× » kubectl explain kubevirt.spec                                    [16:47:48]
GROUP:      kubevirt.io
KIND:       KubeVirt
VERSION:    v1

FIELD: spec <Object>

DESCRIPTION:
    <empty>
FIELDS:
  certificateRotateStrategy     <Object>
    <no description>

  configuration <Object>
    holds kubevirt configurations. same as the virt-configMap

  customizeComponents   <Object>
    <no description>

  imagePullPolicy       <string>
    The ImagePullPolicy to use.

  imagePullSecrets      <[]Object>
    The imagePullSecrets to pull the container images from Defaults to none

  imageRegistry <string>
    The image registry to pull the container images from Defaults to the same
    registry the operator's container image is pulled from.

  imageTag      <string>
    The image tag to use for the continer images installed. Defaults to the same
    tag as the operator's container image.

  infra <Object>
    selectors and tolerations that should apply to KubeVirt infrastructure
    components

  monitorAccount        <string>
    The name of the Prometheus service account that needs read-access to
    KubeVirt endpoints Defaults to prometheus-k8s

  monitorNamespace      <string>
    The namespace Prometheus is deployed in Defaults to openshift-monitor

  productComponent      <string>
    Designate the apps.kubevirt.io/component label for KubeVirt components.
    Useful if KubeVirt is included as part of a product. If ProductComponent is
    not specified, the component label default value is kubevirt.

  productName   <string>
    Designate the apps.kubevirt.io/part-of label for KubeVirt components. Useful
    if KubeVirt is included as part of a product. If ProductName is not
    specified, the part-of label will be omitted.

  productVersion        <string>
    Designate the apps.kubevirt.io/version label for KubeVirt components. Useful
    if KubeVirt is included as part of a product. If ProductVersion is not
    specified, KubeVirt's version will be used.

  serviceMonitorNamespace       <string>
    The namespace the service monitor will be deployed  When
    ServiceMonitorNamespace is set, then we'll install the service monitor
    object in that namespace otherwise we will use the monitoring namespace.

  uninstallStrategy     <string>
    Specifies if kubevirt can be deleted if workloads are still present. This is
    mainly a precaution to avoid accidental data loss

  workloadUpdateStrategy        <Object>
    WorkloadUpdateStrategy defines at the cluster level how to handle automated
    workload updates

  workloads     <Object>
    selectors and tolerations that should apply to KubeVirt workloads

[atanasdinov]

@vasiliy-ul could you please take a look at this and let us know if any of the KubeVirt resource fields could be omitted? I'm somewhat stuck between letting all of them and dropping the ones that the operator is working with / provides e.g. imageRegistry, imageTag, etc. Also unsure how the productX ones are used and if they should be kept.

We've settled on using the single chart approach (at least for Edge 3.0) which means that the operator will be the one dealing with the installation, upgrade and deletion processes through the established hooks mechanism.

Any advice would be much appreciated.

[vasiliy-ul]

TBH, I am not completely sure that all the fields from KubeVirt CR should be made configurable through the chart. This requires certain effort to keep everything in sync on each release, and also the actual value for the end-user of having all of them is not obvious. Most of the fields in the CR are configuration options that can be set in runtime.

I would say that it makes sense to first focus on the ones that affect the deployment of the KubeVirt infra. Keeping only the bare minimum. Other options can be added later if there is certain demand from the customers or the community here in github.

dropping the ones that the operator is working with / provides e.g. imageRegistry, imageTag, etc.

I think having the same options in several places is confusing. Besides, isn't it more error-prone? E.g. how is it handled if you set different values there?

Also unsure how the productX ones are used and if they should be kept.

According to the docs, those just apply a product label to KubeVirt components. Makes sense for projects like Harvester where KubeVirt is used under the hood. Not sure if those are needed when KubeVirt is used standalone.

[atanasdinov]

I would say that it makes sense to first focus on the ones that affect the deployment of the KubeVirt infra. Keeping only the bare minimum. Other options can be added later if there is certain demand from the customers or the community here in github.

Sounds great to me! It'd definitely be easier to maintain for this initial version.

I think having the same options in several places is confusing. Besides, isn't it more error-prone? E.g. how is it handled if you set different values there?

It is indeed more error prone. However, it is equivalent to users manually changing these values in the upstream manifests before deploying them. I was contemplating whether we want to provide a more "open" approach where everything is configurable or one where it is mostly focused on SUSE Edge solutions. Perhaps overengineering it and trying to apply to a wider audience is not that good right from the start.

According to the docs, those just apply a product label to KubeVirt components. Makes sense for projects like Harvester where KubeVirt is used under the hood. Not sure if those are needed when KubeVirt is used standalone.

Yes, those shouldn't be necessary in this case.

---

These are the fields that are shipped in the spec of the KubeVirt manifest:

  certificateRotateStrategy: {}
  configuration:
    developerConfiguration:
      featureGates: []
  customizeComponents: {}
  imagePullPolicy: IfNotPresent
  workloadUpdateStrategy: {}

A trimmed down list I came up with:

  certificateRotateStrategy: {}
  # Holds kubevirt configurations. Same as the virt-configMap.
  configuration: {}
  customizeComponents: {}
  # The ImagePullPolicy to use.
  imagePullPolicy: IfNotPresent
  # Selectors and tolerations that should apply to KubeVirt infrastructure components.
  infra: {}
  # The name of the Prometheus service account that needs read-access to KubeVirt endpoints.
  # Defaults to prometheus-k8s.
  monitorAccount: ""
  # The namespace Prometheus is deployed in.
  # Defaults to openshift-monitor.
  monitorNamespace: ""
  # The namespace where the service monitor will be deployed.
  # Defaults to the monitoring namespace if not specified.
  serviceMonitorNamespace: ""
  # Specifies if KubeVirt can be deleted if workloads are still present.
  # This is mainly a precaution to avoid accidental data loss.
  uninstallStrategy: ""
  # WorkloadUpdateStrategy defines at the cluster level how to handle automated workload updates.
  workloadUpdateStrategy: {}
  # Selectors and tolerations that should apply to KubeVirt workloads.
  workloads: {}

Does this look good or should we drop anything else?

P.S. I'll make monitorNamespace default to something else.

[vasiliy-ul]

I would still drop a couple of options. Please refer to the comments below.

certificateRotateStrategy: {}

I would say that this could be configured in runtime.

configuration: {}

Same.

customizeComponents: {}

This might be useful for someone as it allows applying custom patches to KubeVirt infra deployments. Ok to keep.

imagePullPolicy: IfNotPresent

Ok to keep.

infra: {}

Ok to keep.

monitorAccount: ""
monitorNamespace: ""
serviceMonitorNamespace: ""

I would drop those.

uninstallStrategy: ""
workloadUpdateStrategy: {}

Might be useful. Ok to keep.

workloads: {}

I would drop it.

I think that basically follows what I meant by 'options that affect the deployment of the KubeVirt infra'. Ofc, in extreme cases, someone may want to configure everything with Helm. But then again, this list can be extended when needed.

[atanasdinov]

I'm not familiar enough with the KubeVirt project but all of the explanations make sense to me. We can probably keep configuration: {} though, since the Sylva project will most likely want to use seccompConfiguration as mentioned at #50 (comment) and it'd be much easier for them to configure it.

Our PM will be able to review this early next week as well in order to confirm the choices we make here align with the longer term plans for the chart.

Thank you @vasiliy-ul for the provided feedback! It is very useful! :)

[vasiliy-ul]

@atanasdinov, just a random thought: in principle, KubeVirt CR is a big config and all of its fields can be tuned. Then instead of exposing individual settings from KubeVirt CR via Helm, wouldn't it be more convenient to just expose the whole CR? This will enable flexible and fine-grained configurations for the end-users, and it will not add much maintenance complexity.

Though, do not know if it's a good/common practice to expose the whole CR with Helm.

[atanasdinov]

This is what I basically started with. Templating the whole spec object would look weird but templating all root level spec fields is much more common.

[atanasdinov]

Unless I'm misunderstanding the expose the whole CR part? Could you provide an example?

[vasiliy-ul]

Unless I'm misunderstanding the expose the whole CR part? Could you provide an example?

I was thinking about Templating the whole spec object. It should be easier than templating each field from that spec one by one. I am not a big expert in Helm, but that seems like a valid solution.

[agracey]

Overall, this looks good to me.

My 2c about templating is that the values are a place where we can help communicate what's supportable or not. IMO, this means that having each field be separate provides a better "contract".

[agracey]

Another note: I would suggest removing options that we don't expect to be used as much until explicitly requested. This can help us maintain a higher level of quality due to the reduced maintenance burden and QA needed.

[atanasdinov]

Limited the available options as per the suggestions of @vasiliy-ul above.