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

KEP: Promote sysctl annotations to fields #2093

Merged

Conversation

ingvagabund
Copy link
Contributor

@ingvagabund ingvagabund commented Apr 30, 2018

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
complaints. Promoting the annotations to fields (i.e. to beta) is another step in making the
sysctl feature closer towards the stable API.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 30, 2018
@k8s-ci-robot k8s-ci-robot added sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 30, 2018
@ingvagabund ingvagabund force-pushed the promote-sysctl-annotations-kep branch 6 times, most recently from 7df25db to 07982e8 Compare April 30, 2018 14:32
@ingvagabund ingvagabund force-pushed the promote-sysctl-annotations-kep branch 5 times, most recently from 395afcf to a6f2178 Compare April 30, 2018 15:42
@ingvagabund ingvagabund changed the title WIP: Promote sysctl annotations to fields KEP: Promote sysctl annotations to fields Apr 30, 2018
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 30, 2018
@ingvagabund
Copy link
Contributor Author

@sjenning
Copy link
Contributor

Thanks Jan 👍

/sig node

@k8s-ci-robot k8s-ci-robot added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Apr 30, 2018
authors:
- "@ingvagabund"
owning-sig: sig-node
participating-sigs:
Copy link
Contributor

Choose a reason for hiding this comment

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

As it touches PSP, I think that we may mention sig-auth here.

// SecurityContext holds security configuration that will be applied to a container.
// Some fields are present in both SecurityContext and PodSecurityContext. When both
// are set, the values in SecurityContext take precedence.
type SecurityContext struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

SecurityContext is for container-scoped restrictions. If we want to make it pod-scoped only, we need to modify PodSecurityContext instead.

@php-coder
Copy link
Contributor

@ingvagabund Thank you for working on this!

@php-coder
Copy link
Contributor

/sig auth

@k8s-ci-robot k8s-ci-robot added the sig/auth Categorizes an issue or PR as relevant to SIG Auth. label Apr 30, 2018
@derekwaynecarr
Copy link
Member

Please add section for following:

How to enable unsafe sysctls on node should move away from an experimental flag and become kubelet config api option

@derekwaynecarr
Copy link
Member

The original support predates feature gates, but it’s move to first class fields should come with feature gate. Can you add details on feature gate?

@ingvagabund ingvagabund force-pushed the promote-sysctl-annotations-kep branch 3 times, most recently from 94b9345 to b90d346 Compare May 2, 2018 13:10
@ingvagabund
Copy link
Contributor Author

@derekwaynecarr updated, PTAL

Copy link
Member

@derekwaynecarr derekwaynecarr left a comment

Choose a reason for hiding this comment

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

please just make the one update so this doc is a record of changes rather than link to various WIP prs and dev branches.

this is then LGTM from me.


Upstream issue: https://github.com/kubernetes/kubernetes/issues/61669

### Gate the feature
Copy link
Member

Choose a reason for hiding this comment

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

Please just state what the feature gate flag is and the default behavior.

I have no problem with what @sjenning has started, but this document will be read in the future and linking out like that is painful for future readers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@derekwaynecarr
Copy link
Member

/cc @kubernetes/sig-node-proposals

this is the document that describes the promotion of sysctl to fields for beta support.

if folks have any comments, please provide them this week

@k8s-ci-robot k8s-ci-robot added the kind/design Categorizes issue or PR as related to design. label May 7, 2018
@sttts
Copy link
Contributor

sttts commented May 8, 2018

Lgtm.

@ingvagabund ingvagabund force-pushed the promote-sysctl-annotations-kep branch 2 times, most recently from afdcc25 to ce422be Compare May 10, 2018 12:01
@derekwaynecarr
Copy link
Member

Thank you for the updates @ingvagabund

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 14, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: derekwaynecarr

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 14, 2018
@k8s-ci-robot k8s-ci-robot merged commit 5f826e3 into kubernetes:master May 14, 2018
@sttts
Copy link
Contributor

sttts commented May 15, 2018

🎉

@php-coder
Copy link
Contributor

After 2 weeks after merge, it became outdated. Do we care about making it up-to-date?

k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this pull request Jun 6, 2018
…-to-fields

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Promote sysctl annotations to fields

#


**What this PR does / why we need it**:

Promoting experimental sysctl feature from annotations to API fields.

**Special notes for your reviewer**:

Following sysctl KEP: kubernetes/community#2093

**Release note**:

```release-note
The Sysctls experimental feature has been promoted to beta (enabled by default via the `Sysctls` feature flag). PodSecurityPolicy and Pod objects now have fields for specifying and controlling sysctls. Alpha sysctl annotations will be ignored by 1.11+ kubelets. All alpha sysctl annotations in existing deployments must be converted to API fields to be effective.
```

**TODO**:

* [x] - Promote sysctl annotation in Pod spec
* [x] - Promote sysctl annotation in PodSecuritySpec spec
* [x] - Feature gate the sysctl
* [x] - Promote from alpha to beta
* [x] - docs PR - kubernetes/website#8804
k8s-publishing-bot added a commit to kubernetes/api that referenced this pull request Jun 6, 2018
…-to-fields

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Promote sysctl annotations to fields

#

**What this PR does / why we need it**:

Promoting experimental sysctl feature from annotations to API fields.

**Special notes for your reviewer**:

Following sysctl KEP: kubernetes/community#2093

**Release note**:

```release-note
The Sysctls experimental feature has been promoted to beta (enabled by default via the `Sysctls` feature flag). PodSecurityPolicy and Pod objects now have fields for specifying and controlling sysctls. Alpha sysctl annotations will be ignored by 1.11+ kubelets. All alpha sysctl annotations in existing deployments must be converted to API fields to be effective.
```

**TODO**:

* [x] - Promote sysctl annotation in Pod spec
* [x] - Promote sysctl annotation in PodSecuritySpec spec
* [x] - Feature gate the sysctl
* [x] - Promote from alpha to beta
* [x] - docs PR - kubernetes/website#8804

Kubernetes-commit: b6f75ac30e863531ac73cfd02a0edd57983cc5c0
sttts pushed a commit to sttts/api that referenced this pull request Jun 8, 2018
…-to-fields

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Promote sysctl annotations to fields

#

**What this PR does / why we need it**:

Promoting experimental sysctl feature from annotations to API fields.

**Special notes for your reviewer**:

Following sysctl KEP: kubernetes/community#2093

**Release note**:

```release-note
The Sysctls experimental feature has been promoted to beta (enabled by default via the `Sysctls` feature flag). PodSecurityPolicy and Pod objects now have fields for specifying and controlling sysctls. Alpha sysctl annotations will be ignored by 1.11+ kubelets. All alpha sysctl annotations in existing deployments must be converted to API fields to be effective.
```

**TODO**:

* [x] - Promote sysctl annotation in Pod spec
* [x] - Promote sysctl annotation in PodSecuritySpec spec
* [x] - Feature gate the sysctl
* [x] - Promote from alpha to beta
* [x] - docs PR - kubernetes/website#8804

Kubernetes-commit: b6f75ac30e863531ac73cfd02a0edd57983cc5c0
k8s-publishing-bot added a commit to kubernetes/api that referenced this pull request Jun 8, 2018
…-to-fields

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Promote sysctl annotations to fields

#

**What this PR does / why we need it**:

Promoting experimental sysctl feature from annotations to API fields.

**Special notes for your reviewer**:

Following sysctl KEP: kubernetes/community#2093

**Release note**:

```release-note
The Sysctls experimental feature has been promoted to beta (enabled by default via the `Sysctls` feature flag). PodSecurityPolicy and Pod objects now have fields for specifying and controlling sysctls. Alpha sysctl annotations will be ignored by 1.11+ kubelets. All alpha sysctl annotations in existing deployments must be converted to API fields to be effective.
```

**TODO**:

* [x] - Promote sysctl annotation in Pod spec
* [x] - Promote sysctl annotation in PodSecuritySpec spec
* [x] - Feature gate the sysctl
* [x] - Promote from alpha to beta
* [x] - docs PR - kubernetes/website#8804

Kubernetes-commit: b6f75ac30e863531ac73cfd02a0edd57983cc5c0
sysctls:
- name: kernel.shm_rmid_forced
value: 1
- name: net.ipv4.route.min_pmtu
Copy link
Member

Choose a reason for hiding this comment

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

Why are networking-related knobs in the security context? This seems like the wrong structure to me?

calebamiles pushed a commit to calebamiles/community that referenced this pull request Sep 5, 2018
…notations-kep

KEP: Promote sysctl annotations to fields
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/design Categorizes issue or PR as related to design. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/node Categorizes an issue or PR as relevant to SIG Node. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants