From 92dce2d3c502b6007205baadbb4d5f5663cbb496 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 5068c87b07..736aed287e 100644 --- a/api/v1alpha6/conversion.go +++ b/api/v1alpha6/conversion.go @@ -226,6 +226,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 { @@ -316,6 +321,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 aa96ad27ea..c766f86e9e 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -5551,6 +5551,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 cb48bc42f4..516a9c1da3 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml @@ -2986,6 +2986,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 a5f4c3121a..15c731b8e3 100644 --- a/docs/book/src/topics/crd-changes/v1alpha7-to-v1alpha8.md +++ b/docs/book/src/topics/crd-changes/v1alpha7-to-v1alpha8.md @@ -200,6 +200,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)) + }) + } +}