Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove non-essential defaults from HCO #3163

Open
iholder101 opened this issue Nov 13, 2024 · 4 comments
Open

Remove non-essential defaults from HCO #3163

iholder101 opened this issue Nov 13, 2024 · 4 comments
Labels

Comments

@iholder101
Copy link
Contributor

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):

  1. Install Kubevirt via HCO.
  2. See that HCO's spec fields are auto-assigned.
@tiraboschi
Copy link
Member

tiraboschi commented Nov 13, 2024

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.

@iholder101
Copy link
Contributor Author

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.

@iholder101
Copy link
Contributor Author

FYI @vladikr

@iholder101
Copy link
Contributor Author

@jean-edouard @acardace @xpivarc
would like to know your opinions on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants