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

kubeadm: fix RequiredIPVSKernelModulesAvailable warning message #74033

Conversation

@bart0sh
Copy link
Contributor

commented Feb 13, 2019

What type of PR is this?

/kind bug

What this PR does / why we need it:

RequiredIPVSKernelModulesAvailable warning confuses users suggesting
that the IPVS proxier will not be used, which is not always the case.

Made the warning message less confusing:

        [WARNING RequiredIPVSKernelModulesAvailable]:
The IPVS proxier may not be used because the following required kernel
modules are not loaded: [ip_vs_rr ip_vs_wrr ip_vs_sh]
or no builtin kernel ipvs support was found: map[ip_vs_wrr:{}
ip_vs_sh:{} nf_conntrack_ipv4:{} ip_vs:{} ip_vs_rr:{}].
However, these modules may be loaded automatically by kube-proxy for you
if they are available on your system.

To verify IPVS support:

   Run "lsmod | grep 'ip_vs\|nf_conntrack'" and verify each of the above
modules are listed.

If they are not listed, you can use the following methods to load them:

1. For each missing module run 'modprobe $modulename' (e.g., 'modprobe
ip_vs', 'modprobe ip_vs_rr', ...)
2. If 'modprobe $modulename' returns an error, you will need to install
the missing module support for your kernel.

Which issue(s) this PR fixes:
Fixes kubernetes/kubeadm#975

Special notes for your reviewer: this PR is a proper fix for this issue in my opinion. Feel free to accept it instead of this one.

Does this PR introduce a user-facing change?:

kubeadm: improved RequiredIPVSKernelModulesAvailable warning message
@k8s-ci-robot k8s-ci-robot requested review from m1093782566 and thockin Feb 13, 2019
@k8s-ci-robot k8s-ci-robot added sig/network and removed needs-sig labels Feb 13, 2019
@bart0sh bart0sh force-pushed the bart0sh:PR0060-kubeadm-975-RequiredIPVSKernelModulesAvailable-warning-seems-confusing branch from 6cdd696 to a397218 Feb 13, 2019
@bart0sh

This comment has been minimized.

Copy link
Contributor Author

commented Feb 13, 2019

@k8s-ci-robot k8s-ci-robot requested review from neolit123 and timothysc Feb 13, 2019
Copy link
Member

left a comment

thanks for the message update @bart0sh
i think this is all we can do for now.

added some minor nits.

pkg/util/ipvs/kernelcheck_linux.go Outdated Show resolved Hide resolved
pkg/util/ipvs/kernelcheck_linux.go Outdated Show resolved Hide resolved
pkg/util/ipvs/kernelcheck_linux.go Outdated Show resolved Hide resolved
@neolit123

This comment has been minimized.

Copy link
Member

commented Feb 13, 2019

/kind cleanup
/priority important-longterm

RequiredIPVSKernelModulesAvailable warning confuses users suggesting
that the IPVS proxier will not be used, which is not always the case.

Made the warning message less confusing:

        [WARNING RequiredIPVSKernelModulesAvailable]:
The IPVS proxier may not be used because the following required kernel
modules are not loaded: [ip_vs_rr ip_vs_wrr ip_vs_sh]
or no builtin kernel ipvs support was found: map[ip_vs_wrr:{}
ip_vs_sh:{} nf_conntrack_ipv4:{} ip_vs:{} ip_vs_rr:{}].
However, these modules may be loaded automatically by kube-proxy for you
if they are available on your system.

To verify IPVS support:

   Run "lsmod | grep 'ip_vs\|nf_conntrack'" and verify each of the above
modules are listed.

If they are not listed, you can use the following methods to load them:

1. For each missing module run 'modprobe $modulename' (e.g., 'modprobe
ip_vs', 'modprobe ip_vs_rr', ...)
2. If 'modprobe $modulename' returns an error, you will need to install
the missing module support for your kernel.

Fixes: kubernetes/kubeadm#975
@bart0sh bart0sh force-pushed the bart0sh:PR0060-kubeadm-975-RequiredIPVSKernelModulesAvailable-warning-seems-confusing branch from a397218 to 09a2e49 Feb 13, 2019
@bart0sh

This comment has been minimized.

Copy link
Contributor Author

commented Feb 14, 2019

Kindly ping reviewers for lgtm/approval

Copy link
Member

left a comment

/lgtm
after the Dedent fix in util/ipvs. thanks!

up to the IPVS package approvers at this point.

@k8s-ci-robot k8s-ci-robot added the lgtm label Feb 14, 2019
@neolit123

This comment has been minimized.

Copy link
Member

commented Feb 14, 2019

/assign m1093782566

Copy link
Member

left a comment

Thanks @bart0sh
/lgtm

Copy link
Member

left a comment

/lgtm
/approve

@timothysc timothysc added the approved label Mar 4, 2019
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: bart0sh, timothysc

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

@bart0sh

This comment has been minimized.

Copy link
Contributor Author

commented Mar 4, 2019

/test pull-kubernetes-e2e-gce

@bart0sh

This comment has been minimized.

Copy link
Contributor Author

commented Mar 4, 2019

/test pull-kubernetes-integration

@fejta-bot

This comment has been minimized.

Copy link

commented Mar 5, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

2 similar comments
@fejta-bot

This comment has been minimized.

Copy link

commented Mar 5, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@fejta-bot

This comment has been minimized.

Copy link

commented Mar 5, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@bart0sh

This comment has been minimized.

Copy link
Contributor Author

commented Mar 5, 2019

/test pull-kubernetes-integration

@k8s-ci-robot k8s-ci-robot merged commit 43616fc into kubernetes:master Mar 5, 2019
18 checks passed
18 checks passed
cla/linuxfoundation bart0sh authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-godeps Skipped
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Context retired without replacement.
Details
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped
tide In merge pool.
Details
@stewart-yu

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2019

Thanks for your improvement @bart0sh
LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.