Remove non-essential defaults from HCO
iholder101 opened this issue · 4 comments
What happened:
Currently, HCO sets defaults to many of its spec fields, which is an anti-pattern that should be removed.
As Kubernetes documentation writes [1]:
... spec ... providing a description of the characteristics you want the resource to have: its desired state.
The status describes the current state of the object, supplied and updated by the Kubernetes system and its components.
In other words, the spec should express the desired state from the user's perspective, while the status should report the current state of the object and be updated automatically by system components.
Defaults, by definitions, are values that the user does not care about, since they are missing from the spec. The fact that HCO auto-sets spec values is an anti-pattern if not a bug. This way, it is impossible to distinguish between the desired state which the user actually cares about and the current state which needs to be auto-assigned, but in the status rather than the spec.
As a side effect, changes like done in kubevirt/kubevirt#12848 will not kick in for setups that run HCO. Instead, with the current approach, defaults need to be changed in multiple places, which leads to bugs and misconfigurations.
[1] https://kubernetes.io/docs/concepts/overview/working-with-objects/
What you expected to happen:
Spec should only contain the user's provided desired state and nothing more.
Status should report the current state of an object.
How to reproduce it (as minimally and precisely as possible):
- Install Kubevirt via HCO.
- See that HCO's spec fields are auto-assigned.
What happened: Currently, HCO sets defaults to many of its spec fields, which is an anti-pattern that should be removed.
....
Defaults, by definitions, are values that the user does not care about, since they are missing from the spec. The fact that HCO auto-sets spec values is an anti-pattern if not a bug.
Actually the k8s API conventions are stating:
In general we want default values to be explicitly represented in our APIs, rather than asserting that "unspecified fields get the default behavior". This is important so that:
- default values can evolve and change in newer API versions
- the stored configuration depicts the full desired state, making it easier for the system to determine how to achieve the state, and for the user to know what to anticipate
As a side effect, changes like done in kubevirt/kubevirt#12848 will not kick in for setups that run HCO. Instead, with the current approach, defaults need to be changed in multiple places, which leads to bugs and misconfigurations.
And this is also somehow expected since one of the goals of HCO is to provide an an opinionated deployment of KubeVirt and its helper operators.
KubeVirt can be deployed in different scenarios, HCO is here to provide an opinionated one.
And this is also somehow expected since one of the goals of HCO is to provide an
an opinionated deployment of KubeVirt and its helper operators.KubeVirt can be deployed in different scenarios, HCO is here to provide an opinionated one.
This makes sense. That's why I phrased it as remove non-essential defaults.
In other words, if there's a good reason not to rely on Kubevirt's defaults, I'd be ok with that. But just repeating the defaults is wrong IMO.
FYI @vladikr
@jean-edouard @acardace @xpivarc
would like to know your opinions on this