WIP: Add mock server for OpenStack Swift

[d--j]

Adds a mock server for OpenStack Swift so that the Swift Backend gets more test coverage in default cases (i.e. without tests on live filesystems).

For now the mock server does not handle bulk deletes but copy via COPY. Ideally it would be good to have three different kinds/configurations of the mock server (One that has no special support for anything, one that supports copy via COPY and a third that support bulk delete). But I have no good idea how to do this.

(Since TravisCI is integrated with GitHub I use GitHub for this pull request.)

[d--j]

Why did test t3_verify.py::test_retrieve fail? These changes should not affect it, or do they?

[szepeviktor]

I am very glad to have a mock server.

t3_verify.py::test_retrieve failure is known, see #14 (comment)

[szepeviktor]

Could it be a timing issue? (I don't speak python very well)

[d--j]

I can reproduce the error with the following and some time (up to an hour! will beep three times when test failed):

$ while python -m pytest tests/ -k 'test_retrieve'; do clear; done; echo -ne '\007'; sleep 0.5; echo -ne '\007'; sleep 0.5; echo -ne '\007';

Could it be a timing issue?

I cannot really see how.

I thought that it may be is a problem with pytest-catchlog and included the assertions into the with block but got this, then:

========================================================================================================== test session starts ===========================================================================================================
platform darwin -- Python 3.6.2, pytest-3.2.1, py-1.4.34, pluggy-0.4.0 -- /Users/dj/workspace/venvs/s3ql/bin/python
cachedir: tests/.cache
rootdir: /Users/dj/workspace/s3ql-git/tests, inifile: pytest.ini
plugins: catchlog-1.2.2
collected 300 items                                                                                                                                                                                                                       

tests/t3_verify.py::test_retrieve FAILED

================================================================================================================ FAILURES ================================================================================================================
_____________________________________________________________________________________________________________ test_retrieve ______________________________________________________________________________________________________________
Traceback (most recent call last):
  File "/Users/dj/workspace/s3ql-git/tests/pytest_checklogs.py", line 64, in assert_logs
    yield
  File "/Users/dj/workspace/s3ql-git/tests/t3_verify.py", line 98, in test_retrieve
    assert corrupted_fh.getvalue() == 's3ql_data_%d\n' % obj_ids[2]
AssertionError: assert '' == 's3ql_data_30\n'
  + s3ql_data_30

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/dj/workspace/s3ql-git/tests/t3_verify.py", line 98, in test_retrieve
    assert corrupted_fh.getvalue() == 's3ql_data_%d\n' % obj_ids[2]
  File "/usr/local/Cellar/python3/3.6.2/Frameworks/Python.framework/Versions/3.6/lib/python3.6/contextlib.py", line 99, in __exit__
    self.gen.throw(type, value, traceback)
  File "/Users/dj/workspace/s3ql-git/tests/pytest_checklogs.py", line 71, in assert_logs
    % (count, pattern, handler.count))
AssertionError: Expected to catch 1 '^Object %d is corrupted' messages, but got only 0
---------------------------------------------------------------------------------------------------------- Captured stdout call ----------------------------------------------------------------------------------------------------------
..processed 0 objects (0.00%) so far..
----------------------------------------------------------------------------------------------------------- Captured log call ------------------------------------------------------------------------------------------------------------
verify.py                  118 INFO     Reading all objects...
verify.py                  215 WARNING  Backend seems to have lost object 22
verify.py                  181 INFO     Verified all 3 storage objects.
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
========================================================================================================== 299 tests deselected ==========================================================================================================
================================================================================================ 1 failed, 299 deselected in 2.13 seconds ================================================================================================

[szepeviktor]

@Nikratio I hope it helps a lot.

[Nikratio]

That's great, thanks a lot! Regarding running multiple versions of the Swift Mock server: can't you just make a different class for each version, and list them all in mock_server.handler_list? A quick glance at the code and my vague recollection say that this should work.

Regarding the test: this will have to wait until I have multiple hours of free time in one session. I tried looking at this before, but could not figure out what is happening.

[Nikratio]

One thing that one could try is to change the verify code to not just log the messages, but also write them into a file. Then, when the test fails, we could take a look at the file to determine if the corrupted object was really not found, or if there was some issue with the logging system.

[Nikratio]

Let's discuss the test failure separately in https://bitbucket.org/nikratio/s3ql/issues/272/t3_verifypy-test_retrieve-sometimes-fails. Maybe for now we can just add an expect-fail tag.

[d--j]

test_delete_multi now fails on this line: https://github.com/s3ql/s3ql/blob/master/tests/t1_backends.py#L488

    # Without full consistency, deleting an non-existing object
    # may not give an error
    assert backend.unittest_info.retry_time or len(to_delete) > 0

That's because the backend cannot find not_existing but nevertheless it is removed from the to_delete array since the backend cannot determine which objects were not found (Swift only replies with a counter).

Is this assertion vital or can I remove it?

[Nikratio]

Hmm. Remind me why the backend cannot figure out what couldn't be deleted? Looking at _delete_multi(), there is:

        # Some errors occured, so we need to determine what has
        # been deleted and what hasn't
        failed_keys = []
        offset = len(esc_prefix)
        for error in resp_dict['Errors']:
            fullkey = error[0]
            # stangely the name is url encoded in JSON
            assert fullkey.startswith(esc_prefix)
            key = urllib.parse.unquote(fullkey[offset:])
            failed_keys.append(key)
            log.debug('Delete %s failed with %s', key, error[1])
        for key in keys[:]:
            if key not in failed_keys:
                keys.remove(key)

Shouldn't this list keys that didn't exist?

[Nikratio]

Actually, there's something odd going on in this function: the force parameter just means that the function should not return an error if some objects couldn't be deleted *because they didnt exist*. It seems to me that the current implementation takes *force* to mean "never return an error". delete_multi()` should always return an error if an object existed but couldn't be deleted, no matter if force is true or not.

(see the docstring in AbstractBackend).

[Nikratio]

Coming back to the assertion, I think you shouldn't remove it completely but make it conditional so that it's skipped for the swift backend. An isinstance check on backend should be good enough.

[d--j]

Hmm. Remind me why the backend cannot figure out what couldn't be deleted? Looking at _delete_multi(), there is: [...]
Shouldn't this list keys that didn't exist?

Unfortunately not – this "Errors" list is just for errors besides 404s. In case of 404 only the Number Not Found counter is increased.

See https://github.com/openstack/swift/blob/1eeb354c277d2933fca915806260c05a9d2be51c/swift/common/middleware/bulk.py#L644-L666

Actually, there's something odd going on in this function: the force parameter just means that the function should not return an error if some objects couldn't be deleted because they didnt exist. It seems to me that the current implementation takes force to mean "never return an error".delete_multi()` should always return an error if an object existed but couldn't be deleted, no matter if force is true or not.

(see the docstring in AbstractBackend).

OK I removed this early exit for force = True.

BTW: The S3Backend does the same in_delete_multi and would need a fix, too – wouldn't it?

[Nikratio]

Yes, it seems S3Backend has the same problem. Thanks for spotting it!

[Nikratio]

Merged branch, added https://bitbucket.org/nikratio/s3ql/issues/273/s3delete_multi-force-should-not-suppress for the S3 issue.

Thanks!