Added ability to configure sidecar CPU + Memory requests#1731
Added ability to configure sidecar CPU + Memory requests#1731klingerf merged 1 commit intolinkerd:masterfrom benjdlambert:master
Conversation
|
@benjdlambert did this fix all the HPA problems or were there other issues? ps. this is awesome! |
|
@grampelberg - so it turns out, that our issues were to do with how we were doing health checks, and that our health check caused CPU spikes. So not related to any of the l5d stuff at all.. hah. We might want to be able to configure these limits based on some CLI input, or off of prometheus metrics in the future, as the proxy kind-of scales with the service that it's deployed along side. But I think this is a good start. And I don't think we'll know the use cases until we can start to use it :) |
|
Yeah! I think it is a great start =) I'm doing a HPA talk with linkerd @ kubecon, so I'm going to need to dig in and figure this all out before then! (also, health checks and readiness checks are hard =/ I wish there was more education about how to do them without shooting your foot off) |
|
@grampelberg 🙌 passed! |
|
@benjdlambert Thanks for adding this! Related to this comment:
My preference is to add two new flags to the linkerd inject command: |
|
Ah ok! Sure no problem. It's make sense! |
benjdlambert
changed the title
|
@klingerf - I think this is what you meant? I want to be able to test the validation using the test data way but not sure if possible. I know it works as I've tested manually, but would be good to add a somewhat unit test somewhere. Help would be appreciated if you think it's necessary 👍 |
Horizontal Pod Autoscaling does not work when container definitions in pods do not all have resource requests, so here's the ability to add CPU + Memory requests to install + inject commands by proving proxy options --proxy-cpu + --proxy-memory Fixes #1480 Signed-off-by: Ben Lambert <[email protected]>
|
Also @klingerf - I'm not sure if these cli |
klingerf
left a comment
klingerf
left a comment
There was a problem hiding this comment.
@benjdlambert Awesome! This is exactly what I was talking about. It makes sense to put the options in root.go like you did, since they're equally applicable when installing the linkerd control plane, which is itself injected. And the inject test that you added is great. I agree that it would be nice to have a unit test for the validate() func in root.go, but it's ok as is since none of the other validations in that file are tested. Thanks for addressing my feedback! This is a great change.
3 participants
Horizontal Pod Autoscaling does not work when container definitions in pods do not all have resource requests, so here's the ability to add CPU + Memory requests to install + inject commands by proving proxy options
--proxy-cpu+--proxy-memoryFixes #1480
Signed-off-by: Ben Lambert [email protected]