Kubevirt CR: allow disabling feature gates

[Itamar Holder]

Hey there!

I'm working on "Kubevirt CR: allow disabling feature gates", #14427.

With this approach it is impossible to explicitly disable a feature gate.

Disabling a feature gate is very important for different reasons, the main one arguably being that it opens the door to enable beta feature gates by default. This capability is very important, because it allows widely testing a feature upstream (which will at least be tested by CI and developers, alongside small users) while disabling it downstream. This approach enables to gain wider feedback and confidence before the feature becomes GA.

However, as detailed in this comment, we have a discussion on what's the right approach to tackle this. TLDR, alternatives are:

1. Create another field to set FGs in the form of a feature gate map.
2. Enable special syntax to disable feature gates, using the current slice API field. This PR currently implements "feature=false", another idea was to allow "!feature".
3. Create another DisabledFeatureGates slice for disabling feature gates.

(more info in the linked comment).

Since there are different opinions on this, I'd love to hear your thoughts and get as wide feedback as possible. Feel free to share your thoughts in the PR.

Thanks!

Itamar.

[Dan Kenigsberg]

On Wed, May 14, 2025 at 1:13 PM 'Itamar Holder' via kubevirt-dev <kubevi...@googlegroups.com> wrote:

Hey there!

I'm working on "Kubevirt CR: allow disabling feature gates", #14427.

With this approach it is impossible to explicitly disable a feature gate.

Disabling a feature gate is very important for different reasons, the main one arguably being that it opens the door to enable beta feature gates by default. This capability is very important, because it allows widely testing a feature upstream (which will at least be tested by CI and developers, alongside small users) while disabling it downstream. This approach enables to gain wider feedback and confidence before the feature becomes GA.

However, as detailed in this comment, we have a discussion on what's the right approach to tackle this. TLDR, alternatives are:

• Create another field to set FGs in the form of a feature gate map.

I liked this because it makes clear that the order of FGs does not matter. But the price of deprecating the FeatureGates slice is very high (e.g., we still have not finished paying it for vm.running->vm.runStrategy)

• Enable special syntax to disable feature gates, using the current slice API field. This PR currently implements "feature=false", another idea was to allow "!feature".

 As mentioned by others, this is not structured. It requires more than Marshal/Unmarshal to read and write. I'd stay away.

• Create another DisabledFeatureGates slice for disabling feature gates.

FWIW, I prefer this, as it gives us what we need, with a relatively small cost. Still, it is less pure than the first option, as it provides the user multiple ways to express the same desired state.

FeatureGates: [x,y]

may mean the same as

FeatureGates: [y,x]

as well as the joint

FeatureGates: [x,y,z]

DisableFeatureGates: [z]

 

(more info in the linked comment).

Since there are different opinions on this, I'd love to hear your thoughts and get as wide feedback as possible. Feel free to share your thoughts in the PR.

Thanks!

Itamar.

--
You received this message because you are subscribed to the Google Groups "kubevirt-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubevirt-dev...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/kubevirt-dev/CALpNySzF8XNG%3Dx82eeGzc0F%3DU6c4EGNu8HChoKGj2nqc6aRVag%40mail.gmail.com.

[Luboslav Pivarc]

On Thu, May 15, 2025 at 9:18 AM 'Dan Kenigsberg' via kubevirt-dev <kubevi...@googlegroups.com> wrote:

On Wed, May 14, 2025 at 1:13 PM 'Itamar Holder' via kubevirt-dev <kubevi...@googlegroups.com> wrote:

Hey there!

I'm working on "Kubevirt CR: allow disabling feature gates", #14427.

With this approach it is impossible to explicitly disable a feature gate.

Disabling a feature gate is very important for different reasons, the main one arguably being that it opens the door to enable beta feature gates by default. This capability is very important, because it allows widely testing a feature upstream (which will at least be tested by CI and developers, alongside small users) while disabling it downstream. This approach enables to gain wider feedback and confidence before the feature becomes GA.

However, as detailed in this comment, we have a discussion on what's the right approach to tackle this. TLDR, alternatives are:

• Create another field to set FGs in the form of a feature gate map.

@al...@nvidia.com raised some concern here. Alay can you iterate here as well for the record?

 

I liked this because it makes clear that the order of FGs does not matter. But the price of deprecating the FeatureGates slice is very high (e.g., we still have not finished paying it for vm.running->vm.runStrategy)

• Enable special syntax to disable feature gates, using the current slice API field. This PR currently implements "feature=false", another idea was to allow "!feature".

 As mentioned by others, this is not structured. It requires more than Marshal/Unmarshal to read and write. I'd stay away.

Imho that is negligible.

 

• Create another DisabledFeatureGates slice for disabling feature gates.

FWIW, I prefer this, as it gives us what we need, with a relatively small cost. Still, it is less pure than the first option, as it provides the user multiple ways to express the same desired state.

This would just add validation as option 2 imho.

-Lubo

 

FeatureGates: [x,y]

may mean the same as

FeatureGates: [y,x]

as well as the joint

FeatureGates: [x,y,z]

DisableFeatureGates: [z]

 

(more info in the linked comment).

Since there are different opinions on this, I'd love to hear your thoughts and get as wide feedback as possible. Feel free to share your thoughts in the PR.

Thanks!

Itamar.

--
You received this message because you are subscribed to the Google Groups "kubevirt-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubevirt-dev...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/kubevirt-dev/CALpNySzF8XNG%3Dx82eeGzc0F%3DU6c4EGNu8HChoKGj2nqc6aRVag%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "kubevirt-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubevirt-dev...@googlegroups.com.

To view this discussion visit https://groups.google.com/d/msgid/kubevirt-dev/CAHOEP55J8bJnDx0fPB4mKOD%2Bdp-y11OpEV8fznr2zcv8KYCx%2Bw%40mail.gmail.com.

[Itamar Holder]

On Thu, 15 May 2025 at 11:54, Luboslav Pivarc <lpi...@redhat.com> wrote:

On Thu, May 15, 2025 at 9:18 AM 'Dan Kenigsberg' via kubevirt-dev <kubevi...@googlegroups.com> wrote:

On Wed, May 14, 2025 at 1:13 PM 'Itamar Holder' via kubevirt-dev <kubevi...@googlegroups.com> wrote:

Hey there!

I'm working on "Kubevirt CR: allow disabling feature gates", #14427.

With this approach it is impossible to explicitly disable a feature gate.

Disabling a feature gate is very important for different reasons, the main one arguably being that it opens the door to enable beta feature gates by default. This capability is very important, because it allows widely testing a feature upstream (which will at least be tested by CI and developers, alongside small users) while disabling it downstream. This approach enables to gain wider feedback and confidence before the feature becomes GA.

However, as detailed in this comment, we have a discussion on what's the right approach to tackle this. TLDR, alternatives are:

• Create another field to set FGs in the form of a feature gate map.

@al...@nvidia.com raised some concern here. Alay can you iterate here as well for the record?

This is the link Alay have shared with me:

https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#lists-of-named-subobjects-preferred-over-maps

There they write: "the convention is to use a list of subobjects containing name fields".

IIUC, they recommend having a list of objects instead of a map, which actually makes a lot of sense. So we could potentially use approach (1) above, but use a slice of feature gate objects instead of a map, while each object can have a name field and a field to indicate whether it's enabled. Another benefit of this approach is that we can possibly add more fields in the future if we need to.

At the end of the day, the same pros and cons I've listed still apply. So for the ones supporting this approach, I think this shouldn't change much.

WDYT?

[Alay Patel]

Responses are inline.

From: Itamar Holder <iho...@redhat.com>
Date: Thursday, May 15, 2025 at 6:08 AM
To: Luboslav Pivarc <lpi...@redhat.com>
Cc: Dan Kenigsberg <dan...@redhat.com>, Alay Patel <al...@nvidia.com>, kubevirt-dev <kubevi...@googlegroups.com>
Subject: Re: [kubevirt-dev] Kubevirt CR: allow disabling feature gates

External email: Use caution opening links or attachments

On Thu, 15 May 2025 at 11:54, Luboslav Pivarc <lpi...@redhat.com> wrote:

On Thu, May 15, 2025 at 9:18 AM 'Dan Kenigsberg' via kubevirt-dev <kubevi...@googlegroups.com> wrote:

On Wed, May 14, 2025 at 1:13 PM 'Itamar Holder' via kubevirt-dev <kubevi...@googlegroups.com> wrote:

Hey there!

I'm working on "Kubevirt CR: allow disabling feature gates", #14427.

With this approach it is impossible to explicitly disable a feature gate.

Disabling a feature gate is very important for different reasons, the main one arguably being that it opens the door to enable beta feature gates by default. This capability is very important, because it allows widely testing a feature upstream (which will at least be tested by CI and developers, alongside small users) while disabling it downstream. This approach enables to gain wider feedback and confidence before the feature becomes GA.

However, as detailed in this comment, we have a discussion on what's the right approach to tackle this. TLDR, alternatives are:

• Create another field to set FGs in the form of a feature gate map.

@al...@nvidia.com raised some concern here. Alay can you iterate here as well for the record?

This is the link Alay have shared with me:

https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#lists-of-named-subobjects-preferred-over-maps

There they write: "the convention is to use a list of subobjects containing name fields".

IIUC, they recommend having a list of objects instead of a map, which actually makes a lot of sense. So we could potentially use approach (1) above, but use a slice of feature gate objects instead of a map, while each object can have a name field and a field to indicate whether it's enabled. Another benefit of this approach is that we can possibly add more fields in the future if we need to.

This is indeed correct understanding. There are many issues with using a map in API. While I dont want to hijack this thread, I wanted to write down some off the issue here to make sure future API changes include maps with extreme caution (only if other alternative dont work out).

1. When using a map, the key part of the map is undeterministic, its based on user input. This makes it hard to create automation/validation etc for
2. Maps are computationally more expensive to manage at scale, this thread has some bread crumbs with usptream k8s facing similar scale issues: https://github.com/kubernetes/kubernetes/issues/8190#issuecomment-550952955​ 
3. merging a map with patch or server side apply is hard, it can potentially create hotlooping issues in controllers, example here: https://github.com/kubevirt/kubevirt/issues/14442​

[Itamar Holder]

Hi again,

After discussing this with multiple people, I'm pretty convinced that the best way forward would be to introduce a new field, spec.configuration.featureGates, which will be a slice of a new featureGate struct type.

This way we have a single unified field, containing structs which are both standardized and extendable.

Please let me know if there are any objections. If there are none, I'll adjust the PR in the upcoming days.

Itamar.

[Itamar Holder]

Hello again everyone.

Since there were many different opinions w.r.t. the API we should use, I've now submitted VEP 104: Allow disabling feature gates.

There's a VEP PR here: https://github.com/kubevirt/enhancements/pull/105. I welcome you all to review it and provide feedback.

In addition, as claimed here, I think we can avoid a feature gate for this change.

Let me know what you think in the PR!

Thanks a lot,

Itamar.