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

nautilus: ceph-volume: report correct rejected reason in inventory if device type is invalid #36453

Merged
merged 2 commits into from Sep 15, 2020
Merged

nautilus: ceph-volume: report correct rejected reason in inventory if device type is invalid #36453

merged 2 commits into from Sep 15, 2020

Conversation

jan--f
Copy link
Contributor

@jan--f jan--f commented Aug 4, 2020

backport tracker: https://tracker.ceph.com/issues/46113

backport of #35190
parent tracker: https://tracker.ceph.com/issues/46102

this backport was staged using ceph-backport.sh version 15.1.1.389
find the latest version at https://github.com/ceph/ceph/blob/master/src/script/ceph-backport.sh

@jan--f jan--f added this to the nautilus milestone Aug 4, 2020
@rishabh-d-dave
Copy link
Contributor

jenkins test ceph-volume tox

1 similar comment
@rishabh-d-dave
Copy link
Contributor

jenkins test ceph-volume tox

@rishabh-d-dave
Copy link
Contributor

rishabh-d-dave commented Aug 13, 2020 •
edited

@jan--f

The following failure in unit tests is persistent -

   def test_accept_non_removable_device(self, device_info):
        data = {"/dev/sdb": {"removable": 0, "size": 5368709120}}
        device_info(devices=data)
        disk = device.Device("/dev/sdb")
>       assert disk.available
E       assert False
E        +  where False = <Unknown: /dev/sdb>.available

    def test_accept_non_readonly_device(self, device_info):
        data = {"/dev/sda": {"ro": 0, "size": 5368709120}}
        device_info(devices=data)
        disk = device.Device("/dev/sda")
>       assert disk.available
E       assert False
E        +  where False = <Unknown: /dev/sda>.available

ceph_volume/tests/util/test_device.py:544: AssertionError

@rishabh-d-dave
Copy link
Contributor

I could successfully reproduce these test failures locally too.

@satoru-takeuchi
ceph-volume: cleanup code
85f0260 
Simplify the logic and fix a typo.

Signed-off-by: Satoru Takeuchi <satoru.takeuchi@gmail.com>
(cherry picked from commit 0169b72)
@satoru-takeuchi
ceph-volume: show correct rejected reason in inventory if device type…
210ad89 
… is not acceptable

If device type is not acceptable in `c-v inventory`, its rejected reason
becomes "Insufficient space (<5GB)" by mistake. It's because sys_api is
empty due to skipping devices that are neither `disk` nor `device`. We
should report the target device is not acceptable in this case.

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

Signed-off-by: Satoru Takeuchi <satoru.takeuchi@gmail.com>
(cherry picked from commit 3e5d91d)
@jan--f
Copy link
Contributor Author

jan--f commented Sep 10, 2020

jenkins test ceph-volume tox

@jan--f
Copy link
Contributor Author

jan--f commented Sep 10, 2020

jenkins test make check

@jan--f
Copy link
Contributor Author

jan--f commented Sep 10, 2020

The failing tests where doe to tests we no longer carry after nautilus. So they never got fixed in master. They are CephDisk related and not needed because of this. Fix was easy however and tests pass now. @rishabh-d-dave @guits care to review?

@rishabh-d-dave
Copy link
Contributor

jenkins test make check

rishabh-d-dave
rishabh-d-dave approved these changes Sep 11, 2020
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.

Looks good

@jan--f jan--f merged commit bb414de into ceph:nautilus Sep 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

@guits guits Awaiting requested review from guits

Assignees
No one assigned
Labels
ceph-volume needs-rebase
Projects
None yet
Milestone
nautilus
3 participants
@jan--f @rishabh-d-dave @satoru-takeuchi