Misc fixes #170
Misc fixes #170
Conversation
If no targets are setup or the target is not mapped to a gateway, then the rbd-target-gw metrics call will crash due to trying to access target.wwn or a empty list of tpgs. This just adds some checks to make sure we have a target and that its mapped to a gateway. Signed-off-by: Mike Christie <mchristi@redhat.com>
In python3 check_output returns result as a byte type, where in python2 we got a string. In python3 there is an arg for check_output to control the encoding, but we do not have it in python2. So this has us rely on the the "ceph osd blacklist ..." command returning a proper error code if it fails instead of parsing the output. Signed-off-by: Mike Christie <mchristi@redhat.com>
If we are running the gateway deletion command always try to do the full (lio and config) deletion if the gateway to be deleted is the local host. The force option where we only clean up the config config only makes sense if a remote node is dead. If we only cleanup the config object on the local node, then we would leave the target running in the kernel when we can always clean it up. Signed-off-by: Mike Christie <mchristi@redhat.com>
| format(blacklisted_ip=blacklisted_ip, | ||
| client_name=conf.cluster_client_name, | ||
| cephconf=conf.cephconf), | ||
| stderr=subprocess.STDOUT, shell=True) |
There was a problem hiding this comment.
Don't you need check=True to have it raise an exception? Does the ceph CLI use a non-zero return code for the "isn't blacklisted" case? Could also probably keep the old code but add a results = results.decode('utf-8') to ensure bytes are converted to a string.
There was a problem hiding this comment.
The "isn't blacklisted" case returns 0.
You don't need to pass in check=True when using check_output:
https://docs.python.org/3/library/subprocess.html
says:
If the return code was non-zero it raises a CalledProcessError. The CalledProcessError object will have the return code in the returncode attribute and any output in the output attribute.
This is equivalent to:
run(..., check=True, stdout=PIPE).stdout
I tested the failure case for both python 3 and 2 and you get an exception.
Yes, we could do a "results.decode('utf-8')" instead.
Let me know if you want me to go the decode() route since you know the "ceph osd" command output a lot better than me.
There was a problem hiding this comment.
Does that mean that it was always (silently) throwing an exception if ceph failed? If you have tested it on the happy path of blacklist successfully removed and blacklist IP didn't exist, I'm happy w/ it.
There was a problem hiding this comment.
Yes. It was silently throwing an exception. The thing is its kind of hard to make it return failure under normal circumstances, so we probably never saw it. For probably the common/easy failure case where the cluster is not reachable the command just hangs/waits for at least 5 minutes (I gave up waiting for a failure after 5 min).
I had to test it by disabling the network when doing the "ceph osd blacklist rm" command and adding the arg "--connect-timeout 5", so the command would return failures within a normal time. The "ceph osd blacklist rm" command would then return failure and check_output would raise an exception.
This fixes some misc bugs: