Merge lp:~jtv/maas/single-cluster into lp:~maas-committers/maas/trunk

[Jeroen T. Vermeulen]

Single development db cluster per branch.

Allocate just a single local database cluster per branch, and make sure the "make" targets that need it will start it up. Also, rename maasdb.sh to maasdb and add a simple command set so it's easier to use.

Finally, update settings.py with the applicable changes we've made in development.py.

[Gavin Panella (allenap)]

[1]

-build: bin/buildout
+build: bin/buildout dev-db

I know it's early days, but we probably don't want to create the dev
database for just a build.

[2]

-run:
+run: build
  bin/django runserver 8000

If you agree with [1] then this needs to depend on build and dev-db.

[3]

-harness:
- . bin/maasdb.sh ; maasdb_init_db db/development disposable
+harness: dev-db
  bin/django shell

-syncdb:
+syncdb: dev-db
  bin/django syncdb

However, these don't depend on build, so perhaps run doesn't need to
either.

[4]

+main() {
+ COMMAND="$1"

COMMAND will end up in the calling environment. The local command
sorts this out:

main() {
    local COMMAND="$1"

Please also let's not pretend we're writing for a plain /bin/sh; let's
just use bash and not go quite as insane. Or, read [6] and use Python,
or just about anything *but* shell.

[5]

+if test -n "$*"
+then
+ main $*
+fi

This won't work if sourced within a script that itself has args, or if
"set -- $something" has been used in the shell before sourcing this
script. Because main() defaults to doing nothing if the argument does
not match I think it's safe to just call main() unconditionally.

[6]

+ case $COMMAND in
+ start) maasdb_init_db $* ;;
+ stop) maasdb_stop_cluster $* ;;
+ query) maasdb_query $* ;;
+ shell) maasdb_shell $* ;;
+ delete-cluster) maasdb_delete_cluster $* ;;
+ esac
...
+ main $*

Avoid $* because it rarely does the right thing (in bash at
least). Also, more quotes needed. The code above behaves badly if
there are spaces in the arguments passed in. I think the following
will work without surprises:

main() {
    COMMAND="$1"
    shift
    case "$COMMAND" in
        start) maasdb_init_db "$@" ;;
        stop) maasdb_stop_cluster "$@" ;;
        query) maasdb_query "$@" ;;
        shell) maasdb_shell "$@" ;;
        delete-cluster) maasdb_delete_cluster "$@" ;;
    esac
}

if [ $# -gt 1 ]
then
    main "$@"
fi

Similar modifications ought to be made throughout the script.

To be honest, having to remember crap like this makes me avoid shell
script these days for anything other than the very most trivial
things.

review: Needs Fixing

[Jeroen T. Vermeulen (jtv)]

Thanks for the review. I learned several new things.

> [1]
>
> -build: bin/buildout
> +build: bin/buildout dev-db
>
> I know it's early days, but we probably don't want to create the dev
> database for just a build.

Done.

> [2]
>
> -run:
> +run: build
> bin/django runserver 8000
>
> If you agree with [1] then this needs to depend on build and dev-db.

Done.

> [3]
>
> -harness:
> - . bin/maasdb.sh ; maasdb_init_db db/development disposable
> +harness: dev-db
> bin/django shell
>
> -syncdb:
> +syncdb: dev-db
> bin/django syncdb
>
> However, these don't depend on build, so perhaps run doesn't need to
> either.

It turned out they should have depended on build. Fixed.

> [4]
>
> +main() {
> + COMMAND="$1"
>
> COMMAND will end up in the calling environment. The local command
> sorts this out:

Done, thanks. Wasn't aware of "local."

> [5]
>
> +if test -n "$*"
> +then
> + main $*
> +fi
>
> This won't work if sourced within a script that itself has args, or if
> "set -- $something" has been used in the shell before sourcing this
> script. Because main() defaults to doing nothing if the argument does
> not match I think it's safe to just call main() unconditionally.

Note that this branch eliminates sourcing of the script entirely. I updated comments to remove any suggestion of sourcing it.

> [6]
>
> + case $COMMAND in
> + start) maasdb_init_db $* ;;
> + stop) maasdb_stop_cluster $* ;;
> + query) maasdb_query $* ;;
> + shell) maasdb_shell $* ;;
> + delete-cluster) maasdb_delete_cluster $* ;;
> + esac
> ...
> + main $*
>
> Avoid $* because it rarely does the right thing (in bash at
> least). Also, more quotes needed. The code above behaves badly if
> there are spaces in the arguments passed in. I think the following
> will work without surprises:
>
> main() {
> COMMAND="$1"
> shift
> case "$COMMAND" in
> start) maasdb_init_db "$@" ;;
> stop) maasdb_stop_cluster "$@" ;;
> query) maasdb_query "$@" ;;
> shell) maasdb_shell "$@" ;;
> delete-cluster) maasdb_delete_cluster "$@" ;;
> esac
> }

I wasn't aware of $@ either; reading up on it gave me a much clearer understanding of shell quoting. But since there is no longer any pretense that the functions in the script are for external use, I now shift both the command and the data dir (and fail if they are not given).

> if [ $# -gt 1 ]
> then
> main "$@"
> fi
>
> Similar modifications ought to be made throughout the script.

I don't see any other place where I'd need to make these particular changes.

> To be honest, having to remember crap like this makes me avoid shell
> script these days for anything other than the very most trivial
> things.

You've sold me. This script should not grow; it's just a minimal toolbox.

Jeroen

[Gavin Panella (allenap)]

Looks good, I think this will work well.

[7]

+main $*

I think this still needs to change to

main "$@"

to avoid whitespace issues.

review: Approve

[Gavin Panella (allenap)]

> I don't see any other place where I'd need to make these particular changes.

Ah, um, yeah :)

=== modified file '.bzrignore'
--- .bzrignore	2012-01-16 16:30:23 +0000
+++ .bzrignore	2012-01-17 10:01:24 +0000
@@ -2,6 +2,7 @@
 bin/django
 bin/test
 build
+db
 develop-eggs
 download-cache
 eggs

=== modified file 'Makefile'
--- Makefile	2012-01-16 15:53:24 +0000
+++ Makefile	2012-01-17 10:01:24 +0000
@@ -8,22 +8,26 @@
 	bin/buildout
 	@touch bin/buildout
 
-test:
+dev-db:
+	bin/maasdb start ./db/ disposable
+
+test: dev-db
 	bin/test
 
 lint:
 	pyflakes $(PYTHON_SRC)
 	pylint --rcfile=etc/pylintrc $(PYTHON_SRC)
 
-check: clean bin/buildout
+check: clean bin/buildout dev-db
 	bin/test
 
 clean:
 	find . -type f -name '*.py[co]' -exec rm -f {} \;
-	#rm -f bin/buildout
+	rm -f bin/buildout
 	#bzr clean-tree --unknown --force
 
-realclean: clean
+distclean: clean
+	bin/maasdb delete-cluster ./db/
 	rm -rf download-cache
 	rm -rf eggs
 	rm -rf develop-eggs
@@ -31,12 +35,11 @@
 tags:
 	bin/tags
 
-run:
+run: build dev-db
 	bin/django runserver 8000
 
-harness:
-	. bin/maasdb.sh ; maasdb_init_db db/development disposable
+harness: build dev-db
 	bin/django shell
 
-syncdb:
+syncdb: build dev-db
 	bin/django syncdb

=== modified file 'README.txt'
--- README.txt	2012-01-16 08:33:18 +0000
+++ README.txt	2012-01-17 10:01:24 +0000
@@ -3,21 +3,17 @@
 For more information about MaaS:
 https://launchpad.net/maas
 
+
 = Development MaaS server setup =
 
-You need a database for the maas server.
-
-You can choose to use the database of your choice (note that you will
-need to update the default values in src/maas/development.py if you choose
-to do so).
-
-If you want to use the default for a local development server (postgresql
-database engine, user: 'maas' with 'maas' as password, database name: 'maas'):
-
-    $ echo "CREATE USER maas WITH CREATEDB PASSWORD 'maas';" | psql -U postgres
-    $ createdb -E utf8 -O maas maas
-
-Setup the project (fetch all the required dependencies):
+Access to the database is configured in src/maas/development.py.
+
+The Makefile sets up a development database cluster inside your branch.  It
+lives in the "db" directory, which gets created on demand.  You'll want to
+shut it down before deleting a branch; see below.
+
+First, set up the project.  This fetches all the required dependencies, and
+creates a local database cluster and development database:
 
     $ make
 
@@ -30,3 +26,8 @@
     $ make run
 
 Point your browser to http://localhost:8000/
+
+To shut down the database cluster and clean up all other generated files in
+your branch:
+
+    $ make distclean

=== renamed file 'bin/maasdb.sh' => 'bin/maasdb' (properties changed: -x to +x)
--- bin/maasdb.sh	2012-01-16 14:49:10 +0000
+++ bin/maasdb	2012-01-17 10:01:24 +0000
@@ -1,15 +1,15 @@
-# MaaS database control functions
-#
-# The control functions take as their first argument a database cluster's data
+#! /bin/bash -e
+#
+# MaaS database control script
+#
+# Most functions take as their first argument a database cluster's data
 # directory.  This is where the database's socket, pidfile, log, and data will
-# reside.  The data directory must start with a slash.
+# reside.
 #
 # Some design choices for this module:
 #
 #  * Everything is PostgreSQL on Ubuntu.
-#  * POSIX shell.  Test your changes in dash, not just in bash.
-#  * Each branch gets its own cluster(s).  Kill & delete when done.
-#  * One database per cluster.  May change this later if it's a problem.
+#  * Each branch gets its own cluster.  Kill & delete when done.
 #  * Databases run under the system user that creates them.  No root required.
 #  * No global configuration apart from a basic PostgreSQL install.
 #  * Connections use Unix sockets.  No TCP port hogging.
@@ -18,10 +18,12 @@
 PGCTL="/usr/lib/postgresql/${POSTGRES_VERSION}/bin/pg_ctl"
 
 
-# Figure out the full data directory path for a given cluster, even if it's a
-# relative path.
+# Figure out the full absolute data directory path for a given cluster, even
+# if a relative path is given.
 maasdb_locate() {
+    local DATADIR
     DATADIR="$1"
+
     if test -z "$1"
     then
         echo "Specify a data directory for the MaaS database cluster." >&2
@@ -36,7 +38,9 @@
 
 # Create a database cluster.
 maasdb_create_cluster() {
+    local DATADIR
     DATADIR="`maasdb_locate "$1"`"
+
     if ! test -d "$DATADIR/base"
     then
         mkdir -p -- "$DATADIR"
@@ -47,6 +51,7 @@
 
 # Start a database cluster.
 maasdb_start_cluster() {
+    local DATADIR DISPOSABLE EXTRA_POSTGRES_OPTS
     DATADIR="`maasdb_locate "$1"`"
     # Pass "disposable" as the second argument if the data in this database
     # is not important at all and you're willing to cut corners for speed.
@@ -80,7 +85,9 @@
 
 # Stop a database cluster.
 maasdb_stop_cluster() {
+    local DATADIR
     DATADIR="`maasdb_locate "$1"`"
+
     if test -f "$DATADIR/postmaster.pid"
     then
         $PGCTL stop -D "$DATADIR"
@@ -90,12 +97,15 @@
 
 # Initialize a MaaS database.
 maasdb_init_db() {
+    local DATADIR DISPOSABLE MARKER
     DATADIR="`maasdb_locate "$1"`"
     # Pass "disposable" as the second argument if the data in this database
     # is not important at all and you're willing to cut corners for speed.
     DISPOSABLE="$2"
+
+    maasdb_start_cluster "$DATADIR" "$DISPOSABLE"
+
     MARKER="$DATADIR/maas-created"
-    maasdb_start_cluster "$DATADIR" "$DISPOSABLE"
     if ! test -f "$MARKER"
     then
         createdb -h "$DATADIR" maas && touch "$MARKER"
@@ -105,7 +115,9 @@
 
 # Open a psql shell on a MaaS database.
 maasdb_shell() {
+    local DATADIR
     DATADIR="`maasdb_locate "$1"`"
+
     maasdb_init_db "$DATADIR"
     psql -h "$DATADIR" maas
 }
@@ -113,8 +125,10 @@
 
 # Execute a query on a MaaS database.
 maasdb_query() {
+    local DATADIR QUERY
     DATADIR="`maasdb_locate "$1"`"
     QUERY="$2"
+
     maasdb_init_db "$DATADIR"
     psql -h "$DATADIR" maas -c "$QUERY"
 }
@@ -122,7 +136,9 @@
 
 # Delete an entire MaaS database and cluster.  Use only with extreme care!
 maasdb_delete_cluster() {
+    local DATADIR
     DATADIR="`maasdb_locate "$1"`"
+
     # Before deleting anything, does this at least look like a MaaS database
     # cluster?
     if test -d "$DATADIR/base"
@@ -131,3 +147,22 @@
         rm -rf -- "$DATADIR"
     fi
 }
+
+
+main() {
+    local COMMAND DATADIR
+    COMMAND="$1"
+    DATADIR="$2"
+    shift 2
+
+    case "$COMMAND" in
+        start) maasdb_init_db "$DATADIR" "$@" ;;
+        stop) maasdb_stop_cluster "$DATADIR" "$@" ;;
+        query) maasdb_query "$DATADIR" "$@" ;;
+        shell) maasdb_shell "$DATADIR" "$@" ;;
+        delete-cluster) maasdb_delete_cluster "$DATADIR" "$@" ;;
+    esac
+}
+
+
+main $*

=== modified file 'src/maas/development.py'
--- src/maas/development.py	2012-01-16 16:24:00 +0000
+++ src/maas/development.py	2012-01-17 10:01:24 +0000
@@ -24,7 +24,7 @@
         'NAME': 'maas',
         # For PostgreSQL, a "hostname" starting with a slash indicates a
         # Unix socket directory.
-        'HOST': '%s/db/development' % os.getcwd(),
+        'HOST': '%s/db' % os.getcwd(),
     }
 }
 

=== modified file 'src/maas/settings.py'
--- src/maas/settings.py	2012-01-16 16:15:44 +0000
+++ src/maas/settings.py	2012-01-17 10:01:24 +0000
@@ -17,12 +17,15 @@
 
 DATABASES = {
     'default': {
-        'ENGINE': 'django.db.backends.', # Add 'postgresql_psycopg2', 'postgresql', 'mysql', 'sqlite3' or 'oracle'.
-        'NAME': '',                      # Or path to database file if using sqlite3.
-        'USER': '',                      # Not used with sqlite3.
-        'PASSWORD': '',                  # Not used with sqlite3.
-        'HOST': '',                      # Set to empty string for localhost. Not used with sqlite3.
-        'PORT': '',                      # Set to empty string for default. Not used with sqlite3.
+        # 'postgresql_psycopg2', 'postgresql', 'mysql', 'sqlite3' etc.
+        'ENGINE': 'django.db.backends.postgresql_psycopg2',
+        'NAME': 'maas',
+        'USER': '',
+        'PASSWORD': '',
+        # For PostgreSQL, a "hostname" starting with a slash indicates a
+        # Unix socket directory.
+        'HOST': '',
+        'PORT': '',
     }
 }
 
@@ -33,7 +36,7 @@
 # timezone as the operating system.
 # If running in a Windows environment this must be set to the same as your
 # system time zone.
-TIME_ZONE = 'America/Chicago'
+TIME_ZONE = None
 
 # Language code for this installation. All choices can be found here:
 # http://www.i18nguy.com/unicode/language-identifiers.html