Skip to content

Commit

Permalink
sysctls: create feature gate to track promotion
Browse files Browse the repository at this point in the history
  • Loading branch information
sjenning committed Mar 21, 2018
1 parent 707b7fd commit f4f7220
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 36 deletions.
2 changes: 1 addition & 1 deletion cmd/kubelet/app/options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ func (f *KubeletFlags) AddFlags(fs *pflag.FlagSet) {

// EXPERIMENTAL FLAGS
fs.StringVar(&f.ExperimentalMounterPath, "experimental-mounter-path", f.ExperimentalMounterPath, "[Experimental] Path of mounter binary. Leave empty to use the default mount.")
fs.StringSliceVar(&f.AllowedUnsafeSysctls, "experimental-allowed-unsafe-sysctls", f.AllowedUnsafeSysctls, "Comma-separated whitelist of unsafe sysctls or unsafe sysctl patterns (ending in *). Use these at your own risk.")
fs.StringSliceVar(&f.AllowedUnsafeSysctls, "experimental-allowed-unsafe-sysctls", f.AllowedUnsafeSysctls, "Comma-separated whitelist of unsafe sysctls or unsafe sysctl patterns (ending in *). Use these at your own risk. Presently, you must also enable the Sysctls feature gate for this flag to take effect. Sysctls feature gate is enabled by default.")
fs.BoolVar(&f.ExperimentalKernelMemcgNotification, "experimental-kernel-memcg-notification", f.ExperimentalKernelMemcgNotification, "If enabled, the kubelet will integrate with the kernel memcg notification to determine if memory eviction thresholds are crossed rather than polling.")
fs.StringVar(&f.RemoteRuntimeEndpoint, "container-runtime-endpoint", f.RemoteRuntimeEndpoint, "[Experimental] The endpoint of remote runtime service. Currently unix socket is supported on Linux, and tcp is supported on windows. Examples:'unix:///var/run/dockershim.sock', 'tcp://localhost:3735'")
fs.StringVar(&f.RemoteImageEndpoint, "image-service-endpoint", f.RemoteImageEndpoint, "[Experimental] The endpoint of remote image service. If not specified, it will be the same with container-runtime-endpoint by default. Currently unix socket is supported on Linux, and tcp is supported on windows. Examples:'unix:///var/run/dockershim.sock', 'tcp://localhost:3735'")
Expand Down
32 changes: 17 additions & 15 deletions pkg/apis/core/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,21 +133,23 @@ func ValidatePodSpecificAnnotations(annotations map[string]string, spec *core.Po
allErrs = append(allErrs, ValidateSeccompPodAnnotations(annotations, fldPath)...)
allErrs = append(allErrs, ValidateAppArmorPodAnnotations(annotations, spec, fldPath)...)

sysctls, err := helper.SysctlsFromPodAnnotation(annotations[core.SysctlsPodAnnotationKey])
if err != nil {
allErrs = append(allErrs, field.Invalid(fldPath.Key(core.SysctlsPodAnnotationKey), annotations[core.SysctlsPodAnnotationKey], err.Error()))
} else {
allErrs = append(allErrs, validateSysctls(sysctls, fldPath.Key(core.SysctlsPodAnnotationKey))...)
}
unsafeSysctls, err := helper.SysctlsFromPodAnnotation(annotations[core.UnsafeSysctlsPodAnnotationKey])
if err != nil {
allErrs = append(allErrs, field.Invalid(fldPath.Key(core.UnsafeSysctlsPodAnnotationKey), annotations[core.UnsafeSysctlsPodAnnotationKey], err.Error()))
} else {
allErrs = append(allErrs, validateSysctls(unsafeSysctls, fldPath.Key(core.UnsafeSysctlsPodAnnotationKey))...)
}
inBoth := sysctlIntersection(sysctls, unsafeSysctls)
if len(inBoth) > 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Key(core.UnsafeSysctlsPodAnnotationKey), strings.Join(inBoth, ", "), "can not be safe and unsafe"))
if utilfeature.DefaultFeatureGate.Enabled(features.Sysctls) {

This comment has been minimized.

Copy link
@sttts

sttts Mar 21, 2018

so without the feature being enabled you can set invalid sysctl annotations? Do we want that?

This comment has been minimized.

Copy link
@sjenning

sjenning Mar 21, 2018

Author Owner

The way I figure, it would be like someone just setting some random annotation. Enabling the feature gate makes it meaningful to the kubelet.

The alternative is to fail validation and say "sysctl annotation is invalid.. would not have mattered anyway since you haven't enable the feature gate but... yeah".

Also the precedent elsewhere in this file seems to indicate that only we only do validation on pre-GA things if the feature gates are on.

sysctls, err := helper.SysctlsFromPodAnnotation(annotations[core.SysctlsPodAnnotationKey])
if err != nil {
allErrs = append(allErrs, field.Invalid(fldPath.Key(core.SysctlsPodAnnotationKey), annotations[core.SysctlsPodAnnotationKey], err.Error()))
} else {
allErrs = append(allErrs, validateSysctls(sysctls, fldPath.Key(core.SysctlsPodAnnotationKey))...)
}
unsafeSysctls, err := helper.SysctlsFromPodAnnotation(annotations[core.UnsafeSysctlsPodAnnotationKey])
if err != nil {
allErrs = append(allErrs, field.Invalid(fldPath.Key(core.UnsafeSysctlsPodAnnotationKey), annotations[core.UnsafeSysctlsPodAnnotationKey], err.Error()))
} else {
allErrs = append(allErrs, validateSysctls(unsafeSysctls, fldPath.Key(core.UnsafeSysctlsPodAnnotationKey))...)
}
inBoth := sysctlIntersection(sysctls, unsafeSysctls)
if len(inBoth) > 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Key(core.UnsafeSysctlsPodAnnotationKey), strings.Join(inBoth, ", "), "can not be safe and unsafe"))
}
}

return allErrs
Expand Down
7 changes: 7 additions & 0 deletions pkg/features/kube_features.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,12 @@ const (
// Enable pods to consume pre-allocated huge pages of varying page sizes
HugePages utilfeature.Feature = "HugePages"

// owner: @sjenning
// alpha: v1.4
//
// Enable pods set sysctls on pod
Sysctls utilfeature.Feature = "Sysctls"

// owner @brendandburns
// alpha: v1.9
//
Expand Down Expand Up @@ -296,6 +302,7 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS
PersistentLocalVolumes: {Default: true, PreRelease: utilfeature.Beta},
LocalStorageCapacityIsolation: {Default: true, PreRelease: utilfeature.Beta},
HugePages: {Default: true, PreRelease: utilfeature.Beta},
Sysctls: {Default: true, PreRelease: utilfeature.Alpha},
DebugContainers: {Default: false, PreRelease: utilfeature.Alpha},
PodShareProcessNamespace: {Default: false, PreRelease: utilfeature.Alpha},
PodPriority: {Default: false, PreRelease: utilfeature.Alpha},
Expand Down
38 changes: 20 additions & 18 deletions pkg/kubelet/kubelet.go
Original file line number Diff line number Diff line change
Expand Up @@ -862,25 +862,27 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
klet.evictionManager = evictionManager
klet.admitHandlers.AddPodAdmitHandler(evictionAdmitHandler)

// add sysctl admission
runtimeSupport, err := sysctl.NewRuntimeAdmitHandler(klet.containerRuntime)
if err != nil {
return nil, err
}
safeWhitelist, err := sysctl.NewWhitelist(sysctl.SafeSysctlWhitelist(), v1.SysctlsPodAnnotationKey)
if err != nil {
return nil, err
}
// Safe, whitelisted sysctls can always be used as unsafe sysctls in the spec
// Hence, we concatenate those two lists.
safeAndUnsafeSysctls := append(sysctl.SafeSysctlWhitelist(), allowedUnsafeSysctls...)
unsafeWhitelist, err := sysctl.NewWhitelist(safeAndUnsafeSysctls, v1.UnsafeSysctlsPodAnnotationKey)
if err != nil {
return nil, err
if utilfeature.DefaultFeatureGate.Enabled(features.Sysctls) {
// add sysctl admission
runtimeSupport, err := sysctl.NewRuntimeAdmitHandler(klet.containerRuntime)
if err != nil {
return nil, err
}
safeWhitelist, err := sysctl.NewWhitelist(sysctl.SafeSysctlWhitelist(), v1.SysctlsPodAnnotationKey)
if err != nil {
return nil, err
}
// Safe, whitelisted sysctls can always be used as unsafe sysctls in the spec
// Hence, we concatenate those two lists.
safeAndUnsafeSysctls := append(sysctl.SafeSysctlWhitelist(), allowedUnsafeSysctls...)
unsafeWhitelist, err := sysctl.NewWhitelist(safeAndUnsafeSysctls, v1.UnsafeSysctlsPodAnnotationKey)
if err != nil {
return nil, err
}
klet.admitHandlers.AddPodAdmitHandler(runtimeSupport)
klet.admitHandlers.AddPodAdmitHandler(safeWhitelist)
klet.admitHandlers.AddPodAdmitHandler(unsafeWhitelist)
}
klet.admitHandlers.AddPodAdmitHandler(runtimeSupport)
klet.admitHandlers.AddPodAdmitHandler(safeWhitelist)
klet.admitHandlers.AddPodAdmitHandler(unsafeWhitelist)

// enable active deadline handler
activeDeadlineHandler, err := newActiveDeadlineHandler(klet.statusManager, kubeDeps.Recorder, klet.clock)
Expand Down
6 changes: 4 additions & 2 deletions pkg/kubelet/kuberuntime/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,12 +193,14 @@ func toKubeRuntimeStatus(status *runtimeapi.RuntimeStatus) *kubecontainer.Runtim

// getSysctlsFromAnnotations gets sysctls and unsafeSysctls from annotations.
func getSysctlsFromAnnotations(annotations map[string]string) (map[string]string, error) {
sysctls := make(map[string]string)
if !utilfeature.DefaultFeatureGate.Enabled(features.Sysctls) {
return sysctls, nil
}
apiSysctls, apiUnsafeSysctls, err := v1helper.SysctlsFromPodAnnotations(annotations)
if err != nil {
return nil, err
}

sysctls := make(map[string]string)
for _, c := range apiSysctls {
sysctls[c.Name] = c.Value
}
Expand Down

0 comments on commit f4f7220

Please sign in to comment.