Skip to content

Commit

Permalink
Test out generics for vm config
Browse files Browse the repository at this point in the history
  • Loading branch information
@alperencelik
alperencelik committed Nov 19, 2024
1 parent 83b2c99 commit f9ae0c0
Show file tree
Hide file tree
Showing 4 changed files with 149 additions and 115 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/golangci-lint.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ name: GolangCI-Lint
on:
pull_request:
types: [opened, edited, synchronize, reopened]
push:
branches:
- '**'

jobs:
lint:
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/verify.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ on:
types: [opened, edited, synchronize, reopened]
branches:
- main
push:
branches:
- main

jobs:
verify:
Expand Down
83 changes: 83 additions & 0 deletions pkg/proxmox/generics.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package proxmox

import (
"context"
"fmt"
"reflect"

proxmoxv1alpha1 "github.com/alperencelik/kubemox/api/proxmox/v1alpha1"
"github.com/alperencelik/kubemox/pkg/utils"
"sigs.k8s.io/controller-runtime/pkg/log"
)

func classifyItems[I any](desiredItems, actualItems []I, getKey func(I) string) (
itemsToAdd, itemsToUpdate, itemsToDelete []I) {
// Create a map of actual items
actualItemsMap := make(map[string]I)
for _, item := range actualItems {
key := getKey(item)
actualItemsMap[key] = item
}

itemKeyList := make([]string, len(desiredItems))
for i, item := range desiredItems {
key := getKey(item)
itemKeyList[i] = key
if actualItem, ok := actualItemsMap[key]; ok {
if !reflect.DeepEqual(item, actualItem) {
itemsToUpdate = append(itemsToUpdate, item)
}
} else {
itemsToAdd = append(itemsToAdd, item)
}
}

for _, item := range actualItems {
key := getKey(item)
if !utils.StringInSlice(key, itemKeyList) {
itemsToDelete = append(itemsToDelete, item)
}
}
return
}

func applyChanges[I any](
ctx context.Context,
vm *proxmoxv1alpha1.VirtualMachine,
itemsToAdd, itemsToUpdate, itemsToDelete []I,
getDeviceID func(I) string,
updateConfig func(context.Context, *proxmoxv1alpha1.VirtualMachine, I) error,
operationName string,
// preUpdate func(I) error, // Pre-conditions before updating the item
) error {
for _, item := range itemsToAdd {
deviceID := getDeviceID(item)
log.Log.Info(fmt.Sprintf("Adding %s %s to VirtualMachine %s", operationName, deviceID, vm.Name))
if err := updateConfig(ctx, vm, item); err != nil {
return err
} else {
log.Log.Info(fmt.Sprintf("%s %s of VirtualMachine %s has been added", operationName, deviceID, vm.Name))
}
}

for _, item := range itemsToUpdate {
deviceID := getDeviceID(item)

if err := updateConfig(ctx, vm, item); err != nil {
return err
} else {
log.Log.Info(fmt.Sprintf("%s %s of VirtualMachine %s has been updated", operationName, deviceID, vm.Name))
}
}

for _, item := range itemsToDelete {
deviceID := getDeviceID(item)
log.Log.Info(fmt.Sprintf("Deleting %s %s of VirtualMachine %s", operationName, deviceID, vm.Name))
if _, err := deleteVirtualMachineOption(vm, deviceID); err != nil {
return err
} else {
log.Log.Info(fmt.Sprintf("%s %s of VirtualMachine %s has been deleted", operationName, deviceID, vm.Name))
}
}
return nil
}
175 changes: 60 additions & 115 deletions pkg/proxmox/virtualmachine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -907,67 +907,47 @@ func configureVirtualMachineDisk(vm *proxmoxv1alpha1.VirtualMachine) error {
disksToAdd, disksToUpdate, disksToDelete := classifyDisks(disks, virtualMachineDisksParsed)

// Apply disk changes
if err := applyDiskChanges(vm, disksToAdd, disksToUpdate, disksToDelete); err != nil {
if err := applyDiskChanges(ctx, vm, disksToAdd, disksToUpdate, disksToDelete); err != nil {
return err
}
return nil
}

func classifyDisks(desiredDisks, actualDisks []proxmoxv1alpha1.VirtualMachineSpecTemplateDisk) (
disksToAdd, disksToUpdate, disksToDelete []proxmoxv1alpha1.VirtualMachineSpecTemplateDisk) {
// Create a map of actual disks
actualDisksMap := make(map[string]proxmoxv1alpha1.VirtualMachineSpecTemplateDisk)
for _, disk := range actualDisks {
actualDisksMap[disk.Device] = disk
}

diskDeviceList := make([]string, len(desiredDisks))
for i, disk := range desiredDisks {
diskDeviceList[i] = disk.Device
if actualDisk, ok := actualDisksMap[disk.Device]; ok {
if !reflect.DeepEqual(disk, actualDisk) {
disksToUpdate = append(disksToUpdate, disk)
}
} else {
disksToAdd = append(disksToAdd, disk)
}
getKey := func(disk proxmoxv1alpha1.VirtualMachineSpecTemplateDisk) string {
return disk.Device
}
return classifyItems(desiredDisks, actualDisks, getKey)
}

for _, disk := range actualDisks {
if !utils.StringInSlice(disk.Device, diskDeviceList) {
disksToDelete = append(disksToDelete, disk)
}
func classifyPCIs(desiredPcis, actualPcis []proxmoxv1alpha1.PciDevice) (
pcisToAdd, pcisToUpdate, pcisToDelete []proxmoxv1alpha1.PciDevice) {
getKey := func(pci proxmoxv1alpha1.PciDevice) string {
return pci.DeviceID
}
return

return classifyItems(desiredPcis, actualPcis, getKey)
}

func applyDiskChanges(vm *proxmoxv1alpha1.VirtualMachine,
func applyDiskChanges(
ctx context.Context,
vm *proxmoxv1alpha1.VirtualMachine,
disksToAdd, disksToUpdate, disksToDelete []proxmoxv1alpha1.VirtualMachineSpecTemplateDisk) error {
for _, disk := range disksToAdd {
log.Log.Info(fmt.Sprintf("Adding disk %s to VirtualMachine %s", disk.Device, vm.Name))
if err := updateDiskConfig(ctx, vm, disk); err != nil {
return err
} else {
log.Log.Info(fmt.Sprintf("Disk %s of VirtualMachine %s has been added", disk.Device, vm.Name))
}
}
for _, disk := range disksToUpdate {
// TODO: Implement check for blocking shrink operation
if err := updateDiskConfig(ctx, vm, disk); err != nil {
return err
} else {
log.Log.Info(fmt.Sprintf("Disk %s of VirtualMachine %s has been updated", disk.Device, vm.Name))
}
}
for _, disk := range disksToDelete {
log.Log.Info(fmt.Sprintf("Deleting disk %s of VirtualMachine %s", disk.Device, vm.Name))
if _, err := deleteVirtualMachineOption(vm, disk.Device); err != nil {
return err
} else {
log.Log.Info(fmt.Sprintf("Disk %s of VirtualMachine %s has been deleted", disk.Device, vm.Name))
}
}
return nil
getDeviceID := func(disk proxmoxv1alpha1.VirtualMachineSpecTemplateDisk) string {
return disk.Device
}

return applyChanges(
ctx,
vm,
disksToAdd,
disksToUpdate,
disksToDelete,
getDeviceID,
updateDiskConfig,
"Disk",
)
}

func parseDiskConfiguration(disks map[string]string) ([]proxmoxv1alpha1.VirtualMachineSpecTemplateDisk, error) {
Expand Down Expand Up @@ -1135,7 +1115,7 @@ func configureVirtualMachinePCI(vm *proxmoxv1alpha1.VirtualMachine) error {
PcisToadd, PCIsToUpdate, PcisToDelete := classifyPCIs(PCIs, virtualMachinePCIsParsed)

// Apply PCI changes
if err := applyPCIChanges(vm, PcisToadd, PCIsToUpdate, PcisToDelete); err != nil {
if err := applyPCIChanges(ctx, vm, PcisToadd, PCIsToUpdate, PcisToDelete); err != nil {
return err
}
return nil
Expand All @@ -1150,12 +1130,12 @@ func GetPCIConfiguration(vmName, nodeName string) (map[string]string, error) {
return PCIs, nil
}

func parsePCIConfiguration(PCIs map[string]string) ([]proxmoxv1alpha1.PciDevice, error) {
func parsePCIConfiguration(pcis map[string]string) ([]proxmoxv1alpha1.PciDevice, error) {
var PCIConfigurations []proxmoxv1alpha1.PciDevice
var PCIConfiguration proxmoxv1alpha1.PciDevice
// Parse PCI devices to use as PCI devices
for i, PCI := range PCIs {
pciSplit := strings.Split(PCI, ",")
for i, pci := range pcis {
pciSplit := strings.Split(pci, ",")
PCIConfiguration.DeviceID = pciSplit[0]
for _, pciSplit := range pciSplit[1:] {
kv := strings.SplitN(pciSplit, "=", 2)
Expand All @@ -1178,86 +1158,51 @@ func parsePCIConfiguration(PCIs map[string]string) ([]proxmoxv1alpha1.PciDevice,
return PCIConfigurations, nil
}

func classifyPCIs(desiredPcis, actualPcis []proxmoxv1alpha1.PciDevice) (
PcisToadd, PCIsToUpdate, PcisToDelete []proxmoxv1alpha1.PciDevice) {
// Create a map of actual PCIs
actualPcisMap := make(map[string]proxmoxv1alpha1.PciDevice)
for _, Pci := range actualPcis {
actualPcisMap[Pci.DeviceID] = Pci
}

PciDeviceList := make([]string, len(desiredPcis))
for i, Pci := range desiredPcis {
PciDeviceList[i] = Pci.DeviceID
if actualPci, ok := actualPcisMap[Pci.DeviceID]; ok {
if !reflect.DeepEqual(Pci, actualPci) {
PCIsToUpdate = append(PCIsToUpdate, Pci)
}
} else {
PcisToadd = append(PcisToadd, Pci)
}
}

for _, Pci := range actualPcis {
if !utils.StringInSlice(Pci.DeviceID, PciDeviceList) {
PcisToDelete = append(PcisToDelete, Pci)
}
}
return
}

func applyPCIChanges(vm *proxmoxv1alpha1.VirtualMachine,
PcisToadd, PCIsToUpdate, PcisToDelete []proxmoxv1alpha1.PciDevice) error {
for _, Pci := range PcisToadd {
log.Log.Info(fmt.Sprintf("Adding PCI device %s to VirtualMachine %s", Pci.DeviceID, vm.Name))
if err := updatePCIConfig(ctx, vm, Pci); err != nil {
return err
} else {
log.Log.Info(fmt.Sprintf("PCI device %s of VirtualMachine %s has been added", Pci.DeviceID, vm.Name))
}
}
for _, Pci := range PCIsToUpdate {
if err := updatePCIConfig(ctx, vm, Pci); err != nil {
return err
} else {
log.Log.Info(fmt.Sprintf("PCI device %s of VirtualMachine %s has been updated", Pci.DeviceID, vm.Name))
}
}
for _, Pci := range PcisToDelete {
log.Log.Info(fmt.Sprintf("Deleting PCI device %s of VirtualMachine %s", Pci.DeviceID, vm.Name))
if _, err := deleteVirtualMachineOption(vm, Pci.DeviceID); err != nil {
return err
} else {
log.Log.Info(fmt.Sprintf("PCI device %s of VirtualMachine %s has been deleted", Pci.DeviceID, vm.Name))
}
}
return nil
func applyPCIChanges(
ctx context.Context,
vm *proxmoxv1alpha1.VirtualMachine,
pcisToAdd, pcisToUpdate, pcisToDelete []proxmoxv1alpha1.PciDevice,
) error {
getDeviceID := func(pci proxmoxv1alpha1.PciDevice) string {
return pci.DeviceID
}

return applyChanges(
ctx,
vm,
pcisToAdd,
pcisToUpdate,
pcisToDelete,
getDeviceID,
updatePCIConfig,
"PCI device",
)
}

func updatePCIConfig(ctx context.Context, vm *proxmoxv1alpha1.VirtualMachine,
Pci proxmoxv1alpha1.PciDevice) error {
pci proxmoxv1alpha1.PciDevice) error {
vmName, nodeName := vm.Name, vm.Spec.NodeName

VirtualMachine, err := getVirtualMachine(vmName, nodeName)
if err != nil {
log.Log.Error(err, "Error getting VM")
}
index, err := getIndexOfPCIConfig(vmName, nodeName, Pci)
index, err := getIndexOfPCIConfig(vmName, nodeName, pci)
if err != nil {
log.Log.Error(err, "Error getting index of PCI configuration")
}

taskID, err := VirtualMachine.Config(ctx, proxmox.VirtualMachineOption{
Name: "hostpci" + index,
Value: Pci.DeviceID + func() string {
Value: pci.DeviceID + func() string {
var value string
value += ","
var pcieSet, xVgaSet bool
if Pci.PCIE {
if pci.PCIE {
value += "pcie=1"
pcieSet = true
}
if Pci.PrimaryGPU {
if pci.PrimaryGPU {
if pcieSet {
value += ","
}
Expand Down Expand Up @@ -1296,7 +1241,7 @@ func updatePCIConfig(ctx context.Context, vm *proxmoxv1alpha1.VirtualMachine,
_, _, err = task.WaitForCompleteStatus(ctx, 5, 3)
// Check the task status and return the error if it's not completed
if task.IsFailed {
log.Log.Error(err, fmt.Sprintf("Error starting VirtualMachine %s while attaching %s PCI device", vmName, Pci.DeviceID))
log.Log.Error(err, fmt.Sprintf("Error starting VirtualMachine %s while attaching %s PCI device", vmName, pci.DeviceID))
// Try to revert the changes
err = revertVirtualMachineOption(vmName, nodeName, "hostpci"+index)
if err != nil {
Expand All @@ -1308,20 +1253,20 @@ func updatePCIConfig(ctx context.Context, vm *proxmoxv1alpha1.VirtualMachine,
return err
}

func getIndexOfPCIConfig(vmName, nodeName string, PciDevice proxmoxv1alpha1.PciDevice) (string, error) {
func getIndexOfPCIConfig(vmName, nodeName string, pciDevice proxmoxv1alpha1.PciDevice) (string, error) {
VirtualMachine, err := getVirtualMachine(vmName, nodeName)
if err != nil {
log.Log.Error(err, "Error getting VM")
}
hostPCIs := VirtualMachine.VirtualMachineConfig.MergeHostPCIs()

for i, hostPCI := range hostPCIs {
if strings.Split(hostPCI, ",")[0] == PciDevice.DeviceID {
if strings.Split(hostPCI, ",")[0] == pciDevice.DeviceID {
return i, nil
}
}
// If device ID is not found, return the 0th index to create it
return "0", nil
return "0", err
}

func revertVirtualMachineOption(vmName, nodeName, value string) error {
Expand Down

0 comments on commit f9ae0c0

Please sign in to comment.