Identify Pod's OS during API Server admission #2803

ravisantoshgudimetla · 2021-06-28T17:27:28Z

#2802

One-line PR description:

Issue link:

Other comments:

aravindhp

Thanks for opening this enhancement, @ravisantoshgudimetla

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/kep.yaml

marosset · 2021-07-14T20:57:47Z

So far this looks good.

I'd like to see more details about which all specific plugins will be aware of this feature flag and what specific behavior will change if pods get identified as targeting Windows.

ravisantoshgudimetla

Thanks for the reviews @aravindhp @marosset. I tried answering questions in comments. Let me know if you are fine with the approach. I'll proceed to update the KEP along with sample implementation.

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md

ravisantoshgudimetla

Thanks for the review @tallclair.

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md

ravisantoshgudimetla

Updated to include feedback

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md

liggitt · 2021-07-27T18:41:54Z

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md

+## Design Details
+
+
+We can piggyback on the existing RuntimeClass admission controller to query for the RuntimeClass and see if it has


it's unclear what this is proposing the existing RuntimeClass admission plugin do... forbid pods which set a kubernetes.io/os: windows spec.nodeSelector and don't set a runtimeClassName?

I can update this section to make it clear.

ravisantoshgudimetla

Thank you for the review @liggitt. I tried answering your questions. PTAL and let me know your thoughts.

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md

ravisantoshgudimetla · 2021-08-02T13:02:16Z

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md

+## Design Details
+
+
+We can piggyback on the existing RuntimeClass admission controller to query for the RuntimeClass and see if it has


I can update this section to make it clear.

liggitt · 2021-09-08T16:59:53Z

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md

+and need not necessarily express the user intention fully.
+- The pod may target Windows node but pod OS may be linux or windows
+as Linux Containers on Windows(LCOW) can be supported in future.
+


the actual proposal starts here

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md

derekwaynecarr

aside from the updates requested by @liggitt and my request that we ensure the pod validation for windows correlates to an e2e test that demonstrates the function on windows, this lgtm from a sig-node perspective.

derekwaynecarr · 2021-09-08T17:56:29Z

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md

+// We're making this a struct for possible future expansion.
+type OS struct {
+  // Name of the OS. The values supported are available at:
+  // https://github.com/opencontainers/runtime-spec/blob/master/config.md#platform-specific-configuration


the node platforms are distinct from supported operating system value. i had linked @ravisantoshgudimetla to the OCI spec for reference as a potential future set, but we should only support linux or windows at this time which maps to present support.

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md

deads2k · 2021-09-08T18:36:53Z

the PRR lgtm

/approve

liggitt · 2021-09-08T21:00:10Z

API and PodSecurity bits lgtm

marosset · 2021-09-08T21:12:33Z

/lgtm
/approve
/hold
Adding a hold to make sure all of the feedback from @derekwaynecarr is addressed (I still see his review status as changes requested but he gave an LGTM for node / kubelet changes)

marosset · 2021-09-08T21:13:05Z

/label tide/merge-method-squash

derekwaynecarr · 2021-09-08T21:37:27Z

/approve for sig-node

k8s-ci-robot · 2021-09-08T21:37:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, derekwaynecarr, marosset, ravisantoshgudimetla

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:

~~keps/prod-readiness/OWNERS~~ [deads2k]
~~keps/sig-windows/OWNERS~~ [derekwaynecarr,marosset]

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 · 2021-09-08T21:38:01Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, derekwaynecarr, marosset, ravisantoshgudimetla

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:

~~keps/prod-readiness/OWNERS~~ [deads2k]
~~keps/sig-windows/OWNERS~~ [derekwaynecarr,marosset]

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

derekwaynecarr · 2021-09-08T21:38:04Z

I think all requests were satisfied.

/hold cancel

* Identify Windows Pods during API Server admission * Address reviewers comments * Apply suggestions from code review Co-authored-by: jay vyas <jayunit100.github@gmail.com> * Apply suggestions from code review Co-authored-by: jay vyas <jayunit100.github@gmail.com> Co-authored-by: Wei Huang <weih@hey.com> * Address reviewers comments * Address reviewers comments * Apply suggestions from code review Co-authored-by: jay vyas <jayunit100.github@gmail.com> * Address reviewers comments * Address reviewers comments * Address reviewers comments Co-authored-by: jay vyas <jayunit100.github@gmail.com> Co-authored-by: Wei Huang <weih@hey.com>

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 28, 2021

k8s-ci-robot requested review from ddebroy and mikedanese June 28, 2021 17:27

ravisantoshgudimetla force-pushed the identify-windows-pods branch from 40455d0 to 3a90d78 Compare June 28, 2021 17:28

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 28, 2021

ravisantoshgudimetla changed the title ~~Identify Windows Pods during API Server admission~~ [WIP] Identify Windows Pods during API Server admission Jun 28, 2021

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 28, 2021

aravindhp reviewed Jun 28, 2021

View reviewed changes

marosset reviewed Jul 14, 2021

View reviewed changes

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md Outdated Show resolved Hide resolved

marosset reviewed Jul 14, 2021

View reviewed changes

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md Outdated Show resolved Hide resolved

marosset reviewed Jul 14, 2021

View reviewed changes

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md Outdated Show resolved Hide resolved

marosset reviewed Jul 14, 2021

View reviewed changes

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md Outdated Show resolved Hide resolved

marosset reviewed Jul 14, 2021

View reviewed changes

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md Show resolved Hide resolved

marosset reviewed Jul 14, 2021

View reviewed changes

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/kep.yaml Show resolved Hide resolved

ravisantoshgudimetla commented Jul 15, 2021

View reviewed changes

tallclair reviewed Jul 19, 2021

View reviewed changes

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md Outdated Show resolved Hide resolved

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md Outdated Show resolved Hide resolved

ravisantoshgudimetla commented Jul 19, 2021

View reviewed changes

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md Outdated Show resolved Hide resolved

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md Outdated Show resolved Hide resolved

marosset mentioned this pull request Jul 22, 2021

Recommend runtimeclasses for scheduling kubernetes/website#29062

Closed

ravisantoshgudimetla commented Jul 23, 2021

View reviewed changes

liggitt mentioned this pull request Jul 23, 2021

Windows HostProcess Container Documentation kubernetes/website#28413

Merged

liggitt reviewed Jul 27, 2021

View reviewed changes

ravisantoshgudimetla commented Aug 2, 2021

View reviewed changes

liggitt reviewed Sep 8, 2021

View reviewed changes

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md Outdated Show resolved Hide resolved

liggitt reviewed Sep 8, 2021

View reviewed changes

keps/sig-windows/2802-identify-windows-pods-apiserver-admission/README.md Show resolved Hide resolved

derekwaynecarr requested changes Sep 8, 2021

View reviewed changes

Address reviewers comments

03f3dd8

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 8, 2021

k8s-ci-robot assigned marosset Sep 8, 2021

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 8, 2021

k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Sep 8, 2021

derekwaynecarr approved these changes Sep 8, 2021

View reviewed changes

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

k8s-ci-robot merged commit d6a724c into kubernetes:master Sep 8, 2021

k8s-ci-robot added this to the v1.23 milestone Sep 8, 2021

ravisantoshgudimetla mentioned this pull request Sep 9, 2021

Updated release checklist #2972

Merged

marosset mentioned this pull request Sep 10, 2021

Identify Windows pods at API admission level authoritatively #2802

Closed

26 tasks

ravisantoshgudimetla mentioned this pull request Sep 29, 2021

Security Standards for both Windows and Linux kubernetes/website#29855

Closed

palnabarun mentioned this pull request Jun 12, 2022

canonical kep number json field palnabarun/enhancements#1

Closed

palnabarun mentioned this pull request Jul 26, 2022

KEP Manifest Generator palnabarun/enhancements#2

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Identify Pod's OS during API Server admission #2803

Identify Pod's OS during API Server admission #2803

ravisantoshgudimetla commented Jun 28, 2021

aravindhp left a comment

marosset commented Jul 14, 2021

ravisantoshgudimetla left a comment

ravisantoshgudimetla left a comment

ravisantoshgudimetla left a comment

liggitt Jul 27, 2021

ravisantoshgudimetla Aug 2, 2021

ravisantoshgudimetla left a comment

ravisantoshgudimetla Aug 2, 2021

liggitt Sep 8, 2021

derekwaynecarr left a comment

derekwaynecarr Sep 8, 2021

deads2k commented Sep 8, 2021

liggitt commented Sep 8, 2021

marosset commented Sep 8, 2021

marosset commented Sep 8, 2021

derekwaynecarr commented Sep 8, 2021

k8s-ci-robot commented Sep 8, 2021

k8s-ci-robot commented Sep 8, 2021

derekwaynecarr commented Sep 8, 2021

		## Design Details


		We can piggyback on the existing RuntimeClass admission controller to query for the RuntimeClass and see if it has

Identify Pod's OS during API Server admission #2803

Identify Pod's OS during API Server admission #2803

Conversation

ravisantoshgudimetla commented Jun 28, 2021

aravindhp left a comment

Choose a reason for hiding this comment

marosset commented Jul 14, 2021

ravisantoshgudimetla left a comment

Choose a reason for hiding this comment

ravisantoshgudimetla left a comment

Choose a reason for hiding this comment

ravisantoshgudimetla left a comment

Choose a reason for hiding this comment

liggitt Jul 27, 2021

Choose a reason for hiding this comment

ravisantoshgudimetla Aug 2, 2021

Choose a reason for hiding this comment

ravisantoshgudimetla left a comment

Choose a reason for hiding this comment

ravisantoshgudimetla Aug 2, 2021

Choose a reason for hiding this comment

liggitt Sep 8, 2021

Choose a reason for hiding this comment

derekwaynecarr left a comment

Choose a reason for hiding this comment

derekwaynecarr Sep 8, 2021

Choose a reason for hiding this comment

deads2k commented Sep 8, 2021

liggitt commented Sep 8, 2021

marosset commented Sep 8, 2021

marosset commented Sep 8, 2021

derekwaynecarr commented Sep 8, 2021

k8s-ci-robot commented Sep 8, 2021

k8s-ci-robot commented Sep 8, 2021

derekwaynecarr commented Sep 8, 2021