Modify OpenStackCluster.Spec.Network API

For the BYO scenario, when the `OpenStackCluster.Spec.Network`
is not specified the query to OpenStack will return all the
Networks available in the cloud and fail the reconciliation.
To avoid this, if any Subnets were specified under
`OpenStackCluster.Spec.Subnets` this can be used to identify
which Network to use.

Author: MaysaMacedo

controllers/openstackcluster_controller.go

@@ -23,6 +23,7 @@ import (
 	"reflect"
 	"time"
 
+	"github.com/gophercloud/gophercloud/openstack/networking/v2/subnets"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/client-go/tools/record"
@@ -506,26 +507,33 @@ func reconcileNetworkComponents(scope scope.Scope, cluster *clusterv1.Cluster, o
 			handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to find any network"))
 			return fmt.Errorf("failed to find any network")
 		}
-		if len(networkList) > 1 {
-			handleUpdateOSCError(openStackCluster, fmt.Errorf("found multiple networks (result: %v)", networkList))
-			return fmt.Errorf("found multiple networks (result: %v)", networkList)
-		}
 		if openStackCluster.Status.Network == nil {
 			openStackCluster.Status.Network = &infrav1.NetworkStatusWithSubnets{}
 		}
-		openStackCluster.Status.Network.ID = networkList[0].ID
-		openStackCluster.Status.Network.Name = networkList[0].Name
-		openStackCluster.Status.Network.Tags = networkList[0].Tags
+		if len(networkList) == 1 {
+			networking.ConvertOpenStackNetworkToCAPONetwork(openStackCluster, &networkList[0])
+		}
 
-		subnets, err := filterSubnets(networkingService, openStackCluster)
+		filteredSubnets, err := filterSubnets(networkingService, openStackCluster)
 		if err != nil {
 			return err
 		}
 
+		var subnets []infrav1.Subnet
+		for subnet := range filteredSubnets {
+			subnets = networking.ConvertOpenStackSubnetToCAPOSubnet(subnets, &filteredSubnets[subnet])
+		}
+
 		if err := utils.ValidateSubnets(subnets); err != nil {
 			return err
 		}
 		openStackCluster.Status.Network.Subnets = subnets
+
+		// If network is not yet populated on the Status, use networkID defined in the filtered subnets.
+		err = networkingService.PopulateCAPONetworkFromSubnet(filteredSubnets, openStackCluster)
+		if err != nil {
+			return err
+		}
 	} else {
 		err := networkingService.ReconcileNetwork(openStackCluster, clusterName)
 		if err != nil {
@@ -674,18 +682,23 @@ func handleUpdateOSCError(openstackCluster *infrav1.OpenStackCluster, message er
 }
 
 // filterSubnets retrieves the subnets based on the Subnet filters specified on OpenstackCluster.
-func filterSubnets(networkingService *networking.Service, openStackCluster *infrav1.OpenStackCluster) ([]infrav1.Subnet, error) {
-	var subnets []infrav1.Subnet
+func filterSubnets(networkingService *networking.Service, openStackCluster *infrav1.OpenStackCluster) ([]subnets.Subnet, error) {
+	var filteredSubnets []subnets.Subnet
+	var err error
 	openStackClusterSubnets := openStackCluster.Spec.Subnets
-	if openStackCluster.Status.Network == nil {
-		return nil, nil
+	networkID := ""
+	if openStackCluster.Status.Network != nil {
+		networkID = openStackCluster.Status.Network.ID
 	}
-	networkID := openStackCluster.Status.Network.ID
+
 	if len(openStackClusterSubnets) == 0 {
+		if networkID == "" {
+			return nil, nil
+		}
 		empty := &infrav1.SubnetFilter{}
 		listOpt := empty.ToListOpt()
 		listOpt.NetworkID = networkID
-		filteredSubnets, err := networkingService.GetSubnetsByFilter(listOpt)
+		filteredSubnets, err = networkingService.GetSubnetsByFilter(listOpt)
 		if err != nil {
 			err = fmt.Errorf("failed to find subnets: %w", err)
 			if errors.Is(err, networking.ErrFilterMatch) {
@@ -696,9 +709,6 @@ func filterSubnets(networkingService *networking.Service, openStackCluster *infr
 		if len(filteredSubnets) > 2 {
 			return nil, fmt.Errorf("more than two subnets found in the Network. Specify the subnets in the OpenStackCluster.Spec instead")
 		}
-		for subnet := range filteredSubnets {
-			subnets = networkingService.ConvertOpenStackSubnetToCAPOSubnet(subnets, &filteredSubnets[subnet])
-		}
 	} else {
 		for subnet := range openStackClusterSubnets {
 			filteredSubnet, err := networkingService.GetNetworkSubnetByFilter(networkID, &openStackClusterSubnets[subnet])
@@ -709,8 +719,8 @@ func filterSubnets(networkingService *networking.Service, openStackCluster *infr
 				}
 				return nil, err
 			}
-			subnets = networkingService.ConvertOpenStackSubnetToCAPOSubnet(subnets, filteredSubnet)
+			filteredSubnets = append(filteredSubnets, *filteredSubnet)
 		}
 	}
-	return subnets, nil
+	return filteredSubnets, nil
 }

controllers/openstackcluster_controller_test.go

@@ -518,6 +518,56 @@ var _ = Describe("OpenStackCluster controller", func() {
 		Expect(err).To(BeNil())
 		Expect(len(testCluster.Status.Network.Subnets)).To(Equal(2))
 	})
+
+	It("should allow fetch network by subnet", func() {
+		const clusterNetworkID = "6c90b532-7ba0-418a-a276-5ae55060b5b0"
+		const additionalNetworkID = "e2407c18-c4e7-4d3d-befa-8eec5d8756f2"
+		const clusterSubnetID = "cad5a91a-36de-4388-823b-b0cc82cadfdc"
+
+		testCluster.SetName("subnet-filtering")
+		testCluster.Spec = infrav1.OpenStackClusterSpec{
+			DisableAPIServerFloatingIP: true,
+			APIServerFixedIP:           "10.0.0.1",
+			DisableExternalNetwork:     true,
+			Subnets: []infrav1.SubnetFilter{
+				{ID: clusterSubnetID},
+			},
+		}
+		err := k8sClient.Create(ctx, testCluster)
+		Expect(err).To(BeNil())
+		err = k8sClient.Create(ctx, capiCluster)
+		Expect(err).To(BeNil())
+		scope, err := mockScopeFactory.NewClientScopeFromCluster(ctx, k8sClient, testCluster, nil, logr.Discard())
+		Expect(err).To(BeNil())
+
+		networkClientRecorder := mockScopeFactory.NetworkClient.EXPECT()
+
+		// Fetch cluster network
+		networkClientRecorder.ListNetwork(&networks.ListOpts{}).Return([]networks.Network{
+			{
+				ID: clusterNetworkID,
+			},
+			{
+				ID: additionalNetworkID,
+			},
+		}, nil)
+
+		// Fetching cluster subnets should be filtered by cluster network id
+		networkClientRecorder.GetSubnet(clusterSubnetID).Return(&subnets.Subnet{
+			ID:        clusterSubnetID,
+			CIDR:      "192.168.0.0/24",
+			NetworkID: clusterNetworkID,
+		}, nil)
+
+		// Fetch cluster network using the NetworkID from the filtered Subnets
+		networkClientRecorder.GetNetwork(clusterNetworkID).Return(&networks.Network{
+			ID: clusterNetworkID,
+		}, nil)
+
+		err = reconcileNetworkComponents(scope, capiCluster, testCluster)
+		Expect(err).To(BeNil())
+		Expect(testCluster.Status.Network.ID).To(Equal(clusterNetworkID))
+	})
 })
 
 func createRequestFromOSCluster(openStackCluster *infrav1.OpenStackCluster) reconcile.Request {

docs/book/src/topics/crd-changes/v1alpha7-to-v1alpha8.md

@@ -176,4 +176,9 @@ In v1alpha8, this will be automatically converted to:
 
 `Subnets` allows specifications of maximum two `SubnetFilter` one being IPv4 and the other IPv6. Both subnets must be on the same network. Any filtered subnets will be added to `OpenStackCluster.Status.Network.Subnets`.
 
-When subnets are not specified on `OpenStackCluster` and only the network is, the network is used to identify the subnets to use. If more than two subnets exist in the network, the user must specify which ones to use by defining the `OpenStackCluster.Spec.Subnets` field.
+When subnets are not specified on `OpenStackCluster` and only the network is, the network is used to identify the subnets to use. If more than two subnets exist in the network, the user must specify which ones to use by defining the `OpenStackCluster.Spec.Subnets` field.
+
+
+#### ⚠️ Change to network
+
+In v1alpha8, when the `OpenStackCluster.Spec.Network` is not defined, the `Subnets` are now used to identify the `Network`.

pkg/cloud/services/networking/network.go

@@ -386,7 +386,7 @@ func getNetworkName(clusterName string) string {
 
 // ConvertOpenStackSubnetToCAPOSubnet converts an OpenStack subnet to a capo subnet and adds to a slice.
 // It returns the slice with the converted subnet.
-func (s *Service) ConvertOpenStackSubnetToCAPOSubnet(subnets []infrav1.Subnet, filteredSubnet *subnets.Subnet) []infrav1.Subnet {
+func ConvertOpenStackSubnetToCAPOSubnet(subnets []infrav1.Subnet, filteredSubnet *subnets.Subnet) []infrav1.Subnet {
 	subnets = append(subnets, infrav1.Subnet{
 		ID:   filteredSubnet.ID,
 		Name: filteredSubnet.Name,
@@ -395,3 +395,28 @@ func (s *Service) ConvertOpenStackSubnetToCAPOSubnet(subnets []infrav1.Subnet, f
 	})
 	return subnets
 }
+
+// ConvertOpenStackNetworkToCAPONetwork converts an OpenStack network to a capo network.
+// It returns the converted subnet.
+func ConvertOpenStackNetworkToCAPONetwork(openStackCluster *infrav1.OpenStackCluster, network *networks.Network) {
+	openStackCluster.Status.Network.ID = network.ID
+	openStackCluster.Status.Network.Name = network.Name
+	openStackCluster.Status.Network.Tags = network.Tags
+}
+
+// PopulateCAPONetworkFromSubnet gets a network based on the networkID of the subnets and converts it to the CAPO format.
+// It returns an error in case it failed to retrieve the network.
+func (s *Service) PopulateCAPONetworkFromSubnet(subnets []subnets.Subnet, openStackCluster *infrav1.OpenStackCluster) error {
+	if openStackCluster.Status.Network.ID == "" && len(subnets) > 0 {
+		if len(subnets) > 1 && subnets[0].NetworkID != subnets[1].NetworkID {
+			return fmt.Errorf("unable to identify the Network to use. NetworkID from Subnets differ: %s, %s", subnets[0].NetworkID, subnets[1].NetworkID)
+		}
+
+		network, err := s.client.GetNetwork(subnets[0].NetworkID)
+		if err != nil {
+			return err
+		}
+		ConvertOpenStackNetworkToCAPONetwork(openStackCluster, network)
+	}
+	return nil
+}

pkg/cloud/services/networking/network_test.go

@@ -440,8 +440,7 @@ func Test_ConvertOpenStackSubnetToCAPOSubnet(t *testing.T) {
 		Tags: []string{"tag3", "tag4"},
 	}
 
-	s := Service{}
-	result := s.ConvertOpenStackSubnetToCAPOSubnet(caposubnets, filteredSubnet)
+	result := ConvertOpenStackSubnetToCAPOSubnet(caposubnets, filteredSubnet)
 
 	expected := []infrav1.Subnet{
 		{
@@ -462,3 +461,25 @@ func Test_ConvertOpenStackSubnetToCAPOSubnet(t *testing.T) {
 		t.Errorf("ConvertOpenStackSubnetToCAPOSubnet() = %v, want %v", result, expected)
 	}
 }
+
+func Test_ConvertOpenStackNetworkToCAPONetwork(t *testing.T) {
+	openStackCluster := &infrav1.OpenStackCluster{}
+	openStackCluster.Status.Network = &infrav1.NetworkStatusWithSubnets{}
+
+	filterednetwork := &networks.Network{
+		ID:   "network1",
+		Name: "network1",
+		Tags: []string{"tag1", "tag2"},
+	}
+
+	ConvertOpenStackNetworkToCAPONetwork(openStackCluster, filterednetwork)
+	expected := infrav1.NetworkStatus{
+		ID:   "network1",
+		Name: "network1",
+		Tags: []string{"tag1", "tag2"},
+	}
+
+	if !reflect.DeepEqual(openStackCluster.Status.Network.NetworkStatus, expected) {
+		t.Errorf("ConvertOpenStackNetworkToCAPONetwork() = %v, want %v", openStackCluster.Status.Network.NetworkStatus, expected)
+	}
+}