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

support python3, smaller fixes, work on Fedora for casl #311

Merged
merged 8 commits into from
Jan 18, 2019

Conversation

ericzolf
Copy link
Contributor

What does this PR do?

basically small changes I needed to get casl running on my laptop:

  • Select correct selinux python library depending on python version (hence the name of the branch)
  • Add check_mode false to force listing VMs even in check mode (check mode still doesn't go through but at least further)
  • Add a note that defaults variables must be overwritten in the host's inventory (and not on the VM's one)
  • Make the ISO unmount handler work by replacing with_items with loops (might also be a Python 3 vs. 2 thingy, but the solution should work also for Python 2)
  • Make virt-install command generation more readable by breaking it across multiple lines.

How should this be tested?

Just check that there is no regression on RHEL 7 / Python 2 (no new feature)

Is there a relevant Issue open for this?

n/a see GChat discussion on #casl

Other Relevant info, PRs, etc.

n/a

People to notify

cc: @redhat-cop/infra-ansible

@ericzolf
Copy link
Contributor Author

I've added two smaller fixes:

  1. avoid mounting the ISO at boot time, as it might break the boot process if the ISO or the directory has disappeared (happened to me),
  2. there is no reason to disable the Apache process if it was enabled before, hence removed (and the description of the task was anyway wrong).

Copy link
Contributor

@oybed oybed left a comment

Choose a reason for hiding this comment

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

Overall all good changes - thank you. A few nitpick/questions inline.

roles/virt-install/tasks/create_vm.yml Show resolved Hide resolved
roles/virt-install/tasks/virt-install.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@oybed oybed left a comment

Choose a reason for hiding this comment

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

LGTM

@oybed oybed merged commit ffb968d into redhat-cop:master Jan 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants