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

Adding role to configure hostname #249

Merged
merged 4 commits into from
Aug 22, 2018
Merged

Adding role to configure hostname #249

merged 4 commits into from
Aug 22, 2018

Conversation

oybed
Copy link
Contributor

@oybed oybed commented Aug 22, 2018

What does this PR do?

In preparation to "decouple" casl-ansible from the openshift-ansible-contrib repo, this role introduces the option to configure the hostname of a vm/instance/host.

This PR re-introduces the functionality of the hostnames role, with some clean-up and improvements.

See role's README for further details.

How should this be tested?

Update the inventory in the role's tests directory to mach your target host, then run the test.yml playbook

Is there a relevant Issue open for this?

N/A

Other Relevant info, PRs, etc.

N/A

People to notify

cc: @redhat-cop/infra-ansible

Copy link
Contributor

@tylerauerbeck tylerauerbeck left a comment

Choose a reason for hiding this comment

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

LGTM. One comment below regarding the use of become.


```
---
ansible_become: True
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM. Only comment I have is around ansible_become. I know in other roles we've gone down the path of just using become on the necessary tasks. Did we want to do this here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since all of these tasks requires elevated privileges, I think I could go either way. I can add a block to enforce it though - it's fine.

Copy link
Contributor

@tylerauerbeck tylerauerbeck left a comment

Choose a reason for hiding this comment

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

LGTM

@sabre1041 sabre1041 merged commit f95f7ae into redhat-cop:master Aug 22, 2018
tylerauerbeck pushed a commit to tylerauerbeck/infra-ansible that referenced this pull request Aug 31, 2018
* Adding role to configure hostname

* Updated README

* Updated to have a default value for 'hostname'

* Updating based on Tyler's feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants