Skip to content
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

Add "reboot_vapp_on_removal" field to test's template #1031

Merged
merged 2 commits into from
Mar 27, 2023

Conversation

adezxc
Copy link

@adezxc adezxc commented Mar 27, 2023

Simple PR that adds reboot_vapp_on_removal: True field to the testAccCheckVappNetwork_basic_ipv6 template as it caused the binary test to fail.

Copy link
Collaborator

@Didainius Didainius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - tests passed.
You could add this PR to existing issue 1007

You would do it this way:

* Add `prefix_length` field to `vcd_vapp_network` as creating IPv6 vApp networks was not supported due to the lack of a suitable subnet representation (Issue #999) [GH-1007, GH-1031]

Copy link
Collaborator

@lvirbalas lvirbalas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did the test start failing after we introduced reboot_vapp_on_removal? If yes, I wonder why, as the parameter is optional?
https://registry.terraform.io/providers/vmware/vcd/latest/docs/resources/vapp_network#reboot_vapp_on_removal

p.s. Also, a nit: please capitalize the first word "Add ..." in the PR title :)

@adezxc adezxc changed the title add "reboot_vapp_on_removal" field to test's template Add "reboot_vapp_on_removal" field to test's template Mar 27, 2023
@adezxc
Copy link
Author

adezxc commented Mar 27, 2023

Did the test start failing after we introduced reboot_vapp_on_removal? If yes, I wonder why, as the parameter is optional?
I'm not sure as I don't really remember if the tests passed back then, but if the configuration was the same, then the tests should have failed.

Signed-off-by: Adam Jasinski <[email protected]>
@adezxc adezxc requested a review from lvirbalas March 27, 2023 09:06
@adezxc adezxc merged commit 12e76fd into vmware:main Mar 27, 2023
@adezxc adezxc deleted the fix_vapp_network_binary_test branch November 14, 2023 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants