force the creation of odoo_workdir and odoo_rootdir #90

JordiBForgeFlow · 2018-07-04T21:39:08Z

The parameters 'odoo_workdir', 'odoo_rootdir' and 'odoo_config_data_dir' are defaulted to be under /home/{odoo user}

Line 17 in bb855f4

odoo_workdir: "/home/{{ odoo_user }}/odoo"

But that is not the correct to place applications, according to https://en.wikipedia.org/wiki/Filesystem_Hierarchy_Standard. In /home one should store user's personal files, not applications.

In my opinion a better place, according to my interpretation of the filesystem hierarchy standard, should be:
odoo_workdir: "/opt/odoo"
odoo_rootdir: "/opt/odoo/server"
odoo_config_data_dir: "/srv/odoo"

I can agree that this may not be necessarily the defaulted values. But the problem comes when I change the parameters to what I indicated above. In that case the playbook fails because there's no explicit task in the playbook that creates the odoo_workdir or odoo_rootdir.

In addition the odoo config file was hardcoded to be stored under /home/{odoo user}, but seems that /etc is generally a more accepted place to store config files. So I create a parameter 'odoo_config_file_dir' and force the existence during execution of the playbook.

sebalix

I agree we should comply with the FHS.
Regarding the odoo_config_file_dir (and maybe the others), a danger is to set it to /etc/ (resulting to /etc/odoo.conf) and then Ansible will set the owner of /etc to odoo:odoo. Should we protect against such common places?
We have some old standard installations running with the configuration file in /etc/odoo.conf in fact (for sure having a subdirectory /etc/odoo is better...).

About buildout failures, I'm trying to fix that in https://github.com/osiell/odoo-buildout-example .

sebalix · 2018-07-05T08:14:23Z

tasks/install.yml

+
+- name: Create odoo config file directory
+  file: path={{ odoo_config_file_dir }} state=directory
+        owner={{ odoo_user }} group={{ odoo_user }} force=yes


This task should not be executed for buildout installation, as anybox.recipe.odoo is in charge to create the config file in its own directory (./etc/odoo.cfg by default.).

sebalix · 2018-07-05T12:58:20Z

Buildout tests fixed.

JordiBForgeFlow · 2018-07-05T13:15:04Z

I have introduced some more changes according to your suggestions. My proposal is:

Filestore + downloaded modules and werkzeug sessions will be stored in {{ odoo_config_data_dir }}/{{ odoo_service }} (e.g. /srv/oca)
Therefore we try to ensure that a directory {{ odoo_config_data_dir }}/{{ odoo_service }} exists and is owned by {{ odoo_user }}

Application will be stored in {{ odoo_rootdir }}/{{ odoo_service }}/server (e.g. /opt/oca/server)
Therefore we try to ensure that a directory {{ odoo_rootdir }}/{{ odoo_service }}/server exists and is owned by {{ odoo_user }}

Configuration will be stored in {{ odoo_config_file_dir }}/{{ odoo_service }} (e.g. /etc/oca)
Therefore we try to ensure that a directory {{ odoo_config_file_dir }}/{{ odoo_service }} exists and is owned by {{ odoo_user }}

Odoo working dir will be stored in {{ odoo_workdir }}/{{ odoo_service }}
Therefore we try to ensure that a directory {{ odoo_workdir }}/{{ odoo_service }} exists and is owned by {{ odoo_user }}

sebalix · 2018-07-05T13:20:16Z

But odoo_config_data_dir is where the filestore + downloaded modules and werkzeug sessions are stored, they should not be saved under /etc/ but in /srv/odoo as you suggest, or /var/lib/odoo in Debian-like systems. Am I wrong?

JordiBForgeFlow · 2018-07-05T13:31:57Z

@sebalix Sorry, I corrected the texts.

JordiBForgeFlow · 2018-07-09T12:05:08Z

@sebalix Travis fails in some of the tests, but not sure if it is related to this changes.

sebalix · 2018-07-09T12:31:32Z

defaults/main.yml

@@ -12,10 +12,10 @@ odoo_init: True
 odoo_init_env: {}
    #VAR1: value1
    #VAR2: value2
-odoo_logdir: "/var/log/{{ odoo_user }}"
+odoo_logdir: "/var/log"


Why remove {{ odoo_user }}?

Because that's the base directory that will be used as the basis to create the specific odoo log directory, in the task:

name: Create log directory
file: path={{ odoo_logdir }}/{{ odoo_service }} state=directory
owner={{ odoo_user }} group={{ odoo_user }} force=no
tags:

odoo_log

So why not odoo_logdir = "/var/log/{{ odoo_service }}"?

Because then someone could do odoo_logdir = "/var/log" by mistake, and then the role would change the permissions of the /var/log directory, that is shared.

The idea that I had in order to prevent this kind of errors is that the *dir variables point to the root dir. the final directory is root dir + {{ odoo_service }}

So, {{ odoo_service }} will be used as a subdir of:
/{{ odoo_logdir }}/{{ odoo_service }} => For logs
/{{ odoo_workdir }}/{{ odoo_service }} => For work dir
/{{ odoo_rootdir }}/{{ odoo_service }}/server => For the server

sebalix · 2018-07-09T12:33:06Z

defaults/main.yml

-odoo_workdir: "/home/{{ odoo_user }}/odoo"
-odoo_rootdir: "{{ odoo_install_type == 'buildout' and '/home/'+odoo_user+'/odoo/parts/odoo' or '/home/'+odoo_user+'/odoo/server' }}"
+odoo_workdir: "/home/{{ odoo_user }}/{{ odoo_service }}"
+odoo_rootdir: "{{ '/home/'+odoo_user+'/'+odoo_service }}"


The odoo_rootdir is supposed to be the server directory (containing the odoo executable), which is /home/odoo/odoo/parts/odoo for buildout by default (need to be adapted), /home/odoo/odoo/server/ for standardandpip` (to adapt too). Or you want to change its meaning?

The problem to add absolute paths in a variable, is that someone could set odoo_rootdir = /opt and screw everything, because it would add permissions on odoo for the whole /opt.

sebalix · 2018-07-09T12:40:03Z

defaults/main.yml

@@ -39,7 +39,8 @@ odoo_postgresql_extensions:
    - unaccent

 # Standard installation/configuration options (odoo_install_type == 'standard')
-odoo_config_file: "/home/{{ odoo_user }}/{{ odoo_service }}.conf"
+odoo_config_file_dir: "/home/{{ odoo_user }}"


Maybe use directly {{ odoo_config_file | dirname}} to get the directory path without adding another user option.
Feel free to create a new variable in ./vars/main.yml if you want a variable odoo_config_file_dir, these options are not supposed to be change by the user.

In my opinion {{ odoo_config_file }} should only contain the name of the config file. Because if you want to specify a certain subdir for the odoo config file, how can you possibly create that dir?

Same as above, for any path that you indicate you need a step that ensures that this path exists and is assigned to the right user.

And in order to ensure that people do now screw the standard linux, the paths indicated in parameters would not be created directly, but /{{ odoo_service }}

I understand now, thank you, so we have to drop the compatibility to achieve this, it wasn't clear and I was trying to keep this compatible.
A major release number is also required for such change (./meta/main.yml, to update to 2.0?).

This can be a problem, since it seems that ansible does not seem to support role versioning ansible/proposals#23

It was just to "warn" the users that this new version break the compatibility with the previous ones, nothing more :)

How do we document that? will adding - version: 2.0 just work?

If you want to document this change, you could add a page in the wiki telling how to stay compatible with old releases (list old variable values) and link it on the README? For the moment it's enough.

sebalix · 2018-07-09T12:40:55Z

defaults/main.yml

@@ -106,13 +107,13 @@ odoo_config_custom: {}
    #your_option2: value2

 # Pip installation options (odoo_install_type == 'pip')
-odoo_pip_venv_path: "{{ odoo_workdir }}/sandbox"
+odoo_pip_venv_path: "{{ odoo_workdir }}/{{ odoo_service }}/sandbox"


odoo_workdir already includes the odoo_service value.

sebalix · 2018-07-09T12:41:05Z

defaults/main.yml

 odoo_pip_requirements_url: "file:///home/{{ odoo_user }}/requirements.txt"
 odoo_pip_odoo_bin_path: "{{ odoo_pip_venv_path }}/bin/{{ (odoo_version | int) < 10 and 'odoo.py' or 'odoo' }}"

 # Buildout installation options (odoo_install_type == 'buildout')
 odoo_buildout_version: 2.10.0
-odoo_buildout_venv_path: "{{ odoo_workdir }}/sandbox"
+odoo_buildout_venv_path: "{{ odoo_workdir }}/{{ odoo_service }}/sandbox"


Same remark.

sebalix · 2018-07-09T12:43:04Z

tasks/install.yml

@@ -25,7 +25,7 @@
    - odoo_install_ssh

 - name: Create log directory
-  file: path={{ odoo_logdir }} state=directory
+  file: path={{ odoo_logdir }}/{{ odoo_service }} state=directory


If odoo_service should be part of the odoo_logdir directory, include it directly in this last option, this ensures that the "Create log directory" will effectively create the directory set in odoo_logdir, without any surprise.

sebalix · 2018-07-09T12:44:45Z

tasks/install.yml

 - name: Create odoo data dir directory
-  file: path={{ odoo_config_data_dir }} state=directory
+  file: path={{ odoo_config_data_dir }}/{{ odoo_service }} state=directory


Same remark as for the "Create log directory" task / odoo_logdir option.

sebalix · 2018-07-09T12:45:41Z

vars/Ubuntu-16_Odoo-11.yml

@@ -96,7 +96,7 @@ odoo_buildout_build_dependencies:
    - libwebp-dev

 odoo_buildout_venv_cmd: "virtualenv --no-setuptools --python=python3 {{ odoo_buildout_venv_path }}"
-odoo_pip_venv_cmd: "virtualenv --python=python3 {{ odoo_pip_venv_path }}"
+º: "virtualenv --python=python3 {{ odoo_pip_venv_path }}"


force the creation of odoo_workdir and odoo_rootdir

c96fe54

JordiBForgeFlow requested a review from sebalix July 4, 2018 21:39

fixup!

0c6f02a

sebalix requested changes Jul 5, 2018

View reviewed changes

JordiBForgeFlow added 2 commits July 5, 2018 15:03

renames

4a4e683

fixup!

af4ef6d

JordiBForgeFlow added 2 commits July 5, 2018 15:21

fixup!

2f7be35

fixup!

3da087a

sebalix requested changes Jul 9, 2018

View reviewed changes

bobslee mentioned this pull request Jan 21, 2020

[IMP] Configurable odoo_rootdir. New odoo_serverdir is former odoo_rootdir #111

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

force the creation of odoo_workdir and odoo_rootdir #90

force the creation of odoo_workdir and odoo_rootdir #90

JordiBForgeFlow commented Jul 4, 2018 •

edited

Loading

sebalix left a comment

sebalix Jul 5, 2018

sebalix commented Jul 5, 2018

JordiBForgeFlow commented Jul 5, 2018 •

edited

Loading

sebalix commented Jul 5, 2018

JordiBForgeFlow commented Jul 5, 2018

JordiBForgeFlow commented Jul 9, 2018

sebalix Jul 9, 2018

JordiBForgeFlow Jul 9, 2018

sebalix Jul 9, 2018 •

edited

Loading

JordiBForgeFlow Jul 9, 2018

JordiBForgeFlow Jul 9, 2018 •

edited

Loading

sebalix Jul 9, 2018

JordiBForgeFlow Jul 9, 2018

sebalix Jul 9, 2018

JordiBForgeFlow Jul 9, 2018

sebalix Jul 9, 2018

JordiBForgeFlow Jul 9, 2018

sebalix Jul 9, 2018

JordiBForgeFlow Jul 9, 2018

sebalix Jul 9, 2018

sebalix Jul 9, 2018

sebalix Jul 9, 2018

sebalix Jul 9, 2018

sebalix Jul 9, 2018

sebalix Jul 9, 2018

force the creation of odoo_workdir and odoo_rootdir #90

Are you sure you want to change the base?

force the creation of odoo_workdir and odoo_rootdir #90

Conversation

JordiBForgeFlow commented Jul 4, 2018 • edited Loading

sebalix left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebalix commented Jul 5, 2018

JordiBForgeFlow commented Jul 5, 2018 • edited Loading

sebalix commented Jul 5, 2018

JordiBForgeFlow commented Jul 5, 2018

JordiBForgeFlow commented Jul 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebalix Jul 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JordiBForgeFlow Jul 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JordiBForgeFlow commented Jul 4, 2018 •

edited

Loading

JordiBForgeFlow commented Jul 5, 2018 •

edited

Loading

sebalix Jul 9, 2018 •

edited

Loading

JordiBForgeFlow Jul 9, 2018 •

edited

Loading