From a6c285639916773a6ad9789d5983f32b54ae3c5f Mon Sep 17 00:00:00 2001 From: Cédric Ollivier Date: Thu, 25 Jun 2020 11:28:12 +0200 Subject: Apply "object storage: fix and cleanup header checks" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was highlighted as a fix needed by CNTT during the CNTT RC field trial [1]. [1] http://testresults.opnfv.org/functest/field_trial/ Change-Id: I7e85c952d2a5c9ce5006aa7034eaca6cd1f528ea Signed-off-by: Cédric Ollivier (cherry picked from commit d8326132a9049c063a95e40c46f708387713a9de) --- docker/tempest/Dockerfile | 7 +- ...ect-storage-fix-and-cleanup-header-checks.patch | 171 +++++++++++++++++++++ 2 files changed, 176 insertions(+), 2 deletions(-) create mode 100644 docker/tempest/object-storage-fix-and-cleanup-header-checks.patch (limited to 'docker') diff --git a/docker/tempest/Dockerfile b/docker/tempest/Dockerfile index e268c435c..6a3a34f03 100644 --- a/docker/tempest/Dockerfile +++ b/docker/tempest/Dockerfile @@ -8,6 +8,7 @@ ARG RALLY_OPENSTACK_TAG=1.5.0 COPY Accept-custom-registered-endpoints.patch /tmp/Accept-custom-registered-endpoints.patch COPY Fixes-race-condition-in-test_add_remove_fixed_ip.patch /tmp/Fixes-race-condition-in-test_add_remove_fixed_ip.patch +COPY object-storage-fix-and-cleanup-header-checks.patch /tmp/object-storage-fix-and-cleanup-header-checks.patch COPY Switch-to-threading.Thread-for-Rally-tasks.patch /tmp/Switch-to-threading.Thread-for-Rally-tasks.patch RUN apk --no-cache add --virtual .build-deps --update \ python-dev build-base linux-headers libffi-dev \ @@ -42,9 +43,11 @@ RUN apk --no-cache add --virtual .build-deps --update \ git config --global user.email "opnfv-tech-discuss@lists.opnfv.org" && \ git config --global user.name "Functest" && \ patch -p1 < /tmp/Accept-custom-registered-endpoints.patch && \ + patch -p1 < /tmp/object-storage-fix-and-cleanup-header-checks.patch && \ patch -p1 < /tmp/Fixes-race-condition-in-test_add_remove_fixed_ip.patch && \ git commit -a -m "Backport critical bugfixes" && \ rm ~/.gitconfig) && \ - rm /tmp/Accept-custom-registered-endpoints.patch && \ - rm /tmp/Fixes-race-condition-in-test_add_remove_fixed_ip.patch && \ + rm /tmp/Accept-custom-registered-endpoints.patch \ + /tmp/object-storage-fix-and-cleanup-header-checks.patch \ + /tmp/Fixes-race-condition-in-test_add_remove_fixed_ip.patch && \ apk del .build-deps diff --git a/docker/tempest/object-storage-fix-and-cleanup-header-checks.patch b/docker/tempest/object-storage-fix-and-cleanup-header-checks.patch new file mode 100644 index 000000000..629a98174 --- /dev/null +++ b/docker/tempest/object-storage-fix-and-cleanup-header-checks.patch @@ -0,0 +1,171 @@ +From 42e111c7d8f1cdb5d51a1c4f2ce5c64c3d3471f1 Mon Sep 17 00:00:00 2001 +From: Thomas Morin +Date: Wed, 17 Jun 2020 18:08:49 +0200 +Subject: [PATCH] object storage: fix and cleanup header checks + +As explained in [1] it is not legitimate to require a Transfer-Encoding +header in Swift responses. That prevents running some tests +successfully in the case where Swift is behind a proxy/load-balancer +that does not use any Transfer-Encoding in its responses. + +This change hence removes the checks for the presence of a +"Transfer-Encoding" header, and replaces them by the use +of the existing check methods, after modifying the +custom_matcher checks on which these methods rely on to accept +either a Content-Length or a Transfer-Encoding header. + +Some adaptation was also required to avoid trying to process 'etag' +for DELETE requests. + +A side-effect of this change is a code simplification and +cleanup since the specific header checks in the corresponding +tests are replaced by the generic check methods. + +[1] https://bugs.launchpad.net/tempest/+bug/1819851/comments/3 + +Closes-Bug: 1819851 +Change-Id: Iaccea41640a53b564f72cee73981e2e61d3e80ae +--- + .../api/object_storage/test_account_bulk.py | 37 ++----------------- + tempest/api/object_storage/test_object_slo.py | 13 +------ + tempest/common/custom_matchers.py | 16 +++++++- + 3 files changed, 19 insertions(+), 47 deletions(-) + +diff --git a/tempest/api/object_storage/test_account_bulk.py b/tempest/api/object_storage/test_account_bulk.py +index 6599e432f..80f790f51 100644 +--- a/tempest/api/object_storage/test_account_bulk.py ++++ b/tempest/api/object_storage/test_account_bulk.py +@@ -16,7 +16,6 @@ import tarfile + import tempfile + + from tempest.api.object_storage import base +-from tempest.common import custom_matchers + from tempest.common import utils + from tempest.lib import decorators + +@@ -76,17 +75,7 @@ class BulkTest(base.BaseObjectTest): + resp = self._upload_archive(filepath) + self.containers.append(container_name) + +- # When uploading an archived file with the bulk operation, the response +- # does not contain 'content-length' header. This is the special case, +- # therefore the existence of response headers is checked without +- # custom matcher. +- self.assertIn('transfer-encoding', resp.response) +- self.assertIn('content-type', resp.response) +- self.assertIn('x-trans-id', resp.response) +- self.assertIn('date', resp.response) +- +- # Check only the format of common headers with custom matcher +- self.assertThat(resp.response, custom_matchers.AreAllWellFormatted()) ++ self.assertHeaders(resp.response, 'Account', 'PUT') + + param = {'format': 'json'} + resp, body = self.account_client.list_account_containers(param) +@@ -113,17 +102,7 @@ class BulkTest(base.BaseObjectTest): + data = '%s/%s\n%s' % (container_name, object_name, container_name) + resp = self.bulk_client.delete_bulk_data(data=data) + +- # When deleting multiple files using the bulk operation, the response +- # does not contain 'content-length' header. This is the special case, +- # therefore the existence of response headers is checked without +- # custom matcher. +- self.assertIn('transfer-encoding', resp.response) +- self.assertIn('content-type', resp.response) +- self.assertIn('x-trans-id', resp.response) +- self.assertIn('date', resp.response) +- +- # Check only the format of common headers with custom matcher +- self.assertThat(resp.response, custom_matchers.AreAllWellFormatted()) ++ self.assertHeaders(resp.response, 'Account', 'DELETE') + + # Check if uploaded contents are completely deleted + self._check_contents_deleted(container_name) +@@ -139,17 +118,7 @@ class BulkTest(base.BaseObjectTest): + + resp = self.bulk_client.delete_bulk_data_with_post(data=data) + +- # When deleting multiple files using the bulk operation, the response +- # does not contain 'content-length' header. This is the special case, +- # therefore the existence of response headers is checked without +- # custom matcher. +- self.assertIn('transfer-encoding', resp.response) +- self.assertIn('content-type', resp.response) +- self.assertIn('x-trans-id', resp.response) +- self.assertIn('date', resp.response) +- +- # Check only the format of common headers with custom matcher +- self.assertThat(resp.response, custom_matchers.AreAllWellFormatted()) ++ self.assertHeaders(resp.response, 'Account', 'POST') + + # Check if uploaded contents are completely deleted + self._check_contents_deleted(container_name) +diff --git a/tempest/api/object_storage/test_object_slo.py b/tempest/api/object_storage/test_object_slo.py +index c66776e4e..8bb2e6e4b 100644 +--- a/tempest/api/object_storage/test_object_slo.py ++++ b/tempest/api/object_storage/test_object_slo.py +@@ -17,7 +17,6 @@ import hashlib + from oslo_serialization import jsonutils as json + + from tempest.api.object_storage import base +-from tempest.common import custom_matchers + from tempest.common import utils + from tempest.lib.common.utils import data_utils + from tempest.lib.common.utils import test_utils +@@ -160,17 +159,7 @@ class ObjectSloTest(base.BaseObjectTest): + object_name, + params=params_del) + +- # When deleting SLO using multipart manifest, the response contains +- # not 'content-length' but 'transfer-encoding' header. This is the +- # special case, therefore the existence of response headers is checked +- # outside of custom matcher. +- self.assertIn('transfer-encoding', resp) +- self.assertIn('content-type', resp) +- self.assertIn('x-trans-id', resp) +- self.assertIn('date', resp) +- +- # Check only the format of common headers with custom matcher +- self.assertThat(resp, custom_matchers.AreAllWellFormatted()) ++ self.assertHeaders(resp, 'Object', 'DELETE') + + resp, body = self.container_client.list_container_objects( + self.container_name) +diff --git a/tempest/common/custom_matchers.py b/tempest/common/custom_matchers.py +index c702d8896..f1adeab64 100644 +--- a/tempest/common/custom_matchers.py ++++ b/tempest/common/custom_matchers.py +@@ -62,8 +62,9 @@ class ExistsAllResponseHeaders(object): + # [1] https://bugs.launchpad.net/swift/+bug/1537811 + # [2] http://tracker.ceph.com/issues/13582 + if ('content-length' not in actual and ++ 'transfer-encoding' not in actual and + self._content_length_required(actual)): +- return NonExistentHeader('content-length') ++ return NonExistentHeaders(['content-length', 'transfer-encoding']) + if 'content-type' not in actual: + return NonExistentHeader('content-type') + if 'x-trans-id' not in actual: +@@ -192,6 +193,19 @@ class NonExistentHeader(object): + return {} + + ++class NonExistentHeaders(object): ++ """Informs an error message in the case of missing certain headers""" ++ ++ def __init__(self, headers): ++ self.headers = headers ++ ++ def describe(self): ++ return "none of these headers exist: %s" % self.headers ++ ++ def get_details(self): ++ return {} ++ ++ + class InvalidHeaderValue(object): + """Informs an error message when a header contains a bad value""" + +-- +2.27.0 + -- cgit 1.2.3-korg