Nodes: Some Bugfixes and new attributes (#318)

* Nodes: Some Bugfixes and new attributes

- add idle_memory and allocated_tres properties
- correctly return GRES in allocated_gres property
- trailing whitespace fixes
- add some new tests for GRES parsing

Author: tazend

pyslurm/core/node.pxd

@@ -68,8 +68,12 @@ cdef class Nodes(MultiClusterMap):
     Attributes:
         free_memory (int):
             Amount of free memory in this node collection. (in Mebibytes)
+            Note that this means actual free memory as returned by the `free`
+            command
         real_memory (int):
             Amount of real memory in this node collection. (in Mebibytes)
+        idle_memory (int):
+            Amount of idle memory in this node collection. (in Mebibytes)
         allocated_memory (int):
             Amount of alloc Memory in this node collection. (in Mebibytes)
         total_cpus (int):
@@ -100,7 +104,7 @@ cdef class Node:
 
     Other Parameters:
         configured_gres (dict):
-            Configured GRES for the node 
+            Configured GRES for the node
         address (str):
             Address of the node
         hostname (str):
@@ -160,6 +164,10 @@ cdef class Node:
             Real Memory in Mebibytes configured for this node.
         free_memory (int):
             Free Memory in Mebibytes on the node.
+            Note that this means actual free memory as returned by the `free`
+            command
+        idle_memory (int):
+            Idle Memory in Mebibytes on the node.
         memory_reserved_for_system (int):
             Memory in Mebibytes reserved for the System not usable by Jobs.
         temporary_disk (int):
@@ -194,6 +202,8 @@ cdef class Node:
             Time this node was last busy, as unix timestamp.
         reason_time (int):
             Time the reason was set for the node, as unix timestamp.
+        allocated_tres (dict):
+            Currently allocated Trackable Resources
         allocated_cpus (int):
             Number of allocated CPUs on the node.
         idle_cpus (int):
@@ -235,4 +245,4 @@ cdef class Node:
 
     @staticmethod
     cdef Node from_ptr(node_info_t *in_ptr)
-    
+

pyslurm/core/node.pyx

@@ -33,13 +33,14 @@ from pyslurm import xcollections
 from pyslurm.utils.helpers import (
     uid_to_name,
     gid_to_name,
-    humanize, 
+    humanize,
     _getgrall_to_dict,
     _getpwall_to_dict,
     cpubind_to_num,
     instance_to_dict,
     nodelist_from_range_str,
     nodelist_to_range_str,
+    gres_from_tres_dict,
 )
 
 
@@ -65,7 +66,7 @@ cdef class Nodes(MultiClusterMap):
         """Load all nodes in the system.
 
         Args:
-            preload_passwd_info (bool): 
+            preload_passwd_info (bool):
                 Decides whether to query passwd and groups information from
                 the system.
                 Could potentially speed up access to attributes of the Node
@@ -83,7 +84,7 @@ cdef class Nodes(MultiClusterMap):
             dict passwd = {}
             dict groups = {}
             Nodes nodes = Nodes()
-            int flags = slurm.SHOW_ALL
+            int flags = slurm.SHOW_ALL | slurm.SHOW_DETAIL
             Node node
 
         verify_rpc(slurm_load_node(0, &nodes.info, flags))
@@ -107,14 +108,20 @@ cdef class Nodes(MultiClusterMap):
             # is raised by replacing it with a zeroed-out node_info_t.
             nodes.info.node_array[cnt] = nodes.tmp_info
 
+            name = node.name
+            if not name:
+                # Could be possible if there are nodes configured in
+                # slurm.conf that cannot be reached anymore.
+                continue
+
             if preload_passwd_info:
                 node.passwd = passwd
                 node.groups = groups
 
             cluster = node.cluster
             if cluster not in nodes.data:
                 nodes.data[cluster] = {}
-            nodes.data[cluster][node.name] = node
+            nodes.data[cluster][name] = node
 
         # We have extracted all pointers
         nodes.info.record_count = 0
@@ -162,7 +169,7 @@ cdef class Nodes(MultiClusterMap):
         n._alloc_umsg()
         cstr.fmalloc(&n.umsg.node_names, node_str)
         verify_rpc(slurm_update_node(n.umsg))
-        
+
     @property
     def free_memory(self):
         return xcollections.sum_property(self, Node.free_memory)
@@ -171,6 +178,10 @@ cdef class Nodes(MultiClusterMap):
     def real_memory(self):
         return xcollections.sum_property(self, Node.real_memory)
 
+    @property
+    def idle_memory(self):
+        return xcollections.sum_property(self, Node.idle_memory)
+
     @property
     def allocated_memory(self):
         return xcollections.sum_property(self, Node.allocated_memory)
@@ -186,7 +197,7 @@ cdef class Nodes(MultiClusterMap):
     @property
     def allocated_cpus(self):
         return xcollections.sum_property(self, Node.allocated_cpus)
-    
+
     @property
     def effective_cpus(self):
         return xcollections.sum_property(self, Node.effective_cpus)
@@ -237,7 +248,7 @@ cdef class Node:
         xfree(self.info)
 
     def __dealloc__(self):
-        self._dealloc_impl() 
+        self._dealloc_impl()
 
     def __setattr__(self, name, val):
         # When a user wants to set attributes on a Node instance that was
@@ -264,7 +275,7 @@ cdef class Node:
     cdef _swap_data(Node dst, Node src):
         cdef node_info_t *tmp = NULL
         if dst.info and src.info:
-            tmp = dst.info 
+            tmp = dst.info
             dst.info = src.info
             src.info = tmp
 
@@ -319,7 +330,7 @@ cdef class Node:
         Implements the slurm_create_node RPC.
 
         Args:
-            state (str, optional): 
+            state (str, optional):
                 An optional state the created Node should have. Allowed values
                 are `future` and `cloud`. `future` is the default.
 
@@ -421,7 +432,7 @@ cdef class Node:
 
     @configured_gres.setter
     def configured_gres(self, val):
-        cstr.fmalloc2(&self.info.gres, &self.umsg.gres, 
+        cstr.fmalloc2(&self.info.gres, &self.umsg.gres,
                       cstr.from_gres_dict(val))
 
     @property
@@ -451,7 +462,7 @@ cdef class Node:
     @extra.setter
     def extra(self, val):
         cstr.fmalloc2(&self.info.extra, &self.umsg.extra, val)
-        
+
     @property
     def reason(self):
         return cstr.to_unicode(self.info.reason)
@@ -486,7 +497,7 @@ cdef class Node:
 
     @property
     def allocated_gres(self):
-        return cstr.to_gres_dict(self.info.gres_used)
+        return gres_from_tres_dict(self.allocated_tres)
 
     @property
     def mcs_label(self):
@@ -511,6 +522,11 @@ cdef class Node:
     def free_memory(self):
         return u64_parse(self.info.free_mem)
 
+    @property
+    def idle_memory(self):
+        real = self.real_memory
+        return 0 if not real else real - self.allocated_memory
+
     @property
     def memory_reserved_for_system(self):
         return u64_parse(self.info.mem_spec_limit)
@@ -596,17 +612,17 @@ cdef class Node:
 #       """dict: TRES that are configured on the node."""
 #       return cstr.to_dict(self.info.tres_fmt_str)
 
-#   @property
-#   def tres_alloc(self):
-#       cdef char *alloc_tres = NULL
-#       if self.info.select_nodeinfo:
-#           slurm_get_select_nodeinfo(
-#               self.info.select_nodeinfo,
-#               slurm.SELECT_NODEDATA_TRES_ALLOC_FMT_STR,
-#               slurm.NODE_STATE_ALLOCATED,
-#               &alloc_tres
-#           )
-#       return cstr.to_gres_dict(alloc_tres)
+    @property
+    def allocated_tres(self):
+        cdef char *alloc_tres = NULL
+        if self.info.select_nodeinfo:
+            slurm_get_select_nodeinfo(
+                self.info.select_nodeinfo,
+                slurm.SELECT_NODEDATA_TRES_ALLOC_FMT_STR,
+                slurm.NODE_STATE_ALLOCATED,
+                &alloc_tres
+            )
+        return cstr.to_dict(alloc_tres)
 
     @property
     def allocated_cpus(self):
@@ -671,10 +687,22 @@ cdef class Node:
             "temperature":   u32_parse(self.info.ext_sensors.temperature)
         }
 
+    @property
+    def _node_state(self):
+        idle_cpus = self.idle_cpus
+        state = self.info.node_state
+
+        if idle_cpus and idle_cpus != self.effective_cpus:
+            # If we aren't idle but also not allocated, then set state to
+            # MIXED.
+            state &= slurm.NODE_STATE_FLAGS
+            state |= slurm.NODE_STATE_MIXED
+
+        return state
+
     @property
     def state(self):
-        cdef char* state = slurm_node_state_string_complete(
-                self.info.node_state)
+        cdef char* state = slurm_node_state_string_complete(self._node_state)
         state_str = cstr.to_unicode(state)
         xfree(state)
         return state_str
@@ -685,9 +713,10 @@ cdef class Node:
 
     @property
     def next_state(self):
+        state = self._node_state
         if ((self.info.next_state != slurm.NO_VAL)
-                and (self.info.node_state & slurm.NODE_STATE_REBOOT_REQUESTED
-                or   self.info.node_state & slurm.NODE_STATE_REBOOT_ISSUED)):
+                and (state & slurm.NODE_STATE_REBOOT_REQUESTED
+                or   state & slurm.NODE_STATE_REBOOT_ISSUED)):
             return cstr.to_unicode(
                     slurm_node_state_string(self.info.next_state))
         else:

pyslurm/utils/cstr.pyx

@@ -133,7 +133,7 @@ cpdef dict to_dict(char *str_dict, str delim1=",", str delim2="="):
     which can easily be converted to a dict.
     """
     cdef:
-        str _str_dict = to_unicode(str_dict) 
+        str _str_dict = to_unicode(str_dict)
         str key, val
         dict out = {}
 
@@ -143,7 +143,7 @@ cpdef dict to_dict(char *str_dict, str delim1=",", str delim2="="):
     for kv in _str_dict.split(delim1):
         if delim2 in kv:
             key, val = kv.split(delim2, 1)
-            out[key] = val
+            out[key] = int(val) if val.isdigit() else val
 
     return out
 
@@ -184,10 +184,10 @@ def dict_to_str(vals, prepend=None, delim1=",", delim2="="):
 
     if isinstance(vals, str):
         tmp_dict = validate_str_key_value_format(vals, delim1, delim2)
-    
+
     for k, v in tmp_dict.items():
         if ((delim1 in str(k) or delim2 in str(k)) or
-                delim1 in str(v) or delim2 in str(v)):    
+                delim1 in str(v) or delim2 in str(v)):
             raise ValueError(
                 f"Key or Value cannot contain either {delim1} or {delim2}. "
                 f"Got Key: {k} and Value: {v}."
@@ -208,22 +208,23 @@ cpdef dict to_gres_dict(char *gres):
     cdef:
         dict output = {}
         str gres_str = to_unicode(gres)
+        str gres_delim = "gres:"
 
     if not gres_str or gres_str == "(null)":
         return {}
 
     for item in re.split(",(?=[^,]+?:)", gres_str):
 
         # Remove the additional "gres" specifier if it exists
-        if "gres:" in item:
-            item = item.replace("gres:", "")
+        if gres_delim in item:
+            item = item.replace(gres_delim, "")
 
         gres_splitted = re.split(
-            ":(?=[^:]+?)", 
+            ":(?=[^:]+?)",
             item.replace("(", ":", 1).replace(")", "")
         )
 
-        name, typ, cnt = gres_splitted[0], gres_splitted[1], 0 
+        name, typ, cnt = gres_splitted[0], gres_splitted[1], 0
 
         # Check if we have a gres type.
         if typ.isdigit():
@@ -243,10 +244,10 @@ cpdef dict to_gres_dict(char *gres):
             # Cover cases with IDX
             idx = gres_splitted[3] if not typ else gres_splitted[4]
             output[name_and_typ] = {
-                "count": cnt,
+                "count": int(cnt),
                 "indexes": idx,
             }
-            
+
     return output
 
 

pyslurm/utils/helpers.pxd

@@ -30,3 +30,4 @@ from libc.stdlib cimport free
 
 cpdef uid_to_name(uint32_t uid, err_on_invalid=*, dict lookup=*)
 cpdef gid_to_name(uint32_t gid, err_on_invalid=*, dict lookup=*)
+cpdef gres_from_tres_dict(dict tres_dict)

pyslurm/utils/helpers.pyx

@@ -97,7 +97,7 @@ def user_to_uid(user, err_on_invalid=True):
     try:
         if isinstance(user, str) and not user.isdigit():
             return getpwnam(user).pw_uid
-        
+
         return getpwuid(int(user)).pw_uid
     except KeyError as e:
         if err_on_invalid:
@@ -208,7 +208,7 @@ def nodelist_to_range_str(nodelist):
         char *nl = nodelist
         slurm.hostlist_t hl
         char *hl_ranged = NULL
-    
+
     hl = slurm.slurm_hostlist_create(nl)
     if not hl:
         return None
@@ -219,7 +219,7 @@ def nodelist_to_range_str(nodelist):
     free(hl_ranged)
     slurm.slurm_hostlist_destroy(hl)
 
-    return out 
+    return out
 
 
 def humanize(num, decimals=1):
@@ -378,3 +378,12 @@ def dehumanize_step_id(sid):
         return slurm.SLURM_PENDING_STEP
     else:
         return int(sid)
+
+
+cpdef gres_from_tres_dict(dict tres_dict):
+    gres_prefix = "gres/"
+    return {
+        k.replace(gres_prefix, ""):v
+        for k, v in tres_dict.items()
+        if gres_prefix in k
+    }