Browse Source

Log exceptions when checking status

This change adds logging whenever a request has a status code of >= 300.
The check_status method will now create an SHA1 ID from the request URL
and log the status, reason, text, and headers (in debug) whenever a
request has a return code of >= 300. This will allow us to better debug
image processing issues, even when multiprocessing requests. Deployers
will now have the ability to track down issues to specific images and
endpoints.

Session creation was moved into a single object class to ensure we're
managing sessions in a uniform way.

Change-Id: If4e84a0273e295267248559c63ae994a5a826004
Signed-off-by: Kevin Carter <kecarter@redhat.com>
(cherry picked from commit e06cf4482d)
tags/10.8.1
Kevin Carter 1 month ago
parent
commit
4e6995dc44

+ 40
- 22
tripleo_common/image/image_export.py View File

@@ -17,6 +17,7 @@ import collections
17 17
 import hashlib
18 18
 import json
19 19
 import os
20
+import requests
20 21
 import shutil
21 22
 
22 23
 from oslo_log import log as logging
@@ -54,7 +55,7 @@ def image_tag_from_url(image_url):
54 55
 
55 56
 
56 57
 def export_stream(target_url, layer, layer_stream, verify_digest=True):
57
-    image, tag = image_tag_from_url(target_url)
58
+    image, _ = image_tag_from_url(target_url)
58 59
     digest = layer['digest']
59 60
     blob_dir_path = os.path.join(IMAGE_EXPORT_DIR, 'v2', image, 'blobs')
60 61
     make_dir(blob_dir_path)
@@ -64,37 +65,54 @@ def export_stream(target_url, layer, layer_stream, verify_digest=True):
64 65
 
65 66
     length = 0
66 67
     calc_digest = hashlib.sha256()
68
+
67 69
     try:
68
-        with open(blob_path, 'w+b') as f:
70
+        with open(blob_path, 'wb') as f:
69 71
             for chunk in layer_stream:
70 72
                 if not chunk:
71 73
                     break
72 74
                 f.write(chunk)
73 75
                 calc_digest.update(chunk)
74 76
                 length += len(chunk)
75
-
76
-        layer_digest = 'sha256:%s' % calc_digest.hexdigest()
77
-        LOG.debug('Calculated layer digest: %s' % layer_digest)
78
-
79
-        if verify_digest:
80
-            if digest != layer_digest:
81
-                raise IOError('Expected digest %s '
82
-                              'does not match calculated %s' %
83
-                              (digest, layer_digest))
84
-        else:
85
-            # if the original layer is uncompressed
86
-            # the digest may change on export
87
-            expected_blob_path = os.path.join(
88
-                blob_dir_path, '%s.gz' % layer_digest)
89
-            if blob_path != expected_blob_path:
90
-                os.rename(blob_path, expected_blob_path)
91
-
92 77
     except Exception as e:
93
-        LOG.error('Error while writing blob %s' % blob_path)
94
-        # cleanup blob file
78
+        write_error = 'Write Failure: {}'.format(str(e))
79
+        LOG.error(write_error)
95 80
         if os.path.isfile(blob_path):
96 81
             os.remove(blob_path)
97
-        raise e
82
+            LOG.error('Broken layer found and removed: %s' % blob_path)
83
+        raise IOError(write_error)
84
+    else:
85
+        LOG.info('Layer written successfully: %s' % blob_path)
86
+
87
+    layer_digest = 'sha256:%s' % calc_digest.hexdigest()
88
+    LOG.debug('Provided layer digest: %s' % digest)
89
+    LOG.debug('Calculated layer digest: %s' % layer_digest)
90
+
91
+    if verify_digest:
92
+        if digest != layer_digest:
93
+            hash_request_id = hashlib.sha1(str(target_url.geturl()).encode())
94
+            error_msg = (
95
+                'Image ID: %s, Expected digest "%s" does not match'
96
+                ' calculated digest "%s", Blob path "%s". Blob'
97
+                ' path will be cleaned up.' % (
98
+                    hash_request_id.hexdigest(),
99
+                    digest,
100
+                    layer_digest,
101
+                    blob_path
102
+                )
103
+            )
104
+            LOG.error(error_msg)
105
+            if os.path.isfile(blob_path):
106
+                os.remove(blob_path)
107
+            raise requests.exceptions.HTTPError(error_msg)
108
+    else:
109
+        # if the original layer is uncompressed
110
+        # the digest may change on export
111
+        expected_blob_path = os.path.join(
112
+            blob_dir_path, '%s.gz' % layer_digest
113
+        )
114
+        if blob_path != expected_blob_path:
115
+            os.rename(blob_path, expected_blob_path)
98 116
 
99 117
     layer['digest'] = layer_digest
100 118
     layer['size'] = length

+ 211
- 70
tripleo_common/image/image_uploader.py View File

@@ -21,6 +21,7 @@ import netifaces
21 21
 import os
22 22
 import re
23 23
 import requests
24
+from requests.adapters import HTTPAdapter
24 25
 from requests import auth as requests_auth
25 26
 import shutil
26 27
 import six
@@ -100,6 +101,38 @@ def get_undercloud_registry():
100 101
     return '%s:%s' % (addr, '8787')
101 102
 
102 103
 
104
+class MakeSession(object):
105
+    """Class method to uniformly create sessions.
106
+
107
+    Sessions created by this class will retry on errors with an exponential
108
+    backoff before raising an exception. Because our primary interaction is
109
+    with the container registries the adapter will also retry on 401 and
110
+    404. This is being done because registries commonly return 401 when an
111
+    image is not found, which is commonly a cache miss. See the adapter
112
+    definitions for more on retry details.
113
+    """
114
+    def __init__(self, verify=True):
115
+        self.session = requests.Session()
116
+        self.session.verify = verify
117
+        adapter = HTTPAdapter(
118
+            max_retries=8,
119
+            pool_connections=24,
120
+            pool_maxsize=24,
121
+            pool_block=False
122
+        )
123
+        self.session.mount('http://', adapter)
124
+        self.session.mount('https://', adapter)
125
+
126
+    def create(self):
127
+        return self.__enter__()
128
+
129
+    def __enter__(self):
130
+        return self.session
131
+
132
+    def __exit__(self, *args, **kwargs):
133
+        self.session.close()
134
+
135
+
103 136
 class ImageUploadManager(BaseImageManager):
104 137
     """Manage the uploading of image files
105 138
 
@@ -121,13 +154,15 @@ class ImageUploadManager(BaseImageManager):
121 154
         self.cleanup = cleanup
122 155
         if mirrors:
123 156
             for uploader in self.uploaders.values():
124
-                uploader.mirrors.update(mirrors)
157
+                if hasattr(uploader, 'mirrors'):
158
+                    uploader.mirrors.update(mirrors)
125 159
         if registry_credentials:
126 160
             self.validate_registry_credentials(registry_credentials)
127 161
             for uploader in self.uploaders.values():
128 162
                 uploader.registry_credentials = registry_credentials
129 163
 
130
-    def validate_registry_credentials(self, creds_data):
164
+    @staticmethod
165
+    def validate_registry_credentials(creds_data):
131 166
         if not isinstance(creds_data, dict):
132 167
             raise TypeError('Credentials data must be a dict')
133 168
         for registry, cred_entry in creds_data.items():
@@ -158,7 +193,8 @@ class ImageUploadManager(BaseImageManager):
158 193
     def get_uploader(self, uploader):
159 194
         return self.uploader(uploader)
160 195
 
161
-    def get_push_destination(self, item):
196
+    @staticmethod
197
+    def get_push_destination(item):
162 198
         push_destination = item.get('push_destination')
163 199
         if not push_destination:
164 200
             return get_undercloud_registry()
@@ -247,15 +283,15 @@ class BaseImageUploader(object):
247 283
     def run_modify_playbook(cls, modify_role, modify_vars,
248 284
                             source_image, target_image, append_tag,
249 285
                             container_build_tool='buildah'):
250
-        vars = {}
286
+        run_vars = {}
251 287
         if modify_vars:
252
-            vars.update(modify_vars)
253
-        vars['source_image'] = source_image
254
-        vars['target_image'] = target_image
255
-        vars['modified_append_tag'] = append_tag
256
-        vars['container_build_tool'] = container_build_tool
288
+            run_vars.update(modify_vars)
289
+        run_vars['source_image'] = source_image
290
+        run_vars['target_image'] = target_image
291
+        run_vars['modified_append_tag'] = append_tag
292
+        run_vars['container_build_tool'] = container_build_tool
257 293
         LOG.info('Playbook variables: \n%s' % yaml.safe_dump(
258
-            vars, default_flow_style=False))
294
+            run_vars, default_flow_style=False))
259 295
         playbook = [{
260 296
             'hosts': 'localhost',
261 297
             'tasks': [{
@@ -263,7 +299,7 @@ class BaseImageUploader(object):
263 299
                 'import_role': {
264 300
                     'name': modify_role
265 301
                 },
266
-                'vars': vars
302
+                'vars': run_vars
267 303
             }]
268 304
         }]
269 305
         LOG.info('Playbook: \n%s' % yaml.safe_dump(
@@ -341,11 +377,15 @@ class BaseImageUploader(object):
341 377
                      session=None):
342 378
         netloc = image_url.netloc
343 379
         image, tag = self._image_tag_from_url(image_url)
344
-        self.is_insecure_registry(netloc)
380
+        self.is_insecure_registry(registry_host=netloc)
345 381
         url = self._build_url(image_url, path='/')
382
+        verify = (netloc not in self.no_verify_registries)
383
+        if not session:
384
+            session = MakeSession(verify=verify).create()
385
+        else:
386
+            session.headers.pop('Authorization', None)
387
+            session.verify = verify
346 388
 
347
-        session = requests.Session()
348
-        session.verify = (netloc not in self.no_verify_registries)
349 389
         r = session.get(url, timeout=30)
350 390
         LOG.debug('%s status code %s' % (url, r.status_code))
351 391
         if r.status_code == 200:
@@ -367,12 +407,20 @@ class BaseImageUploader(object):
367 407
             token_param['service'] = re.search(
368 408
                 'service="(.*?)"', www_auth).group(1)
369 409
         token_param['scope'] = 'repository:%s:pull' % image[1:]
410
+
370 411
         auth = None
371 412
         if username:
372 413
             auth = requests_auth.HTTPBasicAuth(username, password)
414
+        LOG.debug('Token parameters: params {}'.format(token_param))
373 415
         rauth = session.get(realm, params=token_param, auth=auth, timeout=30)
374 416
         rauth.raise_for_status()
375 417
         session.headers['Authorization'] = 'Bearer %s' % rauth.json()['token']
418
+        hash_request_id = hashlib.sha1(str(rauth.url).encode())
419
+        LOG.info(
420
+            'Session authenticated: id {}'.format(
421
+                hash_request_id.hexdigest()
422
+            )
423
+        )
376 424
         setattr(session, 'reauthenticate', self.authenticate)
377 425
         setattr(
378 426
             session,
@@ -387,14 +435,93 @@ class BaseImageUploader(object):
387 435
         return session
388 436
 
389 437
     @staticmethod
390
-    def check_status(session, request):
391
-        if hasattr(session, 'reauthenticate'):
392
-            if request.status_code == 401:
393
-                session.reauthenticate(**session.auth_args)
394
-                if hasattr(request, 'text'):
395
-                    raise requests.exceptions.HTTPError(request.text)
396
-                else:
397
-                    raise SystemError()
438
+    def _get_response_text(response, encoding='utf-8', force_encoding=False):
439
+        """Return request response text
440
+
441
+        We need to set the encoding for the response other wise it
442
+        will attempt to detect the encoding which is very time consuming.
443
+        See https://github.com/psf/requests/issues/4235 for additional
444
+        context.
445
+
446
+        :param: response: requests Respoinse object
447
+        :param: encoding: encoding to set if not currently set
448
+        :param: force_encoding: set response encoding always
449
+        """
450
+
451
+        if force_encoding or not response.encoding:
452
+            response.encoding = encoding
453
+        return response.text
454
+
455
+    @staticmethod
456
+    def check_status(session, request, allow_reauth=True):
457
+        hash_request_id = hashlib.sha1(str(request.url).encode())
458
+        request_id = hash_request_id.hexdigest()
459
+        text = getattr(request, 'text', 'unknown')
460
+        reason = getattr(request, 'reason', 'unknown')
461
+        status_code = getattr(request, 'status_code', None)
462
+        headers = getattr(request, 'headers', {})
463
+        session_headers = getattr(session, 'headers', {})
464
+
465
+        if status_code >= 300:
466
+            LOG.info(
467
+                'Non-2xx: id {}, status {}, reason {}, text {}'.format(
468
+                    request_id,
469
+                    status_code,
470
+                    reason,
471
+                    text
472
+                )
473
+            )
474
+
475
+        if status_code == 401:
476
+            LOG.warning(
477
+                'Failure: id {}, status {}, reason {} text {}'.format(
478
+                    request_id,
479
+                    status_code,
480
+                    reason,
481
+                    text
482
+                )
483
+            )
484
+            LOG.debug(
485
+                'Request headers after 401: id {}, headers {}'.format(
486
+                    request_id,
487
+                    headers
488
+                )
489
+            )
490
+            LOG.debug(
491
+                'Session headers after 401: id {}, headers {}'.format(
492
+                    request_id,
493
+                    session_headers
494
+                )
495
+            )
496
+
497
+            www_auth = headers.get(
498
+                'www-authenticate',
499
+                headers.get(
500
+                    'Www-Authenticate'
501
+                )
502
+            )
503
+            if www_auth:
504
+                error = None
505
+                if 'error=' in www_auth:
506
+                    error = re.search('error="(.*?)"', www_auth).group(1)
507
+                    LOG.warning(
508
+                        'Error detected in auth headers: error {}'.format(
509
+                            error
510
+                        )
511
+                    )
512
+                if error == 'invalid_token' and allow_reauth:
513
+                    if hasattr(session, 'reauthenticate'):
514
+                        reauth = int(session.headers.get('_TripleOReAuth', 0))
515
+                        reauth += 1
516
+                        session.headers['_TripleOReAuth'] = str(reauth)
517
+                        LOG.warning(
518
+                            'Re-authenticating: id {}, count {}'.format(
519
+                                request_id,
520
+                                reauth
521
+                            )
522
+                        )
523
+                        session.reauthenticate(**session.auth_args)
524
+
398 525
         request.raise_for_status()
399 526
 
400 527
     @classmethod
@@ -404,10 +531,10 @@ class BaseImageUploader(object):
404 531
             mirror = cls.mirrors[netloc]
405 532
             return '%sv2%s' % (mirror, path)
406 533
         else:
407
-            if netloc in cls.insecure_registries:
408
-                scheme = 'http'
409
-            else:
534
+            if not cls.is_insecure_registry(registry_host=netloc):
410 535
                 scheme = 'https'
536
+            else:
537
+                scheme = 'http'
411 538
             if netloc == 'docker.io':
412 539
                 netloc = 'registry-1.docker.io'
413 540
             return '%s://%s/v2%s' % (scheme, netloc, path)
@@ -457,7 +584,7 @@ class BaseImageUploader(object):
457 584
         tags_r = tags_f.result()
458 585
         cls.check_status(session=session, request=tags_r)
459 586
 
460
-        manifest_str = manifest_r.text
587
+        manifest_str = cls._get_response_text(manifest_r)
461 588
 
462 589
         if 'Docker-Content-Digest' in manifest_r.headers:
463 590
             digest = manifest_r.headers['Docker-Content-Digest']
@@ -512,7 +639,7 @@ class BaseImageUploader(object):
512 639
         }
513 640
 
514 641
     def list(self, registry, session=None):
515
-        self.is_insecure_registry(registry)
642
+        self.is_insecure_registry(registry_host=registry)
516 643
         url = self._image_to_url(registry)
517 644
         catalog_url = self._build_url(
518 645
             url, CALL_CATALOG
@@ -525,7 +652,7 @@ class BaseImageUploader(object):
525 652
         else:
526 653
             raise ImageUploaderException(
527 654
                 'Image registry made invalid response: %s' %
528
-                (catalog_resp.status_code)
655
+                catalog_resp.status_code
529 656
             )
530 657
 
531 658
         tags_get_args = []
@@ -589,7 +716,7 @@ class BaseImageUploader(object):
589 716
                                    fallback_tag=None):
590 717
         labels = i.get('Labels', {})
591 718
 
592
-        if(hasattr(labels, 'keys')):
719
+        if hasattr(labels, 'keys'):
593 720
             label_keys = ', '.join(labels.keys())
594 721
         else:
595 722
             label_keys = ""
@@ -614,7 +741,7 @@ class BaseImageUploader(object):
614 741
                     )
615 742
         else:
616 743
             tag_label = None
617
-            if(isinstance(labels, dict)):
744
+            if isinstance(labels, dict):
618 745
                 tag_label = labels.get(tag_from_label)
619 746
             if tag_label is None:
620 747
                 if fallback_tag:
@@ -639,7 +766,7 @@ class BaseImageUploader(object):
639 766
 
640 767
         # prime self.insecure_registries by testing every image
641 768
         for url in image_urls:
642
-            self.is_insecure_registry(url)
769
+            self.is_insecure_registry(registry_host=url)
643 770
 
644 771
         discover_args = []
645 772
         for image in images:
@@ -655,7 +782,7 @@ class BaseImageUploader(object):
655 782
     def discover_image_tag(self, image, tag_from_label=None,
656 783
                            fallback_tag=None, username=None, password=None):
657 784
         image_url = self._image_to_url(image)
658
-        self.is_insecure_registry(image_url.netloc)
785
+        self.is_insecure_registry(registry_host=image_url.netloc)
659 786
         session = self.authenticate(
660 787
             image_url, username=username, password=password)
661 788
 
@@ -668,7 +795,7 @@ class BaseImageUploader(object):
668 795
         images_with_labels = []
669 796
         for image in images:
670 797
             url = self._image_to_url(image)
671
-            self.is_insecure_registry(url.netloc)
798
+            self.is_insecure_registry(registry_host=url.netloc)
672 799
             session = self.authenticate(
673 800
                 url, username=username, password=password)
674 801
             image_labels = self._image_labels(
@@ -682,19 +809,23 @@ class BaseImageUploader(object):
682 809
         # prime insecure_registries
683 810
         if task.pull_source:
684 811
             self.is_insecure_registry(
685
-                self._image_to_url(task.pull_source).netloc)
812
+                registry_host=self._image_to_url(task.pull_source).netloc
813
+            )
686 814
         else:
687 815
             self.is_insecure_registry(
688
-                self._image_to_url(task.image_name).netloc)
816
+                registry_host=self._image_to_url(task.image_name).netloc
817
+            )
689 818
         self.is_insecure_registry(
690
-            self._image_to_url(task.push_destination).netloc)
819
+            registry_host=self._image_to_url(task.push_destination).netloc
820
+        )
691 821
         self.upload_tasks.append((self, task))
692 822
 
693
-    def is_insecure_registry(self, registry_host):
694
-        if registry_host in self.secure_registries:
823
+    @classmethod
824
+    def is_insecure_registry(cls, registry_host):
825
+        if registry_host in cls.secure_registries:
695 826
             return False
696
-        if (registry_host in self.insecure_registries or
697
-                registry_host in self.no_verify_registries):
827
+        if (registry_host in cls.insecure_registries or
828
+                registry_host in cls.no_verify_registries):
698 829
             return True
699 830
         with requests.Session() as s:
700 831
             try:
@@ -705,7 +836,7 @@ class BaseImageUploader(object):
705 836
                 try:
706 837
                     s.get('https://%s/v2' % registry_host, timeout=30,
707 838
                           verify=False)
708
-                    self.no_verify_registries.add(registry_host)
839
+                    cls.no_verify_registries.add(registry_host)
709 840
                     # Techinically these type of registries are insecure when
710 841
                     # the container engine tries to do a pull. The python
711 842
                     # uploader ignores the certificate problem, but they are
@@ -714,14 +845,14 @@ class BaseImageUploader(object):
714 845
                     return True
715 846
                 except requests.exceptions.SSLError:
716 847
                     # So nope, it's really not a certificate verification issue
717
-                    self.insecure_registries.add(registry_host)
848
+                    cls.insecure_registries.add(registry_host)
718 849
                     return True
719 850
             except Exception:
720 851
                 # for any other error assume it is a secure registry, because:
721 852
                 # - it is secure registry
722 853
                 # - the host is not accessible
723 854
                 pass
724
-        self.secure_registries.add(registry_host)
855
+        cls.secure_registries.add(registry_host)
725 856
         return False
726 857
 
727 858
     @classmethod
@@ -919,14 +1050,11 @@ class SkopeoImageUploader(BaseImageUploader):
919 1050
 
920 1051
         # Pull a single image first, to avoid duplicate pulls of the
921 1052
         # same base layers
922
-        uploader, first_task = self.upload_tasks.pop()
923
-        result = uploader.upload_image(first_task)
924
-        local_images.extend(result)
1053
+        local_images.extend(upload_task(args=self.upload_tasks.pop()))
925 1054
 
926 1055
         # workers will be half the CPU count, to a minimum of 2
927
-        workers = max(2, processutils.get_worker_count() // 2)
1056
+        workers = max(2, (processutils.get_worker_count() - 1))
928 1057
         p = futures.ThreadPoolExecutor(max_workers=workers)
929
-
930 1058
         for result in p.map(upload_task, self.upload_tasks):
931 1059
             local_images.extend(result)
932 1060
         LOG.info('result %s' % local_images)
@@ -1004,7 +1132,11 @@ class PythonImageUploader(BaseImageUploader):
1004 1132
         if not t.modify_role:
1005 1133
             LOG.warning('Completed upload for image %s' % t.image_name)
1006 1134
         else:
1007
-            # Copy ummodified from target to local
1135
+            LOG.info(
1136
+                'Copy ummodified imagename: "{}" from target to local'.format(
1137
+                    t.image_name
1138
+                )
1139
+            )
1008 1140
             self._copy_registry_to_local(t.target_image_source_tag_url)
1009 1141
 
1010 1142
             if t.cleanup in (CLEANUP_FULL, CLEANUP_PARTIAL):
@@ -1056,7 +1188,7 @@ class PythonImageUploader(BaseImageUploader):
1056 1188
             return False
1057 1189
 
1058 1190
         # detect if the registry is push-capable by requesting an upload URL.
1059
-        image, tag = cls._image_tag_from_url(image_url)
1191
+        image, _ = cls._image_tag_from_url(image_url)
1060 1192
         upload_req_url = cls._build_url(
1061 1193
             image_url,
1062 1194
             path=CALL_UPLOAD % {'image': image})
@@ -1091,7 +1223,7 @@ class PythonImageUploader(BaseImageUploader):
1091 1223
         if r.status_code in (403, 404):
1092 1224
             raise ImageNotFoundException('Not found image: %s' % url)
1093 1225
         cls.check_status(session=session, request=r)
1094
-        return r.text
1226
+        return cls._get_response_text(r)
1095 1227
 
1096 1228
     @classmethod
1097 1229
     @tenacity.retry(  # Retry up to 5 times with jittered exponential backoff
@@ -1137,6 +1269,8 @@ class PythonImageUploader(BaseImageUploader):
1137 1269
         chunk_size = 2 ** 20
1138 1270
         with session.get(
1139 1271
                 source_blob_url, stream=True, timeout=30) as blob_req:
1272
+            # TODO(aschultz): unsure if necessary or if only when using .text
1273
+            blob_req.encoding = 'utf-8'
1140 1274
             cls.check_status(session=session, request=blob_req)
1141 1275
             for data in blob_req.iter_content(chunk_size):
1142 1276
                 if not data:
@@ -1313,7 +1447,7 @@ class PythonImageUploader(BaseImageUploader):
1313 1447
             }
1314 1448
         )
1315 1449
         if r.status_code == 400:
1316
-            LOG.error(r.text)
1450
+            LOG.error(cls._get_response_text(r))
1317 1451
             raise ImageUploaderException('Pushing manifest failed')
1318 1452
         cls.check_status(session=target_session, request=r)
1319 1453
 
@@ -1326,25 +1460,34 @@ class PythonImageUploader(BaseImageUploader):
1326 1460
     def _copy_registry_to_local(cls, source_url):
1327 1461
         cls._assert_scheme(source_url, 'docker')
1328 1462
         pull_source = source_url.netloc + source_url.path
1329
-        LOG.info('Pulling %s' % pull_source)
1330
-        cmd = ['buildah', 'pull']
1463
+        cmd = ['buildah', '--debug', 'pull']
1331 1464
 
1332 1465
         if source_url.netloc in [cls.insecure_registries,
1333 1466
                                  cls.no_verify_registries]:
1334 1467
             cmd.append('--tls-verify=false')
1335 1468
 
1336 1469
         cmd.append(pull_source)
1470
+        LOG.info('Pulling %s' % pull_source)
1337 1471
         LOG.info('Running %s' % ' '.join(cmd))
1338
-        env = os.environ.copy()
1339
-        process = subprocess.Popen(cmd, env=env, stdout=subprocess.PIPE,
1340
-                                   universal_newlines=True)
1341
-
1472
+        process = subprocess.Popen(
1473
+            cmd,
1474
+            stdout=subprocess.PIPE,
1475
+            stderr=subprocess.PIPE,
1476
+            universal_newlines=True,
1477
+            close_fds=True
1478
+        )
1342 1479
         out, err = process.communicate()
1343
-        LOG.info(out)
1344 1480
         if process.returncode != 0:
1345
-            LOG.error('Error pulling image:\n%s\n%s' %
1346
-                      (' '.join(cmd), err))
1347
-            raise ImageUploaderException('Pulling image failed')
1481
+            error_msg = (
1482
+                'Pulling image failed: cmd "{}", stdout "{}",'
1483
+                ' stderr "{}"'.format(
1484
+                    ' '.join(cmd),
1485
+                    out,
1486
+                    err
1487
+                )
1488
+            )
1489
+            LOG.error(error_msg)
1490
+            raise ImageUploaderException(error_msg)
1348 1491
         return out
1349 1492
 
1350 1493
     @classmethod
@@ -1604,7 +1747,7 @@ class PythonImageUploader(BaseImageUploader):
1604 1747
         config = json.loads(config_str)
1605 1748
 
1606 1749
         layers = [l['digest'] for l in manifest['layers']]
1607
-        i, tag = cls._image_tag_from_url(image_url)
1750
+        i, _ = cls._image_tag_from_url(image_url)
1608 1751
         digest = image['digest']
1609 1752
         created = image['created']
1610 1753
         labels = config['config']['Labels']
@@ -1668,14 +1811,11 @@ class PythonImageUploader(BaseImageUploader):
1668 1811
 
1669 1812
         # Pull a single image first, to avoid duplicate pulls of the
1670 1813
         # same base layers
1671
-        uploader, first_task = self.upload_tasks.pop()
1672
-        result = uploader.upload_image(first_task)
1673
-        local_images.extend(result)
1814
+        local_images.extend(upload_task(args=self.upload_tasks.pop()))
1674 1815
 
1675
-        # workers will be half the CPU count, to a minimum of 2
1676
-        workers = max(2, processutils.get_worker_count() // 2)
1816
+        # workers will the CPU count minus 1, with a minimum of 2
1817
+        workers = max(2, (processutils.get_worker_count() - 1))
1677 1818
         p = futures.ThreadPoolExecutor(max_workers=workers)
1678
-
1679 1819
         for result in p.map(upload_task, self.upload_tasks):
1680 1820
             local_images.extend(result)
1681 1821
         LOG.info('result %s' % local_images)
@@ -1721,7 +1861,8 @@ class UploadTask(object):
1721 1861
         self.source_image_url = image_to_url(self.source_image)
1722 1862
         self.target_image_url = image_to_url(self.target_image)
1723 1863
         self.target_image_source_tag_url = image_to_url(
1724
-            self.target_image_source_tag)
1864
+            self.target_image_source_tag
1865
+        )
1725 1866
 
1726 1867
 
1727 1868
 def upload_task(args):

+ 3
- 1
tripleo_common/tests/image/test_image_export.py View File

@@ -17,6 +17,7 @@ import hashlib
17 17
 import io
18 18
 import json
19 19
 import os
20
+import requests
20 21
 import shutil
21 22
 import six
22 23
 from six.moves.urllib.parse import urlparse
@@ -116,7 +117,8 @@ class TestImageExport(base.TestCase):
116 117
         }
117 118
         calc_digest = hashlib.sha256()
118 119
         layer_stream = io.BytesIO(blob_compressed)
119
-        self.assertRaises(IOError, image_export.export_stream,
120
+        self.assertRaises(requests.exceptions.HTTPError,
121
+                          image_export.export_stream,
120 122
                           target_url, layer, layer_stream,
121 123
                           verify_digest=True)
122 124
         blob_dir = os.path.join(image_export.IMAGE_EXPORT_DIR,

+ 34
- 14
tripleo_common/tests/image/test_image_uploader.py View File

@@ -53,6 +53,8 @@ class TestImageUploadManager(base.TestCase):
53 53
         files.append('testfile')
54 54
         self.filelist = files
55 55
 
56
+    @mock.patch('tripleo_common.image.image_uploader.'
57
+                'BaseImageUploader.check_status')
56 58
     @mock.patch('tripleo_common.image.image_uploader.'
57 59
                 'PythonImageUploader._fetch_manifest')
58 60
     @mock.patch('tripleo_common.image.image_uploader.'
@@ -75,7 +77,8 @@ class TestImageUploadManager(base.TestCase):
75 77
                 'get_undercloud_registry', return_value='192.0.2.0:8787')
76 78
     def test_file_parsing(self, mock_gur, mockioctl, mockpath,
77 79
                           mock_images_match, mock_is_insecure, mock_inspect,
78
-                          mock_auth, mock_copy, mock_manifest):
80
+                          mock_auth, mock_copy, mock_manifest,
81
+                          check_status):
79 82
 
80 83
         mock_manifest.return_value = '{"layers": []}'
81 84
         mock_inspect.return_value = {}
@@ -527,7 +530,7 @@ class TestBaseImageUploader(base.TestCase):
527 530
         insecure_reg.add('registry-1.docker.io')
528 531
         secure_reg.add('192.0.2.1:8787')
529 532
         self.assertEqual(
530
-            'http://registry-1.docker.io/v2/t/nova-api/manifests/latest',
533
+            'https://registry-1.docker.io/v2/t/nova-api/manifests/latest',
531 534
             build(url2, '/t/nova-api/manifests/latest')
532 535
         )
533 536
         self.assertEqual(
@@ -870,6 +873,8 @@ class TestSkopeoImageUploader(base.TestCase):
870 873
         self.uploader._copy.retry.sleep = mock.Mock()
871 874
         self.uploader._inspect.retry.sleep = mock.Mock()
872 875
 
876
+    @mock.patch('tripleo_common.image.image_uploader.'
877
+                'BaseImageUploader.check_status')
873 878
     @mock.patch('os.environ')
874 879
     @mock.patch('subprocess.Popen')
875 880
     @mock.patch('tripleo_common.image.image_uploader.'
@@ -877,7 +882,7 @@ class TestSkopeoImageUploader(base.TestCase):
877 882
     @mock.patch('tripleo_common.image.image_uploader.'
878 883
                 'BaseImageUploader.authenticate')
879 884
     def test_upload_image(self, mock_auth, mock_inspect,
880
-                          mock_popen, mock_environ):
885
+                          mock_popen, mock_environ, check_status):
881 886
         mock_process = mock.Mock()
882 887
         mock_process.communicate.return_value = ('copy complete', '')
883 888
         mock_process.returncode = 0
@@ -1145,10 +1150,11 @@ class TestPythonImageUploader(base.TestCase):
1145 1150
         u._copy_layer_local_to_registry.retry.sleep = mock.Mock()
1146 1151
         u._copy_layer_registry_to_registry.retry.sleep = mock.Mock()
1147 1152
         u._copy_registry_to_registry.retry.sleep = mock.Mock()
1148
-        u._copy_registry_to_local.retry.sleep = mock.Mock()
1149 1153
         u._copy_local_to_registry.retry.sleep = mock.Mock()
1150 1154
         self.requests = self.useFixture(rm_fixture.Fixture())
1151 1155
 
1156
+    @mock.patch('tripleo_common.image.image_uploader.'
1157
+                'BaseImageUploader.check_status')
1152 1158
     @mock.patch('tripleo_common.image.image_uploader.'
1153 1159
                 'PythonImageUploader.authenticate')
1154 1160
     @mock.patch('tripleo_common.image.image_uploader.'
@@ -1159,7 +1165,7 @@ class TestPythonImageUploader(base.TestCase):
1159 1165
                 'PythonImageUploader._copy_registry_to_registry')
1160 1166
     def test_upload_image(
1161 1167
             self, _copy_registry_to_registry, _cross_repo_mount,
1162
-            _fetch_manifest, authenticate):
1168
+            _fetch_manifest, authenticate, check_status):
1163 1169
 
1164 1170
         target_session = mock.Mock()
1165 1171
         source_session = mock.Mock()
@@ -1237,6 +1243,8 @@ class TestPythonImageUploader(base.TestCase):
1237 1243
             target_session=target_session
1238 1244
         )
1239 1245
 
1246
+    @mock.patch('tripleo_common.image.image_uploader.'
1247
+                'BaseImageUploader.check_status')
1240 1248
     @mock.patch('tripleo_common.image.image_uploader.'
1241 1249
                 'PythonImageUploader.authenticate')
1242 1250
     @mock.patch('tripleo_common.image.image_uploader.'
@@ -1247,7 +1255,7 @@ class TestPythonImageUploader(base.TestCase):
1247 1255
                 'PythonImageUploader._copy_registry_to_registry')
1248 1256
     def test_authenticate_upload_image(
1249 1257
             self, _copy_registry_to_registry, _cross_repo_mount,
1250
-            _fetch_manifest, authenticate):
1258
+            _fetch_manifest, authenticate, check_status):
1251 1259
 
1252 1260
         self.uploader.registry_credentials = {
1253 1261
             'docker.io': {'my_username': 'my_password'},
@@ -1308,6 +1316,8 @@ class TestPythonImageUploader(base.TestCase):
1308 1316
             ),
1309 1317
         ])
1310 1318
 
1319
+    @mock.patch('tripleo_common.image.image_uploader.'
1320
+                'BaseImageUploader.check_status')
1311 1321
     @mock.patch('tripleo_common.image.image_uploader.'
1312 1322
                 'PythonImageUploader.authenticate')
1313 1323
     @mock.patch('tripleo_common.image.image_uploader.'
@@ -1318,7 +1328,7 @@ class TestPythonImageUploader(base.TestCase):
1318 1328
                 'PythonImageUploader._copy_registry_to_registry')
1319 1329
     def test_insecure_registry(
1320 1330
             self, _copy_registry_to_registry, _cross_repo_mount,
1321
-            _fetch_manifest, authenticate):
1331
+            _fetch_manifest, authenticate, check_status):
1322 1332
         target_session = mock.Mock()
1323 1333
         source_session = mock.Mock()
1324 1334
         authenticate.side_effect = [
@@ -1374,6 +1384,8 @@ class TestPythonImageUploader(base.TestCase):
1374 1384
             ),
1375 1385
         ])
1376 1386
 
1387
+    @mock.patch('tripleo_common.image.image_uploader.'
1388
+                'BaseImageUploader.check_status')
1377 1389
     @mock.patch('tripleo_common.image.image_uploader.'
1378 1390
                 'PythonImageUploader.authenticate')
1379 1391
     @mock.patch('tripleo_common.image.image_uploader.'
@@ -1384,7 +1396,7 @@ class TestPythonImageUploader(base.TestCase):
1384 1396
                 'PythonImageUploader._copy_registry_to_registry')
1385 1397
     def test_upload_image_v1_manifest(
1386 1398
             self, _copy_registry_to_registry, _cross_repo_mount,
1387
-            _fetch_manifest, authenticate):
1399
+            _fetch_manifest, authenticate, check_status):
1388 1400
 
1389 1401
         target_session = mock.Mock()
1390 1402
         source_session = mock.Mock()
@@ -1460,6 +1472,8 @@ class TestPythonImageUploader(base.TestCase):
1460 1472
             target_session=target_session
1461 1473
         )
1462 1474
 
1475
+    @mock.patch('tripleo_common.image.image_uploader.'
1476
+                'BaseImageUploader.check_status')
1463 1477
     @mock.patch('tripleo_common.image.image_uploader.'
1464 1478
                 'PythonImageUploader.authenticate')
1465 1479
     @mock.patch('tripleo_common.image.image_uploader.'
@@ -1479,7 +1493,8 @@ class TestPythonImageUploader(base.TestCase):
1479 1493
     def test_upload_image_modify(
1480 1494
             self, _copy_local_to_registry, run_modify_playbook,
1481 1495
             _copy_registry_to_local, _copy_registry_to_registry,
1482
-            _cross_repo_mount, _fetch_manifest, _image_exists, authenticate):
1496
+            _cross_repo_mount, _fetch_manifest, _image_exists, authenticate,
1497
+            check_status):
1483 1498
 
1484 1499
         _image_exists.return_value = False
1485 1500
         target_session = mock.Mock()
@@ -1599,7 +1614,9 @@ class TestPythonImageUploader(base.TestCase):
1599 1614
             session=target_session
1600 1615
         )
1601 1616
 
1602
-    def test_fetch_manifest(self):
1617
+    @mock.patch('tripleo_common.image.image_uploader.'
1618
+                'BaseImageUploader.check_status')
1619
+    def test_fetch_manifest(self, check_status):
1603 1620
         url = urlparse('docker://docker.io/t/nova-api:tripleo-current')
1604 1621
         manifest = '{"layers": []}'
1605 1622
         session = mock.Mock()
@@ -1619,7 +1636,9 @@ class TestPythonImageUploader(base.TestCase):
1619 1636
             }
1620 1637
         )
1621 1638
 
1622
-    def test_upload_url(self):
1639
+    @mock.patch('tripleo_common.image.image_uploader.'
1640
+                'BaseImageUploader.check_status')
1641
+    def test_upload_url(self, check_status):
1623 1642
         # test with previous request
1624 1643
         previous_request = mock.Mock()
1625 1644
         previous_request.headers = {
@@ -1740,12 +1759,15 @@ class TestPythonImageUploader(base.TestCase):
1740 1759
             'docker'
1741 1760
         )
1742 1761
 
1762
+    @mock.patch('tripleo_common.image.image_uploader.'
1763
+                'BaseImageUploader.check_status')
1743 1764
     @mock.patch('tripleo_common.image.image_uploader.'
1744 1765
                 'PythonImageUploader._upload_url')
1745 1766
     @mock.patch('tripleo_common.image.image_uploader.'
1746 1767
                 'PythonImageUploader.'
1747 1768
                 '_copy_layer_registry_to_registry')
1748
-    def test_copy_registry_to_registry(self, _copy_layer, _upload_url):
1769
+    def test_copy_registry_to_registry(self, _copy_layer, _upload_url,
1770
+                                       check_status):
1749 1771
         source_url = urlparse('docker://docker.io/t/nova-api:latest')
1750 1772
         target_url = urlparse('docker://192.168.2.1:5000/t/nova-api:latest')
1751 1773
         _upload_url.return_value = 'https://192.168.2.1:5000/v2/upload'
@@ -1806,7 +1828,6 @@ class TestPythonImageUploader(base.TestCase):
1806 1828
                 params={'digest': 'sha256:1234'},
1807 1829
                 timeout=30
1808 1830
             ),
1809
-            mock.call().raise_for_status(),
1810 1831
             mock.call(
1811 1832
                 'https://192.168.2.1:5000/v2/t/nova-api/manifests/latest',
1812 1833
                 data=mock.ANY,
@@ -1816,7 +1837,6 @@ class TestPythonImageUploader(base.TestCase):
1816 1837
                 },
1817 1838
                 timeout=30
1818 1839
             ),
1819
-            mock.call().raise_for_status(),
1820 1840
         ])
1821 1841
         put_manifest = json.loads(
1822 1842
             target_session.put.call_args[1]['data'].decode('utf-8')

Loading…
Cancel
Save