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

Jump to bottom

Adding dell ome device_info module #53438

Merged
merged 21 commits into from Jun 10, 2019
Merged

Adding dell ome device_info module #53438

merged 21 commits into from Jun 10, 2019

Conversation

Sajna-Shetty
Copy link
Contributor

SUMMARY

Submitting dellemc_ome_device_facts module for contribution

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

dellemc_ome_device_facts

ADDITIONAL INFORMATION
    "device_facts": [
            {
                "Actions": null,
                "AssetTag": null,
                "ChassisServiceTag": null,
                "ConnectionState": true,
                "DeviceManagement": [
                    {
                        "DnsName": "dnsname.host.com",
                        "InstrumentationName": "MX-12345",
                        "MacAddress": "11:10:11:10:11:10",
                        "ManagementId": 12345,
                        "ManagementProfile": [
                            {
                                "HasCreds": 0,
                                "ManagementId": 12345,
                                "ManagementProfileId": 12345,
                                "ManagementURL": "https://192.168.0.1:443",
                                "Status": 1000,
                                "StatusDateTime": "2019-01-21 06:30:08.501"
                            }
                        ],
                        "ManagementType": 2,
                        "NetworkAddress": "192.168.0.1"
                    }
                ],
                "DeviceName": "MX-0003I",
                "DeviceServiceTag": "MXL1234",
                "DeviceSubscription": null,
                "LastInventoryTime": "2019-01-21 06:30:08.501",
                "LastStatusTime": "2019-01-21 06:30:02.492",
                "ManagedState": 3000,
                "Model": "PowerEdge MX7000",
                "PowerState": 17,
                "SlotConfiguration": {},
                "Status": 4000,
                "SystemId": 2031,
                "Type": 2000
            }
        ]

@ansibot
Copy link
Contributor

ansibot commented Mar 7, 2019

cc @rajeevarakkal
click here for bot help

@ansibot ansibot added affects_2.8 This issue/PR affects Ansible v2.8 community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. new_module This PR includes a new module. new_plugin This PR includes a new plugin. remote_management Working Group: https://docs.ansible.com/ansible/latest/community/communication.html support:community This issue/PR relates to code supported by the Ansible community. labels Mar 7, 2019
@mattclay mattclay added the ci_verified Changes made in this PR are causing tests to fail. label Mar 7, 2019
@ansibot

This comment has been minimized.

Sign in to view
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. needs_triage Needs a first human triage before being processed. labels Mar 7, 2019
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Mar 7, 2019
@ansibot

This comment has been minimized.

Sign in to view
@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Mar 7, 2019
@ansibot

This comment has been minimized.

Sign in to view
@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Mar 7, 2019
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Jun 7, 2019
@ansibot
Copy link
Contributor

ansibot commented Jun 7, 2019

The test ansible-test sanity --test shebang [explain] failed with 2 errors:

test/units/module_utils/remote_management/dellemc/test_ome.py:1:1: unexpected non-module shebang: b'#!/usr/bin/python'
test/units/modules/remote_management/dellemc/ome/test_ome_device_info.py:1:1: unexpected non-module shebang: b'#!/usr/bin/python'

click here for bot help

@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Jun 7, 2019
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jun 7, 2019
@Sajna-Shetty
Copy link
Contributor Author

@felixfontein We have added unit test support now.Could you please check it and help us in merging this code?

@Sajna-Shetty
Copy link
Contributor Author

bot_status

@ansibot
Copy link
Contributor

ansibot commented Jun 7, 2019

Components

lib/ansible/module_utils/remote_management/dellemc/ome.py
support: community
maintainers: rajeevarakkal

lib/ansible/modules/remote_management/dellemc/ome/init.py
support: community
maintainers: rajeevarakkal

lib/ansible/modules/remote_management/dellemc/ome/ome_device_info.py
support: community
maintainers: rajeevarakkal

test/units/module_utils/remote_management/init.py
support: core
maintainers:

test/units/module_utils/remote_management/dellemc/init.py
support: core
maintainers:

test/units/module_utils/remote_management/dellemc/test_ome.py
support: core
maintainers:

test/units/modules/remote_management/dellemc/init.py
support: core
maintainers:

test/units/modules/remote_management/dellemc/ome/init.py
support: core
maintainers:

test/units/modules/remote_management/dellemc/ome/test_ome_device_info.py
support: core
maintainers:

Metadata

waiting_on: ansible
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
too many files or commits: False
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): 0
community_shipits (namespace maintainers): 0
ansible_shipits (core team members): 0
shipit_actors (maintainer or core team member): []
shipit_actors_other: []
automerge: automerge shipit test failed

click here for bot help

felixfontein
felixfontein approved these changes Jun 7, 2019
View reviewed changes
Copy link
Contributor

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Thanks for adding the tests!

shipit

@felixfontein
Copy link
Contributor

If nobody complains, I'll merge it on Monday.

@Sajna-Shetty
Copy link
Contributor Author

If nobody complains, I'll merge it on Monday.

Thank you :)

@Sajna-Shetty
Copy link
Contributor Author

bot_status

@felixfontein
Copy link
Contributor

rebuild_merge

@ansibot
Copy link
Contributor

ansibot commented Jun 10, 2019

Components

lib/ansible/module_utils/remote_management/dellemc/ome.py
support: community
maintainers: rajeevarakkal

lib/ansible/modules/remote_management/dellemc/ome/init.py
support: community
maintainers: rajeevarakkal

lib/ansible/modules/remote_management/dellemc/ome/ome_device_info.py
support: community
maintainers: rajeevarakkal

test/units/module_utils/remote_management/init.py
support: core
maintainers:

test/units/module_utils/remote_management/dellemc/init.py
support: core
maintainers:

test/units/module_utils/remote_management/dellemc/test_ome.py
support: core
maintainers:

test/units/modules/remote_management/dellemc/init.py
support: core
maintainers:

test/units/modules/remote_management/dellemc/ome/init.py
support: core
maintainers:

test/units/modules/remote_management/dellemc/ome/test_ome_device_info.py
support: core
maintainers:

Metadata

waiting_on: ansible
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
too many files or commits: False
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): 0
community_shipits (namespace maintainers): 0
ansible_shipits (core team members): 1
shipit_actors (maintainers or core team members): felixfontein
shipit_actors_other: []
automerge: automerge community support test failed

click here for bot help

@ansibot ansibot added shipit This PR is ready to be merged by Core and removed core_review In order to be merged, this PR must follow the core review workflow. labels Jun 10, 2019
@ansibot ansibot merged commit e25269e into ansible:devel Jun 10, 2019
@felixfontein
Copy link
Contributor

It got merged, and will be available in Ansible 2.9! Congrats @Sajna-Shetty! I'm sorry it took so long...

@Sajna-Shetty
Copy link
Contributor Author

It got merged, and will be available in Ansible 2.9! Congrats @Sajna-Shetty! I'm sorry it took so long...

Thanks for taking your time to review 👍 and merging it :)

@rajeevarakkal
Copy link
Contributor

Thanks @felixfontein, @Sajna-Shetty and Ansible Core Team

agowa pushed a commit to agowa/ansible-1 that referenced this pull request Jun 30, 2019
@Sajna-Shetty @agowa
Adding dell ome device_info module (ansible#53438)
d5c72f5 
* Adding device_facts module for contribution

* changes added for pylint error

* Updated code to solve ansible-test compile error

* Changes to avoide comile error added

* Review Comments changes update

* Avoided blank line

* pylint error changes

* Removed ansible_facts return in error case

* Updated description

* modules renamed

* changing from ansible_facts to device_info

* avoide pep8 error

* Updated sample output

* version changed to 2.9

* Changed Copyright license to BSD

* Changed 3-clause BSD license to 2-clause BSD

* Added unit test support for ome_device_info

* version change

* removed pylint error in unit test modules

* Avoid Sanity error for unit test modules

* updated version
@fbecker
Copy link
Contributor

fbecker commented Jul 28, 2019

Is there a reason why this module is located inside a separate folder inside of remote_management/dellemc? Just wondering since the developer guide states that modules should not be nested more than two folders deep.

@fbecker
Copy link
Contributor

fbecker commented Jul 28, 2019

Just noticed that the documentation page for the remote_management category is not displaying anything under dellemc. Maybe above is the reason for this?

@felixfontein
Copy link
Contributor

You're right, I think it should be one level up. I'll create a PR to fix this.

@felixfontein
Copy link
Contributor

(Same applies to the iDRAC modules in remote_management/dellemc/idrac.)

@felixfontein felixfontein mentioned this pull request Jul 28, 2019
Merged
@ansible ansible locked and limited conversation to collaborators Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ffdybuster ffdybuster ffdybuster left review comments

@felixfontein felixfontein felixfontein approved these changes

@rajeevarakkal rajeevarakkal rajeevarakkal approved these changes

Assignees
No one assigned
Labels
affects_2.8 This issue/PR affects Ansible v2.8 module This issue/PR relates to a module. new_contributor This PR is the first contribution by a new community member. new_module This PR includes a new module. new_plugin This PR includes a new plugin. remote_management Working Group: https://docs.ansible.com/ansible/latest/community/communication.html shipit This PR is ready to be merged by Core support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@Sajna-Shetty @ansibot @rajeevarakkal @bcoca @felixfontein @fbecker @ffdybuster @mattclay