Skip to content

src/common : proper handling of units in strict_iec_cast#58898

Merged
rishabh-d-dave merged 3 commits intoceph:mainfrom
neesingh-rh:wip-fix-strict-iec-cast
Oct 24, 2024
Merged

src/common : proper handling of units in strict_iec_cast#58898
rishabh-d-dave merged 3 commits intoceph:mainfrom
neesingh-rh:wip-fix-strict-iec-cast

Conversation

@neesingh-rh
Copy link
Contributor

@neesingh-rh neesingh-rh commented Jul 29, 2024 •
edited
Loading

ceph.quota.max_bytes accepting invalid values like TG, GT, KT and setting the values to first letter.

Fixes: https://tracker.ceph.com/issues/67169
Signed-off-by: Neeraj Pratap Singh neesingh@redhat.com

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

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

batrick
batrick requested changes Aug 6, 2024
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.

Please add tests to src/test/strtol.cc.

@batrick
Copy link
Member

batrick commented Aug 6, 2024

Please also update the tracker to: https://tracker.ceph.com/issues/67169

@neesingh-rh neesingh-rh force-pushed the wip-fix-strict-iec-cast branch from 71ca6d1 to 68dc27f Compare August 6, 2024 15:20
@github-actions github-actions bot added the tests label Aug 7, 2024
@neesingh-rh
Copy link
Contributor Author

neesingh-rh commented Aug 7, 2024

jeninks test make check

dparmar18
dparmar18 reviewed Aug 7, 2024
View reviewed changes
dparmar18
dparmar18 reviewed Aug 7, 2024
View reviewed changes
@dparmar18
Copy link
Contributor

dparmar18 commented Aug 7, 2024

@neesingh-rh apart from adding test cases to src/test/strtol.cc (which are generic cases for func strict_iec_cast) you'd need to add CephFS quota specific cases too i.e. qa/tasks/cephfs/test_quota.py (check test_human_readable_quota_values(), test_human_readable_quota_invalid_values() and test_disable_enable_human_readable_quota_values())

dparmar18
dparmar18 reviewed Aug 7, 2024
View reviewed changes
dparmar18
dparmar18 reviewed Aug 7, 2024
View reviewed changes
@neesingh-rh
Copy link
Contributor Author

neesingh-rh commented Aug 8, 2024

@neesingh-rh apart from adding test cases to src/test/strtol.cc (which are generic cases for func strict_iec_cast) you'd need to add CephFS quota specific cases too i.e. qa/tasks/cephfs/test_quota.py (check test_human_readable_quota_values(), test_human_readable_quota_invalid_values() and test_disable_enable_human_readable_quota_values())

As this PR is related to non-acceptance of invalid values, I have added some more invalid values in test_human_readable_quota_invalid_values()

@neesingh-rh neesingh-rh force-pushed the wip-fix-strict-iec-cast branch from 787f380 to 4b7deaa Compare August 8, 2024 06:41
@github-actions github-actions bot added the cephfs Ceph File System label Aug 8, 2024
dparmar18
dparmar18 reviewed Aug 8, 2024
View reviewed changes
@neesingh-rh neesingh-rh force-pushed the wip-fix-strict-iec-cast branch from 4b7deaa to 8a7e155 Compare August 8, 2024 12:20
dparmar18
dparmar18 reviewed Aug 8, 2024
View reviewed changes
@vshankar vshankar requested a review from a team August 9, 2024 14:04
@neesingh-rh neesingh-rh force-pushed the wip-fix-strict-iec-cast branch from 8a7e155 to 9a44889 Compare September 12, 2024 11:52
batrick
batrick requested changes Sep 12, 2024
View reviewed changes
@neesingh-rh neesingh-rh force-pushed the wip-fix-strict-iec-cast branch from 9a44889 to 0094d73 Compare September 17, 2024 06:23
@neesingh-rh neesingh-rh requested a review from batrick September 17, 2024 06:25
batrick
batrick requested changes Sep 19, 2024
View reviewed changes
@neesingh-rh neesingh-rh force-pushed the wip-fix-strict-iec-cast branch from 0094d73 to fa2b571 Compare September 23, 2024 11:41
@dparmar18
Copy link
Contributor

dparmar18 commented Sep 23, 2024

please align the PR/commit title to the issue being fixed

batrick
batrick requested changes Sep 23, 2024
View reviewed changes
@neesingh-rh neesingh-rh force-pushed the wip-fix-strict-iec-cast branch from fa2b571 to ec945f3 Compare September 23, 2024 16:34
@neesingh-rh neesingh-rh changed the title src/common : fixing bug in strict_iec_cast src/common : proper handling of units in strict_iec_cast Sep 23, 2024
@neesingh-rh neesingh-rh force-pushed the wip-fix-strict-iec-cast branch from ec945f3 to fa73ce3 Compare September 23, 2024 18:53
neeraj pratap singh added 3 commits September 24, 2024 13:33
src/common : proper handling of units in strict_iec_cast
39d1cc5 
Fixes: https://tracker.ceph.com/issues/67169
Signed-off-by: Neeraj Pratap Singh <neesingh@redhat.com>
src/test: test to check proper handling of units by strict_iec_cast
07d5199 
Fixes: https://tracker.ceph.com/issues/67169
Signed-off-by: Neeraj Pratap Singh <neesingh@redhat.com>
qa: modify test_human_readable_quota_invalid_values
0b09d16 
Fixes: https://tracker.ceph.com/issues/67169
Signed-off-by: Neeraj Pratap Singh <neesingh@redhat.com>
@neesingh-rh neesingh-rh force-pushed the wip-fix-strict-iec-cast branch from fa73ce3 to 0b09d16 Compare September 24, 2024 08:04
@dparmar18 dparmar18 mentioned this pull request Sep 24, 2024
Open
14 tasks
@neesingh-rh
Copy link
Contributor Author

neesingh-rh commented Sep 25, 2024

jenkins test make check

@rishabh-d-dave
Copy link
Contributor

rishabh-d-dave commented Oct 4, 2024

This PR is under test in https://tracker.ceph.com/issues/68395.

@rishabh-d-dave
Copy link
Contributor

rishabh-d-dave commented Oct 7, 2024

This PR is under test in https://tracker.ceph.com/issues/68419.

@rishabh-d-dave
Copy link
Contributor

rishabh-d-dave commented Oct 24, 2024

jenkins test make check

rishabh-d-dave
rishabh-d-dave approved these changes Oct 24, 2024
View reviewed changes
Copy link
Contributor

@rishabh-d-dave rishabh-d-dave left a comment

Choose a reason for hiding this comment

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

QA run was successful - https://tracker.ceph.com/projects/cephfs/wiki/Main#wip-rishabh-testing-20241007102024

@rishabh-d-dave rishabh-d-dave merged commit adc6f2d into ceph:main Oct 24, 2024
This was referenced Nov 1, 2024
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@batrick batrick batrick approved these changes

@rishabh-d-dave rishabh-d-dave rishabh-d-dave approved these changes

@dparmar18 dparmar18 dparmar18 approved these changes

Assignees

No one assigned

Labels

cephfs Ceph File System common tests wip-rishabh-testing12

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@neesingh-rh @batrick @dparmar18 @rishabh-d-dave