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 alpha support for Hyper-v on Docker for Windows #94490

Closed
jsturtevant opened this issue Sep 3, 2020 · 18 comments · Fixed by #95505 or #97141
Closed

Remove alpha support for Hyper-v on Docker for Windows #94490

jsturtevant opened this issue Sep 3, 2020 · 18 comments · Fixed by #95505 or #97141
Assignees
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/windows Categorizes an issue or PR as relevant to SIG Windows.

Comments

@jsturtevant
Copy link
Contributor

What happened:
Hyper-v support was added as an experimental feature via annotations for docker. Hyper-v support is now focused on Containerd because of better support of HCSv2 in Containerd (for example allows for multiple containers in a pod in hyper-v).

There is still annotation support that should be removed in all the various locations:

func ShouldIsolatedByHyperV(annotations map[string]string) bool {

What you expected to happen:
hyper-v support for docker will not go past alpha so the annotations should be depreciated following the guidelines at https://kubernetes.io/docs/reference/using-api/deprecation-policy/

How to reproduce it (as minimally and precisely as possible):

Anything else we need to know?:

Environment:

  • Kubernetes version (use kubectl version):
  • Cloud provider or hardware configuration:
  • OS (e.g: cat /etc/os-release): Windows
  • Kernel (e.g. uname -a):
  • Install tools:
  • Network plugin and version (if this is a network-related bug):
  • Others:

/sig windows

@jsturtevant jsturtevant added the kind/bug Categorizes issue or PR as related to a bug. label Sep 3, 2020
@k8s-ci-robot k8s-ci-robot added the sig/windows Categorizes an issue or PR as relevant to SIG Windows. label Sep 3, 2020
@neolit123
Copy link
Member

/remove-kind bug
/kind deprecation cleanup
/sig node

@k8s-ci-robot k8s-ci-robot added kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. sig/node Categorizes an issue or PR as relevant to SIG Node. and removed kind/bug Categorizes issue or PR as related to a bug. labels Sep 3, 2020
@wawa0210
Copy link
Contributor

wawa0210 commented Sep 4, 2020

I can try to contribute to solve this issue

/assign

@wawa0210
Copy link
Contributor

wawa0210 commented Sep 4, 2020

Do we need to discuss whether we confirm that we need to remove this feature in the kubernetes code? If it is removed, it will not support running hyper-v-based windows container through docker.

@jsturtevant
Copy link
Contributor Author

We can add it to the agenda for sig-windows next week to confirm. There is no path forward for hyper-v containers with docker so it should be ok to remove via standard depreciation mechanisms.

We have been working to get hyper-v working via containerd (still work in progress) and have tests passes: https://testgrid.k8s.io/sig-windows-containerd#aks-engine-azure-windows-master-containerd-hyperv

@jsturtevant
Copy link
Contributor Author

@neolit123 What is the process for announcing and removing an experimental annotation? I read through https://kubernetes.io/docs/reference/using-api/deprecation-policy/ but it mostly refers to api changes.

@wawa0210
Copy link
Contributor

wawa0210 commented Sep 4, 2020

I submitted a simple pr for this issue, we can follow up the results of the discussion next week for further operations

@wawa0210
Copy link
Contributor

wawa0210 commented Sep 4, 2020

@neolit123 What is the process for announcing and removing an experimental annotation? I read through https://kubernetes.io/docs/reference/using-api/deprecation-policy/ but it mostly refers to api changes.

I also read this document and found that there is a short description related to this issue

https://kubernetes.io/docs/reference/using-api/deprecation-policy/#deprecating-a-feature-or-behavior

I am confused about this point. It seems that the feature of hyper-v can be removed (because it has been in alpha for a long time, experimental feature), I don’t know if my understanding is correct.

@neolit123
Copy link
Member

alpha feature gates can be removed without a deprecation period in release N+1 if added in release N - i.e. it could have been removed in 1.11 as per the minimum policy.
this is the recommended minimum and exceptions can happen...the feature existed for 10 releases and Windows containers support existed for a few releases as well, which means you could have production clusters using the feature.

my recommendation would be to announce this as deprecated in 1.20 and remove in 1.21.
PRs with release-notes prefixed with "action required: ..." would end up on top of the release change logs to alert users.

ideally sig-windows should decide on the timing.
cc @feiskyer @benmoss @ddebroy

@feiskyer
Copy link
Member

feiskyer commented Sep 5, 2020

my recommendation would be to announce this as deprecated in 1.20 and remove in 1.21.

+1. Could we add a warning log and an announcement in v1.20 and then remove it in v1.21?

@wawa0210
Copy link
Contributor

wawa0210 commented Sep 8, 2020

If sig-windows has the latest discussion results, please let me know and I can try to follow up this issue

@jsturtevant
Copy link
Contributor Author

We discussed this at sig-windows and agree should be a log warning for depreciation in 1.20 and remove support in v1.21.

@wawa0210 can you update the PR to log the warning and add the appropriate release note?

@wawa0210
Copy link
Contributor

@wawa0210 can you update the PR to log the warning and add the appropriate release note?

#95505

i open a new pr for log warning message for this issue

@jsturtevant
Copy link
Contributor Author

jsturtevant commented Oct 20, 2020

This should stay open as a tracking issue untill the feature is removed in 1.21

@jsturtevant
Copy link
Contributor Author

/reopen

@k8s-ci-robot k8s-ci-robot reopened this Oct 20, 2020
@k8s-ci-robot
Copy link
Contributor

@jsturtevant: Reopened this issue.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Oct 20, 2020
@k8s-ci-robot
Copy link
Contributor

@jsturtevant: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 18, 2021
@jsturtevant
Copy link
Contributor Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/windows Categorizes an issue or PR as relevant to SIG Windows.
Projects
None yet
6 participants