[sig-network] Deprecation of SLIRP binding

[Edward Haas]

Hello All,

SLIRP binding has been part of Kubevirt from the early days.

With the introduction of the masquerade binding, SLIRP gradually lost interest due to the performance penalty it comes with.

The SIG Network would like to suggest the deprecation of the SLIRP binding.

Plan:

- Suggest the deprecation (this notification).

- Send the deprecation notice (a week later to give time for feedback).

- Add a warning log/event and annotate the relevant struct in the client to warn users (on main).

- Once the next version is released, remove all SLIRP binding code.

Summary: SLIRP will be available without any warning during the coming v1 release, then will have warnings of deprecation on the main branch and during the next release (i.e. for around 6 months) and totally removed afterwards.

If anyone is currently using SLIRP binding or has feedback related to this deprecation, please let us know.

Thanks,

Edy.

[Fabian Deutsch]

Hey Edy,

I don't have any reservation of deprecating it. But there is one caveat, see below:

On Thu, Jun 22, 2023 at 10:23 AM Edward Haas <edw...@redhat.com> wrote:

Hello All,

SLIRP binding has been part of Kubevirt from the early days.

With the introduction of the masquerade binding, SLIRP gradually lost interest due to the performance penalty it comes with.

The SIG Network would like to suggest the deprecation of the SLIRP binding.

Plan:

- Suggest the deprecation (this notification).

- Send the deprecation notice (a week later to give time for feedback).

- Add a warning log/event and annotate the relevant struct in the client to warn users (on main).

- Once the next version is released, remove all SLIRP binding code.

We are following semver.

The API version will stay (v1), thus we must keep backwards compatibility.

How is backwards compatibility kept when removing the code?

Can you please also specify what you would do with the API after removing the code.

 

Summary: SLIRP will be available without any warning during the coming v1 release, then will have warnings of deprecation on the main branch and during the next release (i.e. for around 6 months) and totally removed afterwards.

If anyone is currently using SLIRP binding or has feedback related to this deprecation, please let us know.

Thanks,

Edy.

--
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 on the web visit https://groups.google.com/d/msgid/kubevirt-dev/CALmkdFRhh4cubGCE71CAL7%3D%3DnPDh2QcWMoMX__TT1Tsfre%2B-wQ%40mail.gmail.com.

[Edward Haas]

On Thu, Jun 22, 2023 at 1:46 PM Fabian Deutsch <fdeu...@redhat.com> wrote:

Hey Edy,

I don't have any reservation of deprecating it. But there is one caveat, see below:

On Thu, Jun 22, 2023 at 10:23 AM Edward Haas <edw...@redhat.com> wrote:

Hello All,

SLIRP binding has been part of Kubevirt from the early days.

With the introduction of the masquerade binding, SLIRP gradually lost interest due to the performance penalty it comes with.

The SIG Network would like to suggest the deprecation of the SLIRP binding.

Plan:

- Suggest the deprecation (this notification).

- Send the deprecation notice (a week later to give time for feedback).

- Add a warning log/event and annotate the relevant struct in the client to warn users (on main).

- Once the next version is released, remove all SLIRP binding code.

We are following semver.

The API version will stay (v1), thus we must keep backwards compatibility.

How is backwards compatibility kept when removing the code?

Can you please also specify what you would do with the API after removing the code.

I am not sure I fully understand the requirements for backward compatibility.

Do you refer to cases where an old VM with SLIRP configuration is upgraded to the version that lacks support for it?

If this is the case, I would like to declare that these VMs cannot be upgraded or migrated, but they can still run.

This may require some tweaks from the other components (e.g. virt-handler), but I think this is do-able.

From an API perspective, any SLIP configuration will eventually be a no-op for newer KubeVirt versions.

There are other alternatives here, but I do not think they are worth the effort needed to maintain them.

(e.g. keeping the code in, but not accepting any requests for bug fixing)

[Fabian Deutsch]

On Thu, Jun 22, 2023 at 1:07 PM Edward Haas <edw...@redhat.com> wrote:

On Thu, Jun 22, 2023 at 1:46 PM Fabian Deutsch <fdeu...@redhat.com> wrote:

Hey Edy,

I don't have any reservation of deprecating it. But there is one caveat, see below:

On Thu, Jun 22, 2023 at 10:23 AM Edward Haas <edw...@redhat.com> wrote:

Hello All,

SLIRP binding has been part of Kubevirt from the early days.

With the introduction of the masquerade binding, SLIRP gradually lost interest due to the performance penalty it comes with.

The SIG Network would like to suggest the deprecation of the SLIRP binding.

Plan:

- Suggest the deprecation (this notification).

- Send the deprecation notice (a week later to give time for feedback).

- Add a warning log/event and annotate the relevant struct in the client to warn users (on main).

- Once the next version is released, remove all SLIRP binding code.

We are following semver.

The API version will stay (v1), thus we must keep backwards compatibility.

How is backwards compatibility kept when removing the code?

Can you please also specify what you would do with the API after removing the code.

I am not sure I fully understand the requirements for backward compatibility.

API backwards compatibility. from semver.org:

1. MAJOR version when you make incompatible API changes

2. MINOR version when you add functionality in a backward compatible manner

…

 

Do you refer to cases where an old VM with SLIRP configuration is upgraded to the version that lacks support for it?

It's two things:

1. What do we do about API usage of this feature? Do we block it, do we remove the API field? Do we have a procedure for deprecating fields? What do we do about the fact that we can not modify the API because it's stable?

2. Ignoring the API, what do we do abou feature usage, and your points below sound sane on that,

 

If this is the case, I would like to declare that these VMs cannot be upgraded or migrated, but they can still run.

This may require some tweaks from the other components (e.g. virt-handler), but I think this is do-able.

From an API perspective, any SLIP configuration will eventually be a no-op for newer KubeVirt versions.

This is breaking semver. Something we have committed to, to not do when we moved the API to v1

See also https://github.com/kubevirt/kubevirt/issues/8360

 

In general it is a MAJOR issue that we do not have an API life-cycle document.

How do we gracefull yintroduce new a) APIs and b) fields in existing APIs. How do we do this with a maturity field.

How do we remove non-stable APis again, and how do we graduate APis to become stable.

Your other email goes into this direction, and we must take action on it.

[Edward Haas]

On Thu, Jun 22, 2023 at 7:50 PM Fabian Deutsch <fdeu...@redhat.com> wrote:

On Thu, Jun 22, 2023 at 1:07 PM Edward Haas <edw...@redhat.com> wrote:

On Thu, Jun 22, 2023 at 1:46 PM Fabian Deutsch <fdeu...@redhat.com> wrote:

Hey Edy,

I don't have any reservation of deprecating it. But there is one caveat, see below:

On Thu, Jun 22, 2023 at 10:23 AM Edward Haas <edw...@redhat.com> wrote:

Hello All,

SLIRP binding has been part of Kubevirt from the early days.

With the introduction of the masquerade binding, SLIRP gradually lost interest due to the performance penalty it comes with.

The SIG Network would like to suggest the deprecation of the SLIRP binding.

Plan:

- Suggest the deprecation (this notification).

- Send the deprecation notice (a week later to give time for feedback).

- Add a warning log/event and annotate the relevant struct in the client to warn users (on main).

- Once the next version is released, remove all SLIRP binding code.

We are following semver.

The API version will stay (v1), thus we must keep backwards compatibility.

How is backwards compatibility kept when removing the code?

Can you please also specify what you would do with the API after removing the code.

I am not sure I fully understand the requirements for backward compatibility.

API backwards compatibility. from semver.org:

1. MAJOR version when you make incompatible API changes

2. MINOR version when you add functionality in a backward compatible manner

…

I was trying to find where we have commited to such a limiting policy without success.

Is this referenced somewhere? Is it a non-written agreement?

 

Do you refer to cases where an old VM with SLIRP configuration is upgraded to the version that lacks support for it?

It's two things:

1. What do we do about API usage of this feature? Do we block it, do we remove the API field? Do we have a procedure for deprecating fields? What do we do about the fact that we can not modify the API because it's stable?

2. Ignoring the API, what do we do abou feature usage, and your points below sound sane on that,

 

If this is the case, I would like to declare that these VMs cannot be upgraded or migrated, but they can still run.

This may require some tweaks from the other components (e.g. virt-handler), but I think this is do-able.

From an API perspective, any SLIP configuration will eventually be a no-op for newer KubeVirt versions.

This is breaking semver. Something we have committed to, to not do when we moved the API to v1

See also https://github.com/kubevirt/kubevirt/issues/8360

 

In general it is a MAJOR issue that we do not have an API life-cycle document.

How do we gracefull yintroduce new a) APIs and b) fields in existing APIs. How do we do this with a maturity field.

How do we remove non-stable APis again, and how do we graduate APis to become stable.

Your other email goes into this direction, and we must take action on it.

I was under the impression that the lack of it allows per-case decisions to be taken.

Re-reading the above, it looks like we are in a deadlock with decrecations in general.

If we are forced not to remove the functionality in v1, then the deprecation will only

be able to declare that support is ending and no fixes/changes are planned.

To ease maintenance, we can extract all logic out (e.g. using future network binding plugin) and archive it.

I still hope exceptions could be considered, especially if no user is using it.

[Fabian Deutsch]

Edy, see below

Please checkout https://github.com/kubevirt/kubevirt/blob/main/docs/release.md

Please also checkout semver.org

 

 

Do you refer to cases where an old VM with SLIRP configuration is upgraded to the version that lacks support for it?

It's two things:

1. What do we do about API usage of this feature? Do we block it, do we remove the API field? Do we have a procedure for deprecating fields? What do we do about the fact that we can not modify the API because it's stable?

2. Ignoring the API, what do we do abou feature usage, and your points below sound sane on that,

 

If this is the case, I would like to declare that these VMs cannot be upgraded or migrated, but they can still run.

This may require some tweaks from the other components (e.g. virt-handler), but I think this is do-able.

From an API perspective, any SLIP configuration will eventually be a no-op for newer KubeVirt versions.

This is breaking semver. Something we have committed to, to not do when we moved the API to v1

See also https://github.com/kubevirt/kubevirt/issues/8360

 

In general it is a MAJOR issue that we do not have an API life-cycle document.

How do we gracefull yintroduce new a) APIs and b) fields in existing APIs. How do we do this with a maturity field.

How do we remove non-stable APis again, and how do we graduate APis to become stable.

Your other email goes into this direction, and we must take action on it.

I was under the impression that the lack of it allows per-case decisions to be taken.

No.

 

Re-reading the above, it looks like we are in a deadlock with decrecations in general.

It's at least non-trivial.

 

If we are forced not to remove the functionality in v1, then the deprecation will only

be able to declare that support is ending and no fixes/changes are planned.

To ease maintenance, we can extract all logic out (e.g. using future network binding plugin) and archive it.

Yes, this is the least we can do. IOW do everything in order to make the eventual removal easy.

Do you mind starting a doc to start discussing feature deprecation?

And because of semver we should split it into two sections: Preparations for removal. Complete removal.

 

I still hope exceptions could be considered, especially if no user is using it.

I'd be open to explore this for "exotic" options (which to me slirp is as well).

- Changing the implementation: Possibly rewire slirp to map to passt instead?

- Keep the field, make it a noop: Keep the API field to reject if it's being used.

This is btw why we also need a feature graduation procedure: If we introduce new features like hot plug lately), then we need a graceperiod in which we can change the api, even if the fields are part of an already stable API.

It would be great to see a SIG, SIG Arch maybe, to push on this end.

[Edward Haas]

Hi All,

The SLIRP binding deprecation discussions revealed that we cannot simply remove the functionality and code,

at least not without breaking KubeVirt API commitments.

As we are still interested in reducing the maintenance burden in regards to SLIRP code base,

the following reworked suggestion is raised:

- Notify about SLIRP deprecation, with the intent to declare the end of support for it.
  - API and functionality is preserved.

  - Enhancements or bugs will not be treated.

- Add a warning log/event and annotate the relevant struct in the client to warn users (on main).

- Extract out SLIRP codebase to an external network binding plugin.

  - The codebase, after proven to work as expected, will be archived and no future changes will be accepted.

Any feedback would be appreciated.

This is in the working, I hope we will be able to provide suggestions in this regard.

 

I still hope exceptions could be considered, especially if no user is using it.

I'd be open to explore this for "exotic" options (which to me slirp is as well).

- Changing the implementation: Possibly rewire slirp to map to passt instead?

- Keep the field, make it a noop: Keep the API field to reject if it's being used.

This is btw why we also need a feature graduation procedure: If we introduce new features like hot plug lately), then we need a graceperiod in which we can change the api, even if the fields are part of an already stable API.

It would be great to see a SIG, SIG Arch maybe, to push on this end.

I hope to see the focus moving from deprecation to this feature lifecycle & graduation.

While deprecation is important, it is hard to execute without feature graduation definition.

I believe we will see progress on both soon.

[Fabian Deutsch]

On Mon, Jul 31, 2023 at 1:13 PM Edward Haas <edw...@redhat.com> wrote:

Hi All,

The SLIRP binding deprecation discussions revealed that we cannot simply remove the functionality and code,

at least not without breaking KubeVirt API commitments.

As we are still interested in reducing the maintenance burden in regards to SLIRP code base,

the following reworked suggestion is raised:

- Notify about SLIRP deprecation, with the intent to declare the end of support for it.
  - API and functionality is preserved.

  - Enhancements or bugs will not be treated.

- Add a warning log/event and annotate the relevant struct in the client to warn users (on main).

All of the above makes sense

 

- Extract out SLIRP codebase to an external network binding plugin.

  - The codebase, after proven to work as expected, will be archived and no future changes will be accepted.

If slirp moves to a binding plugin, what would this mean for the API?

Awesome, please keep us updated!

[Edward Haas]

Hi All,

This is an update of the current status and a suggestion for the next actions.

Current status:

- Slirp has now a binding network plugin implementation.

- The current core API has a new backend implementation where it uses the sidecar from the plugin.

Suggested actions:

- Request for an exception to declare the core Slirp binding as deprecated.

- With the deprecation approval, mark the fields as deprecated and warn users (for v1.2).

- Make the core Slirp binding configuration a no-op in version v1.3.

Thanks,

Edy.

On Thu, Aug 10, 2023 at 12:56 PM Fabian Deutsch <fdeu...@redhat.com> wrote:

On Mon, Jul 31, 2023 at 1:13 PM Edward Haas <edw...@redhat.com> wrote:

Hi All,

The SLIRP binding deprecation discussions revealed that we cannot simply remove the functionality and code,

at least not without breaking KubeVirt API commitments.

As we are still interested in reducing the maintenance burden in regards to SLIRP code base,

the following reworked suggestion is raised:

- Notify about SLIRP deprecation, with the intent to declare the end of support for it.
  - API and functionality is preserved.

  - Enhancements or bugs will not be treated.

- Add a warning log/event and annotate the relevant struct in the client to warn users (on main).

All of the above makes sense

 

- Extract out SLIRP codebase to an external network binding plugin.

  - The codebase, after proven to work as expected, will be archived and no future changes will be accepted.

If slirp moves to a binding plugin, what would this mean for the API?

The current core API has been preserved.

In the backend we started using partial plugin mechanisms to implement it.

To be more specific, it uses a sidecar.

The proposal is online and can be reviewed: https://github.com/kubevirt/community/pull/251

Hopefully we can define an exception path for very old features that have not passed the lifecycle.

[Fabian Deutsch]

Edward,

On Thu, Dec 7, 2023 at 3:19 PM Edward Haas <edw...@redhat.com> wrote:

Hi All,

This is an update of the current status and a suggestion for the next actions.

Current status:

- Slirp has now a binding network plugin implementation.

- The current core API has a new backend implementation where it uses the sidecar from the plugin.

Suggested actions:

- Request for an exception to declare the core Slirp binding as deprecated.

- With the deprecation approval, mark the fields as deprecated and warn users (for v1.2).

- Make the core Slirp binding configuration a no-op in version v1.3.

As a user of slirp, how will I be able to use it in 1.3 (assuming that the in-tree implementation is gone by then)?

[Edward Haas]

Hi Fabian,

It seems I missed sending the response. Sorry.

On Fri, Dec 8, 2023 at 2:39 PM Fabian Deutsch <fdeu...@redhat.com> wrote:

Edward,

On Thu, Dec 7, 2023 at 3:19 PM Edward Haas <edw...@redhat.com> wrote:

Hi All,

This is an update of the current status and a suggestion for the next actions.

Current status:

- Slirp has now a binding network plugin implementation.

- The current core API has a new backend implementation where it uses the sidecar from the plugin.

Suggested actions:

- Request for an exception to declare the core Slirp binding as deprecated.

- With the deprecation approval, mark the fields as deprecated and warn users (for v1.2).

- Make the core Slirp binding configuration a no-op in version v1.3.

As a user of slirp, how will I be able to use it in 1.3 (assuming that the in-tree implementation is gone by then)?

Users can use the SLIRP binding plugin.

It has been already documented in the user-guide [1] [2].

[1] https://kubevirt.io/user-guide/virtual_machines/interfaces_and_networks/#slirp
[2] https://kubevirt.io/user-guide/virtual_machines/net_binding_plugins/slirp/#slirp-network-binding-plugin

I will go ahead and send an exception request email and follow the suggestion points above.

But if you see a problem with this, please let us know.

[Fabian Deutsch]

On Thu, Feb 15, 2024 at 9:58 AM Edward Haas <edw...@redhat.com> wrote:

Hi Fabian,

It seems I missed sending the response. Sorry.

On Fri, Dec 8, 2023 at 2:39 PM Fabian Deutsch <fdeu...@redhat.com> wrote:

Edward,

On Thu, Dec 7, 2023 at 3:19 PM Edward Haas <edw...@redhat.com> wrote:

Hi All,

This is an update of the current status and a suggestion for the next actions.

Current status:

- Slirp has now a binding network plugin implementation.

- The current core API has a new backend implementation where it uses the sidecar from the plugin.

Suggested actions:

- Request for an exception to declare the core Slirp binding as deprecated.

- With the deprecation approval, mark the fields as deprecated and warn users (for v1.2).

- Make the core Slirp binding configuration a no-op in version v1.3.

As a user of slirp, how will I be able to use it in 1.3 (assuming that the in-tree implementation is gone by then)?

Users can use the SLIRP binding plugin.

It has been already documented in the user-guide [1] [2].

[1] https://kubevirt.io/user-guide/virtual_machines/interfaces_and_networks/#slirp
[2] https://kubevirt.io/user-guide/virtual_machines/net_binding_plugins/slirp/#slirp-network-binding-plugin

I will go ahead and send an exception request email and follow the suggestion points above.

But if you see a problem with this, please let us know.

No, thanks, this looks like a reasonable path forward for such a feature.

+1 on setting the standard about exception requests :)

[Edward Haas]

Hello All,

The sig-network would like to request a special exception to deprecate the Slirp network core binding.

All details, including reasoning and implementation has been posted as a PR [1].

Please feel free to give feedback in this thread or on the PR directly.

[1] https://github.com/kubevirt/kubevirt/pull/11701

Thank you,

Edy.

[Edward Haas]

Hello All,

This is a kind reminder to feedback on this deprecation & removal exception.

The PR is approved & lgtm by the sig-network at the moment but will also like to receive feedback from

other maintainers and interested parties.

Thank you,

Edy.