-
Notifications
You must be signed in to change notification settings - Fork 112
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 for release 3.5.0 #761
Conversation
Some resources are not available in 10.1, and we skip the relevant tests * Skip Datasource test for vcd_vdc_group and vcd_library_certificate * Skip Resource List test for vcd_vdc_group Signed-off-by: Giuseppe Maxia <[email protected]>
Dependency names were not referring to the right entities in TestAccVcdVAppVmMultiNIC steps. They were referring to Org Networks instead of the related vApp networks. As a result, the removal of entities was not happeing in the correct order Signed-off-by: Giuseppe Maxia <[email protected]>
Signed-off-by: Giuseppe Maxia <[email protected]>
Signed-off-by: Giuseppe Maxia <[email protected]>
@@ -547,7 +547,7 @@ resource "vcd_vapp_vm" "{{.VMName}}" { | |||
|
|||
network { | |||
type = "org" | |||
name = vcd_network_routed.net.name | |||
name = vcd_vapp_org_network.vappAttachedNet.org_network_name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason behind all these changes from Routed to vApp Org network?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason for the failures was that we had the wrong reference for the network name.
Both references in the end result to the same name, but, when we refer to the vapp network, Terraform will wait for the VM to be destroyed before destroying the vapp network.
When we instead refer to the Org network, Terraform will try destruction in parallel. Sometimes the operation succeeds, and sometimes it fails.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also explained in the commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To add - this actually dates back to when we introduced vcd_vapp_org_network
resource instead of doing "attachment magic" inside vcd_vapp_vm
. Back then we tuned our examples and code in many places, however it looks there are still places left.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, this, indeed, reminds me of very similar issues with dependencies I had long time ago.
@@ -547,7 +547,7 @@ resource "vcd_vapp_vm" "{{.VMName}}" { | |||
|
|||
network { | |||
type = "org" | |||
name = vcd_network_routed.net.name | |||
name = vcd_vapp_org_network.vappAttachedNet.org_network_name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, this, indeed, reminds me of very similar issues with dependencies I had long time ago.
Some resources are not available in 10.1, and we skip the relevant tests
Fix wrong references in TestAccVcdVAppVmMultiNIC