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

VMware: Match powerstate of VM with vmware_guest_powerstate #56006

Open
wants to merge 1 commit into
base: devel
from

Conversation

@stonefix
Copy link

commented May 1, 2019

Fixed #55653

Renamed states:

  • poweredon
  • poweredoff
  • rebootguest
  • shutdownguest

to

  • powered-on
  • powered-off
  • reboot-guest
  • shutdown-guest
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

vmware_guest_powerstate
vmware_guest

Update vmware_guest.py
Renamed states: poweredon, poweredoff, rebootguest, shutdownguest to powered-on, powered-off, reboot-guest, shutdown-guest
@ansibot

This comment has been minimized.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 1, 2019

@stonefix, just so you are aware we have a dedicated Working Group for vmware.
You can find other people interested in this in #ansible-vmware on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

@Akasurde
Copy link
Member

left a comment

Please add integration test for this - here

@@ -49,25 +49,25 @@
configurations conforms to task arguments.'
- 'If C(state) is set to C(absent) and virtual machine exists, then the specified virtual machine
is removed with its associated components.'
- 'If C(state) is set to one of the following C(poweredon), C(poweredoff), C(present), C(restarted), C(suspended)
- 'If C(state) is set to one of the following C(powered-on), C(powered-off), C(present), C(restarted), C(suspended)

This comment has been minimized.

Copy link
@Akasurde

Akasurde May 2, 2019

Member

I would keep all previous states as well in-order to maintain backward compatibility.

This comment has been minimized.

Copy link
@Akasurde

Akasurde May 2, 2019

Member
Suggested change
- 'If C(state) is set to one of the following C(powered-on), C(powered-off), C(present), C(restarted), C(suspended)
- 'If C(state) is set to one of the following C(powered-on), C(poweredon), C(powered-off), C(poweredoff), C(present), C(restarted), C(suspended)
default: present
choices: [ present, absent, poweredon, poweredoff, restarted, suspended, shutdownguest, rebootguest ]
choices: [ present, absent, powered-on, powered-off, restarted, suspended, shutdown-guest, reboot-guest ]

This comment has been minimized.

Copy link
@Akasurde

Akasurde May 2, 2019

Member
Suggested change
choices: [ present, absent, powered-on, powered-off, restarted, suspended, shutdown-guest, reboot-guest ]
choices: [ present, absent, powered-on, powered-off, poweredon, poweredoff, restarted, suspended, shutdown-guest, reboot-guest, shutdownguest, rebootguest ]
@@ -2633,7 +2633,7 @@ def main():
raise AssertionError()
# VM doesn't exist
else:
if module.params['state'] in ['poweredon', 'poweredoff', 'present', 'restarted', 'suspended']:
if module.params['state'] in ['powered-on', 'powered-off', 'present', 'restarted', 'suspended']:

This comment has been minimized.

Copy link
@Akasurde

Akasurde May 2, 2019

Member

Same comment of all places.

@Akasurde Akasurde requested review from goneri and jillr May 2, 2019

@Akasurde Akasurde self-assigned this May 2, 2019

@Akasurde Akasurde removed the needs_triage label May 2, 2019

@@ -49,25 +49,25 @@
configurations conforms to task arguments.'
- 'If C(state) is set to C(absent) and virtual machine exists, then the specified virtual machine
is removed with its associated components.'
- 'If C(state) is set to one of the following C(poweredon), C(poweredoff), C(present), C(restarted), C(suspended)
- 'If C(state) is set to one of the following C(powered-on), C(powered-off), C(present), C(restarted), C(suspended)

This comment has been minimized.

Copy link
@Akasurde

Akasurde May 2, 2019

Member

Add a changelog entry like

@Akasurde Akasurde changed the title Update vmware_guest.py VMware: Match powerstate of VM with vmware_guest_powerstate May 2, 2019

@mattclay mattclay added the ci_verified label May 2, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 5, 2019

@Akasurde

This comment has been minimized.

Copy link
Member

commented May 16, 2019

@stonefix Thanks for providing this PR. I think PR #56187 also tries to solve the problem. Let me know if it is ok to close this in-favor of #56187 since it contains testcase as well.

@ansibot ansibot added the stale_ci label May 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.