Cluster-api-provider-azure: Allow multiple subnets of role "node"

[CecileRobertMichon]

/kind feature

Describe the solution you'd like
[A clear and concise description of what you want to happen.]
Related to https://github.com/kubernetes-sigs/cluster-api-provider-azure/pull/643 and https://github.com/kubernetes-sigs/cluster-api-provider-azure/issues/618.

Currently, CAPZ expects and only allows one subnet per "role", ie. one control plane subnet and one node subnet. In the future, there might be use cases where users need multiple subnets, one for each MachinePool for example.

The challenges/ current blockers for supporting this on the top of my head are:

1. Subnets are defined at Cluster level, there isn't currently a good way to map subnets to machines besides their Role, which only allows one grouping for all Machines that are not control planes.
2. The Subnets type is []*SubnetSpec, ie. a slice of pointers. This makes it difficult to handle operations on subnets where we need to "get" the subnet for a particular role. A slice of pointers is also not very go idiomatic and makes conversion difficult. See #618 for further discussion on how to change that type.
3. Currently, the AzureCluster reconciler only creates 2 subnets and will ignore any additional subnets. We need to change this to allow specifying more than 2 subnets without breaking the BYO vnet use case.

Anything else you would like to add:
[Miscellaneous information that will assist in solving the issue.]

Environment:

• cluster-api-provider-azure version:
• Kubernetes version: (use kubectl version):
• OS (e.g. from /etc/os-release):

[alexeldeib]

cloud provider doesn't care about subnets except for ILB I think, based on my investigation in #773. Machines/MachinePools should probably be able to target a specific subnet (especially for the BYO scenario).

We would then only use the subnets in the cluster spec as an indication of what to provision. We could even select all machines owned by this cluster and ensure the corresponding subnets exist, but I don't think inverting that control is a good idea.

If we let machines target subnets directly, we should probably also have defaulting behavior so users don't have to fully qualify it every time.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[CecileRobertMichon]

/remove-lifecycle stale

[rhockenbury]

Has there been any related updates on this issue? My use case requires provisioning worker pools in different subnets. For reference/context, the vsphere cluster api allows network settings to be defined in the machine template.

[CecileRobertMichon]

@rhockenbury not yet, this is planned for v1alpha4

/milestone v0.5.x

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten

[rhockenbury]

/remove-lifecycle rotten

Still interested in this.

[fiunchinho]

I'm planning on giving this one a shot.

I believe the first challenge highlighted by @CecileRobertMichon is the most important one:

Subnets are defined at Cluster level, there isn't currently a good way to map subnets to machines besides their Role, which only allows one grouping for all Machines that are not control planes.

We need a way to know which subnet to use when creating machines or vmss. Currently:

• for AzureMachines, we check whether or not the Machine has the label cluster.x-k8s.io/control-plane, in which case we use the control-plane subnet, otherwise we use the node subnet.
• for AzureMachinePools we always use the node subnet.

We could add a new field to AzureMachinePoolMachineTemplate and AzureMachineSpec called SubnetName which should reference a Subnet in AzureCluster.Spec.VnetSpec.Subnets with that name.

AzureMachinePoolMachineTemplate struct {
        // VMSize is the size of the Virtual Machine to build.
        // See https://docs.microsoft.com/en-us/rest/api/compute/virtualmachines/createorupdate#virtualmachinesizetypes
        VMSize string `json:"vmSize"`

        (truncated)...(truncated)

        // SpotVMOptions allows the ability to specify the Machine should use a Spot VM
        // +optional
        SpotVMOptions *infrav1.SpotVMOptions `json:"spotVMOptions,omitempty"`

        // SubnetName selects the Subnet where the VMSS will be placed
        // +optional
        SubnetName string
}

AzureMachineSpec struct {
    // ProviderID is the unique identifier as specified by the cloud provider.
    // +optional
    ProviderID *string `json:"providerID,omitempty"`

    VMSize string `json:"vmSize"`

    (truncated)...(truncated)

    // SecurityProfile specifies the Security profile settings for a virtual machine.
    // +optional
    SecurityProfile *SecurityProfile `json:"securityProfile,omitempty"`

    // SubnetName selects the Subnet where the virtual machine will be placed
    // +optional
    SubnetName string
}

What do you think?

I'm not sure what to do with the role field. We could keep it, and default to a random subnet with node role when SubnetName is not specified for a AzureMachine / AzureMachinePool.

[CecileRobertMichon]

We could add a new field to AzureMachinePoolMachineTemplate and AzureMachineSpec called SubnetName which should reference a Subnet in AzureCluster.Spec.VnetSpec.Subnets with that name.

SGTM

I'm not sure what to do with the role field. We could keep it, and default to a random subnet with node role when SubnetName is not specified for a AzureMachine / AzureMachinePool.

We could keep it for now to maintain the existing behavior for users who have existing templates using the role method. I'm not sure about defaulting to a random subnet... we could also require the subnetName to be set for new AzureMachines / AzureMachinePools when there is more than one node subnet defined to avoid ambiguity.

The thing that's unclear to me is what subnetName we need to provide to the Azure Cloud provider in the azure.json config. So far we've put the unique node subnet in there but I'm not sure what happens when we start having more than one subnet. Do we need to set a different subnet per config depending on what run it's on? What about on control plane nodes? @alexeldeib seemed to say this wasn't an issue above:

cloud provider doesn't care about subnets except for ILB I think, based on my investigation in #773. Machines/MachinePools should probably be able to target a specific subnet (especially for the BYO scenario).

[fiunchinho]

The thing that's unclear to me is what subnetName we need to provide to the Azure Cloud provider in the azure.json config. So far we've put the unique node subnet in there but I'm not sure what happens when we start having more than one subnet. Do we need to set a different subnet per config depending on what run it's on? What about on control plane nodes? @alexeldeib seemed to say this wasn't an issue above:

cloud provider doesn't care about subnets except for ILB I think, based on my investigation in #773. Machines/MachinePools should probably be able to target a specific subnet (especially for the BYO scenario).

That's actually what we currently do at our company: every node pool has a different cloud provider config file, depending on the node pool's subnet.

I believe the main differences in this file between node pools would be the primaryScaleSetName, subnet, securityGroupName and routeTableName fields. Do you foresee any issues with this approach?

[CecileRobertMichon]

/assign @fiunchinho