Add networkupdate support for VM, introduce forced guest customization function
#229
Conversation
govcd/vm.go
Outdated
| // Runs synchronously, VM is ready for another operation after this function returns. | ||
| func (vm *VM) UpdateNetworkConnectionSection(networks *types.NetworkConnectionSection) error { | ||
| if vm.VM.HREF == "" { | ||
| return fmt.Errorf("cannot refresh, Object is empty") |
There was a problem hiding this comment.
Wow. That was way off. Fixed.
| @@ -263,12 +263,14 @@ func checkLb(queryUrl string, expectedResponses []string, maxRetryTimeout int) e | |||
| iterations = maxRetryTimeout / 5 | |||
| } | |||
|
|
|||
| httpClient := &http.Client{Timeout: 5 * time.Second} | |||
There was a problem hiding this comment.
Could you add a comment explaining how this timeout affects the expected duration of the test?
I am talking about the estimate given with Waiting for the virtual server to accept responses (%s interval x %d iterations)"
There was a problem hiding this comment.
Yep. This simple loop was actually quite unstable in guessing timing. I put in a time.Ticker so that it does not lie and actually work as the time was set.
govcd/vm.go
Outdated
| // PowerOnAndForceCustomization is a synchronous function which is equivalent to the functionality | ||
| // one has in UI. It triggers customization which may be useful in some cases (like altering NICs) | ||
| // | ||
| // The VM _must_ be Undeployed for this action to actually work. |
There was a problem hiding this comment.
Can we detect the state of the VM to make sure this prerequisite is met?
There was a problem hiding this comment.
IMHO/ Yes, we should. VM should have status.
There was a problem hiding this comment.
I can add a check, but the problem is that it doesn't actually fail to power on and god knows if it can be useful in any scenario. As far as I've investigated it simply never exits "GC_PENDING" if it wasn't un-deployed before using PowerOnAndForceCustomization.
There was a problem hiding this comment.
Now I lost between explanation and doc: "The VM must be Undeployed for this action to actually work." ?
There was a problem hiding this comment.
Ok. On a second read that may not be too clear. Power on works anyway, but the VM never exits GC_PENDING state if it was not undeployed.
There was a problem hiding this comment.
doesn't vm.status == undeployed isn't sufficient?
There was a problem hiding this comment.
It may be. The problem is that if I enforce vm.status == undeployed, I'm not 100% sure it is the only case it would be useful.
There was a problem hiding this comment.
I don't understand. If we can check the status, we can simply abort when the wanted status is not set.
What am I missing?
govcd/vm_test.go
Outdated
|
|
||
| networkBefore, err := vm.GetNetworkConnectionSection() | ||
| check.Assert(err, IsNil) | ||
| //check.Assert(network, IsNil) |
There was a problem hiding this comment.
leftover comment: can it be removed?
govcd/vm.go
Outdated
| // PowerOnAndForceCustomization is a synchronous function which is equivalent to the functionality | ||
| // one has in UI. It triggers customization which may be useful in some cases (like altering NICs) | ||
| // | ||
| // The VM _must_ be Undeployed for this action to actually work. |
There was a problem hiding this comment.
IMHO/ Yes, we should. VM should have status.
govcd/vm.go
Outdated
| // PowerOnAndForceCustomization is a synchronous function which is equivalent to the functionality | ||
| // one has in UI. It triggers customization which may be useful in some cases (like altering NICs) | ||
| // | ||
| // The VM _must_ be Undeployed for this action to actually work. |
There was a problem hiding this comment.
I don't understand. If we can check the status, we can simply abort when the wanted status is not set.
What am I missing?
Ref https://github.com/terraform-providers/terraform-provider-vcd/issues/299, https://github.com/terraform-providers/terraform-provider-vcd/issues/300
This PR introduces a few things:
vm.UpdateNetworkConnectionSection())A few general lint fixes:
SA1015: using time.Tick leaks the underlying ticker, consider using it only in endless functions, tests and the main package, and use time.NewTicker herelb_test.go