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)) + }) + } +}