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

WIP: Vagrant with provision #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dragonito
Copy link

Update

@dragonito dragonito changed the title WIP: Merge pull request #1 from tolry/master WIP: Vagrant with provision Nov 22, 2018
try_files $uri $uri/ /index.php?q=$uri&$args;
}

location ~ \.php$ {
Copy link
Owner

Choose a reason for hiding this comment

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

this pattern could be more restrictive, there is only one dispatcher file that needs to be accessible by php-fpm

Copy link
Author

Choose a reason for hiding this comment

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

right :D copy paste, thx for eagle eyes

- hosts: all
roles:
- role: app
become: true
Copy link
Owner

Choose a reason for hiding this comment

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

out of curiosity: why the become on role level? Most of the time I have mixed permission requirements per role for the different tasks, therefore always set this on a task level, blindly executing everything with root sounds a bit risky to me

Copy link
Author

Choose a reason for hiding this comment

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

This is my way for it. Set it to the roles not global. But i think this could be cleaned up for this special box


- name: Uninstall apache packages.
apt:
name: httpd
Copy link
Owner

Choose a reason for hiding this comment

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

since when is this called httpd in Debian/Ubuntu? sure this is not supposed to be apache

Copy link
Author

Choose a reason for hiding this comment

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

jipp, it could removed in this ubuntu version. Got problems with older boxes

name: httpd
state: absent

- name: start and enable nginx service.
Copy link
Owner

Choose a reason for hiding this comment

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

is this really needed? apt automatically starts the service normally or is this purely for the case that apache had previously blocked the port when nginx was installed? in that case, removing apache before nginx would suffice also without this additional step

Copy link
Author

Choose a reason for hiding this comment

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

Its to get shure that nginx will be restarted when adding my configuration for it, after copy processes I restart it

name: npm
state: present

- name: install yarn
Copy link
Owner

Choose a reason for hiding this comment

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

is this the recommended way of installing yarn right now? I thought their apt repo would be the better choice?

Copy link
Author

Choose a reason for hiding this comment

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

Jay :D its not the best way, i´ll have a look for it

@@ -0,0 +1,18 @@
---
- name: install node sources
shell: curl -sL https://deb.nodesource.com/setup_8.x | sudo -E bash -
Copy link
Owner

Choose a reason for hiding this comment

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

isn't bionic's default node version already 8?

Copy link
Author

Choose a reason for hiding this comment

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

I think its better to set the Version because of further updates


php_ini:
'date.timezone': "Europe/Berlin"
'memory_limit': 2048M
Copy link
Owner

Choose a reason for hiding this comment

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

wait what??

Copy link
Author

Choose a reason for hiding this comment

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

My defaults for projects ;) could remove it, if not needed

@@ -0,0 +1,3 @@
#!/usr/bin/env bash

apt-get -y install python
Copy link
Owner

Choose a reason for hiding this comment

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

Is this really necessary? I thought python was installed by default in Debian and Ubuntu for ages, might be mistaken though.

Not a vagrant expert, might be stupid question: ansible is available automatically or is it executed by the host system and needs to be installed there?

Copy link
Author

Choose a reason for hiding this comment

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

I got problems with provisioning without this pre-installation

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