Support multiple IPv4 network on OpenstackCluster

[memorais]

I recently reached the max number of available IPs from a subnet. Then I added a new subnet inside the same network and started to see errors like that in reconciliation process of capo-controller-manager:

err="multiple IPv4 Subnet is not allowed on OpenstackCluster"

I could be able to provision new worker/controller nodes with the new subnet by modifying OpenstackMachineTemplate and setting the "ports" section with "fixedIPs" using the ID of the new subnet, but the problem with reconciliation on capo-controller-manager logs persists.

According to the documentation, it's possible to have two subnets max but this must be present on OpenstackCluster at the time of its creation and must be one IPv4 subnet and the other must be IPv6.

Why we can't have more than one IPv4 subnet on OpenstackCluster if we can add multiple IPv4 subnets inside Openstack?

It would be nice to support multiple IPv4 subnets transparently to have better handling on that IP exhaustion scenarios.

[bnallapeta]

@memorais Thanks for bringing this up.

The error comes from ValidateSubnets which checks for <= 2 subnets per cluster, and if 2 they must be IPv4 + IPv6. When spec.subnets is empty, the controller lists every subnet in the network on each reconcile.. so adding a second IPv4 subnet there trips the check forever, even though you never touched the spec. Your fixedIPs workaround fixes Machines but not the cluster controller, which does its own listing.

To be clear, I won't flag this as a bug, but rather a design decision. Neutron/Octavia obviously supports this but in CAPO, we grab subnets[0] in a few places (LB VIP, LB pool, default port IPs).

I think this would be a good addition to CAPO. I would imagine this would require API change.. perhaps adding a primarySubnet or something similar to ask the user to choose when there are more than 2 subnets in the same network.

We'd welcome contributions if you're up for it. Please submit a proposal to this so that we can discuss the API changes required before implementing the actual code.

[bnallapeta]

/help
/kind/feature

[k8s-ci-robot]

@bnallapeta:
This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

• Why are we solving this issue?
• To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
• How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

Details

In response to this:

/help
/kind/feature

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[bnallapeta]

/kind feature

[memorais]

@memorais Thanks for bringing this up.

The error comes from ValidateSubnets which checks for <= 2 subnets per cluster, and if 2 they must be IPv4 + IPv6. When spec.subnets is empty, the controller lists every subnet in the network on each reconcile.. so adding a second IPv4 subnet there trips the check forever, even though you never touched the spec. Your fixedIPs workaround fixes Machines but not the cluster controller, which does its own listing.

To be clear, I won't flag this as a bug, but rather a design decision. Neutron/Octavia obviously supports this but in CAPO, we grab subnets[0] in a few places (LB VIP, LB pool, default port IPs).

I think this would be a good addition to CAPO. I would imagine this would require API change.. perhaps adding a primarySubnet or something similar to ask the user to choose when there are more than 2 subnets in the same network.

We'd welcome contributions if you're up for it. Please submit a proposal to this so that we can discuss the API changes required before implementing the actual code.

Sure, I think I can work on a proposal. I will review the contribution guidelines to see how start this.

Thanks.

[Whisper40]

We have encountered the same need today.
We created a OpenstackCluster with a subnet defined. Now we want to use multiple IPV4 subnets so we have try to remove the field as specified, but this field is immutable..

Details

"Subnets specifies existing subnets to use if not ManagedSubnets are
specified. All subnets must be in the network specified by Network.
There can be zero, one, or two subnets. If no subnets are specified,
all subnets in Network will be used. If 2 subnets are specified, one
must be IPv4 and the other IPv6."

[memorais]

Why are we solving this issue?

• Openstack supports the use of multiple subnets inside one network. There is no particular reason on why CAPO should only support two subnets (one IPv4 and other IPv6), which is the currently implementation.
• Openstack admins can easily add new subnets without Kubernetes administrators knowledge and this will break reconciliation process.
• The exhaustion of IPv4 is something that happens and manage this scenario should be more straightforward.

To address this issue, are there any code changes? What needs to be done in the code?

• I propose an API change with the addition of a field called "primarySubnet" inside OpenstackClusterSpec (api/v1beta2/openstackcluster_types.go). This field defines which subnet (by ID or name) will be used as the default network for cluster-wide networking functions. This field should be required when multiple IPv4 are available, either explicitly listed or discovered automatically when subnet is empty. If only one IPv4 subnet is available, this field can be optional and will be used for backward compatibility.

• I propose an change on function ValidateSubnets (pkg/utils/controllers/controllers.go) to remove the restriction of two networks at max, replacing this with the use of "primarySubnet" when multiple IPv4 is available.

• I propose an change on service functions (pkg/cloud/services/loadbalancer/loadbalancer.go, pkg/cloud/services/compute/instance.go and pkg/cloud/services/loadbalancer/loadbalancer.go) to take "primarySubnet" in account.

What is the effect of change?

• Reconciliation process will not break if Openstack administrators include new subnets.
• IP exhaustion could be handle manually by the change of "primarySubnet" when the kubernetes administrator choose to do so.

[lentzi90]

Sounds good to me. I'll just note that the required or optional decision won't happen in the validating webhook. So in practice this field should be optional. For v1beta2 we could decide to make it required though, if we want. I think it would be unnecessary friction though.

When multiple subnets are used/detected, we need to surface this to the user. I do believe we already do that through conditions. So from this regard we are already covered.

[bnallapeta]

Looks good.
Yes, I think we should definitely keep it optional until and unless there's a strong request otherwise.