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

disable localStorageCapacityIsolation for rootless in >= v1.25.0-alpha.3.440+0064010cddfa00 #2846

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 17 additions & 8 deletions pkg/cluster/internal/kubeadm/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,16 @@ type ConfigData struct {
// Labels are the labels, in the format "key1=val1,key2=val2", with which the respective node will be labeled
NodeLabels string

// DerivedConfigData is populated by Derive()
// These auto-generated fields are available to Config templates,
// but not meant to be set by hand
DerivedConfigData

// Provider is running with rootless mode, so kube-proxy needs to be configured
// not to fail on sysctl error.
// RootlessProvider is true if kind is running with rootless mode
RootlessProvider bool

// DisableLocalStorageCapacityIsolation is typically set true based on RootlessProvider
// based on the Kubernetes version, if true kubelet localStorageCapacityIsolation is set false
DisableLocalStorageCapacityIsolation bool

// DerivedConfigData contains fields computed from the other fields for use
// in the config templates and should only be populated by calling Derive()
DerivedConfigData
}

// DerivedConfigData fields are automatically derived by
Expand Down Expand Up @@ -537,6 +539,7 @@ evictionHard:
{{ range $key := .SortedFeatureGateKeys }}
"{{ $key }}": {{ index $.FeatureGates $key }}
{{end}}{{end}}
localStorageCapacityIsolation: {{if .DisableLocalStorageCapacityIsolation}}false{{else}}true{{end}}
Copy link
Member

@AkihiroSuda AkihiroSuda Aug 4, 2022

Choose a reason for hiding this comment

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

Suggested change
localStorageCapacityIsolation: {{if .DisableLocalStorageCapacityIsolation}}false{{else}}true{{end}}
{{if .DisableLocalStorageCapacityIsolation}}localStorageCapacityIsolation: false{{end}}

This may look cleaner and less likely to hit an issue with an older version of kubelet that is unaware of localStorageCapacityIsolation

Copy link
Contributor

Choose a reason for hiding this comment

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

agree

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree on cleaner but the unknown fields will never reach kubelet, kubeadm will drop any unknown field because it defaults kubelet config itself. We’ve run this patch with and without the Kubernetes patch.

{{if ne .KubeProxyMode "None"}}
---
apiVersion: kubeproxy.config.k8s.io/v1alpha1
Expand Down Expand Up @@ -585,7 +588,13 @@ func Config(data ConfigData) (config string, err error) {

// For avoiding err="failed to get rootfs info: failed to get device for dir \"/var/lib/kubelet\": could not find device with major: 0, minor: 41 in cached partitions map"
// https://github.com/kubernetes-sigs/kind/issues/2524
data.FeatureGates["LocalStorageCapacityIsolation"] = false
if ver.LessThan(version.MustParseSemantic("v1.25.0-alpha.3.440+0064010cddfa00")) {
// this feature gate was removed in v1.25 and replaced by an opt-out to disable
data.FeatureGates["LocalStorageCapacityIsolation"] = false
} else {
// added in v1.25 https://github.com/kubernetes/kubernetes/pull/111513
data.DisableLocalStorageCapacityIsolation = true
}
}

// assume the latest API version, then fallback if the k8s version is too low
Expand Down