Merge lp:~tealeg/landscape-client/invoke-registration-from-settings-ui into lp:~landscape/landscape-client/trunk

[Geoff Teale]

Addresses bug #911666

Adds UI and mechanism for invoking landscape-config --silent with the config file generated by the UI.

Note, this will not work with the default values in landscape-client.conf. You need to have a version of landscape-client installed because it will invoke the installed version of landscape-config (or at least not when you have landscape-client installed). You can pass -c /etc/landscape/client.conf if that is the case and get it to work.

[Free Ekanayaka (free.ekanayaka)]

Nice work, a few comments but only about cosmetics. +1!

[1]

+def check_script_users(config):

+def setup_http_proxy(config):

+def check_account_name_and_password(config):

Please add a docstring to these new functions (and maybe to setup() as well while you're at it).

[2]

-def register(config, reactor=None):
+def register(config, message_handler_f, error_handler_f, reactor=None):

We don't usually encode type information in variable names, or at least not with abbreviations. Please consider renaming these parameters to something like "on_finish" and "on_error".

Also what do you think of adding print_text and sys.exit as default values for these new parameters? So you get a more user-friendly interface (you don't need to customize what you don't need) and call sites or tests don't need to be modified.

[3]

+class ObservableRegistration(object):

Please add a class docstring for this class and for all its methods. The class docstring should include __init__ parameters to, for example

class ObservableRegistration(object):
    """Meant to do something useful.

    @param idle_f: Optionally, a function that will be called when...
    """

[4]

+ for fun in self._succeed_observers:
+ fun()
+ self.do_idle()

Please use full names, s/fun/function/

[5]

+ self.notified = False
+ self.notified_message = None
+ self.notified_error = False
+
+ def notify_me(message, error=False):
+ self.notified = True
+ self.notified_message = message
+ self.notified_error = error

This is okay, but of course leaves some "attribute garbage" on the test case instance, that in principle could get in the way of other tests, if don't write them carefully.

Please consider change it to a version without side effects, like:

        notified = []
        notified_message = []
        notified_error = []

        def notify_me(message, error=False):
            notified.append(True)
            notified_message.append(message)
            notified_error.append(error)

plus
        self.assertEqual([True], notified)
        self.assertEqual(notified_message, ["Blimey"])
        self.assertEqual(notified_error, [False])

Also, this has the advantage of asserting also how many times notify_me was called, and not only which parameters it was passed.

Alternatively, you could go for a Mocker version:

    def test_notify_observers(self):
        observable = ObservableRegistration()
        notify_me = self.mocker.mock()
        self.expect(notify_me("Blimey", error=False))
        self.expect(notify_me("Go lummey!", error=True))
        self.mocker.replay()
        observable.notify_observers("Blimey", error=False)
        observable.notify_observers("Gor lummey!", error=True)

and you can rely on mocker to assert that notify_me was called exactly in that order, with those arguments and that number for times. I just thought I'd mention it, not sure how much I like this myself though :)

[6]

+ def default_machine(self):

Please consider renaming this to set_default_computer_title (and maybe other similar methods to). The rationals are that the "set_" prefix makes it more clear wh...

review: Approve

[Geoff Teale (tealeg)]

I've addressed Free's issues from above and repushed.

[lp:~tealeg/landscape-client/invoke-registration-from-settings-ui]

469. By Geoff Teale on 2012-01-11

Add docstrings and defaults as per Free's review suggestion

470. By Geoff Teale on 2012-01-11

Change names of first class function parameters in accordance with comments from Free's review

471. By Geoff Teale on 2012-01-11

Change variable names fun -> function in accordance with comments from Free's review

472. By Geoff Teale on 2012-01-11

Restructured notification tests to use local scope instead of object scope (from Free's review)

473. By Geoff Teale on 2012-01-11

Rename method default_machine -> default_computer_title (From Free's review)

474. By Geoff Teale on 2012-01-11

Monkey patch ConfigController.getfqdn instead of socket.getfqdn (From Free's review)

475. By Geoff Teale on 2012-01-11

Fix lint issues

476. By Geoff Teale on 2012-01-12

Added UI packages to setup.py as per a request from Andreas

[Fernando Correa Neto (fcorrea)]

Looks great! +1

review: Approve

=== modified file 'landscape/configuration.py'
--- landscape/configuration.py	2012-01-05 10:42:54 +0000
+++ landscape/configuration.py	2012-01-12 15:18:24 +0000
@@ -423,60 +423,101 @@
 
 
 def setup_init_script_and_start_client():
+    "Configure the init script to start the client on boot."
     # XXX This function is misnamed; it doesn't start the client.
     sysvconfig = SysVConfig()
     sysvconfig.set_start_on_boot(True)
 
 
 def stop_client_and_disable_init_script():
+    """
+    Stop landscape-client and change configuration to prevent starting
+    landscape-client on boot.
+    """
     sysvconfig = SysVConfig()
     sysvconfig.stop_landscape()
     sysvconfig.set_start_on_boot(False)
 
 
-def setup(config):
-    sysvconfig = SysVConfig()
-    if not config.no_start:
-        if config.silent:
-            setup_init_script_and_start_client()
-        elif not sysvconfig.is_configured_to_run():
-            answer = raw_input("\nThe Landscape client must be started "
-                               "on boot to operate correctly.\n\n"
-                               "Start Landscape client on boot? (Y/n): ")
-            if not answer.upper().startswith("N"):
-                setup_init_script_and_start_client()
-            else:
-                sys.exit("Aborting Landscape configuration")
-
+def setup_http_proxy(config):
+    """
+    If a http_proxy and a https_proxy value are not set then copy the values,
+    if any, from the environment variables L{http_proxy} and L{https_proxy}.
+    """
     if config.http_proxy is None and os.environ.get("http_proxy"):
         config.http_proxy = os.environ["http_proxy"]
     if config.https_proxy is None and os.environ.get("https_proxy"):
         config.https_proxy = os.environ["https_proxy"]
 
+
+def check_account_name_and_password(config):
+    """
+    Ensure that silent configurations which plan to start landscape-client are
+    either configured for OTP or have both an account_name and computer title.
+    """
     if config.silent and not config.no_start:
         if not (config.get("otp") or config.provisioning_otp or
                 (config.get("account_name") and config.get("computer_title"))):
             raise ConfigurationError("An account name and computer title are "
                                      "required.")
-    if config.silent:
-        if config.get("script_users"):
-            invalid_users = get_invalid_users(config.get("script_users"))
-            if invalid_users:
-                raise ConfigurationError("Unknown system users: %s" %
-                                         ", ".join(invalid_users))
-            if not config.include_manager_plugins:
-                config.include_manager_plugins = "ScriptExecution"
-    else:
-        script = LandscapeSetupScript(config)
-        script.run()
-
-    # WARNING: ssl_public_key is misnamed, it's not the key of the certificate,
-    # but the actual certificate itself.
+
+
+def check_script_users(config):
+    """
+    If the configuration allows for script execution ensure that the configured
+    users are valid for that purpose.
+    """
+    if config.get("script_users"):
+        invalid_users = get_invalid_users(config.get("script_users"))
+        if invalid_users:
+            raise ConfigurationError("Unknown system users: %s" %
+                                     ", ".join(invalid_users))
+        if not config.include_manager_plugins:
+            config.include_manager_plugins = "ScriptExecution"
+
+
+def decode_base64_ssl_public_certificate(config):
+    """
+    Decode base64 encoded SSL certificate and push that back into place in the
+    config object.
+    """
+    # WARNING: ssl_public_certificate is misnamed, it's not the key of the
+    # certificate, but the actual certificate itself.
     if config.ssl_public_key and config.ssl_public_key.startswith("base64:"):
         decoded_cert = base64.decodestring(config.ssl_public_key[7:])
         config.ssl_public_key = store_public_key_data(
             config, decoded_cert)
 
+
+def setup(config):
+    """
+    Perform steps to ensure that landscape-client is correctly configured
+    before we attempt to register it with a landscape server.
+
+    If we are not configured to be silent then interrogate the user to provide
+    necessary details for registration.
+    """
+    sysvconfig = SysVConfig()
+    if not config.no_start:
+        if config.silent:
+            setup_init_script_and_start_client()
+        elif not sysvconfig.is_configured_to_run():
+            answer = raw_input("\nThe Landscape client must be started "
+                               "on boot to operate correctly.\n\n"
+                               "Start Landscape client on boot? (Y/n): ")
+            if not answer.upper().startswith("N"):
+                setup_init_script_and_start_client()
+            else:
+                sys.exit("Aborting Landscape configuration")
+
+    setup_http_proxy(config)
+    check_account_name_and_password(config)
+    if config.silent:
+        check_script_users(config)
+    else:
+        script = LandscapeSetupScript(config)
+        script.run()
+    decode_base64_ssl_public_certificate(config)
     config.write()
     # Restart the client to ensure that it's using the new configuration.
     if not config.no_start and not config.otp:
@@ -512,7 +553,7 @@
     return key_filename
 
 
-def register(config, reactor=None):
+def register(config, on_message=print_text, on_error=sys.exit, reactor=None):
     """Instruct the Landscape Broker to register the client.
 
     The broker will be instructed to reload its configuration and then to
@@ -533,16 +574,16 @@
         reactor.stop()
 
     def failure():
-        print_text("Invalid account name or "
+        on_message("Invalid account name or "
                    "registration password.", error=True)
         stop()
 
     def success():
-        print_text("System successfully registered.")
+        on_message("System successfully registered.")
         stop()
 
     def exchange_failure():
-        print_text("We were unable to contact the server. "
+        on_message("We were unable to contact the server. "
                    "Your internet connection may be down. "
                    "The landscape client will continue to try and contact "
                    "the server periodically.",
@@ -555,11 +596,11 @@
         connector.disconnect()
 
     def catch_all(failure):
-        print_text(failure.getTraceback(), error=True)
-        print_text("Unknown error occurred.", error=True)
+        on_message(failure.getTraceback(), error=True)
+        on_message("Unknown error occurred.", error=True)
         stop()
 
-    print_text("Please wait... ", "")
+    on_message("Please wait... ", "")
 
     time.sleep(2)
 
@@ -577,9 +618,9 @@
         return results.addErrback(catch_all)
 
     def got_error(failure):
-        print_text("There was an error communicating with the Landscape "
-                   "client.", error=True)
-        print_text("This machine will be registered with the provided "
+        on_message("There was an error communicating with the Landscape"
+                   " client.", error=True)
+        on_message("This machine will be registered with the provided "
                    "details when the client runs.", error=True)
         if not config.ok_no_register:
             exit_with_error.append(2)
@@ -593,7 +634,7 @@
     reactor.run()
 
     if exit_with_error:
-        sys.exit(exit_with_error[0])
+        on_error(exit_with_error[0])
 
     return result
 

=== modified file 'landscape/tests/test_configuration.py'
--- landscape/tests/test_configuration.py	2012-01-05 10:42:54 +0000
+++ landscape/tests/test_configuration.py	2012-01-12 15:18:24 +0000
@@ -1,4 +1,5 @@
 import os
+import sys
 from getpass import getpass
 from ConfigParser import ConfigParser
 from cStringIO import StringIO
@@ -1163,7 +1164,7 @@
 
         # Registration logic should not be invoked.
         register_mock = self.mocker.replace(register, passthrough=False)
-        register_mock(ANY)
+        register_mock(ANY, ANY, ANY)
         self.mocker.count(0)
 
         self.mocker.replay()
@@ -1669,7 +1670,7 @@
         self.mocker.replay()
 
         # DO IT!
-        return register(service.config)
+        return register(service.config, print_text, sys.exit)
 
     def test_register_failure(self):
         """
@@ -1721,7 +1722,7 @@
         self.mocker.replay()
 
         # DO IT!
-        return register(service.config)
+        return register(service.config, print_text, sys.exit)
 
     def test_register_exchange_failure(self):
         """
@@ -1771,7 +1772,7 @@
         self.mocker.replay()
 
         # DO IT!
-        return register(service.config)
+        return register(service.config, print_text, sys.exit)
 
     def test_register_timeout_failure(self):
         service = self.broker_service
@@ -1815,7 +1816,7 @@
         self.mocker.replay()
 
         # DO IT!
-        return register(service.config)
+        return register(service.config, print_text, sys.exit)
 
     def test_register_bus_connection_failure(self):
         """
@@ -1852,7 +1853,7 @@
         self.mocker.replay()
 
         config = get_config(self, ["-a", "accountname", "--silent"])
-        return register(config)
+        return register(config, print_text, sys.exit)
 
     def test_register_bus_connection_failure_ok_no_register(self):
         """
@@ -1879,7 +1880,7 @@
 
         config = get_config(self, ["-a", "accountname", "--silent",
                                    "--ok-no-register"])
-        return self.assertSuccess(register(config))
+        return self.assertSuccess(register(config, print_text, sys.exit))
 
 
 class RegisterFunctionNoServiceTest(LandscapeTest):
@@ -1932,7 +1933,7 @@
 
         self.mocker.replay()
 
-        return register(configuration)
+        return register(configuration, print_text, sys.exit)
 
 
 class StoreSSLCertificateDataTest(LandscapeTest):

=== modified file 'landscape/ui/controller/configuration.py'
--- landscape/ui/controller/configuration.py	2012-01-09 11:35:16 +0000
+++ landscape/ui/controller/configuration.py	2012-01-12 15:18:24 +0000
@@ -1,5 +1,8 @@
+import socket
 import threading
 
+from landscape.ui.model.registration import ObservableRegistration
+
 
 class ConfigControllerLockError(Exception):
     pass
@@ -14,27 +17,64 @@
 
     HOSTED_HOST_NAME = "landscape.canonical.com"
     DEFAULT_SERVER_HOST_NAME = "landscape.localdomain"
+    DEFAULT_DEDICATED_ACCOUNT_NAME = "standalone"
 
     def __init__(self, configuration, args=[]):
+        self._observers = []
         self._initial_server_host_name = self.DEFAULT_SERVER_HOST_NAME
+        self._initial_account_name = self.DEFAULT_DEDICATED_ACCOUNT_NAME
         self._configuration = configuration
         self._args = args
         self._lock_out = False
         self._lock = threading.Lock()
 
+    def register_observer(self, function):
+        "Register functions that observer modify/unmodify."
+        self._observers.append(function)
+
+    def notify_observers(self, modified):
+        "Notify observers of modification events.  L{Modified} is boolean."
+        for function in self._observers:
+            function(modified)
+
+    def modify(self):
+        "Mark this config as modified and notify observers."
+        self._modified = True
+        self.notify_observers(True)
+
+    def unmodify(self):
+        "Mark this config as being unmodified and notify observers."
+        self._modified = False
+        self.notify_observers(False)
+
     def load(self):
         "Load the initial data from the configuration"
         self.lock()
         self._configuration.load(self._args)
         self._pull_data_from_config()
-        self._modified = False
+        self.default_computer_title()
+        self.unmodify()
         self.unlock()
 
+    def getfqdn(self):
+        """
+        Wrap socket.getfqdn so we can test reliably.
+        """
+        return socket.getfqdn()
+
+    def default_computer_title(self):
+        """
+        Default machine name to FQDN.
+        """
+        if self._computer_title is None:
+            self._computer_title = self.getfqdn()
+
     def default_dedicated(self):
         """
         Set L{server_host_name} to something sane when switching from hosted to
         dedicated.
         """
+        self._account_name = self.DEFAULT_DEDICATED_ACCOUNT_NAME
         if self._initial_server_host_name != self.HOSTED_HOST_NAME:
             self._server_host_name = self._initial_server_host_name
         else:
@@ -43,7 +83,7 @@
                 self._server_host_name)
             self._ping_url = self._derive_ping_url_from_host_name(
                 self._server_host_name)
-        self._modified = True
+        self.modify()
 
     def default_hosted(self):
         """
@@ -56,7 +96,8 @@
             self._server_host_name)
         self._ping_url = self._derive_ping_url_from_host_name(
             self._server_host_name)
-        self._modified = True
+        self._account_name = self._initial_account_name
+        self.modify()
 
     def _pull_data_from_config(self):
         """
@@ -69,6 +110,7 @@
         self._url = self._configuration.url
         self._ping_url = self._configuration.ping_url
         self._account_name = self._configuration.account_name
+        self._initial_account_name = self._account_name
         self._registration_password = \
             self._configuration.registration_password
         self._computer_title = self._configuration.computer_title
@@ -80,7 +122,7 @@
         else:
             self._server_host_name = self.HOSTED_HOST_NAME
         self._initial_server_host_name = self._server_host_name
-        self._modified = False
+        self.unmodify()
         self._lock.release()
 
     def lock(self):
@@ -141,7 +183,7 @@
                 self._server_host_name)
             self._ping_url = self._derive_ping_url_from_host_name(
                 self._server_host_name)
-            self._modified = True
+            self.modify()
             self._lock.release()
     server_host_name = property(_get_server_host_name, _set_server_host_name)
 
@@ -171,7 +213,7 @@
             raise ConfigControllerLockError
         else:
             self._account_name = value
-            self._modified = True
+            self.modify()
             self._lock.release()
     account_name = property(_get_account_name, _set_account_name)
 
@@ -185,7 +227,7 @@
             raise ConfigControllerLockError
         else:
             self._registration_password = value
-            self._modified = True
+            self.modify()
             self._lock.release()
     registration_password = property(_get_registration_password,
                                      _set_registration_password)
@@ -229,4 +271,21 @@
         self._configuration.https_proxy = self._https_proxy
         self._configuration.ping_url = self._ping_url
         self._configuration.write()
-        self._modified = False
+        self.unmodify()
+
+    def register(self, on_notify, on_error, on_success, on_failure, on_idle):
+        "Invoke model level registration without completely locking the view."
+        on_idle()
+        registration = ObservableRegistration(on_idle)
+        on_idle()
+        self.commit()
+        on_idle()
+        registration.register_notification_observer(on_notify)
+        on_idle()
+        registration.register_error_observer(on_error)
+        on_idle()
+        registration.register_succeed_observer(on_success)
+        on_idle()
+        registration.register_fail_observer(on_failure)
+        on_idle()
+        registration.register(self._configuration)

=== modified file 'landscape/ui/controller/tests/test_app.py'
--- landscape/ui/controller/tests/test_app.py	2012-01-09 10:13:34 +0000
+++ landscape/ui/controller/tests/test_app.py	2012-01-12 15:18:24 +0000
@@ -17,16 +17,15 @@
 
 class ConnectionRecordingSettingsApplicationController(
     SettingsApplicationController):
-
     _connections = set()
     _connection_args = {}
     _connection_kwargs = {}
 
-    def __init__(self, get_config_f=None):
+    def __init__(self, get_config=None):
         super(ConnectionRecordingSettingsApplicationController,
               self).__init__()
-        if get_config_f:
-            self.get_config = get_config_f
+        if get_config:
+            self.get_config = get_config
 
     def _make_connection_name(self, signal, func):
         return signal + ">" + func.__name__
@@ -103,7 +102,7 @@
             return config
 
         self.app = ConnectionRecordingSettingsApplicationController(
-            get_config_f=get_config)
+            get_config=get_config)
 
     def tearDown(self):
         Gtk.Dialog.run = self._real_run

=== modified file 'landscape/ui/controller/tests/test_configuration.py'
--- landscape/ui/controller/tests/test_configuration.py	2012-01-09 11:35:16 +0000
+++ landscape/ui/controller/tests/test_configuration.py	2012-01-12 15:18:24 +0000
@@ -26,6 +26,11 @@
         self.config = MyLandscapeSetupConfiguration()
         self.controller = ConfigController(self.config)
 
+        def get_fqdn():
+            return "me.here.com"
+
+        self.controller.getfqdn = get_fqdn
+
     def test_init(self):
         """
         Test that when we create a controller it has initial state read in
@@ -202,8 +207,9 @@
         self.assertEqual(self.controller.registration_password, None)
         self.assertEqual(self.controller.server_host_name,
                          "landscape.canonical.com")
+        self.controller.account_name = "Bungle"
         self.controller.default_dedicated()
-        self.assertEqual(self.controller.account_name, None)
+        self.assertEqual(self.controller.account_name, "standalone")
         self.assertEqual(self.controller.registration_password, None)
         self.assertEqual(self.controller.server_host_name,
                          "landscape.localdomain")
@@ -221,3 +227,12 @@
                          "landscape.canonical.com")
         self.controller.default_dedicated()
         self.assertEqual(self.controller.server_host_name, "test.machine")
+
+    def test_default_computer_title(self):
+        """
+        Test we set the computer title to host name when it isn't already set
+        in the config file.
+        """
+        self.makeFile("", path=self.config_filename)  # Empty config
+        self.controller.load()
+        self.assertEqual(self.controller.computer_title, "me.here.com")

=== added directory 'landscape/ui/model'
=== added file 'landscape/ui/model/__init__.py'
=== added file 'landscape/ui/model/registration.py'
--- landscape/ui/model/registration.py	1970-01-01 00:00:00 +0000
+++ landscape/ui/model/registration.py	2012-01-12 15:18:24 +0000
@@ -0,0 +1,87 @@
+import select
+import subprocess
+import os
+
+
+class ObservableRegistration(object):
+    """
+    L{ObservableRegistration} provides the ability to run the landscape-client
+    registration in a way that can be observed by code using it.  This allows
+    for registration in an environment other than an interactive terminal
+    session.
+
+    @param on_idle: Optionally, a callable which will be invoked by repeatedly
+    during the registration process to allow cooperative yielding of control.
+    This can be used, for example, with Gtk to allow processing of Gtk events
+    without requiring either threading or the leaking of Gtk into the model
+    layer.
+    """
+
+    def __init__(self, on_idle=None):
+        self._notification_observers = []
+        self._error_observers = []
+        self._succeed_observers = []
+        self._fail_observers = []
+        self._on_idle = on_idle
+
+    def do_idle(self):
+        if self._on_idle:
+            self._on_idle()
+
+    def notify_observers(self, message, end="\n", error=False):
+        for function in self._notification_observers:
+            function(message, error)
+            self.do_idle()
+
+    def error_observers(self, error_list):
+        for function in self._error_observers:
+            function(error_list)
+            self.do_idle()
+
+    def register_notification_observer(self, function):
+        self._notification_observers.append(function)
+
+    def register_error_observer(self, function):
+        self._error_observers.append(function)
+
+    def register_succeed_observer(self, function):
+        self._succeed_observers.append(function)
+
+    def register_fail_observer(self, function):
+        self._fail_observers.append(function)
+
+    def succeed(self):
+        for function in self._succeed_observers:
+            function()
+            self.do_idle()
+
+    def fail(self, error=None):
+        for function in self._fail_observers:
+            function(error=error)
+            self.do_idle()
+
+    def register(self, config):
+        self.notify_observers("Trying to register ...\n")
+        cmd = ["landscape-config", "--silent", "-c",
+               os.path.abspath(config.get_config_filename())]
+        process = subprocess.Popen(cmd, stdout=subprocess.PIPE,
+                                   stderr=subprocess.PIPE)
+        return_code = None
+        while return_code is None:
+            readables, w, x = select.select([process.stdout, process.stderr],
+                                            [], [], 0)
+            for readable in readables:
+                message = readable.readline()
+                if readable is process.stdout:
+                    self.notify_observers(message)
+                else:
+                    self.error_observers(message)
+                self.do_idle()
+            return_code = process.poll()
+            self.do_idle()
+        if return_code == 0:
+            self.succeed()
+            return True
+        else:
+            self.fail("Failed with code %s" % str(return_code))
+            return False

=== added directory 'landscape/ui/model/tests'
=== added file 'landscape/ui/model/tests/__init__.py'
=== added file 'landscape/ui/model/tests/test_registration.py'
--- landscape/ui/model/tests/test_registration.py	1970-01-01 00:00:00 +0000
+++ landscape/ui/model/tests/test_registration.py	2012-01-12 15:18:24 +0000
@@ -0,0 +1,80 @@
+from landscape.tests.helpers import LandscapeTest, BrokerServiceHelper
+from landscape.ui.model.registration import ObservableRegistration
+
+
+class RegistrationTest(LandscapeTest):
+
+    helpers = [BrokerServiceHelper]
+
+    def setUp(self):
+        super(RegistrationTest, self).setUp()
+
+    def test_notify_observers(self):
+        """
+        Test that when an observer is registered it is called by
+        L{notify_observers}.
+        """
+        observable = ObservableRegistration()
+        notified = []
+        notified_message = []
+        notified_error = []
+
+        def notify_me(message, error=False):
+            notified.append(True)
+            notified_message.append(message)
+            notified_error.append(error)
+
+        observable.register_notification_observer(notify_me)
+        observable.notify_observers("Blimey", error=False)
+        observable.notify_observers("Gor lummey!", error=True)
+        self.assertEqual(notified, [True, True])
+        self.assertEqual(notified_message, ["Blimey", "Gor lummey!"])
+        self.assertTrue(notified_error, [False, True])
+
+    def test_error_observers(self):
+        """
+        Test that when an error observer is registered it is called by
+        L{error_observers}.
+        """
+        observable = ObservableRegistration()
+        errored = []
+        errored_error_list = []
+
+        def error_me(error_list):
+            errored.append(True)
+            errored_error_list.append(error_list)
+
+        observable.register_error_observer(error_me)
+        observable.error_observers(["Ouch", "Dang"])
+        self.assertTrue(errored, [True])
+        self.assertEqual(errored_error_list, [["Ouch", "Dang"]])
+
+    def test_success_observers(self):
+        """
+        Test that when a success observer is registered it is called when
+        L{succeed} is called on the model.
+        """
+        observable = ObservableRegistration()
+        self.succeeded = False
+
+        def success():
+            self.succeeded = True
+
+        observable.register_succeed_observer(success)
+        observable.succeed()
+        self.assertTrue(self.succeeded)
+
+    def test_failure_observers(self):
+        """
+        Test that when a failure observer is registered it is called when
+        L{fail} is called on the model.
+        """
+        observable = ObservableRegistration()
+        self.failed = False
+
+        def failure(error=None):
+            self.failed = True
+
+        observable.register_fail_observer(failure)
+        observable.fail()
+        self.assertTrue(self.failed)

=== modified file 'landscape/ui/view/configuration.py'
--- landscape/ui/view/configuration.py	2012-01-09 15:14:12 +0000
+++ landscape/ui/view/configuration.py	2012-01-12 15:18:24 +0000
@@ -1,6 +1,6 @@
 import os
 
-from gi.repository import Gtk
+from gi.repository import Gtk, Gdk
 
 
 class ClientSettingsDialog(Gtk.Dialog):
@@ -9,7 +9,9 @@
 
     def __init__(self, controller):
         super(ClientSettingsDialog, self).__init__()
+        self._continue = False
         self.controller = controller
+        self.controller.register_observer(self._on_modify)
         self._ui_path = os.path.join(
             os.path.dirname(__file__), "ui",
             ClientSettingsDialog.GLADE_FILE)
@@ -41,6 +43,18 @@
         self._revert_button.connect("clicked", self.revert)
         self._close_button.connect("clicked", self._possibly_save_and_exit)
 
+    def _setup_registration_controls(self):
+        """
+        Obtain handles for controls relating to the registration process.
+        """
+        self._registration_button = self._builder.get_object(
+            "registration-button")
+        self._registration_image = self._builder.get_object(
+            "registration-image")
+        self._registration_textbuffer = self._builder.get_object(
+            "registration-textbuffer")
+        self._registration_button.connect("clicked", self._register)
+
     def _setup_entries(self):
         """
         Obtain handles for entry widgets, set initial state and connect them to
@@ -73,8 +87,14 @@
         self._setup_radiobuttons()
         self._setup_entries()
         self._setup_action_buttons()
+        self._setup_registration_controls()
 
     def _load_data(self):
+        """
+        Pull data up from the controller into the view widgets.  Note that we
+        don't want to propagate events back to the controller as this will set
+        up an infinite loop - hence the lock.
+        """
         self.controller.lock()
         if not self.controller.account_name is None:
             self._account_entry.set_text(self.controller.account_name)
@@ -147,6 +167,64 @@
         self.controller.commit()
 
     def _possibly_save_and_exit(self, button):
+        """
+        Write back if something has been modified.
+        """
         if self.controller.is_modified:
             self._write_back()
         self.destroy()
+
+    def _process_gtk_events(self):
+        """
+        Deal with any outstanding Gtk events.  Used to keep the GUI ticking
+        over during long running activities.
+        """
+        while Gtk.events_pending():
+            Gtk.main_iteration()
+
+    def _registration_message(self, message, error=None):
+        self._registration_textbuffer.insert_at_cursor(message)
+        self._registration_image.set_from_stock(Gtk.STOCK_DIALOG_INFO, 4)
+        self._process_gtk_events()
+
+    def _registration_error(self, message):
+        self._registration_textbuffer.insert_at_cursor(message)
+        self._registration_image.set_from_stock(Gtk.STOCK_DIALOG_WARNING, 4)
+        self._process_gtk_events()
+
+    def _registration_succeed(self):
+        self._registration_image.set_from_stock(Gtk.STOCK_CONNECT, 4)
+        self._close_button.set_sensitive(True)
+        self._set_normal_cursor()
+
+    def _registration_fail(self, error=None):
+        if error:
+            self._registration_textbuffer.insert_at_cursor(str(error))
+        self._registration_image.set_from_stock(Gtk.STOCK_DISCONNECT, 4)
+        self._set_normal_cursor()
+
+    def _set_wait_cursor(self):
+        watch = Gdk.Cursor(Gdk.CursorType.WATCH)
+        self.get_window().set_cursor(watch)
+        self._process_gtk_events()
+
+    def _set_normal_cursor(self):
+        arrow = Gdk.Cursor(Gdk.CursorType.ARROW)
+        self.get_window().set_cursor(arrow)
+        self._process_gtk_events()
+
+    def _register(self, button):
+        self._set_wait_cursor()
+        self._write_back()
+        self._registration_image.set_from_stock(Gtk.STOCK_CONNECT, 4)
+        self.controller.register(self._registration_message,
+                                 self._registration_error,
+                                 self._registration_succeed,
+                                 self._registration_fail,
+                                 self._process_gtk_events)
+
+    def _on_modify(self, modified):
+        self._close_button.set_sensitive(not modified)
+        if modified:
+            self._registration_image.set_from_stock(Gtk.STOCK_DISCONNECT, 4)
+            self._registration_textbuffer.set_text("")

=== modified file 'landscape/ui/view/tests/test_configuration.py'
--- landscape/ui/view/tests/test_configuration.py	2012-01-05 14:16:47 +0000
+++ landscape/ui/view/tests/test_configuration.py	2012-01-12 15:18:24 +0000
@@ -52,6 +52,12 @@
         self.assertTrue(dialog._account_entry.get_sensitive())
         self.assertTrue(dialog._password_entry.get_sensitive())
         self.assertFalse(dialog._server_host_name_entry.get_sensitive())
+        self.assertTrue(dialog._registration_button.get_sensitive())
+        start, end = dialog._registration_textbuffer.get_bounds()
+        self.assertEqual(
+            len(dialog._registration_textbuffer.get_text(
+                    start, end, include_hidden_chars=True)),
+            0)
 
     def test_toggle_radio_button(self):
         """
@@ -90,6 +96,27 @@
         self.assertEqual(dialog._server_host_name_entry.get_text(),
                          "landscape.canonical.com")
 
+    def test_modification(self):
+        """
+        Test that modifications to the dialog cause the close button to be
+        disabled and the connect button to be reset.
+        """
+        controller = ConfigController(self.config)
+        dialog = ClientSettingsDialog(controller)
+        self.assertEqual(dialog._registration_image.get_stock(),
+                         (Gtk.STOCK_DISCONNECT, 4))
+        dialog._registration_image.set_from_stock(Gtk.STOCK_CONNECT, 4)
+        while Gtk.events_pending():
+            Gtk.main_iteration()
+        self.assertEqual(dialog._registration_image.get_stock(),
+                         (Gtk.STOCK_CONNECT, 4))
+        dialog._account_entry.set_text("Spangles!")
+        while Gtk.events_pending():
+            Gtk.main_iteration()
+        self.assertEqual(dialog._registration_image.get_stock(),
+                         (Gtk.STOCK_DISCONNECT, 4))
+        self.assertFalse(dialog._close_button.get_sensitive())
+
     def test_revert(self):
         """
         Test that we can revert the UI values using the controller.
@@ -102,7 +129,7 @@
                          "landscape.canonical.com")
         dialog._dedicated_radiobutton.set_active(True)
         dialog._server_host_name_entry.set_text("more.barn")
-        self.assertEqual(dialog._account_entry.get_text(), "foo")
+        self.assertEqual(dialog._account_entry.get_text(), "standalone")
         self.assertEqual(dialog._password_entry.get_text(), "bar")
         self.assertEqual(dialog._server_host_name_entry.get_text(),
                          "more.barn")

=== modified file 'landscape/ui/view/ui/landscape-client-settings.glade'
--- landscape/ui/view/ui/landscape-client-settings.glade	2012-01-02 12:04:18 +0000
+++ landscape/ui/view/ui/landscape-client-settings.glade	2012-01-12 15:18:24 +0000
@@ -14,6 +14,7 @@
     <child internal-child="vbox">
       <object class="GtkBox" id="landscape-client-settings-dialog-vbox">
         <property name="can_focus">False</property>
+        <property name="vexpand">True</property>
         <property name="orientation">vertical</property>
         <property name="spacing">2</property>
         <child internal-child="action_area">
@@ -62,6 +63,7 @@
           <object class="GtkBox" id="vbox">
             <property name="visible">True</property>
             <property name="can_focus">False</property>
+            <property name="vexpand">True</property>
             <property name="orientation">vertical</property>
             <property name="spacing">6</property>
             <child>
@@ -383,6 +385,99 @@
                 <property name="position">0</property>
               </packing>
             </child>
+            <child>
+              <object class="GtkBox" id="registration-hbox">
+                <property name="visible">True</property>
+                <property name="can_focus">False</property>
+                <property name="margin_left">12</property>
+                <property name="margin_right">6</property>
+                <property name="hexpand">True</property>
+                <property name="vexpand">True</property>
+                <property name="spacing">12</property>
+                <child>
+                  <object class="GtkButton" id="registration-button">
+                    <property name="visible">True</property>
+                    <property name="can_focus">True</property>
+                    <property name="receives_default">True</property>
+                    <property name="valign">start</property>
+                    <property name="use_action_appearance">False</property>
+                    <child>
+                      <object class="GtkBox" id="registration-button-hbox">
+                        <property name="visible">True</property>
+                        <property name="can_focus">False</property>
+                        <property name="margin_left">6</property>
+                        <property name="margin_right">6</property>
+                        <property name="margin_top">6</property>
+                        <property name="margin_bottom">6</property>
+                        <property name="orientation">vertical</property>
+                        <property name="spacing">6</property>
+                        <property name="homogeneous">True</property>
+                        <child>
+                          <object class="GtkLabel" id="label1">
+                            <property name="visible">True</property>
+                            <property name="can_focus">False</property>
+                            <property name="label" translatable="yes">Connect</property>
+                          </object>
+                          <packing>
+                            <property name="expand">False</property>
+                            <property name="fill">True</property>
+                            <property name="position">0</property>
+                          </packing>
+                        </child>
+                        <child>
+                          <object class="GtkImage" id="registration-image">
+                            <property name="visible">True</property>
+                            <property name="can_focus">False</property>
+                            <property name="stock">gtk-disconnect</property>
+                          </object>
+                          <packing>
+                            <property name="expand">False</property>
+                            <property name="fill">True</property>
+                            <property name="position">1</property>
+                          </packing>
+                        </child>
+                      </object>
+                    </child>
+                  </object>
+                  <packing>
+                    <property name="expand">False</property>
+                    <property name="fill">True</property>
+                    <property name="position">0</property>
+                  </packing>
+                </child>
+                <child>
+                  <object class="GtkScrolledWindow" id="registration-scrolledwindow">
+                    <property name="visible">True</property>
+                    <property name="can_focus">True</property>
+                    <property name="hexpand">True</property>
+                    <property name="vscrollbar_policy">always</property>
+                    <property name="shadow_type">in</property>
+                    <child>
+                      <object class="GtkTextView" id="registration-textview">
+                        <property name="visible">True</property>
+                        <property name="can_focus">True</property>
+                        <property name="hexpand">True</property>
+                        <property name="vexpand">True</property>
+                        <property name="editable">False</property>
+                        <property name="wrap_mode">word-char</property>
+                        <property name="cursor_visible">False</property>
+                        <property name="buffer">registration-textbuffer</property>
+                      </object>
+                    </child>
+                  </object>
+                  <packing>
+                    <property name="expand">False</property>
+                    <property name="fill">True</property>
+                    <property name="position">1</property>
+                  </packing>
+                </child>
+              </object>
+              <packing>
+                <property name="expand">False</property>
+                <property name="fill">True</property>
+                <property name="position">1</property>
+              </packing>
+            </child>
           </object>
           <packing>
             <property name="expand">False</property>
@@ -404,4 +499,5 @@
       <widget name="account-label"/>
     </widgets>
   </object>
+  <object class="GtkTextBuffer" id="registration-textbuffer"/>
 </interface>

=== modified file 'setup.py'
--- setup.py	2011-12-01 14:01:29 +0000
+++ setup.py	2012-01-12 15:18:24 +0000
@@ -16,10 +16,16 @@
                 "landscape.monitor",
                 "landscape.package",
                 "landscape.sysinfo",
+                "landscape.ui",
+                "landscape.ui.model",
+                "landscape.ui.controller",
+                "landscape.ui.view",
                 "landscape.upgraders",
                 "landscape.user",
                 "landscape.lib"],
+      package_data={"landscape.ui.view": ["ui/landscape-client-settings.glade"]},
       scripts=["scripts/landscape-client",
+               "scripts/landscape-client-settings-ui",
                "scripts/landscape-config",
                "scripts/landscape-message",
                "scripts/landscape-broker",