From d9080d1d1acba6ef9c710eabe4b47cb179d6b10d Mon Sep 17 00:00:00 2001 From: lubedacht <132355999+lubedacht@users.noreply.github.com> Date: Thu, 8 Aug 2024 11:44:59 +0200 Subject: [PATCH] :sparkles: allow setting dhcp for additional networks (#203) **What is the purpose of this pull request/Why do we need it?** Some people might want to have a custom DHCP server running and therefore would like to disable DHCP for additional networks. **Description of changes:** Additional networks can now disable DHCP. The default value is `true` **Checklist:** - [x] Unit Tests added - [x] Includes [emojis](https://github.com/kubernetes-sigs/kubebuilder-release-tools?tab=readme-ov-file#kubebuilder-project-versioning) --- api/v1alpha1/ionoscloudmachine_types.go | 7 +++- api/v1alpha1/ionoscloudmachine_types_test.go | 5 +++ api/v1alpha1/zz_generated.deepcopy.go | 13 ++++++- ...e.cluster.x-k8s.io_ionoscloudmachines.yaml | 11 ++++-- ...r.x-k8s.io_ionoscloudmachinetemplates.yaml | 11 ++++-- internal/service/cloud/server.go | 3 +- internal/service/cloud/server_test.go | 39 +++++++++++++++++++ 7 files changed, 79 insertions(+), 10 deletions(-) diff --git a/api/v1alpha1/ionoscloudmachine_types.go b/api/v1alpha1/ionoscloudmachine_types.go index 4adf7fba..730d1b1a 100644 --- a/api/v1alpha1/ionoscloudmachine_types.go +++ b/api/v1alpha1/ionoscloudmachine_types.go @@ -151,7 +151,6 @@ type IonosCloudMachineSpec struct { Disk *Volume `json:"disk"` // AdditionalNetworks defines the additional network configurations for the VM. - // NOTE(lubedacht): We currently only support networks with DHCP enabled. //+optional AdditionalNetworks Networks `json:"additionalNetworks,omitempty"` @@ -183,6 +182,12 @@ type Network struct { // This LAN will be excluded from the deletion process. //+kubebuilder:validation:Minimum=1 NetworkID int32 `json:"networkID"` + + // DHCP indicates whether DHCP is enabled for the LAN. + // The primary NIC will always have DHCP enabled. + //+kubebuilder:default=true + //+optional + DHCP *bool `json:"dhcp,omitempty"` } // Volume is the physical storage on the VM. diff --git a/api/v1alpha1/ionoscloudmachine_types_test.go b/api/v1alpha1/ionoscloudmachine_types_test.go index 8ee86382..94a5c833 100644 --- a/api/v1alpha1/ionoscloudmachine_types_test.go +++ b/api/v1alpha1/ionoscloudmachine_types_test.go @@ -354,6 +354,11 @@ var _ = Describe("IonosCloudMachine Tests", func() { m.Spec.AdditionalNetworks[0].NetworkID = -1 Expect(k8sClient.Create(context.Background(), m)).ToNot(Succeed()) }) + It("DHCP should default to true", func() { + m := defaultMachine() + Expect(k8sClient.Create(context.Background(), m)).To(Succeed()) + Expect(*m.Spec.AdditionalNetworks[0].DHCP).To(BeTrue()) + }) }) }) Context("FailoverIP", func() { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index b6978e0d..e40d4c9a 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -359,7 +359,9 @@ func (in *IonosCloudMachineSpec) DeepCopyInto(out *IonosCloudMachineSpec) { if in.AdditionalNetworks != nil { in, out := &in.AdditionalNetworks, &out.AdditionalNetworks *out = make(Networks, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.FailoverIP != nil { in, out := &in.FailoverIP, &out.FailoverIP @@ -561,6 +563,11 @@ func (in *NICInfo) DeepCopy() *NICInfo { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Network) DeepCopyInto(out *Network) { *out = *in + if in.DHCP != nil { + in, out := &in.DHCP, &out.DHCP + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Network. @@ -578,7 +585,9 @@ func (in Networks) DeepCopyInto(out *Networks) { { in := &in *out = make(Networks, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } } diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ionoscloudmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ionoscloudmachines.yaml index cb757a76..b8c60299 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_ionoscloudmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_ionoscloudmachines.yaml @@ -64,12 +64,17 @@ spec: description: IonosCloudMachineSpec defines the desired state of IonosCloudMachine. properties: additionalNetworks: - description: |- - AdditionalNetworks defines the additional network configurations for the VM. - NOTE(lubedacht): We currently only support networks with DHCP enabled. + description: AdditionalNetworks defines the additional network configurations + for the VM. items: description: Network contains the config for additional LANs. properties: + dhcp: + default: true + description: |- + DHCP indicates whether DHCP is enabled for the LAN. + The primary NIC will always have DHCP enabled. + type: boolean networkID: description: |- NetworkID represents an ID an existing LAN in the data center. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ionoscloudmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ionoscloudmachinetemplates.yaml index 98feb6bb..70a6047b 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_ionoscloudmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_ionoscloudmachinetemplates.yaml @@ -73,13 +73,18 @@ spec: description: Spec is the IonosCloudMachineSpec for the IonosCloudMachineTemplate. properties: additionalNetworks: - description: |- - AdditionalNetworks defines the additional network configurations for the VM. - NOTE(lubedacht): We currently only support networks with DHCP enabled. + description: AdditionalNetworks defines the additional network + configurations for the VM. items: description: Network contains the config for additional LANs. properties: + dhcp: + default: true + description: |- + DHCP indicates whether DHCP is enabled for the LAN. + The primary NIC will always have DHCP enabled. + type: boolean networkID: description: |- NetworkID represents an ID an existing LAN in the data center. diff --git a/internal/service/cloud/server.go b/internal/service/cloud/server.go index aa471894..075dd070 100644 --- a/internal/service/cloud/server.go +++ b/internal/service/cloud/server.go @@ -431,7 +431,8 @@ func (s *Service) buildServerEntities(ms *scope.Machine, params serverEntityPara for _, nic := range ms.IonosMachine.Spec.AdditionalNetworks { items = append(items, sdk.Nic{Properties: &sdk.NicProperties{ - Lan: &nic.NetworkID, + Lan: &nic.NetworkID, + Dhcp: nic.DHCP, }}) } diff --git a/internal/service/cloud/server_test.go b/internal/service/cloud/server_test.go index 8db3f89a..7799e4c7 100644 --- a/internal/service/cloud/server_test.go +++ b/internal/service/cloud/server_test.go @@ -150,6 +150,45 @@ func (s *serverSuite) TestReconcileServerRequestDoneStateAvailableTurnedOff() { s.True(requeue) } +func (s *serverSuite) TestReconcileServerAdditionalNetworks() { + s.machineScope.IonosMachine.Spec.AdditionalNetworks = []infrav1.Network{{ + NetworkID: 43, + DHCP: ptr.To(false), + }, { + NetworkID: 44, + DHCP: ptr.To(true), + }} + + s.prepareReconcileServerRequestTest() + s.mockGetServerCreationRequestCall().Return([]sdk.Request{}, nil) + s.mockListLANsCall().Return(&sdk.Lans{Items: &[]sdk.Lan{{ + Id: ptr.To(exampleLANID), + Properties: &sdk.LanProperties{ + Name: ptr.To(s.service.lanName(s.clusterScope.Cluster)), + Public: ptr.To(true), + }, + }}}, nil) + + properties, entities := s.defaultServerComponents() + *entities.Nics.Items = append(*entities.Nics.Items, sdk.Nic{ + Properties: &sdk.NicProperties{ + Lan: ptr.To[int32](43), + Dhcp: ptr.To(false), + }, + }, sdk.Nic{ + Properties: &sdk.NicProperties{ + Lan: ptr.To[int32](44), + Dhcp: ptr.To(true), + }, + }) + s.mockCreateServerCall(properties, entities).Return(&sdk.Server{Id: ptr.To("12345")}, "location/to/server", nil) + + requeue, err := s.service.ReconcileServer(s.ctx, s.machineScope) + s.Equal("ionos://12345", ptr.Deref(s.machineScope.IonosMachine.Spec.ProviderID, "")) + s.NoError(err) + s.True(requeue) +} + func (s *serverSuite) TestReconcileEnterpriseServerNoRequest() { s.prepareReconcileServerRequestTest() s.mockGetServerCreationRequestCall().Return([]sdk.Request{}, nil)