Merge lp:~dpb/charms/precise/landscape-client/add-landscape-relation into lp:~charmers/charms/precise/landscape-client/trunk

[David Britton]

- Main change: allow association to a landscape-server via relation. This is mostly useful for testing, but could be useful in the future for rapid deployment into an environment that already has a landscape server (which there is no public charm for yet)
- Update README with a couple changes I noticed during testing.
- Add 'tests/' jitsu-test-style test case. Still need to work with m_3 to figure out a couple oddities with it, but it does work for me at least
- centralize hooks into a hooks.py with symlinks
- Add ability to install from a branch (lp: syntax)

[Marco Ceppi (marcoceppi)]

Hi David, sorry it took so long to get around to this. So I'll be brief with the review. This LGTM! Thank you for supplying tests. I've got some more information for you below.

So, we're going to be dropping jitsu very soon as a lot of it isn't compatible with juju-core. I'm working on a real set of testing-helpers ("testing harness") that will replace jitsu-watch and test-helpers.sh with a more robust bash/python library. Keep an eye on the list for more information on that next week. I don't see it necessary to change what you have here but just be aware in the next few months as we move our automated testing environment from just pyjuju to both pyjuju + juju-core and eventually just juju-core that these tests will need to be amended.

review: Approve

=== modified file 'README'
--- README	2013-02-26 00:45:59 +0000
+++ README	2013-04-29 20:44:30 +0000
@@ -14,7 +14,6 @@
 landscape-client:
     account-name: <account_name_here>
     registration-key: <registration_key_here>
-    computer-title: <computer_title_here>
     tags: <csv_tag_list>
 
 The following is a version if you need to customize the communication URL
@@ -25,12 +24,11 @@
 landscape-client:
     account-name: standalone
     registration-key: 128-qosk-7382
-    computer-title: Dev Laptop 09
     tags: laptop,precise,developer
-    ping-url = http://landscape.example.com/ping
-    url = https://landscape.example.com/message-system
-    script-users = ALL
-    include-manager-plugins = ScriptExecution
+    ping-url: http://landscape.example.com/ping
+    url: https://landscape.example.com/message-system
+    script-users: ALL
+    include-manager-plugins: ScriptExecution
     
 Configuration
 =============
@@ -52,9 +50,6 @@
 registration-key:
     The account registration key, found in the Landscape account GUI.  
 
-computer-title:
-    Human readable title for your computer.
-
 tags:
     Comma separated list of tags to apply to the computer once it is
     registered.
 
=== added file 'TESTING'
--- TESTING	1970-01-01 00:00:00 +0000
+++ TESTING	2013-04-29 20:44:30 +0000
@@ -0,0 +1,5 @@
+Simple end-to-end testing can be performed by:
+
+  cd tests
+  make build
+  ./test.sh
 
=== modified file 'config.yaml'
--- config.yaml	2013-02-26 00:45:59 +0000
+++ config.yaml	2013-04-29 20:44:30 +0000
@@ -6,6 +6,7 @@
       distro (default), ppa:somecustom/ppa or a full APT url source
       entry with optional key. For example:
       deb https://[e-mail]/myrepo precise main|YOURAPTKEY"
+      Also supported is a public branch like lp:~landscape/landscape-client/trunk
     type: string
   data-path:
     description: |
 
=== modified file 'hooks/common.py'
--- hooks/common.py	2013-03-02 00:05:38 +0000
+++ hooks/common.py	2013-04-29 20:44:30 +0000
@@ -47,7 +47,12 @@
         stop_client_and_disable_init_script()
         self.exit_code = exit_code
 
-
+def clear_registration():
+    """
+    Do steps necessary to clear the client registration, so that next time
+    it will re-register
+    """
+    stop_client_and_disable_init_script()
 
 def try_to_register():
     """Try to register the client if needed.
@@ -58,6 +63,9 @@
     which means that it's expected that the configuration will be broken
     at that point.
 
+    The following keys need to be set for an attempt to be made:
+     - account_name, computer_title
+
     If an error is encountered, the client will be disabled and the hook
     calling this function will exit with an error code.
     """
@@ -68,7 +76,11 @@
     config = LandscapeSetupConfiguration()
     config.load([])
     config.silent = True
-    if configured or not config.get("computer_title"):
+    if configured:
+        print "Client already registered, skipping"
+        return 0
+    if not config.get("account_name") or not config.get("computer_title"):
+        print "Need account-name and computer-title to proceed, skipping"
         return 0
     try:
         setup(config)
 
=== added file 'hooks/common.pyc'
 Binary files hooks/common.pyc	1970-01-01 00:00:00 +0000 and hooks/common.pyc	2013-04-29 20:44:30 +0000 differ
=== added symlink 'hooks/config-changed'
=== target is u'hooks.py'
=== renamed file 'hooks/config-changed' => 'hooks/hooks.py'
--- hooks/config-changed	2013-02-01 11:55:50 +0000
+++ hooks/hooks.py	2013-04-29 20:44:30 +0000
@@ -3,11 +3,48 @@
 from subprocess import check_output
 import json
 import sys
-
-from common import update_client_config
-
-
-configs = json.loads(check_output(["config-get", "--format=json"]))
-if configs:
-    sys.exit(update_client_config(configs))
-
+import os
+import base64
+
+from common import update_client_config, clear_registration
+
+
+def _write_certificate(certificate, filename):
+    """
+    @param certificate Text of the certificate, base64 encoded.
+    @param filename Full path to file to write
+    """
+    with open(filename, "w") as file:
+        file.write(base64.b64decode(certificate))
+
+def _registration_relation():
+    cert_file = "/etc/ssl/certs/landscape_server_ca.crt"
+    data = json.loads(check_output(["relation-get", "--format=json"]))
+    if "ssl-public-key" in data:
+        _write_certificate(data["ssl-public-key"], cert_file)
+        data["ssl-public-key"] = cert_file
+    if "ping-url" in data:
+        config_changed(relation_data=data)
+
+def config_changed(relation_data={}):
+    """
+    @param relation_data: data from the context of a relation, it should
+                          match the data in the config
+    """
+    configs = json.loads(check_output(["config-get", "--format=json"]))
+    relation_data.update(configs)
+    if 'account-name' in relation_data:
+        sys.exit(update_client_config(relation_data))
+
+def registration_relation_joined():
+    _registration_relation();
+
+def registration_relation_changed():
+    _registration_relation();
+
+def registration_relation_departed():
+    clear_registration()
+
+if __name__ == "__main__":
+    hook = os.path.basename(sys.argv[0]).replace("-", "_")
+    eval("%s()" % hook)
 
=== modified file 'hooks/install'
--- hooks/install	2013-02-27 00:16:45 +0000
+++ hooks/install	2013-04-29 20:44:30 +0000
@@ -45,6 +45,17 @@
             chmod go+r $SOURCES_FILE
             return
             ;;
+        lp*)
+            rm -rf landscape-client-source; bzr branch $src landscape-client-source
+            cd landscape-client-source
+            apt_get install devscripts
+            apt_get build-dep landscape-client
+            DEBUILD_OPTS="-uc -us" make package
+            dpkg -i ../landscape-client_* ../landscape-common_* || /bin/true
+            apt_get -f install
+            cd ../
+            # Should no-op the install later.
+            ;;
         *)
             echo "Invalid repository origin specified: '" $src "'"
             ;;
 
=== added symlink 'hooks/registration-relation-changed'
=== target is u'hooks.py'
=== added symlink 'hooks/registration-relation-departed'
=== target is u'hooks.py'
=== added symlink 'hooks/registration-relation-joined'
=== target is u'hooks.py'
=== modified file 'metadata.yaml'
--- metadata.yaml	2013-02-08 17:37:09 +0000
+++ metadata.yaml	2013-04-29 20:44:30 +0000
@@ -11,3 +11,5 @@
   container:
     interface: juju-info
     scope: container
+  registration:
+    interface: http
 
=== modified file 'revision'
--- revision	2013-03-07 20:22:30 +0000
+++ revision	2013-04-29 20:44:30 +0000
@@ -1,1 +1,1 @@
-9
+10
 
=== added directory 'tests'
=== added file 'tests/001_install.test'
--- tests/001_install.test	1970-01-01 00:00:00 +0000
+++ tests/001_install.test	2013-04-29 20:44:30 +0000
@@ -0,0 +1,95 @@
+#!/bin/bash -eu
+
+source lib/test-helpers.sh
+
+test_setup() {
+    echo "INFO: Setup Test Environment"
+    juju deploy ubuntu
+    juju deploy --repository . local:landscape-client
+    juju add-relation ubuntu landscape-client
+}
+
+teardown() {
+    echo "INFO: Starting Teardown"
+    juju destroy-service ubuntu || /bin/true
+    juju destroy-service landscape-client || /bin/true
+}
+
+assert_command_on_unit() {
+    # Assert the command succeeds on the unit
+    # $1 = unit to contact
+    # $2 = command
+    if ! juju ssh $1 "$2"; then
+        error "CMD: $2 failed in $(diagnose)"
+        exit 1
+    fi
+}
+
+run_command_on_unit() {
+    juju ssh $1 "$2"
+}
+
+assert_config() {
+    # $1 = config
+    # $2 and on = list of strings to run through egrep
+    config=$1
+    shift
+    while [ $# -gt 0 ]; do
+        if ! echo "$config" | egrep "$1"; then
+            error "Config incorrect or missing: $1\n$(diagnose)"
+            exit 1
+        fi
+        shift
+    done
+}
+
+
+start-test "landscape-client add relation verify deploy"
+
+teardown
+test_setup
+
+# Ensure ubuntu started, and landscape-client is related without error
+jitsu watch --failfast \
+    ubuntu --state=started -r "ubuntu landscape-client" \
+    landscape-client --state=started
+
+
+# Ensure all ubuntu service units look as expected (unregistered)
+for i in $(jitsu get-service-info ubuntu public-address); do
+    unit=$(echo $i | cut -d: -f1)
+    host=$(echo $i | cut -d: -f2)
+    assert_command_on_unit $unit "ps -ef | grep -v /usr/bin/landscape-client"
+    assert_command_on_unit $unit "test -e /etc/landscape/client.conf"
+    assert_command_on_unit $unit "sudo cat /etc/landscape/client.conf"
+    assert_command_on_unit $unit "sudo grep computer_title /etc/landscape/client.conf | grep $unit"
+done
+
+# Set appropriate config for testing
+juju set --config lib/test-config.yaml landscape-client
+
+# We expect error since we aren't actually going to register the client
+jitsu watch \
+    landscape-client --state=configure-error
+
+# Ensure all ubuntu service units look as expected (registration-attempt)
+for i in $(jitsu get-service-info ubuntu public-address); do
+    unit=$(echo $i | cut -d: -f1)
+    host=$(echo $i | cut -d: -f2)
+    assert_command_on_unit $unit "ps -ef | grep -v /usr/bin/landscape-client"
+    assert_command_on_unit $unit "test -e /etc/landscape/client.conf"
+    config=$(run_command_on_unit $unit "sudo cat /etc/landscape/client.conf")
+    assert_config "$config" \
+        "ping_url = http://foo.example.com/ping" \
+        "data_path = /var/lib/landscape/client" \
+        "computer_title = $unit" \
+        "tags = foo,bar,baz" \
+        "registration_password = foo-registration-key" \
+        "url = https://foo.example.com/message-system" \
+        "include_manager_plugins = ScriptExecution" \
+        "script_users = ALL" \
+        "registration_key = foo-registration-key" \
+        "account_name = foo-account-name"
+done
+
+end-test "Deployed Successfully"
 
=== added file 'tests/Makefile'
--- tests/Makefile	1970-01-01 00:00:00 +0000
+++ tests/Makefile	2013-04-29 20:44:30 +0000
@@ -0,0 +1,8 @@
+build:
+	@mkdir -p precise
+	@test -e precise/landscape-client || ln -sf ../ precise/landscape-client
+	@echo "- Checking juju status, to make sure juju bootstrap has been done"
+	@juju status > /dev/null 2>/dev/null
+
+test:
+	./install-test
 
=== added directory 'tests/lib'
=== added file 'tests/lib/test-config.yaml'
--- tests/lib/test-config.yaml	1970-01-01 00:00:00 +0000
+++ tests/lib/test-config.yaml	2013-04-29 20:44:30 +0000
@@ -0,0 +1,8 @@
+landscape-client:
+    account-name: foo-account-name
+    registration-key: foo-registration-key
+    tags: foo,bar,baz
+    ping-url: http://foo.example.com/ping
+    url: https://foo.example.com/message-system
+    script-users: ALL
+    include-manager-plugins: ScriptExecution
 
=== added file 'tests/lib/test-helpers.sh'
--- tests/lib/test-helpers.sh	1970-01-01 00:00:00 +0000
+++ tests/lib/test-helpers.sh	2013-04-29 20:44:30 +0000
@@ -0,0 +1,114 @@
+#!/bin/bash
+
+set -eu
+
+# Test setup
+readonly TEST_NAME=$0
+
+# Test counts. Neither skips or errors are necessarily fatal.
+SKIPS=0
+ERRORS=0
+UNHANDLED=0
+
+
+function info() {
+    echo "INFO: $@"
+}
+
+function ok() {
+    echo "PASS: $@"
+}
+
+function skip() {
+    echo "SKIP: $@"
+    (( SKIPS +=1 ))
+}
+
+function error() {
+    echo "FAIL: $@"
+    (( ERRORS += 1 ))
+}
+
+
+function start-test() {
+    readonly DATADIR=$(mktemp -d --tmpdir "${TEST_NAME}.XXXXXXX")
+    info "Executing ${TEST_NAME} with results in $DATADIR: $1"
+    setup
+    trap trapped-teardown EXIT TERM INT
+}
+
+function setup() {
+    # Empty, override in actual test as needed
+    true
+}
+
+
+function end-test() {
+    info "Completed test $TEST_NAME"
+    trap - EXIT
+    test-teardown
+}
+
+
+function teardown() {
+    # Empty, override in actual test as needed
+    true
+}
+
+
+function test-teardown() {
+    teardown
+
+    if [[ "$ERRORS" -gt 0 ]] ; then
+        TEST_RESULT=1
+    elif [[ "$SKIPS" -gt 0 ]] ; then
+        TEST_RESULT=100
+        touch $DATADIR/passed
+    else 
+        TEST_RESULT=0
+        touch $DATADIR/passed
+    fi
+
+    if [ -n "$DATADIR" ] ; then
+        if [ -f $DATADIR/passed ]; then
+            info "Passed test results for ${TEST_NAME} in ${DATADIR}, skips=${SKIPS}"
+            # rm -r $DATADIR
+        else
+            info "Failed test results for ${TEST_NAME} in ${DATADIR}, errors=${ERRORS}, skips=${SKIPS}"
+            if [ -f $DATADIR/wget.log ] ; then
+                info "BEGIN wget.log"
+                cat $DATADIR/wget.log
+                info "END wget.log"
+            fi
+        fi
+    fi
+    exit $TEST_RESULT
+}
+
+
+function diagnose() {
+    # Unpack the caller info to get some useful diagnostics for
+    # tracing any assertion issues
+    local data=( $(caller 1) )
+    local lineno=${data[0]}
+    local function_name=${data[1]}
+    local script_name=${data[2]}
+    echo "function $function_name at line $lineno ($script_name)"
+}
+
+function trapped-teardown() {
+    (( UNHANDLED += 1 ))
+    error "Untrapped error in $(diagnose)"
+    test-teardown
+}
+
+function assert() {
+    # The assert function takes two args, such as:
+    # assert "what we are asserting" "non empty result passes"
+    # If the second arg is empty or undefined, the assertion fails
+    if [[ "$#" -lt 2 || -z "$2" ]] ; then
+        error "$1, assertion failed in $(diagnose)"
+    else
+        ok "$1"
+    fi
+}
 
=== added directory 'tests/precise'
=== added symlink 'tests/precise/landscape-client'
=== target is u'../..'
=== added file 'tests/test.sh'
--- tests/test.sh	1970-01-01 00:00:00 +0000
+++ tests/test.sh	2013-04-29 20:44:30 +0000
@@ -0,0 +1,6 @@
+#!/bin/bash -e
+
+DIR="$( cd "$( dirname "$0" )" && pwd )"
+cd $DIR
+
+run-parts -v --exit-on-error --regex '^.*\.test$' .