-
Notifications
You must be signed in to change notification settings - Fork 36
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 processor pools as resource pools to inventory collection #431
Add processor pools as resource pools to inventory collection #431
Conversation
@miq-bot cross-repo-tests ManageIQ/manageiq#22228 |
From Pull Request: ManageIQ/manageiq-providers-ibm_cloud#431
@alizapeikes seems like this is primarily held up by IBM-Cloud/ibm-cloud-sdk-ruby#70 is that right? |
Yes. @jaywcarman is also working on fixing the powerVS refresher spec VCR so we can include resource_pools in the testing. |
8fe3abf
to
d1da2e4
Compare
@agrare |
With this PR, I can get my test pool from a PowerVS instance, and get it show up properly in a Resource Pool Summary report. |
ec3e9ac
to
fdd3d4c
Compare
I rebased this onto master to include the |
Update refresher VCR cassette to include new shared processor pool API calls.
Unfortunately, the PowerVS region we use for testing has limited capacity and setting too many affinity controls in the specs makes re-recording them a pain.
Only one cloud subnet is expected here - fixing the spec and removing the old TODO.
expect(vm).to have_attributes( | ||
:location => "unknown", | ||
:name => instance_name, | ||
:description => "PVM Instance", | ||
:vendor => "ibm_power_vs", | ||
:power_state => "on", | ||
:placement_group_id => placement_group.id, | ||
:placement_group_id => nil, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey so my only concern here is that we're no longer testing the placement_group lazy find here https://github.com/ManageIQ/manageiq-providers-ibm_cloud/pull/431/files#diff-f985d1bd514a044797c4062d1d8d94ff3da33e97f6bfa8d182955661184e7d45R64
We should also be testing the resource group lazy find https://github.com/ManageIQ/manageiq-providers-ibm_cloud/pull/431/files#diff-f985d1bd514a044797c4062d1d8d94ff3da33e97f6bfa8d182955661184e7d45R65
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. I switched assert_specific_vm
to use a VM that is still a member of a placement group so we can keep the lazy find coverage.
For the resource group lazy find we'll need to record another VCR cassette with a shared processor pool (the current cassette doesn't have any). fyi @miyamotoh
The "specific" VM used in the refresher spec was no longer a member of a placement group (see bed18fd). Using another VM in order to keep the placement_group lazy find spec coverage.
Checked commits alizapeikes/manageiq-providers-ibm_cloud@fdd3d4c~...868d892 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint |
Closing in favor of #443 |
Depends on: