From ccfcb5996a21b1e8029f402747196a645ce6a49a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Dulko?= Date: Tue, 30 Jan 2024 18:25:15 +0100 Subject: [PATCH] Add support to set `allocation_pools` for subnet This commit adds API that allows users to set `allocations_pools` in the subnet created by CAPO. This allows the users to restrict the IP address ranges that will be allocated automatically by OpenStack when creating Machines. Users can utilize this to reserve addresses for VIPs (virtual IPs) or special nodes that will have predefined addresses and will be created later. --- api/v1alpha6/conversion.go | 10 + api/v1alpha6/conversion_test.go | 13 + api/v1alpha7/conversion_test.go | 13 + api/v1alpha8/types.go | 16 ++ api/v1alpha8/zz_generated.deepcopy.go | 20 ++ ...re.cluster.x-k8s.io_openstackclusters.yaml | 20 ++ ...er.x-k8s.io_openstackclustertemplates.yaml | 21 ++ .../crd-changes/v1alpha7-to-v1alpha8.md | 4 + pkg/cloud/services/networking/network.go | 4 + pkg/cloud/services/networking/network_test.go | 260 ++++++++++++++++++ 10 files changed, 381 insertions(+) diff --git a/api/v1alpha6/conversion.go b/api/v1alpha6/conversion.go index 21e6e4f325..6168353c23 100644 --- a/api/v1alpha6/conversion.go +++ b/api/v1alpha6/conversion.go @@ -232,6 +232,11 @@ var v1alpha8OpenStackClusterRestorer = conversion.RestorerFor[*infrav1.OpenStack }, restorev1alpha8ClusterStatus, ), + "managedSubnets": conversion.UnconditionalFieldRestorer( + func(c *infrav1.OpenStackCluster) *[]infrav1.SubnetSpec { + return &c.Spec.ManagedSubnets + }, + ), } func (r *OpenStackCluster) ConvertTo(dstRaw ctrlconversion.Hub) error { @@ -322,6 +327,11 @@ var v1alpha8OpenStackClusterTemplateRestorer = conversion.RestorerFor[*infrav1.O }, restorev1alpha8ManagedSecurityGroups, ), + "managedSubnets": conversion.UnconditionalFieldRestorer( + func(c *infrav1.OpenStackClusterTemplate) *[]infrav1.SubnetSpec { + return &c.Spec.Template.Spec.ManagedSubnets + }, + ), } func (r *OpenStackClusterTemplate) ConvertTo(dstRaw ctrlconversion.Hub) error { diff --git a/api/v1alpha6/conversion_test.go b/api/v1alpha6/conversion_test.go index 77f5c4de76..d457f48e06 100644 --- a/api/v1alpha6/conversion_test.go +++ b/api/v1alpha6/conversion_test.go @@ -142,6 +142,19 @@ func TestFuzzyConversion(t *testing.T) { spec.CIDR = c.RandString() } }, + + func(pool *infrav1.AllocationPool, c fuzz.Continue) { + c.FuzzNoCustom(pool) + + // Start and End are required properties, let's make sure both are set + for pool.Start == "" { + pool.Start = c.RandString() + } + + for pool.End == "" { + pool.End = c.RandString() + } + }, } } diff --git a/api/v1alpha7/conversion_test.go b/api/v1alpha7/conversion_test.go index f3c2338e70..7487eedaa0 100644 --- a/api/v1alpha7/conversion_test.go +++ b/api/v1alpha7/conversion_test.go @@ -110,6 +110,19 @@ func TestFuzzyConversion(t *testing.T) { spec.CIDR = c.RandString() } }, + + func(pool *infrav1.AllocationPool, c fuzz.Continue) { + c.FuzzNoCustom(pool) + + // Start and End are required properties, let's make sure both are set + for pool.Start == "" { + pool.Start = c.RandString() + } + + for pool.End == "" { + pool.End = c.RandString() + } + }, } } diff --git a/api/v1alpha8/types.go b/api/v1alpha8/types.go index 8c206d803d..f47a9ba6ca 100644 --- a/api/v1alpha8/types.go +++ b/api/v1alpha8/types.go @@ -92,9 +92,25 @@ type SubnetSpec struct { // This field is required when defining a subnet. // +required CIDR string `json:"cidr"` + // DNSNameservers holds a list of DNS server addresses that will be provided when creating // the subnet. These addresses need to have the same IP version as CIDR. DNSNameservers []string `json:"dnsNameservers,omitempty"` + + // AllocationPools is an array of AllocationPool objects that will be applied to OpenStack Subnet being created. + // If set, OpenStack will only allocate these IPs for Machines. It will still be possible to create ports from + // outside of these ranges manually. + AllocationPools []AllocationPool `json:"allocationPools,omitempty"` +} + +type AllocationPool struct { + // Start represents the start of the AllocationPool, that is the lowest IP of the pool. + // +required + Start string `json:"start"` + + // End represents the end of the AlloctionPool, that is the highest IP of the pool. + // +required + End string `json:"end"` } type PortOpts struct { diff --git a/api/v1alpha8/zz_generated.deepcopy.go b/api/v1alpha8/zz_generated.deepcopy.go index 2b52b02cd2..01aa5b7698 100644 --- a/api/v1alpha8/zz_generated.deepcopy.go +++ b/api/v1alpha8/zz_generated.deepcopy.go @@ -83,6 +83,21 @@ func (in *AddressPair) DeepCopy() *AddressPair { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AllocationPool) DeepCopyInto(out *AllocationPool) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AllocationPool. +func (in *AllocationPool) DeepCopy() *AllocationPool { + if in == nil { + return nil + } + out := new(AllocationPool) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Bastion) DeepCopyInto(out *Bastion) { *out = *in @@ -1248,6 +1263,11 @@ func (in *SubnetSpec) DeepCopyInto(out *SubnetSpec) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.AllocationPools != nil { + in, out := &in.AllocationPools, &out.AllocationPools + *out = make([]AllocationPool, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SubnetSpec. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml index bdf1340bbe..c8e8b86013 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -5537,6 +5537,26 @@ spec: subnet is supported. If you leave this empty, no network will be created. items: properties: + allocationPools: + description: |- + AllocationPools is an array of AllocationPool objects that will be applied to OpenStack Subnet being created. + If set, OpenStack will only allocate these IPs for Machines. It will still be possible to create ports from + outside of these ranges manually. + items: + properties: + end: + description: End represents the end of the AlloctionPool, + that is the highest IP of the pool. + type: string + start: + description: Start represents the start of the AllocationPool, + that is the lowest IP of the pool. + type: string + required: + - end + - start + type: object + type: array cidr: description: |- CIDR is representing the IP address range used to create the subnet, e.g. 10.0.0.0/24. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml index 5624a26977..629284730c 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml @@ -2972,6 +2972,27 @@ spec: subnet is supported. If you leave this empty, no network will be created. items: properties: + allocationPools: + description: |- + AllocationPools is an array of AllocationPool objects that will be applied to OpenStack Subnet being created. + If set, OpenStack will only allocate these IPs for Machines. It will still be possible to create ports from + outside of these ranges manually. + items: + properties: + end: + description: End represents the end of the AlloctionPool, + that is the highest IP of the pool. + type: string + start: + description: Start represents the start of the + AllocationPool, that is the lowest IP of the + pool. + type: string + required: + - end + - start + type: object + type: array cidr: description: |- CIDR is representing the IP address range used to create the subnet, e.g. 10.0.0.0/24. diff --git a/docs/book/src/topics/crd-changes/v1alpha7-to-v1alpha8.md b/docs/book/src/topics/crd-changes/v1alpha7-to-v1alpha8.md index 8b929c1ea2..8ea68de168 100644 --- a/docs/book/src/topics/crd-changes/v1alpha7-to-v1alpha8.md +++ b/docs/book/src/topics/crd-changes/v1alpha7-to-v1alpha8.md @@ -210,6 +210,10 @@ In v1alpha8, this will be automatically converted to: Please note that currently `managedSubnets` can only hold one element. +#### Addition of allocationPools + +In v1alpha8, an `AllocationPools` property is introduced to `OpenStackCluster.Spec.ManagedSubnets`. When specified, OpenStack subnet created by CAPO will have the given values set as the `allocation_pools` property. This allows users to make sure OpenStack will not allocate some IP ranges in the subnet automatically. If the subnet is precreated and configured, CAPO will ignore `AllocationPools` property. + #### Change to managedSecurityGroups The field `managedSecurityGroups` is now a pointer to a `ManagedSecurityGroups` object rather than a boolean. diff --git a/pkg/cloud/services/networking/network.go b/pkg/cloud/services/networking/network.go index 7629abdee8..18c79d9433 100644 --- a/pkg/cloud/services/networking/network.go +++ b/pkg/cloud/services/networking/network.go @@ -256,6 +256,10 @@ func (s *Service) createSubnet(openStackCluster *infrav1.OpenStackCluster, clust Description: names.GetDescription(clusterName), } + for _, pool := range openStackCluster.Spec.ManagedSubnets[0].AllocationPools { + opts.AllocationPools = append(opts.AllocationPools, subnets.AllocationPool{Start: pool.Start, End: pool.End}) + } + subnet, err := s.client.CreateSubnet(opts) if err != nil { record.Warnf(openStackCluster, "FailedCreateSubnet", "Failed to create subnet %s: %v", name, err) diff --git a/pkg/cloud/services/networking/network_test.go b/pkg/cloud/services/networking/network_test.go index 47ff27783a..f1c351fc8a 100644 --- a/pkg/cloud/services/networking/network_test.go +++ b/pkg/cloud/services/networking/network_test.go @@ -24,12 +24,14 @@ import ( "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" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha8" "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients/mock" "sigs.k8s.io/cluster-api-provider-openstack/pkg/scope" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/names" ) func Test_ReconcileNetwork(t *testing.T) { @@ -420,3 +422,261 @@ func Test_ReconcileExternalNetwork(t *testing.T) { }) } } + +func Test_ReconcileSubnet(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + clusterName := "test-cluster" + expectedSubnetName := getSubnetName(clusterName) + expectedSubnetDesc := names.GetDescription(clusterName) + fakeSubnetID := "d08803fc-2fa5-4179-b9d7-8c43d0af2fe6" + fakeCIDR := "10.0.0.0/24" + fakeNetworkID := "d08803fc-2fa5-4279-b9f7-8c45d0ff2fe6" + fakeDNS := "10.0.10.200" + + tests := []struct { + name string + openStackCluster *infrav1.OpenStackCluster + expect func(m *mock.MockNetworkClientMockRecorder) + want *infrav1.OpenStackClusterStatus + }{ + { + name: "ensures status set when reconciling an existing subnet", + openStackCluster: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ + ManagedSubnets: []infrav1.SubnetSpec{ + { + CIDR: fakeCIDR, + }, + }, + }, + Status: infrav1.OpenStackClusterStatus{ + Network: &infrav1.NetworkStatusWithSubnets{ + NetworkStatus: infrav1.NetworkStatus{ + ID: fakeNetworkID, + }, + }, + }, + }, + expect: func(m *mock.MockNetworkClientMockRecorder) { + m. + ListSubnet(subnets.ListOpts{NetworkID: fakeNetworkID, CIDR: fakeCIDR}). + Return([]subnets.Subnet{ + { + ID: fakeSubnetID, + Name: expectedSubnetName, + CIDR: fakeCIDR, + }, + }, nil) + }, + want: &infrav1.OpenStackClusterStatus{ + Network: &infrav1.NetworkStatusWithSubnets{ + NetworkStatus: infrav1.NetworkStatus{ + ID: fakeNetworkID, + }, + Subnets: []infrav1.Subnet{ + { + Name: expectedSubnetName, + ID: fakeSubnetID, + CIDR: fakeCIDR, + }, + }, + }, + }, + }, + { + name: "creation without any parameter", + openStackCluster: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ + ManagedSubnets: []infrav1.SubnetSpec{ + { + CIDR: fakeCIDR, + }, + }, + }, + Status: infrav1.OpenStackClusterStatus{ + Network: &infrav1.NetworkStatusWithSubnets{ + NetworkStatus: infrav1.NetworkStatus{ + ID: fakeNetworkID, + }, + }, + }, + }, + expect: func(m *mock.MockNetworkClientMockRecorder) { + m. + ListSubnet(subnets.ListOpts{NetworkID: fakeNetworkID, CIDR: fakeCIDR}). + Return([]subnets.Subnet{}, nil) + + m. + CreateSubnet(subnets.CreateOpts{ + NetworkID: fakeNetworkID, + Name: expectedSubnetName, + IPVersion: 4, + CIDR: fakeCIDR, + Description: expectedSubnetDesc, + }). + Return(&subnets.Subnet{ + ID: fakeSubnetID, + Name: expectedSubnetName, + CIDR: fakeCIDR, + }, nil) + }, + want: &infrav1.OpenStackClusterStatus{ + Network: &infrav1.NetworkStatusWithSubnets{ + NetworkStatus: infrav1.NetworkStatus{ + ID: fakeNetworkID, + }, + Subnets: []infrav1.Subnet{ + { + Name: expectedSubnetName, + ID: fakeSubnetID, + CIDR: fakeCIDR, + }, + }, + }, + }, + }, + { + name: "creation with DNSNameservers", + openStackCluster: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ + ManagedSubnets: []infrav1.SubnetSpec{ + { + CIDR: fakeCIDR, + DNSNameservers: []string{fakeDNS}, + }, + }, + }, + Status: infrav1.OpenStackClusterStatus{ + Network: &infrav1.NetworkStatusWithSubnets{ + NetworkStatus: infrav1.NetworkStatus{ + ID: fakeNetworkID, + }, + }, + }, + }, + expect: func(m *mock.MockNetworkClientMockRecorder) { + m. + ListSubnet(subnets.ListOpts{NetworkID: fakeNetworkID, CIDR: fakeCIDR}). + Return([]subnets.Subnet{}, nil) + + m. + CreateSubnet(subnets.CreateOpts{ + NetworkID: fakeNetworkID, + Name: expectedSubnetName, + IPVersion: 4, + CIDR: fakeCIDR, + Description: expectedSubnetDesc, + DNSNameservers: []string{fakeDNS}, + }). + Return(&subnets.Subnet{ + ID: fakeSubnetID, + Name: expectedSubnetName, + CIDR: fakeCIDR, + }, nil) + }, + want: &infrav1.OpenStackClusterStatus{ + Network: &infrav1.NetworkStatusWithSubnets{ + NetworkStatus: infrav1.NetworkStatus{ + ID: fakeNetworkID, + }, + Subnets: []infrav1.Subnet{ + { + Name: expectedSubnetName, + ID: fakeSubnetID, + CIDR: fakeCIDR, + }, + }, + }, + }, + }, + { + name: "creation with allocationPools", + openStackCluster: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ + ManagedSubnets: []infrav1.SubnetSpec{ + { + CIDR: fakeCIDR, + AllocationPools: []infrav1.AllocationPool{ + { + Start: "10.0.0.1", + End: "10.0.0.10", + }, + { + Start: "10.0.0.20", + End: "10.0.0.254", + }, + }, + }, + }, + }, + Status: infrav1.OpenStackClusterStatus{ + Network: &infrav1.NetworkStatusWithSubnets{ + NetworkStatus: infrav1.NetworkStatus{ + ID: fakeNetworkID, + }, + }, + }, + }, + expect: func(m *mock.MockNetworkClientMockRecorder) { + m. + ListSubnet(subnets.ListOpts{NetworkID: fakeNetworkID, CIDR: fakeCIDR}). + Return([]subnets.Subnet{}, nil) + + m. + CreateSubnet(subnets.CreateOpts{ + NetworkID: fakeNetworkID, + Name: expectedSubnetName, + IPVersion: 4, + CIDR: fakeCIDR, + Description: expectedSubnetDesc, + AllocationPools: []subnets.AllocationPool{ + { + Start: "10.0.0.1", + End: "10.0.0.10", + }, + { + Start: "10.0.0.20", + End: "10.0.0.254", + }, + }, + }). + Return(&subnets.Subnet{ + ID: fakeSubnetID, + Name: expectedSubnetName, + CIDR: fakeCIDR, + }, nil) + }, + want: &infrav1.OpenStackClusterStatus{ + Network: &infrav1.NetworkStatusWithSubnets{ + NetworkStatus: infrav1.NetworkStatus{ + ID: fakeNetworkID, + }, + Subnets: []infrav1.Subnet{ + { + Name: expectedSubnetName, + ID: fakeSubnetID, + CIDR: fakeCIDR, + }, + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + mockClient := mock.NewMockNetworkClient(mockCtrl) + tt.expect(mockClient.EXPECT()) + s := Service{ + client: mockClient, + scope: scope.NewMockScopeFactory(mockCtrl, "", logr.Discard()), + } + err := s.ReconcileSubnet(tt.openStackCluster, clusterName) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(tt.openStackCluster.Status).To(Equal(*tt.want)) + }) + } +}