Skip to content

test/rgw/notifications: split tests between basic, kafka and amqp#55688

Merged
yuvalif merged 1 commit intoceph:mainfrom
yuvalif:wip-yuval-split-notif-test
Feb 22, 2024
Merged

test/rgw/notifications: split tests between basic, kafka and amqp#55688
yuvalif merged 1 commit intoceph:mainfrom
yuvalif:wip-yuval-split-notif-test

Conversation

@yuvalif
Copy link
Contributor

@yuvalif yuvalif commented Feb 21, 2024

see comment: https://tracker.ceph.com/issues/64184#note-3

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
  • jenkins test rook e2e

@github-actions github-actions bot added the rgw label Feb 21, 2024
@yuvalif yuvalif force-pushed the wip-yuval-split-notif-test branch 2 times, most recently from b07d9a3 to 44266cc Compare February 21, 2024 12:05
@yuvalif yuvalif marked this pull request as ready for review February 21, 2024 17:45
@yuvalif yuvalif requested a review from a team as a code owner February 21, 2024 17:45
@yuvalif
Copy link
Contributor Author

yuvalif commented Feb 21, 2024

@cbodley
Copy link
Contributor

cbodley commented Feb 21, 2024

this duplicates a lot between the basic/amqp/kafka jobs. currently the tasks are laid out as:

~/ceph/qa/suites/rgw/notifications $ ls tasks
+  0-install.yaml  test_amqp.yaml  test_kafka.yaml  test_others.yaml

the + tells teuthology to concatenate those yaml fragments into a single job. if we changed that to a % instead (or just remove the +), it would create separate jobs for each. we'd just need to move out the 0-install.yaml so it's included for each job. something like this maybe?

rgw/notifications/
                  0-install.yaml
                  1-brokers/
                            %
                            test_amqp.yaml
                            test_kafka.yaml
                            test_basic.yaml
                  (everything else the same)

@yuvalif
Copy link
Contributor Author

yuvalif commented Feb 21, 2024

this duplicates a lot between the basic/amqp/kafka jobs. currently the tasks are laid out as:

~/ceph/qa/suites/rgw/notifications $ ls tasks
+  0-install.yaml  test_amqp.yaml  test_kafka.yaml  test_others.yaml

the + tells teuthology to concatenate those yaml fragments into a single job. if we changed that to a % instead (or just remove the +), it would create separate jobs for each. we'd just need to move out the 0-install.yaml so it's included for each job. something like this maybe?

rgw/notifications/
                  0-install.yaml
                  1-brokers/
                            %
                            test_amqp.yaml
                            test_kafka.yaml
                            test_basic.yaml
                  (everything else the same)

i do want a different distro setup for the tests.
currently, i have amqp install script specific to centos, but the other tests can run on any distro
so, instead of writing the amqp install script for ubuntu (https://tracker.ceph.com/issues/63893), i kept amqp to be centos9, and the other tests to be a random distro

@cbodley
Copy link
Contributor

cbodley commented Feb 21, 2024

could try something like this then?

1-brokers/
  amqp/
    +
    centos_latest.yaml
    test_amqp.yaml
  basic/
    +
    supported-random-distro$
    test_basic.yaml
  kafka/
    +
    supported-random-distro$
    test_kafka.yaml

@yuvalif yuvalif force-pushed the wip-yuval-split-notif-test branch from 44266cc to 6f28930 Compare February 22, 2024 09:58
@yuvalif
Copy link
Contributor Author

yuvalif commented Feb 22, 2024

the 3 tests are passing: https://pulpito.ceph.com/yuvalif-2024-02-22_09:59:48-rgw:notifications-main-distro-default-smithi/
the amqp test if failing to cleanup. @TRYTOBE8TME i remember we had this issue i nthe past. can you have a look?

@yuvalif
Copy link
Contributor Author

yuvalif commented Feb 22, 2024

all pass now: https://pulpito.ceph.com/yuvalif-2024-02-22_17:36:31-rgw:notifications-main-distro-default-smithi/

@yuvalif yuvalif merged commit 365171e into ceph:main Feb 22, 2024
@cbodley
Copy link
Contributor

cbodley commented Feb 22, 2024

i rebased #55657 to pick up these changes, rerunning qa there

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.

2 participants