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

remove feature gate SupportIPVSProxyMode. #94514

Merged
merged 1 commit into from Sep 10, 2020
Merged

remove feature gate SupportIPVSProxyMode. #94514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 0 additions & 7 deletions pkg/features/kube_features.go
View file
Open in desktop
Expand Up @@ -194,12 +194,6 @@ const (
// Postpone deletion of a PV or a PVC when they are being used
StorageObjectInUseProtection featuregate.Feature = "StorageObjectInUseProtection"

// owner: @m1093782566
// GA: v1.11
//
// Implement IPVS-based in-cluster service load balancing
SupportIPVSProxyMode featuregate.Feature = "SupportIPVSProxyMode"

// owner: @dims, @derekwaynecarr
// alpha: v1.10
// beta: v1.14
Expand Down Expand Up @@ -682,7 +676,6 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
CSINodeInfo: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.19
BlockVolume: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.20
StorageObjectInUseProtection: {Default: true, PreRelease: featuregate.GA},
SupportIPVSProxyMode: {Default: true, PreRelease: featuregate.GA},
Copy link
Member

Choose a reason for hiding this comment

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

Based on https://kubernetes.io/docs/reference/using-api/deprecation-policy/#deprecation I think we need to set LockToDefault for 2 releases before we can remove it.

Copy link
Member

Choose a reason for hiding this comment

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

Deprecation talks about announcing the deprecation (which we did in CHANGELOG/CHANGELOG-1.10.md) and about a warning when used (which happens by nature of being marked GA).

Deprecation also says "must be documented in both in the release notes and the corresponding CLI help" but CLI help filters out GA and deprecated gates.

Given the error is in the gates lib, I think we should be OK to delete this, and then we can fix the doc and the lib.

Disaagree?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think in this case I agree with that, as there are no further references in other parts of the code about the Feature gate.

But in case of #94581 I actually don't know if we should remove (as the Feature Gate went to GA in v1.18) or only mark it as LockToDefault: true, as that PR also removes some logic from the code. This also wasn't announced in the CHANGELOG/CHANGELOG-1.18.md when it turned into GA, should it be?

Copy link
Member

Choose a reason for hiding this comment

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

a warning when used (which happens by nature of being marked GA).

TIL :)

In rule #7 of the Deprecation Policy:

GA: the feature gate is deprecated (see "Deprecation") and becomes non-operational.

Does "non-operational" mean we don't check the gate anywhere or does it also imply the gate is locked? If the former, then sgtm to remove it now.

SupportPodPidsLimit: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.21
SupportNodePidsLimit: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.21
HyperVContainer: {Default: false, PreRelease: featuregate.Alpha},
Expand Down
19 changes: 3 additions & 16 deletions pkg/proxy/ipvs/README.md
View file
Open in desktop
Expand Up @@ -334,29 +334,20 @@ export KUBE_PROXY_MODE=ipvs

### Cluster Created by Kubeadm

If you are using kubeadm with a [configuration file](https://kubernetes.io/docs/reference/setup-tools/kubeadm/kubeadm-init/#config-file), you have to add `mode: ipvs` and also add `SupportIPVSProxyMode: true` below the `kubeProxy` field as part of the kubeadm configuration.
Copy link
Contributor

Choose a reason for hiding this comment

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

This part you can remove the reference to the feature flag, but keep the 'mode: ipvs' as this is still true

If you are using kubeadm with a [configuration file](https://kubernetes.io/docs/reference/setup-tools/kubeadm/kubeadm-init/#config-file), you have to add `mode: ipvs` below the `kubeProxy` field as part of the kubeadm configuration.

```yaml
...
kubeProxy:
config:
featureGates:
SupportIPVSProxyMode: true
mode: ipvs
...
```
Note that in Kubernetes 1.11 and later, `SupportIPVSProxyMode` is set to `true` by default.

before running

`kube init --config <path_to_configuration_file>`

If you are using Kubernetes v1.8, you can also add the flag `--feature-gates=SupportIPVSProxyMode=true` (deprecated since v1.9) in `kubeadm init` command

```
kubeadm init --feature-gates=SupportIPVSProxyMode=true
```

to specify the ipvs mode before deploying the cluster.

**Notes**
Expand Down Expand Up @@ -412,14 +403,10 @@ UDP 10.0.0.10:53 rr

Use the following check list to help you solve the problems:

**1. Enable IPVS feature gateway**

For Kubernetes v1.10 and later, feature gate `SupportIPVSProxyMode` is set to `true` by default. However, you need to enable `--feature-gates=SupportIPVSProxyMode=true` explicitly for Kubernetes before v1.10.

**2. Specify proxy-mode=ipvs**
**1. Specify proxy-mode=ipvs**

Check whether the kube-proxy mode has been set to `ipvs`.

**3. Install required kernel modules and packages**
**2. Install required kernel modules and packages**

Check whether the IPVS required kernel modules have been compiled into the kernel and packages installed. (see Prerequisite)