feat: vmref: Support HA managed state#466
Conversation
|
Related: #465 |
Tinyblargon
left a comment
Tinyblargon
left a comment
There was a problem hiding this comment.
Looks good, only one functional change.
If possible please add a test?
| Name GuestName `json:"name"` | ||
| State PowerState `json:"state"` | ||
| Uptime time.Duration `json:"uptime"` | ||
| HaManaged int `json:"hamanaged"` |
There was a problem hiding this comment.
Could you change this to a bool as we abstract away PVE's handling of bool as int.
There was a problem hiding this comment.
Where do you know that it is boolean? I couldn't find documentation for the object that is returned.
There was a problem hiding this comment.
Did some digging. This is has the same fields as the ha object in the status. They only refer to managed with 0 and 1, given that PVE uses 1 and 0 instead of true and fasle in API requests to safe a few bytes. I'm 99% confident this is an int encoded boolean.
| return 0 | ||
| } | ||
|
|
||
| func (raw RawGuestStatus) GetHaManaged() int { |
There was a problem hiding this comment.
Could you add a test case to?
proxmox-api-go/proxmox/vmref_test.go
Line 458 in 4174b8e
Tinyblargon
added
type/enhancement
|
Hey, sorry for the late reply. We switched to bpg's proxmox provider, so I'm not allowed to work on this anymore. Feel free to take the changes and add what's missing :) |
2 participants
This will allow the terraform provider to check if a VM is HA managed before removing the HA resource.
This doesn't fix the broken check, but checking the HA state before removing the VM is the correct thing to do. Please fix or remove the broken HA managed error check.