Merge lp:~free.ekanayaka/landscape-client/juju-machine-info into lp:~landscape/landscape-client/trunk

[Free Ekanayaka]

This branch changes the get_juju_info utility to return information
about the Juju machine ID the client runs in. Since we're in the
middle of the transition, both the old info with unit names and the
new info with the machine ID are returned. When the transition completes
in the server too the old info can be dropped.

This branch changes the get_juju_info utility to return information
about the Juju machine ID the client runs in. Since we're in the
middle of the transition, both the old info with unit names and the
new info with the machine ID are returned. When the transition completes
in the server too the old info can be dropped.

[lp:~free.ekanayaka/landscape-client/juju-machine-info]

784. By Free Ekanayaka on 2014-08-22

Fix failing tests

[Adam Collard (adam-collard)]

3 inline comments

review: Needs Information

[Alberto Donato (ack)]

Code looks good, just a question:

what's the plan for the charm? Will we just add the "machine-id" and keep "unit-name" for backwards compatibility?

review: Needs Information

[Free Ekanayaka (free.ekanayaka)]

Alberto: for the charm we don't need backward compatibility since never released and LDS with support for Juju integration. So the current code in the charm is basically dead code from the user perspective, and we can change it to send just machine-id. Said that, for sake of our internal migration we'll keep both for a bit, till we complete the process.

[Free Ekanayaka (free.ekanayaka)]

Thanks Adam, replied/fixed all three issues.

[lp:~free.ekanayaka/landscape-client/juju-machine-info]

785. By Free Ekanayaka on 2014-08-27

Address review comments

[Alberto Donato (ack)]

+1!

review: Approve

[Adam Collard (adam-collard)]

Thanks Free.

Lint issue:

landscape/broker/tests/test_registration.py:5: 'SERVER_API' imported but unused

Otherwise looks good. +1

review: Approve

[lp:~free.ekanayaka/landscape-client/juju-machine-info]

786. By Free Ekanayaka on 2014-08-28

Fix lint

=== modified file 'landscape/broker/registration.py'
--- landscape/broker/registration.py	2014-07-16 18:04:22 +0000
+++ landscape/broker/registration.py	2014-08-28 07:37:26 +0000
@@ -126,7 +126,7 @@
 
     def _get_juju_data(self):
         """Load Juju information."""
-        juju_info = get_juju_info(self._config)
+        juju_info = get_juju_info(self._config)[0]
         if juju_info is None:
             return None
         self._juju_data = juju_info  # A list of dicts
 
=== modified file 'landscape/broker/service.py'
--- landscape/broker/service.py	2013-05-07 22:47:06 +0000
+++ landscape/broker/service.py	2014-08-28 07:37:26 +0000
@@ -1,7 +1,7 @@
 """Deployment code for the monitor."""
 
 import os
-from landscape.lib.fetch import fetch_async
+
 from landscape.service import LandscapeService, run_landscape_service
 from landscape.amp import ComponentPublisher
 from landscape.broker.registration import RegistrationHandler, Identity
@@ -61,7 +61,7 @@
             self.reactor, self.identity, self.exchanger, config)
         self.registration = RegistrationHandler(
             config, self.identity, self.reactor, self.exchanger, self.pinger,
-            self.message_store, fetch_async)
+            self.message_store)
         self.broker = BrokerServer(self.config, self.reactor, self.exchanger,
                                    self.registration, self.message_store,
                                    self.pinger)
 
=== modified file 'landscape/broker/tests/helpers.py'
--- landscape/broker/tests/helpers.py	2014-06-09 08:18:05 +0000
+++ landscape/broker/tests/helpers.py	2014-08-28 07:37:26 +0000
@@ -7,7 +7,6 @@
 """
 import os
 
-from landscape.lib.fetch import fetch_async
 from landscape.lib.persist import Persist
 from landscape.watchdog import bootstrap_list
 from landscape.reactor import FakeReactor
@@ -112,20 +111,12 @@
     The following attributes will be set in your test case:
 
       - C{handler}: A L{RegistrationHandler}.
-
-      - C{fetch_func}: The C{fetch_async} function used by the C{handler}, it
-        can be customised by test cases.
     """
 
     def set_up(self, test_case):
         super(RegistrationHelper, self).set_up(test_case)
         test_case.pinger = Pinger(test_case.reactor, test_case.identity,
                                   test_case.exchanger, test_case.config)
-
-        def fetch_func(*args, **kwargs):
-            return test_case.fetch_func(*args, **kwargs)
-
-        test_case.fetch_func = fetch_async
         test_case.config.cloud = getattr(test_case, "cloud", False)
         if hasattr(test_case, "juju_contents"):
             if not os.path.exists(test_case.config.juju_directory):
@@ -135,8 +126,7 @@
                 dirname=test_case.config.juju_directory, suffix=".json")
         test_case.handler = RegistrationHandler(
             test_case.config, test_case.identity, test_case.reactor,
-            test_case.exchanger, test_case.pinger, test_case.mstore,
-            fetch_async=fetch_func)
+            test_case.exchanger, test_case.pinger, test_case.mstore)
 
 
 class BrokerServerHelper(RegistrationHelper):
 
=== modified file 'landscape/broker/tests/test_exchange.py'
--- landscape/broker/tests/test_exchange.py	2014-07-16 11:41:29 +0000
+++ landscape/broker/tests/test_exchange.py	2014-08-28 07:37:26 +0000
@@ -1,7 +1,6 @@
 from landscape import SERVER_API, CLIENT_API
 from landscape.lib.persist import Persist
 from landscape.lib.hashlib import md5
-from landscape.lib.fetch import fetch_async
 from landscape.schema import Message, Int
 from landscape.broker.config import BrokerConfiguration
 from landscape.broker.exchange import get_accepted_types_diff, MessageExchange
@@ -1061,7 +1060,7 @@
         # message types that we want to catch as well
         self.handler = RegistrationHandler(
             self.config, self.identity, self.reactor, self.exchanger,
-            self.pinger, self.mstore, fetch_async)
+            self.pinger, self.mstore)
 
     def test_register_accepted_message_type(self):
         self.exchanger.register_client_accepted_message_type("type-B")
 
=== modified file 'landscape/broker/tests/test_registration.py'
--- landscape/broker/tests/test_registration.py	2014-07-24 12:21:06 +0000
+++ landscape/broker/tests/test_registration.py	2014-08-28 07:37:26 +0000
@@ -2,7 +2,6 @@
 import logging
 import socket
 
-from landscape import SERVER_API
 from landscape.broker.registration import (
     InvalidCredentialsError, Identity)
 from landscape.tests.helpers import LandscapeTest
 
=== modified file 'landscape/lib/juju.py'
--- landscape/lib/juju.py	2014-06-11 08:47:29 +0000
+++ landscape/lib/juju.py	2014-08-28 07:37:26 +0000
@@ -9,22 +9,28 @@
 
 def get_juju_info(config):
     """
-    Returns the list of Juju info or C{None} if the path referenced from
+    Returns available Juju info or C{None} if the path referenced from
     L{config} is not a valid directory.
 
-    The list of juju info is constructed by appending all the contents of
+    XXX At the moment this function returns a 2-tuple because we're
+    transitioning from unit-computer associations to machine-computer
+    associations. Once the transition is completed in the server, the
+    old format can be dropped.
+
+    The list of old juju info is constructed by appending all the contents of
     *.json files found in the path referenced from the L{config}.
     """
     juju_directory = config.juju_directory
     legacy_juju_file = config.juju_filename
 
+    new_juju_info = {}
     juju_info_list = []
     juju_file_list = glob("%s/*.json" % juju_directory)
 
     if os.path.exists(legacy_juju_file):
         juju_file_list.append(legacy_juju_file)
 
-    for juju_file in juju_file_list:
+    for index, juju_file in enumerate(juju_file_list):
 
         json_contents = read_file(juju_file)
         try:
@@ -36,11 +42,25 @@
             logging.exception(
                 "Error attempting to read JSON from %s" % juju_file)
             return None
-        else:
-            if "api-addresses" in juju_info:
-                split = juju_info["api-addresses"].split()
-                juju_info["api-addresses"] = split
-            juju_info_list.append(juju_info)
+
+        if "api-addresses" in juju_info:
+            split = juju_info["api-addresses"].split()
+            juju_info["api-addresses"] = split
+
+        # Strip away machine-id, which is not understood by the old format
+        machine_id = juju_info.pop("machine-id", None)
+
+        if index == 0 and machine_id is not None:
+            # We care only about the first file, as the machine ID is the same
+            # for all
+            new_juju_info["environment-uuid"] = juju_info["environment-uuid"]
+            new_juju_info["api-addresses"] = juju_info["api-addresses"]
+            new_juju_info["machine-id"] = machine_id
+
+        juju_info_list.append(juju_info)
 
     juju_info_list.sort(key=lambda x: x["unit-name"])
-    return juju_info_list or None
+
+    if juju_info_list:
+        return juju_info_list, new_juju_info
+    return None
 
=== modified file 'landscape/lib/tests/test_juju.py'
--- landscape/lib/tests/test_juju.py	2014-06-11 08:47:29 +0000
+++ landscape/lib/tests/test_juju.py	2014-08-28 07:37:26 +0000
@@ -6,11 +6,13 @@
 
 
 SAMPLE_JUJU_INFO = json.dumps({"environment-uuid": "DEAD-BEEF",
+                               "machine-id": "1",
                                "unit-name": "service/0",
                                "api-addresses": "10.0.3.1:17070",
                                "private-address": "127.0.0.1"})
 
 SAMPLE_JUJU_INFO_2 = json.dumps({"environment-uuid": "DEAD-BEEF",
+                                 "machine-id": "1",
                                  "unit-name": "service-2/0",
                                  "api-addresses": "10.0.3.2:17070",
                                  "private-address": "127.0.0.1"})
@@ -28,11 +30,23 @@
         return self.makeFile(
             contents, dirname=self.stub_config.juju_directory, suffix=".json")
 
+    def test_get_juju_info(self):
+        """
+        L{get_juju_info} parses JSON data from the '*.json' files in the
+        juju_directory. A single file is present.
+        """
+        self._create_tmp_juju_file(SAMPLE_JUJU_INFO)
+        _, juju_info = get_juju_info(self.stub_config)
+        self.assertEqual(
+            {u"environment-uuid": "DEAD-BEEF",
+             u"machine-id": "1",
+             u"api-addresses": ["10.0.3.1:17070"]}, juju_info)
+
     def test_get_juju_info_sample_data(self):
         """L{get_juju_info} parses JSON data from the '*.json' files in the
         juju_directory. A single file is present."""
         self._create_tmp_juju_file(SAMPLE_JUJU_INFO)
-        juju_info = get_juju_info(self.stub_config)
+        juju_info, _ = get_juju_info(self.stub_config)
         self.assertEqual([
             {u"environment-uuid": "DEAD-BEEF",
              u"unit-name": "service/0",
@@ -45,7 +59,7 @@
         landscape-client charm."""
         self.stub_config = self.Config(self.makeDir(), self.makeFile(
             content=SAMPLE_JUJU_INFO, basename="juju-info", suffix=".json"))
-        juju_info = get_juju_info(self.stub_config)
+        juju_info, _ = get_juju_info(self.stub_config)
         self.assertEqual([
             {u"environment-uuid": "DEAD-BEEF",
              u"unit-name": "service/0",
@@ -59,7 +73,7 @@
         """
         self._create_tmp_juju_file(SAMPLE_JUJU_INFO)
         self._create_tmp_juju_file(SAMPLE_JUJU_INFO_2)
-        juju_info = get_juju_info(self.stub_config)
+        juju_info, _ = get_juju_info(self.stub_config)
         self.assertEqual([
             {u"environment-uuid": "DEAD-BEEF",
              u"unit-name": "service-2/0",
@@ -77,7 +91,7 @@
         self._create_tmp_juju_file(SAMPLE_JUJU_INFO)
         self.makeFile(SAMPLE_JUJU_INFO_2, suffix=".txt",
                       dirname=self.stub_config.juju_directory)
-        juju_info = get_juju_info(self.stub_config)
+        juju_info, _ = get_juju_info(self.stub_config)
         self.assertEqual([
             {u"environment-uuid": "DEAD-BEEF",
              u"unit-name": "service/0",
@@ -89,10 +103,9 @@
         If L{get_juju_info} is called with a configuration pointing to
         an empty file, it returns C{None}.
         """
+        self.log_helper.ignore_errors(ValueError)
         self._create_tmp_juju_file("")
-        juju_info = get_juju_info(self.stub_config)
-        self.log_helper.ignore_errors(ValueError)
-        self.assertEqual(juju_info, None)
+        self.assertIs(None, get_juju_info(self.stub_config))
         self.assertIn("Error attempting to read JSON", self.logfile.getvalue())
 
     def test_get_juju_info_no_json_file(self):
@@ -100,19 +113,19 @@
         If L{get_juju_info} is called with a configuration pointing to
         a directory containing no json files, it returns None.
         """
-        juju_info = get_juju_info(self.stub_config)
-        self.assertIs(juju_info, None)
+        self.assertIs(None, get_juju_info(self.stub_config))
 
     def test_get_juju_info_multiple_endpoints(self):
         """L{get_juju_info} turns space separated API addresses into a list."""
         juju_multiple_endpoints = json.dumps({
                 "environment-uuid": "DEAD-BEEF",
+                "machine-id": "0",
                 "unit-name": "service/0",
                 "api-addresses": "10.0.3.1:17070 10.0.3.2:18080",
                 "private-address": "127.0.0.1"})
 
         self._create_tmp_juju_file(juju_multiple_endpoints)
-        juju_info = get_juju_info(self.stub_config)
+        juju_info, _ = get_juju_info(self.stub_config)
         self.assertEqual([
             {"environment-uuid": "DEAD-BEEF",
              "unit-name": "service/0",
 
=== modified file 'landscape/monitor/jujuinfo.py'
--- landscape/monitor/jujuinfo.py	2014-06-30 18:40:29 +0000
+++ landscape/monitor/jujuinfo.py	2014-08-28 07:37:26 +0000
@@ -5,7 +5,12 @@
 
 
 class JujuInfo(MonitorPlugin):
-    """Plugin for reporting Juju information."""
+    """Plugin for reporting Juju information.
+
+    XXX this plugin is going to be dropped when the transition from
+    unit-computer association to machine-computer association is
+    completed on the server.
+    """
 
     persist_name = "juju-info"
     scope = "juju"
@@ -35,6 +40,9 @@
         """
         juju_info = get_juju_info(self.registry.config)
 
+        if juju_info:
+            juju_info = juju_info[0]
+
         if juju_info != self._persist.get("juju-info"):
             self._persist.set("juju-info", juju_info)
             message = {"type": "juju-units-info",
 
=== modified file 'landscape/monitor/tests/test_jujuinfo.py'
--- landscape/monitor/tests/test_jujuinfo.py	2014-06-27 11:02:24 +0000
+++ landscape/monitor/tests/test_jujuinfo.py	2014-08-28 07:37:26 +0000
@@ -6,6 +6,7 @@
 
 
 SAMPLE_JUJU_INFO = json.dumps({"environment-uuid": "DEAD-BEEF",
+                               "machine-id": "1",
                                "unit-name": "juju-unit-name",
                                "api-addresses": "10.0.3.1:17070",
                                "private-address": "127.0.0.1"})
@@ -72,6 +73,7 @@
 
         self.makeFile(
             json.dumps({"environment-uuid": "FEED-BEEF",
+                        "machine-id": "1",
                         "unit-name": "changed-unit-name",
                         "api-addresses": "10.0.3.2:17070",
                         "private-address": "127.0.1.1"}),