-
Notifications
You must be signed in to change notification settings - Fork 19
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
Add cpu_pinning and isolate_emulator_thread option #63
base: master
Are you sure you want to change the base?
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.
LGTM. Thanks.
@FrankYang0529 do we need to ensure there is no such an upgrade path that upgrading from a Rancher with the fields to a Rancher without the new fields? |
Yes, we need. If we only add it to latest rancher v2.9.x version, I think there is no way to downgrade to old rancher version. |
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.
the changes look good, may be we just need to fine tune the usage message on the two flags.
harvester/flags.go
Outdated
mcnflag.BoolFlag{ | ||
EnvVar: "HARVESTER_CPU_PINNING", | ||
Name: "harvester-cpu-pinning", | ||
Usage: "enable vm cpu pinning, make sure the the harvester cluster enable cpu manager in at least one node", |
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.
Usage: "enable vm cpu pinning, make sure the the harvester cluster enable cpu manager in at least one node", | |
Usage: "enable vm cpu pinning, please ensure the harvester cluster has cpu manager enabled in at least one node", |
harvester/flags.go
Outdated
mcnflag.BoolFlag{ | ||
EnvVar: "HARVESTER_ISOLATE_EMULATOR_THREAD", | ||
Name: "harvester-isolate-emulator-thread", | ||
Usage: "enable vm isolatate emulator thread, note that enabling this feature will acquire one more cpu", |
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.
Usage: "enable vm isolatate emulator thread, note that enabling this feature will acquire one more cpu", | |
Usage: "enable vm isolatated emulator thread", |
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 the comments! I've addressed everything in the latest commit.
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.
lgtm. thanks.
related to harvester/harvester#6551, as title, add cpu_pinning and isolate_emulator_thread option.
Test Plan
Prerequisite
> 8 CPUs
and> 12GB mem
Test steps
cluster-registration-url
)/opt/drivers/management-state/bin
in rancher container<YOUR_...
in the yaml examples.)