feat: add DisableOutboundSNAT feature #1708

sylr · 2019-08-01T12:56:28Z

Signed-off-by: Sylvain Rabot [email protected]

Reason for Change:

aks-engine currently does not allow to specify the cloud provider setting disableOutboundSnat.

Issue Fixed:

Related to #1689
Fixes #1001

Requirements:

uses conventional commit messages
includes documentation
adds unit tests
tested upgrade from previous version

Notes:

jackfrancis · 2019-08-02T00:16:49Z

/azp run pr-e2e

azure-pipelines · 2019-08-02T00:16:57Z

Azure Pipelines successfully started running 1 pipeline(s).

jackfrancis · 2019-08-06T16:28:49Z

@sylr What's the purpose of this change? Is it to override the default behavior and explicitly disable outbound SNAT for Standard LB configurations?

@feiskyer, does it make sense to expose this configuration to users?

jackfrancis · 2019-08-06T16:29:06Z

Here is the current default implementation that I'm referring to:

https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go#L394

CecileRobertMichon · 2019-08-06T18:01:50Z

pkg/api/types.go

@@ -116,6 +116,7 @@ type FeatureFlags struct {
 	EnableCSERunInBackground bool `json:"enableCSERunInBackground,omitempty"`
 	BlockOutboundInternet    bool `json:"blockOutboundInternet,omitempty"`
 	EnableIPv6DualStack      bool `json:"enableIPv6DualStack,omitempty"`
+	DisableOutboundSNAT      bool `json:"disableOutboundSnat,omitempty"`


does this belong in KubernetesConfig instead? featureFlags is not meant to be used for k8s configuration but instead allow testing of experimental features without exposing them in docs. The loadBalancerSku and excludeMasterFromStandardLB knobs are in KubernetesConfig for example.

If you do move it to KubernetesConfig, could you also include it in docs/topics/clusterdefinitions.md?

feiskyer · 2019-08-07T01:32:57Z

@feiskyer, does it make sense to expose this configuration to users?

Yep, for SLB with outbound rules defined, disableOutboundSnat could be enabled to ensure external traffic is going through outbound rules. Refer https://docs.microsoft.com/en-us/azure/load-balancer/load-balancer-outbound-rules-overview#outbound-rule.

jackfrancis · 2019-08-23T18:18:50Z

pkg/engine/armvariables.go

@@ -80,6 +80,62 @@ func getK8sMasterVars(cs *api.ContainerService) (map[string]interface{}, error)
 		kubernetesVersion = orchProfile.OrchestratorVersion + AzureStackSuffix
 	}

+	provisionScriptParametersCommonString := "[concat(" +


Thank you @sylr for this code maintenance improvement :)

@CecileRobertMichon @devigned @mboersma FYI

(Also, I manually validated that nothing was lost during translation.)

jackfrancis · 2019-08-23T20:20:23Z

/azp run pr-e2e

azure-pipelines · 2019-08-23T20:20:34Z

Azure Pipelines successfully started running 1 pipeline(s).

jackfrancis · 2019-08-23T21:07:46Z

/azp run pr-e2e

azure-pipelines · 2019-08-23T21:07:55Z

Azure Pipelines successfully started running 1 pipeline(s).

mboersma · 2019-08-30T14:26:11Z

/azp run pr-e2e

azure-pipelines · 2019-08-30T14:26:21Z

Azure Pipelines successfully started running 1 pipeline(s).

jackfrancis · 2019-09-21T01:21:01Z

/azp run pr-e2e

azure-pipelines · 2019-09-21T01:21:18Z

Azure Pipelines successfully started running 1 pipeline(s).

sylr · 2019-09-23T14:25:47Z

pkg/api/types.go

@@ -320,6 +320,7 @@ type CloudProviderConfig struct {
 	CloudProviderRateLimitQPSWrite    string `json:"cloudProviderRateLimitQPSWrite,omitempty"`
 	CloudProviderRateLimitBucket      int    `json:"cloudProviderRateLimitBucket,omitempty"`
 	CloudProviderRateLimitBucketWrite int    `json:"cloudProviderRateLimitBucketWrite,omitempty"`
+	CloudProviderDisableOutboundSNAT  bool   `json:"cloudProviderDisableOutboundSnat,omitempty"`


Snat instead of SNAT ?

jackfrancis · 2019-10-29T23:41:08Z

/azp run pr-e2e

azure-pipelines · 2019-10-30T00:38:32Z

Azure Pipelines successfully started running 1 pipeline(s).

codecov · 2019-10-30T01:12:24Z

Codecov Report

Merging #1708 into master will increase coverage by 0.07%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1708      +/-   ##
==========================================
+ Coverage    71.6%   71.67%   +0.07%     
==========================================
  Files         142      142              
  Lines       24817    24881      +64     
==========================================
+ Hits        17770    17834      +64     
  Misses       5911     5911              
  Partials     1136     1136

marcel-dempers · 2019-11-12T03:57:56Z

Hi all,

Just checking in to see updates on this item ?

Currently facing an issue of Standard LB not honoring outbound rules because of disableOutboundSnat=false. I've been comparing the LB between aks-engine and aks and support engineer from Microsoft told me disableOutboundSnat should be set to true for egress to work correctly. AKS currently sets it to true for all LB rules. AKS-engine seems to use default (false).

Let me know if there is anything I can contribute \ test out.

Signed-off-by: Sylvain Rabot <[email protected]>

jackfrancis · 2019-11-12T23:19:25Z

/azp run pr-e2e

azure-pipelines · 2019-11-12T23:19:35Z

Azure Pipelines successfully started running 1 pipeline(s).

CecileRobertMichon · 2019-11-13T00:30:05Z

pkg/api/defaults.go

+				o.KubernetesConfig.CloudProviderDisableOutboundSNAT = to.BoolPtr(false)
+			}
+		} else {
+			// CloudProviderDisableOutboundSNAT is only valid in the context of Standard LB, statically set to false if not Standard LB


Can we drop a note in docs about this?

CecileRobertMichon

lgtm pending docs

jackfrancis · 2019-11-13T01:10:33Z

docs/topics/clusterdefinitions.md

@@ -73,6 +73,18 @@ $ aks-engine get-versions
 | maximumLoadBalancerRuleCount     | no                        | Maximum allowed LoadBalancer Rule Count is the limit enforced by Azure Load balancer. Default is 250                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                      |
 | kubeProxyMode                    | no                        | kube-proxy --proxy-mode value, either "iptables" or "ipvs". Default is "iptables". See https://kubernetes.io/blog/2018/07/09/ipvs-based-in-cluster-load-balancing-deep-dive/ for further reference.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                       |
 | outboundRuleIdleTimeoutInMinutes | no                        | Specifies a value for IdleTimeoutInMinutes to control the outbound flow idle timeout of the agent standard loadbalancer. This value is set greater than the default Linux idle timeout (15.4 min): https://pracucci.com/linux-tcp-rto-min-max-and-tcp-retries2.html                                                                                                                                                                                                                                                                                                                                                                                                                                                                       |
+| cloudProviderBackoff | no                        | Use the Azure cloudprovider exponential backoff implementation when encountering retry-able errors from the Azure API. Defaults to `true` for Kubernetes v1.14.0 and greater.                                                                                                                                                    |


@CecileRobertMichon There was a rather large cloudprovider config documentation gap, so I used this PR as an excuse to backfill that info.

acs-bot · 2019-11-13T01:10:50Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sylr
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: marosset

If they are not already assigned, you can assign the PR to them by writing /assign @marosset in a comment when ready.

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:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jackfrancis · 2019-11-13T01:11:25Z

/azp run pr-e2e

azure-pipelines · 2019-11-13T01:11:36Z

Azure Pipelines successfully started running 1 pipeline(s).

sylr force-pushed the disable-outbound-nat branch 2 times, most recently from 2b039b8 to 2eae604 Compare August 1, 2019 13:24

CecileRobertMichon added the failing tests label Aug 5, 2019

sylr force-pushed the disable-outbound-nat branch from 2eae604 to 3b8ec46 Compare August 6, 2019 14:06

acs-bot added the size/M label Aug 6, 2019

CecileRobertMichon reviewed Aug 6, 2019

View reviewed changes

jackfrancis force-pushed the disable-outbound-nat branch from 3b8ec46 to 79dad8b Compare August 23, 2019 18:10

jackfrancis reviewed Aug 23, 2019

View reviewed changes

acs-bot added size/L and removed size/M labels Aug 23, 2019

jackfrancis force-pushed the disable-outbound-nat branch from d9bb943 to 3f14d1b Compare September 19, 2019 21:23

sylr commented Sep 23, 2019

View reviewed changes

sylr force-pushed the disable-outbound-nat branch from 0818c73 to 546c1df Compare October 7, 2019 13:25

jackfrancis force-pushed the disable-outbound-nat branch from 2ec4ad6 to 380664a Compare October 29, 2019 22:46

jackfrancis added this to the v0.44.0 milestone Oct 31, 2019

Sylvain Rabot and others added 7 commits November 12, 2019 13:25

feat: add DisableOutboundSNAT feature

3fe6db2

Signed-off-by: Sylvain Rabot <[email protected]>

chore: move to KubernetesConfig

5fc75b5

chore: errata and generated code

59c62f2

fix: remove unused mastervars, update UT

348e353

chore: align SNAT case

cd7eca9

Signed-off-by: Sylvain Rabot <[email protected]>

fix: printf fixes

06b117c

chore: generated code

c17be20

jackfrancis force-pushed the disable-outbound-nat branch from cfeb7b0 to c17be20 Compare November 12, 2019 21:47

fix: this works

c18afad

CecileRobertMichon removed the failing tests label Nov 13, 2019

CecileRobertMichon reviewed Nov 13, 2019

View reviewed changes

CecileRobertMichon previously approved these changes Nov 13, 2019

View reviewed changes

acs-bot added the approved label Nov 13, 2019

docs: add lots of cloudprovider config docs

eb7c975

jackfrancis dismissed CecileRobertMichon’s stale review via eb7c975 November 13, 2019 01:08

jackfrancis reviewed Nov 13, 2019

View reviewed changes

acs-bot removed the approved label Nov 13, 2019

jackfrancis merged commit 8cf7c2a into Azure:master Nov 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add DisableOutboundSNAT feature #1708

feat: add DisableOutboundSNAT feature #1708

sylr commented Aug 1, 2019 •

edited by jackfrancis

Loading

jackfrancis commented Aug 2, 2019

azure-pipelines bot commented Aug 2, 2019

jackfrancis commented Aug 6, 2019

jackfrancis commented Aug 6, 2019

CecileRobertMichon Aug 6, 2019

CecileRobertMichon Aug 6, 2019

feiskyer commented Aug 7, 2019

jackfrancis Aug 23, 2019

jackfrancis commented Aug 23, 2019

azure-pipelines bot commented Aug 23, 2019

jackfrancis commented Aug 23, 2019

azure-pipelines bot commented Aug 23, 2019

mboersma commented Aug 30, 2019

azure-pipelines bot commented Aug 30, 2019

jackfrancis commented Sep 21, 2019

azure-pipelines bot commented Sep 21, 2019

sylr Sep 23, 2019

jackfrancis commented Oct 29, 2019

azure-pipelines bot commented Oct 30, 2019

codecov bot commented Oct 30, 2019 •

edited

Loading

marcel-dempers commented Nov 12, 2019

jackfrancis commented Nov 12, 2019

azure-pipelines bot commented Nov 12, 2019

CecileRobertMichon Nov 13, 2019

CecileRobertMichon left a comment

jackfrancis Nov 13, 2019

acs-bot commented Nov 13, 2019

jackfrancis commented Nov 13, 2019

azure-pipelines bot commented Nov 13, 2019

feat: add DisableOutboundSNAT feature #1708

feat: add DisableOutboundSNAT feature #1708

Conversation

sylr commented Aug 1, 2019 • edited by jackfrancis Loading

jackfrancis commented Aug 2, 2019

azure-pipelines bot commented Aug 2, 2019

jackfrancis commented Aug 6, 2019

jackfrancis commented Aug 6, 2019

CecileRobertMichon Aug 6, 2019

Choose a reason for hiding this comment

CecileRobertMichon Aug 6, 2019

Choose a reason for hiding this comment

feiskyer commented Aug 7, 2019

jackfrancis Aug 23, 2019

Choose a reason for hiding this comment

jackfrancis commented Aug 23, 2019

azure-pipelines bot commented Aug 23, 2019

jackfrancis commented Aug 23, 2019

azure-pipelines bot commented Aug 23, 2019

mboersma commented Aug 30, 2019

azure-pipelines bot commented Aug 30, 2019

jackfrancis commented Sep 21, 2019

azure-pipelines bot commented Sep 21, 2019

sylr Sep 23, 2019

Choose a reason for hiding this comment

jackfrancis commented Oct 29, 2019

azure-pipelines bot commented Oct 30, 2019

codecov bot commented Oct 30, 2019 • edited Loading

Codecov Report

marcel-dempers commented Nov 12, 2019

jackfrancis commented Nov 12, 2019

azure-pipelines bot commented Nov 12, 2019

CecileRobertMichon Nov 13, 2019

Choose a reason for hiding this comment

CecileRobertMichon left a comment

Choose a reason for hiding this comment

jackfrancis Nov 13, 2019

Choose a reason for hiding this comment

acs-bot commented Nov 13, 2019

jackfrancis commented Nov 13, 2019

azure-pipelines bot commented Nov 13, 2019

sylr commented Aug 1, 2019 •

edited by jackfrancis

Loading

codecov bot commented Oct 30, 2019 •

edited

Loading