Skip to content
This repository has been archived by the owner on Apr 7, 2022. It is now read-only.

[1LP][RFR] Conversion of ConfigManager to use BaseProvider and BaseCollection #9317

Merged
merged 5 commits into from
Dec 10, 2019

Conversation

john-dupuy
Copy link
Contributor

@john-dupuy john-dupuy commented Sep 6, 2019

Converting config manager (i.e. Satellite and Ansible Tower Providers) to use BaseProvider and BaseCollection. This is an initial stab in order to get these providers up to speed with the rest of the framework.

With this enhancement, we'll be able to parametrize tests against config management providers in the same way that we parametrize tests against normal providers.

In addition to updating config managers to use BaseEntity and BaseCollection this pr also

  1. Removes duplicate ansible tower code, the entry_point for the ansible_tower_providers collection is now provided by cfme.infrastructure.config_management.ansible_tower
  2. Removes the test generation for config managers. This is now replaced by the stander pytest.mark.provider([AnsibleTowerProvider, SatelliteProvider])
  3. Added ConfigProfiles as a subcollection for ConfigManagers
  4. Added ConfigSystems as a subcollection for ConfigProfiles

Requires yaml MR 864 -> this MR brings in updates to our yamls so that the config managers are considered as normal providers.

Note PRT will fail due to the yaml update requirement. For test results search for the jenkin's job jdupuy-pr-9317-510z.

{{ pytest: --long-running cfme/tests/infrastructure/test_config_management.py cfme/tests/infrastructure/test_config_management_rest.py cfme/tests/services/test_config_provider_servicecatalogs.py cfme/tests/services/test_ansible_workflow_servicecatalogs.py
}}

@john-dupuy john-dupuy changed the title [WIP] initial commit of conversion of ConfigManager to Entity [WIP] Conversion of ConfigManager to use BaseEntity and BaseCollection Sep 9, 2019
@dajoRH dajoRH added needs-lint and removed lint-ok labels Sep 10, 2019
@john-dupuy john-dupuy force-pushed the config-manager-collection branch 2 times, most recently from 902beca to 6b04193 Compare September 10, 2019 17:29
@dajoRH dajoRH added lint-ok and removed needs-lint labels Sep 10, 2019
@john-dupuy
Copy link
Contributor Author

@nachandr I'd like your input on what I've done here. It is a pretty major refactor on the configuration management code.

@john-dupuy john-dupuy changed the title [WIP] Conversion of ConfigManager to use BaseEntity and BaseCollection [WIPTEST] Conversion of ConfigManager to use BaseEntity and BaseCollection Sep 11, 2019
@dajoRH dajoRH added WIP-testing and removed WIP labels Sep 11, 2019
@john-dupuy john-dupuy force-pushed the config-manager-collection branch 2 times, most recently from 5b3bafe to 168940e Compare September 18, 2019 18:10
@john-dupuy john-dupuy force-pushed the config-manager-collection branch from 168940e to 84ad7eb Compare September 20, 2019 15:33
@john-dupuy john-dupuy requested a review from nachandr September 23, 2019 15:38
@john-dupuy
Copy link
Contributor Author

PRT failures do not appear to be the result of these changes, marking RFR.

@john-dupuy john-dupuy changed the title [WIPTEST] Conversion of ConfigManager to use BaseEntity and BaseCollection [RFR] Conversion of ConfigManager to use BaseEntity and BaseCollection Sep 23, 2019
@lina-is-here
Copy link
Contributor

@john-dupuy please resolve the conflicts

@lina-is-here lina-is-here changed the title [RFR] Conversion of ConfigManager to use BaseEntity and BaseCollection [WIP] Conversion of ConfigManager to use BaseEntity and BaseCollection Oct 8, 2019
@john-dupuy john-dupuy force-pushed the config-manager-collection branch from 5fde130 to ff4459c Compare October 8, 2019 11:27
@john-dupuy john-dupuy changed the title [WIP] Conversion of ConfigManager to use BaseEntity and BaseCollection [RFR] Conversion of ConfigManager to use BaseEntity and BaseCollection Oct 8, 2019
@mshriver mshriver self-assigned this Oct 11, 2019
Copy link
Member

@mshriver mshriver left a comment

Choose a reason for hiding this comment

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

Couple small changes across the test modules and framework modules.

Want to see resolution to some of the PRT failures for AttributeError on the collection name, and handling of the object used for navigation (property object has no attribute server)

Copy link
Contributor

@digitronik digitronik left a comment

Choose a reason for hiding this comment

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

LGTM 👍

kind = "Configuration Profile"
if self.manager.type == "ansible_tower":
kind = "Inventory Group"
return kind
Copy link
Contributor

Choose a reason for hiding this comment

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

return if self.manager.type == "ansible_tower" else ""Configuration Profile""

prerequisite = NavigateToSibling('All')

@navigator.register(ConfigManagerProviderCollection, "All")
class ConfigManagerAllPage(CFMENavigateStep):
Copy link
Contributor

Choose a reason for hiding this comment

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

If no destination available then better to remove this still we will get proper error msg.

Copy link
Contributor Author

@john-dupuy john-dupuy Dec 10, 2019

Choose a reason for hiding this comment

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

            "There is no page in MIQ that displays all config managers."
            " Use 'AllOfType' on a config manager provider instance."
        )

The error message I provided provides some extra details about what you should do instead. Do you think we should remove that additional info?

Copy link
Member

Choose a reason for hiding this comment

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

Mixed feelings - but I do appreciate the extra context provided by the exception here.

I think if it stays, it should raise navmazing.NavigationDestinationNotFound instead of NotImplementedError.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified to raise NavigationDestinationNotFound

Copy link
Member

Choose a reason for hiding this comment

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

Good - I'm willing to accept as some 'tech-debt' to just remove it once callers are familiar with new structure and definition.

@digitronik digitronik changed the title [RFR] Conversion of ConfigManager to use BaseProvider and BaseCollection [1LP][WIPTEST] Conversion of ConfigManager to use BaseProvider and BaseCollection Dec 10, 2019
@john-dupuy john-dupuy force-pushed the config-manager-collection branch 2 times, most recently from 1311a56 to 4276f05 Compare December 10, 2019 15:42
@john-dupuy john-dupuy force-pushed the config-manager-collection branch from 4276f05 to 6ec5f9b Compare December 10, 2019 16:13
@dajoRH
Copy link
Contributor

dajoRH commented Dec 10, 2019

I detected some fixture changes in commit 6ec5f9b

Show fixtures

The local fixture config_system is used in the following files:

  • cfme/tests/infrastructure/test_config_management.py
    • test_config_system_tag
    • test_config_system_reprovision

The local fixture authentications is used in the following files:

  • cfme/tests/infrastructure/test_config_management_rest.py
    • _check_edited_authentications
    • test_query_authentications_attributes
    • test_authentications_edit_single
    • test_authentications_edit_multiple
    • test_delete_authentications_from_detail_post
    • test_delete_authentications_from_detail_delete
    • test_delete_authentications_from_collection

The local fixture config_manager_rest is used in the following files:

  • cfme/tests/infrastructure/test_config_management_rest.py
    • test_config_manager_create_rest
    • test_config_manager_edit_rest
    • test_config_manager_delete_rest

The local fixture ansible_workflow_catitem is used in the following files:

  • cfme/tests/services/test_ansible_workflow_servicecatalogs.py
    • test_tower_workflow_item
    • test_retire_ansible_workflow

The local fixture catalog_item is used in the following files:

  • cfme/tests/services/test_config_provider_servicecatalogs.py
    • test_order_tower_catalog_item
    • test_retire_ansible_service

Please, consider creating a PRT run to make sure your fixture changes do not break existing usage 😃

@jawatts jawatts assigned jawatts and unassigned mshriver Dec 10, 2019
@john-dupuy john-dupuy changed the title [1LP][WIPTEST] Conversion of ConfigManager to use BaseProvider and BaseCollection [1LP][RFR] Conversion of ConfigManager to use BaseProvider and BaseCollection Dec 10, 2019
@jawatts
Copy link
Contributor

jawatts commented Dec 10, 2019

Results from jenkins run look good. Any test failures will be handled in separate PRs.

@jawatts jawatts merged commit 04c00ba into ManageIQ:master Dec 10, 2019
spusateri pushed a commit to spusateri/integration_tests that referenced this pull request Jan 27, 2020
…llection (ManageIQ#9317)

* initial commit of conversion of ConfigManager to Entity

* First commit of conversion to provider

* Config manager as a provider updates

* Fix collection test_advanced_search

* Add GH blocker and use eq over cmp
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants