-
Notifications
You must be signed in to change notification settings - Fork 295
Reserve resources for calico typha #1607
Reserve resources for calico typha #1607
Conversation
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Good idea! In which case they should be configurable via the cluster.yaml so that users can increase them if the defaults prove too small. Can you add them in as options? |
Codecov Report
@@ Coverage Diff @@
## master #1607 +/- ##
==========================================
- Coverage 25.49% 25.41% -0.08%
==========================================
Files 98 98
Lines 5049 5064 +15
==========================================
Hits 1287 1287
- Misses 3619 3634 +15
Partials 143 143
Continue to review full report at Codecov.
|
Any chance you could sign the CLA and address David's comments @zonzamas? |
Hi, Im have it sing already with the other email I used to use to contribute to kube-aws. I will definitely revisit this when I have some free time or when I need it for my current company. |
I signed it |
a593004
to
0ca54a8
Compare
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
Thanks @zonzamas, I'd still like to see this be configurable through |
0ca54a8
to
7b830c0
Compare
Sure, how about this? |
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.
Looks great! Would you be able to make the small change I requested?
/lgtm |
Hi, many thanks - would you be so kind as to cherry-pick this change and PR to our v0.13.x branch and v0.14.x branch? That way your change will go out with those upcoming updates rather than when v0.15.x is released. Thanks for your contribution! 🙏 |
* Reserve resources for calico typha * Parametrize typha computing resources * Defaults typha resources in new cluster fuction
* Reserve resources for calico typha * Parametrize typha computing resources * Defaults typha resources in new cluster fuction
Calico-typha does not have any reserved resource so it works in a best effort mode. Since it is a 'system service' it should have some resources reserved. What do you think?