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.

Sign up for GitHub

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

Jump to bottom

rgw: fix: S3 API KeyCount incorrect return. #37396

Merged
merged 1 commit into from Oct 21, 2020
Merged

rgw: fix: S3 API KeyCount incorrect return. #37396

merged 1 commit into from Oct 21, 2020

Conversation

huww98
Copy link
Contributor

@huww98 huww98 commented Sep 24, 2020

KeyCount should return object count + common prefix count.
see S3 example: https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjectsV2.html#API_ListObjectsV2_Example_5

Related: distribution/distribution#3200

Signed-off-by: 胡玮文 huww98@outlook.com

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@huww98
rgw: fix: S3 API KeyCount incorrect return.
f96a6fd 
KeyCount should return object count + common prefix count.
see S3 example: https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjectsV2.html#API_ListObjectsV2_Example_5

Related: distribution/distribution#3200

Signed-off-by: 胡玮文 <huww98@outlook.com>
mattbenjamin
mattbenjamin approved these changes Sep 24, 2020
View reviewed changes
Copy link
Contributor

@mattbenjamin mattbenjamin left a comment

Choose a reason for hiding this comment

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

sounds valid--but then we should have an s3-test that checks for this

huww98 added a commit to huww98/s3-tests that referenced this pull request Sep 24, 2020
@huww98
Test list_objects_v2 KeyCount with Delimiter
30db28e 
Test for: ceph/ceph#37396

Signed-off-by: 胡玮文 <huww98@outlook.com>
@huww98 huww98 mentioned this pull request Sep 24, 2020
Merged
@huww98
Copy link
Contributor Author

huww98 commented Sep 24, 2020

I add a simple assertion here ceph/s3-tests#359. Do you think that's enough?

I don't want to deploy a test cluster. maybe jenkins can run it automatically?

@huww98
Copy link
Contributor Author

huww98 commented Oct 2, 2020

Ping. Does anything prevent this from merging? We want to see this in the next Octopus release. Thanks.

@dang dang added needs-qa and removed needs-review labels Oct 2, 2020
mattbenjamin pushed a commit to ceph/s3-tests that referenced this pull request Oct 5, 2020
@huww98 @mattbenjamin
Test list_objects_v2 KeyCount with Delimiter
1981250 
Test for: ceph/ceph#37396

Signed-off-by: 胡玮文 <huww98@outlook.com>
@cbodley
Copy link
Contributor

cbodley commented Oct 8, 2020

after merging, please cherry-pick the new test case in ceph/s3-tests#359 onto the s3-tests/ceph-master branch

@mattbenjamin
Copy link
Contributor

jenkins test api

@huww98
Copy link
Contributor Author

huww98 commented Oct 13, 2020

API tests are still failing. And the error seems not related to this PR.

I don't know if I have permission to retry.

jenkins test api

@theanalyst
Copy link
Member

jenkins test api

1 similar comment
@huww98
Copy link
Contributor Author

huww98 commented Oct 18, 2020

jenkins test api

@huww98
Copy link
Contributor Author

huww98 commented Oct 21, 2020

Hi, after many attempts, finally all checks have passed. We hope this 1-line fix can be merged soon. Thanks.

@dang dang merged commit 0477cc2 into ceph:master Oct 21, 2020
@huww98 huww98 deleted the fix-keycount branch October 21, 2020 12:49
@huww98
Copy link
Contributor Author

huww98 commented Oct 26, 2020

This is my first time here. Do I still need to open an issue in tracker as documented in https://github.com/ceph/ceph/blob/master/SubmittingPatches-backports.rst? I noticed there are some batch backports. will this be included in one of batch backports automatically?

@mattbenjamin
Copy link
Contributor

Hi @huww98 ; If this is needed in a backport, we need to create a tracker issue referencing this PR, indicate which releases need it (e.g., octopus), and set the state to pending-backport.

@huww98
Copy link
Contributor Author

huww98 commented Oct 26, 2020

Thank you. I've created https://tracker.ceph.com/issues/48000

This was referenced Oct 26, 2020
Merged
Merged
cbodley pushed a commit to ceph/s3-tests that referenced this pull request Jan 17, 2022
@huww98 @cbodley
Test list_objects_v2 KeyCount with Delimiter
ae981dd 
Test for: ceph/ceph#37396

Signed-off-by: 胡玮文 <huww98@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattbenjamin mattbenjamin mattbenjamin approved these changes

Assignees
No one assigned
Labels
needs-qa rgw
Projects
None yet
Milestone
No milestone
6 participants
@huww98 @cbodley @mattbenjamin @theanalyst @dang @batrick