kubeadm: preflight check for enabled swap #50840

kad · 2017-08-17T11:21:29Z

What this PR does / why we need it:
Recent versions of kubelet require special flags if runned
on the system with enabled swap. Thus, remind user about either
disabling swap or add appropriate flag to kubelet settings

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

Release note:

NONE

k8s-ci-robot · 2017-08-17T11:21:36Z

Hi @kad. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

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. I understand the commands that are listed here.

dmmcquay

Just a question about this potentially being a warning over an error.

dmmcquay · 2017-08-17T14:34:35Z

cmd/kubeadm/app/preflight/checks.go

+		buf = append(buf, scanner.Text())
+	}
+	if len(buf) > 1 {
+		return []error{fmt.Errorf("Running with swap on is not supported. Please disable swap or set kubelet's --fail-swap-on flag to false.")}, nil


Should this potentially be a warning instead of an error? Will the kubelet fail if there is swap?

Note the return values order. it will return that message as warning now (errors is nil).
Kubelet actually will fail by default, but if user use --fail-swap-on=false it will work, so this check is inteded to be warning, as I don't see reason to force user to turn off swap if user knows consequences.

Yep, I just read it incorrectly.

dmmcquay · 2017-08-17T14:35:19Z

/ok-to-test

dmmcquay · 2017-08-17T18:14:14Z

cmd/kubeadm/app/preflight/checks.go

+		buf = append(buf, scanner.Text())
+	}
+	if len(buf) > 1 {
+		return []error{fmt.Errorf("Running with swap on is not supported. Please disable swap or set kubelet's --fail-swap-on flag to false.")}, nil


Yep, I just read it incorrectly.

dmmcquay · 2017-08-17T18:14:24Z

/lgtm

luxas · 2017-08-18T21:36:30Z

cc @kubernetes/sig-node-pr-reviews @Random-Liu @yujuhong
Could we move this into the system verification check instead?

kad · 2017-08-22T11:24:15Z

Rebased.

kad · 2017-08-28T12:10:21Z

Anyone from sig-node to give an opinion about suggestion from @luxas ?

dmmcquay · 2017-08-29T16:53:10Z

/test pull-kubernetes-e2e-gce-etcd3

krousey · 2017-08-29T17:11:33Z

cmd/kubeadm/app/preflight/checks.go

+	scanner := bufio.NewScanner(f)
+	for scanner.Scan() {
+		buf = append(buf, scanner.Text())
+	}


Shouldn't you also be checking the scanner.Err()?

Recent versions of kubelet require special flags if runned on the system with enabled swap. Thus, remind user about either disabling swap or add appropriate flag to kubelet settings

kad · 2017-08-29T18:45:30Z

@krousey possibility that scanner.Err will be non-nil is quite small, but possible. Added check for it. Thanks, good catch!

kad · 2017-08-30T11:56:58Z

All what I see in failure logs seems to be flakes: 1. gcloud.compute.networks.create - quota exceeded on cluster. 2. Firewall check for e2e cluster setup. seems to be also sig-network flake related.

Can some org member re-trigger it enable bot to re trigger automatically ?

kad · 2017-08-31T21:39:15Z

can someone kick bot to /retest flakes?

dmmcquay · 2017-08-31T21:59:28Z

/test pull-kubernetes-e2e-gce-gpu
/test pull-kubernetes-e2e-gce-bazel
/test pull-kubernetes-e2e-gce-etcd3

kad · 2017-09-01T09:42:39Z

timed out waiting for the condition in resource quota. Unrelated flake it seems.

dmmcquay · 2017-09-01T16:22:53Z

/test pull-kubernetes-e2e-gce-etcd3

kad · 2017-09-01T17:17:54Z

thx @dmmcquay for kicking test bot.
So, what do we do now with that PR ? code freeze is soon, it would be good to make decision.
May I suggest that if there is no comments from sig-node people, we can merge it as is now, and then move it to other place if it will be need ?

luxas

/lgtm

dmmcquay · 2017-09-01T22:39:05Z

/lgtm

luxas · 2017-09-01T22:40:15Z

/approve no-issue

k8s-github-robot · 2017-09-01T22:40:55Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dmmcquay, kad, luxas

Associated issue requirement bypassed by: luxas

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

Needs approval from an approver in each of these OWNERS Files:

~~cmd/kubeadm/OWNERS~~ [luxas]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

CaoShuFeng · 2017-09-02T14:31:39Z

/test pull-kubernetes-verify

luxas · 2017-09-02T20:35:06Z

/retest

k8s-ci-robot · 2017-09-02T23:47:21Z

@kad: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-e2e-kops-aws	`2e12787`	link	`/test pull-kubernetes-e2e-kops-aws`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

k8s-github-robot · 2017-09-02T23:53:33Z

Automatic merge from submit-queue

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 17, 2017

k8s-github-robot assigned dmmcquay and krousey Aug 17, 2017

k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. labels Aug 17, 2017

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 17, 2017

dmmcquay suggested changes Aug 17, 2017

View reviewed changes

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 17, 2017

dmmcquay approved these changes Aug 17, 2017

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 17, 2017

k8s-ci-robot added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Aug 18, 2017

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 22, 2017

kad force-pushed the swapcheck branch from ac047b2 to 46449c1 Compare August 22, 2017 11:23

k8s-github-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Aug 22, 2017

krousey reviewed Aug 29, 2017

View reviewed changes

kubeadm: preflight check for enabled swap

2e12787

Recent versions of kubelet require special flags if runned on the system with enabled swap. Thus, remind user about either disabling swap or add appropriate flag to kubelet settings

kad force-pushed the swapcheck branch from 46449c1 to 2e12787 Compare August 29, 2017 18:44

luxas approved these changes Sep 1, 2017

View reviewed changes

k8s-ci-robot assigned luxas Sep 1, 2017

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 1, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 1, 2017

abgworrall modified the milestone: v1.8 Sep 2, 2017

k8s-github-robot merged commit 2dd659d into kubernetes:master Sep 2, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kubeadm: preflight check for enabled swap #50840

kubeadm: preflight check for enabled swap #50840

kad commented Aug 17, 2017

k8s-ci-robot commented Aug 17, 2017

dmmcquay left a comment

dmmcquay Aug 17, 2017

kad Aug 17, 2017

dmmcquay Aug 17, 2017

dmmcquay commented Aug 17, 2017

dmmcquay Aug 17, 2017

dmmcquay commented Aug 17, 2017

luxas commented Aug 18, 2017

kad commented Aug 22, 2017

kad commented Aug 28, 2017 •

edited

Loading

dmmcquay commented Aug 29, 2017

krousey Aug 29, 2017

kad commented Aug 29, 2017

kad commented Aug 30, 2017

kad commented Aug 31, 2017

dmmcquay commented Aug 31, 2017

kad commented Sep 1, 2017

dmmcquay commented Sep 1, 2017

kad commented Sep 1, 2017

luxas left a comment

dmmcquay commented Sep 1, 2017

luxas commented Sep 1, 2017

k8s-github-robot commented Sep 1, 2017

CaoShuFeng commented Sep 2, 2017

luxas commented Sep 2, 2017

k8s-ci-robot commented Sep 2, 2017 •

edited

Loading

k8s-github-robot commented Sep 2, 2017

kubeadm: preflight check for enabled swap #50840

kubeadm: preflight check for enabled swap #50840

Conversation

kad commented Aug 17, 2017

k8s-ci-robot commented Aug 17, 2017

dmmcquay left a comment

Choose a reason for hiding this comment

dmmcquay Aug 17, 2017

Choose a reason for hiding this comment

kad Aug 17, 2017

Choose a reason for hiding this comment

dmmcquay Aug 17, 2017

Choose a reason for hiding this comment

dmmcquay commented Aug 17, 2017

dmmcquay Aug 17, 2017

Choose a reason for hiding this comment

dmmcquay commented Aug 17, 2017

luxas commented Aug 18, 2017

kad commented Aug 22, 2017

kad commented Aug 28, 2017 • edited Loading

dmmcquay commented Aug 29, 2017

krousey Aug 29, 2017

Choose a reason for hiding this comment

kad commented Aug 29, 2017

kad commented Aug 30, 2017

kad commented Aug 31, 2017

dmmcquay commented Aug 31, 2017

kad commented Sep 1, 2017

dmmcquay commented Sep 1, 2017

kad commented Sep 1, 2017

luxas left a comment

Choose a reason for hiding this comment

dmmcquay commented Sep 1, 2017

luxas commented Sep 1, 2017

k8s-github-robot commented Sep 1, 2017

CaoShuFeng commented Sep 2, 2017

luxas commented Sep 2, 2017

k8s-ci-robot commented Sep 2, 2017 • edited Loading

k8s-github-robot commented Sep 2, 2017

kad commented Aug 28, 2017 •

edited

Loading

k8s-ci-robot commented Sep 2, 2017 •

edited

Loading