← Back to team overview

cloud-init-dev team mailing list archive

[Merge] ~chad.smith/cloud-init:fix/pylxd-3.0 into cloud-init:master

 

Chad Smith has proposed merging ~chad.smith/cloud-init:fix/pylxd-3.0 into cloud-init:master.

Commit message:
tests: fix integration tests to support lxd 3.0 release
    
Integration tests previously had a logic path that was unexercised on
jenkins because we were on an older version of lxc. With an upgrade to lxd
version 3.0 we need to bump pylxd dependency pin and fix a typo in
integration tests which checked the lxd version.



Requested reviews:
  cloud-init commiters (cloud-init-dev)

For more details, see:
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/342617

see commit message
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:fix/pylxd-3.0 into cloud-init:master.
diff --git a/cloudinit/ec2_utils.py b/cloudinit/ec2_utils.py
index dc3f0fc..18553f0 100644
--- a/cloudinit/ec2_utils.py
+++ b/cloudinit/ec2_utils.py
@@ -134,9 +134,9 @@ class MetadataMaterializer(object):
         return joined
 
 
-def _skip_retry_on_codes(status_codes, _request_args, cause):
-    """Returns False if cause.code is in status_codes."""
-    return cause.code not in status_codes
+def _raise_cause_if_not_found(status_codes, _request_args, cause):
+    if cause.code not in SKIP_USERDATA_CODES:
+        raise cause
 
 
 def get_instance_userdata(api_version='latest',
@@ -148,13 +148,9 @@ def get_instance_userdata(api_version='latest',
     try:
         # It is ok for userdata to not exist (thats why we are stopping if
         # NOT_FOUND occurs) and just in that case returning an empty string.
-        exception_cb = functools.partial(_skip_retry_on_codes,
-                                         SKIP_USERDATA_CODES)
-        response = util.read_file_or_url(ud_url,
-                                         ssl_details=ssl_details,
-                                         timeout=timeout,
-                                         retries=retries,
-                                         exception_cb=exception_cb)
+        response = util.read_file_or_url(
+            ud_url, ssl_details=ssl_details, timeout=timeout, retries=retries,
+            exception_cb=_raise_cause_if_not_found)
         user_data = response.contents
     except url_helper.UrlError as e:
         if e.code not in SKIP_USERDATA_CODES:
diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
index 0ee622e..b2244f5 100644
--- a/cloudinit/sources/DataSourceAzure.py
+++ b/cloudinit/sources/DataSourceAzure.py
@@ -452,10 +452,10 @@ class DataSourceAzure(sources.DataSource):
 
         def exc_cb(msg, exception):
             if isinstance(exception, UrlError) and exception.code == 404:
-                return True
+                raise exception
             # If we get an exception while trying to call IMDS, we
             # call DHCP and setup the ephemeral network to acquire the new IP.
-            return False
+            return
 
         need_report = report_ready
         while True:
diff --git a/cloudinit/sources/DataSourceScaleway.py b/cloudinit/sources/DataSourceScaleway.py
index e2502b0..a89e125 100644
--- a/cloudinit/sources/DataSourceScaleway.py
+++ b/cloudinit/sources/DataSourceScaleway.py
@@ -103,21 +103,25 @@ def query_data_api_once(api_address, timeout, requests_session):
     ports below 1024). If requests raises ConnectionError (EADDRINUSE), the
     caller should retry to call this function on an other port.
     """
+    def _exc_cb(msg, exception):
+        """raise the provided exception if it is not 404."""
+        if (exception.code == 404 or
+                isinstance(exception.cause,
+                           requests.exceptions.ConnectionError)):
+            raise exception
+
     try:
         resp = url_helper.readurl(
             api_address,
             data=None,
             timeout=timeout,
-            # It's the caller's responsability to recall this function in case
+            # It's the caller's responsibility to recall this function in case
             # of exception. Don't let url_helper.readurl() retry by itself.
             retries=0,
             session=requests_session,
             # If the error is a HTTP/404 or a ConnectionError, go into raise
             # block below and don't bother retrying.
-            exception_cb=lambda _, exc: exc.code != 404 and (
-                not isinstance(exc.cause, requests.exceptions.ConnectionError)
-            )
-        )
+            exception_cb=_exc_cb)
         return util.decode_binary(resp.contents)
     except url_helper.UrlError as exc:
         # Empty user data.
diff --git a/cloudinit/sources/helpers/openstack.py b/cloudinit/sources/helpers/openstack.py
index 26f3168..b29a00e 100644
--- a/cloudinit/sources/helpers/openstack.py
+++ b/cloudinit/sources/helpers/openstack.py
@@ -456,17 +456,15 @@ class MetadataReader(BaseReader):
             try:
                 code = int(cause.code)
                 if code >= 400:
-                    return False
+                    return
             except (TypeError, ValueError):
                 # Older versions of requests didn't have a code.
                 pass
-            return True
+            raise cause
 
-        response = url_helper.readurl(path,
-                                      retries=self.retries,
-                                      ssl_details=self.ssl_details,
-                                      timeout=self.timeout,
-                                      exception_cb=should_retry_cb)
+        response = url_helper.readurl(
+            path, retries=self.retries, ssl_details=self.ssl_details,
+            timeout=self.timeout, exception_cb=should_retry_cb)
         if decode:
             return response.contents.decode()
         else:
diff --git a/cloudinit/url_helper.py b/cloudinit/url_helper.py
index 03a573a..bbd6834 100644
--- a/cloudinit/url_helper.py
+++ b/cloudinit/url_helper.py
@@ -260,11 +260,12 @@ def readurl(url, data=None, timeout=None, retries=0, sec_between=1,
                     # ssl exceptions are not going to get fixed by waiting a
                     # few seconds
                     break
-            if exception_cb and not exception_cb(req_args.copy(), excps[-1]):
-                # if an exception callback was given, it should return True
-                # to continue retrying and False to break and re-raise the
-                # exception
-                break
+            if exception_cb:
+                import pdb; pdb.set_trace()
+                # Any exceptions that an exception_cb raises (including
+                # re-raising this one) are not caught here.
+                exception_cb(req_args.copy(), excps[-1])
+
             if (infinite and sec_between > 0) or \
                (i + 1 < manual_tries and sec_between > 0):
                 LOG.debug("Please wait %s seconds while we wait to try again",
@@ -290,6 +291,8 @@ def wait_for_url(urls, max_wait=None, timeout=None,
                 for request.
     exception_cb: call method with 2 arguments 'msg' (per status_cb) and
                   'exception', the exception that occurred.
+                  if this method raises exception, then that will not be
+                  caught.
     sleep_time_cb: call method with 2 arguments (response, loop_n) that
                    generates the next sleep time.
 
@@ -375,9 +378,6 @@ def wait_for_url(urls, max_wait=None, timeout=None,
                                                               reason)
             status_cb(status_msg)
             if exception_cb:
-                # This can be used to alter the headers that will be sent
-                # in the future, for example this is what the MAAS datasource
-                # does.
                 exception_cb(msg=status_msg, exception=url_exc)
 
         if timeup(max_wait, start_time):
@@ -487,7 +487,7 @@ class OauthUrlHelper(object):
         ret = None
         try:
             if extra_exception_cb:
-                ret = extra_exception_cb(msg, exception)
+                extra_exception_cb(msg, exception)
         finally:
                 self.exception_cb(msg, exception)
         return ret
diff --git a/integration-requirements.txt b/integration-requirements.txt
index 45baac6..382538d 100644
--- a/integration-requirements.txt
+++ b/integration-requirements.txt
@@ -13,7 +13,7 @@ paramiko==2.4.0
 
 # lxd backend
 # 01/10/2018: enables use of lxd as snap support
-git+https://github.com/lxc/pylxd.git@0722955260a6557e6d2ffde1896bfe0707bbca27
+git+https://github.com/lxc/pylxd.git@1a85a12a23401de6e96b1aeaf59ecbff2e88f49d
 
 
 # finds latest image information
diff --git a/tests/cloud_tests/platforms/lxd/instance.py b/tests/cloud_tests/platforms/lxd/instance.py
index 0488da5..0d957bc 100644
--- a/tests/cloud_tests/platforms/lxd/instance.py
+++ b/tests/cloud_tests/platforms/lxd/instance.py
@@ -210,7 +210,7 @@ def _has_proper_console_support():
         reason = "LXD server does not support console api extension"
     else:
         dver = info.get('environment', {}).get('driver_version', "")
-        if dver.startswith("2.") or dver.startwith("1."):
+        if dver.startswith("2.") or dver.startswith("1."):
             reason = "LXD Driver version not 3.x+ (%s)" % dver
         else:
             try: