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

vcd/resource_vcd_vapp_network.go: Check for RESOLVED status for powering off vApp #1092

Merged
merged 2 commits into from
Aug 24, 2023

Conversation

NilsBusche
Copy link
Contributor

@NilsBusche NilsBusche commented Jul 20, 2023

Fixes problem described in #1091
Closes #1091

@vmwclabot
Copy link
Member

@NilsBusche, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

1 similar comment
@vmwclabot
Copy link
Member

@NilsBusche, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

…ing off vApp.

Signed-off-by: Nils Busche <nils.busche@bertelsmann.de>
@Didainius
Copy link
Collaborator

@NilsBusche ,
Thanks for the PR!. I'd like to clarify when do you get it to RESOLVED state?
I will have to reinforce it with a test so want to find out how to replicate the problem

@NilsBusche
Copy link
Contributor Author

@Didainius vApp gets into RESOLVED, if you create it without any VMs in it or if you have a stopped vApp and remove all VMs from it.

@Didainius
Copy link
Collaborator

Thanks!

I have created a test that fails without your patch. The stupid part is that I can't make a PR to your repository as in the end it will probably ruin CLAbot (it doesn't like PRs that have multiple contributors).
Therefore I have put the patch to a gist - https://gist.github.com/Didainius/59b072055eb30949eed84dfe6c6a8621

Could you do a git apply patch.git after downloading that file. After it is here, we should be able to run a broader test suite to verify that nothing got broken along the way.

@NilsBusche
Copy link
Contributor Author

I've added the patch

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. I am good with it. I ran tests with vm vapp network tags on 10.3.3 (which wouldn't be impacted anyway) and 10.4.2.

vcd/resource_vcd_vapp_network_test.go Outdated Show resolved Hide resolved
vcd/resource_vcd_vapp_network_test.go Outdated Show resolved Hide resolved
vcd/resource_vcd_vapp_network_test.go Show resolved Hide resolved
…status.

Signed-off-by: Nils Busche <nils.busche@bertelsmann.de>
Copy link
Collaborator

@dataclouder dataclouder left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

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.

Thank you for contributing!

@Didainius Didainius removed the request for review from adambarreiro August 24, 2023 08:42
@Didainius Didainius merged commit bf00d5d into vmware:main Aug 24, 2023
2 checks passed
@Didainius
Copy link
Collaborator

Thank you @NilsBusche , this is merged and set for 3.11

Didainius added a commit to Didainius/terraform-provider-vcd that referenced this pull request Aug 24, 2023
Signed-off-by: Dainius Serplis <dserplis@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants