Change lbu timeout #174
Change lbu timeout #174
Conversation
|
Closes #106 |
|
Closes #173 |
ghost
requested review from
| @@ -206,7 +206,7 @@ func reconcileSecurityGroupRule(ctx context.Context, clusterScope *scope.Cluster | |||
| func deleteSecurityGroup(ctx context.Context, clusterScope *scope.ClusterScope, securityGroupId string, securityGroupSvc security.OscSecurityGroupInterface, clock_time clock.Clock) (reconcile.Result, error) { | |||
| clusterScope.Info("Check loadbalancer deletion") | |||
|
|
|||
| currentTimeout := clock_time.Now().Add(time.Second * 20) | |||
| currentTimeout := clock_time.Now().Add(time.Second * 600) | |||
There was a problem hiding this comment.
one question, how did you end up choosing this numbers ? (600 rather than 20 for example? )
There was a problem hiding this comment.
I test several values not to have (#173)
There was a problem hiding this comment.
This is just a remark but rather than setting timeouts, is it possible in case of failure to just try in the next reconciliation loop ?
There was a problem hiding this comment.
We can set reconcile.Result{RequeueAfter: 30 * time.Second}
There was a problem hiding this comment.
It will be looked during this refacto #183 if you are ok ?
| err = vmSvc.CheckVmState(5, 120, "running", vmId) | ||
| err = vmSvc.CheckVmState(5, 240, "running", vmId) | ||
| if err != nil { |
There was a problem hiding this comment.
In the deletion process you check that the VM is running and fail if not ?
There was a problem hiding this comment.
Thanks.
You right.
In vpc, unlink does not depends on vm state.
And unlinkLoadBalancerBackendMachin does not depend on vm state.
I will remove it from reconcileDelete.
|
@outscale-mdr @outscale-hmi Are you ok with the latest changes ? |
* Increase timeout to delete vm * Machine can be destroyed with any state (provisonning, provisionned, running)
ghost
added
bug
What type of PR is this?
/kind bug
What this PR does / why we need it:
Delete vm (for vm and loadbalancer) can be too long so it can execeded timeout.
Machine should be able to be destroyed with any state (provisioning, provisioned, running)
Also to have idempotency vm functions
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #173
Fixes #106
Special notes for your reviewer:
It incluse also vm unit test refacto.
TODOs: