Update-ips script breaking on some providers with capi

[AlbinB97]

Experienced behaviour

If you set up a capi-cluster on a provider like Elastx, when you run the update-ips script it adds the individual nodes to network policies instead of the subnet. Which breaks the network policies for ingress-nginx.

This is because in the update-ips script it checks if a cluster installer is capi and the provider is openstack here. But while setting up a cluster with Elastx, the provider is elastx, not openstack, which means it will try to add the individual nodes instead of the subnet range, which will block traffic through the loadbalancer.

Expected behaviour

When I run update-ips script, I expect it to give me the correct ips/range needed for a functioning network policy, even if my provider is a selected provider like elastx, instead of a generic one like openstack.

Steps to reproduce

1. Set up capi cluster on elastx
2. Run update-ips
3. Watch it add node ips instead of subnet range
4. Apply to cluster
5. Watch loadbalancer not working

Welkin Apps Version

v0.49.1

Kubernetes Version

v1.33.6

Additional context

The solution could be tricky because we still want the solution to be cloud agnostic, and not have to specify our scripts to certain cloud providers and having to add special cases for all of them.

[Zash]

If we want our scripts to be cloud-agnostic, then perhaps this decision could be moved into configuration, i.e. config/providers/, either

• by having an explicit setting for this node network policy handling that can be set in both elastx and openstack config templates,
• or by splitting the provider setting into a generic (e.g. openstack) and specific (e.g. elastx) such as this node network policy lives in the former

In this case the setting would also need to be conditional on the installer, so perhaps a config template tree under installers like config/installer/$installer/provider/$generic/$specific.yaml ?

[simonklb]

Note that this has always been the case and that we usually manually configure the entire range. The addition of being able to grab the entire range for a very specific case has been added fairly recently.

Why this hasn't been as big of a problem before is that when the installer is Kubespray the nodes aren't replaced as often. Now that we are starting to use CAPI more the network policies that only specify individual IPs are starting to become more of an issue.

So while I agree with your assessment I would argue that this is a feature request rather than a bug. Not that it matters a lot.

I do hope this gets prioritized though. If not we'll likely prioritize this as part of GOTO time.

[AlbinB97]

Note that this has always been the case and that we usually manually configure the entire range. The addition of being able to grab the entire range for a very specific case has been added fairly recently.

Why this hasn't been as big of a problem before is that when the installer is Kubespray the nodes aren't replaced as often. Now that we are starting to use CAPI more the network policies that only specify individual IPs are starting to become more of an issue.

So while I agree with your assessment I would argue that this is a feature request rather than a bug. Not that it matters a lot.

I do hope this gets prioritized though. If not we'll likely prioritize this as part of GOTO time.

I see, I guess I agree that it is a feature request from that point of view. Only reason I would categorize it as a bug still in this case is because there's nothing telling you that this needs to be set manually, and the script proceeds to just do the normal method of adding individual nodes, which in this case is breaking the netpols for loadbalancer to ingress and if you're not aware that this is a problem, it's quite a hassle to figure out that you actually need to set this manually. (There might a note somewhere, but not that I have found 🤔 ). But I understand that the feature to grab the entire range was added fairly recently and have not been fully fledged. Either way, I think it's a good idea to do something about it to avoid someone else running into the same problem as me in the future 😅 Alternatively instead of adding the range manually, you can also just override the provider to openstack in config before running update-ips to get the correct functionality for this feature, for elastx in this example. But either way, I'd love to see that it just works out of the box. But I agree that we'll see if it gets prioritized, otherwise it's great if picked up on GOTO time.

[simonklb]

Note that this has always been the case and that we usually manually configure the entire range. The addition of being able to grab the entire range for a very specific case has been added fairly recently.
Why this hasn't been as big of a problem before is that when the installer is Kubespray the nodes aren't replaced as often. Now that we are starting to use CAPI more the network policies that only specify individual IPs are starting to become more of an issue.
So while I agree with your assessment I would argue that this is a feature request rather than a bug. Not that it matters a lot.
I do hope this gets prioritized though. If not we'll likely prioritize this as part of GOTO time.

I see, I guess I agree that it is a feature request from that point of view. Only reason I would categorize it as a bug still in this case is because there's nothing telling you that this needs to be set manually, and the script proceeds to just do the normal method of adding individual nodes, which in this case is breaking the netpols for loadbalancer to ingress and if you're not aware that this is a problem, it's quite a hassle to figure out that you actually need to set this manually. (There might a note somewhere, but not that I have found 🤔 ). But I understand that the feature to grab the entire range was added fairly recently and have not been fully fledged. Either way, I think it's a good idea to do something about it to avoid someone else running into the same problem as me in the future 😅 Alternatively instead of adding the range manually, you can also just override the provider to openstack in config before running update-ips to get the correct functionality for this feature, for elastx in this example. But either way, I'd love to see that it just works out of the box. But I agree that we'll see if it gets prioritized, otherwise it's great if picked up on GOTO time.

If you have verified that the same implementation that already exists for .global.ck8sCloudProvider == "openstack" works for .global.ck8sCloudProvider == "elastx" feel free to just add that small patch. 😄

Making it work for all providers might be more complicated, but let's not dwell on that if we can improve it for the Elastx case!

[AlbinB97]

Note that this has always been the case and that we usually manually configure the entire range. The addition of being able to grab the entire range for a very specific case has been added fairly recently.
Why this hasn't been as big of a problem before is that when the installer is Kubespray the nodes aren't replaced as often. Now that we are starting to use CAPI more the network policies that only specify individual IPs are starting to become more of an issue.
So while I agree with your assessment I would argue that this is a feature request rather than a bug. Not that it matters a lot.
I do hope this gets prioritized though. If not we'll likely prioritize this as part of GOTO time.

I see, I guess I agree that it is a feature request from that point of view. Only reason I would categorize it as a bug still in this case is because there's nothing telling you that this needs to be set manually, and the script proceeds to just do the normal method of adding individual nodes, which in this case is breaking the netpols for loadbalancer to ingress and if you're not aware that this is a problem, it's quite a hassle to figure out that you actually need to set this manually. (There might a note somewhere, but not that I have found 🤔 ). But I understand that the feature to grab the entire range was added fairly recently and have not been fully fledged. Either way, I think it's a good idea to do something about it to avoid someone else running into the same problem as me in the future 😅 Alternatively instead of adding the range manually, you can also just override the provider to openstack in config before running update-ips to get the correct functionality for this feature, for elastx in this example. But either way, I'd love to see that it just works out of the box. But I agree that we'll see if it gets prioritized, otherwise it's great if picked up on GOTO time.

If you have verified that the same implementation that already exists for .global.ck8sCloudProvider == "openstack" works for .global.ck8sCloudProvider == "elastx" feel free to just add that small patch. 😄

Making it work for all providers might be more complicated, but let's not dwell on that if we can improve it for the Elastx case!

Alright, I included a fix for it in v0.49.2 and v0.50.1 patch since I noticed while testing for these patches. This will only work for Elastx case though, might be the same problem with Safespring CAPI since it's openstack as well?

EDIT:

• Staging 0.49.2 #2909
• Staging 0.50.1 #2910