htools: replace two hardcoded uses of pri+sec nodes

These two cases use explicit uses of primary and secondary nodes with
Instance.allNodes, which means the code is more flexible if the
internal layout of the instance changes.

I've verified that the output of involvedNodes  is not required to be
4-element long, and as such the function docstring has been updated.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

diff --git a/htools/Ganeti/HTools/Cluster.hs b/htools/Ganeti/HTools/Cluster.hs
index 1822048..37ad970 100644 (file)
--- a/htools/Ganeti/HTools/Cluster.hs
+++ b/htools/Ganeti/HTools/Cluster.hs
@@ -582,8 +582,7 @@ tryBalance ini_tbl disk_moves inst_moves evac_mode mg_limit min_gain =
         all_inst' = if evac_mode
                     then let bad_nodes = map Node.idx . filter Node.offline $
                                          Container.elems ini_nl
         all_inst' = if evac_mode
                     then let bad_nodes = map Node.idx . filter Node.offline $
                                          Container.elems ini_nl

-                         in filter (\e -> Instance.sNode e `elem` bad_nodes ||
-                                          Instance.pNode e `elem` bad_nodes)
+                         in filter (any (`elem` bad_nodes) . Instance.allNodes)

                             all_inst
                     else all_inst
         reloc_inst = filter Instance.movable all_inst'
                             all_inst
                     else all_inst
         reloc_inst = filter Instance.movable all_inst'

@@ -1223,13 +1222,22 @@ printSolutionLine nl il nmlen imlen plc pos =
        cmds)
 
 -- | Return the instance and involved nodes in an instance move.
        cmds)
 
 -- | Return the instance and involved nodes in an instance move.

-involvedNodes :: Instance.List -> Placement -> [Ndx]
+--
+-- Note that the output list length can vary, and is not required nor
+-- guaranteed to be of any specific length.
+involvedNodes :: Instance.List -- ^ Instance list, used for retrieving
+                               -- the instance from its index; note
+                               -- that this /must/ be the original
+                               -- instance list, so that we can
+                               -- retrieve the old nodes
+              -> Placement     -- ^ The placement we're investigating,
+                               -- containing the new nodes and
+                               -- instance index
+              -> [Ndx]         -- ^ Resulting list of node indices

 involvedNodes il plc =
     let (i, np, ns, _, _) = plc
         inst = Container.find i il
 involvedNodes il plc =
     let (i, np, ns, _, _) = plc
         inst = Container.find i il

-        op = Instance.pNode inst
-        os = Instance.sNode inst
-    in nub [np, ns, op, os]
+    in nub $ [np, ns] ++ Instance.allNodes inst

 
 -- | Inner function for splitJobs, that either appends the next job to
 -- the current jobset, or starts a new jobset.
 
 -- | Inner function for splitJobs, that either appends the next job to
 -- the current jobset, or starts a new jobset.