Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes: cloud-init can't be created bug #303

Merged
merged 3 commits into from
Jan 23, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
feat: add pre update, delete step
Tinyblargon committed Jan 22, 2024
commit ad3dafe8bdb20caec8d5631301262d31990b779b
2 changes: 1 addition & 1 deletion proxmox/config_guest.go
Original file line number Diff line number Diff line change
@@ -172,7 +172,7 @@ func GuestHasPendingChanges(vmr *VmRef, client *Client) (bool, error) {
if err != nil {
return false, err
}
return keyExists(params, "pending"), nil
return keyExists(params, "pending") || keyExists(params, "delete"), nil
}

// Reboot the specified guest
75 changes: 62 additions & 13 deletions proxmox/config_qemu.go
Original file line number Diff line number Diff line change
@@ -90,6 +90,10 @@ type ConfigQemu struct {
VmID int `json:"vmid,omitempty"` // TODO should be a custom type as there are limitations
}

const (
ConfigQemu_Error_UnableToUpdateWithoutReboot string = "unable to update vm without rebooting"
)

// Create - Tell Proxmox API to make the VM
func (config ConfigQemu) Create(vmr *VmRef, client *Client) (err error) {
_, err = config.setAdvanced(nil, false, vmr, client)
@@ -856,8 +860,13 @@ func (newConfig ConfigQemu) setAdvanced(currentConfig *ConfigQemu, rebootIfNeede

if currentConfig != nil {
// Update
url := "/nodes/" + vmr.node + "/" + vmr.vmType + "/" + strconv.Itoa(vmr.vmId) + "/config"
var itemsToDeleteBeforeUpdate string // this is for items that should be removed before they can be created again e.g. cloud-init disks. (convert to array when needed)
stopped := false

var markedDisks qemuUpdateChanges
if newConfig.Disks != nil && currentConfig.Disks != nil {
markedDisks := newConfig.Disks.markDiskChanges(*currentConfig.Disks)
markedDisks = *newConfig.Disks.markDiskChanges(*currentConfig.Disks)
// move disk to different storage or change disk format
for _, e := range markedDisks.Move {
_, err = e.move(true, vmr, client)
@@ -869,18 +878,44 @@ func (newConfig ConfigQemu) setAdvanced(currentConfig *ConfigQemu, rebootIfNeede
for _, e := range markedDisks.Resize {
_, err = e.resize(vmr, client)
if err != nil {
return
return false, err
}
}
// Moving disks changes the disk id. we need to get the config again if any disk was moved
if len(markedDisks.Move) != 0 {
currentConfig, err = NewConfigQemuFromApi(vmr, client)
if err != nil {
return
itemsToDeleteBeforeUpdate = newConfig.Disks.cloudInitRemove(*currentConfig.Disks)
}

if itemsToDeleteBeforeUpdate != "" {
err = client.Put(map[string]interface{}{"delete": itemsToDeleteBeforeUpdate}, url)
if err != nil {
return false, fmt.Errorf("error updating VM: %v", err)
}
// Deleteing these items can create pending changes
rebootRequired, err = GuestHasPendingChanges(vmr, client)
if err != nil {
return
}
if rebootRequired { // shutdown vm if reboot is required
if rebootIfNeeded {
if err = GuestShutdown(vmr, client, true); err != nil {
return
}
stopped = true
rebootRequired = false
} else {
return rebootRequired, errors.New(ConfigQemu_Error_UnableToUpdateWithoutReboot)
}
}
}

// TODO GuestHasPendingChanges() has the current vm config technically. We can use this to avoid an extra API call.
// Moving disks changes the disk id. we need to get the config again if any disk was moved.
if len(markedDisks.Move) != 0 {
currentConfig, err = NewConfigQemuFromApi(vmr, client)
if err != nil {
return
}
}

// Migrate VM
if newConfig.Node != currentConfig.Node {
vmr.SetNode(currentConfig.Node)
@@ -896,23 +931,37 @@ func (newConfig ConfigQemu) setAdvanced(currentConfig *ConfigQemu, rebootIfNeede
if err != nil {
return
}
exitStatus, err = client.PutWithTask(params, "/nodes/"+vmr.node+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/config")
exitStatus, err = client.PutWithTask(params, url)
if err != nil {
return false, fmt.Errorf("error updating VM: %v, error status: %s (params: %v)", err, exitStatus, params)
}

if !rebootRequired {
if !rebootRequired && !stopped { // only check if reboot is required if the vm is not already stopped
rebootRequired, err = GuestHasPendingChanges(vmr, client)
if err != nil {
return
}
}

if rebootRequired && rebootIfNeeded {
if err = GuestReboot(vmr, client); err != nil {
return
if stopped { // start vm if it was stopped
if rebootIfNeeded {
if err = GuestStart(vmr, client); err != nil {
return
}
stopped = false
rebootRequired = false
} else {
return true, nil
}
} else if rebootRequired { // reboot vm if it is running
if rebootIfNeeded {
if err = GuestReboot(vmr, client); err != nil {
return
}
rebootRequired = false
} else {
return rebootRequired, nil
}
rebootRequired = false
}
} else {
// Create
34 changes: 34 additions & 0 deletions proxmox/config_qemu_disk.go
Original file line number Diff line number Diff line change
@@ -1002,6 +1002,40 @@ type QemuStorages struct {
VirtIO *QemuVirtIODisks `json:"virtio,omitempty"`
}

// Return the cloud init disk that should be removed.
func (newStorages QemuStorages) cloudInitRemove(currentStorages QemuStorages) string {
newCloudInit := newStorages.listCloudInitDisk()
currentCloudInit := currentStorages.listCloudInitDisk()
if newCloudInit != "" && currentCloudInit != "" && newCloudInit != currentCloudInit {
return currentCloudInit
}
return ""
}

func (q QemuStorages) listCloudInitDisk() string {
if q.Ide != nil {
if disk := q.Ide.listCloudInitDisk(); disk != "" {
return disk
}
}
if q.Sata != nil {
if disk := q.Sata.listCloudInitDisk(); disk != "" {
return disk
}
}
if q.Scsi != nil {
if disk := q.Scsi.listCloudInitDisk(); disk != "" {
return disk
}
}
if q.VirtIO != nil {
if disk := q.VirtIO.listCloudInitDisk(); disk != "" {
return disk
}
}
return ""
}

func (storages QemuStorages) mapToApiValues(currentStorages QemuStorages, vmID, linkedVmId uint, params map[string]interface{}) (delete string) {
if storages.Ide != nil {
delete = storages.Ide.mapToApiValues(currentStorages.Ide, vmID, linkedVmId, params, delete)
10 changes: 10 additions & 0 deletions proxmox/config_qemu_disk_ide.go
Original file line number Diff line number Diff line change
@@ -56,6 +56,16 @@ type QemuIdeDisks struct {
Disk_3 *QemuIdeStorage `json:"3,omitempty"`
}

func (q QemuIdeDisks) listCloudInitDisk() string {
diskMap := q.mapToIntMap()
for i := range diskMap {
if diskMap[i] != nil && diskMap[i].CloudInit != nil {
return "ide" + strconv.Itoa(int(i))
}
}
return ""
}

func (disks QemuIdeDisks) mapToApiValues(currentDisks *QemuIdeDisks, vmID, LinkedVmId uint, params map[string]interface{}, delete string) string {
tmpCurrentDisks := QemuIdeDisks{}
if currentDisks != nil {
10 changes: 10 additions & 0 deletions proxmox/config_qemu_disk_sata.go
Original file line number Diff line number Diff line change
@@ -58,6 +58,16 @@ type QemuSataDisks struct {
Disk_5 *QemuSataStorage `json:"5,omitempty"`
}

func (q QemuSataDisks) listCloudInitDisk() string {
diskMap := q.mapToIntMap()
for i := range diskMap {
if diskMap[i] != nil && diskMap[i].CloudInit != nil {
return "sata" + strconv.Itoa(int(i))
}
}
return ""
}

func (disks QemuSataDisks) mapToApiValues(currentDisks *QemuSataDisks, vmID, LinkedVmId uint, params map[string]interface{}, delete string) string {
tmpCurrentDisks := QemuSataDisks{}
if currentDisks != nil {
10 changes: 10 additions & 0 deletions proxmox/config_qemu_disk_scsi.go
Original file line number Diff line number Diff line change
@@ -87,6 +87,16 @@ type QemuScsiDisks struct {
Disk_30 *QemuScsiStorage `json:"30,omitempty"`
}

func (q QemuScsiDisks) listCloudInitDisk() string {
diskMap := q.mapToIntMap()
for i := range diskMap {
if diskMap[i] != nil && diskMap[i].CloudInit != nil {
return "scsi" + strconv.Itoa(int(i))
}
}
return ""
}

func (disks QemuScsiDisks) mapToApiValues(currentDisks *QemuScsiDisks, vmID, linkedVmId uint, params map[string]interface{}, delete string) string {
tmpCurrentDisks := QemuScsiDisks{}
if currentDisks != nil {
63 changes: 63 additions & 0 deletions proxmox/config_qemu_disk_test.go
Original file line number Diff line number Diff line change
@@ -834,6 +834,69 @@ func Test_qemuDiskShort_Validate(t *testing.T) {
}
}

func Test_QemuStorages_cloudInitRemove(t *testing.T) {
type testInput struct {
currentStorages QemuStorages
newStorages QemuStorages
}
tests := []struct {
name string
input testInput
output string
}{
{name: "Different Slot, Different Type",
input: testInput{
currentStorages: QemuStorages{
Sata: &QemuSataDisks{Disk_2: &QemuSataStorage{CloudInit: &QemuCloudInitDisk{Format: QemuDiskFormat_Raw, Storage: "Test"}}}},
newStorages: QemuStorages{
Scsi: &QemuScsiDisks{Disk_8: &QemuScsiStorage{CloudInit: &QemuCloudInitDisk{Format: QemuDiskFormat_Raw, Storage: "Test"}}}},
},
output: "sata2",
},
{name: "Different Slot, Same Type",
input: testInput{
currentStorages: QemuStorages{
Ide: &QemuIdeDisks{Disk_1: &QemuIdeStorage{CloudInit: &QemuCloudInitDisk{Format: QemuDiskFormat_Raw, Storage: "Test"}}}},
newStorages: QemuStorages{
Ide: &QemuIdeDisks{Disk_3: &QemuIdeStorage{CloudInit: &QemuCloudInitDisk{Format: QemuDiskFormat_Raw, Storage: "Test"}}}},
},
output: "ide1",
},
{name: "Same Slot",
input: testInput{
currentStorages: QemuStorages{
Ide: &QemuIdeDisks{Disk_2: &QemuIdeStorage{CloudInit: &QemuCloudInitDisk{Format: QemuDiskFormat_Raw, Storage: "Test"}}}},
newStorages: QemuStorages{
Ide: &QemuIdeDisks{Disk_2: &QemuIdeStorage{CloudInit: &QemuCloudInitDisk{Format: QemuDiskFormat_Raw, Storage: "Test"}}}},
},
output: "",
},
{name: "Same Slot, CloudInit Disk",
input: testInput{
currentStorages: QemuStorages{
Ide: &QemuIdeDisks{Disk_2: &QemuIdeStorage{CloudInit: &QemuCloudInitDisk{Format: QemuDiskFormat_Raw, Storage: "Test"}}}},
newStorages: QemuStorages{
Ide: &QemuIdeDisks{Disk_2: &QemuIdeStorage{Disk: &QemuIdeDisk{Format: QemuDiskFormat_Raw, Storage: "Test"}}}},
},
output: "",
},
{name: "Same Slot, Disk CloudInit",
input: testInput{
currentStorages: QemuStorages{
Sata: &QemuSataDisks{Disk_4: &QemuSataStorage{Disk: &QemuSataDisk{Format: QemuDiskFormat_Raw, Storage: "Test"}}}},
newStorages: QemuStorages{
Sata: &QemuSataDisks{Disk_4: &QemuSataStorage{CloudInit: &QemuCloudInitDisk{Format: QemuDiskFormat_Raw, Storage: "Test"}}}},
},
output: "",
},
}
for _, test := range tests {
t.Run(test.name, func(*testing.T) {
require.Equal(t, test.output, test.input.newStorages.cloudInitRemove(test.input.currentStorages), test.name)
})
}
}

func Test_QemuStorages_markDiskChanges(t *testing.T) {
format_Raw := QemuDiskFormat_Raw
tests := []struct {
10 changes: 10 additions & 0 deletions proxmox/config_qemu_disk_virtio.go
Original file line number Diff line number Diff line change
@@ -70,6 +70,16 @@ type QemuVirtIODisks struct {
Disk_15 *QemuVirtIOStorage `json:"15,omitempty"`
}

func (q QemuVirtIODisks) listCloudInitDisk() string {
diskMap := q.mapToIntMap()
for i := range diskMap {
if diskMap[i] != nil && diskMap[i].CloudInit != nil {
return "virtio" + strconv.Itoa(int(i))
}
}
return ""
}

func (disks QemuVirtIODisks) mapToApiValues(currentDisks *QemuVirtIODisks, vmID, linkedVmId uint, params map[string]interface{}, delete string) string {
tmpCurrentDisks := QemuVirtIODisks{}
if currentDisks != nil {