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

Add support for two more kubelet flags #80

Merged
merged 1 commit into from
Nov 26, 2018

Conversation

puneetguptanitj
Copy link
Contributor

Added support for

  • cpu-manager-policy and
  • kube-reserved

cpu-manager-policy and kube-reserved
Copy link
Contributor

@ojmhetar ojmhetar left a comment

Choose a reason for hiding this comment

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

Curious, is the customer currently seeing kubelet being cpu starved or is this a preventative measure? Also, why not include a memory key as well?

Copy link
Contributor

@dlipovetsky dlipovetsky left a comment

Choose a reason for hiding this comment

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

Can we commit this to a branch, not master? I ask this selfishly, because this will conflict with my 1.11 work. No objections, though.

@puneetguptanitj
Copy link
Contributor Author

@ojmhetar thanks for taking a look, right now we want to do just enough to allow cpu-manager-policy being set to static. Eventually these fixes will be removed when we move to 1.11 and have dynamic kubelet config support.

@puneetguptanitj
Copy link
Contributor Author

@dlipovetsky thanks for taking a look, had a chat about this with @sarun87 as well. We were thinking of branching off after @ojmhetar fix for making VIP optional. It would be simpler as both these are required by the customer. As part of 1.11 changes would remove all the custom changes for kubelet config

@sarun87
Copy link
Contributor

sarun87 commented Nov 26, 2018

@dlipovetsky thanks for taking a look, had a chat about this with @sarun87 as well. We were thinking of branching off after @ojmhetar fix for making VIP optional. It would be simpler as both these are required by the customer. As part of 1.11 changes would remove all the custom changes for kubelet config

I think we should push any non 1.11 changes to master and then branch out to say k8s-1.10.4 and commit any 1.11 specific change to master. That would mean this change & optional vip changes that are in flight would need to land in master, then branch off and then 1.11 changes to be merged to master. @dlipovetsky what say?

@dlipovetsky
Copy link
Contributor

If I understand correctly, this PR would be undone as part of the 1.11 changes (since in 1.11 nodeadm uses kubeadm to pass flags to kubelet). For that reason, I thought it would be easier to omit it from master. But it's not a big deal--go ahead and merge to master.

@puneetguptanitj puneetguptanitj merged commit 6a69caf into master Nov 26, 2018
@puneetguptanitj puneetguptanitj deleted the private/kubelet-cpu-flags branch November 27, 2018 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants