From c27f84473a91b74fc7597af8ac037e46f9fa0e42 Mon Sep 17 00:00:00 2001 From: Shawn Warren Date: Thu, 20 Feb 2025 08:15:29 -0600 Subject: [PATCH] feat: add subnet support to external networks --- api/v1beta1/openstackcluster_types.go | 4 +- api/v1beta1/types.go | 10 ++ .../openstackcluster_controller_test.go | 12 ++- .../loadbalancer/loadbalancer_test.go | 9 +- .../services/networking/floatingip_test.go | 12 ++- pkg/cloud/services/networking/network.go | 12 +-- pkg/cloud/services/networking/network_test.go | 92 ++++++++++++++----- 7 files changed, 110 insertions(+), 41 deletions(-) diff --git a/api/v1beta1/openstackcluster_types.go b/api/v1beta1/openstackcluster_types.go index 840c816181..20b3dd0d38 100644 --- a/api/v1beta1/openstackcluster_types.go +++ b/api/v1beta1/openstackcluster_types.go @@ -88,7 +88,7 @@ type OpenStackClusterSpec struct { // If ExternalNetwork is not defined and there are no external networks // the controller will proceed as though DisableExternalNetwork was set. // +optional - ExternalNetwork *NetworkParam `json:"externalNetwork,omitempty"` + ExternalNetwork *ExternalNetworkParam `json:"externalNetwork,omitempty"` // DisableExternalNetwork specifies whether or not to attempt to connect the cluster // to an external network. This allows for the creation of clusters when connecting @@ -208,7 +208,7 @@ type OpenStackClusterStatus struct { // ExternalNetwork contains information about the external network used for default ingress and egress traffic. // +optional - ExternalNetwork *NetworkStatus `json:"externalNetwork,omitempty"` + ExternalNetwork *NetworkStatusWithSubnets `json:"externalNetwork,omitempty"` // Router describes the default cluster router // +optional diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index 99bc29d906..fffc19be34 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -167,6 +167,16 @@ type NetworkParam struct { Filter *NetworkFilter `json:"filter,omitempty"` } +// ExternalNetworkParam specifies an OpenStack external network. It may be specified by either ID or Filter, but not both. +// +kubebuilder:validation:MinProperties:=1 +type ExternalNetworkParam struct { + // Network specifies an OpenStack external network. It may be specified by either ID or Filter, but not both. + Network NetworkParam `json:"network"` + // Subnets specifies an list of OpenStack external network subnet. Each may be specified by either ID or Filter, but not both. + // +optional + Subnets []SubnetParam `json:"subnets,omitempty"` +} + // NetworkFilter specifies a query to select an OpenStack network. At least one property must be set. // +kubebuilder:validation:MinProperties:=1 type NetworkFilter struct { diff --git a/controllers/openstackcluster_controller_test.go b/controllers/openstackcluster_controller_test.go index ef566682e6..6e0c29fe29 100644 --- a/controllers/openstackcluster_controller_test.go +++ b/controllers/openstackcluster_controller_test.go @@ -257,8 +257,10 @@ var _ = Describe("OpenStackCluster controller", func() { }, DisableAPIServerFloatingIP: ptr.To(true), APIServerFixedIP: ptr.To("10.0.0.1"), - ExternalNetwork: &infrav1.NetworkParam{ - ID: ptr.To(externalNetworkID), + ExternalNetwork: &infrav1.ExternalNetworkParam{ + Network: infrav1.NetworkParam{ + ID: ptr.To(externalNetworkID), + }, }, Network: &infrav1.NetworkParam{ ID: ptr.To(clusterNetworkID), @@ -327,8 +329,10 @@ var _ = Describe("OpenStackCluster controller", func() { }, DisableAPIServerFloatingIP: ptr.To(true), APIServerFixedIP: ptr.To("10.0.0.1"), - ExternalNetwork: &infrav1.NetworkParam{ - ID: ptr.To(externalNetworkID), + ExternalNetwork: &infrav1.ExternalNetworkParam{ + Network: infrav1.NetworkParam{ + ID: ptr.To(externalNetworkID), + }, }, Network: &infrav1.NetworkParam{ ID: ptr.To(clusterNetworkID), diff --git a/pkg/cloud/services/loadbalancer/loadbalancer_test.go b/pkg/cloud/services/loadbalancer/loadbalancer_test.go index b9470ce7a8..31759eeb7a 100644 --- a/pkg/cloud/services/loadbalancer/loadbalancer_test.go +++ b/pkg/cloud/services/loadbalancer/loadbalancer_test.go @@ -76,8 +76,13 @@ func Test_ReconcileLoadBalancer(t *testing.T) { }, }, Status: infrav1.OpenStackClusterStatus{ - ExternalNetwork: &infrav1.NetworkStatus{ - ID: "aaaaaaaa-bbbb-cccc-dddd-111111111111", + ExternalNetwork: &infrav1.NetworkStatusWithSubnets{ + NetworkStatus: infrav1.NetworkStatus{ + ID: "aaaaaaaa-bbbb-cccc-dddd-111111111111", + }, + Subnets: []infrav1.Subnet{ + {ID: "aaaaaaaa-bbbb-cccc-dddd-111111111111"}, + }, }, Network: &infrav1.NetworkStatusWithSubnets{ Subnets: []infrav1.Subnet{ diff --git a/pkg/cloud/services/networking/floatingip_test.go b/pkg/cloud/services/networking/floatingip_test.go index 96a125166c..e4bb16eccc 100644 --- a/pkg/cloud/services/networking/floatingip_test.go +++ b/pkg/cloud/services/networking/floatingip_test.go @@ -68,8 +68,16 @@ func Test_GetOrCreateFloatingIP(t *testing.T) { }, } openStackCluster := &infrav1.OpenStackCluster{Status: infrav1.OpenStackClusterStatus{ - ExternalNetwork: &infrav1.NetworkStatus{ - ID: "", + ExternalNetwork: &infrav1.NetworkStatusWithSubnets{ + NetworkStatus: infrav1.NetworkStatus{ + ID: "", + }, + Subnets: []infrav1.Subnet{ + { + ID: "", + CIDR: "", + }, + }, }, }} for _, tt := range tests { diff --git a/pkg/cloud/services/networking/network.go b/pkg/cloud/services/networking/network.go index 2dbafc3eb4..b896e024ea 100644 --- a/pkg/cloud/services/networking/network.go +++ b/pkg/cloud/services/networking/network.go @@ -78,17 +78,17 @@ func (s *Service) ReconcileExternalNetwork(openStackCluster *infrav1.OpenStackCl } } else { var err error - network, err = s.GetNetworkByParam(openStackCluster.Spec.ExternalNetwork, ExternalNetworksOnly) + network, err = s.GetNetworkByParam(&openStackCluster.Spec.ExternalNetwork.Network, ExternalNetworksOnly) if err != nil { return fmt.Errorf("failed to get external network: %w", err) } } - openStackCluster.Status.ExternalNetwork = &infrav1.NetworkStatus{ - ID: network.ID, - Name: network.Name, - Tags: network.Tags, - } + openStackCluster.Status.ExternalNetwork = &infrav1.NetworkStatusWithSubnets{} + openStackCluster.Status.ExternalNetwork.ID = network.ID + openStackCluster.Status.ExternalNetwork.Name = network.Name + openStackCluster.Status.ExternalNetwork.Tags = openStackCluster.Spec.Tags + s.scope.Logger().Info("External network found", "id", network.ID) return nil } diff --git a/pkg/cloud/services/networking/network_test.go b/pkg/cloud/services/networking/network_test.go index 200ffc30d8..aa3c2ebddc 100644 --- a/pkg/cloud/services/networking/network_test.go +++ b/pkg/cloud/services/networking/network_test.go @@ -208,6 +208,8 @@ func Test_ReconcileNetwork(t *testing.T) { func Test_ReconcileExternalNetwork(t *testing.T) { fakeNetworkID := "d08803fc-2fa5-4179-b9f7-8c43d0af2fe6" fakeNetworkname := "external-network" + fakeSubnetID := "d08803fc-2fa5-4179-b9d7-8c43d0af2fe6" + fakeCIDR := "10.0.0.0/24" // Use gomega to match the ListOptsBuilder argument getExternalNetwork := func(g Gomega, listOpts networks.ListOpts, ret []networks.Network) func(networks.ListOptsBuilder) ([]networks.Network, error) { @@ -233,8 +235,10 @@ func Test_ReconcileExternalNetwork(t *testing.T) { name: "reconcile external network by ID", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - ExternalNetwork: &infrav1.NetworkParam{ - ID: ptr.To(fakeNetworkID), + ExternalNetwork: &infrav1.ExternalNetworkParam{ + Network: infrav1.NetworkParam{ + ID: ptr.To(fakeNetworkID), + }, }, }, }, @@ -246,14 +250,24 @@ func Test_ReconcileExternalNetwork(t *testing.T) { }, want: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - ExternalNetwork: &infrav1.NetworkParam{ - ID: ptr.To(fakeNetworkID), + ExternalNetwork: &infrav1.ExternalNetworkParam{ + Network: infrav1.NetworkParam{ + ID: ptr.To(fakeNetworkID), + }, }, }, Status: infrav1.OpenStackClusterStatus{ - ExternalNetwork: &infrav1.NetworkStatus{ - ID: fakeNetworkID, - Name: fakeNetworkname, + ExternalNetwork: &infrav1.NetworkStatusWithSubnets{ + NetworkStatus: infrav1.NetworkStatus{ + ID: fakeNetworkID, + Name: fakeNetworkname, + }, + Subnets: []infrav1.Subnet{ + { + ID: fakeSubnetID, + CIDR: fakeCIDR, + }, + }, }, }, }, @@ -263,8 +277,10 @@ func Test_ReconcileExternalNetwork(t *testing.T) { name: "reconcile external network by name", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - ExternalNetwork: &infrav1.NetworkParam{ - Filter: &infrav1.NetworkFilter{Name: fakeNetworkname}, + ExternalNetwork: &infrav1.ExternalNetworkParam{ + Network: infrav1.NetworkParam{ + Filter: &infrav1.NetworkFilter{Name: fakeNetworkname}, + }, }, }, }, @@ -279,14 +295,24 @@ func Test_ReconcileExternalNetwork(t *testing.T) { }, want: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - ExternalNetwork: &infrav1.NetworkParam{ - Filter: &infrav1.NetworkFilter{Name: fakeNetworkname}, + ExternalNetwork: &infrav1.ExternalNetworkParam{ + Network: infrav1.NetworkParam{ + Filter: &infrav1.NetworkFilter{Name: fakeNetworkname}, + }, }, }, Status: infrav1.OpenStackClusterStatus{ - ExternalNetwork: &infrav1.NetworkStatus{ - ID: fakeNetworkID, - Name: fakeNetworkname, + ExternalNetwork: &infrav1.NetworkStatusWithSubnets{ + NetworkStatus: infrav1.NetworkStatus{ + ID: fakeNetworkID, + Name: fakeNetworkname, + }, + Subnets: []infrav1.Subnet{ + { + ID: fakeSubnetID, + CIDR: fakeCIDR, + }, + }, }, }, }, @@ -296,8 +322,10 @@ func Test_ReconcileExternalNetwork(t *testing.T) { name: "reconcile external network by ID when external network by id not found", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - ExternalNetwork: &infrav1.NetworkParam{ - ID: ptr.To(fakeNetworkID), + ExternalNetwork: &infrav1.ExternalNetworkParam{ + Network: infrav1.NetworkParam{ + ID: ptr.To(fakeNetworkID), + }, }, }, }, @@ -306,8 +334,10 @@ func Test_ReconcileExternalNetwork(t *testing.T) { }, want: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - ExternalNetwork: &infrav1.NetworkParam{ - ID: ptr.To(fakeNetworkID), + ExternalNetwork: &infrav1.ExternalNetworkParam{ + Network: infrav1.NetworkParam{ + ID: ptr.To(fakeNetworkID), + }, }, }, }, @@ -317,8 +347,10 @@ func Test_ReconcileExternalNetwork(t *testing.T) { name: "reconcile external network by ID when external network by name not found", openStackCluster: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - ExternalNetwork: &infrav1.NetworkParam{ - Filter: &infrav1.NetworkFilter{Name: fakeNetworkname}, + ExternalNetwork: &infrav1.ExternalNetworkParam{ + Network: infrav1.NetworkParam{ + Filter: &infrav1.NetworkFilter{Name: fakeNetworkname}, + }, }, }, }, @@ -328,8 +360,10 @@ func Test_ReconcileExternalNetwork(t *testing.T) { }, want: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ - ExternalNetwork: &infrav1.NetworkParam{ - Filter: &infrav1.NetworkFilter{Name: fakeNetworkname}, + ExternalNetwork: &infrav1.ExternalNetworkParam{ + Network: infrav1.NetworkParam{ + Filter: &infrav1.NetworkFilter{Name: fakeNetworkname}, + }, }, }, }, @@ -387,9 +421,17 @@ func Test_ReconcileExternalNetwork(t *testing.T) { want: &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{}, Status: infrav1.OpenStackClusterStatus{ - ExternalNetwork: &infrav1.NetworkStatus{ - ID: fakeNetworkID, - Name: fakeNetworkname, + ExternalNetwork: &infrav1.NetworkStatusWithSubnets{ + NetworkStatus: infrav1.NetworkStatus{ + ID: fakeNetworkID, + Name: fakeNetworkname, + }, + Subnets: []infrav1.Subnet{ + { + ID: fakeSubnetID, + CIDR: fakeCIDR, + }, + }, }, }, },