-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
promote ServiceIPStaticSubrange to beta #3413
Conversation
aojea
commented
Jun 20, 2022
- One-line PR description: KEP-3070: promote to beta
- Issue link: Reserve Service IP Ranges For Dynamic and Static IP Allocation #3070
- Other comments:
/assign @thockin @johnbelamaric |
…the merged KEP's Test Plan section.
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!
/lgtm
/approve
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.
One little question but otherwise lgtm.
###### Will enabling / using this feature result in increasing time taken by any operations covered by existing SLIs/SLOs? | ||
|
||
No |
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.
Theoretically, it would be possible that IP allocations take slightly longer, of in the event of some sort of bug, a lot longer. I see metrics above around error rates, do we track IP allocation time today (I suspect NOT).
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.
There are no metrics about IP allocation time, I've considered to add those metrics but the problem is that they are tied to the implementation of the allocator Interface, in this case the bitmap, but the same logic is used for nodeports, so you can't differentiate IPs from NodePorts when you are using the bitmaps. However, sounds a good metrics to add in the new API for Service allocation.
Operations can take longer, but this only changes the strategy to find a free number into a bitmap, with current change instead of doing a linear search in the whole bitmap with size N, we split the bitmap by a constant. We do the linear search on the N-K, and if not found , do the search in the remaining K bits, and K is always a small number, I don't think that there is much impact on a Service Create API call
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.
Agree with Antonio.
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aojea, johnbelamaric, thockin 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 |