Merge pull request #2338 from simonostendorf/feat/reconcile-pre-existing-router

✨ feat: reconcile pre-existing router

Author: k8s-ci-robot

Diff for: controllers/openstackcluster_controller.go

@@ -695,10 +695,10 @@ func reconcileNetworkComponents(scope *scope.WithLogger, cluster *clusterv1.Clus
 }
 
 // reconcilePreExistingNetworkComponents reconciles the cluster network status when the cluster is
-// using pre-existing networks and subnets which are not provisioned by the
+// using pre-existing networks, subnets and router which are not provisioned by the
 // cluster controller.
 func reconcilePreExistingNetworkComponents(scope *scope.WithLogger, networkingService *networking.Service, openStackCluster *infrav1.OpenStackCluster) error {
-	scope.Logger().V(4).Info("No need to reconcile network, searching network and subnet instead")
+	scope.Logger().V(4).Info("No need to reconcile network, searching network, subnet and router instead")
 
 	if openStackCluster.Status.Network == nil {
 		openStackCluster.Status.Network = &infrav1.NetworkStatusWithSubnets{}
@@ -745,9 +745,33 @@ func reconcilePreExistingNetworkComponents(scope *scope.WithLogger, networkingSe
 		setClusterNetwork(openStackCluster, network)
 	}
 
+	if openStackCluster.Spec.Router != nil {
+		router, err := networkingService.GetRouterByParam(openStackCluster.Spec.Router)
+		if err != nil {
+			handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to find router: %w", err), false)
+			return fmt.Errorf("error fetching cluster router: %w", err)
+		}
+
+		scope.Logger().V(4).Info("Found pre-existing router", "id", router.ID, "name", router.Name)
+
+		routerIPs := []string{}
+		for _, ip := range router.GatewayInfo.ExternalFixedIPs {
+			routerIPs = append(routerIPs, ip.IPAddress)
+		}
+
+		openStackCluster.Status.Router = &infrav1.Router{
+			Name: router.Name,
+			ID:   router.ID,
+			Tags: router.Tags,
+			IPs:  routerIPs,
+		}
+	}
+
 	return nil
 }
 
+// reconcileProvisionedNetworkComponents reconciles the cluster network status when the cluster is
+// using networks, subnets and router provisioned by the cluster controller.
 func reconcileProvisionedNetworkComponents(networkingService *networking.Service, openStackCluster *infrav1.OpenStackCluster, clusterResourceName string) error {
 	err := networkingService.ReconcileNetwork(openStackCluster, clusterResourceName)
 	if err != nil {

Diff for: controllers/openstackcluster_controller_test.go

@@ -22,6 +22,7 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/layer3/routers"
 	"github.com/gophercloud/gophercloud/v2/openstack/networking/v2/networks"
 	"github.com/gophercloud/gophercloud/v2/openstack/networking/v2/subnets"
 	. "github.com/onsi/ginkgo/v2" //nolint:revive
@@ -38,6 +39,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
+	"sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/networking"
 	"sigs.k8s.io/cluster-api-provider-openstack/pkg/scope"
 )
 
@@ -431,6 +433,140 @@ var _ = Describe("OpenStackCluster controller", func() {
 		Expect(err).To(BeNil())
 		Expect(testCluster.Status.Network.ID).To(Equal(clusterNetworkID))
 	})
+
+	It("reconcile pre-existing network components by id", func() {
+		const clusterNetworkID = "6c90b532-7ba0-418a-a276-5ae55060b5b0"
+		const clusterSubnetID = "cad5a91a-36de-4388-823b-b0cc82cadfdc"
+		const clusterRouterID = "e2407c18-c4e7-4d3d-befa-8eec5d8756f2"
+
+		testCluster.SetName("pre-existing-network-components-by-id")
+		testCluster.Spec = infrav1.OpenStackClusterSpec{
+			Network: &infrav1.NetworkParam{
+				ID: ptr.To(clusterNetworkID),
+			},
+			Subnets: []infrav1.SubnetParam{
+				{
+					ID: ptr.To(clusterSubnetID),
+				},
+			},
+			ManagedSubnets: []infrav1.SubnetSpec{},
+			Router: &infrav1.RouterParam{
+				ID: ptr.To(clusterRouterID),
+			},
+		}
+		err := k8sClient.Create(ctx, testCluster)
+		Expect(err).To(BeNil())
+		err = k8sClient.Create(ctx, capiCluster)
+		Expect(err).To(BeNil())
+
+		log := GinkgoLogr
+		clientScope, err := mockScopeFactory.NewClientScopeFromObject(ctx, k8sClient, nil, log, testCluster)
+		Expect(err).To(BeNil())
+		scope := scope.NewWithLogger(clientScope, log)
+
+		networkClientRecorder := mockScopeFactory.NetworkClient.EXPECT()
+
+		networkClientRecorder.GetSubnet(clusterSubnetID).Return(&subnets.Subnet{
+			ID:        clusterSubnetID,
+			CIDR:      "192.168.0.0/24",
+			NetworkID: clusterNetworkID,
+		}, nil)
+
+		networkClientRecorder.GetNetwork(clusterNetworkID).Return(&networks.Network{
+			ID: clusterNetworkID,
+		}, nil)
+
+		networkClientRecorder.GetRouter(clusterRouterID).Return(&routers.Router{
+			ID: clusterRouterID,
+		}, nil)
+
+		networkingService, err := networking.NewService(scope)
+		Expect(err).To(BeNil())
+
+		err = reconcilePreExistingNetworkComponents(scope, networkingService, testCluster)
+		Expect(err).To(BeNil())
+		Expect(testCluster.Status.Network.ID).To(Equal(clusterNetworkID))
+		Expect(testCluster.Status.Network.Subnets[0].ID).To(Equal(clusterSubnetID))
+		Expect(testCluster.Status.Router.ID).To(Equal(clusterRouterID))
+	})
+
+	It("reconcile pre-existing network components by name", func() {
+		const clusterNetworkID = "6c90b532-7ba0-418a-a276-5ae55060b5b0"
+		const clusterNetworkName = "capo"
+		const clusterSubnetID = "cad5a91a-36de-4388-823b-b0cc82cadfdc"
+		const clusterSubnetName = "capo"
+		const clusterRouterID = "e2407c18-c4e7-4d3d-befa-8eec5d8756f2"
+		const clusterRouterName = "capo"
+
+		testCluster.SetName("pre-existing-network-components-by-id")
+		testCluster.Spec = infrav1.OpenStackClusterSpec{
+			Network: &infrav1.NetworkParam{
+				Filter: &infrav1.NetworkFilter{
+					Name: clusterNetworkName,
+				},
+			},
+			Subnets: []infrav1.SubnetParam{
+				{
+					Filter: &infrav1.SubnetFilter{
+						Name: clusterSubnetName,
+					},
+				},
+			},
+			ManagedSubnets: []infrav1.SubnetSpec{},
+			Router: &infrav1.RouterParam{
+				Filter: &infrav1.RouterFilter{
+					Name: clusterRouterName,
+				},
+			},
+		}
+		err := k8sClient.Create(ctx, testCluster)
+		Expect(err).To(BeNil())
+		err = k8sClient.Create(ctx, capiCluster)
+		Expect(err).To(BeNil())
+
+		log := GinkgoLogr
+		clientScope, err := mockScopeFactory.NewClientScopeFromObject(ctx, k8sClient, nil, log, testCluster)
+		Expect(err).To(BeNil())
+		scope := scope.NewWithLogger(clientScope, log)
+
+		networkClientRecorder := mockScopeFactory.NetworkClient.EXPECT()
+
+		networkClientRecorder.ListNetwork(networks.ListOpts{
+			Name: clusterNetworkName,
+		}).Return([]networks.Network{
+			{
+				ID: clusterNetworkID,
+			},
+		}, nil)
+
+		networkClientRecorder.ListSubnet(subnets.ListOpts{
+			Name:      clusterSubnetName,
+			NetworkID: clusterNetworkID,
+		}).Return([]subnets.Subnet{
+			{
+				ID:        clusterSubnetID,
+				CIDR:      "192.168.0.0/24",
+				NetworkID: clusterNetworkID,
+			},
+		}, nil)
+
+		networkClientRecorder.ListRouter(routers.ListOpts{
+			Name: clusterRouterName,
+		}).Return([]routers.Router{
+			{
+				ID: clusterRouterID,
+			},
+		}, nil)
+
+		networkingService, err := networking.NewService(scope)
+		Expect(err).To(BeNil())
+
+		err = reconcilePreExistingNetworkComponents(scope, networkingService, testCluster)
+		Expect(err).To(BeNil())
+		Expect(testCluster.Status.Network.ID).To(Equal(clusterNetworkID))
+		Expect(testCluster.Status.Network.Subnets[0].ID).To(Equal(clusterSubnetID))
+		Expect(testCluster.Status.Router.ID).To(Equal(clusterRouterID))
+	})
 })
 
 func createRequestFromOSCluster(openStackCluster *infrav1.OpenStackCluster) reconcile.Request {

Diff for: pkg/cloud/services/networking/router.go

@@ -157,10 +157,10 @@ func (s *Service) getExternallyManagedRouter(openStackCluster *infrav1.OpenStack
 	if openStackCluster.Status.Router != nil {
 		return s.client.GetRouter(openStackCluster.Status.Router.ID)
 	}
-	return s.getRouterByParam(openStackCluster.Spec.Router)
+	return s.GetRouterByParam(openStackCluster.Spec.Router)
 }
 
-func (s *Service) getRouterByParam(routerParam *infrav1.RouterParam) (*routers.Router, error) {
+func (s *Service) GetRouterByParam(routerParam *infrav1.RouterParam) (*routers.Router, error) {
 	if routerParam.ID != nil {
 		return s.client.GetRouter(*routerParam.ID)
 	}