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.

Sign up for GitHub

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

Jump to bottom

Action Required: Windows hyper-v container featuregate is deprecated in 1.20 and will be removed in 1.21 #95505

Merged
merged 1 commit into from
Oct 20, 2020
Merged

Action Required: Windows hyper-v container featuregate is deprecated in 1.20 and will be removed in 1.21 #95505

merged 1 commit into from
Oct 20, 2020

Conversation

wawa0210
Copy link
Contributor

@wawa0210 wawa0210 commented Oct 13, 2020 •
edited
Loading

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #94490

Special notes for your reviewer:
Warn that hyper-v featuregate will be deprecated in 1.20 and will be removed in 1.21

According to the results of sig-windows discussion, the hyper-v featuregate will be removed in version 1.21, so a warning message will be given in version 1.20

Does this PR introduce a user-facing change?:

Action Required: Windows hyper-v container featuregate is deprecated in 1.20 and will be removed in 1.21

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

/assign @derekwaynecarr

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Oct 13, 2020
@k8s-ci-robot
Copy link
Contributor

@wawa0210: 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.

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Oct 13, 2020
@k8s-ci-robot k8s-ci-robot added area/kubelet sig/node Categorizes an issue or PR as relevant to SIG Node. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Oct 13, 2020
@wawa0210 wawa0210 mentioned this pull request Oct 13, 2020
Closed
@wawa0210
Copy link
Contributor Author

/sig windows

@k8s-ci-robot k8s-ci-robot added the sig/windows Categorizes an issue or PR as relevant to SIG Windows. label Oct 13, 2020
@jsturtevant
Copy link
Contributor

I think the feature flag can be marked as depreciated which will also give a warning. Adding the log message to the actual use of the annotations makes sense as you have done.

https://github.com/kubernetes/kubernetes/blob/68b81f22d1243edd3db52e7b142441f450938372/pkg/features/kube_features.go#L678

Example:

https://github.com/kubernetes/kubernetes/blob/68b81f22d1243edd3db52e7b142441f450938372/pkg/features/kube_features.go#L742

jsturtevant
jsturtevant reviewed Oct 19, 2020
View reviewed changes
pkg/kubelet/apis/well_known_annotations_windows.go Outdated
@@ -34,6 +35,8 @@ const (

// ShouldIsolatedByHyperV returns true if a windows container should be run with hyperv isolation.
func ShouldIsolatedByHyperV(annotations map[string]string) bool {
klog.Warningf("The hyper-v FeatureGate will be deprecated in 1.20 and removed in 1.21")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
klog.Warningf("The hyper-v FeatureGate will be deprecated in 1.20 and removed in 1.21")
klog.Warningf("The hyper-v FeatureGate is deprecated in 1.20 and will be removed in 1.21")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree with the suggestion

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx comments~ done

@jsturtevant
Copy link
Contributor

The release note should also include Action required. Something like Action Required: Windows hyper-v container featuregate is deprecated in 1.20 and will be removed in 1.21

@marosset
Copy link
Contributor

/milestone v1.20

@k8s-ci-robot k8s-ci-robot added this to the v1.20 milestone Oct 19, 2020
@marosset
Copy link
Contributor

@wawa0210 please update the release notes as James mentioned and then we can get this change merged. Thanks!

@k8s-ci-robot k8s-ci-robot added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Oct 20, 2020
@wawa0210
Copy link
Contributor Author

@wawa0210 please update the release notes as James mentioned and then we can get this change merged. Thanks!

done

@wawa0210 wawa0210 changed the title Warn that hyper-v featuregate will be deprecated in 1.20 and will be removed in 1.21 Action Required: Windows hyper-v container featuregate is deprecated in 1.20 and will be removed in 1.21 Oct 20, 2020
@wawa0210
Copy link
Contributor Author

/test pull-kubernetes-e2e-azure-disk-windows

michmike
michmike approved these changes Oct 20, 2020
View reviewed changes
Copy link
Contributor

@michmike michmike left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 20, 2020
@derekwaynecarr
Copy link
Member

kubelet changes lgtm.

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: derekwaynecarr, michmike, wawa0210

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 20, 2020
@k8s-ci-robot k8s-ci-robot merged commit 0900340 into kubernetes:master Oct 20, 2020
@marosset
Copy link
Contributor

Thanks @wawa0210 !

@wawa0210 wawa0210 deleted the fix-94490 branch October 20, 2020 23:01
@wawa0210 wawa0210 mentioned this pull request Dec 9, 2020
Merged
jsturtevant added a commit to jsturtevant/website that referenced this pull request Jul 15, 2021
@jsturtevant
Remove annotation for Experimental Windows Hyper-v
fe7771a 
removed in 1.21: kubernetes/kubernetes#95505
@jsturtevant jsturtevant mentioned this pull request Jul 15, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michmike michmike michmike approved these changes

@smarterclayton smarterclayton Awaiting requested review from smarterclayton

@yujuhong yujuhong Awaiting requested review from yujuhong

@jsturtevant jsturtevant Awaiting requested review from jsturtevant

Assignees

@derekwaynecarr derekwaynecarr

@michmike michmike

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubelet cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/windows Categorizes an issue or PR as relevant to SIG Windows. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
v1.20
Development

Successfully merging this pull request may close these issues.

Remove alpha support for Hyper-v on Docker for Windows
6 participants
@wawa0210 @k8s-ci-robot @jsturtevant @marosset @derekwaynecarr @michmike