Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ceph s3 tests for Riak CS 2.1.0 with bug fixes #1212

Merged
merged 12 commits into from
Aug 24, 2015

Conversation

shino
Copy link
Contributor

@shino shino commented Aug 10, 2015

Main purpose of this PR is to update ceph/s3-tests to latest commit at upstream because upstream has many updates / additions to s3 test suite.

To integrate s3-tests, now nosetest attribute filter is used to minimize changes to s3-tests fork for riak cs.
The filter is implemented in Makefile under ceph_test/ directory.

Also, it includes a few bug fixes:

  • Add XML message for 412 that WM generates
  • Include UploadId in requests for cetain errors for MP requests
  • Add handling of x-amz-metadata directive in PUT Object Copy
  • Fix untracked parts that have to be deleted
  • Don't include NextMarker element when not truncated
  • Remove logging with error severity in 4xx response (not a bug strictly,
    but severity with error+ may produce big load to disk)

Following commits are niether bug fix nor refactoring, they make integration
to s3-tests somewhat easier.

  • Use "Not Found" reason phrase for 404, instead of "Object Not Found"
  • Add list versioned object endpoint WM callback to respond with NotImplemented
    errors

This PR requires basho/s3-tests#4

And change ceph/s3-tests branch to updated one for Riak CS 2.1 release
Errors by client side mistake or (supposed) race is not for error
severity in server operator sense.  Also, lager's logging with error
level triggers disk sync in default settings, it may load disk.
…sions

Example response:

HTTP/1.1 501 Not Implemented
Server: Riak CS
Date: Fri, 07 Aug 2015 05:18:13 GMT
Content-Type: application/xml
Content-Length: 217

<?xml version="1.0" encoding="UTF-8"?>
<Error>
  <Code>NotImplemented</Code>
  <Message>A request you provided implies functionality that is not implemented</Message>
  <Resource>/test/</Resource>
  <RequestId/>
</Error>
Reference
- PUT Object - Copy - Amazon Simple Storage Service :
  http://docs.aws.amazon.com/AmazonS3/latest/API/RESTO   bjectCOPY.html
- Test cases in ceph/s3-tests
  s3tests.functional.test_s3:test_object_copy_replacing_metadata
  s3tests.functional.test_s3:test_object_copy_retaining_metadata
Before this commit, parts to be deleted is calculated based on keys
of {PartNumber, ETag}. If there is retry of upload, there exist multiple
parts with the same {PartNumber, ETag} pair.
This commit fixes it by using ?PART_MANIFEST.part_id, UUIDv4, to
extract parts to be deleted.
@shino
Copy link
Contributor Author

shino commented Aug 11, 2015

All riak_test passed in my local environment. Ready for review now.

For reviewers, this PR should reference changes in basho/s3-tests#4.
make test-client or external_client_tests may refer it if
client_tests/python_tests/ceph_tests/s3-tests does not exists.
As always for PR with dependent PRs in other repos, please checkout
branch riakcs-2.1-honest-but-long-way-around at the directory or
rewrite branch name at client_tests/python_tests/ceph_tests/Makefile
after checking out this PR branch.

-spec start_put_fsm(binary(), binary(), lfs_manifest(),
-spec new_metadata(lfs_manifest(), #wm_reqdata{}) -> {binary(), [{string(), string()}]}.
new_metadata(SrcManifest, RD) ->
case wrq:get_req_header("x-amz-metadata-directive", RD) of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The document states that Constraints: Values other than COPY or REPLACE result in an immediate 400-based error response. You cannot copy an object to itself unless the MetadataDirective header is specified and its value set to REPLACE. I think we have to handle other values here as 400 or something. And handle undefined separately.

@kuenishi
Copy link
Contributor

Great work! I addressed several concerns and questions; let's discussed after our vacation. I worry it's tiresome to write a release note for this pull request listing up all bugs fixed here :)

@kuenishi
Copy link
Contributor

TODO: identify how much known bugs is fixed with this.

@shino
Copy link
Contributor Author

shino commented Aug 24, 2015

The document states that Constraints: Values other than COPY or REPLACE result in an immediate 400-based error response.

This is left unimplemented because I was just sloppy 🙈 Will push the change.

You cannot copy an object to itself unless the MetadataDirective header is specified and its value set to REPLACE.

This is another incompatibility bug of riak cs that can be captured by ceph/s3-tests but is now just skipped.

@shino
Copy link
Contributor Author

shino commented Aug 24, 2015

x-amz-metadata-directive constraint is implemented by 3776448. Ready for review again.

borshop added a commit that referenced this pull request Aug 24, 2015
…stly

Update ceph s3 tests for Riak CS 2.1.0 with bug fixes

Reviewed-by: kuenishi
@kuenishi
Copy link
Contributor

+1

@kuenishi
Copy link
Contributor

@borshop merge

@borshop borshop merged commit c652521 into develop Aug 24, 2015
@shino shino deleted the feature/update-ceph-s3-tests-honestly branch August 24, 2015 06:53
@kuenishi
Copy link
Contributor

kuenishi commented Sep 2, 2015

Release notes are in RCS-257 (#1212)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants