Merge lp:~jseutter/landscape-client/autodiscover-questions into lp:~landscape/landscape-client/trunk

[Jerry Seutter]

This branch adds questions to debconf for server autodiscovery. It asks 3 questions:
- should server autodiscovery be done?
- What string should be used for DNS SRV queries (default to _landscape._tcp.localdomain)
- What string should be used for DNS A queries (default to landscape.localdomain)

I don't have any experience with this so I could be doing something dubious, dastardly or even diabolical.

[lp:~jseutter/landscape-client/autodiscover-questions]

459. By Jerry Seutter on 2012-02-09

Fixing bugs in landscape-client.config

460. By Jerry Seutter on 2012-02-09

Transmitting new settings from debconf back to landscape-client.

461. By Jerry Seutter on 2012-02-10

Adding a default setting to the templates file

462. By Jerry Seutter on 2012-02-24

Merge from trunk.

Unmerged revisions

=== modified file 'debian/landscape-client.config'
--- debian/landscape-client.config	2011-10-07 12:14:19 +0000
+++ debian/landscape-client.config	2012-02-08 00:58:28 +0000
@@ -29,6 +29,9 @@
     HTTPS_PROXY=$(get_var_from_file "https_proxy" "$CONFIGFILE")
     OTP=$(get_var_from_file "otp" "$CONFIGFILE")
     TAGS=$(get_var_from_file "tags" "$CONFIG_FILE")
+    SERVER_AUTODISCOVER=$(get_var_from_file "server_autodiscover" "$CONFIG_FILE")
+    AUTODISCOVER_SRV_QUERY_STRING=$(get_var_from_file "autodiscover_srv_query_string" "$CONFIG_FILE")
+    AUTODISCOVER_A_QUERY_STRING=$(get_var_from_file "autodiscover_a_query_string" "$CONFIG_FILE")
 
     # Store values from config file into
     # debconf db.
@@ -45,6 +48,9 @@
     db_set $PACKAGE/https_proxy $HTTPS_PROXY
     db_set $PACKAGE/otp $OTP
     db_set $PACKAGE/tags $TAGS
+    db_set $PACKAGE/server_autodiscover $TAGS
+    db_set $PACKAGE/autodiscover_srv_query_string $TAGS
+    db_set $PACKAGE/autodiscover_a_query_string $TAGS
 fi
 
 # Ask questions.
@@ -58,5 +64,8 @@
 db_input "$priority" $PACKAGE/computer_title || true
 db_input "$priority" $PACKAGE/account_name || true
 db_input "$priority" $PACKAGE/registration_password || true
+db_input "$priority" $PACKAGE/server_autodiscover || true
+db_input "$priority" $PACKAGE/autodiscover_srv_query_string || true
+db_input "$priority" $PACKAGE/autodiscover_a_query_string || true
 db_go || true
 

=== modified file 'debian/landscape-client.templates'
--- debian/landscape-client.templates	2011-10-20 10:59:10 +0000
+++ debian/landscape-client.templates	2012-02-08 00:58:28 +0000
@@ -83,3 +83,24 @@
  Register this system with a preexisting Landscape account.  Please
   go to http://landscape.canonical.com if you need a Landscape account.
 
+Template: landscape-client/server_autodiscover
+Type: boolean
+Default: false
+_Description: Enable server autodiscovery?
+ Enable DNS searches to discover the hostname of a Landscape Dedicated
+ Server.
+
+Template: landscape-client/autodiscover_srv_query_string
+Type: string
+Default: _landscape._tcp.localdomain
+_Description: DNS SRV query string for discovering the server name:
+ When server autodiscovery is enabled, this string is used to make a
+ DNS SRV query to find the location of the Landscape server.  This
+ will override portions of the ping url and https url settings.
+
+Template: landscape-client/autodiscover_a_query_string
+Type: string
+_Description: DNS A name query string for discovering the server name:
+ When server autodiscovery is enabled, this string is used to make a
+ DNS A name query to find the location of the Landscape server.  This
+ will override portions of the ping url and https url settings.

=== modified file 'landscape-client.conf'
--- landscape-client.conf	2012-01-21 00:07:53 +0000
+++ landscape-client.conf	2012-02-08 00:58:28 +0000
@@ -9,5 +9,3 @@
 log_level = debug
 pid_file = /tmp/landscape/landscape-client.pid
 ping_url = http://localhost:8081/ping
-server_autodiscover = false      
-

=== modified file 'landscape/broker/config.py'
--- landscape/broker/config.py	2012-02-02 02:49:13 +0000
+++ landscape/broker/config.py	2012-02-08 00:58:28 +0000
@@ -45,6 +45,9 @@
               - C{otp}
               - C{record}
               - C{provisioning_otp}
+              - C{server_autodiscover}
+              - C{autodiscover_srv_query_string}
+              - C{autodiscover_a_query_string}
         """
         parser = super(BrokerConfiguration, self).make_parser()
 
@@ -91,6 +94,13 @@
                           help="The OTP to use for a provisioned machine.")
         parser.add_option("--server-autodiscover", type="string",
                           default="false", help="Enable server autodiscovery.")
+        parser.add_option("--autodiscover-srv-query-string", type="string",
+                          default="_tcp._landscape.localdomain",
+                          help="autodiscovery string for DNS SRV queries")
+        parser.add_option("--autodiscover-a-query-string", type="string",
+                          default="landscape.localdomain",
+                          help="autodiscovery string for DNS A queries")
+
         return parser
 
     @property

=== modified file 'landscape/broker/dnslookup.py'
--- landscape/broker/dnslookup.py	2012-02-02 01:39:15 +0000
+++ landscape/broker/dnslookup.py	2012-02-08 00:58:28 +0000
@@ -5,31 +5,36 @@
 from twisted.names.client import Resolver
 
 
-def discover_server(resolver=None):
+def discover_server(resolver=None, autodiscover_srv_query_string="",
+                    autodiscover_a_query_string=""):
     """
     Look up the dns location of the landscape server.
 
     @type resolver: The resolver to use.  If none is specified a resolver that
         uses settings from /etc/resolv.conf will be created.
+    @param autodiscover_srv_query_string: The query string to send to the DNS
+        server when making a SRV query.
+    @param autodiscover_a_query_string: The query string to send to the DNS
+        server when making a A query.
     """
     if not resolver:
         resolver = Resolver("/etc/resolv.conf")
-    d = lookup_server_record(resolver)
-    d.addErrback(lookup_hostname, resolver)
+    d = lookup_server_record(resolver, autodiscover_srv_query_string)
+    d.addErrback(lookup_hostname, resolver, autodiscover_a_query_string)
     return d
 
 
-def lookup_server_record(resolver):
+def lookup_server_record(resolver, service_name):
     """
     Do a DNS SRV record lookup for the location of the landscape server.
 
     @type resolver: A resolver to use for DNS lookups
         L{twisted.names.client.Resolver}.
+    @param service_name: The query string to send to the DNS server when
+        making a SRV query.
     @return: A deferred containing either the hostname of the landscape server
         if found or an empty string if not found.
     """
-    service_name = "_landscape._tcp.mylandscapehost.com"
-
     def lookup_done(result):
         name = ""
         for item in result:
@@ -49,17 +54,17 @@
     return d
 
 
-def lookup_hostname(result, resolver):
+def lookup_hostname(result, resolver, hostname):
     """
     Do a DNS name lookup for the location of the landscape server.
 
     @param result: The result from a call to lookup_server_record.
     @param resolver: The resolver to use for DNS lookups.
+    @param hostname: The query string to send to the DNS server when making
+        a A query.
     @param return: A deferred containing the ip address of the landscape
         server if found or None if not found.
     """
-    hostname = "landscape.localdomain"
-
     def lookup_done(result):
         return result
 

=== modified file 'landscape/broker/ping.py'
--- landscape/broker/ping.py	2012-01-27 22:00:02 +0000
+++ landscape/broker/ping.py	2012-02-08 00:58:28 +0000
@@ -22,10 +22,19 @@
     @param url: The URL to ask the question to.
     @type identity: L{landscape.broker.registration.Identity}
     @param identity: This client's identity.
+    @param get_page: The method to use to retrieve content.  If not specified,
+        landscape.lib.fetch.fetch is used.
+    @param server_autodiscovery: Server autodiscovery is performed if True,
+        otherwise server autodiscover is not done.
+    @param autodiscover_srv_query_string: If server autodiscovery is done, this
+        string will be sent to the DNS server when making a SRV query.
+    @param autodiscover_a_query_string: If server autodiscovery is done, this
+        string will be sent to the DNS server when making an A query.
     """
 
     def __init__(self, reactor, url, identity, get_page=None,
-                 server_autodiscover=False):
+                 server_autodiscover=False, autodiscover_srv_query_string="",
+                 autodiscover_a_query_string=""):
         if get_page is None:
             get_page = fetch
         self._reactor = reactor
@@ -33,6 +42,8 @@
         self.get_page = get_page
         self.url = url
         self._server_autodiscover = server_autodiscover
+        self._autodiscover_srv_query_string = autodiscover_srv_query_string
+        self._autodiscover_a_query_string = autodiscover_a_query_string
 
     def ping(self):
         """Ask the question.
@@ -72,7 +83,9 @@
             return defer.succeed(False)
 
         if self._server_autodiscover:
-            lookup_deferred = discover_server()
+            lookup_deferred = discover_server(
+                None, self._autodiscover_srv_query_string,
+                self._autodiscover_a_query_string)
             lookup_deferred.addCallback(handle_result)
             lookup_deferred.addCallback(do_rest)
             return lookup_deferred
@@ -103,7 +116,8 @@
 
     def __init__(self, reactor, url, identity, exchanger,
                  interval=30, ping_client_factory=PingClient,
-                 server_autodiscover=False):
+                 server_autodiscover=False, autodiscover_srv_query_string="",
+                 autodiscover_a_query_string=""):
         self._url = url
         self._interval = interval
         self._identity = identity
@@ -113,6 +127,8 @@
         self._ping_client = None
         self.ping_client_factory = ping_client_factory
         self._server_autodiscover = server_autodiscover
+        self._autodiscover_srv_query_string = autodiscover_srv_query_string
+        self._autodiscover_a_query_string = autodiscover_a_query_string
         reactor.call_on("message", self._handle_set_intervals)
 
     def get_url(self):
@@ -130,7 +146,9 @@
         """Start pinging."""
         self._ping_client = self.ping_client_factory(
             self._reactor, self._url, self._identity,
-            server_autodiscover=self._server_autodiscover)
+            server_autodiscover=self._server_autodiscover,
+            autodiscover_srv_query_string=self._autodiscover_srv_query_string,
+            autodiscover_a_query_string=self._autodiscover_a_query_string)
         self._call_id = self._reactor.call_every(self._interval, self.ping)
 
     def ping(self):

=== modified file 'landscape/broker/service.py'
--- landscape/broker/service.py	2012-01-30 16:11:20 +0000
+++ landscape/broker/service.py	2012-02-08 00:58:28 +0000
@@ -55,7 +55,9 @@
 
         self.transport = self.transport_factory(
             self.reactor, config.url, config.ssl_public_key,
-            self.payload_recorder, config.server_autodiscover)
+            self.payload_recorder, config.server_autodiscover,
+            config.autodiscover_srv_query_string,
+            config.autodiscover_a_query_string)
         self.message_store = get_default_message_store(
             self.persist, config.message_store_path)
         self.identity = Identity(self.config, self.persist)
@@ -66,7 +68,9 @@
         self.pinger = self.pinger_factory(
             self.reactor, config.ping_url, self.identity, self.exchanger,
             interval=config.ping_interval,
-            server_autodiscover=config.server_autodiscover)
+            server_autodiscover=config.server_autodiscover,
+            autodiscover_srv_query_string=config.autodiscover_srv_query_string,
+            autodiscover_a_query_string=config.autodiscover_a_query_string)
         self.registration = RegistrationHandler(
             config, self.identity, self.reactor, self.exchanger, self.pinger,
             self.message_store, fetch_async)

=== modified file 'landscape/broker/tests/test_config.py'
--- landscape/broker/tests/test_config.py	2012-02-02 02:49:13 +0000
+++ landscape/broker/tests/test_config.py	2012-02-08 00:58:28 +0000
@@ -92,19 +92,19 @@
         self.assertEqual(configuration.url,
                          "https://landscape.canonical.com/message-system")
 
-    def test_server_autodiscovery_handling(self):
+    def test_server_autodiscover_handling(self):
         """
         server_autodiscover is parsed and converted to a boolean value by
         load().
         """
         configuration = BrokerConfiguration()
         configuration.load([])
-        self.assertEquals(configuration.server_autodiscover, False)
+        self.assertEqual(configuration.server_autodiscover, False)
 
         configuration = BrokerConfiguration()
         configuration.load(["--server-autodiscover=true"])
-        self.assertEquals(configuration.server_autodiscover, True)
+        self.assertEqual(configuration.server_autodiscover, True)
 
         configuration = BrokerConfiguration()
         configuration.load(["--server-autodiscover=false"])
-        self.assertEquals(configuration.server_autodiscover, False)
+        self.assertEqual(configuration.server_autodiscover, False)

=== modified file 'landscape/broker/tests/test_dnslookup.py'
--- landscape/broker/tests/test_dnslookup.py	2012-02-02 01:39:15 +0000
+++ landscape/broker/tests/test_dnslookup.py	2012-02-08 00:58:28 +0000
@@ -46,13 +46,16 @@
     def __init__(self):
         self.results = None
         self.name = None
+        self.queried = None
 
     def lookupService(self, arg1):
+        self.queried = arg1
         deferred = defer.Deferred()
         deferred.callback(self.results)
         return deferred
 
     def getHostByName(self, arg1):
+        self.queried = arg1
         deferred = defer.Deferred()
         deferred.callback(self.name)
         return deferred
@@ -84,11 +87,13 @@
         fake_result.payload.target.name = "a.b.com"
         fake_resolver = FakeResolver()
         fake_resolver.results = [[fake_result]]
+        query_string = "_landscape._tcp.mylandscapehost.com"
 
         def check(result):
-            self.assertEquals("a.b.com", result)
+            self.assertEqual(fake_resolver.queried, query_string)
+            self.assertEqual("a.b.com", result)
 
-        d = lookup_server_record(fake_resolver)
+        d = lookup_server_record(fake_resolver, query_string)
         d.addCallback(check)
         return d
 
@@ -101,9 +106,10 @@
         fake_resolver.results = [[]]
 
         def check(result):
-            self.assertEquals("", result)
+            self.assertEqual("", result)
 
-        d = lookup_server_record(fake_resolver)
+        d = lookup_server_record(fake_resolver,
+                                 "_landscape._tcp.mylandscapehost.com")
         d.addCallback(check)
         return d
 
@@ -115,7 +121,8 @@
                      "failed.")
         self.mocker.replay()
 
-        d = lookup_server_record(BadResolver())
+        d = lookup_server_record(BadResolver(),
+                                 "_landscape._tcp.mylandscapehost.com")
         self.assertFailure(d, ResolverError)
         return d
 
@@ -127,11 +134,13 @@
         """
         fake_resolver = FakeResolver()
         fake_resolver.name = "a.b.com"
+        query_string = "landscape.localdomain"
 
         def check(result):
-            self.assertEquals("a.b.com", result)
+            self.assertEqual(fake_resolver.queried, query_string)
+            self.assertEqual("a.b.com", result)
 
-        d = lookup_hostname(None, fake_resolver)
+        d = lookup_hostname(None, fake_resolver, query_string)
         d.addCallback(check)
         return d
 
@@ -144,9 +153,9 @@
         fake_resolver.name = None
 
         def check(result):
-            self.assertEquals(None, result)
+            self.assertEqual(None, result)
 
-        d = lookup_hostname(None, fake_resolver)
+        d = lookup_hostname(None, fake_resolver, "landscape.localdomain")
         d.addCallback(check)
         return d
 
@@ -157,7 +166,7 @@
         logging_mock("Name lookup of landscape.localdomain failed.")
         self.mocker.replay()
 
-        d = lookup_hostname(None, BadResolver())
+        d = lookup_hostname(None, BadResolver(), "landscape.localdomain")
         self.assertFailure(d, ResolverError)
         return d
 
@@ -174,7 +183,7 @@
         d = discover_server(fake_resolver)
 
         def check(result):
-            self.assertEquals("a.b.com", result)
+            self.assertEqual("a.b.com", result)
 
         d.addCallback(check)
         return d
@@ -187,13 +196,13 @@
         d = discover_server(fake_resolver)
 
         def check(result):
-            self.assertEquals("x.y.com", result)
+            self.assertEqual("x.y.com", result)
 
         d.addCallback(check)
         return d
 
     def test_failed_lookup(self):
         """A resolver error is returned when server autodiscovery fails."""
-        d = lookup_server_record(BadResolver())
+        d = lookup_server_record(BadResolver(), "landscape.localdomain")
         self.assertFailure(d, ResolverError)
         return d

=== modified file 'landscape/broker/tests/test_ping.py'
--- landscape/broker/tests/test_ping.py	2012-01-19 23:34:55 +0000
+++ landscape/broker/tests/test_ping.py	2012-02-08 00:58:28 +0000
@@ -128,7 +128,9 @@
         self.url = "http://localhost:8081/whatever"
         self.page_getter = FakePageGetter(None)
 
-        def factory(reactor, url, insecure_id, server_autodiscover):
+        def factory(reactor, url, insecure_id, server_autodiscover,
+                    autodiscover_srv_query_string,
+                    autodiscover_a_query_string):
             return PingClient(reactor, url, insecure_id,
                               get_page=self.page_getter.get_page)
         self.pinger = Pinger(self.broker_service.reactor,

=== modified file 'landscape/broker/transport.py'
--- landscape/broker/transport.py	2012-01-26 23:37:53 +0000
+++ landscape/broker/transport.py	2012-02-08 00:58:28 +0000
@@ -22,15 +22,24 @@
     @param pubkey: SSH public key used for secure communication.
     @param payload_recorder: PayloadRecorder used for recording exchanges
         with the server.  If C{None}, exchanges will not be recorded.
+    @param server_autodiscovery: Server autodiscovery is performed if True,
+        otherwise server autodiscover is not done.
+    @param autodiscover_srv_query_string: If server autodiscovery is done, this
+        string will be sent to the DNS server when making a SRV query.
+    @param autodiscover_a_query_string: If server autodiscovery is done, this
+        string will be sent to the DNS server when making an A query.
     """
 
     def __init__(self, reactor, url, pubkey=None, payload_recorder=None,
-                 server_autodiscover=False):
+                 server_autodiscover=False, autodiscover_srv_query_string="",
+                 autodiscover_a_query_string=""):
         self._reactor = reactor
         self._url = url
         self._pubkey = pubkey
         self._payload_recorder = payload_recorder
         self._server_autodiscover = server_autodiscover
+        self._autodiscover_srv_query_string = autodiscover_srv_query_string
+        self._autodiscover_a_query_string = autodiscover_a_query_string
 
     def get_url(self):
         """Get the URL of the remote message system."""
@@ -42,7 +51,10 @@
 
     def _curl(self, payload, computer_id, message_api):
         if self._server_autodiscover:
-            result = blockingCallFromThread(self._reactor, discover_server)
+            result = blockingCallFromThread(
+                self._reactor, discover_server, None,
+                self._autodiscover_srv_query_string,
+                self._autodiscover_a_query_string)
             if result is not None:
                 self._url = "https://%s/message-system" % result
             else:
@@ -166,7 +178,9 @@
     """Fake transport for testing purposes."""
 
     def __init__(self, reactor=None, url=None, pubkey=None,
-                 payload_recorder=None, server_autodiscover=False):
+                 payload_recorder=None, server_autodiscover=False,
+                 autodiscover_srv_query_string="",
+                 autodiscover_a_query_string=""):
         self._pubkey = pubkey
         self._payload_recorder = payload_recorder
         self.payloads = []
@@ -179,6 +193,8 @@
         self._url = url
         self._reactor = reactor
         self._server_autodiscover = server_autodiscover
+        self._autodiscover_srv_query_string = autodiscover_srv_query_string
+        self._autodiscover_a_query_string = autodiscover_a_query_string
 
     def get_url(self):
         return self._url