Merge lp:~therve/landscape-client/ec2-no-ramdisk into lp:~landscape/landscape-client/trunk

[Thomas Herve]

[Free Ekanayaka (free.ekanayaka)]

Looks good! +1

[1]

+ deferred.addCallback(
+ lambda ignore: self._fetch_async(
+ EC2_API + "/meta-data/ramdisk-id").addErrback(log_failure))
+ deferred.addCallback(ec2_data.append)

Maybe it'd be worth adding a comment about why we need to special case ramdisk-id.

review: Approve

[Andreas Hasenack (ahasenack)]

I don't think it's working, or I did something wrong. The registration doesn't "stick". Attached is the full broker log of this client running on a lucid instance in the APAC region. Below are the last bits of my broker.log, but the pattern is the same all over the log:

2010-05-05 14:43:40,370 INFO [MainThread] Broker stopped with config /etc/landscape/client.conf
2010-05-05 14:43:41,865 INFO [MainThread] Broker started with config /etc/landscape/client.conf
2010-05-05 14:43:41,881 ERROR [MainThread] None
Traceback (most recent call last):
  File "/usr/lib/python2.6/dist-packages/twisted/python/threadpool.py", line 210, in _worker
    result = context.call(ctx, function, *args, **kwargs)
  File "/usr/lib/python2.6/dist-packages/twisted/python/context.py", line 59, in callWithContext
    return self.currentContext().callWithContext(ctx, func, *args, **kw)
  File "/usr/lib/python2.6/dist-packages/twisted/python/context.py", line 37, in callWithContext
    return func(*args,**kw)
  File "/usr/lib/python2.6/dist-packages/landscape/lib/fetch.py", line 98, in fetch
    raise HTTPCodeError(http_code, body)
HTTPCodeError: Server returned HTTP code 404
2010-05-05 14:44:41,870 INFO [MainThread] Queueing message to register with OTP
2010-05-05 14:44:41,872 INFO [MainThread] Starting urgent message exchange with https://staging.landscape.canonical.com/message-system.
2010-05-05 14:44:44,462 INFO [Dummy-1 ] Sent 1146 bytes and received 70 bytes in 2.59s.
2010-05-05 14:44:44,466 INFO [MainThread] Switching to normal exchange mode.
2010-05-05 14:44:44,466 INFO [MainThread] Server UUID changed (old=af6f2dcf-1967-11de-8dd0-001a4b4d8d10, new=7c6efe47-12fb-11de-91bb-0016353c8658).
2010-05-05 14:44:44,467 INFO [MainThread] Message exchange completed in 2.60s.
2010-05-05 14:59:44,474 INFO [MainThread] Queueing message to register with OTP
2010-05-05 14:59:44,477 INFO [MainThread] Starting message exchange with https://staging.landscape.canonical.com/message-system.
2010-05-05 14:59:47,343 INFO [Dummy-1 ] Sent 1146 bytes and received 70 bytes in 2.87s.
2010-05-05 14:59:47,348 INFO [MainThread] Message exchange completed in 2.87s.

[Thomas Herve (therve)]

It can't work against staging or production because I updated the message schema to allow the ramdisk to be None.

[Andreas Hasenack (ahasenack)]

Cool. Works fine after I start a trunk server using this client branch.

[Sidnei da Silva (sidnei)]

Looks good. Please add a comment as per Free's request. +1.

review: Approve

[lp:~therve/landscape-client/ec2-no-ramdisk]

243. By Thomas Herve on 2010-05-11

Add comment

=== modified file 'landscape/broker/registration.py'
--- landscape/broker/registration.py	2010-04-28 13:07:53 +0000
+++ landscape/broker/registration.py	2010-05-11 09:40:57 +0000
@@ -146,19 +146,23 @@
                 "/meta-data/public-hostname",
                 "/meta-data/ami-launch-index",
                 "/meta-data/kernel-id",
-                "/meta-data/ramdisk-id",
                 "/meta-data/ami-id"]
             # We're not using a DeferredList here because we want to keep the
             # number of connections to the backend minimal. See lp:567515.
             for path in paths:
                 deferred.addCallback(
                     lambda ignore, path=path: self._get_data(path, ec2_data))
+            # Special case the ramdisk retrieval, because it may not be present
+            deferred.addCallback(
+                lambda ignore: self._fetch_async(
+                    EC2_API + "/meta-data/ramdisk-id").addErrback(log_failure))
+            deferred.addCallback(ec2_data.append)
 
             def record_data(ignore):
                 """Record the instance data returned by the EC2 API."""
                 (raw_user_data, instance_key, reservation_key,
                  local_hostname, public_hostname, launch_index,
-                 kernel_key, ramdisk_key, ami_key) = ec2_data
+                 kernel_key, ami_key, ramdisk_key) = ec2_data
                 self._ec2_data = {
                     "instance_key": instance_key,
                     "reservation_key": reservation_key,
@@ -169,6 +173,8 @@
                     "ramdisk_key": ramdisk_key,
                     "image_key": ami_key}
                 for k, v in self._ec2_data.items():
+                    if v is None and k == "ramdisk_key":
+                        continue
                     self._ec2_data[k] = v.decode("utf-8")
                 self._ec2_data["launch_index"] = int(
                     self._ec2_data["launch_index"])

=== modified file 'landscape/broker/tests/test_registration.py'
--- landscape/broker/tests/test_registration.py	2010-01-06 15:11:24 +0000
+++ landscape/broker/tests/test_registration.py	2010-05-11 09:40:57 +0000
@@ -35,8 +35,9 @@
         self.assertEquals(getattr(self.identity, attr), value,
                           "%r attribute should be %r, not %r" %
                           (attr, value, getattr(self.identity, attr)))
-        self.assertEquals(self.persist.get(persist_name), value,
-                          "%r not set to %r in persist" % (persist_name, value))
+        self.assertEquals(
+            self.persist.get(persist_name), value,
+            "%r not set to %r in persist" % (persist_name, value))
 
     def check_config_property(self, attr):
         value = "VALUE"
@@ -151,8 +152,7 @@
                               "account_name": "account_name",
                               "registration_password": None,
                               "hostname": "ooga.local",
-                              "tags": None,}
-                            ])
+                              "tags": None}])
         self.assertEquals(self.logfile.getvalue().strip(),
                           "INFO: Queueing message to register with account "
                           "'account_name' without a password.")
@@ -170,8 +170,7 @@
                               "account_name": "account_name",
                               "registration_password": "SEKRET",
                               "hostname": "ooga.local",
-                              "tags": None,}
-                            ])
+                              "tags": None}])
         self.assertEquals(self.logfile.getvalue().strip(),
                           "INFO: Queueing message to register with account "
                           "'account_name' with a password.")
@@ -193,8 +192,7 @@
                               "account_name": "account_name",
                               "registration_password": "SEKRET",
                               "hostname": "ooga.local",
-                              "tags": u"computer,tag"}
-                            ])
+                              "tags": u"computer,tag"}])
         self.assertEquals(self.logfile.getvalue().strip(),
                           "INFO: Queueing message to register with account "
                           "'account_name' and tags computer,tag "
@@ -219,8 +217,7 @@
                               "account_name": "account_name",
                               "registration_password": "SEKRET",
                               "hostname": "ooga.local",
-                              "tags": None}
-                            ])
+                              "tags": None}])
         self.assertEquals(self.logfile.getvalue().strip(),
                           "ERROR: Invalid tags provided for cloud "
                           "registration.\n    "
@@ -238,14 +235,14 @@
         self.config.registration_password = "SEKRET"
         self.config.tags = u"prova\N{LATIN SMALL LETTER J WITH CIRCUMFLEX}o"
         self.reactor.fire("pre-exchange")
-        self.assertMessages(self.mstore.get_pending_messages(),
-                            [{"type": "register",
-                              "computer_title": "Computer Title",
-                              "account_name": "account_name",
-                              "registration_password": "SEKRET",
-                              "hostname": "ooga.local",
-                              "tags": u"prova\N{LATIN SMALL LETTER J WITH CIRCUMFLEX}o"}
-                            ])
+        self.assertMessages(
+            self.mstore.get_pending_messages(),
+            [{"type": "register",
+              "computer_title": "Computer Title",
+              "account_name": "account_name",
+              "registration_password": "SEKRET",
+              "hostname": "ooga.local",
+              "tags": u"prova\N{LATIN SMALL LETTER J WITH CIRCUMFLEX}o"}])
         self.assertEquals(self.logfile.getvalue().strip(),
                           "INFO: Queueing message to register with account "
                           "'account_name' and tags prova\xc4\xb5o "
@@ -333,9 +330,11 @@
 
         calls = [0]
         d = self.handler.register()
+
         def add_call(result):
             self.assertEquals(result, None)
             calls[0] += 1
+
         d.addCallback(add_call)
 
         # This should somehow callback the deferred.
@@ -355,8 +354,10 @@
     def test_resynchronize_fired_when_registration_done(self):
 
         results = []
+
         def append():
             results.append(True)
+
         self.reactor.call_on("resynchronize-clients", append)
 
         self.handler.register()
@@ -373,10 +374,12 @@
 
         calls = [0]
         d = self.handler.register()
+
         def add_call(failure):
             exception = failure.value
             self.assertTrue(isinstance(exception, InvalidCredentialsError))
             calls[0] += 1
+
         d.addErrback(add_call)
 
         # This should somehow callback the deferred.
@@ -428,8 +431,7 @@
                               "account_name": "account_name",
                               "registration_password": "SEKRET",
                               "hostname": socket.getfqdn(),
-                              "tags": None,}
-                             ])
+                              "tags": None}])
 
 
 class CloudRegistrationHandlerTest(RegistrationHandlerTestBase):
@@ -550,9 +552,10 @@
         # This *should* be asynchronous, but I think a billion tests are
         # written like this
         self.assertEquals(len(self.transport.payloads), 1)
-        self.assertMessages(self.transport.payloads[0]["messages"],
-                            [self.get_expected_cloud_message(tags=u"server,london")])
- 
+        self.assertMessages(
+            self.transport.payloads[0]["messages"],
+            [self.get_expected_cloud_message(tags=u"server,london")])
+
     def test_cloud_registration_with_invalid_tags(self):
         """
         Invalid tags in the configuration should result in the tags not being
@@ -694,7 +697,8 @@
         self.prepare_cloud_registration()
 
         failed = []
-        self.reactor.call_on("registration-failed", lambda: failed.append(True))
+        self.reactor.call_on(
+            "registration-failed", lambda: failed.append(True))
 
         self.log_helper.ignore_errors("Got error while fetching meta-data")
         self.reactor.fire("run")
@@ -724,6 +728,24 @@
                                 account_name=u"onward",
                                 registration_password=u"password")])
 
+    def test_cloud_registration_continues_without_ramdisk(self):
+        """
+        If the instance doesn't have a ramdisk (ie, the query for ramdisk
+        returns a 404), then register-cloud-vm still occurs.
+        """
+        self.log_helper.ignore_errors(HTTPCodeError)
+        self.prepare_query_results(ramdisk_key=HTTPCodeError(404, "ohno"))
+        self.prepare_cloud_registration()
+
+        self.reactor.fire("run")
+        self.exchanger.exchange()
+        self.assertIn("HTTPCodeError: Server returned HTTP code 404",
+                      self.logfile.getvalue())
+        self.assertEquals(len(self.transport.payloads), 1)
+        self.assertMessages(self.transport.payloads[0]["messages"],
+                            [self.get_expected_cloud_message(
+                                ramdisk_key=None)])
+
     def test_fall_back_to_normal_registration_when_metadata_fetch_fails(self):
         """
         If fetching metadata fails, but we do have an account name, then we
@@ -747,7 +769,7 @@
                               "account_name": u"onward",
                               "registration_password": u"password",
                               "hostname": socket.getfqdn(),
-                              "tags": None,}])
+                              "tags": None}])
 
     def test_should_register_in_cloud(self):
         """
@@ -830,8 +852,9 @@
 
     def test_is_managed(self):
         """
-        L{is_cloud_managed} returns True if the EC2 user-data contains Landscape
-        instance information.  It fetches the EC2 data with low timeouts.
+        L{is_cloud_managed} returns True if the EC2 user-data contains
+        Landscape instance information.  It fetches the EC2 data with low
+        timeouts.
         """
         user_data = {"otps": ["otp1"], "exchange-url": "http://exchange",
                      "ping-url": "http://ping"}
@@ -890,15 +913,19 @@
         self.assertFalse(is_cloud_managed(self.fake_fetch))
 
     def test_is_managed_fetch_not_found(self):
+
         def fake_fetch(url, connect_timeout=None):
             raise HTTPCodeError(404, "ohnoes")
+
         self.mock_socket()
         self.mocker.replay()
         self.assertFalse(is_cloud_managed(fake_fetch))
 
     def test_is_managed_fetch_error(self):
+
         def fake_fetch(url, connect_timeout=None):
             raise FetchError(7, "couldn't connect to host")
+
         self.mock_socket()
         self.mocker.replay()
         self.assertFalse(is_cloud_managed(fake_fetch))
@@ -930,6 +957,7 @@
         """
         We'll only wait five minutes for the network to come up.
         """
+
         def fake_fetch(url, connect_timeout=None):
             raise FetchError(7, "couldn't connect to host")
 

=== modified file 'landscape/message_schemas.py'
--- landscape/message_schemas.py	2010-04-26 21:55:00 +0000
+++ landscape/message_schemas.py	2010-05-11 09:40:57 +0000
@@ -153,7 +153,7 @@
      "public_hostname": Unicode(),
      "local_hostname": Unicode(),
      "kernel_key": Unicode(),
-     "ramdisk_key": Unicode(),
+     "ramdisk_key": Any(Unicode(), Constant(None)),
      "launch_index": Int(),
      "image_key": Unicode(),
      "tags": Any(utf8, Constant(None))},