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.

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

ceph-volume allow filtering by uuid, do not require osd id #17606

Merged
merged 12 commits into from Sep 11, 2017

Conversation

alfredodeza
Copy link
Contributor

When creating an OSD, it should suffice using/requiring the OSD fsid (uuid), these changes will enable that ability.

While implementing that and writing new tests, it was clear that all the .filter() calls with lvm tags where matching on any one tag, vs. matching all the tags that were passed on.

This would cause (internally) filtering volumes by ceph.type=journal, ceph.osd_id=1 would return more than one volume if journal was matched (regardless of the id).

Fixes for that issue meant fixing them for Volume Group and Physical Volume APIs too. Tests where added for those.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1478598

@alfredodeza alfredodeza changed the title Wip bz1478598 ceph-volume allow filtering by uuid, do not require osd id Sep 8, 2017
@alfredodeza
Copy link
Contributor Author

jenkins test ceph-volume tox

@alfredodeza
Copy link
Contributor Author

jenkins test ceph-volume centos7-create

@alfredodeza
Copy link
Contributor Author

jenkins test ceph-volume xenial-create

Copy link
Contributor

@andrewschoen andrewschoen left a comment

Choose a reason for hiding this comment

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

Just the small note about the unneeded comment, everything else looks good.

vg_tags = "ceph.group=dmcache,ceph.disk_type=ssd"
osd = api.VolumeGroup(vg_name='volume1', vg_path='/dev/vg/lv', vg_tags=vg_tags)
volume_groups.append(osd)
# note the different osd_id!
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this comment is needed here.

@andrewschoen
Copy link
Contributor

jenkins test ceph-volume xenial-create

@andrewschoen
Copy link
Contributor

jenkins test ceph-volume centos7-create

@alfredodeza
Copy link
Contributor Author

jenkins test ceph-volume xenial-create

@alfredodeza
Copy link
Contributor Author

jenkins test ceph-volume centos7-create

Alfredo Deza added 6 commits September 11, 2017 09:08
Signed-off-by: Alfredo Deza <adeza@redhat.com>
Signed-off-by: Alfredo Deza <adeza@redhat.com>
Signed-off-by: Alfredo Deza <adeza@redhat.com>
Signed-off-by: Alfredo Deza <adeza@redhat.com>
Signed-off-by: Alfredo Deza <adeza@redhat.com>
Signed-off-by: Alfredo Deza <adeza@redhat.com>
@alfredodeza
Copy link
Contributor Author

jenkins test ceph-volume xenial-create

@alfredodeza
Copy link
Contributor Author

jenkins test ceph-volume centos7-create

1 similar comment
@alfredodeza
Copy link
Contributor Author

jenkins test ceph-volume centos7-create

@alfredodeza
Copy link
Contributor Author

jenkins test ceph-volume xenial-create

Alfredo Deza and others added 6 commits September 11, 2017 14:21
Signed-off-by: Alfredo Deza <adeza@redhat.com>
Signed-off-by: Alfredo Deza <adeza@redhat.com>
…just one

Signed-off-by: Alfredo Deza <adeza@redhat.com>
This updates our Vagrantfile to match the changes in ceph-ansible
introduced by ceph/ceph-ansible@298a63c43

Signed-off-by: Andrew Schoen <aschoen@redhat.com>
When the osd ID existed in the UUID in the same format (e.g. '1-') the
parsing would break returning a bogus UUID that is impossible to find

Signed-off-by: Alfredo Deza <adeza@redhat.com>
Signed-off-by: Alfredo Deza <adeza@redhat.com>
@alfredodeza alfredodeza merged commit e5e50f7 into master Sep 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants