Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

Azure CNI enabled by default for Kubernetes clusters (Linux only) #1887

Merged
merged 11 commits into from
Dec 6, 2017

Conversation

jackfrancis
Copy link
Member

@jackfrancis jackfrancis commented Dec 5, 2017

What this PR does / why we need it: Ships Azure CNI as network implementation for new Kubernetes clusters, unless otherwise specified.

Docs updated to reflect default change from kubenet to Azure CNI.

Fixes #1815 Fixes #1791

Release note:

Azure CNI enabled by default for Kubernetes clusters

@ghost ghost assigned jackfrancis Dec 5, 2017
@ghost ghost added the in progress label Dec 5, 2017
@jackfrancis jackfrancis changed the title Azure CNI enabled by default for Kubernetes clusters Azure CNI enabled by default for Kubernetes clusters (Linux only) Dec 5, 2017
@jackfrancis
Copy link
Member Author

@sharmasushant this is ready for review. This PR will deliver Azure CNI as a default networking implementation, with appropriate documentation and test configuration changes.

Following this functional PR, we can work to improve higher order concerns with respect to Kubernetes network config (api model interface, documentation especially). This is a first pass that delivers Azure CNI to new cluster deployments by default.

"vmSize": "Standard_D2_v2",
"vnetSubnetId": "/subscriptions/SUB_ID/resourceGroups/RG_NAME/providers/Microsoft.Network/virtualNetworks/VNET_NAME/subnets/SUBNET_NAME",
"firstConsecutiveStaticIP": "10.239.255.239",
"vnetCidr": "10.239.0.0/16"
Copy link
Member Author

Choose a reason for hiding this comment

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

vnetCidr is the interesting bit here for a new Azure CNI custom VNET example api model

@@ -0,0 +1,14 @@
#!/bin/bash
Copy link
Member Author

Choose a reason for hiding this comment

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

This file is basically the same as the existing kubenet VNET predeploy script, but we create a custom VNET with only one address space to more accurately match the current functional support for k8s w/ Azure CNI (documentation explains it above, but the short story is that we don't have a 100% resilient solution yet for custom VNETs w/ lots of address space that can't be rationalized into a single CIDR network block).

@@ -0,0 +1 @@
ACSE_PREDEPLOY=examples/vnet/k8s-vnet-azure-cni-predeploy.sh
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that there's no post-deploy reference here because Azure CNI custom VNET attachment doesn't need the post-deployment route table attachment step that kubenet VNET attachment requires.

// DefaultNetworkPolicy defines the network policy to use by default
DefaultNetworkPolicy = "azure"
// DefaultNetworkPolicyWindows defines the network policy to use by default for clusters with Windows agent pools
DefaultNetworkPolicyWindows = "none"
Copy link
Member Author

Choose a reason for hiding this comment

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

Windows clusters don't yet work w/ Azure CNI.

@@ -14,7 +14,8 @@
"dnsPrefix": "masterdns1",
"vmSize": "Standard_D2_v2",
"vnetSubnetId": "/subscriptions/SUBSCRIPTION/resourceGroups/KubeVnet/providers/Microsoft.Network/virtualNetworks/KubernetesCustomVNET/subnets/KubernetesSubnet",
"firstConsecutiveStaticIP": "10.239.255.245"
"firstConsecutiveStaticIP": "10.239.255.245",
"vnetCidr": "172.40.0.0/16"
Copy link
Member Author

Choose a reason for hiding this comment

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

I converted this into an Azure CNI vnet configuration.

@@ -135,13 +135,6 @@ func (cli *CLIProvisioner) provision() error {
return fmt.Errorf("Error while trying to create deployment:%s", err)
}

if cli.CreateVNET {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is in the ginkgo E2E runner. We're converting the custom VNET attachment over to Azure CNI.

@@ -15,7 +14,8 @@
"vmSize": "Standard_D2_v2",
"OSDiskSizeGB": 200,
"vnetSubnetId": "/subscriptions/SUB_ID/resourceGroups/RG_NAME/providers/Microsoft.Network/virtualNetworks/VNET_NAME/subnets/SUBNET_NAME",
"firstConsecutiveStaticIP": "10.239.255.239"
"firstConsecutiveStaticIP": "10.239.255.239",
"vnetCidr": "10.230.0.0/16"
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the kubernetes api model that we feed as input to the ginkgo E2E runner. Added vnetCidr as a way of converting this api model over to an Azure CNI w/ custom VNET cluster.

Copy link
Contributor

@CecileRobertMichon CecileRobertMichon left a comment

Choose a reason for hiding this comment

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

Not sure but should the 3rd line of the table in features.md ( |Custom VNET|Beta|vlabs|kubernetesvnet.json|Description|) point to the azure-cni example since that's default now?

After a cluster finishes provisioning, fetch the id of the Route Table resource from `Microsoft.Network` provider in your new cluster's Resource Group.
### Kubenet Networking Custom VNET

If you're not using Azure CNI (e.g., `"networkPolicy": "nono"` in the `kubernetesConfig` api model configuration object): After a custom VNET-configured cluster finishes provisioning, fetch the id of the Route Table resource from `Microsoft.Network` provider in your new cluster's Resource Group.
Copy link
Contributor

Choose a reason for hiding this comment

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

typo here "nono"

@jackfrancis
Copy link
Member Author

@CecileRobertMichon Thanks! incorporated both of your comments.

@jackfrancis jackfrancis closed this Dec 6, 2017
@ghost ghost removed the in progress label Dec 6, 2017
@jackfrancis jackfrancis reopened this Dec 6, 2017
Copy link
Contributor

@CecileRobertMichon CecileRobertMichon 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

@jackfrancis jackfrancis merged commit cef6aac into Azure:master Dec 6, 2017
@jackfrancis jackfrancis deleted the azure-cni-on-by-default branch December 6, 2017 22:43
@ghost ghost removed the in progress label Dec 6, 2017
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.

2 participants