From ebe242e86c6b2af2888c18c4b1a34c59d8dfb4c6 Mon Sep 17 00:00:00 2001 From: Mohamed Chiheb Ben Jemaa Date: Mon, 8 Jan 2024 17:08:31 +0100 Subject: [PATCH] Implement DHCP for machine network --- api/v1alpha1/helpers.go | 141 ++++++++++++++ api/v1alpha1/proxmoxcluster_types.go | 173 +++--------------- api/v1alpha1/proxmoxcluster_types_test.go | 44 ++--- api/v1alpha1/proxmoxmachine_types.go | 14 +- api/v1alpha1/proxmoxmachine_types_test.go | 14 +- api/v1alpha1/zz_generated.deepcopy.go | 73 ++++++-- ...ture.cluster.x-k8s.io_proxmoxclusters.yaml | 28 ++- ...ture.cluster.x-k8s.io_proxmoxmachines.yaml | 27 ++- ...ster.x-k8s.io_proxmoxmachinetemplates.yaml | 28 ++- .../controller/proxmoxcluster_controller.go | 32 +++- .../proxmoxcluster_controller_test.go | 20 +- internal/service/vmservice/bootstrap.go | 76 ++++++-- internal/service/vmservice/bootstrap_test.go | 75 ++++---- internal/service/vmservice/helpers_test.go | 40 ++-- internal/service/vmservice/ip.go | 134 ++++++++++---- internal/service/vmservice/ip_test.go | 71 ++++--- internal/service/vmservice/utils_test.go | 6 +- internal/service/vmservice/vm.go | 77 ++++++-- internal/service/vmservice/vm_test.go | 55 +++--- .../webhook/proxmoxcluster_webhook_test.go | 19 +- pkg/kubernetes/ipam/ipam.go | 4 +- pkg/kubernetes/ipam/ipam_test.go | 16 +- pkg/proxmox/client.go | 2 + pkg/proxmox/goproxmox/api_client.go | 10 + pkg/proxmox/proxmoxtest/mock_client.go | 56 ++++++ 25 files changed, 813 insertions(+), 422 deletions(-) create mode 100644 api/v1alpha1/helpers.go diff --git a/api/v1alpha1/helpers.go b/api/v1alpha1/helpers.go new file mode 100644 index 00000000..76e2f0da --- /dev/null +++ b/api/v1alpha1/helpers.go @@ -0,0 +1,141 @@ +package v1alpha1 + +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// SetInClusterIPPoolRef will set the reference to the provided InClusterIPPool. +// If nil was provided, the status field will be cleared. +func (c *ProxmoxCluster) SetInClusterIPPoolRef(pool metav1.Object) { + if pool == nil || pool.GetName() == "" { + c.Status.InClusterIPPoolRef = nil + return + } + + if c.Status.InClusterIPPoolRef == nil { + c.Status.InClusterIPPoolRef = []corev1.LocalObjectReference{ + {Name: pool.GetName()}, + } + } + + found := false + for _, ref := range c.Status.InClusterIPPoolRef { + if ref.Name == pool.GetName() { + found = true + } + } + if !found { + c.Status.InClusterIPPoolRef = append(c.Status.InClusterIPPoolRef, corev1.LocalObjectReference{Name: pool.GetName()}) + } +} + +// AddNodeLocation will add a node location to either the control plane or worker +// node locations based on the `isControlPlane` parameter. +func (c *ProxmoxCluster) AddNodeLocation(loc NodeLocation, isControlPlane bool) { + if c.Status.NodeLocations == nil { + c.Status.NodeLocations = new(NodeLocations) + } + + if !c.HasMachine(loc.Machine.Name, isControlPlane) { + c.addNodeLocation(loc, isControlPlane) + } +} + +// RemoveNodeLocation removes a node location from the status. +func (c *ProxmoxCluster) RemoveNodeLocation(machineName string, isControlPlane bool) { + nodeLocations := c.Status.NodeLocations + + if nodeLocations == nil { + return + } + + if !c.HasMachine(machineName, isControlPlane) { + return + } + + if isControlPlane { + for i, v := range nodeLocations.ControlPlane { + if v.Machine.Name == machineName { + nodeLocations.ControlPlane = append(nodeLocations.ControlPlane[:i], nodeLocations.ControlPlane[i+1:]...) + } + } + return + } + + for i, v := range nodeLocations.Workers { + if v.Machine.Name == machineName { + nodeLocations.Workers = append(nodeLocations.Workers[:i], nodeLocations.Workers[i+1:]...) + } + } +} + +// UpdateNodeLocation will update the node location based on the provided machine name. +// If the node location does not exist, it will be added. +// +// The function returns true if the value was added or updated, otherwise false. +func (c *ProxmoxCluster) UpdateNodeLocation(machineName, node string, isControlPlane bool) bool { + if !c.HasMachine(machineName, isControlPlane) { + loc := NodeLocation{ + Node: node, + Machine: corev1.LocalObjectReference{Name: machineName}, + } + c.AddNodeLocation(loc, isControlPlane) + return true + } + + locations := c.Status.NodeLocations.Workers + if isControlPlane { + locations = c.Status.NodeLocations.ControlPlane + } + + for i, loc := range locations { + if loc.Machine.Name == machineName { + if loc.Node != node { + locations[i].Node = node + return true + } + + return false + } + } + + return false +} + +// HasMachine returns if true if a machine was found on any node. +func (c *ProxmoxCluster) HasMachine(machineName string, isControlPlane bool) bool { + return c.GetNode(machineName, isControlPlane) != "" +} + +// GetNode tries to return the Proxmox node for the provided machine name. +func (c *ProxmoxCluster) GetNode(machineName string, isControlPlane bool) string { + if c.Status.NodeLocations == nil { + return "" + } + + if isControlPlane { + for _, cpl := range c.Status.NodeLocations.ControlPlane { + if cpl.Machine.Name == machineName { + return cpl.Node + } + } + } else { + for _, wloc := range c.Status.NodeLocations.Workers { + if wloc.Machine.Name == machineName { + return wloc.Node + } + } + } + + return "" +} + +func (c *ProxmoxCluster) addNodeLocation(loc NodeLocation, isControlPlane bool) { + if isControlPlane { + c.Status.NodeLocations.ControlPlane = append(c.Status.NodeLocations.ControlPlane, loc) + return + } + + c.Status.NodeLocations.Workers = append(c.Status.NodeLocations.Workers, loc) +} diff --git a/api/v1alpha1/proxmoxcluster_types.go b/api/v1alpha1/proxmoxcluster_types.go index 1a9050e9..cf4e4df0 100644 --- a/api/v1alpha1/proxmoxcluster_types.go +++ b/api/v1alpha1/proxmoxcluster_types.go @@ -20,8 +20,6 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - - ipamicv1 "sigs.k8s.io/cluster-api-ipam-provider-in-cluster/api/v1alpha2" ) const ( @@ -44,25 +42,49 @@ type ProxmoxClusterSpec struct { // +optional AllowedNodes []string `json:"allowedNodes,omitempty"` + ClusterNetworkConfig `json:",inline"` +} + +// ClusterNetworkConfig represents information about the cluster network configuration. +type ClusterNetworkConfig struct { // IPv4Config contains information about available IPV4 address pools and the gateway. - // this can be combined with ipv6Config in order to enable dual stack. + // this can be combined with ipv6Config to enable dual stack. // either IPv4Config or IPv6Config must be provided. // +optional - // +kubebuilder:validation:XValidation:rule="self.addresses.size() > 0",message="IPv4Config addresses must be provided" - IPv4Config *ipamicv1.InClusterIPPoolSpec `json:"ipv4Config,omitempty"` + IPv4Config *IPConfig `json:"ipv4Config,omitempty"` // IPv6Config contains information about available IPV6 address pools and the gateway. - // this can be combined with ipv4Config in order to enable dual stack. + // this can be combined with ipv4Config to enable dual stack. // either IPv4Config or IPv6Config must be provided. // +optional - // +kubebuilder:validation:XValidation:rule="self.addresses.size() > 0",message="IPv6Config addresses must be provided" - IPv6Config *ipamicv1.InClusterIPPoolSpec `json:"ipv6Config,omitempty"` + IPv6Config *IPConfig `json:"ipv6Config,omitempty"` // DNSServers contains information about nameservers used by machines network-config. // +kubebuilder:validation:MinItems=1 DNSServers []string `json:"dnsServers"` } +// IPConfig contains information about available IP config. +type IPConfig struct { + // Addresses is a list of IP addresses that can be assigned. This set of + // addresses can be non-contiguous. + // +kubebuilder:validation:MinItems=1 + Addresses []string `json:"addresses,omitempty"` + + // Prefix is the network prefix to use. + // +kubebuilder:validation:Maximum=128 + // +optional + Prefix int `json:"prefix,omitempty"` + + // Gateway + // +optional + Gateway string `json:"gateway,omitempty"` + + // DHCP indicates if DHCP should be used to assign IP addresses. + // +optional + DHCP bool `json:"dhcp,omitempty"` +} + // ProxmoxClusterStatus defines the observed state of ProxmoxCluster. type ProxmoxClusterStatus struct { // Ready indicates that the cluster is ready. @@ -142,141 +164,6 @@ func (c *ProxmoxCluster) SetConditions(conditions clusterv1.Conditions) { c.Status.Conditions = conditions } -// SetInClusterIPPoolRef will set the reference to the provided InClusterIPPool. -// If nil was provided, the status field will be cleared. -func (c *ProxmoxCluster) SetInClusterIPPoolRef(pool *ipamicv1.InClusterIPPool) { - if pool == nil || pool.GetName() == "" { - c.Status.InClusterIPPoolRef = nil - return - } - - if c.Status.InClusterIPPoolRef == nil { - c.Status.InClusterIPPoolRef = []corev1.LocalObjectReference{ - {Name: pool.GetName()}, - } - } - - found := false - for _, ref := range c.Status.InClusterIPPoolRef { - if ref.Name == pool.GetName() { - found = true - } - } - if !found { - c.Status.InClusterIPPoolRef = append(c.Status.InClusterIPPoolRef, corev1.LocalObjectReference{Name: pool.GetName()}) - } -} - -// AddNodeLocation will add a node location to either the control plane or worker -// node locations based on the `isControlPlane` parameter. -func (c *ProxmoxCluster) AddNodeLocation(loc NodeLocation, isControlPlane bool) { - if c.Status.NodeLocations == nil { - c.Status.NodeLocations = new(NodeLocations) - } - - if !c.HasMachine(loc.Machine.Name, isControlPlane) { - c.addNodeLocation(loc, isControlPlane) - } -} - -// RemoveNodeLocation removes a node location from the status. -func (c *ProxmoxCluster) RemoveNodeLocation(machineName string, isControlPlane bool) { - nodeLocations := c.Status.NodeLocations - - if nodeLocations == nil { - return - } - - if !c.HasMachine(machineName, isControlPlane) { - return - } - - if isControlPlane { - for i, v := range nodeLocations.ControlPlane { - if v.Machine.Name == machineName { - nodeLocations.ControlPlane = append(nodeLocations.ControlPlane[:i], nodeLocations.ControlPlane[i+1:]...) - } - } - return - } - - for i, v := range nodeLocations.Workers { - if v.Machine.Name == machineName { - nodeLocations.Workers = append(nodeLocations.Workers[:i], nodeLocations.Workers[i+1:]...) - } - } -} - -// UpdateNodeLocation will update the node location based on the provided machine name. -// If the node location does not exist, it will be added. -// -// The function returns true if the value was added or updated, otherwise false. -func (c *ProxmoxCluster) UpdateNodeLocation(machineName, node string, isControlPlane bool) bool { - if !c.HasMachine(machineName, isControlPlane) { - loc := NodeLocation{ - Node: node, - Machine: corev1.LocalObjectReference{Name: machineName}, - } - c.AddNodeLocation(loc, isControlPlane) - return true - } - - locations := c.Status.NodeLocations.Workers - if isControlPlane { - locations = c.Status.NodeLocations.ControlPlane - } - - for i, loc := range locations { - if loc.Machine.Name == machineName { - if loc.Node != node { - locations[i].Node = node - return true - } - - return false - } - } - - return false -} - -// HasMachine returns if true if a machine was found on any node. -func (c *ProxmoxCluster) HasMachine(machineName string, isControlPlane bool) bool { - return c.GetNode(machineName, isControlPlane) != "" -} - -// GetNode tries to return the Proxmox node for the provided machine name. -func (c *ProxmoxCluster) GetNode(machineName string, isControlPlane bool) string { - if c.Status.NodeLocations == nil { - return "" - } - - if isControlPlane { - for _, cpl := range c.Status.NodeLocations.ControlPlane { - if cpl.Machine.Name == machineName { - return cpl.Node - } - } - } else { - for _, wloc := range c.Status.NodeLocations.Workers { - if wloc.Machine.Name == machineName { - return wloc.Node - } - } - } - - return "" -} - -func (c *ProxmoxCluster) addNodeLocation(loc NodeLocation, isControlPlane bool) { - if isControlPlane { - c.Status.NodeLocations.ControlPlane = append(c.Status.NodeLocations.ControlPlane, loc) - return - } - - c.Status.NodeLocations.Workers = append(c.Status.NodeLocations.Workers, loc) -} - func init() { SchemeBuilder.Register(&ProxmoxCluster{}, &ProxmoxClusterList{}) } diff --git a/api/v1alpha1/proxmoxcluster_types_test.go b/api/v1alpha1/proxmoxcluster_types_test.go index 7d4b314d..328c09a8 100644 --- a/api/v1alpha1/proxmoxcluster_types_test.go +++ b/api/v1alpha1/proxmoxcluster_types_test.go @@ -80,11 +80,13 @@ func defaultCluster() *ProxmoxCluster { Namespace: metav1.NamespaceDefault, }, Spec: ProxmoxClusterSpec{ - IPv4Config: &ipamicv1.InClusterIPPoolSpec{ - Addresses: []string{"10.0.0.0/24"}, - Prefix: 24, + ClusterNetworkConfig: ClusterNetworkConfig{ + IPv4Config: &IPConfig{ + Addresses: []string{"10.0.0.0/24"}, + Prefix: 24, + }, + DNSServers: []string{"1.2.3.4"}, }, - DNSServers: []string{"1.2.3.4"}, }, } } @@ -96,13 +98,6 @@ var _ = Describe("ProxmoxCluster Test", func() { }) Context("IPv4Config", func() { - It("Should not allow empty addresses", func() { - dc := defaultCluster() - dc.Spec.IPv4Config.Addresses = []string{} - - Expect(k8sClient.Create(context.Background(), dc)).Should(MatchError(ContainSubstring("IPv4Config addresses must be provided"))) - }) - It("Should not allow prefix higher than 128", func() { dc := defaultCluster() dc.Spec.IPv4Config.Prefix = 129 @@ -116,6 +111,13 @@ var _ = Describe("ProxmoxCluster Test", func() { dc.Spec.IPv4Config = nil Expect(k8sClient.Create(context.Background(), dc)).Should(MatchError(ContainSubstring("at least one ip config must be set"))) }) + + It("Should allow DHCP for IPv4 config", func() { + dc := defaultCluster() + dc.Spec.ClusterNetworkConfig.IPv4Config.DHCP = true + + Expect(k8sClient.Create(context.Background(), dc)).To(Succeed()) + }) }) It("Should not allow empty DNS servers", func() { @@ -130,19 +132,9 @@ var _ = Describe("ProxmoxCluster Test", func() { }) Context("IPV6Config", func() { - It("Should not allow empty addresses", func() { - dc := defaultCluster() - dc.Spec.IPv6Config = &ipamicv1.InClusterIPPoolSpec{ - Addresses: []string{}, - Prefix: 0, - Gateway: "", - } - Expect(k8sClient.Create(context.Background(), dc)).Should(MatchError(ContainSubstring("IPv6Config addresses must be provided"))) - }) - It("Should not allow prefix higher than 128", func() { dc := defaultCluster() - dc.Spec.IPv6Config = &ipamicv1.InClusterIPPoolSpec{ + dc.Spec.IPv6Config = &IPConfig{ Addresses: []string{}, Prefix: 129, Gateway: "", @@ -150,6 +142,14 @@ var _ = Describe("ProxmoxCluster Test", func() { Expect(k8sClient.Create(context.Background(), dc)).Should(MatchError(ContainSubstring("should be less than or equal to 128"))) }) + + It("Should allow DHCP for IPV6 config", func() { + dc := defaultCluster() + dc.Spec.IPv6Config = &IPConfig{ + DHCP: true, + } + Expect(k8sClient.Create(context.Background(), dc)).Should(Succeed()) + }) }) }) diff --git a/api/v1alpha1/proxmoxmachine_types.go b/api/v1alpha1/proxmoxmachine_types.go index c843dd3a..900dcc10 100644 --- a/api/v1alpha1/proxmoxmachine_types.go +++ b/api/v1alpha1/proxmoxmachine_types.go @@ -216,12 +216,22 @@ type NetworkDevice struct { // +kubebuilder:validation:Enum=e1000;virtio;rtl8139;vmxnet3 // +kubebuilder:default=virtio Model *string `json:"model,omitempty"` + + // DHCP4 indicates if DHCP should be used to assign IPv4 addresses. + // DHCP4 enforces cluster.spec.ipv4Config to use DHCP. + // +optional + DHCP4 bool `json:"dhcp4,omitempty"` + + // DHCP6 indicates if DHCP should be used to assign IPv6 addresses. + // DHCP6 enforces cluster.spec.ipv6Config to use DHCP. + // +optional + DHCP6 bool `json:"dhcp6,omitempty"` } // AdditionalNetworkDevice the definition of a Proxmox network device. -// +kubebuilder:validation:XValidation:rule="self.ipv4PoolRef != null || self.ipv6PoolRef != null",message="at least one pool reference must be set, either ipv4PoolRef or ipv6PoolRef" +// +kubebuilder:validation:XValidation:rule="(self.ipv4PoolRef != null || self.ipv6PoolRef != null || self.dhcp4 || self.dhcp6)",message="at least dhcp and/or one pool reference must be set, either ipv4PoolRef or ipv6PoolRef" type AdditionalNetworkDevice struct { - NetworkDevice `json:",inline"` + *NetworkDevice `json:",inline"` // Name is the network device name. // must be unique within the virtual machine and different from the primary device 'net0'. diff --git a/api/v1alpha1/proxmoxmachine_types_test.go b/api/v1alpha1/proxmoxmachine_types_test.go index 4a77b375..1dc28341 100644 --- a/api/v1alpha1/proxmoxmachine_types_test.go +++ b/api/v1alpha1/proxmoxmachine_types_test.go @@ -109,7 +109,7 @@ var _ = Describe("ProxmoxMachine Test", func() { Bridge: "vmbr0", }, AdditionalDevices: []AdditionalNetworkDevice{{ - NetworkDevice: NetworkDevice{}, + NetworkDevice: &NetworkDevice{}, Name: "net0", IPv4PoolRef: &corev1.TypedLocalObjectReference{ APIGroup: ptr.To("ipam.cluster.x-k8s.io"), @@ -127,7 +127,7 @@ var _ = Describe("ProxmoxMachine Test", func() { dm := defaultMachine() dm.Spec.Network = &NetworkSpec{ AdditionalDevices: []AdditionalNetworkDevice{{ - NetworkDevice: NetworkDevice{}, + NetworkDevice: &NetworkDevice{}, Name: "net1", IPv4PoolRef: &corev1.TypedLocalObjectReference{ APIGroup: ptr.To("apps"), @@ -143,7 +143,7 @@ var _ = Describe("ProxmoxMachine Test", func() { dm := defaultMachine() dm.Spec.Network = &NetworkSpec{ AdditionalDevices: []AdditionalNetworkDevice{{ - NetworkDevice: NetworkDevice{}, + NetworkDevice: &NetworkDevice{}, Name: "net1", IPv4PoolRef: &corev1.TypedLocalObjectReference{ APIGroup: ptr.To("ipam.cluster.x-k8s.io"), @@ -160,7 +160,7 @@ var _ = Describe("ProxmoxMachine Test", func() { dm := defaultMachine() dm.Spec.Network = &NetworkSpec{ AdditionalDevices: []AdditionalNetworkDevice{{ - NetworkDevice: NetworkDevice{}, + NetworkDevice: &NetworkDevice{}, Name: "net1", IPv6PoolRef: &corev1.TypedLocalObjectReference{ APIGroup: ptr.To("apps"), @@ -176,7 +176,7 @@ var _ = Describe("ProxmoxMachine Test", func() { dm := defaultMachine() dm.Spec.Network = &NetworkSpec{ AdditionalDevices: []AdditionalNetworkDevice{{ - NetworkDevice: NetworkDevice{}, + NetworkDevice: &NetworkDevice{}, Name: "net1", IPv6PoolRef: &corev1.TypedLocalObjectReference{ APIGroup: ptr.To("ipam.cluster.x-k8s.io"), @@ -193,12 +193,12 @@ var _ = Describe("ProxmoxMachine Test", func() { dm := defaultMachine() dm.Spec.Network = &NetworkSpec{ AdditionalDevices: []AdditionalNetworkDevice{{ - NetworkDevice: NetworkDevice{}, + NetworkDevice: &NetworkDevice{}, Name: "net1", }, }, } - Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("at least one pool reference must be set, either ipv4PoolRef or ipv6PoolRef"))) + Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("at least dhcp and/or one pool reference must be set, either ipv4PoolRef or ipv6PoolRef"))) }) }) }) diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index a51b4668..06cafed0 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -24,7 +24,6 @@ package v1alpha1 import ( "k8s.io/api/core/v1" runtime "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/cluster-api-ipam-provider-in-cluster/api/v1alpha2" "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/errors" ) @@ -32,7 +31,11 @@ import ( // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AdditionalNetworkDevice) DeepCopyInto(out *AdditionalNetworkDevice) { *out = *in - in.NetworkDevice.DeepCopyInto(&out.NetworkDevice) + if in.NetworkDevice != nil { + in, out := &in.NetworkDevice, &out.NetworkDevice + *out = new(NetworkDevice) + (*in).DeepCopyInto(*out) + } if in.IPv4PoolRef != nil { in, out := &in.IPv4PoolRef, &out.IPv4PoolRef *out = new(v1.TypedLocalObjectReference) @@ -60,6 +63,36 @@ func (in *AdditionalNetworkDevice) DeepCopy() *AdditionalNetworkDevice { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterNetworkConfig) DeepCopyInto(out *ClusterNetworkConfig) { + *out = *in + if in.IPv4Config != nil { + in, out := &in.IPv4Config, &out.IPv4Config + *out = new(IPConfig) + (*in).DeepCopyInto(*out) + } + if in.IPv6Config != nil { + in, out := &in.IPv6Config, &out.IPv6Config + *out = new(IPConfig) + (*in).DeepCopyInto(*out) + } + if in.DNSServers != nil { + in, out := &in.DNSServers, &out.DNSServers + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterNetworkConfig. +func (in *ClusterNetworkConfig) DeepCopy() *ClusterNetworkConfig { + if in == nil { + return nil + } + out := new(ClusterNetworkConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DiskSize) DeepCopyInto(out *DiskSize) { *out = *in @@ -90,6 +123,26 @@ func (in *IPAddress) DeepCopy() *IPAddress { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *IPConfig) DeepCopyInto(out *IPConfig) { + *out = *in + if in.Addresses != nil { + in, out := &in.Addresses, &out.Addresses + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IPConfig. +func (in *IPConfig) DeepCopy() *IPConfig { + if in == nil { + return nil + } + out := new(IPConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NetworkDevice) DeepCopyInto(out *NetworkDevice) { *out = *in @@ -266,21 +319,7 @@ func (in *ProxmoxClusterSpec) DeepCopyInto(out *ProxmoxClusterSpec) { *out = make([]string, len(*in)) copy(*out, *in) } - if in.IPv4Config != nil { - in, out := &in.IPv4Config, &out.IPv4Config - *out = new(v1alpha2.InClusterIPPoolSpec) - (*in).DeepCopyInto(*out) - } - if in.IPv6Config != nil { - in, out := &in.IPv6Config, &out.IPv6Config - *out = new(v1alpha2.InClusterIPPoolSpec) - (*in).DeepCopyInto(*out) - } - if in.DNSServers != nil { - in, out := &in.DNSServers, &out.DNSServers - *out = make([]string, len(*in)) - copy(*out, *in) - } + in.ClusterNetworkConfig.DeepCopyInto(&out.ClusterNetworkConfig) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProxmoxClusterSpec. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclusters.yaml index 61a7bb90..faebb286 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclusters.yaml @@ -82,15 +82,19 @@ spec: ipv4Config: description: IPv4Config contains information about available IPV4 address pools and the gateway. this can be combined with ipv6Config - in order to enable dual stack. either IPv4Config or IPv6Config must - be provided. + to enable dual stack. either IPv4Config or IPv6Config must be provided. properties: addresses: description: Addresses is a list of IP addresses that can be assigned. This set of addresses can be non-contiguous. items: type: string + minItems: 1 type: array + dhcp: + description: DHCP indicates if DHCP should be used to assign IP + addresses. + type: boolean gateway: description: Gateway type: string @@ -98,25 +102,23 @@ spec: description: Prefix is the network prefix to use. maximum: 128 type: integer - required: - - addresses - - prefix type: object - x-kubernetes-validations: - - message: IPv4Config addresses must be provided - rule: self.addresses.size() > 0 ipv6Config: description: IPv6Config contains information about available IPV6 address pools and the gateway. this can be combined with ipv4Config - in order to enable dual stack. either IPv4Config or IPv6Config must - be provided. + to enable dual stack. either IPv4Config or IPv6Config must be provided. properties: addresses: description: Addresses is a list of IP addresses that can be assigned. This set of addresses can be non-contiguous. items: type: string + minItems: 1 type: array + dhcp: + description: DHCP indicates if DHCP should be used to assign IP + addresses. + type: boolean gateway: description: Gateway type: string @@ -124,13 +126,7 @@ spec: description: Prefix is the network prefix to use. maximum: 128 type: integer - required: - - addresses - - prefix type: object - x-kubernetes-validations: - - message: IPv6Config addresses must be provided - rule: self.addresses.size() > 0 required: - dnsServers type: object diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml index 14dc44cd..013ec2aa 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml @@ -128,6 +128,16 @@ spec: machine. minLength: 1 type: string + dhcp4: + description: DHCP4 indicates if DHCP should be used to assign + IPv4 addresses. DHCP4 enforces cluster.spec.ipv4Config + to use DHCP. + type: boolean + dhcp6: + description: DHCP6 indicates if DHCP should be used to assign + IPv6 addresses. DHCP6 enforces cluster.spec.ipv6Config + to use DHCP. + type: boolean dnsServers: description: DNSServers contains information about nameservers to be used for this interface. If this field is not set, @@ -219,9 +229,10 @@ spec: - name type: object x-kubernetes-validations: - - message: at least one pool reference must be set, either ipv4PoolRef - or ipv6PoolRef - rule: self.ipv4PoolRef != null || self.ipv6PoolRef != null + - message: at least dhcp and/or one pool reference must be set, + either ipv4PoolRef or ipv6PoolRef + rule: (self.ipv4PoolRef != null || self.ipv6PoolRef != null + || self.dhcp4 || self.dhcp6) type: array x-kubernetes-list-map-keys: - name @@ -236,6 +247,16 @@ spec: machine. minLength: 1 type: string + dhcp4: + description: DHCP4 indicates if DHCP should be used to assign + IPv4 addresses. DHCP4 enforces cluster.spec.ipv4Config to + use DHCP. + type: boolean + dhcp6: + description: DHCP6 indicates if DHCP should be used to assign + IPv6 addresses. DHCP6 enforces cluster.spec.ipv6Config to + use DHCP. + type: boolean model: default: virtio description: Model is the network device model. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml index 180655aa..0f714b48 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml @@ -135,6 +135,16 @@ spec: to the machine. minLength: 1 type: string + dhcp4: + description: DHCP4 indicates if DHCP should be used + to assign IPv4 addresses. DHCP4 enforces cluster.spec.ipv4Config + to use DHCP. + type: boolean + dhcp6: + description: DHCP6 indicates if DHCP should be used + to assign IPv6 addresses. DHCP6 enforces cluster.spec.ipv6Config + to use DHCP. + type: boolean dnsServers: description: DNSServers contains information about nameservers to be used for this interface. If @@ -238,10 +248,10 @@ spec: - name type: object x-kubernetes-validations: - - message: at least one pool reference must be set, - either ipv4PoolRef or ipv6PoolRef - rule: self.ipv4PoolRef != null || self.ipv6PoolRef - != null + - message: at least dhcp and/or one pool reference must + be set, either ipv4PoolRef or ipv6PoolRef + rule: (self.ipv4PoolRef != null || self.ipv6PoolRef + != null || self.dhcp4 || self.dhcp6) type: array x-kubernetes-list-map-keys: - name @@ -256,6 +266,16 @@ spec: to the machine. minLength: 1 type: string + dhcp4: + description: DHCP4 indicates if DHCP should be used + to assign IPv4 addresses. DHCP4 enforces cluster.spec.ipv4Config + to use DHCP. + type: boolean + dhcp6: + description: DHCP6 indicates if DHCP should be used + to assign IPv6 addresses. DHCP6 enforces cluster.spec.ipv6Config + to use DHCP. + type: boolean model: default: virtio description: Model is the network device model. diff --git a/internal/controller/proxmoxcluster_controller.go b/internal/controller/proxmoxcluster_controller.go index 5cedab9b..b1c12aea 100644 --- a/internal/controller/proxmoxcluster_controller.go +++ b/internal/controller/proxmoxcluster_controller.go @@ -168,13 +168,14 @@ func (r *ProxmoxClusterReconciler) reconcileNormal(ctx context.Context, clusterS // If the ProxmoxCluster doesn't have our finalizer, add it. ctrlutil.AddFinalizer(clusterScope.ProxmoxCluster, infrav1alpha1.ClusterFinalizer) - res, err := r.reconcileIPAM(ctx, clusterScope) - if err != nil { - return ctrl.Result{}, err - } - - if !res.IsZero() { - return res, nil + if !dhcpEnabled(clusterScope.ProxmoxCluster) { + res, err := r.reconcileIPAM(ctx, clusterScope) + if err != nil { + return ctrl.Result{}, err + } + if !res.IsZero() { + return res, nil + } } conditions.MarkTrue(clusterScope.ProxmoxCluster, infrav1alpha1.ProxmoxClusterReady) @@ -193,7 +194,7 @@ func (r *ProxmoxClusterReconciler) reconcileIPAM(ctx context.Context, clusterSco return ctrl.Result{}, err } - if clusterScope.ProxmoxCluster.Spec.IPv4Config != nil { + if clusterScope.ProxmoxCluster.Spec.IPv4Config != nil && !clusterScope.ProxmoxCluster.Spec.IPv4Config.DHCP { poolV4, err := clusterScope.IPAMHelper.GetDefaultInClusterIPPool(ctx, infrav1alpha1.IPV4Format) if err != nil { if apierrors.IsNotFound(err) { @@ -204,7 +205,7 @@ func (r *ProxmoxClusterReconciler) reconcileIPAM(ctx context.Context, clusterSco } clusterScope.ProxmoxCluster.SetInClusterIPPoolRef(poolV4) } - if clusterScope.ProxmoxCluster.Spec.IPv6Config != nil { + if clusterScope.ProxmoxCluster.Spec.IPv6Config != nil && !clusterScope.ProxmoxCluster.Spec.IPv6Config.DHCP { poolV6, err := clusterScope.IPAMHelper.GetDefaultInClusterIPPool(ctx, infrav1alpha1.IPV6Format) if err != nil { if apierrors.IsNotFound(err) { @@ -243,3 +244,16 @@ func (r *ProxmoxClusterReconciler) SetupWithManager(ctx context.Context, mgr ctr builder.WithPredicates(predicates.ClusterUnpaused(ctrl.LoggerFrom(ctx)))). Complete(r) } + +func dhcpEnabled(cluster *infrav1alpha1.ProxmoxCluster) bool { + switch { + case (cluster.Spec.IPv6Config != nil && cluster.Spec.IPv6Config.DHCP) && (cluster.Spec.IPv4Config != nil && cluster.Spec.IPv4Config.DHCP): + return true + case (cluster.Spec.IPv6Config != nil && cluster.Spec.IPv6Config.DHCP) && cluster.Spec.IPv4Config == nil: + return true + case (cluster.Spec.IPv4Config != nil && cluster.Spec.IPv4Config.DHCP) && cluster.Spec.IPv6Config == nil: + return true + default: + return false + } +} diff --git a/internal/controller/proxmoxcluster_controller_test.go b/internal/controller/proxmoxcluster_controller_test.go index a19af4ca..efad836b 100644 --- a/internal/controller/proxmoxcluster_controller_test.go +++ b/internal/controller/proxmoxcluster_controller_test.go @@ -116,7 +116,7 @@ var _ = Describe("Controller Test", func() { }) It("Should successfully create IPAM IPV6 related resources", func() { cl := buildProxmoxCluster(clusterName) - cl.Spec.IPv6Config = &ipamicv1.InClusterIPPoolSpec{ + cl.Spec.IPv6Config = &infrav1.IPConfig{ Addresses: []string{"2001:db8::/64"}, Prefix: 64, Gateway: "2001:db8::1", @@ -232,16 +232,18 @@ func buildProxmoxCluster(name string) infrav1.ProxmoxCluster { Host: "10.10.10.11", Port: 6443, }, - IPv4Config: &ipamicv1.InClusterIPPoolSpec{ - Addresses: []string{ - "10.10.10.2-10.10.10.10", - "10.10.10.100-10.10.10.125", - "10.10.10.192/64", + ClusterNetworkConfig: infrav1.ClusterNetworkConfig{ + IPv4Config: &infrav1.IPConfig{ + Addresses: []string{ + "10.10.10.2-10.10.10.10", + "10.10.10.100-10.10.10.125", + "10.10.10.192/64", + }, + Gateway: "10.10.10.1", + Prefix: 24, }, - Gateway: "10.10.10.1", - Prefix: 24, + DNSServers: []string{"8.8.8.8", "8.8.4.4"}, }, - DNSServers: []string{"8.8.8.8", "8.8.4.4"}, }, } diff --git a/internal/service/vmservice/bootstrap.go b/internal/service/vmservice/bootstrap.go index 513b202b..c6755bb1 100644 --- a/internal/service/vmservice/bootstrap.go +++ b/internal/service/vmservice/bootstrap.go @@ -40,9 +40,15 @@ func reconcileBootstrapData(ctx context.Context, machineScope *scope.MachineScop return false, nil } - if !machineHasIPAddress(machineScope.ProxmoxMachine) { - // skip machine doesn't have an IpAddress yet. - conditions.MarkFalse(machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition, infrav1alpha1.WaitingForStaticIPAllocationReason, clusterv1.ConditionSeverityWarning, "no ip address") + // TODO check if this needs to be re-enabled + //if !machineHasIPAddress(machineScope.ProxmoxMachine) { + // // skip machine doesn't have an IpAddress yet. + // conditions.MarkFalse(machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition, infrav1alpha1.WaitingForStaticIPAllocationReason, clusterv1.ConditionSeverityWarning, "no ip address") + // return true, nil + //} + if machineScope.VirtualMachine == nil { + // skip machine is not yet provisioned. + conditions.MarkFalse(machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition, infrav1alpha1.WaitingForBootstrapDataReason, clusterv1.ConditionSeverityWarning, "no ip address") return true, nil } @@ -141,23 +147,47 @@ func getNetworkConfigData(ctx context.Context, machineScope *scope.MachineScope) return networkConfigData, nil } -func getNetworkConfigDataForDevice(ctx context.Context, machineScope *scope.MachineScope, device string) (*cloudinit.NetworkConfigData, error) { +func getNetworkConfigDataForDevice(ctx context.Context, machineScope *scope.MachineScope, device *infrav1alpha1.NetworkDevice, deviceName, format string) (*cloudinit.NetworkConfigData, error) { nets := machineScope.VirtualMachine.VirtualMachineConfig.MergeNets() // For nics supporting multiple IP addresses, we need to cut the '-inet' or '-inet6' part, // to retrieve the correct MAC address. - formattedDevice, _, _ := strings.Cut(device, "-") + formattedDevice, _, _ := strings.Cut(deviceName, "-") macAddress := extractMACAddress(nets[formattedDevice]) if len(macAddress) == 0 { machineScope.Logger.Error(errors.New("unable to extract mac address"), "device has no mac address", "device", device) return nil, errors.New("unable to extract mac address") } + dns := machineScope.InfraCluster.ProxmoxCluster.Spec.DNSServers + + var ipConfig *infrav1alpha1.IPConfig + if format == infrav1alpha1.IPV4Format { + ipConfig = machineScope.InfraCluster.ProxmoxCluster.Spec.IPv4Config + } else if format == infrav1alpha1.IPV6Format { + ipConfig = machineScope.InfraCluster.ProxmoxCluster.Spec.IPv6Config + } + + // check if DHCP is enabled. + if hasDHCPEnabled(ipConfig, device, formattedDevice, format) { + cloudinitNetworkConfigData := &cloudinit.NetworkConfigData{ + MacAddress: macAddress, + DNSServers: dns, + } + + if format == infrav1alpha1.IPV6Format { + cloudinitNetworkConfigData.DHCP6 = true + } else if format == infrav1alpha1.IPV4Format { + cloudinitNetworkConfigData.DHCP4 = true + } + + return cloudinitNetworkConfigData, nil + } + // retrieve IPAddress. - ipAddr, err := findIPAddress(ctx, machineScope, device) + ipAddr, err := findIPAddress(ctx, machineScope, deviceName) if err != nil { - return nil, errors.Wrapf(err, "unable to find IPAddress, device=%s", device) + return nil, errors.Wrapf(err, "unable to find IPAddress, device=%s", formattedDevice) } - dns := machineScope.InfraCluster.ProxmoxCluster.Spec.DNSServers ip := IPAddressWithPrefix(ipAddr.Spec.Address, ipAddr.Spec.Prefix) gw := ipAddr.Spec.Gateway @@ -181,9 +211,18 @@ func getNetworkConfigDataForDevice(ctx context.Context, machineScope *scope.Mach func getDefaultNetworkDevice(ctx context.Context, machineScope *scope.MachineScope) ([]cloudinit.NetworkConfigData, error) { var config cloudinit.NetworkConfigData + // check if the default network device is specified. + var networkDevice *infrav1alpha1.NetworkDevice + if machineScope.ProxmoxMachine.Spec.Network != nil && machineScope.ProxmoxMachine.Spec.Network.Default != nil { + networkDevice = machineScope.ProxmoxMachine.Spec.Network.Default + } // default network device ipv4. if machineScope.InfraCluster.ProxmoxCluster.Spec.IPv4Config != nil { - conf, err := getNetworkConfigDataForDevice(ctx, machineScope, DefaultNetworkDeviceIPV4) + conf, err := getNetworkConfigDataForDevice(ctx, + machineScope, + networkDevice, + DefaultNetworkDeviceIPV4, + infrav1alpha1.IPV4Format) if err != nil { return nil, errors.Wrapf(err, "unable to get network config data for device=%s", DefaultNetworkDeviceIPV4) } @@ -192,7 +231,11 @@ func getDefaultNetworkDevice(ctx context.Context, machineScope *scope.MachineSco // default network device ipv6. if machineScope.InfraCluster.ProxmoxCluster.Spec.IPv6Config != nil { - conf, err := getNetworkConfigDataForDevice(ctx, machineScope, DefaultNetworkDeviceIPV6) + conf, err := getNetworkConfigDataForDevice(ctx, + machineScope, + networkDevice, + DefaultNetworkDeviceIPV6, + infrav1alpha1.IPV6Format) if err != nil { return nil, errors.Wrapf(err, "unable to get network config data for device=%s", DefaultNetworkDeviceIPV6) } @@ -203,6 +246,7 @@ func getDefaultNetworkDevice(ctx context.Context, machineScope *scope.MachineSco case config.MacAddress != conf.MacAddress: return nil, errors.New("default network device ipv4 and ipv6 have different mac addresses") default: + config.DHCP6 = conf.DHCP6 config.IPV6Address = conf.IPV6Address config.Gateway6 = conf.Gateway6 } @@ -220,7 +264,11 @@ func getAdditionalNetworkDevices(ctx context.Context, machineScope *scope.Machin if nic.IPv4PoolRef != nil { device := fmt.Sprintf("%s-%s", nic.Name, infrav1alpha1.DefaultSuffix) - conf, err := getNetworkConfigDataForDevice(ctx, machineScope, device) + conf, err := getNetworkConfigDataForDevice(ctx, + machineScope, + nic.NetworkDevice, + device, + infrav1alpha1.IPV4Format) if err != nil { return nil, errors.Wrapf(err, "unable to get network config data for device=%s", device) } @@ -233,7 +281,11 @@ func getAdditionalNetworkDevices(ctx context.Context, machineScope *scope.Machin if nic.IPv6PoolRef != nil { suffix := infrav1alpha1.DefaultSuffix + "6" device := fmt.Sprintf("%s-%s", nic.Name, suffix) - conf, err := getNetworkConfigDataForDevice(ctx, machineScope, device) + conf, err := getNetworkConfigDataForDevice(ctx, + machineScope, + nic.NetworkDevice, + device, + infrav1alpha1.IPV6Format) if err != nil { return nil, errors.Wrapf(err, "unable to get network config data for device=%s", device) } diff --git a/internal/service/vmservice/bootstrap_test.go b/internal/service/vmservice/bootstrap_test.go index b989a67d..f7471e3a 100644 --- a/internal/service/vmservice/bootstrap_test.go +++ b/internal/service/vmservice/bootstrap_test.go @@ -24,10 +24,9 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" "k8s.io/utils/ptr" - "sigs.k8s.io/cluster-api-ipam-provider-in-cluster/api/v1alpha2" "sigs.k8s.io/cluster-api/util/conditions" - infrav1alpha1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1" + infrav1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1" "github.com/ionos-cloud/cluster-api-provider-proxmox/internal/inject" "github.com/ionos-cloud/cluster-api-provider-proxmox/pkg/cloudinit" "github.com/ionos-cloud/cluster-api-provider-proxmox/pkg/scope" @@ -43,18 +42,18 @@ func TestReconcileBootstrapData_MissingIPAddress(t *testing.T) { requeue, err := reconcileBootstrapData(context.Background(), machineScope) require.NoError(t, err) require.True(t, requeue) - requireConditionIsFalse(t, machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition) + requireConditionIsFalse(t, machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition) } func TestReconcileBootstrapData_MissingMACAddress(t *testing.T) { machineScope, _, _ := setupReconcilerTest(t) machineScope.SetVirtualMachine(newStoppedVM()) - machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1alpha1.IPAddress{infrav1alpha1.DefaultNetworkDevice: {IPV4: "10.10.10.10"}} + machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1.IPAddress{infrav1.DefaultNetworkDevice: {IPV4: "10.10.10.10"}} requeue, err := reconcileBootstrapData(context.Background(), machineScope) require.NoError(t, err) require.True(t, requeue) - require.False(t, conditions.Has(machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition)) + require.False(t, conditions.Has(machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition)) } func TestReconcileBootstrapData_NoNetworkConfig_UpdateStatus(t *testing.T) { @@ -67,23 +66,23 @@ func TestReconcileBootstrapData_NoNetworkConfig_UpdateStatus(t *testing.T) { vm := newVMWithNets("virtio=A6:23:64:4D:84:CB,bridge=vmbr0") vm.VirtualMachineConfig.SMBios1 = biosUUID machineScope.SetVirtualMachine(vm) - machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1alpha1.IPAddress{infrav1alpha1.DefaultNetworkDevice: {IPV4: "10.10.10.10"}} - createIP4AddressResource(t, kubeClient, machineScope, infrav1alpha1.DefaultNetworkDevice, "10.10.10.10") + machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1.IPAddress{infrav1.DefaultNetworkDevice: {IPV4: "10.10.10.10"}} + createIP4AddressResource(t, kubeClient, machineScope, infrav1.DefaultNetworkDevice, "10.10.10.10") createBootstrapSecret(t, kubeClient, machineScope) requeue, err := reconcileBootstrapData(context.Background(), machineScope) require.NoError(t, err) require.False(t, requeue) - require.False(t, conditions.Has(machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition)) + require.False(t, conditions.Has(machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition)) require.True(t, *machineScope.ProxmoxMachine.Status.BootstrapDataProvided) } func TestReconcileBootstrapData_UpdateStatus(t *testing.T) { machineScope, _, kubeClient := setupReconcilerTest(t) - machineScope.ProxmoxMachine.Spec.Network = &infrav1alpha1.NetworkSpec{ - AdditionalDevices: []infrav1alpha1.AdditionalNetworkDevice{ + machineScope.ProxmoxMachine.Spec.Network = &infrav1.NetworkSpec{ + AdditionalDevices: []infrav1.AdditionalNetworkDevice{ { - NetworkDevice: infrav1alpha1.NetworkDevice{Bridge: "vmbr1", Model: ptr.To("virtio")}, + NetworkDevice: &infrav1.NetworkDevice{Bridge: "vmbr1", Model: ptr.To("virtio")}, Name: "net1", DNSServers: []string{"1.2.3.4"}, }, @@ -92,8 +91,8 @@ func TestReconcileBootstrapData_UpdateStatus(t *testing.T) { vm := newVMWithNets("virtio=A6:23:64:4D:84:CB,bridge=vmbr0", "virtio=AA:23:64:4D:84:CD,bridge=vmbr1") vm.VirtualMachineConfig.SMBios1 = biosUUID machineScope.SetVirtualMachine(vm) - machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1alpha1.IPAddress{infrav1alpha1.DefaultNetworkDevice: {IPV4: "10.10.10.10"}, "net1": {IPV4: "10.100.10.10"}} - createIP4AddressResource(t, kubeClient, machineScope, infrav1alpha1.DefaultNetworkDevice, "10.10.10.10") + machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1.IPAddress{infrav1.DefaultNetworkDevice: {IPV4: "10.10.10.10"}, "net1": {IPV4: "10.100.10.10"}} + createIP4AddressResource(t, kubeClient, machineScope, infrav1.DefaultNetworkDevice, "10.10.10.10") createIP4AddressResource(t, kubeClient, machineScope, "net1", "10.100.10.10") createBootstrapSecret(t, kubeClient, machineScope) getISOInjector = func(_ *proxmox.VirtualMachine, _ []byte, _, _ cloudinit.Renderer) isoInjector { @@ -104,7 +103,7 @@ func TestReconcileBootstrapData_UpdateStatus(t *testing.T) { requeue, err := reconcileBootstrapData(context.Background(), machineScope) require.NoError(t, err) require.False(t, requeue) - require.False(t, conditions.Has(machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition)) + require.False(t, conditions.Has(machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition)) require.True(t, *machineScope.ProxmoxMachine.Status.BootstrapDataProvided) } @@ -121,7 +120,7 @@ func TestGetNetworkConfigDataForDevice_MissingIPAddress(t *testing.T) { vm := newVMWithNets("virtio=A6:23:64:4D:84:CB,bridge=vmbr0") machineScope.SetVirtualMachine(vm) - cfg, err := getNetworkConfigDataForDevice(context.Background(), machineScope, "net0") + cfg, err := getNetworkConfigDataForDevice(context.Background(), machineScope, nil, "net0", infrav1.IPV4Format) require.Error(t, err) require.Nil(t, cfg) } @@ -130,14 +129,14 @@ func TestGetNetworkConfigDataForDevice_MissingMACAddress(t *testing.T) { machineScope, _, _ := setupReconcilerTest(t) machineScope.SetVirtualMachine(newStoppedVM()) - cfg, err := getNetworkConfigDataForDevice(context.Background(), machineScope, "net2") + cfg, err := getNetworkConfigDataForDevice(context.Background(), machineScope, nil, "net2", infrav1.IPV4Format) require.Error(t, err) require.Nil(t, cfg) } func TestReconcileBootstrapData_DualStack(t *testing.T) { machineScope, _, kubeClient := setupReconcilerTest(t) - machineScope.InfraCluster.ProxmoxCluster.Spec.IPv6Config = &v1alpha2.InClusterIPPoolSpec{ + machineScope.InfraCluster.ProxmoxCluster.Spec.IPv6Config = &infrav1.IPConfig{ Addresses: []string{"2001:db8::/64"}, Prefix: 64, Gateway: "2001:db8::1", @@ -146,9 +145,9 @@ func TestReconcileBootstrapData_DualStack(t *testing.T) { vm := newVMWithNets("virtio=A6:23:64:4D:84:CB,bridge=vmbr0", "virtio=AA:23:64:4D:84:CD,bridge=vmbr1") vm.VirtualMachineConfig.SMBios1 = biosUUID machineScope.SetVirtualMachine(vm) - machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1alpha1.IPAddress{infrav1alpha1.DefaultNetworkDevice: {IPV4: "10.10.10.10", IPV6: "2001:db8::2"}} - createIP4AddressResource(t, kubeClient, machineScope, infrav1alpha1.DefaultNetworkDevice, "10.10.10.10") - createIP6AddressResource(t, kubeClient, machineScope, infrav1alpha1.DefaultNetworkDevice, "2001:db8::2") + machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1.IPAddress{infrav1.DefaultNetworkDevice: {IPV4: "10.10.10.10", IPV6: "2001:db8::2"}} + createIP4AddressResource(t, kubeClient, machineScope, infrav1.DefaultNetworkDevice, "10.10.10.10") + createIP6AddressResource(t, kubeClient, machineScope, infrav1.DefaultNetworkDevice, "2001:db8::2") createBootstrapSecret(t, kubeClient, machineScope) getISOInjector = func(_ *proxmox.VirtualMachine, _ []byte, _, _ cloudinit.Renderer) isoInjector { @@ -159,22 +158,22 @@ func TestReconcileBootstrapData_DualStack(t *testing.T) { requeue, err := reconcileBootstrapData(context.Background(), machineScope) require.NoError(t, err) require.False(t, requeue) - require.False(t, conditions.Has(machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition)) + require.False(t, conditions.Has(machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition)) require.True(t, *machineScope.ProxmoxMachine.Status.BootstrapDataProvided) } func TestReconcileBootstrapData_DualStack_AdditionalDevices(t *testing.T) { machineScope, _, kubeClient := setupReconcilerTest(t) - machineScope.InfraCluster.ProxmoxCluster.Spec.IPv6Config = &v1alpha2.InClusterIPPoolSpec{ + machineScope.InfraCluster.ProxmoxCluster.Spec.IPv6Config = &infrav1.IPConfig{ Addresses: []string{"2001:db8::/64"}, Prefix: 64, Gateway: "2001:db8::1", } - machineScope.ProxmoxMachine.Spec.Network = &infrav1alpha1.NetworkSpec{ - AdditionalDevices: []infrav1alpha1.AdditionalNetworkDevice{ + machineScope.ProxmoxMachine.Spec.Network = &infrav1.NetworkSpec{ + AdditionalDevices: []infrav1.AdditionalNetworkDevice{ { - NetworkDevice: infrav1alpha1.NetworkDevice{Bridge: "vmbr1", Model: ptr.To("virtio")}, + NetworkDevice: &infrav1.NetworkDevice{Bridge: "vmbr1", Model: ptr.To("virtio")}, Name: "net1", DNSServers: []string{"1.2.3.4"}, IPv6PoolRef: &corev1.TypedLocalObjectReference{ @@ -194,10 +193,10 @@ func TestReconcileBootstrapData_DualStack_AdditionalDevices(t *testing.T) { vm := newVMWithNets("virtio=A6:23:64:4D:84:CB,bridge=vmbr0", "virtio=AA:23:64:4D:84:CD,bridge=vmbr1") vm.VirtualMachineConfig.SMBios1 = biosUUID machineScope.SetVirtualMachine(vm) - machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1alpha1.IPAddress{infrav1alpha1.DefaultNetworkDevice: {IPV4: "10.10.10.10", IPV6: "2001:db8::2"}, "net1": {IPV4: "10.0.0.10", IPV6: "2001:db8::9"}} + machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1.IPAddress{infrav1.DefaultNetworkDevice: {IPV4: "10.10.10.10", IPV6: "2001:db8::2"}, "net1": {IPV4: "10.0.0.10", IPV6: "2001:db8::9"}} createIPPools(t, kubeClient, machineScope) - createIP4AddressResource(t, kubeClient, machineScope, infrav1alpha1.DefaultNetworkDevice, "10.10.10.10") - createIP6AddressResource(t, kubeClient, machineScope, infrav1alpha1.DefaultNetworkDevice, "2001:db8::2") + createIP4AddressResource(t, kubeClient, machineScope, infrav1.DefaultNetworkDevice, "10.10.10.10") + createIP6AddressResource(t, kubeClient, machineScope, infrav1.DefaultNetworkDevice, "2001:db8::2") createIP4AddressResource(t, kubeClient, machineScope, "net1", "10.0.0.10") createIP6AddressResource(t, kubeClient, machineScope, "net1", "2001:db8::9") createBootstrapSecret(t, kubeClient, machineScope) @@ -209,7 +208,7 @@ func TestReconcileBootstrapData_DualStack_AdditionalDevices(t *testing.T) { requeue, err := reconcileBootstrapData(context.Background(), machineScope) require.NoError(t, err) require.False(t, requeue) - require.False(t, conditions.Has(machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition)) + require.False(t, conditions.Has(machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition)) require.True(t, *machineScope.ProxmoxMachine.Status.BootstrapDataProvided) } @@ -228,15 +227,15 @@ func TestReconcileBootstrapDataMissingSecret(t *testing.T) { vm.VirtualMachineConfig.SMBios1 = biosUUID machineScope.SetVirtualMachine(vm) - machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1alpha1.IPAddress{infrav1alpha1.DefaultNetworkDevice: {IPV4: "10.10.10.10"}} - createIP4AddressResource(t, kubeClient, machineScope, infrav1alpha1.DefaultNetworkDevice, "10.10.10.10") + machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1.IPAddress{infrav1.DefaultNetworkDevice: {IPV4: "10.10.10.10"}} + createIP4AddressResource(t, kubeClient, machineScope, infrav1.DefaultNetworkDevice, "10.10.10.10") requeue, err := reconcileBootstrapData(context.Background(), machineScope) require.Error(t, err) require.False(t, requeue) - require.True(t, conditions.Has(machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition)) - require.True(t, conditions.IsFalse(machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition)) - require.True(t, conditions.GetReason(machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition) == infrav1alpha1.CloningFailedReason) + require.True(t, conditions.Has(machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition)) + require.True(t, conditions.IsFalse(machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition)) + require.True(t, conditions.GetReason(machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition) == infrav1.CloningFailedReason) } func TestReconcileBootstrapDataMissingNetworkConfig(t *testing.T) { @@ -245,15 +244,15 @@ func TestReconcileBootstrapDataMissingNetworkConfig(t *testing.T) { vm.VirtualMachineConfig.SMBios1 = biosUUID machineScope.SetVirtualMachine(vm) - machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1alpha1.IPAddress{infrav1alpha1.DefaultNetworkDevice: {IPV4: "10.10.10.10"}} + machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1.IPAddress{infrav1.DefaultNetworkDevice: {IPV4: "10.10.10.10"}} createBootstrapSecret(t, kubeClient, machineScope) requeue, err := reconcileBootstrapData(context.Background(), machineScope) require.Error(t, err) require.False(t, requeue) - require.True(t, conditions.Has(machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition)) - require.True(t, conditions.IsFalse(machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition)) - require.True(t, conditions.GetReason(machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition) == infrav1alpha1.WaitingForStaticIPAllocationReason) + require.True(t, conditions.Has(machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition)) + require.True(t, conditions.IsFalse(machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition)) + require.True(t, conditions.GetReason(machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition) == infrav1.WaitingForStaticIPAllocationReason) } func TestDefaultISOInjector(t *testing.T) { diff --git a/internal/service/vmservice/helpers_test.go b/internal/service/vmservice/helpers_test.go index ace17a38..c3b897a3 100644 --- a/internal/service/vmservice/helpers_test.go +++ b/internal/service/vmservice/helpers_test.go @@ -36,7 +36,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" - infrav1alpha1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1" + infrav1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1" "github.com/ionos-cloud/cluster-api-provider-proxmox/pkg/kubernetes/ipam" "github.com/ionos-cloud/cluster-api-provider-proxmox/pkg/proxmox/proxmoxtest" "github.com/ionos-cloud/cluster-api-provider-proxmox/pkg/scope" @@ -66,38 +66,40 @@ func setupReconcilerTest(t *testing.T) (*scope.MachineScope, *proxmoxtest.MockCl }, } - infraCluster := &infrav1alpha1.ProxmoxCluster{ + infraCluster := &infrav1.ProxmoxCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test", Namespace: metav1.NamespaceDefault, Finalizers: []string{ - infrav1alpha1.ClusterFinalizer, + infrav1.ClusterFinalizer, }, }, - Spec: infrav1alpha1.ProxmoxClusterSpec{ - IPv4Config: &ipamicv1.InClusterIPPoolSpec{ - Addresses: []string{"10.0.0.10-10.0.0.20"}, - Prefix: 24, - Gateway: "10.0.0.1", + Spec: infrav1.ProxmoxClusterSpec{ + ClusterNetworkConfig: infrav1.ClusterNetworkConfig{ + IPv4Config: &infrav1.IPConfig{ + Addresses: []string{"10.0.0.10-10.0.0.20"}, + Prefix: 24, + Gateway: "10.0.0.1", + }, + DNSServers: []string{"1.2.3.4"}, }, - DNSServers: []string{"1.2.3.4"}, }, - Status: infrav1alpha1.ProxmoxClusterStatus{ - NodeLocations: &infrav1alpha1.NodeLocations{}, + Status: infrav1.ProxmoxClusterStatus{ + NodeLocations: &infrav1.NodeLocations{}, }, } - infraCluster.Status.InClusterIPPoolRef = []corev1.LocalObjectReference{{Name: ipam.InClusterPoolFormat(infraCluster, infrav1alpha1.IPV4Format)}} + infraCluster.Status.InClusterIPPoolRef = []corev1.LocalObjectReference{{Name: ipam.InClusterPoolFormat(infraCluster, infrav1.IPV4Format)}} - infraMachine := &infrav1alpha1.ProxmoxMachine{ + infraMachine := &infrav1.ProxmoxMachine{ ObjectMeta: metav1.ObjectMeta{ Name: "test", Namespace: metav1.NamespaceDefault, Finalizers: []string{ - infrav1alpha1.MachineFinalizer, + infrav1.MachineFinalizer, }, }, - Spec: infrav1alpha1.ProxmoxMachineSpec{ - VirtualMachineCloneSpec: infrav1alpha1.VirtualMachineCloneSpec{ + Spec: infrav1.ProxmoxMachineSpec{ + VirtualMachineCloneSpec: infrav1.VirtualMachineCloneSpec{ SourceNode: "node1", TemplateID: ptr.To[int32](123), }, @@ -109,11 +111,11 @@ func setupReconcilerTest(t *testing.T) (*scope.MachineScope, *proxmoxtest.MockCl require.NoError(t, clusterv1.AddToScheme(scheme)) require.NoError(t, ipamv1.AddToScheme(scheme)) require.NoError(t, ipamicv1.AddToScheme(scheme)) - require.NoError(t, infrav1alpha1.AddToScheme(scheme)) + require.NoError(t, infrav1.AddToScheme(scheme)) kubeClient := fake.NewClientBuilder(). WithScheme(scheme). WithObjects(cluster, machine, infraCluster, infraMachine). - WithStatusSubresource(&infrav1alpha1.ProxmoxCluster{}, &infrav1alpha1.ProxmoxMachine{}). + WithStatusSubresource(&infrav1.ProxmoxCluster{}, &infrav1.ProxmoxMachine{}). Build() ipamHelper := ipam.NewHelper(kubeClient, infraCluster) @@ -148,7 +150,7 @@ func setupReconcilerTest(t *testing.T) (*scope.MachineScope, *proxmoxtest.MockCl } func getIPSuffix(addr string) string { - suffix := infrav1alpha1.DefaultSuffix + suffix := infrav1.DefaultSuffix ip := netip.MustParseAddr(addr) if ip.Is6() { suffix += "6" diff --git a/internal/service/vmservice/ip.go b/internal/service/vmservice/ip.go index 5f7beca4..42492570 100644 --- a/internal/service/vmservice/ip.go +++ b/internal/service/vmservice/ip.go @@ -30,7 +30,7 @@ import ( "sigs.k8s.io/cluster-api/util/conditions" "sigs.k8s.io/controller-runtime/pkg/client" - infrav1alpha1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1" + infrav1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1" "github.com/ionos-cloud/cluster-api-provider-proxmox/pkg/scope" ) @@ -40,9 +40,9 @@ func reconcileIPAddresses(ctx context.Context, machineScope *scope.MachineScope) return false, nil } machineScope.Logger.V(4).Info("reconciling IPAddresses.") - conditions.MarkFalse(machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition, infrav1alpha1.WaitingForStaticIPAllocationReason, clusterv1.ConditionSeverityInfo, "") + conditions.MarkFalse(machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition, infrav1.WaitingForStaticIPAllocationReason, clusterv1.ConditionSeverityInfo, "") - addresses := make(map[string]infrav1alpha1.IPAddress) + addresses := make(map[string]infrav1.IPAddress) // default device. if requeue, err = handleDefaultDevice(ctx, machineScope, addresses); err != nil || requeue { @@ -74,13 +74,13 @@ func formatIPAddressName(name, device string) string { return fmt.Sprintf("%s-%s", name, device) } -func machineHasIPAddress(machine *infrav1alpha1.ProxmoxMachine) bool { - return machine.Status.IPAddresses[infrav1alpha1.DefaultNetworkDevice] != (infrav1alpha1.IPAddress{}) +func machineHasIPAddress(machine *infrav1.ProxmoxMachine) bool { + return machine.Status.IPAddresses[infrav1.DefaultNetworkDevice] != (infrav1.IPAddress{}) } func handleIPAddressForDevice(ctx context.Context, machineScope *scope.MachineScope, device, format string, ipamRef *corev1.TypedLocalObjectReference) (string, error) { - suffix := infrav1alpha1.DefaultSuffix - if format == infrav1alpha1.IPV6Format { + suffix := infrav1.DefaultSuffix + if format == infrav1.IPV6Format { suffix += "6" } formattedDevice := fmt.Sprintf("%s-%s", device, suffix) @@ -107,7 +107,7 @@ func handleIPAddressForDevice(ctx context.Context, machineScope *scope.MachineSc ipTag := fmt.Sprintf("ip_%s_%s", device, ip) // Add ip tag if the Virtual Machine doesn't have it. - if vm := machineScope.VirtualMachine; device == infrav1alpha1.DefaultNetworkDevice && !vm.HasTag(ipTag) && isIPV4(ip) { + if vm := machineScope.VirtualMachine; device == infrav1.DefaultNetworkDevice && !vm.HasTag(ipTag) && isIPV4(ip) { machineScope.Logger.V(4).Info("adding virtual machine ip tag.") t, err := machineScope.InfraCluster.ProxmoxClient.TagVM(ctx, vm, ipTag) if err != nil { @@ -120,55 +120,102 @@ func handleIPAddressForDevice(ctx context.Context, machineScope *scope.MachineSc return ip, nil } -func handleDefaultDevice(ctx context.Context, machineScope *scope.MachineScope, addresses map[string]infrav1alpha1.IPAddress) (bool, error) { +func handleDefaultDevice(ctx context.Context, machineScope *scope.MachineScope, addresses map[string]infrav1.IPAddress) (bool, error) { + + // check if the default network device is specified. + var networkDevice *infrav1.NetworkDevice + if machineScope.ProxmoxMachine.Spec.Network != nil && machineScope.ProxmoxMachine.Spec.Network.Default != nil { + networkDevice = machineScope.ProxmoxMachine.Spec.Network.Default + } // default network device ipv4. if machineScope.InfraCluster.ProxmoxCluster.Spec.IPv4Config != nil { - ip, err := handleIPAddressForDevice(ctx, machineScope, infrav1alpha1.DefaultNetworkDevice, infrav1alpha1.IPV4Format, nil) - if err != nil || ip == "" { - return true, err - } - addresses[infrav1alpha1.DefaultNetworkDevice] = infrav1alpha1.IPAddress{ - IPV4: ip, + if hasDHCPEnabled(machineScope.InfraCluster.ProxmoxCluster.Spec.IPv4Config, + networkDevice, + infrav1.DefaultNetworkDevice, + infrav1.IPV4Format) { + // skip device ipv4 has DHCP enabled. + addresses[infrav1.DefaultNetworkDevice] = infrav1.IPAddress{ + IPV4: "DHCP", + } + } else { + ip, err := handleIPAddressForDevice(ctx, machineScope, infrav1.DefaultNetworkDevice, infrav1.IPV4Format, nil) + if err != nil || ip == "" { + return true, err + } + addresses[infrav1.DefaultNetworkDevice] = infrav1.IPAddress{ + IPV4: ip, + } } } // default network device ipv6. if machineScope.InfraCluster.ProxmoxCluster.Spec.IPv6Config != nil { - ip, err := handleIPAddressForDevice(ctx, machineScope, infrav1alpha1.DefaultNetworkDevice, infrav1alpha1.IPV6Format, nil) - if err != nil || ip == "" { - return true, err + if hasDHCPEnabled(machineScope.InfraCluster.ProxmoxCluster.Spec.IPv6Config, + networkDevice, + infrav1.DefaultNetworkDevice, + infrav1.IPV6Format) { + // skip device ipv6 has DHCP enabled. + addr := addresses[infrav1.DefaultNetworkDevice] + addr.IPV6 = "DHCP" + addresses[infrav1.DefaultNetworkDevice] = addr + } else { + ip, err := handleIPAddressForDevice(ctx, machineScope, infrav1.DefaultNetworkDevice, infrav1.IPV6Format, nil) + if err != nil || ip == "" { + return true, err + } + addr := addresses[infrav1.DefaultNetworkDevice] + addr.IPV6 = ip + addresses[infrav1.DefaultNetworkDevice] = addr } - addr := addresses[infrav1alpha1.DefaultNetworkDevice] - addr.IPV6 = ip - addresses[infrav1alpha1.DefaultNetworkDevice] = addr } return false, nil } -func handleAdditionalDevices(ctx context.Context, machineScope *scope.MachineScope, addresses map[string]infrav1alpha1.IPAddress) (bool, error) { +func handleAdditionalDevices(ctx context.Context, machineScope *scope.MachineScope, addresses map[string]infrav1.IPAddress) (bool, error) { // additional network devices. for _, net := range machineScope.ProxmoxMachine.Spec.Network.AdditionalDevices { if net.IPv4PoolRef != nil { - ip, err := handleIPAddressForDevice(ctx, machineScope, net.Name, infrav1alpha1.IPV4Format, net.IPv4PoolRef) - if err != nil || ip == "" { - return true, errors.Wrapf(err, "unable to handle IPAddress for device %s", net.Name) + // additionalDevices don't rely on the cluster network configuration. + // so we need to check if the device has DHCP enabled only. + if net.DHCP4 { + // skip device ipv4 has DHCP enabled. + addresses[net.Name] = infrav1.IPAddress{ + IPV4: "DHCP", + } + } else { + ip, err := handleIPAddressForDevice(ctx, machineScope, net.Name, infrav1.IPV4Format, net.IPv4PoolRef) + if err != nil || ip == "" { + return true, errors.Wrapf(err, "unable to handle IPAddress for device %s", net.Name) + } + + addresses[net.Name] = infrav1.IPAddress{ + IPV4: ip, + } } - addresses[net.Name] = infrav1alpha1.IPAddress{ - IPV4: ip, - } } if net.IPv6PoolRef != nil { - ip, err := handleIPAddressForDevice(ctx, machineScope, net.Name, infrav1alpha1.IPV6Format, net.IPv6PoolRef) - if err != nil || ip == "" { - return true, errors.Wrapf(err, "unable to handle IPAddress for device %s", net.Name) + // additionalDevices don't rely on the cluster network configuration. + // so we need to check if the device has DHCP enabled only. + if net.DHCP6 { + // skip device ipv4 has DHCP enabled. + addresses[net.Name] = infrav1.IPAddress{ + IPV6: "DHCP", + } + return false, nil + } else { + ip, err := handleIPAddressForDevice(ctx, machineScope, net.Name, infrav1.IPV6Format, net.IPv6PoolRef) + if err != nil || ip == "" { + return true, errors.Wrapf(err, "unable to handle IPAddress for device %s", net.Name) + } + + addresses[net.Name] = infrav1.IPAddress{ + IPV6: ip, + } } - addresses[net.Name] = infrav1alpha1.IPAddress{ - IPV6: ip, - } } } @@ -178,3 +225,22 @@ func handleAdditionalDevices(ctx context.Context, machineScope *scope.MachineSco func isIPV4(ip string) bool { return netip.MustParseAddr(ip).Is4() } + +func hasDHCPEnabled(config *infrav1.IPConfig, device *infrav1.NetworkDevice, deviceName, format string) bool { + switch { + case deviceName == infrav1.DefaultNetworkDevice: + if format == infrav1.IPV4Format { + return (config != nil && config.DHCP) || (device != nil && device.DHCP4) + } else if format == infrav1.IPV6Format { + return (config != nil && config.DHCP) || (device != nil && device.DHCP6) + } + default: + // additionalDevices don't rely on the cluster network configuration. + if format == infrav1.IPV4Format { + return device != nil && device.DHCP4 + } else if format == infrav1.IPV6Format { + return device != nil && device.DHCP6 + } + } + return false +} diff --git a/internal/service/vmservice/ip_test.go b/internal/service/vmservice/ip_test.go index 6c40792a..d6297e1d 100644 --- a/internal/service/vmservice/ip_test.go +++ b/internal/service/vmservice/ip_test.go @@ -20,11 +20,12 @@ import ( "context" "testing" + "k8s.io/utils/ptr" + "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" - infrav1alpha1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1" - ipamicv1 "sigs.k8s.io/cluster-api-ipam-provider-in-cluster/api/v1alpha2" + infrav1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1" ) const ipTag = "ip_net0_10.10.10.10" @@ -35,26 +36,32 @@ func TestReconcileIPAddresses_CreateDefaultClaim(t *testing.T) { requeue, err := reconcileIPAddresses(context.Background(), machineScope) require.NoError(t, err) require.True(t, requeue) - requireConditionIsFalse(t, machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition) + requireConditionIsFalse(t, machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition) } func TestReconcileIPAddresses_CreateAdditionalClaim(t *testing.T) { machineScope, _, kubeClient := setupReconcilerTest(t) - machineScope.ProxmoxMachine.Spec.Network = &infrav1alpha1.NetworkSpec{ - AdditionalDevices: []infrav1alpha1.AdditionalNetworkDevice{ - {Name: "net1", IPv4PoolRef: &corev1.TypedLocalObjectReference{Kind: "InClusterIPPool", Name: "custom"}}, + machineScope.ProxmoxMachine.Spec.Network = &infrav1.NetworkSpec{ + AdditionalDevices: []infrav1.AdditionalNetworkDevice{ + { + Name: "net1", + NetworkDevice: &infrav1.NetworkDevice{ + Model: ptr.To("virtio"), + Bridge: "vmbr0", + }, + IPv4PoolRef: &corev1.TypedLocalObjectReference{Kind: "InClusterIPPool", Name: "custom"}}, }, } vm := newStoppedVM() vm.VirtualMachineConfig.Tags = ipTag machineScope.SetVirtualMachine(vm) - createIP4AddressResource(t, kubeClient, machineScope, infrav1alpha1.DefaultNetworkDevice, "10.10.10.10") + createIP4AddressResource(t, kubeClient, machineScope, infrav1.DefaultNetworkDevice, "10.10.10.10") createIPPools(t, kubeClient, machineScope) requeue, err := reconcileIPAddresses(context.Background(), machineScope) require.NoError(t, err) require.True(t, requeue) - requireConditionIsFalse(t, machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition) + requireConditionIsFalse(t, machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition) } func TestReconcileIPAddresses_AddIPTag(t *testing.T) { @@ -62,42 +69,48 @@ func TestReconcileIPAddresses_AddIPTag(t *testing.T) { vm := newStoppedVM() task := newTask() machineScope.SetVirtualMachine(vm) - createIP4AddressResource(t, kubeClient, machineScope, infrav1alpha1.DefaultNetworkDevice, "10.10.10.10") + createIP4AddressResource(t, kubeClient, machineScope, infrav1.DefaultNetworkDevice, "10.10.10.10") proxmoxClient.EXPECT().TagVM(context.TODO(), vm, ipTag).Return(task, nil).Once() requeue, err := reconcileIPAddresses(context.Background(), machineScope) require.NoError(t, err) require.True(t, requeue) - requireConditionIsFalse(t, machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition) + requireConditionIsFalse(t, machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition) } func TestReconcileIPAddresses_SetIPAddresses(t *testing.T) { machineScope, _, kubeClient := setupReconcilerTest(t) - machineScope.ProxmoxMachine.Spec.Network = &infrav1alpha1.NetworkSpec{ - AdditionalDevices: []infrav1alpha1.AdditionalNetworkDevice{ - {Name: "net1", IPv4PoolRef: &corev1.TypedLocalObjectReference{Kind: "GlobalInClusterIPPool", Name: "custom"}}, + machineScope.ProxmoxMachine.Spec.Network = &infrav1.NetworkSpec{ + AdditionalDevices: []infrav1.AdditionalNetworkDevice{ + { + Name: "net1", + NetworkDevice: &infrav1.NetworkDevice{ + Model: ptr.To("virtio"), + Bridge: "vmbr0", + }, + IPv4PoolRef: &corev1.TypedLocalObjectReference{Kind: "GlobalInClusterIPPool", Name: "custom"}}, }, } vm := newStoppedVM() vm.VirtualMachineConfig.Tags = ipTag machineScope.SetVirtualMachine(vm) - createIP4AddressResource(t, kubeClient, machineScope, infrav1alpha1.DefaultNetworkDevice, "10.10.10.10") + createIP4AddressResource(t, kubeClient, machineScope, infrav1.DefaultNetworkDevice, "10.10.10.10") createIP4AddressResource(t, kubeClient, machineScope, "net1", "10.100.10.10") createIPPools(t, kubeClient, machineScope) requeue, err := reconcileIPAddresses(context.Background(), machineScope) require.NoError(t, err) require.True(t, requeue) - requireConditionIsFalse(t, machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition) + requireConditionIsFalse(t, machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition) } func TestReconcileIPAddresses_MultipleDevices(t *testing.T) { machineScope, _, kubeClient := setupReconcilerTest(t) - machineScope.ProxmoxMachine.Spec.Network = &infrav1alpha1.NetworkSpec{ - AdditionalDevices: []infrav1alpha1.AdditionalNetworkDevice{ - {Name: "net1", IPv4PoolRef: &corev1.TypedLocalObjectReference{Kind: "GlobalInClusterIPPool", Name: "ipv4pool"}}, - {Name: "net2", IPv6PoolRef: &corev1.TypedLocalObjectReference{Kind: "GlobalInClusterIPPool", Name: "ipv6pool"}}, + machineScope.ProxmoxMachine.Spec.Network = &infrav1.NetworkSpec{ + AdditionalDevices: []infrav1.AdditionalNetworkDevice{ + {Name: "net1", NetworkDevice: &infrav1.NetworkDevice{Bridge: "vmbr0"}, IPv4PoolRef: &corev1.TypedLocalObjectReference{Kind: "GlobalInClusterIPPool", Name: "ipv4pool"}}, + {Name: "net2", NetworkDevice: &infrav1.NetworkDevice{Bridge: "vmbr0"}, IPv6PoolRef: &corev1.TypedLocalObjectReference{Kind: "GlobalInClusterIPPool", Name: "ipv6pool"}}, }, } @@ -105,7 +118,7 @@ func TestReconcileIPAddresses_MultipleDevices(t *testing.T) { vm.VirtualMachineConfig.Tags = ipTag machineScope.SetVirtualMachine(vm) - createIP4AddressResource(t, kubeClient, machineScope, infrav1alpha1.DefaultNetworkDevice, "10.10.10.10") + createIP4AddressResource(t, kubeClient, machineScope, infrav1.DefaultNetworkDevice, "10.10.10.10") createIP4AddressResource(t, kubeClient, machineScope, "net1", "10.100.10.10") createIP6AddressResource(t, kubeClient, machineScope, "net2", "fe80::ffee") createIPPools(t, kubeClient, machineScope) @@ -115,38 +128,38 @@ func TestReconcileIPAddresses_MultipleDevices(t *testing.T) { require.True(t, requeue) require.Len(t, machineScope.ProxmoxMachine.Status.IPAddresses, 3) - expected := map[string]infrav1alpha1.IPAddress{ + expected := map[string]infrav1.IPAddress{ "net0": {IPV4: "10.10.10.10"}, "net1": {IPV4: "10.100.10.10"}, "net2": {IPV6: "fe80::ffee"}, } require.Equal(t, expected, machineScope.ProxmoxMachine.Status.IPAddresses) - requireConditionIsFalse(t, machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition) + requireConditionIsFalse(t, machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition) } func TestReconcileIPAddresses_IPV6(t *testing.T) { machineScope, _, kubeClient := setupReconcilerTest(t) - machineScope.InfraCluster.ProxmoxCluster.Spec.IPv6Config = &ipamicv1.InClusterIPPoolSpec{ + machineScope.InfraCluster.ProxmoxCluster.Spec.IPv6Config = &infrav1.IPConfig{ Addresses: []string{"fe80::/64"}, Prefix: 64, Gateway: "fe80::1", } - machineScope.ProxmoxMachine.Spec.Network = &infrav1alpha1.NetworkSpec{ - AdditionalDevices: []infrav1alpha1.AdditionalNetworkDevice{ - {Name: "net1", IPv4PoolRef: &corev1.TypedLocalObjectReference{Kind: "GlobalInClusterIPPool", Name: "custom"}}, + machineScope.ProxmoxMachine.Spec.Network = &infrav1.NetworkSpec{ + AdditionalDevices: []infrav1.AdditionalNetworkDevice{ + {Name: "net1", NetworkDevice: &infrav1.NetworkDevice{Bridge: "vmbr0"}, IPv4PoolRef: &corev1.TypedLocalObjectReference{Kind: "GlobalInClusterIPPool", Name: "custom"}}, }, } vm := newStoppedVM() vm.VirtualMachineConfig.Tags = ipTag machineScope.SetVirtualMachine(vm) - createIP4AddressResource(t, kubeClient, machineScope, infrav1alpha1.DefaultNetworkDevice, "10.10.10.10") - createIP6AddressResource(t, kubeClient, machineScope, infrav1alpha1.DefaultNetworkDevice, "fe80::1") + createIP4AddressResource(t, kubeClient, machineScope, infrav1.DefaultNetworkDevice, "10.10.10.10") + createIP6AddressResource(t, kubeClient, machineScope, infrav1.DefaultNetworkDevice, "fe80::1") createIP4AddressResource(t, kubeClient, machineScope, "net1", "10.100.10.10") createIPPools(t, kubeClient, machineScope) requeue, err := reconcileIPAddresses(context.Background(), machineScope) require.NoError(t, err) require.True(t, requeue) - requireConditionIsFalse(t, machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition) + requireConditionIsFalse(t, machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition) } diff --git a/internal/service/vmservice/utils_test.go b/internal/service/vmservice/utils_test.go index 119401d0..221ee3ee 100644 --- a/internal/service/vmservice/utils_test.go +++ b/internal/service/vmservice/utils_test.go @@ -120,7 +120,7 @@ func TestShouldUpdateNetworkDevices_MissingAdditionalDeviceOnVM(t *testing.T) { machineScope, _, _ := setupReconcilerTest(t) machineScope.ProxmoxMachine.Spec.Network = &infrav1alpha1.NetworkSpec{ AdditionalDevices: []infrav1alpha1.AdditionalNetworkDevice{ - {Name: "net1", NetworkDevice: infrav1alpha1.NetworkDevice{Bridge: "vmbr1", Model: ptr.To("virtio")}}, + {Name: "net1", NetworkDevice: &infrav1alpha1.NetworkDevice{Bridge: "vmbr1", Model: ptr.To("virtio")}}, }, } machineScope.SetVirtualMachine(newVMWithNets("virtio=A6:23:64:4D:84:CB,bridge=vmbr0")) @@ -132,7 +132,7 @@ func TestShouldUpdateNetworkDevices_AdditionalDeviceNeedsUpdate(t *testing.T) { machineScope, _, _ := setupReconcilerTest(t) machineScope.ProxmoxMachine.Spec.Network = &infrav1alpha1.NetworkSpec{ AdditionalDevices: []infrav1alpha1.AdditionalNetworkDevice{ - {Name: "net1", NetworkDevice: infrav1alpha1.NetworkDevice{Bridge: "vmbr1", Model: ptr.To("virtio")}}, + {Name: "net1", NetworkDevice: &infrav1alpha1.NetworkDevice{Bridge: "vmbr1", Model: ptr.To("virtio")}}, }, } machineScope.SetVirtualMachine(newVMWithNets("", "virtio=A6:23:64:4D:84:CB,bridge=vmbr0")) @@ -145,7 +145,7 @@ func TestShouldUpdateNetworkDevices_NoUpdate(t *testing.T) { machineScope.ProxmoxMachine.Spec.Network = &infrav1alpha1.NetworkSpec{ Default: &infrav1alpha1.NetworkDevice{Bridge: "vmbr0", Model: ptr.To("virtio")}, AdditionalDevices: []infrav1alpha1.AdditionalNetworkDevice{ - {Name: "net1", NetworkDevice: infrav1alpha1.NetworkDevice{Bridge: "vmbr1", Model: ptr.To("virtio")}}, + {Name: "net1", NetworkDevice: &infrav1alpha1.NetworkDevice{Bridge: "vmbr1", Model: ptr.To("virtio")}}, }, } machineScope.SetVirtualMachine(newVMWithNets("virtio=A6:23:64:4D:84:CD,bridge=vmbr0", "virtio=A6:23:64:4D:84:CD,bridge=vmbr1")) diff --git a/internal/service/vmservice/vm.go b/internal/service/vmservice/vm.go index 978c301d..788fa08b 100644 --- a/internal/service/vmservice/vm.go +++ b/internal/service/vmservice/vm.go @@ -19,6 +19,7 @@ package vmservice import ( "context" + "strings" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -86,7 +87,7 @@ func ReconcileVM(ctx context.Context, scope *scope.MachineScope) (infrav1alpha1. return vm, err } - if err := reconcileMachineAddresses(scope); err != nil { + if err := reconcileMachineAddresses(ctx, scope); err != nil { return vm, err } @@ -220,8 +221,8 @@ func reconcileVirtualMachineConfig(ctx context.Context, machineScope *scope.Mach return true, nil } -func reconcileMachineAddresses(scope *scope.MachineScope) error { - addr, err := getMachineAddresses(scope) +func reconcileMachineAddresses(ctx context.Context, scope *scope.MachineScope) error { + addr, err := getMachineAddresses(ctx, scope) if err != nil { scope.Error(err, "failed to retrieve machine addresses") return err @@ -231,7 +232,7 @@ func reconcileMachineAddresses(scope *scope.MachineScope) error { return nil } -func getMachineAddresses(scope *scope.MachineScope) ([]clusterv1.MachineAddress, error) { +func getMachineAddresses(ctx context.Context, scope *scope.MachineScope) ([]clusterv1.MachineAddress, error) { if !machineHasIPAddress(scope.ProxmoxMachine) { return nil, errors.New("machine does not yet have an ip address") } @@ -247,21 +248,73 @@ func getMachineAddresses(scope *scope.MachineScope) ([]clusterv1.MachineAddress, }, } - if scope.InfraCluster.ProxmoxCluster.Spec.IPv4Config != nil { - addresses = append(addresses, clusterv1.MachineAddress{ + networkSpec := scope.ProxmoxMachine.Spec.Network + if networkSpec == nil { + networkSpec = &infrav1alpha1.NetworkSpec{ + Default: &infrav1alpha1.NetworkDevice{}, + } + } + var ipv4, ipv6 string + // get ip addresses from vm if dhcp is enabled + if hasDHCPEnabled(scope.InfraCluster.ProxmoxCluster.Spec.IPv4Config, + networkSpec.Default, + infrav1alpha1.DefaultNetworkDevice, + infrav1alpha1.IPV4Format) || + hasDHCPEnabled(scope.InfraCluster.ProxmoxCluster.Spec.IPv6Config, + networkSpec.Default, + infrav1alpha1.DefaultNetworkDevice, + infrav1alpha1.IPV6Format) { + + nets, err := scope.InfraCluster.ProxmoxClient.GetVMNetwork(ctx, scope.VirtualMachine) + if err != nil { + return nil, errors.Wrapf(err, "unable to get network interfaces for vm %s", scope.ProxmoxMachine.GetName()) + } + + for _, net := range nets { + // default device + mac := extractMACAddress(scope.VirtualMachine.VirtualMachineConfig.Net0) + if strings.EqualFold(mac, net.HardwareAddress) { + for _, ip := range net.IPAddresses { + if ip.IPAddressType == "ipv4" && ip.IPAddress != scope.InfraCluster.ProxmoxCluster.Spec.ControlPlaneEndpoint.Host { + ipv4 = ip.IPAddress + } else if ip.IPAddressType == "ipv6" && !strings.Contains(ip.IPAddress, "fe80:") { + ipv6 = ip.IPAddress + } + } + } + + } + + setMachineAddresses(scope, &addresses, ipv4, ipv6) + return addresses, nil + } else { + if scope.InfraCluster.ProxmoxCluster.Spec.IPv4Config != nil { + ipv4 = scope.ProxmoxMachine.Status.IPAddresses[infrav1alpha1.DefaultNetworkDevice].IPV4 + } + + if scope.InfraCluster.ProxmoxCluster.Spec.IPv6Config != nil { + ipv6 = scope.ProxmoxMachine.Status.IPAddresses[infrav1alpha1.DefaultNetworkDevice].IPV6 + } + } + + setMachineAddresses(scope, &addresses, ipv4, ipv6) + return addresses, nil +} + +func setMachineAddresses(machineScope *scope.MachineScope, addresses *[]clusterv1.MachineAddress, ipv4, ipv6 string) { + if machineScope.InfraCluster.ProxmoxCluster.Spec.IPv4Config != nil && ipv4 != "" { + *addresses = append(*addresses, clusterv1.MachineAddress{ Type: clusterv1.MachineInternalIP, - Address: scope.ProxmoxMachine.Status.IPAddresses[infrav1alpha1.DefaultNetworkDevice].IPV4, + Address: ipv4, }) } - if scope.InfraCluster.ProxmoxCluster.Spec.IPv6Config != nil { - addresses = append(addresses, clusterv1.MachineAddress{ + if machineScope.InfraCluster.ProxmoxCluster.Spec.IPv6Config != nil && ipv6 != "" { + *addresses = append(*addresses, clusterv1.MachineAddress{ Type: clusterv1.MachineInternalIP, - Address: scope.ProxmoxMachine.Status.IPAddresses[infrav1alpha1.DefaultNetworkDevice].IPV6, + Address: ipv6, }) } - - return addresses, nil } func createVM(ctx context.Context, scope *scope.MachineScope) (proxmox.VMCloneResponse, error) { diff --git a/internal/service/vmservice/vm_test.go b/internal/service/vmservice/vm_test.go index 3965c295..45f2d335 100644 --- a/internal/service/vmservice/vm_test.go +++ b/internal/service/vmservice/vm_test.go @@ -24,32 +24,37 @@ import ( "github.com/stretchr/testify/require" "k8s.io/utils/ptr" - infrav1alpha1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1" + infrav1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1" "github.com/ionos-cloud/cluster-api-provider-proxmox/internal/service/scheduler" "github.com/ionos-cloud/cluster-api-provider-proxmox/pkg/proxmox" "github.com/ionos-cloud/cluster-api-provider-proxmox/pkg/scope" - ipamicv1 "sigs.k8s.io/cluster-api-ipam-provider-in-cluster/api/v1alpha2" ) func TestReconcileVM_EverythingReady(t *testing.T) { machineScope, proxmoxClient, _ := setupReconcilerTest(t) vm := newRunningVM() machineScope.SetVirtualMachineID(int64(vm.VMID)) - machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1alpha1.IPAddress{infrav1alpha1.DefaultNetworkDevice: {IPV4: "10.10.10.10"}} + machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1.IPAddress{infrav1.DefaultNetworkDevice: {IPV4: "10.10.10.10"}} machineScope.ProxmoxMachine.Status.BootstrapDataProvided = ptr.To(true) + machineScope.ProxmoxMachine.Spec.Network = &infrav1.NetworkSpec{ + Default: &infrav1.NetworkDevice{ + Bridge: "vmbr1", + Model: ptr.To("virtio"), + }, + } - proxmoxClient.EXPECT().GetVM(context.TODO(), "node1", int64(123)).Return(vm, nil).Once() + proxmoxClient.EXPECT().GetVM(context.Background(), "node1", int64(123)).Return(vm, nil).Once() result, err := ReconcileVM(context.Background(), machineScope) require.NoError(t, err) - require.Equal(t, infrav1alpha1.VirtualMachineStateReady, result.State) + require.Equal(t, infrav1.VirtualMachineStateReady, result.State) require.Equal(t, "10.10.10.10", machineScope.ProxmoxMachine.Status.Addresses[1].Address) } func TestEnsureVirtualMachine_CreateVM_FullOptions(t *testing.T) { machineScope, proxmoxClient, _ := setupReconcilerTest(t) machineScope.ProxmoxMachine.Spec.Description = ptr.To("test vm") - machineScope.ProxmoxMachine.Spec.Format = ptr.To(infrav1alpha1.TargetStorageFormatRaw) + machineScope.ProxmoxMachine.Spec.Format = ptr.To(infrav1.TargetStorageFormatRaw) machineScope.ProxmoxMachine.Spec.Full = ptr.To(true) machineScope.ProxmoxMachine.Spec.Pool = ptr.To("pool") machineScope.ProxmoxMachine.Spec.SnapName = ptr.To("snap") @@ -75,7 +80,7 @@ func TestEnsureVirtualMachine_CreateVM_FullOptions(t *testing.T) { require.Equal(t, "node2", *machineScope.ProxmoxMachine.Status.ProxmoxNode) require.True(t, machineScope.InfraCluster.ProxmoxCluster.HasMachine(machineScope.Name(), false)) - requireConditionIsFalse(t, machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition) + requireConditionIsFalse(t, machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition) } func TestEnsureVirtualMachine_CreateVM_SelectNode(t *testing.T) { @@ -97,7 +102,7 @@ func TestEnsureVirtualMachine_CreateVM_SelectNode(t *testing.T) { require.Equal(t, "node3", *machineScope.ProxmoxMachine.Status.ProxmoxNode) require.True(t, machineScope.InfraCluster.ProxmoxCluster.HasMachine(machineScope.Name(), false)) - requireConditionIsFalse(t, machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition) + requireConditionIsFalse(t, machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition) } func TestEnsureVirtualMachine_CreateVM_SelectNode_InsufficientMemory(t *testing.T) { @@ -113,7 +118,7 @@ func TestEnsureVirtualMachine_CreateVM_SelectNode_InsufficientMemory(t *testing. require.Error(t, err) require.False(t, machineScope.InfraCluster.ProxmoxCluster.HasMachine(machineScope.Name(), false)) - requireConditionIsFalse(t, machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition) + requireConditionIsFalse(t, machineScope.ProxmoxMachine, infrav1.VMProvisionedCondition) require.True(t, machineScope.HasFailed()) } @@ -159,12 +164,12 @@ func TestReconcileVirtualMachineConfig_ApplyConfig(t *testing.T) { machineScope.ProxmoxMachine.Spec.NumSockets = 4 machineScope.ProxmoxMachine.Spec.NumCores = 4 machineScope.ProxmoxMachine.Spec.MemoryMiB = 16 * 1024 - machineScope.ProxmoxMachine.Spec.Network = &infrav1alpha1.NetworkSpec{ - Default: &infrav1alpha1.NetworkDevice{Bridge: "vmbr0", Model: ptr.To("virtio")}, - AdditionalDevices: []infrav1alpha1.AdditionalNetworkDevice{ + machineScope.ProxmoxMachine.Spec.Network = &infrav1.NetworkSpec{ + Default: &infrav1.NetworkDevice{Bridge: "vmbr0", Model: ptr.To("virtio")}, + AdditionalDevices: []infrav1.AdditionalNetworkDevice{ { Name: "net1", - NetworkDevice: infrav1alpha1.NetworkDevice{Bridge: "vmbr1", Model: ptr.To("virtio")}, + NetworkDevice: &infrav1.NetworkDevice{Bridge: "vmbr1", Model: ptr.To("virtio")}, }, }, } @@ -190,8 +195,8 @@ func TestReconcileVirtualMachineConfig_ApplyConfig(t *testing.T) { func TestReconcileDisks_RunningVM(t *testing.T) { machineScope, _, _ := setupReconcilerTest(t) - machineScope.ProxmoxMachine.Spec.Disks = &infrav1alpha1.Storage{ - BootVolume: &infrav1alpha1.DiskSize{Disk: "ide0", SizeGB: 100}, + machineScope.ProxmoxMachine.Spec.Disks = &infrav1.Storage{ + BootVolume: &infrav1.DiskSize{Disk: "ide0", SizeGB: 100}, } machineScope.SetVirtualMachine(newRunningVM()) @@ -200,8 +205,8 @@ func TestReconcileDisks_RunningVM(t *testing.T) { func TestReconcileDisks_ResizeDisk(t *testing.T) { machineScope, proxmoxClient, _ := setupReconcilerTest(t) - machineScope.ProxmoxMachine.Spec.Disks = &infrav1alpha1.Storage{ - BootVolume: &infrav1alpha1.DiskSize{Disk: "ide0", SizeGB: 100}, + machineScope.ProxmoxMachine.Spec.Disks = &infrav1.Storage{ + BootVolume: &infrav1.DiskSize{Disk: "ide0", SizeGB: 100}, } vm := newStoppedVM() machineScope.SetVirtualMachine(vm) @@ -216,10 +221,10 @@ func TestReconcileMachineAddresses_IPV4(t *testing.T) { vm := newRunningVM() machineScope.SetVirtualMachine(vm) machineScope.SetVirtualMachineID(int64(vm.VMID)) - machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1alpha1.IPAddress{infrav1alpha1.DefaultNetworkDevice: {IPV4: "10.10.10.10"}} + machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1.IPAddress{infrav1.DefaultNetworkDevice: {IPV4: "10.10.10.10"}} machineScope.ProxmoxMachine.Status.BootstrapDataProvided = ptr.To(true) - require.NoError(t, reconcileMachineAddresses(machineScope)) + require.NoError(t, reconcileMachineAddresses(context.Background(), machineScope)) require.Equal(t, machineScope.ProxmoxMachine.Status.Addresses[0].Address, machineScope.ProxmoxMachine.GetName()) require.Equal(t, machineScope.ProxmoxMachine.Status.Addresses[1].Address, "10.10.10.10") } @@ -227,7 +232,7 @@ func TestReconcileMachineAddresses_IPV4(t *testing.T) { func TestReconcileMachineAddresses_IPV6(t *testing.T) { machineScope, _, _ := setupReconcilerTest(t) machineScope.InfraCluster.ProxmoxCluster.Spec.IPv4Config = nil - machineScope.InfraCluster.ProxmoxCluster.Spec.IPv6Config = &ipamicv1.InClusterIPPoolSpec{ + machineScope.InfraCluster.ProxmoxCluster.Spec.IPv6Config = &infrav1.IPConfig{ Addresses: []string{"2001:db8::/64"}, Prefix: 64, Gateway: "2001:db8::1", @@ -236,17 +241,17 @@ func TestReconcileMachineAddresses_IPV6(t *testing.T) { vm := newRunningVM() machineScope.SetVirtualMachine(vm) machineScope.SetVirtualMachineID(int64(vm.VMID)) - machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1alpha1.IPAddress{infrav1alpha1.DefaultNetworkDevice: {IPV6: "2001:db8::2"}} + machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1.IPAddress{infrav1.DefaultNetworkDevice: {IPV6: "2001:db8::2"}} machineScope.ProxmoxMachine.Status.BootstrapDataProvided = ptr.To(true) - require.NoError(t, reconcileMachineAddresses(machineScope)) + require.NoError(t, reconcileMachineAddresses(context.Background(), machineScope)) require.Equal(t, machineScope.ProxmoxMachine.Status.Addresses[0].Address, machineScope.ProxmoxMachine.GetName()) require.Equal(t, machineScope.ProxmoxMachine.Status.Addresses[1].Address, "2001:db8::2") } func TestReconcileMachineAddresses_DualStack(t *testing.T) { machineScope, _, _ := setupReconcilerTest(t) - machineScope.InfraCluster.ProxmoxCluster.Spec.IPv6Config = &ipamicv1.InClusterIPPoolSpec{ + machineScope.InfraCluster.ProxmoxCluster.Spec.IPv6Config = &infrav1.IPConfig{ Addresses: []string{"2001:db8::/64"}, Prefix: 64, Gateway: "2001:db8::1", @@ -255,10 +260,10 @@ func TestReconcileMachineAddresses_DualStack(t *testing.T) { vm := newRunningVM() machineScope.SetVirtualMachine(vm) machineScope.SetVirtualMachineID(int64(vm.VMID)) - machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1alpha1.IPAddress{infrav1alpha1.DefaultNetworkDevice: {IPV4: "10.10.10.10", IPV6: "2001:db8::2"}} + machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1.IPAddress{infrav1.DefaultNetworkDevice: {IPV4: "10.10.10.10", IPV6: "2001:db8::2"}} machineScope.ProxmoxMachine.Status.BootstrapDataProvided = ptr.To(true) - require.NoError(t, reconcileMachineAddresses(machineScope)) + require.NoError(t, reconcileMachineAddresses(context.Background(), machineScope)) require.Equal(t, machineScope.ProxmoxMachine.Status.Addresses[0].Address, machineScope.ProxmoxMachine.GetName()) require.Equal(t, machineScope.ProxmoxMachine.Status.Addresses[1].Address, "10.10.10.10") require.Equal(t, machineScope.ProxmoxMachine.Status.Addresses[2].Address, "2001:db8::2") diff --git a/internal/webhook/proxmoxcluster_webhook_test.go b/internal/webhook/proxmoxcluster_webhook_test.go index 60d3a087..b5b88724 100644 --- a/internal/webhook/proxmoxcluster_webhook_test.go +++ b/internal/webhook/proxmoxcluster_webhook_test.go @@ -23,7 +23,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - ipamicv1 "sigs.k8s.io/cluster-api-ipam-provider-in-cluster/api/v1alpha2" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -58,7 +57,7 @@ var _ = Describe("Controller Test", func() { It("should disallow invalid IPV6 IPs", func() { cluster := validProxmoxCluster("test-cluster") - cluster.Spec.IPv6Config = &ipamicv1.InClusterIPPoolSpec{ + cluster.Spec.IPv6Config = &infrav1.IPConfig{ Addresses: []string{"invalid"}, Prefix: 64, Gateway: "2001:db8::1", @@ -69,7 +68,7 @@ var _ = Describe("Controller Test", func() { It("should disallow endpoint IP to intersect with node IPs", func() { cluster := invalidProxmoxCluster("test-cluster") cluster.Spec.ControlPlaneEndpoint.Host = "[2001:db8::1]" - cluster.Spec.IPv6Config = &ipamicv1.InClusterIPPoolSpec{ + cluster.Spec.IPv6Config = &infrav1.IPConfig{ Addresses: []string{"2001:db8::/64"}, Prefix: 64, Gateway: "2001:db8::1", @@ -109,14 +108,16 @@ func validProxmoxCluster(name string) infrav1.ProxmoxCluster { Host: "10.10.10.1", Port: 6443, }, - IPv4Config: &ipamicv1.InClusterIPPoolSpec{ - Addresses: []string{ - "10.10.10.2-10.10.10.10", + ClusterNetworkConfig: infrav1.ClusterNetworkConfig{ + IPv4Config: &infrav1.IPConfig{ + Addresses: []string{ + "10.10.10.2-10.10.10.10", + }, + Gateway: "10.10.10.1", + Prefix: 24, }, - Gateway: "10.10.10.1", - Prefix: 24, + DNSServers: []string{"8.8.8.8", "8.8.4.4"}, }, - DNSServers: []string{"8.8.8.8", "8.8.4.4"}, }, } } diff --git a/pkg/kubernetes/ipam/ipam.go b/pkg/kubernetes/ipam/ipam.go index be7f20e3..87a2e3e1 100644 --- a/pkg/kubernetes/ipam/ipam.go +++ b/pkg/kubernetes/ipam/ipam.go @@ -67,7 +67,7 @@ var ErrMissingAddresses = errors.New("no valid ip addresses defined for the ip p // by Proxmox in order to avoid conflicts. func (h *Helper) CreateOrUpdateInClusterIPPool(ctx context.Context) error { // ipv4 - if h.cluster.Spec.IPv4Config != nil { + if h.cluster.Spec.IPv4Config != nil && !h.cluster.Spec.IPv4Config.DHCP { ipv4Config := h.cluster.Spec.IPv4Config v4Pool := &ipamicv1.InClusterIPPool{ @@ -94,7 +94,7 @@ func (h *Helper) CreateOrUpdateInClusterIPPool(ctx context.Context) error { } // ipv6 - if h.cluster.Spec.IPv6Config != nil { + if h.cluster.Spec.IPv6Config != nil && !h.cluster.Spec.IPv6Config.DHCP { v6Pool := &ipamicv1.InClusterIPPool{ ObjectMeta: metav1.ObjectMeta{ Name: InClusterPoolFormat(h.cluster, infrav1.IPV6Format), diff --git a/pkg/kubernetes/ipam/ipam_test.go b/pkg/kubernetes/ipam/ipam_test.go index c1cbedf0..d53ac427 100644 --- a/pkg/kubernetes/ipam/ipam_test.go +++ b/pkg/kubernetes/ipam/ipam_test.go @@ -104,7 +104,7 @@ func (s *IPAMTestSuite) Test_CreateOrUpdateInClusterIPPool() { s.Equal(ipamConfig.Gateway, pool.Spec.Gateway) // ipv6 - s.cluster.Spec.IPv6Config = &ipamicv1.InClusterIPPoolSpec{ + s.cluster.Spec.IPv6Config = &infrav1.IPConfig{ Addresses: []string{"2001:db8::/64"}, Prefix: 64, Gateway: "2001:db8::1", @@ -141,7 +141,7 @@ func (s *IPAMTestSuite) Test_GetDefaultInClusterIPPool() { s.Equal(&pool, found) // ipv6 - s.cluster.Spec.IPv6Config = &ipamicv1.InClusterIPPoolSpec{ + s.cluster.Spec.IPv6Config = &infrav1.IPConfig{ Addresses: []string{"2001:db8::/64"}, Prefix: 64, Gateway: "2001:db8::1", @@ -303,7 +303,7 @@ func (s *IPAMTestSuite) Test_CreateIPAddressClaim() { s.NoError(err) // IPV6. - s.cluster.Spec.IPv6Config = &ipamicv1.InClusterIPPoolSpec{ + s.cluster.Spec.IPv6Config = &infrav1.IPConfig{ Addresses: []string{"2001:db8::/64"}, Prefix: 64, Gateway: "2001:db8::1", @@ -356,10 +356,12 @@ func getCluster() *infrav1.ProxmoxCluster { Namespace: "test", }, Spec: infrav1.ProxmoxClusterSpec{ - IPv4Config: &ipamicv1.InClusterIPPoolSpec{ - Addresses: []string{"10.10.0.1/24"}, - Gateway: "10.0.0.0", - Prefix: 24, + ClusterNetworkConfig: infrav1.ClusterNetworkConfig{ + IPv4Config: &infrav1.IPConfig{ + Addresses: []string{"10.10.0.1/24"}, + Gateway: "10.0.0.0", + Prefix: 24, + }, }, }, } diff --git a/pkg/proxmox/client.go b/pkg/proxmox/client.go index 3877e662..548bb8d3 100644 --- a/pkg/proxmox/client.go +++ b/pkg/proxmox/client.go @@ -46,4 +46,6 @@ type Client interface { StartVM(ctx context.Context, vm *proxmox.VirtualMachine) (*proxmox.Task, error) TagVM(ctx context.Context, vm *proxmox.VirtualMachine, tag string) (*proxmox.Task, error) + + GetVMNetwork(ctx context.Context, vm *proxmox.VirtualMachine) (iFaces []*proxmox.AgentNetworkIface, err error) } diff --git a/pkg/proxmox/goproxmox/api_client.go b/pkg/proxmox/goproxmox/api_client.go index b4ad09b4..5ba9ef5a 100644 --- a/pkg/proxmox/goproxmox/api_client.go +++ b/pkg/proxmox/goproxmox/api_client.go @@ -241,3 +241,13 @@ func (c *APIClient) StartVM(ctx context.Context, vm *proxmox.VirtualMachine) (*p func (c *APIClient) TagVM(ctx context.Context, vm *proxmox.VirtualMachine, tag string) (*proxmox.Task, error) { return vm.AddTag(ctx, tag) } + +// GetVMNetwork returns a VM network interfaces based on nodeName and vmID. +func (c *APIClient) GetVMNetwork(ctx context.Context, vm *proxmox.VirtualMachine) (iFaces []*proxmox.AgentNetworkIface, err error) { + networkInterfaces, err := vm.AgentGetNetworkIFaces(ctx) + if err != nil { + return nil, fmt.Errorf("cannot get network interfaces for vm with id %d: %w", vm.VMID, err) + } + + return networkInterfaces, nil +} diff --git a/pkg/proxmox/proxmoxtest/mock_client.go b/pkg/proxmox/proxmoxtest/mock_client.go index 24477f46..2ce401c4 100644 --- a/pkg/proxmox/proxmoxtest/mock_client.go +++ b/pkg/proxmox/proxmoxtest/mock_client.go @@ -19,6 +19,7 @@ package proxmoxtest import ( "context" + go_proxmox "github.com/luthermonson/go-proxmox" mock "github.com/stretchr/testify/mock" @@ -637,6 +638,61 @@ func (_c *MockClient_TagVM_Call) RunAndReturn(run func(context.Context, *go_prox return _c } +// GetVMNetwork returns a VM network interfaces based on nodeName and vmID. +func (_m *MockClient) GetVMNetwork(ctx context.Context, vm *go_proxmox.VirtualMachine) ([]*go_proxmox.AgentNetworkIface, error) { + ret := _m.Called(ctx, vm) + + var r0 []*go_proxmox.AgentNetworkIface + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *go_proxmox.VirtualMachine) ([]*go_proxmox.AgentNetworkIface, error)); ok { + return rf(ctx, vm) + } + if rf, ok := ret.Get(0).(func(context.Context, *go_proxmox.VirtualMachine) []*go_proxmox.AgentNetworkIface); ok { + r0 = rf(ctx, vm) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*go_proxmox.AgentNetworkIface) + } + } + + if rf, ok := ret.Get(1).(func(*go_proxmox.VirtualMachine) error); ok { + r1 = rf(vm) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// MockClient_GetVMNetwork_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetVMNetwork' +type MockClient_GetVMNetwork_Call struct { + *mock.Call +} + +// TagVM is a helper method to define mock.On call +// - vm *go_proxmox.VirtualMachine +// - tag string +func (_e *MockClient_Expecter) GetVMNetwork(ctx context.Context, vm *go_proxmox.VirtualMachine) *MockClient_GetVMNetwork_Call { + return &MockClient_GetVMNetwork_Call{Call: _e.mock.On("GetVMNetwork", ctx, vm)} +} + +func (_c *MockClient_GetVMNetwork_Call) Run(run func(ctx context.Context, vm *go_proxmox.VirtualMachine)) *MockClient_GetVMNetwork_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[0].(*go_proxmox.VirtualMachine)) + }) + return _c +} + +func (_c *MockClient_GetVMNetwork_Call) Return(_a0 []*go_proxmox.AgentNetworkIface, _a1 error) *MockClient_GetVMNetwork_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *MockClient_GetVMNetwork_Call) RunAndReturn(run func(context.Context, *go_proxmox.VirtualMachine) ([]*go_proxmox.AgentNetworkIface, error)) *MockClient_GetVMNetwork_Call { + _c.Call.Return(run) + return _c +} + // NewMockClient creates a new instance of MockClient. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. // The first argument is typically a *testing.T value. func NewMockClient(t interface {