vmware-archive / legacy-terraform-provider-vra7

Terraform provider for vRealize Automation 7
Mozilla Public License 2.0
60 stars 35 forks source link

Is wait_timeout attribute required? #154

Open Prativa20 opened 5 years ago

Prativa20 commented 5 years ago

There is a wait_timeout attribute in the schema. Is there a real usecase that requires this? Most of the terraform provider returns only when the resource request reaches a termination state, failed or successful. We want to follow the same with vra7_deployment resource as well.

Adding wait_timeout requires us to maintain another state, that is, in-progress and in maintaining this, we are hitting an issue when update times out which is as follows.

  1. Creating a resource creates a catalog request id (say, id1) (saved in the state file, used to query the provisioned deployment). Status of id1 will always return successful once creation is successful.

  2. To update, query by id1 and get all the machines resources of the deployment. Reconfigure action on each machine creates new request ids (say r1 for machine1 and r2 ). To check the status of reconfigure, r1 and r2 has to be checked. But id1 still remains in state file and not replaced by r1 or r2.

  3. If update times out in step 2, we loose r1 and r2 and terraform refresh will always query id1 saved in the state file and that will always return successful.

Not having wait_timeout will make is very simple. Provider will return after the request has reached a terminal state and the code will be less complex and align with the other providers.

Any feedback/suggestion is appreciated.

@markpeek @dmettem @hobovirtual @VincenzoDo @mponton @nferro @jasonlivie @d-luu @sametimesolutions @vaficionado

hobovirtual commented 5 years ago

Hi @Prativa20

Personally i think we should remove the wait_timeout, simplicity is key in everything. As long as the provider doesn't timeout on request, we don't need the wait_timeout option.

Thanks!

mponton commented 5 years ago

Hi @Prativa20 , I agree with @hobovirtual, I would remove the option as well.

Thanks for involving us like that!

nferro commented 5 years ago

We have a use case where the wait_timeout is useful: the team that provides us VRA has some common integrations and only times out after 2h, however for the blueprints we use we know that every request taking more that 20m will fail. If we don't specify the wait_timeout we have to wait 2h before failing instead of 20m.

Prativa20 commented 5 years ago

@nferro wait_timeout was introduced in #10 because depends_on was not working as expected. The reason for that issue was that the provider was not waiting correctly for the request to complete.

But that has been fixed during the refactoring and the provider returns only when the status of the request reaches a terminal state. I have tested this with one of the depends_on use case without any wait_timeout.

Is the use case that you have mentioned in your comment here different from #10? We would like to understand that as it looks more like a vRA issue.

Thanks, Prativa

jonnyborbs commented 5 years ago

@nferro can you confirm our assumption here?

nferro commented 5 years ago

@vaficionado that’s correct.

Also, I believe it’s useful to be able to specify a timeout after which the provider will fail. You may very well say it’s a VRA issue but the users not always control that kind of integrations and should be able to fail as fast as they can.