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

Bump default API version to V36.0 (VCD 10.3+) #500

Merged
merged 11 commits into from
Aug 25, 2022

Conversation

Didainius
Copy link
Collaborator

@Didainius Didainius commented Aug 19, 2022

VCD 10.2 is going EOL for 2022-10-15 and the next release is going to be later than that therefore bumping default API version to V36.0. (for those with explicit needs there is an option to override the default version by using GOVCD_API_VERSION environment variable).

Additionally, this PR removes conditional checks that were being used for versions < 36.0

It should also fix all currently known flaky tests, which should make test run clean:

  • Auth endpoint returned URLs changed

From

<VersionInfo deprecated="true">
    <Version>35.0</Version>
    <LoginUrl>https://HOST/api/sessions</LoginUrl>
</VersionInfo>

To

<VersionInfo deprecated="false">
  <Version>36.0</Version>
  <LoginUrl>https://HOST/cloudapi/1.0.0/sessions</LoginUrl>
  <ProviderLoginUrl>https://HOST/cloudapi/1.0.0/sessions/provider</ProviderLoginUrl>
</VersionInfo>

Test TestClient_getloginurl started failing immediately because it relied on a matching path which has changed.

  • VCD introduced new state for VM PARTIALLY_POWERED_OFF in 10.3. Test Test_VMToggleHardwareVirtualization failed because function ToggleHardwareVirtualization specifically relied on VM being POWERED_OFF. This function is adjusted to also match PARTIALLY_POWERED_OFF
START: vm_test.go:610: TestVCD.Test_VMToggleHardwareVirtualization
vm_test.go:646:
    check.Assert(err, IsNil)
... value *errors.errorString = &errors.errorString{s:"hardware virtualization can be changed from powered off state, status: PARTIALLY_POWERED_OFF"} ("hardware virtualization can be changed from powered off state, status: PARTIALLY_POWERED_OFF")

FAIL: vm_test.go:610: TestVCD.Test_VMToggleHardwareVirtualization
  • VCD introduced new state for VM PARTIALLY_POWERED_OFF in 10.3. Test_VMPowerOnPowerOff started failing.
START: vm_test.go:666: TestVCD.Test_VMPowerOnPowerOff
vm_test.go:710:
    check.Assert(vmStatus, Equals, "POWERED_OFF")
... obtained string = "PARTIALLY_POWERED_OFF"
... expected string = "POWERED_OFF"

FAIL: vm_test.go:666: TestVCD.Test_VMPowerOnPowerOff
  • Fix Test_RemoveAllNetworks by ensureing vApp is powered off before removing all networks.
START: vapp_test.go:611: TestVCD.Test_RemoveAllNetworks
vapp_test.go:670:
    check.Assert(err, IsNil)
... value *errors.errorString = &errors.errorString{s:"task did not complete successfully:  [409:INVALID_STATE] - [ 35010cb4-48ac-4fe2
-a4c2-f6af984e243c ] The requested operation could not be executed on vApp \"f54723e7-638f-4255-8ca0-0255c9c93e29\". Stop the vApp and
 try again."} ("task did not complete successfully:  [409:INVALID_STATE] - [ 35010cb4-48ac-4fe2-a4c2-f6af984e243c ] The requested oper
ation could not be executed on vApp \"f54723e7-638f-4255-8ca0-0255c9c93e29\". Stop the vApp and try again.")

FAIL: vapp_test.go:611: TestVCD.Test_RemoveAllNetworks

…n conditional checks

Signed-off-by: Dainius Serplis <dserplis@vmware.com>
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.

This is great, thank you! As I will be away for a few days, I'm giving an early LGTM, so you don't have to wait on me for merging.

My only request is to see what the whole test suite returns after this change, hopefully nothing complains :)

@Didainius
Copy link
Collaborator Author

My only request is to see what the whole test suite returns after this change, hopefully, nothing complains :)

Yeah. I am now fixing things up, but tests will take their time before this lands.

Signed-off-by: Dainius Serplis <dserplis@vmware.com>
Signed-off-by: Dainius Serplis <dserplis@vmware.com>
Signed-off-by: Dainius Serplis <dserplis@vmware.com>
@Didainius Didainius changed the title Bump default API version to V36.0 (VCD 10.3+) and remove older versio… Bump default API version to V36.0 (VCD 10.3+) Aug 22, 2022
Signed-off-by: Dainius Serplis <dserplis@vmware.com>
Signed-off-by: Dainius Serplis <dserplis@vmware.com>
Signed-off-by: Dainius Serplis <dserplis@vmware.com>
Signed-off-by: Dainius Serplis <dserplis@vmware.com>
Signed-off-by: Dainius Serplis <dserplis@vmware.com>
@Didainius Didainius marked this pull request as ready for review August 25, 2022 04:32
govcd/common_test.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@adambarreiro adambarreiro left a comment

Choose a reason for hiding this comment

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

Nice 😄

Signed-off-by: Dainius Serplis <dserplis@vmware.com>
@Didainius Didainius merged commit ab093d7 into vmware:main Aug 25, 2022
@Didainius Didainius deleted the bump-api-v36 branch August 25, 2022 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants