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.

Sign up for GitHub

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

Update hpilo_boot.py #24499

Merged
merged 4 commits into from
Jul 13, 2017
Merged

Update hpilo_boot.py #24499

merged 4 commits into from
Jul 13, 2017

Conversation

bushvin
Copy link
Contributor

@bushvin bushvin commented May 11, 2017

Add option to change the ssl version to connect to the remotr iLO

SUMMARY

Default ssl connections may be unsupported, or disabled due to security requirements, hence the need to be able to change the type used.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

hpilo_boot

ANSIBLE VERSION
2.3.0.0
ADDITIONAL INFORMATION

@bushvin
Update hpilo_boot.py
0cb443d 
Add option to change the ssl version to connect to the remotr iLO
@ansibot
Copy link
Contributor

ansibot commented May 11, 2017

cc @dagwieers @haad
click here for bot help

@ansibot ansibot added affects_2.4 This issue/PR affects Ansible v2.4 community_review In order to be merged, this PR must follow the community review workflow. feature_pull_request module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. 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. and removed community_review In order to be merged, this PR must follow the community review workflow. labels May 11, 2017
@bushvin
Update hpilo_boot.py
abba240 
added version_added
@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. and removed ci_verified Changes made in this PR are causing tests to fail. labels May 11, 2017
@bushvin
Update hpilo_boot.py
4de2536 
fix lib/ansible/modules/remote_management/hpilo/hpilo_boot.py:83:25: W291 trailing whitespace
@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 May 11, 2017
dagwieers
dagwieers approved these changes May 11, 2017
View reviewed changes
Copy link
Contributor

@dagwieers dagwieers left a comment

Choose a reason for hiding this comment

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

shipit

@dagwieers
Copy link
Contributor

shipit

dagwieers
dagwieers reviewed May 11, 2017
View reviewed changes
lib/ansible/modules/remote_management/hpilo/hpilo_boot.py Outdated
@@ -75,6 +75,12 @@
- As a safeguard, without force, hpilo_boot will refuse to reboot a server that is already running.
default: no
choices: [ "yes", "no" ]
ssl_version:
description:
- Change the ssl_version used
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to nitpick, I would have indented this one as the others, and have a trailing dot.
But those are not showstoppers.

dagwieers
dagwieers reviewed May 11, 2017
View reviewed changes
lib/ansible/modules/remote_management/hpilo/hpilo_boot.py Outdated
description:
- Change the ssl_version used
default: TLSv1
choices: ['SSLv3','SSLv23','TLSv1','TLSv1_1','TLSv1_2']
Copy link
Contributor

Choose a reason for hiding this comment

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

Another nitpick, the various items do not have to be quoted in YAML/JSON.
They have to be quoted for "yes" and "no" because these would otherwise be considered booleans.
Yes, YAML is tricky in many ways :-P

dagwieers
dagwieers reviewed May 11, 2017
View reviewed changes
lib/ansible/modules/remote_management/hpilo/hpilo_boot.py
@@ -147,8 +154,9 @@ def main():
image = module.params['image']
state = module.params['state']
force = module.params['force']
ssl_version = getattr(hpilo.ssl, 'PROTOCOL_' + module.params.get('ssl_version').upper().replace('V','v'))
Copy link
Contributor

Choose a reason for hiding this comment

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

The upper() and replace() calls here seem unneeded given that the input is very strictly formatted already.
But that's not a showstopper either.

@dagwieers
Copy link
Contributor

@bushvin If you could do the same for hpilo_facts, that would be appreciated.

@bushvin
Copy link
Contributor Author

bushvin commented May 11, 2017

Will do so @dagwieers

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label May 11, 2017
@bushvin
Update hpilo_boot.py
53d524f 
nitpicking fixes :)
@ansibot ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. needs_repo This PR no longer has an associated branch as it was removed by the submitter. support:community This issue/PR relates to code supported by the Ansible community. labels Jun 23, 2017
@haad
Copy link
Contributor

haad commented Jul 13, 2017

shipit

@haad
Copy link
Contributor

haad commented Jul 13, 2017

bot_status

@ansibot
Copy link
Contributor

ansibot commented Jul 13, 2017

waiting_on: maintainer
module: hpilo_boot
supported_by: community
maintainers: dagwieers haad
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
mergeable_state: clean
shippable_status: success
maintainer_shipits: 1
community_shipits: 0
ansible_shipits: 0
shipit_actors: haad

click here for bot help

@dagwieers
Copy link
Contributor

shipit

@ansibot ansibot added automerge This PR was automatically merged by ansibot. shipit This PR is ready to be merged by Core and removed community_review In order to be merged, this PR must follow the community review workflow. labels Jul 13, 2017
@ansibot ansibot merged commit 267d0fb into ansible:devel Jul 13, 2017
@dagwieers dagwieers mentioned this pull request Jan 24, 2018
Closed
@ansible ansible deleted a comment from ansibot Jan 24, 2018
@ansible ansible deleted a comment from ansibot Jan 24, 2018
@ansibot ansibot added feature This issue/PR relates to a feature request. and removed feature_pull_request labels Mar 4, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dagwieers dagwieers dagwieers approved these changes

Assignees
No one assigned
Labels
affects_2.4 This issue/PR affects Ansible v2.4 automerge This PR was automatically merged by ansibot. feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_repo This PR no longer has an associated branch as it was removed by the submitter. shipit This PR is ready to be merged by Core stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@bushvin @ansibot @dagwieers @haad