Skip to content

Commit

Permalink
feat: Add ability to specify range of VM IDs to use (#286)
Browse files Browse the repository at this point in the history 
* 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 <[email protected]>
  • Loading branch information
@rybnico @mcbenjemaa
rybnico and mcbenjemaa authored Dec 5, 2024
1 parent c1f7482 commit 5f3ba2f
Show file tree
Hide file tree
Showing 15 changed files with 599 additions and 72 deletions.
29 changes: 27 additions & 2 deletions api/v1alpha1/proxmoxmachine_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/errors"
clusterapierrors "sigs.k8s.io/cluster-api/errors"
)

const (
Expand Down Expand Up @@ -101,6 +101,11 @@ type ProxmoxMachineSpec struct {
// +optional
Network *NetworkSpec `json:"network,omitempty"`

// VMIDRange is the range of VMIDs to use for VMs.
// +optional
// +kubebuilder:validation:XValidation:rule="self.end >= self.start",message="end should be greater than or equal to start"
VMIDRange *VMIDRange `json:"vmIDRange,omitempty"`

Checks *ProxmoxMachineChecks `json:"checks,omitempty"`
}

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

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

// VMIDRange defines the range of VMIDs to use for VMs.
type VMIDRange struct {
// VMIDRangeStart is the start of the VMID range to use for VMs.
// +kubebuilder:validation:Minimum=100
// +kubebuilder:validation:ExclusiveMinimum=false
// +kubebuilder:validation:Maximum=999999999
// +kubebuilder:validation:ExclusiveMaximum=false
// +kubebuilder:validation:Required
Start int64 `json:"start"`

// VMIDRangeEnd is the end of the VMID range to use for VMs.
// Only used if VMIDRangeStart is set.
// +kubebuilder:validation:Minimum=100
// +kubebuilder:validation:ExclusiveMinimum=false
// +kubebuilder:validation:Maximum=999999999
// +kubebuilder:validation:ExclusiveMaximum=false
// +kubebuilder:validation:Required
End int64 `json:"end"`
}

// +kubebuilder:object:root=true
// +kubebuilder:storageversion
// +kubebuilder:subresource:status
Expand Down
151 changes: 100 additions & 51 deletions api/v1alpha1/proxmoxmachine_types_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,16 +108,18 @@ var _ = Describe("ProxmoxMachine Test", func() {
Default: &NetworkDevice{
Bridge: "vmbr0",
},
AdditionalDevices: []AdditionalNetworkDevice{{
NetworkDevice: NetworkDevice{},
Name: "net0",
InterfaceConfig: InterfaceConfig{
IPv4PoolRef: &corev1.TypedLocalObjectReference{
APIGroup: ptr.To("ipam.cluster.x-k8s.io"),
Kind: "InClusterIPPool",
Name: "some-pool",
}},
},
AdditionalDevices: []AdditionalNetworkDevice{
{
NetworkDevice: NetworkDevice{},
Name: "net0",
InterfaceConfig: InterfaceConfig{
IPv4PoolRef: &corev1.TypedLocalObjectReference{
APIGroup: ptr.To("ipam.cluster.x-k8s.io"),
Kind: "InClusterIPPool",
Name: "some-pool",
},
},
},
},
}

Expand All @@ -127,15 +129,17 @@ var _ = Describe("ProxmoxMachine Test", func() {
It("Should only allow IPAM pool resources in IPv4PoolRef apiGroup", func() {
dm := defaultMachine()
dm.Spec.Network = &NetworkSpec{
AdditionalDevices: []AdditionalNetworkDevice{{
NetworkDevice: NetworkDevice{},
Name: "net1",
InterfaceConfig: InterfaceConfig{
IPv4PoolRef: &corev1.TypedLocalObjectReference{
APIGroup: ptr.To("apps"),
Name: "some-app",
}},
},
AdditionalDevices: []AdditionalNetworkDevice{
{
NetworkDevice: NetworkDevice{},
Name: "net1",
InterfaceConfig: InterfaceConfig{
IPv4PoolRef: &corev1.TypedLocalObjectReference{
APIGroup: ptr.To("apps"),
Name: "some-app",
},
},
},
},
}
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("ipv4PoolRef allows only IPAM apiGroup ipam.cluster.x-k8s.io")))
Expand All @@ -144,15 +148,16 @@ var _ = Describe("ProxmoxMachine Test", func() {
It("Should only allow IPAM pool resources in IPv4PoolRef kind", func() {
dm := defaultMachine()
dm.Spec.Network = &NetworkSpec{
AdditionalDevices: []AdditionalNetworkDevice{{
NetworkDevice: NetworkDevice{},
Name: "net1",
InterfaceConfig: InterfaceConfig{IPv4PoolRef: &corev1.TypedLocalObjectReference{
APIGroup: ptr.To("ipam.cluster.x-k8s.io"),
Kind: "ConfigMap",
Name: "some-app",
}},
},
AdditionalDevices: []AdditionalNetworkDevice{
{
NetworkDevice: NetworkDevice{},
Name: "net1",
InterfaceConfig: InterfaceConfig{IPv4PoolRef: &corev1.TypedLocalObjectReference{
APIGroup: ptr.To("ipam.cluster.x-k8s.io"),
Kind: "ConfigMap",
Name: "some-app",
}},
},
},
}
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("ipv4PoolRef allows either InClusterIPPool or GlobalInClusterIPPool")))
Expand All @@ -161,15 +166,17 @@ var _ = Describe("ProxmoxMachine Test", func() {
It("Should only allow IPAM pool resources in IPv6PoolRef apiGroup", func() {
dm := defaultMachine()
dm.Spec.Network = &NetworkSpec{
AdditionalDevices: []AdditionalNetworkDevice{{
NetworkDevice: NetworkDevice{},
Name: "net1",
InterfaceConfig: InterfaceConfig{
IPv6PoolRef: &corev1.TypedLocalObjectReference{
APIGroup: ptr.To("apps"),
Name: "some-app",
}},
},
AdditionalDevices: []AdditionalNetworkDevice{
{
NetworkDevice: NetworkDevice{},
Name: "net1",
InterfaceConfig: InterfaceConfig{
IPv6PoolRef: &corev1.TypedLocalObjectReference{
APIGroup: ptr.To("apps"),
Name: "some-app",
},
},
},
},
}
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("ipv6PoolRef allows only IPAM apiGroup ipam.cluster.x-k8s.io")))
Expand All @@ -178,16 +185,18 @@ var _ = Describe("ProxmoxMachine Test", func() {
It("Should only allow IPAM pool resources in IPv6PoolRef kind", func() {
dm := defaultMachine()
dm.Spec.Network = &NetworkSpec{
AdditionalDevices: []AdditionalNetworkDevice{{
NetworkDevice: NetworkDevice{},
Name: "net1",
InterfaceConfig: InterfaceConfig{
IPv6PoolRef: &corev1.TypedLocalObjectReference{
APIGroup: ptr.To("ipam.cluster.x-k8s.io"),
Kind: "ConfigMap",
Name: "some-app",
}},
},
AdditionalDevices: []AdditionalNetworkDevice{
{
NetworkDevice: NetworkDevice{},
Name: "net1",
InterfaceConfig: InterfaceConfig{
IPv6PoolRef: &corev1.TypedLocalObjectReference{
APIGroup: ptr.To("ipam.cluster.x-k8s.io"),
Kind: "ConfigMap",
Name: "some-app",
},
},
},
},
}
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("ipv6PoolRef allows either InClusterIPPool or GlobalInClusterIPPool")))
Expand All @@ -196,10 +205,11 @@ var _ = Describe("ProxmoxMachine Test", func() {
It("Should only allow Machine with additional devices with at least a pool ref", func() {
dm := defaultMachine()
dm.Spec.Network = &NetworkSpec{
AdditionalDevices: []AdditionalNetworkDevice{{
NetworkDevice: NetworkDevice{},
Name: "net1",
},
AdditionalDevices: []AdditionalNetworkDevice{
{
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")))
Expand Down Expand Up @@ -286,4 +296,43 @@ var _ = Describe("ProxmoxMachine Test", func() {
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("should be less than or equal to 4094")))
})
})

Context("VMIDRange", func() {
It("Should only allow spec.vmIDRange.start >= 100", func() {
dm := defaultMachine()
dm.Spec.VMIDRange = &VMIDRange{
Start: 1,
}
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("should be greater than or equal to 100")))
})
It("Should only allow spec.vmIDRange.end >= 100", func() {
dm := defaultMachine()
dm.Spec.VMIDRange = &VMIDRange{
End: 1,
}
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("should be greater than or equal to 100")))
})
It("Should only allow spec.vmIDRange.end >= spec.vmIDRange.start", func() {
dm := defaultMachine()
dm.Spec.VMIDRange = &VMIDRange{
Start: 101,
End: 100,
}
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("should be greater than or equal to start")))
})
It("Should only allow spec.vmIDRange.start if spec.vmIDRange.end is set", func() {
dm := defaultMachine()
dm.Spec.VMIDRange = &VMIDRange{
Start: 100,
}
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("spec.vmIDRange.end in body should be greater than or equal to 100")))
})
It("Should only allow spec.vmIDRange.end if spec.vmIDRange.start is set", func() {
dm := defaultMachine()
dm.Spec.VMIDRange = &VMIDRange{
End: 100,
}
Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("spec.vmIDRange.start in body should be greater than or equal to 100")))
})
})
})
20 changes: 20 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 26 additions & 0 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclusters.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,32 @@ spec:
for the ProxmoxMachine VM.
format: int64
type: integer
vmIDRange:
description: VMIDRange is the range of VMIDs to use for
VMs.
properties:
end:
description: |-
VMIDRangeEnd is the end of the VMID range to use for VMs.
Only used if VMIDRangeStart is set.
format: int64
maximum: 999999999
minimum: 100
type: integer
start:
description: VMIDRangeStart is the start of the VMID
range to use for VMs.
format: int64
maximum: 999999999
minimum: 100
type: integer
required:
- end
- start
type: object
x-kubernetes-validations:
- message: end should be greater than or equal to start
rule: self.end >= self.start
required:
- sourceNode
type: object
Expand Down
27 changes: 27 additions & 0 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclustertemplates.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,33 @@ spec:
for the ProxmoxMachine VM.
format: int64
type: integer
vmIDRange:
description: VMIDRange is the range of VMIDs to
use for VMs.
properties:
end:
description: |-
VMIDRangeEnd is the end of the VMID range to use for VMs.
Only used if VMIDRangeStart is set.
format: int64
maximum: 999999999
minimum: 100
type: integer
start:
description: VMIDRangeStart is the start of
the VMID range to use for VMs.
format: int64
maximum: 999999999
minimum: 100
type: integer
required:
- end
- start
type: object
x-kubernetes-validations:
- message: end should be greater than or equal to
start
rule: self.end >= self.start
required:
- sourceNode
type: object
Expand Down
25 changes: 25 additions & 0 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,31 @@ spec:
VM.
format: int64
type: integer
vmIDRange:
description: VMIDRange is the range of VMIDs to use for VMs.
properties:
end:
description: |-
VMIDRangeEnd is the end of the VMID range to use for VMs.
Only used if VMIDRangeStart is set.
format: int64
maximum: 999999999
minimum: 100
type: integer
start:
description: VMIDRangeStart is the start of the VMID range to
use for VMs.
format: int64
maximum: 999999999
minimum: 100
type: integer
required:
- end
- start
type: object
x-kubernetes-validations:
- message: end should be greater than or equal to start
rule: self.end >= self.start
required:
- sourceNode
type: object
Expand Down
Loading

0 comments on commit 5f3ba2f

Please sign in to comment.