resolving symlinks specified in osd-devices results in non-persistent device entries being used for pristinity checking

Bug #1782439 reported by Dmitrii Shcherbakov
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ceph OSD Charm
Fix Released
High
James Page

Bug Description

In order to use persistent device names persistent device symlinks are used in the osd-devices charm config as follows: /dev/disk/by-dname/bcache<integer-number>

However, in local unitdb they are listed as /dev/bcache<number> because of this code that resolves symlinks:

https://github.com/openstack/charm-ceph-osd/blob/stable/18.05/hooks/ceph_hooks.py#L513-L514
            if os.path.isabs(path):
                devices.append(os.path.realpath(path))

After a reboot those /dev/bcache<number> entries often get re-shuffled due to how the kernel device enumeration works and the unitdb check for "don't touch devices that have already been processed" becomes invalid.
https://github.com/openstack/charm-ceph-osd/blob/stable/18.05/hooks/ceph_hooks.py#L430-L434

For example, those entries:
sqlite3 .unit-state.db
sqlite> select * from kv;
# ...
osd-devices|["/dev/bcache3", "/dev/bcache2", "/dev/bcache5", "/dev/bcache4", "/dev/bcache1", "/dev/bcache0"]

May not match the output of `tree /dev/disk/by-dname/` or `tree /dev/bcache/`

There may be some areas of this charm code that compare those device values without dereferencing them first assuming that get_devices() have already done this previously.

We need to consider retaining persistent device symlink usage and in any places where there needs to be a device path comparison we need to use something like: realpath_and_compare(devpath1, devpath2).

Tags: cpe-onsite
Changed in charm-ceph-osd:
status: New → Triaged
importance: Undecided → High
description: updated
description: updated
James Page (james-page)
Changed in charm-ceph-osd:
assignee: nobody → Liam Young (gnuoy)
Ryan Beisner (1chb1n)
Changed in charm-ceph-osd:
milestone: none → 18.08
Revision history for this message
James Page (james-page) wrote :

Dmitrii - I thought the kernel device enumeration for bcache devices had been fixed, so that we do get consistent bcache device naming between reboots?

Revision history for this message
Ryan Beisner (1chb1n) wrote :

Please gather juju crashdump, juju status, and a sanitized bundle for this deployment.

Revision history for this message
Dmitrii Shcherbakov (dmitriis) wrote :

James,

From the IRC discussion:

* /dev/bcache entries are volatile which is expected due to how kernel enumeration works
* Persistent device symlink generation is tied to bcache superblock UUID so /dev/disk/by-dname, /dev/bcache/by-label and /dev/bcache/by-uuid symlinks are truly persistent. Label and dname links represent deployment time device names in maas
* The problem is that the charm resolves persistent links to volatile names and stores them in unitdb. This causes issues after reboot because persistent links are resolved to different device numbers (but same actual devices) in post-reboot charm executions and they do not match the ones stored in unitdb

Storing symlinks and resolving on demand without ever saving volatile device paths is the right approach.

Revision history for this message
Ashley Lai (alai) wrote :
Revision history for this message
Dmitrii Shcherbakov (dmitriis) wrote :

One of the important things to handle in a fix for this is an upgrade path for old osd-devices and journal or wal/db entries in unitdb which are already resolved.

Revision history for this message
James Page (james-page) wrote :
Changed in charm-ceph-osd:
status: Triaged → In Progress
assignee: Liam Young (gnuoy) → James Page (james-page)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to charm-ceph-osd (master)

Reviewed: https://review.openstack.org/585419
Committed: https://git.openstack.org/cgit/openstack/charm-ceph-osd/commit/?id=488f7dffefa2fdf9bf439d676103c0168f21d2ea
Submitter: Zuul
Branch: master

commit 488f7dffefa2fdf9bf439d676103c0168f21d2ea
Author: James Page <email address hidden>
Date: Tue Jul 24 17:05:12 2018 +0100

    Use provided device paths

    Ensure that device paths provided by end users are used for OSD's,
    rather than the link target device as this may change between
    reboots. The specific use case is bcache, where:

       /dev/bcacheX:
            changes between reboots
       /dev/disk/by-dname/bcacheX:
            udev managed and consistent

    This change also ensures that any unit data is updated to switch
    back to using the provided block device path, rather than the
    actual target which may have been used in prior charm revisions.

    Change-Id: If5e88d93b9323052ea762d3a4b66f2442d4a19be
    Depends-On: If0e1fbc62bfe7d0f9e21db9bfdeee761060de846
    Closes-Bug: 1782439

Changed in charm-ceph-osd:
status: In Progress → Fix Committed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to charm-ceph-osd (stable/18.05)

Fix proposed to branch: stable/18.05
Review: https://review.openstack.org/586700

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to charm-ceph-osd (stable/18.05)

Reviewed: https://review.openstack.org/586700
Committed: https://git.openstack.org/cgit/openstack/charm-ceph-osd/commit/?id=02c14e95ffe34cf7c57f5e15aa10bdbbb2b20953
Submitter: Zuul
Branch: stable/18.05

commit 02c14e95ffe34cf7c57f5e15aa10bdbbb2b20953
Author: James Page <email address hidden>
Date: Tue Jul 24 17:05:12 2018 +0100

    Use provided device paths

    Ensure that device paths provided by end users are used for OSD's,
    rather than the link target device as this may change between
    reboots. The specific use case is bcache, where:

       /dev/bcacheX:
            changes between reboots
       /dev/disk/by-dname/bcacheX:
            udev managed and consistent

    This change also ensures that any unit data is updated to switch
    back to using the provided block device path, rather than the
    actual target which may have been used in prior charm revisions.

    Change-Id: If5e88d93b9323052ea762d3a4b66f2442d4a19be
    Depends-On: If0e1fbc62bfe7d0f9e21db9bfdeee761060de846
    Closes-Bug: 1782439
    (cherry picked from commit 488f7dffefa2fdf9bf439d676103c0168f21d2ea)

David Ames (thedac)
Changed in charm-ceph-osd:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.