Skip to content

Commit

Permalink
Modify OpenStackCluster.Spec.Network API
Browse files Browse the repository at this point in the history 
For the BYO scenario, when the `OpenStackCluster.Spec.Network`
is not specified the query to OpenStack would 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.
  • Loading branch information
@MaysaMacedo
MaysaMacedo committed Feb 15, 2024
1 parent 88dcecf commit 0efefe8
Show file tree
Hide file tree
Showing 5 changed files with 159 additions and 82 deletions.
114 changes: 85 additions & 29 deletions controllers/openstackcluster_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand All @@ -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])
Expand All @@ -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
}
63 changes: 63 additions & 0 deletions controllers/openstackcluster_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package controllers
import (
"context"
"fmt"
"reflect"
"testing"

"github.com/go-logr/logr"
"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
}
}
5 changes: 4 additions & 1 deletion docs/book/src/topics/crd-changes/v1alpha7-to-v1alpha8.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,4 +195,7 @@ In v1alpha8, this will be automatically converted to:
dnsNameservers: "10.0.0.123"
```

Please note that currently `managedSubnets` can only hold one element.
Please note that currently `managedSubnets` can only hold one element.
#### ⚠️ Change to network

In v1alpha8, when the `OpenStackCluster.Spec.Network` is not defined, the `Subnets` are now used to identify the `Network`.
17 changes: 7 additions & 10 deletions pkg/cloud/services/networking/network.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
42 changes: 0 additions & 42 deletions pkg/cloud/services/networking/network_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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)
}
}

0 comments on commit 0efefe8

Please sign in to comment.