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

Fixes 36950. Added support for missing options capabilities and root_device in properties of os_ironic.py ansible module #37113

Merged
merged 4 commits into from Dec 12, 2018

Conversation

sekharvajjula
Copy link
Contributor

@sekharvajjula sekharvajjula commented Mar 7, 2018

SUMMARY

Added support for missing options capabilities and root_device in properties of os_ironic.py ansible module

Fixes: #36950

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

os_ironic.py

ANSIBLE VERSION
2.5
ADDITIONAL INFORMATION

@ansibot
Copy link
Contributor

ansibot commented Mar 7, 2018

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/cloud/openstack/os_ironic.py:0:0: E305 DOCUMENTATION.options.properties.suboptions.root_device.description.0: expected str @ data['options']['properties']['suboptions']['root_device']['description'][0]. Got {'Root disk selections. eg': '/dev/sda'}

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Mar 7, 2018

@ansibot ansibot added bug This issue/PR relates to a bug. ci_verified Changes made in this PR are causing tests to fail. cloud module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_triage Needs a first human triage before being processed. openstack support:community This issue/PR relates to code supported by the Ansible community. labels Mar 7, 2018
@gundalow
Copy link
Contributor

gundalow commented Mar 7, 2018

default: ""
root_device:
description:
- Root disk selections. eg: /dev/sda
Copy link
Contributor

Choose a reason for hiding this comment

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

Having an extra : confuses the yaml parser, perhaps
Root disk selection, such as C(/dev/sda)

The example and documentation don't seem to match up. In the EXAMPLE it looks like it may support a list

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Mar 7, 2018
@opendev-zuul
Copy link

opendev-zuul bot commented Mar 7, 2018

Build succeeded (third-party-check pipeline).

@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, 2018
@opendev-zuul
Copy link

opendev-zuul bot commented Mar 7, 2018

Build succeeded (third-party-check pipeline).

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Mar 15, 2018
@sekharvajjula
Copy link
Contributor Author

Any comments?

@ansibot
Copy link
Contributor

ansibot commented May 11, 2018

cc @omgjlk
click here for bot help

Copy link
Contributor

@juliakreger juliakreger left a comment

Choose a reason for hiding this comment

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

You should change references from pike to latest. i.e. https://docs.openstack.org/ironic/latest

Additionally.... /dev/md0 would never really be valid. /dev/sda is better... but I can see the reference in our own docs in ironic. facepalm

@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label May 15, 2018
@opendev-zuul
Copy link

opendev-zuul bot commented May 15, 2018

Build succeeded (third-party-check pipeline).

@opendev-zuul
Copy link

opendev-zuul bot commented May 16, 2018

Build succeeded (third-party-check pipeline).

@ansibot ansibot added the affects_2.6 This issue/PR affects Ansible v2.6 label May 24, 2018
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. support:core This issue/PR relates to code supported by the Ansible Engineering Team. and removed support:community This issue/PR relates to code supported by the Ansible community. labels Sep 21, 2018
@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. support:community This issue/PR relates to code supported by the Ansible community. and removed core_review In order to be merged, this PR must follow the core review workflow. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Oct 7, 2018
@ansibot
Copy link
Contributor

ansibot commented Oct 15, 2018

cc @mnaser
click here for bot help

@mnaser
Copy link
Contributor

mnaser commented Oct 20, 2018

shipit

@ansibot
Copy link
Contributor

ansibot commented Oct 28, 2018

@sekharvajjula
Copy link
Contributor Author

Any plans when this change would be accepted?

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. community_review In order to be merged, this PR must follow the community review workflow. labels Dec 12, 2018
@gundalow gundalow added feature This issue/PR relates to a feature request. and removed bug This issue/PR relates to a bug. labels Dec 12, 2018
@gundalow
Copy link
Contributor

rebuild_merge

@odyssey4me
Copy link
Contributor

shipit

@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Dec 12, 2018
@opendev-zuul
Copy link

opendev-zuul bot commented Dec 12, 2018

Build succeeded (third-party-check pipeline).

@gundalow gundalow merged commit 2f8001a into ansible:devel Dec 12, 2018
@sekharvajjula
Copy link
Contributor Author

Thank you!!

kbreit pushed a commit to kbreit/ansible that referenced this pull request Jan 11, 2019
…device in properties of os_ironic.py ansible module (ansible#37113)

* Fix for 36950. Added support for missing options capabilities and root_device in properties of os_ironic.py ansible module

* Updated docstring to pass documentation validation

* Updated review comments from juliakreger

* version_added: "2.8"
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.6 This issue/PR affects Ansible v2.6 cloud community_review In order to be merged, this PR must follow the community review workflow. feature This issue/PR relates to a feature request. module This issue/PR relates to a module. openstack support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ansible module os_ironic missing options "capabilities" and "root_device"
6 participants