Skip to content

api: make version checks more generic - #53

Open
mikechristie wants to merge 2 commits into
ceph:masterfrom
mikechristie:rm-distro-checks
Open

mikechristie wants to merge 2 commits into
ceph:masterfrom
mikechristie:rm-distro-checks

Conversation

@mikechristie

Copy link
Copy Markdown
Contributor

Instead of checking for specific rpms, check for the minimal package
versions we need.

This drops the kernel check because ceph-iscsi-config and rtslib
will request specific kernel features (like the qfull setting and alua)
and if they are not supported a unsupported kernel error will be
reported.

This also drops the tcmu-runner check. This is also dynamically handled
due to the detection of missing settings like osd_op_timeout not being
supported by unsupported versions of tcmu-runner.

Signed-off-by: Mike Christie mchristi@redhat.com

@mikechristie mikechristie mentioned this pull request Nov 30, 2017
@pcuzner

pcuzner commented Nov 30, 2017

Copy link
Copy Markdown
Collaborator

Would this mean that a gateway would be accepted into the config that doesn't have the correct versions of tcmu-runner and kernel? And if so, I'd have to diagnose that by knowing to look at tcmu-runner and syslog?

@mikechristie

Copy link
Copy Markdown
Contributor Author

Currently, yes, but I am trying to make it so the create command returns a error string like "ALUA not supported in kernel" or "Image not created due to unsupported feature XYZ in ABC", instead of just logging it and returning the generic internal server error. Will something like that be ok?

Or, if we want to do these checks at checksversion time, do you think we should have the remote caller pass in its version requirements and check against the local versions? That way we could detect if the user was running a version 2 on the remote gw, and on the local gw we are running version 1. And, version 1 meets the minimal requirements when it was coded but there is some issue where we need all versions sync'd to version 2.

I can do either one or do we need both?

Mike Christie added 2 commits March 30, 2018 02:38
This drops the OS and kernel checks in pre_reqs_errors.
We will rely on the kernel exporting the needed features
and if not present we will return error for the specific
command that is not supported.

This also fixes the error code for gw creation so you get
a error code like

Failed : iscsi1 failed package validation checks - Found ceph_iscsi_config 2.3. 102.3 or newer required.

instead of the

Failed : Malformed REST API response

due to the bug

Mar 30 02:01:16 iscsi1 rbd-target-api[4050]:     this_ver, this_rel = this_kernel.split('-')
Mar 30 02:01:16 iscsi1 rbd-target-api[4050]: ValueError: need more than 1 value to unpack

Signed-off-by: Mike Christie <mchristi@redhat.com>
The last patch removed the OS and kernel checks. We instead
rely on tcmu-runner and rtslib to check the kernel for us.

This patch has us return a detailed error message. Instead of:

Failed : disk create/update failed on iscsi1. LUN allocation failure

we get something like:

Failed : disk create/update failed on iscsi1. Could not set LIO device attribute cmd_time_out/qfull_time_out for device: rbd.disk_2. Kernel not supported. - error(Cannot find attribute: qfull_time_out)
Signed-off-by: Mike Christie <mchristi@redhat.com>
@mikechristie

Copy link
Copy Markdown
Contributor Author

Paul, I made the patches return error strings for the gwcli commands. gw create and disk add return messages like:

Failed : iscsi1 failed package validation checks - Found ceph_iscsi_config 2.3. 102.3 or newer required.

Failed : disk create/update failed on iscsi1. Could not set LIO device attribute cmd_time_out/qfull_time_out for device: rbd.disk_2. Kernel not supported. - error(Cannot find attribute: qfull_time_out)

Comment thread rbd-target-api.py
from rpm import labelCompare
import rados

import pkg_resources

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Need to add python-setuptools to spec Requires section

Comment thread rbd-target-api.py
# with backported tcmu patches.

for pkg in required_pkgs:
version_str = pkg_resources.require(pkg['name'])[0].version

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This can throw an exception. In my CentOS distro, rtslib_fb throws an error about python-pyudev being too old (due to the requirements in the package). It will also throw if the package isn't installed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok. Will add exception handling and do more testing with combos of installed/uninstalled packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants