From 2523a61809839e6bf69694a1393d303ea1dbbc25 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 | 6 +- pkg/cloud/services/networking/network.go | 4 + pkg/cloud/services/networking/network_test.go | 259 ++++++++++++++++++ 10 files changed, 381 insertions(+), 1 deletion(-) diff --git a/api/v1alpha6/conversion.go b/api/v1alpha6/conversion.go index 177a2f0f8c..1603959e0f 100644 --- a/api/v1alpha6/conversion.go +++ b/api/v1alpha6/conversion.go @@ -195,6 +195,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 { @@ -275,6 +280,11 @@ var v1alpha8OpenStackClusterTemplateRestorer = conversion.RestorerFor[*infrav1.O }, restorev1alpha8Subnets, ), + "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 e6ed8f4c9e..1279c1d7b4 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 97763d7d4b..1d60a82f7a 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 1278f83639..727a3fdc6c 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 5be338cb7f..b24dac17ae 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 @@ -1129,6 +1144,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 31c214580b..05c8d64775 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -5466,6 +5466,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 b9c154dc7f..9e8868961e 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml @@ -2901,6 +2901,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 37df012a8c..1a4d6c0ab9 100644 --- a/docs/book/src/topics/crd-changes/v1alpha7-to-v1alpha8.md +++ b/docs/book/src/topics/crd-changes/v1alpha7-to-v1alpha8.md @@ -195,4 +195,8 @@ In v1alpha8, this will be automatically converted to: dnsNameservers: "10.0.0.123" ``` -Please note that currently `managedSubnets` can only hold one element. \ No newline at end of file +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. diff --git a/pkg/cloud/services/networking/network.go b/pkg/cloud/services/networking/network.go index 540dce22f4..b3becbb889 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 512cc53146..5bc37b6a2b 100644 --- a/pkg/cloud/services/networking/network_test.go +++ b/pkg/cloud/services/networking/network_test.go @@ -32,6 +32,7 @@ import ( 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) { @@ -423,6 +424,264 @@ 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)) + }) + } +} + func Test_ConvertOpenStackSubnetToCAPOSubnet(t *testing.T) { caposubnets := []infrav1.Subnet{ {