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

pacific: Backport of bucket notifications tests #41850

Closed
wants to merge 7 commits into from

Conversation

TRYTOBE8TME
Copy link

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

@yuvalif
Copy link
Contributor

yuvalif commented Jun 16, 2021

can you please paste a link to passing teuthology run?

@yuvalif
Copy link
Contributor

yuvalif commented Jun 16, 2021

in the commits where you had conflicts, please remove the following text fro the commit comment (keep all the rest of your comments):

It looks like you may be committing a cherry-pick.
 If this is not correct, please run
	git update-ref -d CHERRY_PICK_HEAD
 and try again.

- notification-tests:
client.0:
force-branch: wip-backport-to-pacific
git_remote: https://github.com/TRYTOBE8TME/
Copy link
Contributor

Choose a reason for hiding this comment

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

don't forget to fix that before merge

Copy link
Author

Choose a reason for hiding this comment

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

Yup, I'll take care.

- notification-tests:
client.0:
force-branch: wip-backport-to-pacific
git_remote: https://github.com/TRYTOBE8TME/
Copy link
Contributor

Choose a reason for hiding this comment

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

don't forget to fix that before merge

- notification-tests:
client.0:
force-branch: wip-backport-to-pacific
git_remote: https://github.com/TRYTOBE8TME/
Copy link
Contributor

Choose a reason for hiding this comment

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

don't forget to fix that before merge

@TRYTOBE8TME
Copy link
Author

can you please paste a link to passing teuthology run?

Teuthology link: https://pulpito.ceph.com/kapandya-2021-06-14_15:42:05-rgw:notifications-wip-bntests-backport-distro-basic-smithi/

@yuvalif yuvalif self-requested a review June 17, 2021 11:50
@github-actions
Copy link

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

@cbodley
Copy link
Contributor

cbodley commented Aug 17, 2021

please rebase

@github-actions
Copy link

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

@yuriw
Copy link
Contributor

yuriw commented Oct 1, 2021

@TRYTOBE8TME pls rebase and add needs-qa, thx

cbodley
cbodley previously requested changes Aug 4, 2022
Copy link
Contributor

@cbodley cbodley left a comment

Choose a reason for hiding this comment

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

failed qa

@yuriw
Copy link
Contributor

yuriw commented Aug 4, 2022

@TRYTOBE8TME pls add needs-qa tag when it's ready for testing



@attr('amqp_test')
def test_ps_s3_multipart_on_master():
Copy link
Contributor

Choose a reason for hiding this comment

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

please change according to: https://github.com/ceph/ceph/blob/main/src/test/rgw/bucket_notification/test_bn.py#L2069

the multipart fix was backported. see: 64bb327



@attr('amqp_test')
def test_ps_s3_metadata_on_master():
Copy link
Contributor

Choose a reason for hiding this comment

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



@attr('amqp_test')
def test_ps_s3_versioned_deletion_on_master():
Copy link
Contributor

Choose a reason for hiding this comment

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

@TRYTOBE8TME
Copy link
Author

Passing teuthology: https://pulpito.ceph.com/kapandya-2022-09-22_06:30:56-rgw:notifications-wip-backport-to-pacific-distro-default-smithi/

P.S: the error seen here was already resolved as a part of another PR which is pending backport.

@dang
Copy link
Contributor

dang commented Sep 27, 2022

@cbodley Can we merge this?

@cbodley
Copy link
Contributor

cbodley commented Sep 27, 2022

@TRYTOBE8TME it looks like there are unresolved comments about git_remote: https://github.com/TRYTOBE8TME/

also move the amqp ssl tests to 'test_bn.py'
this fix combines commit: 1418bcc
with commit: 979335f

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

Signed-off-by: Yuval Lifshitz <ylifshit@redhat.com>
(cherry picked from commit 0fcb337)

 Conflicts:
	src/test/rgw/bucket_notification/test_bn.py

 To resolve the conflicts I removed skip_amqp from the beginning of amqp tests and there was a addition of amqp_ssl_test in the list.
 I've added 'etags' to verify_s3_records_by_elements function and also added etags parameter in some places in test_ps_s3_notification_push_kafka_on_master.
@TRYTOBE8TME
Copy link
Author

jenkins test make check

@TRYTOBE8TME
Copy link
Author

@TRYTOBE8TME it looks like there are unresolved comments about git_remote: https://github.com/TRYTOBE8TME/

resolved! Thanks for pointing @cbodley

@TRYTOBE8TME
Copy link
Author

jenkins test make check

@github-actions
Copy link

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

@yuvalif
Copy link
Contributor

yuvalif commented Sep 12, 2023

@cbodley do we still need to backport tests to pacific?

@cbodley
Copy link
Contributor

cbodley commented Sep 13, 2023

@cbodley do we still need to backport tests to pacific?

@yuvalif we'll likely do one or two more point releases for pacific. but these bucket notification tests may not be worth the effort

@yuvalif
Copy link
Contributor

yuvalif commented Sep 14, 2023

closing, according to the comment above

@yuvalif yuvalif closed this Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants