diff --git a/api/v1alpha6/conversion.go b/api/v1alpha6/conversion.go index d5ec80a861..0ddba2a083 100644 --- a/api/v1alpha6/conversion.go +++ b/api/v1alpha6/conversion.go @@ -195,6 +195,11 @@ var v1alpha8OpenStackClusterRestorer = conversion.RestorerFor[*infrav1.OpenStack }, restorev1alpha8ClusterStatus, ), + "nodeSubnets": conversion.UnconditionalFieldRestorer( + func(c *infrav1.OpenStackCluster) *[]infrav1.SubnetSpec { + return &c.Spec.NodeSubnets + }, + ), } func (r *OpenStackCluster) ConvertTo(dstRaw ctrlconversion.Hub) error { @@ -275,6 +280,11 @@ var v1alpha8OpenStackClusterTemplateRestorer = conversion.RestorerFor[*infrav1.O }, restorev1alpha8Subnets, ), + "nodeSubnets": conversion.UnconditionalFieldRestorer( + func(c *infrav1.OpenStackClusterTemplate) *[]infrav1.SubnetSpec { + return &c.Spec.Template.Spec.NodeSubnets + }, + ), } 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 cd22269348..a316699793 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 5889754012..260bbc786f 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -5495,6 +5495,26 @@ spec: 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 554a5be9d0..af51c3f7fc 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml @@ -2930,6 +2930,27 @@ spec: 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 df2b72e468..d4fa9b9dab 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 `nodeSubnets` can only hold one element. \ No newline at end of file +Please note that currently `nodeSubnets` can only hold one element. + +#### Addition of allocationPools + +In v1alpha8, an `AllocationPools` property is introduced to `OpenStackCluster.Spec.NodeSubnets`. 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. \ No newline at end of file diff --git a/pkg/cloud/services/networking/network.go b/pkg/cloud/services/networking/network.go index d79a916e12..cad4a30c75 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.NodeSubnets[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..29554e0b12 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,260 @@ 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.OpenStackCluster + }{ + { + name: "ensures status set when reconciling an existing subnet", + openStackCluster: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ + NodeSubnets: []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.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{}, + Status: infrav1.OpenStackClusterStatus{ + Network: &infrav1.NetworkStatusWithSubnets{ + Subnets: []infrav1.Subnet{ + { + Name: expectedSubnetName, + ID: fakeSubnetID, + CIDR: fakeCIDR, + }, + }, + }, + }, + }, + }, + { + name: "creation without any parameter", + openStackCluster: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ + NodeSubnets: []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, + }, nil) + }, + want: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{}, + Status: infrav1.OpenStackClusterStatus{ + Network: &infrav1.NetworkStatusWithSubnets{ + Subnets: []infrav1.Subnet{ + { + Name: expectedSubnetName, + ID: fakeSubnetID, + CIDR: fakeCIDR, + }, + }, + }, + }, + }, + }, + { + name: "creation with DNSNameservers", + openStackCluster: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ + NodeSubnets: []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, + }, nil) + }, + want: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{}, + Status: infrav1.OpenStackClusterStatus{ + Network: &infrav1.NetworkStatusWithSubnets{ + Subnets: []infrav1.Subnet{ + { + Name: expectedSubnetName, + ID: fakeSubnetID, + CIDR: fakeCIDR, + }, + }, + }, + }, + }, + }, + { + name: "creation with allocationPools", + openStackCluster: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ + NodeSubnets: []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, + }, nil) + }, + want: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{}, + Status: infrav1.OpenStackClusterStatus{ + Network: &infrav1.NetworkStatusWithSubnets{ + 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()) + }) + } +} + func Test_ConvertOpenStackSubnetToCAPOSubnet(t *testing.T) { caposubnets := []infrav1.Subnet{ {