From 9d3fada5135467550adfb01a8fb9d34f148a748e Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Thu, 21 May 2009 00:37:03 +0100
Subject: [PATCH] Add initial validation checks in Cluster.loadData

This patch converts loadTabular and loadData to a monadic form, thus
allowing meaningful error messages from the node/instance load routines.
---
 Ganeti/HTools/Cluster.hs | 92 ++++++++++++++++++++++------------------
 1 file changed, 50 insertions(+), 42 deletions(-)

diff --git a/Ganeti/HTools/Cluster.hs b/Ganeti/HTools/Cluster.hs
index 73060a1f5..77237f73d 100644
--- a/Ganeti/HTools/Cluster.hs
+++ b/Ganeti/HTools/Cluster.hs
@@ -40,6 +40,7 @@ import Data.List
 import Data.Maybe (isNothing, fromJust)
 import Text.Printf (printf)
 import Data.Function
+import Control.Monad
 
 import qualified Ganeti.HTools.Container as Container
 import qualified Ganeti.HTools.Instance as Instance
@@ -694,15 +695,15 @@ This function converts a text in tabular format as generated by
 supplied conversion function.
 
 -}
-loadTabular :: String -> ([String] -> (String, a))
-            -> (a -> Int -> a) -> ([(String, Int)], [(Int, a)])
-loadTabular text_data convert_fn set_fn =
-    let lines_data = lines text_data
-        rows = map (sepSplit '|') lines_data
-        kerows = (map convert_fn rows)
-        idxrows = map (\ (idx, (k, v)) -> ((k, idx), (idx, set_fn v idx)))
-                  (zip [0..] kerows)
-    in unzip idxrows
+loadTabular :: (Monad m) => String -> ([String] -> m (String, a))
+            -> (a -> Int -> a) -> m ([(String, Int)], [(Int, a)])
+loadTabular text_data convert_fn set_fn = do
+  let lines_data = lines text_data
+      rows = map (sepSplit '|') lines_data
+  kerows <- mapM convert_fn rows
+  let idxrows = map (\ (idx, (k, v)) -> ((k, idx), (idx, set_fn v idx)))
+                (zip [0..] kerows)
+  return $ unzip idxrows
 
 -- | For each instance, add its index to its primary and secondary nodes
 fixNodes :: [(Int, Node.Node)]
@@ -742,6 +743,13 @@ stripSuffix suffix lst =
     let sflen = length suffix in
     map (\ (key, name) -> (key, take ((length name) - sflen) name)) lst
 
+-- | Lookups a node into an assoc list
+lookupNode :: (Monad m) => String -> String -> [(String, Int)] -> m Int
+lookupNode node inst ktn =
+    case lookup node ktn of
+      Nothing -> fail $ "Unknown node " ++ node ++ " for instance " ++ inst
+      Just idx -> return idx
+
 {-| Initializer function that loads the data from a node and list file
     and massages it into the correct format. -}
 loadData :: String -- ^ Node data in text format
@@ -749,39 +757,39 @@ loadData :: String -- ^ Node data in text format
          -> Result (Container.Container Node.Node,
                     Container.Container Instance.Instance,
                     String, NameList, NameList)
-loadData ndata idata =
-    let
-    {- node file: name t_mem n_mem f_mem t_disk f_disk -}
-        (ktn, nl) = loadTabular ndata
-                    (\ (name:tm:nm:fm:td:fd:fo:[]) ->
-                         (name,
-                          if any (== "?") [tm,nm,fm,td,fd] || fo == "Y" then
-                              Node.create 0 0 0 0 0 True
-                          else
-                              Node.create (read tm) (read nm) (read fm)
-                                      (read td) (read fd) False
-                         ))
-                    Node.setIdx
-    {- instance file: name mem disk status pnode snode -}
-        (kti, il) = loadTabular idata
-                    (\ (name:mem:dsk:status:pnode:snode:[]) ->
-                         (name,
-                          Instance.create (read mem) (read dsk)
-                              status
-                              (fromJust $ lookup pnode ktn)
-                              (fromJust $ lookup snode ktn)))
-                    Instance.setIdx
-        nl2 = fixNodes nl il
-        il3 = Container.fromAssocList il
-        nl3 = Container.fromAssocList
-             (map (\ (k, v) -> (k, Node.buildPeers v il3 (length nl2))) nl2)
-        xtn = swapPairs ktn
-        xti = swapPairs kti
-        common_suffix = longestDomain (xti ++ xtn)
-        stn = stripSuffix common_suffix xtn
-        sti = stripSuffix common_suffix xti
-    in
-      Ok (nl3, il3, common_suffix, stn, sti)
+loadData ndata idata = do
+  {- node file: name t_mem n_mem f_mem t_disk f_disk -}
+  (ktn, nl) <- loadTabular ndata
+               (\ (name:tm:nm:fm:td:fd:fo:[]) ->
+                    return (name,
+                            if any (== "?") [tm,nm,fm,td,fd] || fo == "Y" then
+                                Node.create 0 0 0 0 0 True
+                            else
+                                Node.create (read tm) (read nm) (read fm)
+                                        (read td) (read fd) False
+                           ))
+               Node.setIdx
+      {- instance file: name mem disk status pnode snode -}
+  (kti, il) <- loadTabular idata
+                  (\ (name:mem:dsk:status:pnode:snode:[]) -> do
+                     pidx <- lookupNode pnode name ktn
+                     sidx <- lookupNode snode name ktn
+                     let newinst = Instance.create (read mem) (read dsk)
+                                   status pidx sidx
+                     return (name, newinst)
+                  )
+                  Instance.setIdx
+  let
+      nl2 = fixNodes nl il
+      il3 = Container.fromAssocList il
+      nl3 = Container.fromAssocList
+            (map (\ (k, v) -> (k, Node.buildPeers v il3 (length nl2))) nl2)
+      xtn = swapPairs ktn
+      xti = swapPairs kti
+      common_suffix = longestDomain (xti ++ xtn)
+      stn = stripSuffix common_suffix xtn
+      sti = stripSuffix common_suffix xti
+  return (nl3, il3, common_suffix, stn, sti)
 
 -- | Compute the amount of memory used by primary instances on a node.
 nodeImem :: Node.Node -> InstanceList -> Int
-- 
GitLab