✨ Modify OpenStackCluster.Spec.Network API

controllers/openstackcluster_controller.go

@@ -23,6 +23,8 @@ import (
 	"reflect"
 	"time"
 
+	"github.com/gophercloud/gophercloud/openstack/networking/v2/networks"
+	"github.com/gophercloud/gophercloud/openstack/networking/v2/subnets"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
@@ -507,36 +509,41 @@ func reconcileNetworkComponents(scope scope.Scope, cluster *clusterv1.Cluster, o
 	if len(openStackCluster.Spec.ManagedSubnets) == 0 {
 		scope.Logger().V(4).Info("No need to reconcile network, searching network and subnet instead")
 
-		netOpts := openStackCluster.Spec.Network.ToListOpt()
-		networkList, err := networkingService.GetNetworksByFilter(&netOpts)
-		if err != nil {
-			handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to find network: %w", err))
-			return fmt.Errorf("failed to find network: %w", err)
-		}
-		if len(networkList) == 0 {
-			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
 
-		subnets, err := filterSubnets(networkingService, openStackCluster)
+		err := getCAPONetwork(openStackCluster, networkingService)
 		if err != nil {
 			return err
 		}
 
+		filteredSubnets, err := filterSubnets(networkingService, openStackCluster)
+		if err != nil {
+			return err
+		}
+
+		var subnets []infrav1.Subnet
+		for subnet := range filteredSubnets {
+			filterSubnet := &filteredSubnets[subnet]
+			subnets = append(subnets, infrav1.Subnet{
+				ID:   filterSubnet.ID,
+				Name: filterSubnet.Name,
+				CIDR: filterSubnet.CIDR,
+				Tags: filterSubnet.Tags,
+			})
+		}
+
 		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 to get the Network.
+		err = populateCAPONetworkFromSubnet(networkingService, filteredSubnets, openStackCluster)
+		if err != nil {
+			return err
+		}
 	} else if len(openStackCluster.Spec.ManagedSubnets) == 1 {
 		err := networkingService.ReconcileNetwork(openStackCluster, clusterName)
 		if err != nil {
@@ -687,18 +694,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) {
@@ -709,9 +721,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])
@@ -722,8 +731,55 @@ 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
+}
+
+// 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 populateCAPONetworkFromSubnet(networkingService *networking.Service, 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 %s from subnet %s does not match NetworkID %s from subnet %s", subnets[0].NetworkID, subnets[0].ID, subnets[1].NetworkID, subnets[1].ID)
+		}
+
+		network, err := networkingService.GetNetworkByID(subnets[0].NetworkID)
+		if err != nil {
+			return err
+		}
+		convertOpenStackNetworkToCAPONetwork(openStackCluster, network)
+	}
+	return nil
+}
+
+// getCAPONetwork gets a network based on a filter, if defined, and convert the network to the CAPO format.
+// It returns an error in case it failed to retrieve the network.
+func getCAPONetwork(openStackCluster *infrav1.OpenStackCluster, networkingService *networking.Service) error {
+	emptyNetwork := infrav1.NetworkFilter{}
+	if openStackCluster.Spec.Network != emptyNetwork {
+		netOpts := openStackCluster.Spec.Network.ToListOpt()
+		networkList, err := networkingService.GetNetworksByFilter(&netOpts)
+		if err != nil {
+			handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to find network: %w", err))
+			return fmt.Errorf("failed to find network: %w", err)
+		}
+		if len(networkList) == 0 {
+			handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to find any network"))
+			return fmt.Errorf("failed to find any network")
+		}
+		if len(networkList) == 1 {
+			convertOpenStackNetworkToCAPONetwork(openStackCluster, &networkList[0])
+		}
+	}
+	return nil
 }

controllers/openstackcluster_controller_test.go

@@ -19,6 +19,8 @@ package controllers
 import (
 	"context"
 	"fmt"
+	"reflect"
+	"testing"
 
 	"github.com/go-logr/logr"
 	"github.com/golang/mock/gomock"
@@ -544,6 +546,45 @@ 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 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()
+
+		// 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 {
@@ -554,3 +595,25 @@ func createRequestFromOSCluster(openStackCluster *infrav1.OpenStackCluster) reco
 		},
 	}
 }
+
+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)
+	}
+}

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

@@ -243,3 +243,7 @@ Now the user needs to request creation of the security group rules by using the
 
 Note that when upgrading from a previous version, the Calico CNI security group rules will be added automatically to
 allow backwards compatibility if `allowAllInClusterTraffic` is set to false.
+
+#### ⚠️ 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,14 +386,11 @@ func getNetworkName(clusterName string) string {
 	return fmt.Sprintf("%s-cluster-%s", networkPrefix, clusterName)
 }
 
-// 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 {
-	subnets = append(subnets, infrav1.Subnet{
-		ID:   filteredSubnet.ID,
-		Name: filteredSubnet.Name,
-		CIDR: filteredSubnet.CIDR,
-		Tags: filteredSubnet.Tags,
-	})
-	return subnets
+// GetNetworkByID retrieves network by the ID.
+func (s *Service) GetNetworkByID(networkID string) (*networks.Network, error) {
+	network, err := s.client.GetNetwork(networkID)
+	if err != nil {
+		return &networks.Network{}, err
+	}
+	return network, nil
 }

pkg/cloud/services/networking/network_test.go

@@ -17,15 +17,13 @@ limitations under the License.
 package networking
 
 import (
-	"reflect"
 	"testing"
 
 	"github.com/go-logr/logr"
 	"github.com/golang/mock/gomock"
 	"github.com/gophercloud/gophercloud"
 	"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/external"
 	"github.com/gophercloud/gophercloud/openstack/networking/v2/networks"
-	"github.com/gophercloud/gophercloud/openstack/networking/v2/subnets"
 	. "github.com/onsi/gomega"
 	"k8s.io/utils/pointer"
 
@@ -422,43 +420,3 @@ func Test_ReconcileExternalNetwork(t *testing.T) {
 		})
 	}
 }
-
-func Test_ConvertOpenStackSubnetToCAPOSubnet(t *testing.T) {
-	caposubnets := []infrav1.Subnet{
-		{
-			ID:   "subnet1",
-			Name: "subnet1",
-			CIDR: "10.0.0.0/24",
-			Tags: []string{"tag1", "tag2"},
-		},
-	}
-
-	filteredSubnet := &subnets.Subnet{
-		ID:   "subnet2",
-		Name: "subnet2",
-		CIDR: "192.168.0.0/24",
-		Tags: []string{"tag3", "tag4"},
-	}
-
-	s := Service{}
-	result := s.ConvertOpenStackSubnetToCAPOSubnet(caposubnets, filteredSubnet)
-
-	expected := []infrav1.Subnet{
-		{
-			ID:   "subnet1",
-			Name: "subnet1",
-			CIDR: "10.0.0.0/24",
-			Tags: []string{"tag1", "tag2"},
-		},
-		{
-			ID:   "subnet2",
-			Name: "subnet2",
-			CIDR: "192.168.0.0/24",
-			Tags: []string{"tag3", "tag4"},
-		},
-	}
-
-	if !reflect.DeepEqual(result, expected) {
-		t.Errorf("ConvertOpenStackSubnetToCAPOSubnet() = %v, want %v", result, expected)
-	}
-}