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

Fix the ems_custom_attributes inventory collection #21623

Merged

Conversation

agrare
Copy link
Member

@agrare agrare commented Dec 16, 2021

This had a manager_ref of just :name but these belong to a polymorphic resource and just the name is not unique.

Adding the :resource to the manager_ref array makes this unique.

Spec test showing the failure: ManageIQ/manageiq-providers-vmware#771

Fixes ManageIQ/manageiq-providers-vmware#727

This had a manager_ref of just `:name` but these belong to a polymorphic
resource and just the name is not unique.

Adding the `:resource` to the manager_ref array makes this unique.
:manager_ref => %i(name),
:parent_inventory_collections => %i(vms)
:manager_ref => %i(resource name),
:parent_inventory_collections => %i(vms miq_templates)
Copy link
Member Author

Choose a reason for hiding this comment

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

This collection (vm_and_template_ems_custom_fields) is used by ovirt and exhibits the same bug, I am going to follow-up with a refactoring to get them both onto the same inventory_collection but want to keep this clean of refactorings for backport

@agrare
Copy link
Member Author

agrare commented Dec 16, 2021

@miq-bot cross-repo-tests ManageIQ/manageiq-providers-vmware#771, manageiq-providers-ovirt

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Dec 16, 2021
@miq-bot
Copy link
Member

miq-bot commented Dec 16, 2021

Checked commit agrare@128cef6 with ruby 2.6.3, rubocop 1.13.0, haml-lint 0.35.0, and yamllint
1 file checked, 3 offenses detected

app/models/manageiq/providers/inventory/persister/builder/infra_manager.rb

@agrare
Copy link
Member Author

agrare commented Dec 16, 2021

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Dec 16, 2021
@agrare
Copy link
Member Author

agrare commented Dec 16, 2021

@Fryguy PTAL cross repo is green

@Fryguy Fryguy merged commit 72021d9 into ManageIQ:master Dec 16, 2021
@agrare agrare deleted the fix_ems_custom_attributes_inventory_collection branch December 16, 2021 22:32
@Fryguy
Copy link
Member

Fryguy commented Jan 10, 2022

Backported to morphy in commit e8d5379.

commit e8d5379c601c35ff609e9c1514f8bcf4ae7d259b
Author: Jason Frey <[email protected]>
Date:   Thu Dec 16 17:31:54 2021 -0500

    Merge pull request #21623 from agrare/fix_ems_custom_attributes_inventory_collection
    
    Fix the ems_custom_attributes inventory collection
    
    (cherry picked from commit 72021d99d89fa9d698fbdf3c27847b441ccfe6b6)

Fryguy added a commit that referenced this pull request Jan 10, 2022
…tory_collection

Fix the ems_custom_attributes inventory collection

(cherry picked from commit 72021d9)
@Fryguy
Copy link
Member

Fryguy commented Jan 10, 2022

Backported to lasker in commit 48d43b2.

commit 48d43b21f55feec397c65d56688fb5a20aa0345f
Author: Jason Frey <[email protected]>
Date:   Thu Dec 16 17:31:54 2021 -0500

    Merge pull request #21623 from agrare/fix_ems_custom_attributes_inventory_collection
    
    Fix the ems_custom_attributes inventory collection
    
    (cherry picked from commit 72021d99d89fa9d698fbdf3c27847b441ccfe6b6)

Fryguy added a commit that referenced this pull request Jan 10, 2022
…tory_collection

Fix the ems_custom_attributes inventory collection

(cherry picked from commit 72021d9)
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.

A duplicate record was detected and destroyed
4 participants