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

Migrate KEP-34 to new template #2471

Merged
merged 2 commits into from
Feb 9, 2021
Merged
Show file tree
Hide file tree
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
3 changes: 3 additions & 0 deletions keps/prod-readiness/sig-node/34.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kep-number: 34
stable:
approver: "@johnbelamaric"
ehashman marked this conversation as resolved.
Show resolved Hide resolved
314 changes: 274 additions & 40 deletions keps/sig-node/34-sysctl-fields/README.md
Original file line number Diff line number Diff line change
@@ -1,23 +1,115 @@
# Promote sysctl annotations to fields

## Table of Contents
# KEP-34: Promote sysctl annotations to fields

<!-- toc -->
- [Release Signoff Checklist](#release-signoff-checklist)
- [Summary](#summary)
- [Motivation](#motivation)
- [Promote annotations to fields](#promote-annotations-to-fields)
- [Goals/Non-Goals](#goalsnon-goals)
- [Proposal](#proposal)
- [Promote annotations to fields (beta)](#promote-annotations-to-fields-beta)
- [Promote <code>--experimental-allowed-unsafe-sysctls</code> kubelet flag to kubelet config api option](#promote--kubelet-flag-to-kubelet-config-api-option)
- [Gate the feature](#gate-the-feature)
- [Proposal](#proposal)
- [User Stories](#user-stories)
- [Implementation Details/Notes/Constraints](#implementation-detailsnotesconstraints)
- [User Stories (Optional)](#user-stories-optional)
- [Notes/Constraints/Caveats](#notesconstraintscaveats)
- [Risks and Mitigations](#risks-and-mitigations)
- [Graduation Criteria](#graduation-criteria)
- [Design Details](#design-details)
- [Test Plan](#test-plan)
- [Graduation Criteria](#graduation-criteria)
- [Alpha](#alpha)
- [Beta](#beta)
- [Graduation](#graduation)
- [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy)
- [Version Skew Strategy](#version-skew-strategy)
- [Production Readiness Review Questionnaire](#production-readiness-review-questionnaire)
- [Feature Enablement and Rollback](#feature-enablement-and-rollback)
- [Rollout, Upgrade and Rollback Planning](#rollout-upgrade-and-rollback-planning)
- [Monitoring Requirements](#monitoring-requirements)
- [Dependencies](#dependencies)
- [Scalability](#scalability)
- [Troubleshooting](#troubleshooting)
- [Implementation History](#implementation-history)
- [Drawbacks / Alternatives](#drawbacks--alternatives)
- [Infrastructure Needed (Optional)](#infrastructure-needed-optional)
<!-- /toc -->

## Release Signoff Checklist

<!--
**ACTION REQUIRED:** In order to merge code into a release, there must be an
issue in [kubernetes/enhancements] referencing this KEP and targeting a release
milestone **before the [Enhancement Freeze](https://git.k8s.io/sig-release/releases)
of the targeted release**.

For enhancements that make changes to code or processes/procedures in core
Kubernetes—i.e., [kubernetes/kubernetes], we require the following Release
Signoff checklist to be completed.

Check these off as they are completed for the Release Team to track. These
checklist items _must_ be updated for the enhancement to be released.
-->

Items marked with (R) are required *prior to targeting to a milestone / release*.

- [x] (R) Enhancement issue in release milestone, which links to KEP dir in [kubernetes/enhancements] (not the initial KEP PR)
- [x] (R) KEP approvers have approved the KEP status as `implementable`
- [x] (R) Design details are appropriately documented
- [x] (R) Test plan is in place, giving consideration to SIG Architecture and SIG Testing input
- [x] (R) Graduation criteria is in place
- [x] (R) Production readiness review completed
- [x] (R) Production readiness review approved
- [ ] "Implementation History" section is up-to-date for milestone
- [x] User-facing documentation has been created in [kubernetes/website], for publication to [kubernetes.io]
- [ ] Supporting documentation—e.g., additional design documents, links to mailing list discussions/SIG meetings, relevant PRs/issues, release notes

<!--
**Note:** This checklist is iterative and should be reviewed and updated every time this enhancement is being considered for a milestone.
-->

[kubernetes.io]: https://kubernetes.io/
[kubernetes/enhancements]: https://git.k8s.io/enhancements
[kubernetes/kubernetes]: https://git.k8s.io/kubernetes
[kubernetes/website]: https://git.k8s.io/website

## Summary

This proposal aims at extending the current pod specification with support
for namespaced kernel parameters (sysctls) set for each pod.

See the [abstract and motivation](https://github.com/kubernetes/community/blob/master/contributors/design-proposals/node/sysctl.md#abstract) from the original proposal in v1.4.

## Motivation

See the original design proposal's [motivation section](https://github.com/kubernetes/community/blob/master/contributors/design-proposals/node/sysctl.md#motivation).

As mentioned in [contributors/devel/api_changes.md#alpha-field-in-existing-api-version](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api_changes.md#alpha-field-in-existing-api-version):

> Previously, annotations were used for experimental alpha features, but are no longer recommended for several reasons:
>
> They expose the cluster to "time-bomb" data added as unstructured annotations against an earlier API server (https://issue.k8s.io/30819)
> They cannot be migrated to first-class fields in the same API version (see the issues with representing a single value in multiple places in backward compatibility gotchas)
>
> The preferred approach adds an alpha field to the existing object, and ensures it is disabled by default:
>
> ...

The annotations as a means to set `sysctl` are no longer necessary.
The original intent of annotations was to provide additional description of Kubernetes
objects through metadata.
It's time to separate the ability to annotate from the ability to change sysctls settings
so a cluster operator can elevate the distinction between experimental and supported usage
of the feature.


### Goals/Non-Goals

See: original [constraints and assumptions](https://github.com/kubernetes/community/blob/master/contributors/design-proposals/node/sysctl.md#constraints-and-assumptions)

## Proposal

See the [original design proposal](https://github.com/kubernetes/community/blob/master/contributors/design-proposals/node/sysctl.md#proposed-design) for alpha.

### Promote annotations to fields (beta)

Setting the `sysctl` parameters through annotations provided a successful story
for defining better constraints of running applications.
The `sysctl` feature has been tested by a number of people without any serious
Expand Down Expand Up @@ -61,28 +153,6 @@ in the following way:

The `sysctl` design document with more details and rationals is available at [design-proposals/node/sysctl.md](https://github.com/kubernetes/community/blob/master/contributors/design-proposals/node/sysctl.md#pod-api-changes)

## Motivation

As mentioned in [contributors/devel/api_changes.md#alpha-field-in-existing-api-version](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api_changes.md#alpha-field-in-existing-api-version):

> Previously, annotations were used for experimental alpha features, but are no longer recommended for several reasons:
>
> They expose the cluster to "time-bomb" data added as unstructured annotations against an earlier API server (https://issue.k8s.io/30819)
> They cannot be migrated to first-class fields in the same API version (see the issues with representing a single value in multiple places in backward compatibility gotchas)
>
> The preferred approach adds an alpha field to the existing object, and ensures it is disabled by default:
>
> ...

The annotations as a means to set `sysctl` are no longer necessary.
The original intent of annotations was to provide additional description of Kubernetes
objects through metadata.
It's time to separate the ability to annotate from the ability to change sysctls settings
so a cluster operator can elevate the distinction between experimental and supported usage
of the feature.

### Promote annotations to fields

* Introduce native `sysctl` fields in pods through `spec.securityContext.sysctl` field as:

```yaml
Expand Down Expand Up @@ -139,11 +209,9 @@ If disabled, the fields and the whitelist are just ignored.

[1] https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/

## Proposal

This is where we get down to the nitty gritty of what the proposal actually is.
### User Stories (Optional)

### User Stories
See also: [original sysctl proposal](https://github.com/kubernetes/community/blob/master/contributors/design-proposals/node/sysctl.md#abstract-use-cases)

* As a cluster admin, I want to have `sysctl` feature versioned so I can assure backward compatibility
and proper transformation between versioned to internal representation and back..
Expand All @@ -152,7 +220,7 @@ This is where we get down to the nitty gritty of what the proposal actually is.
* As a cluster admin, I want to be able to apply the `sysctl` constraints on the cluster level so
I can define the default constraints for all pods.

### Implementation Details/Notes/Constraints
### Notes/Constraints/Caveats

Extending `SecurityContext` struct with `Sysctls` field:

Expand Down Expand Up @@ -188,15 +256,181 @@ Validation checks implemented as part of [#27180](https://github.com/kubernetes/
We need to assure backward compatibility, i.e. object specifications with `sysctl` annotations
must still work after the graduation.

## Graduation Criteria
## Design Details

All of the above details were copied out of earlier proposals. For graduation, the PRR template below is completed.

### Test Plan

- Unit tests and e2es for all applicable changes.
- Any required conformance tests for graduation.

### Graduation Criteria

#### Alpha

* add sysctl support to pods
* e2e tests

Alpha since 1.4.

#### Beta

* API changes allowing to configure the pod-scoped `sysctl` via `spec.securityContext` field.
* API changes allowing to configure the cluster-scoped `sysctl` via `PodSecurityPolicy` object
* Promote `--experimental-allowed-unsafe-sysctls` kubelet flag to kubelet config api option
* feature gate enabled by default
* e2e tests

Beta since 1.11.

#### Graduation

* Promote `--experimental-allowed-unsafe-sysctls` kubelet flag to kubelet config api option
Copy link
Member

Choose a reason for hiding this comment

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

this flag does not exist.

it was changed to allowed-unsafe-sysctls and no further action is needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

This was done, but it was done post-beta so I will keep it in GA criteria and add it into implementation history.

For some reason the issue was still open despite this having been fixed so I closed that issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

* lock feature gate on

### Upgrade / Downgrade Strategy

There are [e2es](https://github.com/kubernetes/kubernetes/blob/28e2e12b887fe082929d3ece4b3cbd572dc15d39/test/e2e/upgrades/sysctl.go) for sysctl behaviour on upgrades.

### Version Skew Strategy

N/A

## Production Readiness Review Questionnaire

### Feature Enablement and Rollback

###### How can this feature be enabled / disabled in a live cluster?

- [X] Feature gate (also fill in values in `kep.yaml`)
- Feature gate name: Sysctls
- Components depending on the feature gate: kubelet, apiserver
- [ ] Other
- Describe the mechanism:
- Will enabling / disabling the feature require downtime of the control
plane?
- Will enabling / disabling the feature require downtime or reprovisioning
of a node? (Do not assume `Dynamic Kubelet Config` feature is enabled).

###### Does enabling the feature change any default behavior?

No. Enabling the feature allows the use of sysctls.

###### Can the feature be disabled once it has been enabled (i.e. can we roll back the enablement)?

Yes, disable the feature flag.

###### What happens if we reenable the feature if it was previously rolled back?

Feature will become available again on the component.

###### Are there any tests for feature enablement/disablement?

Not currently. Feature has defaulted to on since 1.11; graduation criteria would lock feature to on.

### Rollout, Upgrade and Rollback Planning

###### How can a rollout fail? Can it impact already running workloads?

N/A

###### What specific metrics should inform a rollback?

N/A

###### Were upgrade and rollback tested? Was the upgrade->downgrade->upgrade path tested?

Yes: https://github.com/kubernetes/kubernetes/blob/28e2e12b887fe082929d3ece4b3cbd572dc15d39/test/e2e/upgrades/sysctl.go

###### Is the rollout accompanied by any deprecations and/or removals of features, APIs, fields of API types, flags, etc.?

N/A

### Monitoring Requirements

###### How can an operator determine if the feature is in use by workloads?

No metric currently exists. Feature flag will be set to on and Pod or PSP specifications will include sysctl fields set.

###### What are the SLIs (Service Level Indicators) an operator can use to determine the health of the service?

N/A, not a service.

###### What are the reasonable SLOs (Service Level Objectives) for the above SLIs?

N/A, not a service.

###### Are there any missing metrics that would be useful to have to improve observability of this feature?

N/A

### Dependencies

Underlying kernel support for sysctls.

###### Does this feature depend on any specific services running in the cluster?

No.

### Scalability

###### Will enabling / using this feature result in any new API calls?

No.

###### Will enabling / using this feature result in introducing new API types?

No.

###### Will enabling / using this feature result in any new calls to the cloud provider?

No.

###### Will enabling / using this feature result in increasing size or count of the existing API objects?

Yes: pods and PSPs have new fields for sysctl values.

###### Will enabling / using this feature result in increasing time taken by any operations covered by existing SLIs/SLOs?

No.

###### Will enabling / using this feature result in non-negligible increase of resource usage (CPU, RAM, disk, IO, ...) in any components?

No.

### Troubleshooting

###### How does this feature react if the API server and/or etcd is unavailable?

Feature is an API field on pod specification; kubelets behave as usual when API server/etcd are unavailable.

###### What are other known failure modes?

- https://github.com/kubernetes/kubernetes/issues/72593
- https://github.com/kubernetes/kubernetes/issues/74151

There may be some follow-ups required to improve usability, but I do not
believe this should block graduation.

Any scheduling enhancement we make around a node that is configured to allow
unsafe sysctls would be a distinct feature.

###### What steps should be taken if SLOs are not being met to determine the problem?

SLOs do not apply, N/A.

## Implementation History

The `sysctl` feature is tracked as part of [features#34](https://github.com/kubernetes/features/issues/34).
This is one of the goals to promote the annotations to fields.
- 2017-06-12: [Original design proposal](https://github.com/kubernetes/community/pull/700)
- 2018-05-14: [Update KEP with beta criteria](https://github.com/kubernetes/community/pull/2093)
- 2018-06-06: [Promote sysctl annotations to fields](https://github.com/kubernetes/kubernetes/pull/63717)
- 2018-06-14: [Update sysctls to beta on website](https://github.com/kubernetes/website/pull/8804)
- 2019-07-02: [Add allowed sysctl to KubeletConfiguration](https://github.com/kubernetes/kubernetes/pull/72974)
- 2021-02-08: Update KEP with final graduation criteria/complete PRR questionnaire

## Drawbacks / Alternatives

See also: [original design alternatives and considerations](https://github.com/kubernetes/community/blob/master/contributors/design-proposals/node/sysctl.md#design-alternatives-and-considerations)

## Infrastructure Needed (Optional)

N/A
Loading