Skip to content

Commit 5f3ba2f

Browse files
rybnicomcbenjemaa
rybnico
and
mcbenjemaa
authored
feat: Add ability to specify range of VM IDs to use (#286)
* Return VMIDFreeErr or the error object itself if CheckID returns true/an error * Rename VMIDFreeErr to ErrVMIDFree and fix comment to make linter happy * feat: Add ability to specify range of VM IDs to use * Fix codespell error: fix spelling * When checking if a vmid is free, first check the existing ProxmoxMachines before querying the Proxmox API. * Check that the vmid of the Proxmox machine is set (not -1) before adding it to usedVMIDs * Move spec.vmidRange from ProxmoxCluster to ProxmoxMachine * Update github.com/luthermonson/go-proxmox to v0.2.0 * Revert "Update github.com/luthermonson/go-proxmox to v0.2.0" This reverts commit c5d15e5. Because of this bug luthermonson/go-proxmox#169 * Update github.com/luthermonson/go-proxmox to v0.2.1 * Add test for ClusterScope.ListProxmoxMachinesForCluster * Fix wording in ProxmoxMachine types test * Rename vmidRange to vmIDRange to follow k8s API conventions * Add validation for vmIDRange: end should be greater than or equal to start * Set failureMessage and failureReason when ErrNoVMIDInRangeFree is thrown * Refactor getVMID to improve code quality --------- Co-authored-by: Mohamed Chiheb Ben Jemaa <mc.benjemaa@gmail.com>
1 parent c1f7482 commit 5f3ba2f

15 files changed

+599
-72
lines changed

‎api/v1alpha1/proxmoxmachine_types.go‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
corev1 "k8s.io/api/core/v1"
2424
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2525
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
26-
"sigs.k8s.io/cluster-api/errors"
26+
clusterapierrors "sigs.k8s.io/cluster-api/errors"
2727
)
2828

2929
const (
@@ -101,6 +101,11 @@ type ProxmoxMachineSpec struct {
101101
// +optional
102102
Network *NetworkSpec `json:"network,omitempty"`
103103

104+
// VMIDRange is the range of VMIDs to use for VMs.
105+
// +optional
106+
// +kubebuilder:validation:XValidation:rule="self.end >= self.start",message="end should be greater than or equal to start"
107+
VMIDRange *VMIDRange `json:"vmIDRange,omitempty"`
108+
104109
Checks *ProxmoxMachineChecks `json:"checks,omitempty"`
105110
}
106111

@@ -434,7 +439,7 @@ type ProxmoxMachineStatus struct {
434439
// can be added as events to the ProxmoxMachine object and/or logged in the
435440
// controller's output.
436441
// +optional
437-
FailureReason *errors.MachineStatusError `json:"failureReason,omitempty"`
442+
FailureReason *clusterapierrors.MachineStatusError `json:"failureReason,omitempty"`
438443

439444
// FailureMessage will be set in the event that there is a terminal problem
440445
// reconciling the Machine and will contain a more verbose string suitable
@@ -471,6 +476,26 @@ type IPAddress struct {
471476
IPV6 string `json:"ipv6,omitempty"`
472477
}
473478

479+
// VMIDRange defines the range of VMIDs to use for VMs.
480+
type VMIDRange struct {
481+
// VMIDRangeStart is the start of the VMID range to use for VMs.
482+
// +kubebuilder:validation:Minimum=100
483+
// +kubebuilder:validation:ExclusiveMinimum=false
484+
// +kubebuilder:validation:Maximum=999999999
485+
// +kubebuilder:validation:ExclusiveMaximum=false
486+
// +kubebuilder:validation:Required
487+
Start int64 `json:"start"`
488+
489+
// VMIDRangeEnd is the end of the VMID range to use for VMs.
490+
// Only used if VMIDRangeStart is set.
491+
// +kubebuilder:validation:Minimum=100
492+
// +kubebuilder:validation:ExclusiveMinimum=false
493+
// +kubebuilder:validation:Maximum=999999999
494+
// +kubebuilder:validation:ExclusiveMaximum=false
495+
// +kubebuilder:validation:Required
496+
End int64 `json:"end"`
497+
}
498+
474499
// +kubebuilder:object:root=true
475500
// +kubebuilder:storageversion
476501
// +kubebuilder:subresource:status

‎api/v1alpha1/proxmoxmachine_types_test.go‎

Lines changed: 100 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -108,16 +108,18 @@ var _ = Describe("ProxmoxMachine Test", func() {
108108
Default: &NetworkDevice{
109109
Bridge: "vmbr0",
110110
},
111-
AdditionalDevices: []AdditionalNetworkDevice{{
112-
NetworkDevice: NetworkDevice{},
113-
Name: "net0",
114-
InterfaceConfig: InterfaceConfig{
115-
IPv4PoolRef: &corev1.TypedLocalObjectReference{
116-
APIGroup: ptr.To("ipam.cluster.x-k8s.io"),
117-
Kind: "InClusterIPPool",
118-
Name: "some-pool",
119-
}},
120-
},
111+
AdditionalDevices: []AdditionalNetworkDevice{
112+
{
113+
NetworkDevice: NetworkDevice{},
114+
Name: "net0",
115+
InterfaceConfig: InterfaceConfig{
116+
IPv4PoolRef: &corev1.TypedLocalObjectReference{
117+
APIGroup: ptr.To("ipam.cluster.x-k8s.io"),
118+
Kind: "InClusterIPPool",
119+
Name: "some-pool",
120+
},
121+
},
122+
},
121123
},
122124
}
123125

@@ -127,15 +129,17 @@ var _ = Describe("ProxmoxMachine Test", func() {
127129
It("Should only allow IPAM pool resources in IPv4PoolRef apiGroup", func() {
128130
dm := defaultMachine()
129131
dm.Spec.Network = &NetworkSpec{
130-
AdditionalDevices: []AdditionalNetworkDevice{{
131-
NetworkDevice: NetworkDevice{},
132-
Name: "net1",
133-
InterfaceConfig: InterfaceConfig{
134-
IPv4PoolRef: &corev1.TypedLocalObjectReference{
135-
APIGroup: ptr.To("apps"),
136-
Name: "some-app",
137-
}},
138-
},
132+
AdditionalDevices: []AdditionalNetworkDevice{
133+
{
134+
NetworkDevice: NetworkDevice{},
135+
Name: "net1",
136+
InterfaceConfig: InterfaceConfig{
137+
IPv4PoolRef: &corev1.TypedLocalObjectReference{
138+
APIGroup: ptr.To("apps"),
139+
Name: "some-app",
140+
},
141+
},
142+
},
139143
},
140144
}
141145
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("ipv4PoolRef allows only IPAM apiGroup ipam.cluster.x-k8s.io")))
@@ -144,15 +148,16 @@ var _ = Describe("ProxmoxMachine Test", func() {
144148
It("Should only allow IPAM pool resources in IPv4PoolRef kind", func() {
145149
dm := defaultMachine()
146150
dm.Spec.Network = &NetworkSpec{
147-
AdditionalDevices: []AdditionalNetworkDevice{{
148-
NetworkDevice: NetworkDevice{},
149-
Name: "net1",
150-
InterfaceConfig: InterfaceConfig{IPv4PoolRef: &corev1.TypedLocalObjectReference{
151-
APIGroup: ptr.To("ipam.cluster.x-k8s.io"),
152-
Kind: "ConfigMap",
153-
Name: "some-app",
154-
}},
155-
},
151+
AdditionalDevices: []AdditionalNetworkDevice{
152+
{
153+
NetworkDevice: NetworkDevice{},
154+
Name: "net1",
155+
InterfaceConfig: InterfaceConfig{IPv4PoolRef: &corev1.TypedLocalObjectReference{
156+
APIGroup: ptr.To("ipam.cluster.x-k8s.io"),
157+
Kind: "ConfigMap",
158+
Name: "some-app",
159+
}},
160+
},
156161
},
157162
}
158163
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("ipv4PoolRef allows either InClusterIPPool or GlobalInClusterIPPool")))
@@ -161,15 +166,17 @@ var _ = Describe("ProxmoxMachine Test", func() {
161166
It("Should only allow IPAM pool resources in IPv6PoolRef apiGroup", func() {
162167
dm := defaultMachine()
163168
dm.Spec.Network = &NetworkSpec{
164-
AdditionalDevices: []AdditionalNetworkDevice{{
165-
NetworkDevice: NetworkDevice{},
166-
Name: "net1",
167-
InterfaceConfig: InterfaceConfig{
168-
IPv6PoolRef: &corev1.TypedLocalObjectReference{
169-
APIGroup: ptr.To("apps"),
170-
Name: "some-app",
171-
}},
172-
},
169+
AdditionalDevices: []AdditionalNetworkDevice{
170+
{
171+
NetworkDevice: NetworkDevice{},
172+
Name: "net1",
173+
InterfaceConfig: InterfaceConfig{
174+
IPv6PoolRef: &corev1.TypedLocalObjectReference{
175+
APIGroup: ptr.To("apps"),
176+
Name: "some-app",
177+
},
178+
},
179+
},
173180
},
174181
}
175182
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("ipv6PoolRef allows only IPAM apiGroup ipam.cluster.x-k8s.io")))
@@ -178,16 +185,18 @@ var _ = Describe("ProxmoxMachine Test", func() {
178185
It("Should only allow IPAM pool resources in IPv6PoolRef kind", func() {
179186
dm := defaultMachine()
180187
dm.Spec.Network = &NetworkSpec{
181-
AdditionalDevices: []AdditionalNetworkDevice{{
182-
NetworkDevice: NetworkDevice{},
183-
Name: "net1",
184-
InterfaceConfig: InterfaceConfig{
185-
IPv6PoolRef: &corev1.TypedLocalObjectReference{
186-
APIGroup: ptr.To("ipam.cluster.x-k8s.io"),
187-
Kind: "ConfigMap",
188-
Name: "some-app",
189-
}},
190-
},
188+
AdditionalDevices: []AdditionalNetworkDevice{
189+
{
190+
NetworkDevice: NetworkDevice{},
191+
Name: "net1",
192+
InterfaceConfig: InterfaceConfig{
193+
IPv6PoolRef: &corev1.TypedLocalObjectReference{
194+
APIGroup: ptr.To("ipam.cluster.x-k8s.io"),
195+
Kind: "ConfigMap",
196+
Name: "some-app",
197+
},
198+
},
199+
},
191200
},
192201
}
193202
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("ipv6PoolRef allows either InClusterIPPool or GlobalInClusterIPPool")))
@@ -196,10 +205,11 @@ var _ = Describe("ProxmoxMachine Test", func() {
196205
It("Should only allow Machine with additional devices with at least a pool ref", func() {
197206
dm := defaultMachine()
198207
dm.Spec.Network = &NetworkSpec{
199-
AdditionalDevices: []AdditionalNetworkDevice{{
200-
NetworkDevice: NetworkDevice{},
201-
Name: "net1",
202-
},
208+
AdditionalDevices: []AdditionalNetworkDevice{
209+
{
210+
NetworkDevice: NetworkDevice{},
211+
Name: "net1",
212+
},
203213
},
204214
}
205215
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("at least one pool reference must be set, either ipv4PoolRef or ipv6PoolRef")))
@@ -286,4 +296,43 @@ var _ = Describe("ProxmoxMachine Test", func() {
286296
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("should be less than or equal to 4094")))
287297
})
288298
})
299+
300+
Context("VMIDRange", func() {
301+
It("Should only allow spec.vmIDRange.start >= 100", func() {
302+
dm := defaultMachine()
303+
dm.Spec.VMIDRange = &VMIDRange{
304+
Start: 1,
305+
}
306+
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("should be greater than or equal to 100")))
307+
})
308+
It("Should only allow spec.vmIDRange.end >= 100", func() {
309+
dm := defaultMachine()
310+
dm.Spec.VMIDRange = &VMIDRange{
311+
End: 1,
312+
}
313+
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("should be greater than or equal to 100")))
314+
})
315+
It("Should only allow spec.vmIDRange.end >= spec.vmIDRange.start", func() {
316+
dm := defaultMachine()
317+
dm.Spec.VMIDRange = &VMIDRange{
318+
Start: 101,
319+
End: 100,
320+
}
321+
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("should be greater than or equal to start")))
322+
})
323+
It("Should only allow spec.vmIDRange.start if spec.vmIDRange.end is set", func() {
324+
dm := defaultMachine()
325+
dm.Spec.VMIDRange = &VMIDRange{
326+
Start: 100,
327+
}
328+
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("spec.vmIDRange.end in body should be greater than or equal to 100")))
329+
})
330+
It("Should only allow spec.vmIDRange.end if spec.vmIDRange.start is set", func() {
331+
dm := defaultMachine()
332+
dm.Spec.VMIDRange = &VMIDRange{
333+
End: 100,
334+
}
335+
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("spec.vmIDRange.start in body should be greater than or equal to 100")))
336+
})
337+
})
289338
})

‎api/v1alpha1/zz_generated.deepcopy.go‎

Lines changed: 20 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclusters.yaml‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,32 @@ spec:
553553
for the ProxmoxMachine VM.
554554
format: int64
555555
type: integer
556+
vmIDRange:
557+
description: VMIDRange is the range of VMIDs to use for
558+
VMs.
559+
properties:
560+
end:
561+
description: |-
562+
VMIDRangeEnd is the end of the VMID range to use for VMs.
563+
Only used if VMIDRangeStart is set.
564+
format: int64
565+
maximum: 999999999
566+
minimum: 100
567+
type: integer
568+
start:
569+
description: VMIDRangeStart is the start of the VMID
570+
range to use for VMs.
571+
format: int64
572+
maximum: 999999999
573+
minimum: 100
574+
type: integer
575+
required:
576+
- end
577+
- start
578+
type: object
579+
x-kubernetes-validations:
580+
- message: end should be greater than or equal to start
581+
rule: self.end >= self.start
556582
required:
557583
- sourceNode
558584
type: object

‎config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclustertemplates.yaml‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,6 +593,33 @@ spec:
593593
for the ProxmoxMachine VM.
594594
format: int64
595595
type: integer
596+
vmIDRange:
597+
description: VMIDRange is the range of VMIDs to
598+
use for VMs.
599+
properties:
600+
end:
601+
description: |-
602+
VMIDRangeEnd is the end of the VMID range to use for VMs.
603+
Only used if VMIDRangeStart is set.
604+
format: int64
605+
maximum: 999999999
606+
minimum: 100
607+
type: integer
608+
start:
609+
description: VMIDRangeStart is the start of
610+
the VMID range to use for VMs.
611+
format: int64
612+
maximum: 999999999
613+
minimum: 100
614+
type: integer
615+
required:
616+
- end
617+
- start
618+
type: object
619+
x-kubernetes-validations:
620+
- message: end should be greater than or equal to
621+
start
622+
rule: self.end >= self.start
596623
required:
597624
- sourceNode
598625
type: object

‎config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,31 @@ spec:
518518
VM.
519519
format: int64
520520
type: integer
521+
vmIDRange:
522+
description: VMIDRange is the range of VMIDs to use for VMs.
523+
properties:
524+
end:
525+
description: |-
526+
VMIDRangeEnd is the end of the VMID range to use for VMs.
527+
Only used if VMIDRangeStart is set.
528+
format: int64
529+
maximum: 999999999
530+
minimum: 100
531+
type: integer
532+
start:
533+
description: VMIDRangeStart is the start of the VMID range to
534+
use for VMs.
535+
format: int64
536+
maximum: 999999999
537+
minimum: 100
538+
type: integer
539+
required:
540+
- end
541+
- start
542+
type: object
543+
x-kubernetes-validations:
544+
- message: end should be greater than or equal to start
545+
rule: self.end >= self.start
521546
required:
522547
- sourceNode
523548
type: object

0 commit comments

Comments
 (0)