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

qa: add subvolume option flavors #45556

Merged
merged 3 commits into from
Jun 13, 2022
Merged

qa: add subvolume option flavors #45556

merged 3 commits into from
Jun 13, 2022

Conversation

mchangir
Copy link
Contributor

@mchangir mchangir commented Mar 22, 2022 •
edited
Loading

PR To help test subvolumes with different subvolume creation options

Fixes: https://tracker.ceph.com/issues/54317
Signed-off-by: Milnd Changire mchangir@redhat.com

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

@github-actions github-actions bot added cephfs Ceph File System tests labels Mar 22, 2022
@mchangir mchangir force-pushed the qa-add-subvolume-option-flavors branch 14 times, most recently from c1a6d24 to 68d977e Compare March 24, 2022 07:55
@mchangir mchangir force-pushed the qa-add-subvolume-option-flavors branch 6 times, most recently from ff55776 to fa3c54a Compare March 24, 2022 13:09
batrick
batrick previously requested changes Mar 24, 2022
View reviewed changes
Copy link
Member

@batrick batrick left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM. Good work!

qa/tasks/ceph_fuse.py Outdated Show resolved Hide resolved
qa/tasks/cephfs/filesystem.py Outdated Show resolved Hide resolved
qa/tasks/cephfs/filesystem.py

subvol_path = self.mon_manager.raw_cluster_cmd(
'fs', 'subvolume', 'getpath', self.name, sv_name)
subvol_path = subvol_path.strip()
Copy link
Member

Choose a reason for hiding this comment

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

Is this actually necessary? (It should not be!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

stripping off the the trailing '\n' is necessary
I found this out when I looked up the teuthology logs and saw the dumped path which included the trailing '\n'

Copy link
Member

Choose a reason for hiding this comment

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

Seems you found a bug then? I don't think it should be in the output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@batrick fyi - check qa/tasks/cephfs/test_volumes.py
every place where a getpath is requested, there's a strip operation following it

qa/tasks/cephfs/fuse_mount.py Outdated Show resolved Hide resolved
qa/suites/fs/workload/subvolume/with-namespace-isolated-and-quota.yaml Show resolved Hide resolved
qa/suites/fs/workload/subvolume/with-namespace-isolated-and-quota.yaml Outdated Show resolved Hide resolved
@mchangir mchangir force-pushed the qa-add-subvolume-option-flavors branch 4 times, most recently from 1c4e3b1 to 9a9329f Compare March 29, 2022 02:45
@mchangir
Copy link
Contributor Author

Teuthology runs with subvolume tests

@mchangir mchangir force-pushed the qa-add-subvolume-option-flavors branch from 9a9329f to 12bfb41 Compare March 29, 2022 19:25
@mchangir mchangir force-pushed the qa-add-subvolume-option-flavors branch from c5c5e37 to f4435bf Compare April 8, 2022 03:36
@ajarr
Copy link
Contributor

ajarr commented Apr 8, 2022

but there's no per-test setUp/tearDown methods available
also per test setUp and tearDown will get too cumbersome

You don't have to do that. setUp() and tearDown() are called at the start and end of every test method respectively.
https://docs.python.org/3/library/unittest.html#organizing-tests
https://docs.python.org/3/library/unittest.html#unittest.TestCase.setUp
https://docs.python.org/3/library/unittest.html#unittest.TestCase.tearDown

ajarr
ajarr reviewed Apr 8, 2022
View reviewed changes
qa/tasks/cephfs/test_subvolume.py Show resolved Hide resolved
qa/tasks/cephfs/test_subvolume.py Outdated Show resolved Hide resolved
qa/tasks/cephfs/test_subvolume.py Outdated Show resolved Hide resolved
@mchangir
qa: replace subvolume.sh with test_subvolume.py
8d9bf97 
qa/workunits/fs/misc/subvolume.sh is getting in the way of fs:workload
testing with subvolumes. Hence moved this script to a python test.

Signed-off-by: Milind Changire <mchangir@redhat.com>
@mchangir mchangir force-pushed the qa-add-subvolume-option-flavors branch from f4435bf to 8d9bf97 Compare April 10, 2022 16:05
@ajarr
Copy link
Contributor

ajarr commented Apr 11, 2022

Thanks, Milind! Looks good!

@vshankar
Copy link
Contributor

fs:workload test run: https://pulpito.ceph.com/vshankar-2022-04-11_17:09:49-fs:workload-wip-45556-20220405-111715-testing-default-smithi/

@vshankar
Copy link
Contributor

fs:workload test run: https://pulpito.ceph.com/vshankar-2022-04-11_17:09:49-fs:workload-wip-45556-20220405-111715-testing-default-smithi/

I'm seeing failures that might be real ones since these changes involve testing with options that we do no have coverage on. I will create trackers for those. Also, I will plan to test this with #45407 since it fixes a genuine bug involved with namespace isolation.

@vshankar
Copy link
Contributor

jenkins test make check

@vshankar
Copy link
Contributor

Test run with PR #45407 - https://pulpito.ceph.com/vshankar-2022-04-19_07:30:36-fs:workload-wip-45556-20220418-102656-testing-default-smithi/

@vshankar
Copy link
Contributor

Test run with PR #45407 - https://pulpito.ceph.com/vshankar-2022-04-19_07:30:36-fs:workload-wip-45556-20220418-102656-testing-default-smithi/

The ffsb failures are probably fixed now after merging a fix in ffsb. I'll rerun this fs:worklolads.

@vshankar
Copy link
Contributor

@mchangir FYI - https://pulpito.ceph.com/vshankar-2022-04-26_06:23:29-fs:workload-wip-45556-20220418-102656-testing-default-smithi/

@vshankar
Copy link
Contributor

vshankar commented May 4, 2022

@rishabh-d-dave As discussed, please check if the failures if the fs:workload suite are exposing and bugs (and create trackers for them).

@djgalloway djgalloway changed the base branch from master to main May 25, 2022 20:01
@rishabh-d-dave
Copy link
Contributor

rishabh-d-dave commented May 31, 2022 •
edited
Loading

@vshankar

On QA run 7 jobs failed and 1 job died. Following 4 tickets have been created -

I could reproduce first one every single time. Rest of the jobs being workload tests weren't so reproducible. I can dig deeper and make a reproducing recipe eventually.

@vshankar
Copy link
Contributor

vshankar commented Jun 1, 2022

jenkins test make check

@vshankar
Copy link
Contributor

vshankar commented Jun 1, 2022

Nice work @rishabh-d-dave. Let's get these changes in esp since it has caught a few bugs.

vshankar
vshankar approved these changes Jun 1, 2022
View reviewed changes
Copy link
Contributor

@vshankar vshankar left a comment

Choose a reason for hiding this comment

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

@mchangir Nice work!

@vshankar
Copy link
Contributor

vshankar commented Jun 9, 2022

This caught a bug when running pjd tests - https://tracker.ceph.com/issues/55804

vshankar
vshankar approved these changes Jun 13, 2022
View reviewed changes
Copy link
Contributor

@vshankar vshankar left a comment

Choose a reason for hiding this comment

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

https://tracker.ceph.com/projects/cephfs/wiki/Master#2022-Jun-13

@vshankar vshankar dismissed batrick’s stale review June 13, 2022 06:58

Comment addressed.

@vshankar vshankar merged commit 27f4729 into ceph:main Jun 13, 2022
This was referenced Apr 10, 2023
Closed
Closed
Merged
@mchangir mchangir mentioned this pull request May 16, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@batrick batrick batrick left review comments

@ajarr ajarr ajarr approved these changes

@kotreshhr kotreshhr kotreshhr approved these changes

@vshankar vshankar vshankar approved these changes

Assignees

@rishabh-d-dave rishabh-d-dave

Labels
cephadm cephfs Ceph File System tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mchangir @vshankar @ajarr @rishabh-d-dave @batrick @kotreshhr