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

chore: use transparent mode for Azure CNI #3958

Merged
merged 2 commits into from
Oct 29, 2020

Conversation

jackfrancis
Copy link
Member

Reason for Change:

This PR changes the Azure CNI implementation to use "transparent" mode by default, rather than using a bridge interface. This change is due to an observed reduction in UDP edge cases, specifically intermittent high DNS lookup latency when the resolver is an IP address that sits in front of a distributed set of resolvers (DNAT). In the Kubernetes context, this is reproduced if there are more than one coredns pods in the cluster.

Fixes #3955

Issue Fixed:

Credit Where Due:

Does this change contain code from or inspired by another project?

  • No
  • Yes

If "Yes," did you notify that project's maintainers and provide attribution?

  • No
  • Yes

Requirements:

Notes:

@acs-bot acs-bot added the size/S label Oct 21, 2020
@jackfrancis
Copy link
Member Author

@chandanAggarwal
Copy link

cc @vakalapa

@codecov
Copy link

codecov bot commented Oct 22, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@e2e07a0). Click here to learn what that means.
The diff coverage is n/a.


if o.KubernetesConfig.NetworkPlugin == NetworkPluginAzure {
if o.KubernetesConfig.NetworkMode == "" {
o.KubernetesConfig.NetworkMode = NetworkModeTransparent
Copy link
Member

Choose a reason for hiding this comment

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

sorry for a dumb question but what actually actions on network mode? I couldn't find it in my novice sluething. I just found this in cse.

if [[ "${NETWORK_POLICY}" == "calico" ]]; then

284 | sed -i 's#"mode":"bridge"#"mode":"transparent"#g' $CNI_CONFIG_DIR/10-azure.conflist

Choose a reason for hiding this comment

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

sed -i 's#"mode":"bridge"#"mode":"transparent"#g' $CNI_CONFIG_DIR/10-azure.conflist

is it this one?

Copy link
Member Author

Choose a reason for hiding this comment

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

@weng271190436 has it right

Basically, the Azure CNI release artifact comes with a default static config, and we simply extract it, then sed replace "bridge" with "transparent" if the api model says to do so.

Copy link
Member

Choose a reason for hiding this comment

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

Missed it by two lines and thought it was only calico that mattered. @matmerr planning on just having a transparent only conflist in cni 1.1.9. That means if someone explicity sets bridge mode it wouldn't work anymore. They'd have to know to fall back to 1.1.8.

In aks we control the cni url so we were planning on toggling that not using network mode.


if o.KubernetesConfig.NetworkPlugin == NetworkPluginAzure {
if o.KubernetesConfig.NetworkMode == "" {
o.KubernetesConfig.NetworkMode = NetworkModeTransparent

Choose a reason for hiding this comment

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

sed -i 's#"mode":"bridge"#"mode":"transparent"#g' $CNI_CONFIG_DIR/10-azure.conflist

is it this one?

Copy link
Member

@mboersma mboersma left a comment

Choose a reason for hiding this comment

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

/lgtm

@acs-bot
Copy link

acs-bot commented Oct 29, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jackfrancis, mboersma, weng271190436

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 [jackfrancis,mboersma]

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

@jackfrancis jackfrancis merged commit a8def60 into Azure:master Oct 29, 2020
@jackfrancis jackfrancis deleted the azure-cni-transparent branch October 29, 2020 18:34
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.

DNS latency when using Azure CNI bridge mode + multiple coredns pods
6 participants