-
Notifications
You must be signed in to change notification settings - Fork 40.9k
[WIP] Validate OS not windows for PodLevelResources #132627
8000 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
base: master
Are you sure you want to change the base?
[WIP] Validate OS not windows for PodLevelResources #132627
Conversation
Reject Pod with PodLevelResources in spec if Pod targets Windows OS.
/sig node |
/assign @ndixita |
if spec.OS != nil && spec.OS.Name == core.Windows { | ||
// Do not include more detailed errors on the resources field value | ||
// if the resources field cannot be set on the target OS. | ||
return field.ErrorList{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LMK if I should append the error instead.
/triage accepted |
/ok-to-test |
Reject pods with PodLevelResources in kubelet on nodes running Windows OS.
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: annasong20 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@@ -1199,6 +1199,11 @@ func (m *kubeGenericRuntimeManager) SyncPod(ctx context.Context, pod *v1.Pod, po | |||
podIPs = podStatus.IPs 8000 | |||
} | |||
|
|||
if isAllowed, msg := allocation.IsPodLevelResourcesAllowed(pod); pod.Spec.Resources != nil && !isAllowed { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ndixita LMK if I should move this check somewhere else in the SyncPod
method or add logs. I could alternatively define a SyncAction
for PodLevelResources
.
Once I clarify the behavior, I'll also need to figure out if I can find a _windows_test.go
and _linux_test.go
that calls SyncPod
to test this behavior.
Reject Pod with PodLevelResources in spec if Pod targets Windows OS.
What type of PR is this?
/kind bug
What this PR does / why we need it:
Adds check to
Which issue(s) this PR is related to:
Fixes #132582.
PodLevelResources KEP that documents this behavior: https://github.com/kubernetes/enhancements/blob/master/keps/sig-node/2837-pod-level-resource-spec/README.md#future-kep-consideration-in-135-support-for-windows.
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: