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

Revert "Add instructions for switching to iptables-legacy" #19773

Merged
merged 1 commit into from
Mar 26, 2020

Conversation

danwinship
Copy link
Contributor

As of kube 1.17, kubeadm supports systems using iptables in nft mode, but we forgot to remove the warnings claiming that it doesn't.

This patch removes the sentence "RHEL 8 does not support switching to legacy mode, and is therefore incompatible with current kubeadm packages.". However, nothing in the doc had claimed that RHEL 8 was supported before this, so I don't know if there are other issues, so I didn't add it to the list of supported platforms at the top.

This should be backported to the 1.17 docs. I'm not sure if that requires more than just "/cherry-pick release-1.17"? Note that this doesn't require any sort of belated release notes update, because it was already in the 1.17 release notes ("The official kube-proxy image (used by kubeadm, among other things) is now compatible with systems running iptables 1.8 in “nft” mode, and will autodetect which mode it should use.")

/cc @praseodym

…#16271)"

This reverts commit 9cdaf4e.

As of kube 1.17, kubeadm is compatible with iptables-nft
@k8s-ci-robot
Copy link
Contributor

@danwinship: GitHub didn't allow me to request PR reviews from the following users: praseodym.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

As of kube 1.17, kubeadm supports systems using iptables in nft mode, but we forgot to remove the warnings claiming that it doesn't.

This patch removes the sentence "RHEL 8 does not support switching to legacy mode, and is therefore incompatible with current kubeadm packages.". However, nothing in the doc had claimed that RHEL 8 was supported before this, so I don't know if there are other issues, so I didn't add it to the list of supported platforms at the top.

This should be backported to the 1.17 docs. I'm not sure if that requires more than just "/cherry-pick release-1.17"? Note that this doesn't require any sort of belated release notes update, because it was already in the 1.17 release notes ("The official kube-proxy image (used by kubeadm, among other things) is now compatible with systems running iptables 1.8 in “nft” mode, and will autodetect which mode it should use.")

/cc @praseodym

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 cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Mar 21, 2020
@netlify
Copy link

netlify bot commented Mar 21, 2020

Deploy preview for kubernetes-io-master-staging ready!

Built with commit 38ab1d3

https://deploy-preview-19773--kubernetes-io-master-staging.netlify.com

@praseodym
Copy link
Contributor

Haven’t tested the nftables compatibility myself, but I assume that has been well tested already.

/lgtm

@k8s-ci-robot
Copy link
Contributor

@praseodym: changing LGTM is restricted to collaborators

In response to this:

Haven’t tested the nftables compatibility myself, but I assume that has been well tested already.

/lgtm

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.

@neolit123
Copy link
Member

neolit123 commented Mar 21, 2020

@kubernetes/sig-network-pr-reviews

As of kube 1.17, kubeadm supports systems using iptables in nft mode, but we forgot to remove the warnings claiming that it doesn't.

this was a passage added with the approval of sig-network and sig-release.
i'm fine with removing it if is claimed that it works since 1.17.

This patch removes the sentence "RHEL 8 does not support switching to legacy mode, and is therefore incompatible with current kubeadm packages.". However, nothing in the doc had claimed that RHEL 8 was supported before this, so I don't know if there are other issues, so I didn't add it to the list of supported platforms at the top.

sig-release creates kubeadm packages for RedHat* and Debian*.
there are some distro-family specifics in those packages, but overall kubeadm is distro-agnostic.

the kubeadm docs can include coverage for kube-proxy or distro specific issues only on a best effort basis, as we cannot test all distros out there.

This should be backported to the 1.17 docs.

has to be added to the dev-1.18 branch too, but overall k/website does not maintain older docs release branches.

@kubernetes/sig-docs-en-owners

@k8s-ci-robot k8s-ci-robot added the sig/network Categorizes an issue or PR as relevant to SIG Network. label Mar 21, 2020
@neolit123
Copy link
Member

/sig network cluster-lifecycle

@k8s-ci-robot k8s-ci-robot added the sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. label Mar 21, 2020
@sftim
Copy link
Contributor

sftim commented Mar 22, 2020

has to be added to the dev-1.18 branch too, but overall k/website does not maintain older docs release branches.

AIUI it'd be OK to target master and then when dev-1.18 merges into master then the change will persist.

@sftim
Copy link
Contributor

sftim commented Mar 23, 2020

@kubernetes/sig-cluster-lifecycle-pr-reviews - happy to give this a /lgtm ?

@neolit123
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 23, 2020
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sftim

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 Mar 23, 2020
@VineethReddy02
Copy link
Contributor

/hold
pending v1.18 Kubernetes release

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 25, 2020
@VineethReddy02
Copy link
Contributor

/hold cancel
v1.18 release is out.

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 26, 2020
@k8s-ci-robot k8s-ci-robot merged commit 8830000 into kubernetes:master Mar 26, 2020
fancc pushed a commit to fancc/website that referenced this pull request Apr 26, 2020
…#16271)" (kubernetes#19773)

This reverts commit 9cdaf4e.

As of kube 1.17, kubeadm is compatible with iptables-nft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/network Categorizes an issue or PR as relevant to SIG Network. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants