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 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.
  • Loading branch information
MaysaMacedo committed Jan 23, 2024
1 parent 05571b9 commit c48fa08
Show file tree
Hide file tree
Showing 5 changed files with 133 additions and 20 deletions.
48 changes: 29 additions & 19 deletions controllers/openstackcluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
networkingService.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 = networkingService.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 {
Expand Down Expand Up @@ -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) {
Expand All @@ -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])
Expand All @@ -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
}
50 changes: 50 additions & 0 deletions controllers/openstackcluster_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 6 additions & 1 deletion docs/book/src/topics/crd-changes/v1alpha7-to-v1alpha8.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
25 changes: 25 additions & 0 deletions pkg/cloud/services/networking/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (s *Service) 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
}
s.ConvertOpenStackNetworkToCAPONetwork(openStackCluster, network)
}
return nil
}
23 changes: 23 additions & 0 deletions pkg/cloud/services/networking/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -462,3 +462,26 @@ 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"},
}

s := Service{}
s.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)
}
}

0 comments on commit c48fa08

Please sign in to comment.