Skip to content

Handle nova api version > 2.87 for hypervisor#3028

Merged
stephenfin merged 2 commits intogophercloud:masterfrom
samuelallan72:fix-hypervisor-get
Aug 9, 2024
Merged

Handle nova api version > 2.87 for hypervisor#3028
stephenfin merged 2 commits intogophercloud:masterfrom
samuelallan72:fix-hypervisor-get

Conversation

@samuelallan72
Copy link
Copy Markdown

@samuelallan72 samuelallan72 commented Apr 10, 2024

NOTE: this is based on and tested with v1.5.0 (I'm testing for openstack-exporter/openstack-exporter#268 ). Please let me know if this should be rebased, targeted to v1 branch, etc.? Help appreciated for testing this. :)

Fixes #3026

@github-actions github-actions bot added the semver:patch No API change label Apr 10, 2024
github-actions[bot]
github-actions bot reviewed Apr 10, 2024
View reviewed changes
Copy link
Copy Markdown

@github-actions github-actions bot 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 submitting your first PR! Be sure that we will be looking at it but keep in mind
this sometimes takes a while.
Please let the maintainers know if your PR has not got enough attention after a few days.
If any doubt, please consult our PR tutorial.

@EmilienM
Copy link
Copy Markdown
Contributor

EmilienM commented May 6, 2024

nice addition, please rebase

@github-actions github-actions bot removed the semver:patch No API change label May 7, 2024
@samuelallan72
Copy link
Copy Markdown
Author

samuelallan72 commented May 7, 2024

Thanks @EmilienM , I've rebased it. :)

@github-actions github-actions bot added the semver:patch No API change label May 7, 2024
@github-actions github-actions bot added semver:patch No API change and removed semver:patch No API change labels May 29, 2024
@samuelallan72
Copy link
Copy Markdown
Author

samuelallan72 commented May 29, 2024 •
edited
Loading

@EmilienM I've rebased again and fixed the merge conflict. Could you take another look please? 🙂

@coveralls
Copy link
Copy Markdown

coveralls commented Jun 7, 2024

Coverage Status

coverage: 78.7% (+0.004%) from 78.696%
when pulling 24aa11d on samuelallan72:fix-hypervisor-get
into 09c5d7c on gophercloud:master.

@stephenfin
Copy link
Copy Markdown
Contributor

stephenfin commented Jul 17, 2024

Would it be possible to get a unit test for this? You can base it on the TestGetHypervisorEmptyCPUInfo test in openstack/compute/v2/hypervisors/testing/requests_test.go, though obviously you will need a different request handler that returns a v2.87-like response (i.e. no cpu_info field, among other things). Otherwise, this looks good to me. The functional test failure is unrelated (the test was broken and has been removed #3114)

@github-actions github-actions bot added semver:patch No API change and removed semver:patch No API change labels Jul 17, 2024
@github-actions github-actions bot removed the semver:patch No API change label Jul 17, 2024
@samuelallan72
Copy link
Copy Markdown
Author

samuelallan72 commented Jul 17, 2024

@stephenfin sure of course, thanks for the reminder. :) Added.

@github-actions github-actions bot added the semver:patch No API change label Jul 17, 2024
stephenfin
stephenfin approved these changes Jul 18, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@stephenfin stephenfin left a comment

Choose a reason for hiding this comment

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

Assuming CI is happy, LGTM

@coveralls
Copy link
Copy Markdown

coveralls commented Jul 18, 2024

Coverage Status

coverage: 78.741% (+0.003%) from 78.738%
when pulling 5f4d249 on samuelallan72:fix-hypervisor-get
into 9a4f157 on gophercloud:master.

@samuelallan72
Copy link
Copy Markdown
Author

samuelallan72 commented Aug 9, 2024

@EmilienM @stephenfin hi, is there anything else you need from me before this can be landed? :)

@stephenfin stephenfin added hold Do not merge and removed hold Do not merge labels Aug 9, 2024
@stephenfin stephenfin merged commit 5cb81d7 into gophercloud:master Aug 9, 2024
@samuelallan72 samuelallan72 deleted the fix-hypervisor-get branch August 12, 2024 00:12
@samuelallan72
Copy link
Copy Markdown
Author

samuelallan72 commented Aug 12, 2024

Thanks @stephenfin 😄

@samuelallan72
Copy link
Copy Markdown
Author

samuelallan72 commented Oct 10, 2024

@EmilienM @stephenfin is there any chance of backporting this fix to v1.14? :) We're hoping to get this fix in openstack-exporter. Thanks

@stephenfin
Copy link
Copy Markdown
Contributor

stephenfin commented Oct 11, 2024

I think that's more of an question for @pierreprinetti. I'm not sure if we're still doing backports to v1?

@samuelallan72
Copy link
Copy Markdown
Author

samuelallan72 commented Oct 15, 2024

Thanks, also do you know if this will be released to v2? I'm not sure what your release process looks like - which release should we target to get this patch? Thanks.

@kayrus
Copy link
Copy Markdown
Contributor

kayrus commented Oct 15, 2024

In theory this can be backported to v2.
Regarding the changes, this looks similar to #3159

@kayrus kayrus added the backport-v2 This PR will be backported to v2 label Oct 15, 2024
@gophercloud-backport-bot gophercloud-backport-bot bot mentioned this pull request Oct 15, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@stephenfin stephenfin stephenfin approved these changes

Assignees

No one assigned

Labels

backport-v2 This PR will be backported to v2 semver:patch No API change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Nova API > 2.87 breaks fetching cpu_info from hypervisor

5 participants

@samuelallan72 @EmilienM @coveralls @stephenfin @kayrus