Tried to wrap some todos #16
Conversation
smitthakkar96
commented
Jul 10, 2017
smitthakkar96
commented
- Modified the code to create fstat user before we start everything
- Added task to create fstat user
- Added task to create virtualenv and pull/clone fstat code
- Added task to install dependencies if requirements file is updated
- Modified the code to create fstat user before we start everything - Added task to create fstat user - Added task to create virtualenv and pull/clone fstat code - Added task to install dependencies if requirements file is updated
|
@mscherer what should we set in selinux policy permissive/enforcing/disabled |
roles/fstat/tasks/main.yml
Outdated
| @@ -1,17 +1,35 @@ | |||
| --- | |||
| - user: name={{user}} | |||
| sudo: yes | |||
There was a problem hiding this comment.
The sudo is not needed, the playbook is executed as root.
There was a problem hiding this comment.
Also, I would prefer to use full yaml form (ie, name: rather than name=)
roles/fstat/tasks/main.yml
Outdated
| - name: Install required packages | ||
| package: | ||
| yum: |
There was a problem hiding this comment.
We should keep package, so the role is portable (like, on Fedora that use dnf)
roles/fstat/tasks/main.yml
Outdated
| file: path=/fstat/code state=directory | ||
|
|
||
| - name: create virtualenv if it does not exists | ||
| command: virtualenv /fstat/venv |
There was a problem hiding this comment.
This command will be run each time, I think that's suboptimal, shouldn't it verify that ?
roles/fstat/tasks/main.yml
Outdated
| # and selinux boolean | ||
|
|
||
| - name: create fstat directory | ||
| file: path=/fstat/code state=directory |
There was a problem hiding this comment.
Please use full yaml style here too.
|
@mscherer is it ok now or do we need any other changes and can you tell me what should we set as selinux boolean? |
roles/fstat/tasks/main.yml
Outdated
| path: /fstat/code state=directory | ||
|
|
||
| - name: Check if virtualenv exists | ||
| local_action: stat path=/fstat/venv |
There was a problem hiding this comment.
Please use the /fstat/env folder for virtualenv. Otherwise you'll break our current install.
|
Seems to be ok, could you squash the change in 1 commit ? |
|
Sure if I am not wrong github allows you to squash and merge, can you do that? |
|
We are not using github to merge PR. And rereading the PR, i did see I missed the the part about local_action, gonna comment on it. |
roles/fstat/tasks/main.yml
Outdated
| path: /fstat/code state=directory | ||
|
|
||
| - name: Check if virtualenv exists | ||
| local_action: stat path=/fstat/env |
There was a problem hiding this comment.
That is not correct, the local_action is gonna be used on the control system, not on the remote server.
roles/fstat/tasks/main.yml
Outdated
| register: env | ||
|
|
||
| - name: create virtualenv if it does not exists | ||
| command: virtualenv /fstat/env |
There was a problem hiding this comment.
You should use creates ( https://docs.ansible.com/ansible/latest/command_module.html )
There was a problem hiding this comment.
So I think that's not exactly clear on what I meant, but that's using creates: /fstat/venv so the task is not run each time.
|
@mscherer can you look at the PR if it is ready I can squash it |
| register: env | ||
|
|
||
| - name: creates virtualenv if it does not exists | ||
| command: virtualenv /fstat/env |
There was a problem hiding this comment.
Take a look at the creates parameter for the command module. That's what misc asked you to use. It makes the lines 21-24 and 28 redundant.
