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

Reboot - Fix command not found, add Apline support, fix Solaris command #49272

Merged
merged 3 commits into from Dec 11, 2018

Conversation

@samdoran
Member

samdoran commented Nov 29, 2018

SUMMARY

Fixes #46723 - Run setup to gather distribution information. Use that to run the correct command for Alpine.
Fixes #47131 - Run find module to search common paths to get the full path to the executable since secure_path is not always set on the managed node.
Fixes #48986 - Add arguments for older version of Solaris

Supersedes #45656

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

reboot.py

ADDITIONAL INFORMATION

This may need to be broken up into separate pull requests. It was easier to fix these all at once rather than piecemeal, but I can break them up if that's better.

@ansibot

This comment has been minimized.

Contributor

ansibot commented Nov 29, 2018

Hi @samdoran, thank you for submitting this pull-request!

click here for bot help

@samdoran samdoran requested review from nitzmahone and jborean93 Nov 29, 2018

@samdoran samdoran force-pushed the samdoran:issue/47131-reboot-bin-path branch from 24090e4 to 1a0c24e Nov 29, 2018

@ansibot ansibot removed the needs_rebase label Nov 29, 2018

@samdoran samdoran force-pushed the samdoran:issue/47131-reboot-bin-path branch 2 times, most recently from b430596 to 4ea5d52 Nov 29, 2018

@samdoran samdoran changed the title from [WIP] Reboot - Fix command not found, add Apline support, fix Solaris command to Reboot - Fix command not found, add Apline support, fix Solaris command Nov 29, 2018

@ansibot ansibot added core_review and removed WIP labels Nov 29, 2018

@samdoran samdoran force-pushed the samdoran:issue/47131-reboot-bin-path branch from 730a51c to ac7f3c8 Nov 29, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Nov 29, 2018

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

lib/ansible/plugins/action/win_reboot.py:73:28: too-few-format-args Not enough arguments for format string

click here for bot help

@ansibot ansibot added needs_revision and removed core_review labels Nov 29, 2018

@samdoran samdoran force-pushed the samdoran:issue/47131-reboot-bin-path branch from fd75df9 to 2cf2d7f Nov 29, 2018

@samdoran samdoran removed the needs_triage label Dec 4, 2018

@samdoran samdoran added this to In progress - (This means you are ACTIVELY working on this. WIP it. WIP it good.) in Ansible 2.8 Dec 5, 2018

@samdoran samdoran force-pushed the samdoran:issue/47131-reboot-bin-path branch from fcefcf6 to 84f2a63 Dec 5, 2018

@samdoran samdoran changed the title from [WIP] Reboot - Fix command not found, add Apline support, fix Solaris command to Reboot - Fix command not found, add Apline support, fix Solaris command Dec 5, 2018

@ansibot ansibot removed the WIP label Dec 5, 2018

samdoran added some commits Nov 28, 2018

Fix various bugs related in reboot
- Use format strings for consistency and improve debug log messages
- Use local variables instead of class attributes in order to be thread safe
- Run setup module to get distribution and version
- Run find module to get full path of shutdown command
- Use ansible_os_family and ansible_distribution to find commands and args
- Use same command for all Solaris/SunOS distributions
- Move delay calculations to properties
- Reliably check for module run failure
- Fix bug in run_test_command() that accidentally made the method work properly
- Use better exceptions rather than Exception
- Use dict literals rather than constructors
- Correct _check_delay() so it always returns a value, not None
- Don't store and return result in run_test_command() because it's not used anywhere
Improve tests
- add test for post reboot command that fails
- test negative values for delay parameters

@samdoran samdoran force-pushed the samdoran:issue/47131-reboot-bin-path branch from a8ef156 to 174464d Dec 6, 2018

@ansibot

This comment has been minimized.

@maxamillion

This comment has been minimized.

Contributor

maxamillion commented Dec 10, 2018

shipit

@ansibot ansibot added shipit and removed needs_revision labels Dec 10, 2018

@samdoran samdoran merged commit c1589c3 into ansible:devel Dec 11, 2018

1 check passed

Shippable Run 97292 status is SUCCESS.
Details

@samdoran samdoran deleted the samdoran:issue/47131-reboot-bin-path branch Dec 11, 2018

samdoran added a commit to samdoran/ansible that referenced this pull request Dec 11, 2018

[stable-2.7] Reboot - Fix command not found, add Apline support, fix …
…Solaris command (ansible#49272)

* Fix various bugs related in reboot

- Use format strings for consistency and improve debug log messages
- Use local variables instead of class attributes in order to be thread safe
- Run setup module to get distribution and version
- Run find module to get full path of shutdown command
- Use ansible_os_family and ansible_distribution to find commands and args
- Use same command for all Solaris/SunOS distributions
- Move delay calculations to properties
- Reliably check for module run failure
- Fix bug in run_test_command() that accidentally made the method work properly
- Use better exceptions rather than Exception
- Use dict literals rather than constructors
- Correct _check_delay() so it always returns a value, not None
- Don't store and return result in run_test_command() because it's not used anywhere
- add test for post reboot command that fails
- test negative values for delay parameters.
(cherry picked from commit c1589c3)

Co-authored-by: Sam Doran <sdoran@redhat.com>

@samdoran samdoran moved this from In progress - (This means you are ACTIVELY working on this. WIP it. WIP it good.) to Done - This means done, tests, integration, everything... obvi. in Ansible 2.8 Dec 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment