Skip to content

osd_df and cluster_df methods #32

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.

Sign up for GitHub

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

Open
oms4suse wants to merge 5 commits into
base: master
Choose a base branch
from
Open

osd_df and cluster_df methods #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6a71648
ceph_cfg.model:Add 'ceph osd df' output
oms4suse Sep 5, 2016
69fd270
ceph_cfg.presenter:Output 'ceph osd df'
oms4suse Sep 5, 2016
4b7b68e
ceph_cfg.ops_cluster:Get 'ceph osd df' and place in model
oms4suse Sep 5, 2016
a1b66e5
ceph_cfg.init: Provide osd_df method
oms4suse Sep 5, 2016
a8b1f0c
ceph_cfg.presenter: Provide df method
oms4suse Sep 5, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions ceph_cfg/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@
from . import keyring_use
from . import ops_osd


try: # check whether python knows about 'basestring'
basestring
except NameError: # no, it doesn't (it's Python3); use 'str' instead
basestring=str


log = logging.getLogger(__name__)


Expand Down Expand Up @@ -231,6 +238,43 @@ def osd_reweight(**kwargs):
return ops_osd.reweight(**kwargs)


def osd_df(**kwargs):
"""
OSD disk space

Args:
**kwargs: Arbitrary keyword arguments.
cluster_name : Set the cluster name. Defaults to "ceph".
cluster_uuid : Set the cluster date will be added too. Defaults to
the value found in local config.
osd_number : OSD number to query.
"""
osd_number_input = kwargs.get("osd_number")
if osd_number_input is None:
raise Error("osd_number is not specified")
osd_number_as_int = None
if isinstance(osd_number_input, int):
osd_number_as_int = osd_number_input
if isinstance(osd_number_input, basestring):
osd_number_as_int = int(osd_number_input.strip())
if osd_number_as_int is None:
raise Error("osd_number could not be converted to int")
mdl = model.model(**kwargs)
u = mdl_updater.model_updater(mdl)
u.symlinks_refresh()
u.defaults_refresh()
u.partitions_all_refresh()
u.discover_partitions_refresh()
u.defaults_refresh()
u.load_confg(mdl.cluster_name)
u.mon_members_refresh()
# Validate input
cluster_ops = ops_cluster.ops_cluster(mdl)
cluster_ops.df()
p = presenter.mdl_presentor(mdl)
return p.df_osd(osd_number_as_int)


def keyring_create(**kwargs):
"""
Create keyring for cluster
Expand Down Expand Up @@ -1042,6 +1086,32 @@ def cephfs_ls(**kwargs):
return p.cephfs_list()


def cluster_df(**kwargs):
"""
Cluster disk space

Args:
**kwargs: Arbitrary keyword arguments.
cluster_name : Set the cluster name. Defaults to "ceph".
cluster_uuid : Set the cluster date will be added too. Defaults to
the value found in local config.
"""
mdl = model.model(**kwargs)
u = mdl_updater.model_updater(mdl)
u.symlinks_refresh()
u.defaults_refresh()
u.partitions_all_refresh()
u.discover_partitions_refresh()
u.defaults_refresh()
u.load_confg(mdl.cluster_name)
u.mon_members_refresh()
# Validate input
cluster_ops = ops_cluster.ops_cluster(mdl)
cluster_ops.df()
p = presenter.mdl_presentor(mdl)
return p.df()


def cephfs_add(fs_name, **kwargs):
"""
Make new cephfs file system
Expand Down
2 changes: 2 additions & 0 deletions ceph_cfg/model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ def __init__(self, **kwargs):
self.mon_status = None
# Remote connection details
self.connection = connection()
# Stores safe version of 'ceph osd df'
self.cluster_df = None


def kargs_apply(self, **kwargs):
Expand Down
70 changes: 70 additions & 0 deletions ceph_cfg/ops_cluster.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,73 @@ def status_refresh(self):
output["stderr"])
)
self.model.cluster_status = json.loads(output["stdout"].strip())


def _df_node_to_osd_dict(self, node):
node_type = node.get("type")
if node_type != "osd":
return None
osd_id = node.get("id")
osd_name = node.get("name")
osd_weight_crush = node.get("crush_weight")
osd_weight = node.get("reweight")
osd_kb_total = node.get("kb")
osd_kb_used = node.get("kb_used")
osd_kb_avail = node.get("kb_avail")
osd_pgs = node.get("pgs")
osd_utilization = node.get("utilization")
return {
"id" : osd_id,
Copy link

@l-mb l-mb Sep 9, 2016

Choose a reason for hiding this comment

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

Do the temporary variables make sense? You could either directly construct it in the return statement ("id" : node.get("id")) or iterate over a set of keywords and build it up like that? Or use a list comprehension maybe? http://stackoverflow.com/questions/1747817/create-a-dictionary-with-list-comprehension-in-python

Copy link
Owner Author

@oms4suse oms4suse Sep 9, 2016

Choose a reason for hiding this comment

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

Temporary variables provide little advantage without error checking for None. The only real advantage is code clarity.

As for list comprehension, This would make sense if the input and output fields where identical.

I felt that particularly "crush_weight" was more ceph implementation specific, and "weight_crush" being presented to the user (and stored in the model) was easier to understand since it would (in default representation of json by python it is sorted alphabetically) be next to "crush_weight".

Which would you prefer?

I add a comment explaining the rational, or move to list comprehension?

Copy link

@l-mb l-mb Sep 9, 2016

Choose a reason for hiding this comment

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

I'd think that sticking to the ceph naming would probably be easier to understand, even if it is not the naming I'd have chosen (because then we're consistent with how ceph calls things and users don't have to remember multiple naming schemes).

Copy link

@jan--f jan--f Sep 9, 2016

Choose a reason for hiding this comment

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

I would agree, either directly construct in return statement or list comprehension/dict comprehension if keys stay the same. Direct construction would make key change obvious enough I guess.

"name" : osd_name,
"weight_crush" : osd_weight_crush,
"weight" : osd_weight,
"kb_total" : osd_kb_total,
"kb_used" : osd_kb_used,
"kb_avail" : osd_kb_avail,
"pgs" : osd_pgs,
"utilization" : osd_utilization
}


def df(self):
prefix_arguments = [
util_which.which_ceph.path
]
postfix_arguments = [
'osd',
'df',
'-f',
'json'
]
connection_arguments = self.connection.arguments_get()
arguments = prefix_arguments + connection_arguments + postfix_arguments
output = utils.execute_local_command(arguments)
if output["retcode"] != 0:
raise Error("Failed executing '%s' Error rc=%s, stdout=%s stderr=%s" % (
" ".join(arguments),
output["retcode"],
output["stdout"],
output["stderr"])
)
df_dict = json.loads(output["stdout"])
nodes_list = df_dict.get("nodes")
osd_details = {}
if not nodes_list is None:
for node in nodes_list:
osd_dict = self._df_node_to_osd_dict(node)
if not osd_dict is None:
osd_details[osd_dict.get("id")] = osd_dict

summary_raw = df_dict.get("summary")
summary_model = {}
if not summary_raw is None:
summary_model["kb_total"] = summary_raw.get("total_kb")
summary_model["kb_used"] = summary_raw.get("kb_used")
summary_model["kb_avail"] = summary_raw.get("total_kb_avail")
summary_model["utilization_average"] = summary_raw.get("average_utilization")
stored_model = {
'osd' : osd_details,
'summary' : summary_model
}
self.model.cluster_df = stored_model
return self.model.cluster_df
36 changes: 36 additions & 0 deletions ceph_cfg/presenter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,16 @@
log = logging.getLogger(__name__)


class Error(Exception):
"""
Error
"""

def __str__(self):
doc = self.__doc__.strip()
return ': '.join([doc] + [str(a) for a in self.args])


class mdl_presentor():
"""
Since presentation should be clean to the end user
Expand Down Expand Up @@ -349,3 +359,29 @@ def ceph_version(self):

def cephfs_list(self):
return self.model.cephfs_list


def df_osd(self, osd_number):
Copy link

@l-mb l-mb Sep 9, 2016

Choose a reason for hiding this comment

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

This function is called in exactly one place, and besides the error checking has exactly two lines of data mangling. Does it make sense to fold into the calling place? (Also, osd_df calling df_osd is slightly confusing naming scheme ...)

Copy link
Owner Author

@oms4suse oms4suse Sep 9, 2016

Choose a reason for hiding this comment

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

Main rational is I wanted to put all functions presenting to the user the model data structures in one place. Since I had already mangled the ceph output this operation became trivial.

The only reason for this code is consistency.

if self.model.cluster_df is None:
msg = "Programming error : self.model.cluster_df is None"
log.error(msg)
raise(msg)
osd_details = self.model.cluster_df.get("osd")
if osd_details is None:
msg = "Programming error : self.model.cluster_df[osd] is None"
log.error(msg)
raise(msg)
details = osd_details.get(osd_number)
Copy link

@l-mb l-mb Sep 9, 2016

Choose a reason for hiding this comment

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

Does this get the stats for all OSDs? Is that costly for a single lookup?

Copy link
Owner Author

@oms4suse oms4suse Sep 9, 2016

Choose a reason for hiding this comment

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

Yes it does stats for all OSD's.

I am not sure how expensive it it. I had not really thought about cost here as I wanted to focus on functionality.

It might be better to use another call for osd fullness in osd removal. I can list PG by OSD for a less costly operation.

For admin though I see utility in providing a call to do this for inspection.

if details is None:
msg = "Osd '{number}' not found".format(number=osd_number)
log.error(msg)
raise(msg)
return details


def df(self):
if self.model.cluster_df is None:
msg = "Programming error : self.model.cluster_df is None"
log.error(msg)
raise(msg)
return self.model.cluster_df