Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

fix: default LB AllocatedOutboundPorts to 0 #2526

Merged
merged 4 commits into from
Jan 30, 2020

Conversation

jackfrancis
Copy link
Member

@jackfrancis jackfrancis commented Jan 7, 2020

Reason for Change:

This PR sets the AllocatedOutboundPorts configuration to 0, as this is a preferred default configuration for LB outbound rules.

0 is equivalent to "automatic", which means is preferable to the Azure-provided default of 1024 , which essentially caps SNAT ports at that number.

See here for more info:

https://docs.microsoft.com/en-us/azure/load-balancer/load-balancer-outbound-rules-overview

Issue Fixed:

Fixes #2558

Requirements:

Notes:

@codecov
Copy link

codecov bot commented Jan 7, 2020

Codecov Report

Merging #2526 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2526      +/-   ##
==========================================
+ Coverage   71.79%   71.79%   +<.01%     
==========================================
  Files         131      131              
  Lines       24787    24789       +2     
==========================================
+ Hits        17795    17797       +2     
  Misses       5966     5966              
  Partials     1026     1026

@Michael-Sinz
Copy link
Collaborator

Setting this to 4096 is not a good default as it will keep clusters to under 15 nodes.

Setting this to 0 will use the adaptive behavior of the LB:

https://docs.microsoft.com/en-us/azure/load-balancer/load-balancer-outbound-rules-overview#snatports

The option/value to set a fixed smaller number (say 32, in order to be able to reach 1000 nodes like the automatic table says) to prevent the strange networking problem when crossing a VM count boundary as describe here:

https://docs.microsoft.com/en-us/azure/load-balancer/load-balancer-outbound-connections#preallocatedports

@acs-bot acs-bot added size/M and removed size/S labels Jan 10, 2020
@jackfrancis jackfrancis changed the title [WIP] feat: configurable AllocatedOutboundPorts in LB fix: default LB AllocatedOutboundPorts to 0 Jan 10, 2020
@jackfrancis
Copy link
Member Author

We will do #2377 as a follow-up PR to make this configurable

Michael-Sinz
Michael-Sinz previously approved these changes Jan 10, 2020
Copy link
Collaborator

@Michael-Sinz Michael-Sinz left a comment

Choose a reason for hiding this comment

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

Looks good to me and matches how I have manually tested the creation of clusters for our systems.

This extra value (set to 0) matches how prior Azure load balancer setup was done in the past and allows scaling into the hundreds of VMs in our clusters.

At some point it would be nice to allow specifying a specific value here but it is a complex set of choices for people and the automatic setting provides the best tradeoff for the general rule.

Michael-Sinz
Michael-Sinz previously approved these changes Jan 22, 2020
@acs-bot
Copy link

acs-bot commented Jan 22, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jackfrancis

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jackfrancis jackfrancis force-pushed the allocatedOutboundPorts branch from bc36175 to 237de5b Compare January 24, 2020 18:14
@jackfrancis jackfrancis added this to the v0.47.0 milestone Jan 29, 2020
@jackfrancis jackfrancis merged commit 142c538 into Azure:master Jan 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AllocatedOutboundPorts defaults to 1024, which is too low
3 participants