vendor: update system-validators to v1.3.0 #96378

neolit123 · 2020-11-09T16:11:53Z

What this PR does / why we need it:

Includes the following changes to kernel validation:

add required options: CGROUP_PIDS, FAIR_GROUP_SCHED
add optional options: CFS_BANDWIDTH, CGROUP_HUGETLB

Which issue(s) this PR fixes:

Special notes for your reviewer:
NONE

Does this PR introduce a user-facing change?:

kubeadm: amend the node kernel validation to treat CGROUP_PIDS, FAIR_GROUP_SCHED as required and CFS_BANDWIDTH, CGROUP_HUGETLB as optional

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

neolit123 · 2020-11-09T16:12:50Z

/kind feature
/sig cluster-lifecycle node
/priority important-soon
/milestone v1.20

neolit123 · 2020-11-09T16:14:21Z

vendor/k8s.io/system-validators/validators/types_unix.go

@@ -48,11 +49,14 @@ var DefaultSysSpec = SysSpec{
 			{Name: "PROC_FS"},
 			{Name: "NETFILTER_XT_TARGET_REDIRECT", Aliases: []string{"IP_NF_TARGET_REDIRECT"}},
 			{Name: "NETFILTER_XT_MATCH_COMMENT"},
+			{Name: "FAIR_GROUP_SCHED"},


WRT the new required fields:
kubernetes/kubeadm#2335

@neolit123

both these PRs introduce new required configuration options (FAIR_GROUP_SCHED and CGROUP_PIDS), which can trip existing setups:
kubernetes/system-validators#19
kubernetes/system-validators#20
how common are these options and do you have any concerns about that?
i'm less concerned about CGROUP_PIDS, for some reason.

@odinuge

It has never been supported (or been working) running with a configuration that fails with the new validation, so I think it should be good to go. The only places they might be disabled, from my experience, are raspberry-pi like devices used for prototyping.

it feels like this validation should be enabled regardless; kernels that lack these options can skip validation.

can you clarify what the implications of "both these PRs introduce new required configuration options" are? does this mean an existing configuration will no longer work?

@odinuge confirmed that kernels without these options (now required by the library) never worked with the kubelet.

neolit123 · 2020-11-09T16:16:34Z

waiting to see if our CI passes with the new validation.

neolit123 · 2020-11-09T18:42:12Z

https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/96378/pull-kubernetes-bazel-test/1325833508236562432

go_test: //pkg/kubelet/volumemanager/reconciler/go_default_test:run_1_of_2

https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/96378/pull-kubernetes-integration/1325833508295282688

k8s.io/kubernetes/test/integration/dryrun: TestDryRun

flakes
/retest

soltysh

/lgtm
/approve

soltysh · 2020-11-09T21:04:33Z

/triage accepted

odinuge

Nice!
/lgtm

dims · 2020-11-14T01:33:55Z

@neolit123 looks like this needs a rebase. please assign to liggitt after that.

dims · 2020-11-14T01:34:23Z

/area code-organization

neolit123 · 2020-11-14T01:48:11Z

@neolit123 looks like this needs a rebase. please assign to liggitt after that.

rebased.
/assign @liggitt

Includes the following changes to kernel validation: - add required options: CGROUP_PIDS, FAIR_GROUP_SCHED - add optional options: CFS_BANDWIDTH, CGROUP_HUGETLB

odinuge · 2020-11-16T11:37:17Z

/retest

liggitt · 2020-11-17T16:35:22Z

vendor/k8s.io/system-validators/validators/types_unix.go

+			{Name: "CFS_BANDWIDTH", Description: "Required for CPU quota."},
+			{Name: "CGROUP_HUGETLB", Description: "Required for hugetlb cgroup."},


what does "Required for ..." mean in an Optional list?

can be interpreted as required kernel options for optional k8s features. no preference on the wording in the description on my side.
cc @KentaTada

liggitt · 2020-11-17T16:44:54Z

/lgtm
/approve
dependencies look good

/hold is this fixing a blocker/critical bug for 1.20 (it's flagged as a feature)? if not, let's move to the 1.21 milestone

k8s-ci-robot · 2020-11-17T16:45:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, neolit123, odinuge, soltysh

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:

~~OWNERS~~ [liggitt]

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

neolit123 · 2020-11-17T16:50:19Z

/hold is this fixing a blocker/critical bug for 1.20 (it's flagged as a feature)? if not, let's move to the 1.21 milestone

a validating feature that didn't merge before code-freeze.
punting to 1.21 seems fine.

/milestone v1.21

neolit123 · 2020-12-29T18:46:40Z

/hold cancel

k8s-ci-robot requested review from brendandburns, liggitt and a team November 9, 2020 16:12

k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. area/dependency Issues or PRs related to dependency changes sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. labels Nov 9, 2020

k8s-ci-robot added this to the v1.20 milestone Nov 9, 2020

neolit123 commented Nov 9, 2020

View reviewed changes

neolit123 mentioned this pull request Nov 9, 2020

Check for /sys/fs/cgroup/cpu/cpu.cfs_quota_us / config_cfs_bandwith on init kubernetes/kubeadm#2335

Closed

neolit123 changed the title ~~vendor: update system-validators to 1.3.0~~ vendor: update system-validators to v1.3.0 Nov 9, 2020

soltysh approved these changes Nov 9, 2020

View reviewed changes

k8s-ci-robot assigned soltysh Nov 9, 2020

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Nov 9, 2020

k8s-ci-robot removed the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Nov 9, 2020

odinuge approved these changes Nov 9, 2020

View reviewed changes

k8s-ci-robot assigned odinuge Nov 9, 2020

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 14, 2020

k8s-ci-robot added the area/code-organization Issues or PRs related to kubernetes code organization label Nov 14, 2020

neolit123 force-pushed the 1.20-update-system-validators branch from e75b7a0 to bc66f9a Compare November 14, 2020 01:47

k8s-ci-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 Nov 14, 2020

k8s-ci-robot assigned liggitt Nov 14, 2020

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 14, 2020

vendor: update system-validators to 1.3.0

77df449

Includes the following changes to kernel validation: - add required options: CGROUP_PIDS, FAIR_GROUP_SCHED - add optional options: CFS_BANDWIDTH, CGROUP_HUGETLB

neolit123 force-pushed the 1.20-update-system-validators branch from bc66f9a to 77df449 Compare November 15, 2020 16:07

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 15, 2020

liggitt reviewed Nov 17, 2020

View reviewed changes

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Nov 17, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 17, 2020

k8s-ci-robot modified the milestones: v1.20, v1.21 Nov 17, 2020

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 29, 2020

k8s-ci-robot merged commit d664958 into kubernetes:master Dec 29, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

vendor: update system-validators to v1.3.0 #96378

vendor: update system-validators to v1.3.0 #96378

neolit123 commented Nov 9, 2020

neolit123 commented Nov 9, 2020

neolit123 Nov 9, 2020 •

edited

liggitt Nov 17, 2020

neolit123 Nov 17, 2020

neolit123 commented Nov 9, 2020

neolit123 commented Nov 9, 2020

soltysh left a comment

soltysh commented Nov 9, 2020

odinuge left a comment

dims commented Nov 14, 2020

dims commented Nov 14, 2020

neolit123 commented Nov 14, 2020

odinuge commented Nov 16, 2020

liggitt Nov 17, 2020

neolit123 Nov 17, 2020

liggitt commented Nov 17, 2020

k8s-ci-robot commented Nov 17, 2020

neolit123 commented Nov 17, 2020

neolit123 commented Dec 29, 2020

		{Name: "CFS_BANDWIDTH", Description: "Required for CPU quota."},
		{Name: "CGROUP_HUGETLB", Description: "Required for hugetlb cgroup."},

vendor: update system-validators to v1.3.0 #96378

vendor: update system-validators to v1.3.0 #96378

Conversation

neolit123 commented Nov 9, 2020

neolit123 commented Nov 9, 2020

neolit123 Nov 9, 2020 • edited

Choose a reason for hiding this comment

liggitt Nov 17, 2020

Choose a reason for hiding this comment

neolit123 Nov 17, 2020

Choose a reason for hiding this comment

neolit123 commented Nov 9, 2020

neolit123 commented Nov 9, 2020

soltysh left a comment

Choose a reason for hiding this comment

soltysh commented Nov 9, 2020

odinuge left a comment

Choose a reason for hiding this comment

dims commented Nov 14, 2020

dims commented Nov 14, 2020

neolit123 commented Nov 14, 2020

odinuge commented Nov 16, 2020

liggitt Nov 17, 2020

Choose a reason for hiding this comment

neolit123 Nov 17, 2020

Choose a reason for hiding this comment

liggitt commented Nov 17, 2020

k8s-ci-robot commented Nov 17, 2020

neolit123 commented Nov 17, 2020

neolit123 commented Dec 29, 2020

neolit123 Nov 9, 2020 •

edited