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

plugins: simplify Nova flavor listing #579

Merged
merged 2 commits into from
Oct 18, 2024
Merged

Conversation

majewsky
Copy link
Contributor

@majewsky majewsky commented Oct 9, 2024

In Nova microversion 2.61, flavors.ListDetail() includes the extra specs, so we can save a ton of work querying them one flavor at a time. Gophercloud already has support for this, too.

Draft: I have not tested this yet. Feel free to have a look at the implementation; I will test this in QA after my leave. I have now tested this. There was a minor bug caused by the microversion bump which is now fixed. The test-scan-capacity in qa-de-1 went from needing 787 API calls to just 59. Nice improvement.

In Nova microversion 2.61, flavors.ListDetail() includes the extra
specs, so we can save a ton of work querying them one flavor at a time.
Gophercloud already has support for this, too.
@coveralls
Copy link

coveralls commented Oct 9, 2024

Coverage Status

coverage: 78.435% (-0.06%) from 78.498%
when pulling cad1d81 on simplify-flavor-listing
into 9ea9d1f on master.

Copy link
Member

@VoigtS VoigtS left a comment

Choose a reason for hiding this comment

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

I like beeing able to reduce on API calls. This looks good.

By increasing the Nova API microversion in the previous commit, the
hypervisor "id" field has changed from a number to a UUID. We are not
using this value anywhere, but whatever.
@majewsky majewsky marked this pull request as ready for review October 18, 2024 14:56
@VoigtS VoigtS merged commit 2fbae7d into master Oct 18, 2024
7 checks passed
@VoigtS VoigtS deleted the simplify-flavor-listing branch October 18, 2024 14:59
majewsky added a commit that referenced this pull request Oct 21, 2024
Pagination was introduced in microversion 2.33, so our microversion upgrade
in 2fbae7d (PR #579) broke this
accidentally. This was not caught in QA because the QA regions are not
big enough to reach a 4-digit hypervisor count.
majewsky added a commit that referenced this pull request Oct 21, 2024
Pagination was introduced in microversion 2.33, so our microversion upgrade
in 2fbae7d (PR #579) broke this
accidentally. This was not caught in QA because the QA regions are not
big enough to reach a 4-digit hypervisor count.
majewsky added a commit that referenced this pull request Oct 21, 2024
Pagination was introduced in microversion 2.33, so our microversion upgrade
in 2fbae7d (PR #579) broke this
accidentally. This was not caught in QA because the QA regions are not
big enough to reach a 4-digit hypervisor count.
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.

3 participants