-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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
fix function NodeAllocatableRoot #88970
fix function NodeAllocatableRoot #88970
Conversation
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.
Thanks for your pr :)
Could you please include, either in your commit message or pr description (or ideally both), a description of the fix that you're making? Having the description makes it a lot easier for a reviewer the pr makes the intended change :)
096379a
to
41aa4d8
Compare
ping @dashpole |
/test pull-kubernetes-e2e-gce |
func NodeAllocatableRoot(cgroupRoot, cgroupDriver string) string { | ||
root := ParseCgroupfsToCgroupName(cgroupRoot) | ||
nodeAllocatableRoot := NewCgroupName(root, defaultNodeAllocatableCgroupName) | ||
func NodeAllocatableRoot(cgroupRoot string, cgroupsPerQOS bool, cgroupDriver string) string { |
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.
you will have to change this function in other builds as well.
Good find. This lgtm after the cross-builds are fixed. |
I can not understand "cross-builds are fixed.", would you like to explain it for me ? |
41aa4d8
to
6783f99
Compare
/test pull-kubernetes-e2e-gce-100-performance |
ping @dashpole |
ping @mattjmcnaughton |
/test pull-kubernetes-typecheck |
ping @mattjmcnaughton ,please add to lgtm and approved,thanks |
The type-check test is essentially a lightweight cross-build. I.E. make sure this builds for "other" systems. /lgtm |
You need sign off from one of the owners, which I'm not :) |
/priority important-soon |
thanks for your explanation, i have modified it. |
ping @yujuhong ,please add approved label,thanks |
ping @yujuhong |
ping @yujuhong @Random-Liu @vishh ,please add approved label,thanks |
ping @yujuhong @Random-Liu @vishh ,please add approved label,thanks |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dashpole, mysunshine92, yujuhong 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 |
What type of PR is this?
/kind bug
/kind cleanup
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #88969
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: