Merge lp:~danilo/charms/trusty/glance-simplestreams-sync/use-upstream-user-agent-support into lp:~landscape/charms/trusty/glance-simplestreams-sync/landscape

[Данило Шеган]

Drop the monkey-patching approach and use the built-in user-agent support in python-simplestreams that's only available in a PPA (ppa:danilo/simplestreams for now, see https://code.launchpad.net/~danilo/simplestreams/custom-user-agent/+merge/292403).

Testing instructions:

1. Deploy OPL from lp:~danilo/landscape/gs3-simplestreams-from-ppa, enable openstack-image-refresh flag and deploy a cloud
2. Note how everything still works, and even user agent is passed on (I override mirror_list to point at a URL I control [run apache on], and watch for user agent strings)

Note: You'll have to deploy the glance-simplestreams-sync with source/key set (see MP above for https://code.launchpad.net/~danilo/landscape/gs3-simplestreams-from-ppa), since it does not reconfigure or apt update when set post installation.

[Geoff Teale (tealeg)]

+1 Approve.

review: Approve

[Bogdana Vereha (bogdana)]

Looks good. +1

review: Approve

[Данило Шеган (danilo)]

Thanks for reviews, merged and published as cs:~landscape-charmers/trusty/glance-simplestreams-sync-6

=== modified file 'scripts/glance-simplestreams-sync.py'
--- scripts/glance-simplestreams-sync.py	2016-04-15 09:13:24 +0000
+++ scripts/glance-simplestreams-sync.py	2016-04-27 13:08:23 +0000
@@ -44,14 +44,12 @@
 
 
 import atexit
-import errno
 import fcntl
 import glanceclient
 from keystoneclient.v2_0 import client as keystone_client
 import keystoneclient.exceptions as keystone_exceptions
 import kombu
 import os
-import simplestreams.contentsource
 from simplestreams.mirrors import glance, UrlMirrorReader
 from simplestreams.objectstores.swift import SwiftObjectStore
 from simplestreams.util import read_signed, path_from_mirror_url
@@ -59,7 +57,6 @@
 import sys
 import time
 import traceback
-from urlparse import urlsplit
 import yaml
 import requests
 import requests.exceptions
@@ -169,117 +166,6 @@
     os.environ['OS_REGION_NAME'] = charm_conf['region']
 
 
-class CustomUrlMirrorReader(UrlMirrorReader):
-    """
-    A custom UrlMirrorReader to pass user agent to simplestreams.
-
-    This monkey-patches a UrlMirrorReader._cs with a custom UrlContentSource
-    allowing passing of a url_reader in, and uses a python-requests-based
-    UrlReader that takes a user_agent parameter.
-
-    XXX Danilo 2016-03-11: we are in the process of moving user-agent support
-    to simplestreams proper.  We are using this approach so we are not blocked
-    until that gets into python-simplestreams package in trusty and xenial
-    at least.
-    """
-    def __init__(self, prefix, mirrors=None,
-                 policy=simplestreams.util.policy_read_signed,
-                 user_agent=None):
-        super(CustomUrlMirrorReader, self).__init__(prefix, mirrors, policy)
-        if user_agent is not None:
-            # Create a custom UrlReader with the user_agent passed in.
-            def url_reader_factory(*args, **kwargs):
-                return CustomRequestsUrlReader(
-                    *args, user_agent=user_agent, **kwargs)
-
-            # Create a custom content source with url_reader passed in.
-            def content_source_factory(path, mirrors):
-                return CustomUrlContentSource(
-                    path, mirrors, url_reader=url_reader_factory)
-
-            self._cs = content_source_factory
-
-
-class CustomUrlContentSource(simplestreams.contentsource.UrlContentSource):
-    """
-    Custom UrlContentSource allowing use of a custom UrlReader.
-
-    XXX Danilo 2016-03-11: This seems to be already supported in more recent
-    python-simplestreams versions (at least in 0.1.0~bzr423-0ubuntu1 in 16.04).
-    """
-    def __init__(self, url, mirrors=None, url_reader=None):
-        super(CustomUrlContentSource, self).__init__(url, mirrors)
-        self.url_reader = url_reader
-
-    # This is a copy of python-simplestreams UrlContentSource._urlinfo()
-    # with the support for custom url_reader as opposed to Urllib/Requests
-    # based url readers.
-    def _urlinfo(self, url):
-        parsed = urlsplit(url)
-        if not parsed.scheme:
-            if url.startswith("/"):
-                url = "file://%s" % url
-            else:
-                url = "file://%s/%s" % (os.getcwd(), url)
-            parsed = urlsplit(url)
-
-        if parsed.scheme == "file":
-
-            def binopen(path, offset=None):
-                f = open(path, "rb")
-                if offset is not None:
-                    f.seek(offset)
-                return f
-
-            return (url, binopen, (parsed.path,))
-        else:
-            return (url, self.url_reader, (url,))
-
-
-class CustomRequestsUrlReader(simplestreams.contentsource.RequestsUrlReader):
-    """
-    A custom UrlReader using requests and passing in a user-agent string.
-
-    It derives and overrides user-agent string and sets it to the value
-    of the "user_agent" attribute.
-    """
-    # This is a verbatim copy of the RequestsUrlReader.__init__() with the
-    # only change being addition of the user_agent argument and passing it
-    # inside the headers with every request.
-    def __init__(self, url, buflen=None, offset=None, user_agent=None):
-        self.url = url
-        (url, user, password) = simplestreams.contentsource.parse_url_auth(url)
-        if user is None:
-            auth = None
-        else:
-            auth = (user, password)
-
-        headers = {}
-        if user_agent is not None:
-            headers['User-Agent'] = user_agent
-        if offset is not None:
-            headers['Range'] = 'bytes=%d-' % offset
-
-        self.req = requests.get(url, stream=True, auth=auth, headers=headers)
-        self.r_iter = None
-        if buflen is None:
-            buflen = simplestreams.contentsource.READ_BUFFER_SIZE
-        self.buflen = buflen
-        self.leftover = bytes()
-        self.consumed = False
-
-        if (self.req.status_code == requests.codes.NOT_FOUND):
-            myerr = IOError("Unable to open %s" % url)
-            myerr.errno = errno.ENOENT
-            raise myerr
-
-        ce = self.req.headers.get('content-encoding', '').lower()
-        if 'gzip' in ce or 'deflate' in ce:
-            self._read = self.read_compressed
-        else:
-            self._read = self.read_raw
-
-
 def do_sync(charm_conf, status_exchange):
 
     user_agent = charm_conf.get("user_agent")
@@ -289,7 +175,7 @@
 
         log.info("configuring sync for url {}".format(mirror_info))
 
-        smirror = CustomUrlMirrorReader(
+        smirror = UrlMirrorReader(
             mirror_url, policy=policy, user_agent=user_agent)
 
         if charm_conf['use_swift']:
@@ -347,8 +233,8 @@
     for endpoint_type in ['publicURL', 'internalURL']:
         catalog[endpoint_type] += "/{}".format(SWIFT_DATA_DIR)
 
-    endpoints = [e._info for e in ksc.endpoints.list()
-                 if e._info['region'] == region]
+    endpoints = [endpoint._info for endpoint in ksc.endpoints.list()
+                 if endpoint._info['region'] == region]
     ps_services = [s for s in services
                    if s['name'] == PRODUCT_STREAMS_SERVICE_NAME]
     if len(ps_services) != 1:
@@ -358,8 +244,8 @@
 
     ps_service_id = ps_services[0]['id']
 
-    ps_endpoints = [e for e in endpoints
-                    if e['service_id'] == ps_service_id]
+    ps_endpoints = [endpoint for endpoint in endpoints
+                    if endpoint['service_id'] == ps_service_id]
 
     if len(ps_endpoints) != 1:
         log.warning("found {} product-streams endpoints in region {},"