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: Return correct status code in case of DeleteBucketTagging API. #47262

Merged
merged 1 commit into from Aug 1, 2022
Merged

rgw: Return correct status code in case of DeleteBucketTagging API. #47262

merged 1 commit into from Aug 1, 2022

Conversation

shriya-deshmukh
Copy link
Contributor

@shriya-deshmukh shriya-deshmukh commented Jul 25, 2022 •
edited

Problem:
Observed discrepancy with AWS for DeleteBucketTagging API.
In ceph-RADOS DeleteBucketTagging API returning status code=200
on success, while in AWS it is returning status code=204.

Resolution:
Assign op_ret=STATUS_NO_CONTENT when DeleteBucketTagging operation
is successful.

Issue: https://tracker.ceph.com/issues/56987

Signed-off-by: Shriya Deshmukh shriya.deshmukh@seagate.com

Contribution Guidelines

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
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 dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows

@shriya-deshmukh
rgw: Return correct status code in case of DeleteBucketTagging API.
5f6f4f6 
Problem:
Observed discrepancy with AWS for DeleteBucketTagging API.
In ceph-RADOS DeleteBucketTagging API returning status code=200
on success, while in AWS it is returning status code=204.

Resolution:
Assign op_ret=STATUS_NO_CONTENT when DeleteBucketTagging operation
is successful.

Signed-off-by: Shriya Deshmukh <shriya.deshmukh@seagate.com>
@github-actions github-actions bot added the rgw label Jul 25, 2022
@shriya-deshmukh
Copy link
Contributor Author

@alimaredia @cbodley @dang Could you please review this PR.

@cbodley
Copy link
Contributor

cbodley commented Jul 26, 2022

can you please open a tracker issue at https://tracker.ceph.com/projects/rgw/issues/new, and write a s3test case to verify the fix?

@shriya-deshmukh
Copy link
Contributor Author

can you please open a tracker issue at https://tracker.ceph.com/projects/rgw/issues/new, and write a s3test case to verify the fix?

Sure, will do.
Just wanted to confirm for s3test test cases I need to do the code changes in the ceph/s3_tests repo in https://github.com/ceph/s3-tests/blob/master/s3tests_boto3/functional/test_s3.py this file?

@cbodley
Copy link
Contributor

cbodley commented Jul 27, 2022

Just wanted to confirm for s3test test cases I need to do the code changes in the ceph/s3_tests repo in https://github.com/ceph/s3-tests/blob/master/s3tests_boto3/functional/test_s3.py this file?

exactly right, thanks

shriya-deshmukh added a commit to shriya-deshmukh/s3-tests that referenced this pull request Jul 27, 2022
@shriya-deshmukh
rgw/s3_boto3: Added status code verification for DeleteBucketTagging
baaee00 
case.

Updated test_set_bucket_tagging test for verifying the http status code
for DeleteBucketTagging case.

Related CEPH PR: ceph/ceph#47262

Signed-off-by: Shriya Deshmukh <shriya.deshmukh@seagate.com>
shriya-deshmukh added a commit to shriya-deshmukh/s3-tests that referenced this pull request Jul 27, 2022
@shriya-deshmukh
rgw/s3_boto3: Added http status code verification for DeleteBucketTag…
c8fc8cd 
…ging

case.

Updated test_set_bucket_tagging test for verifying the http status code
for DeleteBucketTagging case.

Related CEPH PR: ceph/ceph#47262

Signed-off-by: Shriya Deshmukh <shriya.deshmukh@seagate.com>
@shriya-deshmukh shriya-deshmukh mentioned this pull request Jul 27, 2022
Merged
@shriya-deshmukh
Copy link
Contributor Author

shriya-deshmukh commented Jul 28, 2022 •
edited

Just wanted to confirm for s3test test cases I need to do the code changes in the ceph/s3_tests repo in https://github.com/ceph/s3-tests/blob/master/s3tests_boto3/functional/test_s3.py this file?

exactly right, thanks

Created a ceph/s3_tests PR - ceph/s3-tests#464 : for validating the status code for DeleteBucketTagging API.
Tested 'test_set_bucket_tagging' updated test case with this PR changes, it is working fine.

@shriya-deshmukh shriya-deshmukh mentioned this pull request Jul 28, 2022
Merged
14 tasks
@alimaredia
Copy link
Contributor

@cdeshmukh I'll take a look at the s3tests PR and do some further review.

@alimaredia
Copy link
Contributor

jenkins test make check

@alimaredia
Copy link
Contributor

jenkins test ceph windows tests

@alimaredia
Copy link
Contributor

jenkins test windows

@alimaredia alimaredia self-requested a review July 28, 2022 19:19
@alimaredia
Copy link
Contributor

@cdeshmukh this looks good to me, I'm going to run it through teuthology (with your new test) and then merge it if it passes

@cdeshmukh
Copy link

cdeshmukh commented Jul 29, 2022 •
edited

@cdeshmukh this looks good to me, I'm going to run it through teuthology (with your new test) and then merge it if it passes

thanks @alimaredia, if found all good, requesting to see if we can merge by monday here, and we will incorporate same in our forked cortx-rgw repo also within some internal timelines. Its just a request if possible, i know its hard to help with such timelines

btw its @shriya-deshmukh's PR

@alimaredia
Copy link
Contributor

Test run on teuthology: https://pulpito.ceph.com/amaredia-2022-07-29_19:29:07-rgw-wip-rgw-delete-bucket-tagging-204-distro-default-smithi/

@shriya-deshmukh
Copy link
Contributor Author

can you please open a tracker issue at https://tracker.ceph.com/projects/rgw/issues/new, and write a s3test case to verify the fix?

Created a tracker issue - https://tracker.ceph.com/issues/56987

@alimaredia alimaredia merged commit 35bfa8d into ceph:main Aug 1, 2022
alimaredia pushed a commit to ceph/s3-tests that referenced this pull request Aug 1, 2022
@shriya-deshmukh @alimaredia
rgw/s3_boto3: Added http status code verification for DeleteBucketTag…
d3e1ccd 
…ging

case.

Updated test_set_bucket_tagging test for verifying the http status code
for DeleteBucketTagging case.

Related CEPH PR: ceph/ceph#47262

Signed-off-by: Shriya Deshmukh <shriya.deshmukh@seagate.com>
(cherry picked from commit c8fc8cd)
@cdeshmukh
Copy link

thanks @alimaredia

ketanarlulkar added a commit to Seagate/s3-tests that referenced this pull request Aug 17, 2022
@ketanarlulkar @soumyakoduri
@cbodley @dang @alimaredia @ifed01 @shriya-deshmukh
Update Master with Ceph Master (#20)
e57c6d4 
* Adjust wait time for cloud-transition test failures

Signed-off-by: Soumya Koduri <skoduri@redhat.com>

* s3tests.conf.SAMPLE: comment out [s3 cloud] section

Signed-off-by: Casey Bodley <cbodley@redhat.com>

* Tag sse* tests with 'fails_on_dbstore' attr

Signed-off-by: Soumya Koduri <skoduri@redhat.com>

* fix GetBucketTagging error code

related to https://tracker.ceph.com/issues/55460

Signed-off-by: Casey Bodley <cbodley@redhat.com>

* remove tests that fail on boto3's parameter validation

Fixes: https://tracker.ceph.com/issues/55193

Signed-off-by: Casey Bodley <cbodley@redhat.com>

* Test case for locked and "marked-deleted" object version removal.

Reproduces: https://tracker.ceph.com/issues/55766
Signed-off by: Igor Fedotov <igor.fedotov@croit.io>

* Fix User Policy test failures on dbstore

Tag User policy tests failing on dbstore as 'fails_on_dbstore'

Signed-off-by: Soumya Koduri <skoduri@redhat.com>

* rgw/s3_boto3: Added http status code verification for DeleteBucketTagging
case.

Updated test_set_bucket_tagging test for verifying the http status code
for DeleteBucketTagging case.

Related CEPH PR: ceph/ceph#47262

Signed-off-by: Shriya Deshmukh <shriya.deshmukh@seagate.com>

Signed-off-by: Soumya Koduri <skoduri@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Shriya Deshmukh <shriya.deshmukh@seagate.com>
Co-authored-by: Soumya Koduri <skoduri@redhat.com>
Co-authored-by: Casey Bodley <cbodley@redhat.com>
Co-authored-by: Daniel Gryniewicz <dang@redhat.com>
Co-authored-by: Ali Maredia <amaredia@redhat.com>
Co-authored-by: Igor Fedotov <igor.fedotov@croit.io>
Co-authored-by: Shriya Deshmukh <shriya.deshmukh@seagate.com>
Co-authored-by: Casey Bodley <cbodley@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alimaredia alimaredia alimaredia approved these changes

@cdeshmukh cdeshmukh cdeshmukh approved these changes

@selvakumaar5496 selvakumaar5496 selvakumaar5496 approved these changes

Assignees
No one assigned
Labels
needs-review needs-test rgw
Projects
None yet
Milestone
No milestone
5 participants
@shriya-deshmukh @cbodley @alimaredia @cdeshmukh @selvakumaar5496