-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fixed osds_out/osds_down count + ceph iscsi GW reboot #2
base: master
Are you sure you want to change the base?
Conversation
@@ -49,21 +49,21 @@ def unset_noup(self): | |||
|
|||
def get_down_osds(self): | |||
data, _ = self.client.run_cmd('ceph osd tree -f json') | |||
down_osds = [] | |||
self.down_osds = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not neccessary. See L20: if this file
The constructor of this class is calling
self.down_osds = self.get_down_osds()
self.out_osds = self.get_out_osds()
which makes the assignment of self.down_osds
redundant.
If you encountered that down_osds
is not populated you might the root cause in this function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the issue that I saw was that the self.out_osds
was populated initially through the constructor but when the script is marking some disk out it doesn't get updated and that causes removing disks ignoring the threshold which in the end breaks the ceph cluster. I first checked the get_out_osds()
function and it looks good.
down_osds.append(node_osd.get('id')) | ||
return down_osds | ||
self.down_osds.append(node_osd.get('id')) | ||
return self.down_osds | ||
|
||
def get_out_osds(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here.
@@ -30,8 +30,8 @@ def __init__(self): | |||
self.vmwareops = VMwareOps() | |||
self.ops.get_vm_list() | |||
|
|||
if len(self.gateways) < 2: | |||
self.reboot_allowed = False | |||
if len(self.gateways) >= 2: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reverted logic makes more sense here. Good catch
No description provided.