Merge lp:~jose/charms/precise/tracks/fixes into lp:charms/tracks

Proposed by José Antonio Rey
Status: Merged
Merged at revision: 5
Proposed branch: lp:~jose/charms/precise/tracks/fixes
Merge into: lp:charms/tracks
Diff against target: 287 lines (+143/-46)
9 files modified
README (+0/-21)
README.md (+40/-0)
config.yaml (+1/-1)
hooks/config-changed (+31/-0)
hooks/db-relation-changed (+27/-13)
hooks/install (+37/-7)
hooks/stop (+3/-4)
hooks/website-relation-joined (+2/-0)
metadata.yaml (+2/-0)
To merge this branch: bzr merge lp:~jose/charms/precise/tracks/fixes
Reviewer Review Type Date Requested Status
Charles Butler (community) Approve
Cory Johns (community) +1 Approve
Review via email: mp+215987@code.launchpad.net

Description of the change

 * Converted the README to Markdown
 * Added support to change the port number if it's already been configured

To post a comment you must log in.
Revision history for this message
Cory Johns (johnsca) wrote :

Jose,

Thank you very much for your contribution to this charm. Unfortunately, I was unable to deploy the charm, so I was unable to properly test your changes, though the error doesn't seem to be related to your changes here. The error I got was:

    INFO install Errno::ENOENT: No such file or directory - /Users/eweaver/p/configuration/gem_certificates/evan_weaver-original-private_key.pem
    INFO install An error occurred while installing has_many_polymorphs (2.13), and Bundler
    INFO install cannot continue.
    INFO install Make sure that `gem install has_many_polymorphs -v '2.13'` succeeds before
    INFO install bundling.
    ERROR juju.worker.uniter uniter.go:490 hook failed: exit status 5

I tried on two different environments and got the same error. If you were able to deploy the charm, was there anything you did differently to avoid this error?

Regarding your changes, they seem good, though I would suggest that you add indentation for the if blocks in the config-changed and db-relation-changed hooks, to make them more readable.

Again, thank you for this improvement, and if you can assist me getting this deployed, I can test it to give it my +1.

Revision history for this message
Cory Johns (johnsca) :
review: Needs Information
Revision history for this message
Marco Ceppi (marcoceppi) wrote :

Moved to "work in progress" pending feedback, once ready for another review please move to "Needs Review"

Revision history for this message
José Antonio Rey (jose) wrote :

Cory,

Googling a bit, this is a problem that has already been reported (see bug #1155358), and looks like some gems that ship with tracks are broken. We will need to figure out if there is some workaround for this first, I'll make sure to take a look into it soon.

Revision history for this message
José Antonio Rey (jose) wrote :

Charm now goes in the right track - fixed the errors and it should be good to go.

lp:~jose/charms/precise/tracks/fixes updated
9. By José Antonio Rey

Fixed some bugs, still need to fix upstart job

10. By José Antonio Rey

Fixed all errors, ready for review

Revision history for this message
Cory Johns (johnsca) wrote :

José,

Thanks so much for these improvements! This charm is so much more robust and secure for these changes!

However, I noticed an issue regarding the port handling: if you set the port to a small number initially, a log message indicating that the port is not allowed is recorded, but the upstart script is still created with the low-numbered port. Further, if you set the port to a high-numbered port and then later change the port number, because the .portnotsupported file doesn't exist, the the port check is skipped entirely and the port is not changed.

I think that the .portnotsupported file check can be removed entirely, and the port check in the install script should be moved down to wrap the creation of the upstart conf, with an else clause to remove the conf if it exists. The existing service will also need to be stopped before the upstart conf is changed or removed and restarted after the new conf is in place (though, there's also the database to consider, so there might need to be a .db-configured marker file or something, that is checked before starting the service from hooks/install).

Additionally, to allow this to work with a reverse proxy, as is the intention, I believe the website-relation-joined hook must be added, but I think it can be a simple boiler-plate hook, such as: http://pastebin.ubuntu.com/7412868/

Lastly, I had a question regarding the db-relation-changed hook: do you know if it matters if rake db:migrate is called multiple times? If it does, then that hook isn't idempotent and needs a check, but since the original hook doesn't have that, maybe it doesn't matter if it's called again.

lp:~jose/charms/precise/tracks/fixes updated
11. By José Antonio Rey

Fixed some bugs

12. By José Antonio Rey

Fixed a problem with ports check

Revision history for this message
José Antonio Rey (jose) wrote :

Hey Cory,

Fixed as discussed on IRC.

Thank you!

lp:~jose/charms/precise/tracks/fixes updated
13. By José Antonio Rey

Fixed redundancy

14. By José Antonio Rey

Fixed problem with brackets and non-executable website-relation-joined hook

15. By José Antonio Rey

Fixed port-changing blocker

Revision history for this message
Cory Johns (johnsca) wrote :

One small issue remaining: http://pastebin.ubuntu.com/7422057/

Looks like [[ "$PORT" < 1025 ]] should be [[ "$PORT" -lt 1025 ]]

lp:~jose/charms/precise/tracks/fixes updated
16. By José Antonio Rey

Changed < for -lt

Revision history for this message
Cory Johns (johnsca) wrote :

José,

Thank you very much for working with me to get this charm fixed, since it turned out to be broken upstream. With your changes, not only does the charm work again, but it is more secure (no longer running as root) and robust (using upstart, working with reverse proxy charms, etc). Excellent work!

I definitely give this MP my +1 and it should be merged by a charmer soon.

review: Approve (+1)
Revision history for this message
Charles Butler (lazypower) wrote :

Jose, thanks for the submission!

I've reviewed the associated content and everything seems to be in order. +1 and merging.

Keep up the great work!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== removed file 'README'
--- README 2013-02-21 03:31:54 +0000
+++ README 1970-01-01 00:00:00 +0000
@@ -1,21 +0,0 @@
1Tracks is a Ruby on Rails based web service that helps you to manage to
2do lists and get things done (GTD). Tracks is based on David Allen's Get
3Things Done (TM) methodology.
4
5To deploy:
6 juju bootstrap
7 juju deploy tracks
8 juju deploy mysql
9 juju add-relation mysql tracks
10 juju expose tracks
11
12 Then do a `juju status` to determine the IP of the service,
13 and just open it in your browser. On the initial screen you can
14 create your user, then sign-in and begin using Tracks. This charm
15 offers one configuration option, the port on which to run the web
16 server, default is 80. Tracks uses a built-in webserver, WEBrick.
17
18Links:
19
20- Homepage: http://getontracks.org/
21- Code: https://github.com/TracksApp/tracks
220
=== added file 'README.md'
--- README.md 1970-01-01 00:00:00 +0000
+++ README.md 2014-05-12 19:08:21 +0000
@@ -0,0 +1,40 @@
1# Overview
2
3Tracks is a Ruby on Rails based web service that helps you to manage to do lists
4and get things done (GTD). Tracks is based on David Allen's Get Things Done (TM)
5methodology.
6
7Please note that this charm uses the brightbox/ruby-ng PPA in order to get a
8newer version of Ruby.
9
10# Usage
11
12To deploy:
13
14 juju bootstrap
15 juju deploy tracks
16 juju deploy mysql
17 juju add-relation mysql tracks
18 juju expose tracks
19
20Then do a `juju status` to determine the IP of the service, and just open it in
21your browser. On the initial screen you can create your user, then sign-in and
22begin using Tracks. It uses a built-in webserver, WEBrick.
23
24## Configuration
25
26This charm offers one configuration option, the port on which to run the web
27server, default is 3000. At this point, it can not be ran in a port minor to
281025. If you set a port lower or equal to 1024, it will refuse to install
29until you change it. If you want to use port 80, for example, you can add a
30reverse proxy server, such as haproxy (also available in Juju).
31
32# Known Limitations
33
34As mentioned in the configuration part, the charm does not currently allow a
35port number lower than 1025.
36
37# Links
38
39- Homepage: <http://getontracks.org/>
40- Code: <https://github.com/TracksApp/tracks>
041
=== modified file 'config.yaml'
--- config.yaml 2012-11-27 22:20:45 +0000
+++ config.yaml 2014-05-12 19:08:21 +0000
@@ -1,5 +1,5 @@
1options:1options:
2 port:2 port:
3 type: int3 type: int
4 default: 804 default: 3000
5 description: The port number for the tracks webserver5 description: The port number for the tracks webserver
66
=== added file 'hooks/config-changed'
--- hooks/config-changed 1970-01-01 00:00:00 +0000
+++ hooks/config-changed 2014-05-12 19:08:21 +0000
@@ -0,0 +1,31 @@
1#!/bin/bash
2
3set -eux
4
5PORT=`config-get port`
6
7if [[ "$PORT" -lt 1025 ]]; then
8 juju-log "At this time, ports minor than 1024 are not supported. Please, change your port number."
9 exit 0
10fi
11
12if [[ -f .portnotsupported ]] && [[ "$PORT" -lt 1024 ]]; then
13 juju-log "Yay, you've set a valid port now. Installing!"
14 hooks/install
15 rm .portnotsupported
16fi
17
18if [ -f .port ] && [ `cat .port` != "${PORT}" ]; then
19 currentport=`cat .port`
20 juju-log "Switching port to ${PORT}"
21 close-port ${currentport}/tcp
22 open-port ${PORT}/tcp
23 echo "${PORT}" > .port
24 if [ -f .started ]; then
25 stop tracksapp
26 fi
27 sed -i "s/$currentport/$PORT/" /etc/init/tracksapp.conf
28 if [ -f .started ]; then
29 start tracksapp
30 fi
31fi
032
=== modified file 'hooks/db-relation-changed'
--- hooks/db-relation-changed 2012-11-25 02:33:40 +0000
+++ hooks/db-relation-changed 2014-05-12 19:08:21 +0000
@@ -17,30 +17,44 @@
17juju-log "Retrieved hostname: $hostname"17juju-log "Retrieved hostname: $hostname"
1818
19juju-log "configuring database.yml to use ${database}"19juju-log "configuring database.yml to use ${database}"
20cat > tracks/config/database.yml << EOF20cat > /home/tracks/tracks/config/database.yml << EOF
21production:21production:
22 adapter: mysql22 adapter: mysql2
23 database: tracks23 database: tracks
24 host: ${mysqlhost}24 host: ${mysqlhost}
25 username: ${user}25 username: ${user}
26 password: ${password}26 password: ${password}
27EOF27EOF
2828
29juju-log "stopping old instances"29if [ -f .started ]; then
30# XXX make this grep line more specific30 juju-log "stopping old instances"
31PID=`ps -ef | grep "ruby script/server" | grep -v grep | awk '{ print $2 }'`31 stop tracksapp
32if [[ ! -z ${PID} ]]; then32 rm .started
33 kill ${PID} || true
34fi33fi
3534
36juju-log "db:migrate"35juju-log "db:migrate"
37juju-log `pwd`36juju-log `pwd`
38cd tracks37cd /home/tracks/tracks
39bundle exec rake db:migrate RAILS_ENV=production38bundle exec rake db:migrate RAILS_ENV=production
4039
41PORT=`config-get port`40PORT=`config-get port`
42juju-log "Opening port ${PORT}"41if [ ! -f $CHARM_DIR/.port ]; then
43open-port ${PORT}/tcp42 juju-log "Opening port ${PORT}"
4443 open-port ${PORT}/tcp
45juju-log "Starting Tracks"44 echo "${PORT}" > $CHARM_DIR/.port
46bundle exec script/server -e production -p 8045elif [ -f $CHARM_DIR/.port ] && [ `cat $CHARM_DIR/.port` != "${PORT}" ]; then
46 currentport=`cat $CHARM_DIR/.port`
47 juju-log "Switching port to ${PORT}"
48 close-port ${currentport}/tcp
49 open-port ${PORT}/tcp
50 echo "${PORT}" > $CHARM_DIR/.port
51fi
52
53juju-log "Setting necessary permissions"
54chown -hR tracks /home/tracks/tracks
55
56if [ ! -f .started ]; then
57 juju-log "Starting Tracks"
58 start tracksapp
59 touch $CHARM_DIR/.started
60fi
4761
=== modified file 'hooks/install'
--- hooks/install 2013-02-21 03:31:54 +0000
+++ hooks/install 2014-05-12 19:08:21 +0000
@@ -5,17 +5,29 @@
55
6set -eux6set -eux
77
8PORT=`config-get port`
9
10juju-log "Checking if port is >1024"
11if [[ "$PORT" -lt 1025 ]]; then
12 juju-log "At this point, the use of ports minor than 1025 are not supported. Please, change your port number."
13 touch .portnotsupported
14 exit 0
15fi
16
17juju-log "creating user tracks"
18useradd -rm tracks
19
8juju-log "installing pre-reqs"20juju-log "installing pre-reqs"
21add-apt-repository ppa:brightbox/ruby-ng -y
9apt-get update22apt-get update
10apt-get install -qqy unzip ruby-full build-essential libmysqlclient-dev \23apt-get install -qqy unzip ruby2.1 build-essential libmysqlclient-dev \
11libruby sqlite3 libmysql-ruby libmysql-ruby libsqlite3-ruby libsqlite3-dev \24libruby sqlite3 libmysql-ruby libmysql-ruby libsqlite3-ruby libsqlite3-dev \
12imagemagick graphicsmagick libgraphicsmagick1-dev librmagick-ruby \25imagemagick graphicsmagick libgraphicsmagick1-dev librmagick-ruby \
13libxslt-dev libxml2-dev pwgen rubygems mysql-client wget26libxslt-dev libxml2-dev pwgen rubygems mysql-client wget ruby2.1-dev
1427
15juju-log "installing tracks"28juju-log "installing tracks"
16wget -O tracks.zip https://github.com/TracksApp/tracks/zipball/v2.129cd /home/tracks/
17unzip tracks.zip30git clone https://github.com/TracksApp/tracks -b 2.2_branch
18mv TracksApp-tracks-* tracks
19cd tracks31cd tracks
20chmod -R 664 *32chmod -R 664 *
21find -type d -exec chmod a+x '{}' \;33find -type d -exec chmod a+x '{}' \;
@@ -23,8 +35,6 @@
2335
24juju-log "installing bundler & rake"36juju-log "installing bundler & rake"
25gem install --source https://rubygems.org/ bundler rake37gem install --source https://rubygems.org/ bundler rake
26juju-log "correcting gemfile source"
27sed -i 's/http:\/\/gems.github.com/https:\/\/rubygems.org/' Gemfile
28juju-log "installing bundles"38juju-log "installing bundles"
29bundle install --without=test39bundle install --without=test
3040
@@ -37,3 +47,23 @@
3747
38juju-log "fixing htaccess"48juju-log "fixing htaccess"
39mv public/.htaccess public/dontneedhtaccess49mv public/.htaccess public/dontneedhtaccess
50
51juju-log "fixing error in prod env config"
52sed -i "s/config.assets.compile \= false/config.assets.compile \= true/" config/environments/production.rb
53
54juju-log "creating upstart job"
55cat > /etc/init/tracksapp.conf << EOF
56env FOLDER=/home/tracks/tracks
57env PORT=$PORT
58
59setuid tracks
60
61pre-start script
62chdir \$FOLDER
63end script
64
65script
66chdir \$FOLDER
67bundle exec rails server -e production -p \$PORT
68end script
69EOF
4070
=== modified file 'hooks/stop'
--- hooks/stop 2012-11-25 02:33:40 +0000
+++ hooks/stop 2014-05-12 19:08:21 +0000
@@ -1,8 +1,7 @@
1#!/bin/bash1#!/bin/bash
2juju-log "Stopping Tracks"2if [ -f .started ]; then
3PID=`ps -ef | grep "bundle exec" | grep -v grep | awk '{ print $2 }'`3 juju-log "Stopping Tracks"
4if [[ ! -z ${PID} ]]; then4 stop tracksapp
5 kill ${PID} || true
6fi5fi
7PORT=`config-get port`6PORT=`config-get port`
8juju-log "Closing port ${PORT}"7juju-log "Closing port ${PORT}"
98
=== added file 'hooks/website-relation-joined'
--- hooks/website-relation-joined 1970-01-01 00:00:00 +0000
+++ hooks/website-relation-joined 2014-05-12 19:08:21 +0000
@@ -0,0 +1,2 @@
1#!/bin/bash
2relation-set hostname=`unit-get private-address` port=`config-get port`
03
=== modified file 'metadata.yaml'
--- metadata.yaml 2012-11-27 22:20:45 +0000
+++ metadata.yaml 2014-05-12 19:08:21 +0000
@@ -6,6 +6,8 @@
66
7 This setup uses a mysql instance and the built-in webserver.7 This setup uses a mysql instance and the built-in webserver.
8maintainer: Matt Fischer <matt@mattfischer.com>8maintainer: Matt Fischer <matt@mattfischer.com>
9categories:
10 - applications
9requires:11requires:
10 db:12 db:
11 interface: mysql13 interface: mysql

Subscribers

People subscribed via source and target branches

to all changes: