V9 enhance script readability

[ghost]

the echo command sequence used to constitute an init script replaced with more appropriate cat command, which makes this part of installer script much more human readable...

[ghost]

NOTE: not (yet) tested!

[ghost]

but it should work normally :)

[Yenthe666]

@tvibliani

Thanks a lot for cleaning up this code and making it a lot more readable! I will give it a test run this afternoon. If everything works fine I will merge it 👍

[ghost]

I tried it on Debian Jessie, as sudo is not installed on debian by default, first I've got:

# ./odoo_install.sh 

---- Update Server ----
./odoo_install.sh: line 43: sudo: command not found

then installed sudo with: apt-get install sudo -y and it worked fine, but still were some issues, as few missed -y options of apt-get install command, that required me to enter "y" in order to continue installation. I added these -y options in previous commit. Also I missed unescaped cat command inside the outer cat that creates init script and the last commit above fixes this.

[ghost]

I still have problem (not related to the changes in this PR) with wkhtmltopdf as it's dependency libjpeg-turbo8 can't be found on Debian Jessie... the command used in the script "dpkg -i" installs the package regardless of dependency availability. so this installation script were completed successfully, but wkhtmlpdf is broken because of missing dependency and cause internal server error of odoo instance on Debian Jessie. it's better way to install the deb package with "gdebi" command, but I don't know if it's available by default on Ubuntu or not, so I do not know if it'll be safe to replace "dpkg -i " with "gdebi "

[ghost]

I should just adapt wkhtmltopdf download link to my distribution :) as per the download table at official download page

[Yenthe666]

@tvibliani

I did one run on an Ubuntu 14.04 and everything worked fine with your cleaned up code so far.
I'll do detailed checks tommorow but everything looks fine.
You're correct that one password is asked for at this step:

echo -e "\n---- Create custom module directory ----"
sudo su $OE_USER -c "mkdir $OE_HOME/custom"
sudo su $OE_USER -c "mkdir $OE_HOME/custom/addons"

I should fix that too.

[ghost]

@Yenthe666

I'd like to throw out all the sudo stuff from the script. We can put one check at the beginning of the script, like:

# Make sure only root can run our script
if [ "$(id -u)" != "0" ]; then
   echo "This script must be run as root" 1>&2
   exit 1
fi

then user will have to run the script as root, and no more need to have prefixed all the commands with sudo. then for example, installer script itself may be run with one line command(on Ubuntu): sudo ./odoo_install.sh OR $ sudo -u root ./odoo_install.sh, OR by becoming root first, with command: $ sudo su and then run installer normally: # ./odoo_install.sh and finally # ctrl+D will take out the user from root console... there is several ways of course. On Debian even simpler, there is Root Terminal, etc...

By doing so:

• Script will become more human readable, personally for me it's a bit harder to read with sudo stuff included, as almost all commands are prefixed by sudo, I've to skip the sudo part on each line and seek for a real command, in order to get sight what's going on... it'll be cleaner code without sudo.
• On Debian systems sudo command is not as popular as in Ubuntu, it's even not installed by default and may cause an error, as I mentioned above. Whereas with root user it'll run nicely without sudo stuff, in both systems..
• Finally, it'll fix the issue, when password for sudo is requested in the middle of script execution. Simply no sudo, no password request for sudo...

What you think about?

[ghost]

the pasword request possibly were come from "su", but there is no need for su, we can remove it as well

[ghost]

the fragment:

echo -e "\n---- Create custom module directory ----"
sudo su $OE_USER -c "mkdir $OE_HOME/custom"
sudo su $OE_USER -c "mkdir $OE_HOME/custom/addons"

will become:

echo -e "\n---- Create custom module directory ----"
mkdir -p $OE_HOME/custom/addons
chown $OE_USER:$OE_USER $OE_HOME/custom/addons

that's it, no sudo, no su and no interactive mode in the middle of installation.

[Yenthe666]

@tvibliani

I'm not very experience with pull requests and Github so forgive me for not pulling this in.
I've decided to make a new folder while we're further developing and testing this.
You can see your latest version here:
https://github.com/Yenthe666/InstallScript/blob/master/development/odoo_install.sh
I've also placed in the code to remove the su part for the custom folders.

As for removing all sudo parts and only allowing root installations with this code:

# Make sure only root can run our script
if [ "$(id -u)" != "0" ]; then
   echo "This script must be run as root" 1>&2
   exit 1
fi

I understand what you want to do and the idea is nice but it has a major downside. A lot of users that have access to the system have no root acces but enough rights to execute scripts like this. When we'd limit this script to root you would need the root credentials and acces to execute this script, which is in my opinion more of a downside than a plus side. Don't you think?

[ghost]

@Yenthe666

I've NOT removed any sudo yet, and I'm not going to. at least in this PR. I wanted to discuss such a possiblility first. But script is already limited to sudoers, such a change will not add necessity for root credentials, as any sudoer can run the script after. Now we have to run ./odoo_install.sh and all the commands are prefixed by sudo inside the script, after we'll have sudo ./odoo_install.sh and no sudo prefix on each line inside the script. That does NOT really changes any current limitations. Probably the message echo "This script must be run as root" 1>&2 is not adapted well, probably it should be echo "This script must be run with admin rights" 1>&2 or something, that involve sudoers as well. also, there is several times used su root inside the script, so even with current state, it requires user to be able become root, through su... otherwise they can't run this script.

[Yenthe666]

@tvibliani

I know! I just decided to split this script up until it is done. When it is fully done & fully working I'll pull them over to the 9.0 script! 👍
Actually after reading your answer it might be a good idea to remove all the sudo commands from the script so we can run it with ./odoo_install.sh.
Have you thought about any downsides or found any?

[Yenthe666]

Note to self:

echo -e "\n---- Setting permissions on home folder ----"
sudo chown -R $OE_USER:$OE_USER $OE_HOME/*

This line still requests a password. Because of the sudo chown.

[ghost]

Note, if we remove sudo from inside, then it'll require sudo outside of script, so it'll be sudo ./odoo_install.sh for normal Ubuntu user.. but that's not a big deal.

You're right, there is too much changes for one PR, but I was not going to do so much at the beginning... first I've done only readability cleanup, but later I added new feature, that adapts download link of wkhtml2pdf to host architecture x64 or x32, because as I tried to install odoo9 with the original script on Ubuntu x32 machine, I've got error because of incompatible wkhtml2pdf (the wkhtmltox-0.12.1_linux-trusty-amd64.deb one):

This package is uninstallable
Wrong architecture 'amd64'

now it's fixed. I tested it on Ubuntu 14.4 - x32, and now download link were adapted correctly and installation were successful.

[Yenthe666]

@tvibliani

I understand. I have lots of ideas to continue this script though!
For now I've added all your modifications in the master folder, which you can find here: https://github.com/Yenthe666/InstallScript/blob/master/development/odoo_install.sh
I'd like to build in an option that automaticly takes the correct port specified under /etc/${OE_CONFIG}.conf and runs the Odoo directly on that port, also when the server is rebooted and the processed is restarted.
But I'm not 100% sure how to accomplish this safely. I'd assume by modifying the /etc/${OE_CONFIG}.conf structure so that it adds the -c /etc/${OE_CONFIG}.conf when you start/stop/restart the service.. Any good ideas on that? 🔹

Edit: The latest script version seems to work perfect on Ubuntu 14.04.

[ghost]

I think that's already managed to be that way. if you start/stop/restart an odoo instance using init script (the /etc/init.d/$OE_CONFIG one, or with sudo service $OE_CONFIG start/stop/restart ) then it always applies -c /etc/${OE_CONFIG}.conf to the odoo instance process... you can check that from terminal, using ps aux |grep $OE_CONFIG command. For me, ps -aux |grep odoo-server shows that odoo instance is running with following command: python /odoo/odoo-server/openerp-server -c /etc/odoo-server.conf so, if you configure/change port inside of /etc/${OE_CONFIG}.conf (/etc/odoo-server.conf in my case), then it should always run on the same port, as it's configured inside of config file...

[ghost]

@Yenthe666

I tested it on Debian Jessie, just with wkhtml2pdf download likns adapted, using following:

WKHTMLTOX_X64=http://download.gna.org/wkhtmltopdf/0.12/0.12.2.1/wkhtmltox-0.12.2.1_linux-jessie-amd64.deb
WKHTMLTOX_X32=http://download.gna.org/wkhtmltopdf/0.12/0.12.2.1/wkhtmltox-0.12.2.1_linux-jessie-i386.deb 

recommended wkhtml2pdf version according Odoo v8 Documentation is 0.12.1, but that version is not installable on Jessie, so I used 0.12.2 and above links are corresponding to this version. So far it woks fine (I printed some documents from Odoo).

A latest version is tested on Ubuntu 14.4 - x32 as well.

As I noted before, I'd like to remove sudo stuff and also there is a few things I do not like in the original script, for example odoo system user should NOT be in sudoers group and should NOT be created with --shell=/bin/bash, there is always more to do as we move forward. I think further changes should be considered as a separate task and I'll join you later for that in the separated development folder that you created, but I'm done with this PR I think. At this point installation script is working as expected. Please review and merge the PR.

Thanks,

[ghost]

fixes #3

[ghost]

@Yenthe666

I've advanced the installation script a bit, I wanted to propose that changes through new PR, but as I tried to do so, it showed up to me old changes from this PR. because of that it could not be a clean PR... so I'm waiting this pr to be merged and I'll make new PR after. meanwhile you can check latest version in "development" branch at my fork.

I'm agree with you about having development version separately, but separated folder is not an elegant way to do so. I've finally used separated branch at my fork, called "development" instead of separate folder. It should be nice if you create as well separated branch for development, if you still want to have separate development version (also note that other hand, script is small enough to edit it directly in place), so we'll be able to push some WIP to that branch, without altering baseline code in your master branch. Of course it's possible to do an other way around, to have development in master and create "production" branch for tested/finalised code, but separate development/production versions is better through branches than through directories I think.

[Yenthe666]

@tvibliani,

I decided to clean up my whole repo structure since this was clearly not working well.
I've now created branches for 8.0, 9.0 and master.
V8 = for Odoo V8 install scripts
V9 = for Odoo V9 install scripts
master = all development that is in progress / test and does not belong to 8.0/9.0

Could you create a new pull request against my current version 9.0 please? We'll continue from there on? 👍

[ghost]

@Yenthe666

Nice. I'll make 2 pull requests then, the part of changes that are present in this PR, are tested by both of us and it can be go directly to the 9.0 branch I think, but later changes that I've made at my side are not yet well tested, I'll create separated PR from that changes to master. so it'll two separated PR, one to 9.0 (tested code), another to master(later code). is that OK?

[ghost]

PR is moved. Please refer to #4

[Yenthe666]

@tvibliani

Thats perfect, thanks! I'll try to test and review the master version ASAP!
A major thanks for all the help and contributions :+1

[ghost]

@Yenthe666

Thank you for merge the PR. 👍

[Yenthe666]

@tvibliani

No problem! I've just did a testrun for the V9.0 script and it seems to work fine. It doesn't ask for any password anymore and everything runs fine out of the box. So the script seems to perfectly fine. 😄

[ghost]

👍