feat: Support for static AzureCNI without overlay networking via generating additional ip configurations #365

Bryce-Soghigian · 2024-05-21T07:20:41Z

Fixes #367

Description
This PR adds support for azure cni without overlay, as well as introduces some makefile goodness for creating clusters or other cni configurations.

Why Do We Need Secondary IP Configs For AZ CNI Without Overlay?

When a pod is created, the Azure CNI plugin allocates an IP address from the pool of secondary IP addresses configured on the NIC of the node where the pod is scheduled. The Azure CNI plugin manages the allocation and de-allocation of these IP addresses through the IP Address Manager (IPAM), ensuring each pod receives a unique IP address and tracking the usage of these addresses.

In this setup, pods are assigned IP addresses from the node's subnet, allowing for direct IP connectivity. This enables pods within the same virtual network to communicate without the need for Network Address Translation (NAT). The node's NIC routes traffic to the appropriate pod based on the assigned IP.

Flow

Node NIC: Primary IP and multiple secondary IP addresses are assigned to the nic on node creation, and the nic is assigned to the node
Pod Create: Pod requests an IP address on intialization
Azure CNI Plugin: Assigns a secondary IP to the pod from the node’s NIC.
Network Interface within Node: We use transparent mode, which doesn't change any properties of the eth0 interface on the nic. Azure CNI creates and adds host-side pod veth pair interfaces that are added to the host network.
Pod to Pod Communication: Pods communicate using their assigned IPs directly within the virtual network. Pod to Pod communication is over layer 3 and L3 routing rules route the pod traffic.

Learn more about specifics here

How was this change tested?

e2e tests see: https://github.com/Azure/karpenter-provider-azure/actions/runs/10950653145/job/30407746822
unit tests
manual scale ups and goldpinger

What this PR does not include

E2E Suite for our Upstream E2Es here testing other network connectivity(ingress + egress)
Port of our E2Es to run fully each time on azure cni without overlay
Does this change impact docs?
Yes, PR includes docs updates
Yes, issue opened: #
No

Release Note

support added for static AzureCNI without overlay networking

…k-plugin-mode

…or non azure cni overlay setups

…lugin

…ed constants in the project

…overlay

… of the cluster

…on of propagating kubelet configuration this way

…rlay configuration

… know have two tests checking cluster state

pkg/test/expectations/expectations.go

Bryce-Soghigian

/test

…gian/aks-karpenter into bsoghigian/azurecni-v1-clean

Bryce-Soghigian · 2024-09-10T18:33:09Z

pkg/providers/instance/instance.go

+type createNICOptions struct {
+	NICName           string
+	BackendPools      *loadbalancer.BackendAddressPools
+	InstanceType      *corecloudprovider.InstanceType
+	LaunchTemplate    *launchtemplate.Template
+	NetworkPluginMode string
+}


There probably exists a better pattern for this that splits the population of the creation options and the actual reading of those options. Its a bit messy here because we set some values like BackendPools after the fact rather than initially.

Also creation options as a pattern could be more widely leveraged for VM creation as well. This was feedback placed here originally to split up some options rather than just using context as a global retrieval object.

But seems like a lot to tackle restructuring the entire dataflow for the scope of this PR. It deserves its own proper refactoring PR.

Bryce-Soghigian

/test

Bryce-Soghigian

/test

pkg/providers/imagefamily/azlinux.go

…etworkPluginNotReady message:Network plugin returns error: cni plugin not initialized

hack/deploy/configure-values.sh

pkg/operator/options/options.go

Bryce-Soghigian · 2024-09-19T20:51:41Z

From what I can tell, in terms of the contract there are two places where NETWORK_PLUGIN is used

https://github.com/Azure/AgentBaker/blob/1c4f87e96b26cd8933a16cc31eaa9d18025932f1/self-contained/bootstrap_install.sh#L62

installNetworkPlugin() {
    if [[ "${NETWORK_PLUGIN}" = "azure" ]]; then
        installAzureCNI
    fi
    installCNI  #reference plugins. Mostly for kubenet but loop back used by contaierd until containerd 2
    rm -rf $CNI_DOWNLOADS_DIR &
}

https://github.com/Azure/AgentBaker/blob/1c4f87e96b26cd8933a16cc31eaa9d18025932f1/self-contained/bootstrap_config.sh#L283

configureCNIIPTables() {
    if [[ "${NETWORK_PLUGIN}" = "azure" ]]; then
        mv $CNI_BIN_DIR/10-azure.conflist $CNI_CONFIG_DIR/
        chmod 600 $CNI_CONFIG_DIR/10-azure.conflist
        if [[ "${NETWORK_POLICY}" == "calico" ]]; then
          sed -i 's#"mode":"bridge"#"mode":"transparent"#g' $CNI_CONFIG_DIR/10-azure.conflist
        elif [[ "${NETWORK_POLICY}" == "" || "${NETWORK_POLICY}" == "none" ]] && [[ "${NETWORK_MODE}" == "transparent" ]]; then
          sed -i 's#"mode":"bridge"#"mode":"transparent"#g' $CNI_CONFIG_DIR/10-azure.conflist
        fi
        /sbin/ebtables -t nat --list
    fi
}

but we need it to support BYO CNI it seems based on these two references in Agentbaker

…reason:NetworkPluginNotReady message:Network plugin returns error: cni plugin not initialized" This reverts commit 5d88685.

This reverts commit 965830f.

…ce-Soghigian/aks-karpenter into bsoghigian/azurecni-v1-clean

Bryce-Soghigian · 2024-09-20T01:14:53Z

E2E Run: https://github.com/Azure/karpenter-provider-azure/actions/runs/10950653145/job/30407746822

tangyouzzz · 2024-09-20T01:22:24Z

From what I can tell, in terms of the contract there are two places where NETWORK_PLUGIN is used

https://github.com/Azure/AgentBaker/blob/1c4f87e96b26cd8933a16cc31eaa9d18025932f1/self-contained/bootstrap_install.sh#L62

installNetworkPlugin() {
    if [[ "${NETWORK_PLUGIN}" = "azure" ]]; then
        installAzureCNI
    fi
    installCNI  #reference plugins. Mostly for kubenet but loop back used by contaierd until containerd 2
    rm -rf $CNI_DOWNLOADS_DIR &
}

https://github.com/Azure/AgentBaker/blob/1c4f87e96b26cd8933a16cc31eaa9d18025932f1/self-contained/bootstrap_config.sh#L283

configureCNIIPTables() {
    if [[ "${NETWORK_PLUGIN}" = "azure" ]]; then
        mv $CNI_BIN_DIR/10-azure.conflist $CNI_CONFIG_DIR/
        chmod 600 $CNI_CONFIG_DIR/10-azure.conflist
        if [[ "${NETWORK_POLICY}" == "calico" ]]; then
          sed -i 's#"mode":"bridge"#"mode":"transparent"#g' $CNI_CONFIG_DIR/10-azure.conflist
        elif [[ "${NETWORK_POLICY}" == "" || "${NETWORK_POLICY}" == "none" ]] && [[ "${NETWORK_MODE}" == "transparent" ]]; then
          sed -i 's#"mode":"bridge"#"mode":"transparent"#g' $CNI_CONFIG_DIR/10-azure.conflist
        fi
        /sbin/ebtables -t nat --list
    fi
}

but we need it to support BYO CNI it seems based on these two references in Agentbaker

After adding the NETWORK-POLICY variable, Karpenter should be able to be used in underlay network and Azure Cni AKS

tangyouzzz · 2024-09-20T01:42:00Z

pkg/providers/imagefamily/resolver.go

+	if kubeletConfig == nil {
+		kubeletConfig = &corev1beta1.KubeletConfiguration{}
+	}
+	kubeletConfig.MaxPods = lo.ToPtr[int32](consts.DefaultKubernetesMaxPodsAzure)


`consts.DefaultKubernetesMaxPodsAzure' is a fixed value, and some nodes may not seem to need to allocate so many IPs. Can we first obtain it through the maxpod field in the nodepool, and only use the default value if it is not defined?

kubelet configuration isnt something we are supporting for GA

Okay, thank you for your answer

Bryce-Soghigian and others added 25 commits April 28, 2024 01:59

feat: adding --network-plugin-mode flag to options

9ef7fd4

test: adding --network-plugin-mode to test env

c666565

feat: conditionally setting labels based on network-policy and networ…

a1f47c3

…k-plugin-mode

test: validating that network plugin labels are not placed on nodes f…

fbfcf55

…or non azure cni overlay setups

style: make presubmit golangci-lint

5ec81ef

feat: adding network-plugin-mode to helm values

fec18f4

fix: modiying comment to specify network policy rather than network p…

60d85ee

…lugin

style: better comment for NetworkPluginMode

8f49918

Merge branch 'main' into bsoghigian/overlay-plumbing

5421f1f

chore: removing ciliummm conditional logic

d13abc7

Merge branch 'main' into bsoghigian/overlay-plumbing

eed47a6

refactor: MaxPods into utils and adding a new consts package for shar…

b5e0423

…ed constants in the project

feat: set secondary ipconfigs on the nic when podnetwork type is not …

d7f215c

…overlay

test: adding new cluster create commands

ce0784e

fix: do not allocate additional ips for host network addons

e140d38

fix: properly counting max ips

a277cd6

test: running e2es on azure cni v1

06d94c6

fix: az-configure-values should be aware of the network configuration…

bb26d2e

… of the cluster

fix: cleaning up tests and DefaultMaxPods calc

db41576

refactor: removing maxPods from launch template as we have no intenti…

6356024

…on of propagating kubelet configuration this way

test: validating nic configurations still valid for azurecni with ove…

6fd6759

…rlay configuration

style: grammar fixes after a read through

8368a5e

test: adding ginkgo helper for checking kubelet flags

0152faf

test: modifying instancetypes tests to use the ExpectKubeletFlags helper

664da18

fix: resetting cluster state in instance provider suite test since we…

84deccb

… know have two tests checking cluster state

Bryce-Soghigian commented May 21, 2024

View reviewed changes

pkg/test/expectations/expectations.go Outdated Show resolved Hide resolved

Bryce-Soghigian commented May 21, 2024

View reviewed changes

Bryce-Soghigian and others added 3 commits May 21, 2024 15:05

ci: running golangci-lint

1631143

Merge branch 'main' into bsoghigian/azurecni-v1-clean

124d6ab

Merge branch 'bsoghigian/azurecni-v1-clean' of github.com:Bryce-Soghi…

3f7e27e

…gian/aks-karpenter into bsoghigian/azurecni-v1-clean

Bryce-Soghigian requested review from tallaxes, rakechill and charliedmcb June 14, 2024 17:34

Bryce-Soghigian commented Sep 10, 2024

View reviewed changes

Bryce-Soghigian force-pushed the bsoghigian/azurecni-v1-clean branch from 1413fe8 to 36cc5a2 Compare September 12, 2024 21:11

Merge branch 'main' into bsoghigian/azurecni-v1-clean

91964a7

Bryce-Soghigian commented Sep 12, 2024

View reviewed changes

fix: conflict

7b35dca

Bryce-Soghigian commented Sep 12, 2024

View reviewed changes

ci: lint

a31e10a

tangyouzzz reviewed Sep 19, 2024

View reviewed changes

pkg/providers/imagefamily/azlinux.go Outdated Show resolved Hide resolved

fix: container runtime network not ready: NetworkReady=false reason:N…

5d88685

…etworkPluginNotReady message:Network plugin returns error: cni plugin not initialized

Bryce-Soghigian force-pushed the bsoghigian/azurecni-v1-clean branch from d2dbd58 to 5d88685 Compare September 19, 2024 16:12

refactor: removing network plugin from values

965830f

Bryce-Soghigian commented Sep 19, 2024

View reviewed changes

hack/deploy/configure-values.sh Show resolved Hide resolved

tallaxes reviewed Sep 19, 2024

View reviewed changes

pkg/operator/options/options.go Show resolved Hide resolved

Bryce-Soghigian added 5 commits September 19, 2024 14:29

revert: progress

356491f

Revert "fix: container runtime network not ready: NetworkReady=false …

fd5b15e

…reason:NetworkPluginNotReady message:Network plugin returns error: cni plugin not initialized" This reverts commit 5d88685.

....

a7fd642

Revert "refactor: removing network plugin from values"

19ae6e2

This reverts commit 965830f.

fix: test

4752103

Bryce-Soghigian requested a review from tallaxes September 19, 2024 23:26

Bryce-Soghigian and others added 4 commits September 19, 2024 16:30

Merge branch 'main' into bsoghigian/azurecni-v1-clean

bf1830c

fix: ci...............

2b83536

Merge branch 'bsoghigian/azurecni-v1-clean' of https://github.com/Bry…

f5f5f9f

…ce-Soghigian/aks-karpenter into bsoghigian/azurecni-v1-clean

Merge branch 'main' into bsoghigian/azurecni-v1-clean

31056e7

tangyouzzz reviewed Sep 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Support for static AzureCNI without overlay networking via generating additional ip configurations #365

feat: Support for static AzureCNI without overlay networking via generating additional ip configurations #365

Bryce-Soghigian commented May 21, 2024 •

edited

Loading

Bryce-Soghigian left a comment

Bryce-Soghigian Sep 10, 2024

Bryce-Soghigian left a comment

Bryce-Soghigian left a comment

Bryce-Soghigian commented Sep 19, 2024

Bryce-Soghigian commented Sep 20, 2024

tangyouzzz commented Sep 20, 2024

tangyouzzz Sep 20, 2024

Bryce-Soghigian Sep 20, 2024

tangyouzzz Sep 20, 2024

feat: Support for static AzureCNI without overlay networking via generating additional ip configurations #365

Are you sure you want to change the base?

feat: Support for static AzureCNI without overlay networking via generating additional ip configurations #365

Conversation

Bryce-Soghigian commented May 21, 2024 • edited Loading

Why Do We Need Secondary IP Configs For AZ CNI Without Overlay?

Flow

Bryce-Soghigian left a comment

Choose a reason for hiding this comment

Bryce-Soghigian Sep 10, 2024

Choose a reason for hiding this comment

Bryce-Soghigian left a comment

Choose a reason for hiding this comment

Bryce-Soghigian left a comment

Choose a reason for hiding this comment

Bryce-Soghigian commented Sep 19, 2024

Bryce-Soghigian commented Sep 20, 2024

tangyouzzz commented Sep 20, 2024

tangyouzzz Sep 20, 2024

Choose a reason for hiding this comment

Bryce-Soghigian Sep 20, 2024

Choose a reason for hiding this comment

tangyouzzz Sep 20, 2024

Choose a reason for hiding this comment

Bryce-Soghigian commented May 21, 2024 •

edited

Loading