Στις 27 Σεπ 2013, 8:14 μ.μ., ο/η Michele Tartara <[email protected]> έγραψε:

> On Fri, Sep 27, 2013 at 12:02 PM, Spyros Trigazis <[email protected]> wrote:
> Implement functionality to import MonD data from a mock file to
> HTools (mainly for testing purposes) with the --mond-data
> option.
> 
> Signed-off-by: Spyros Trigazis <[email protected]>
> ---
>  src/Ganeti/HTools/CLI.hs           |   11 +++++++
>  src/Ganeti/HTools/ExtLoader.hs     |   63 
> ++++++++++++++++++++++++++++++------
>  src/Ganeti/HTools/Program/Hbal.hs  |    1 +
>  src/Ganeti/HTools/Program/Hinfo.hs |    1 +
>  4 files changed, 66 insertions(+), 10 deletions(-)
> 
> diff --git a/src/Ganeti/HTools/CLI.hs b/src/Ganeti/HTools/CLI.hs
> index b01df61..05ee655 100644
> --- a/src/Ganeti/HTools/CLI.hs
> +++ b/src/Ganeti/HTools/CLI.hs
> @@ -49,6 +49,7 @@ module Ganeti.HTools.CLI
>    , oSpindleUse
>    , oDynuFile
>    , oMonD
> +  , oMonDDataFile
>    , oEvacMode
>    , oExInst
>    , oExTags
> @@ -125,6 +126,8 @@ data Options = Options
>    , optDynuFile    :: Maybe FilePath -- ^ Optional file with dynamic use data
>    , optIgnoreDynu  :: Bool           -- ^ Do not use dynamic use data
>    , optMonD        :: Bool           -- ^ Query MonDs
> +  , optMonDDataFile :: Maybe FilePath -- ^ Optional file with data provided
> +                                     -- ^ by MonDs
> 
> Extreme nitpicking: we try to keep all the comments aligned, so you should 
> add one space in front of all the comments to align them to the one you 
> added. Or, shorten the variable name by one character. :-D 

I'll change the variable name, to affect less lines. 

> 
>    , optEvacMode    :: Bool           -- ^ Enable evacuation mode
>    , optExInst      :: [String]       -- ^ Instances to be excluded
>    , optExTags      :: Maybe [String] -- ^ Tags to use for exclusion
> @@ -181,6 +184,7 @@ defaultOptions  = Options
>    , optIgnoreDynu  = False
>    , optDynuFile    = Nothing
>    , optMonD        = False
> +  , optMonDDataFile = Nothing
>    , optEvacMode    = False
>    , optExInst      = []
>    , optExTags      = Nothing
> @@ -290,6 +294,13 @@ oMonD =
>     "Query MonDs",
>     OptComplNone)
> 
> +oMonDDataFile :: OptType
> +oMonDDataFile =
> +  (Option "" ["mond-data"]
> +   (ReqArg (\ f opts -> Ok opts { optMonDDataFile = Just f }) "FILE")
> +   "Import data provided by MonDs from the given FILE",
> +   OptComplFile)
> +
>  oDiskTemplate :: OptType
>  oDiskTemplate =
>    (Option "" ["disk-template"]
> diff --git a/src/Ganeti/HTools/ExtLoader.hs b/src/Ganeti/HTools/ExtLoader.hs
> index a294033..50b91c5 100644
> --- a/src/Ganeti/HTools/ExtLoader.hs
> +++ b/src/Ganeti/HTools/ExtLoader.hs
> @@ -46,6 +46,8 @@ import System.Time (getClockTime)
>  import Text.Printf (hPrintf)
> 
>  import qualified Text.JSON as J
> +import qualified Data.Map as Map
> +import qualified Data.List as L
> 
>  import qualified Ganeti.Constants as C
>  import qualified Ganeti.DataCollectors.CPUload as CPUload
> @@ -167,18 +169,46 @@ collectors opts =
>      then []
>      else [ DataCollector CPUload.dcName CPUload.dcCategory ]
> 
> +-- | MonDs Data parsed by a mock file. Representing (node name, list of 
> reports
> +-- produced by MonDs Data Collectors).
> +type MonDData = (String, [DCReport])
> +
> +-- | A map storing MonDs data.
> +type MapMonDData = Map.Map String [DCReport]
> +
> +-- | Parse MonD data file contents.
> +pMonDData :: String -> Result [MonDData]
> +pMonDData input =
> +  loadJSArray "Parsing MonD's answer" input >>=
> +  mapM (pMonDN . J.fromJSObject)
> +
> +-- | Parse a node's JSON record.
> +pMonDN :: JSRecord -> Result MonDData
> +pMonDN a = do
> +  node <- tryFromObj "Parsing node's name" a "node"
> +  reports <- tryFromObj "Parsing node's reports" a "reports"
> +  return (node, reports)
> +
>  -- | Query all MonDs for all Data Collector.
> -queryAllMonDDCs :: ClusterData -> IO ClusterData
> -queryAllMonDDCs cdata = do
> +queryAllMonDDCs :: ClusterData -> Options -> IO ClusterData
> +queryAllMonDDCs cdata opts = do
> +  map_mDD <-
> +    case optMonDDataFile opts of
> +      Nothing -> return Nothing
> +      Just fp -> do
> +        monDData_contents <- readFile fp
> +        monDData <- exitIfBad "can't parse MonD data"
> +                    . pMonDData $ monDData_contents
> +        return . Just $ Map.fromList monDData
>    let (ClusterData _ nl il _ _) = cdata
> -  (nl', il') <- foldM queryAllMonDs (nl, il) (collectors opts)
> +  (nl', il') <- foldM (queryAllMonDs map_mDD) (nl, il) (collectors opts)
>    return $ cdata {cdNodes = nl', cdInstances = il'}
> 
>  -- | Query all MonDs for a single Data Collector.
> -queryAllMonDs :: (Node.List, Instance.List) -> DataCollector
> -                 -> IO (Node.List, Instance.List)
> -queryAllMonDs (nl, il) dc = do
> -  elems <- mapM (queryAMonD dc) (Container.elems nl)
> +queryAllMonDs :: Maybe MapMonDData -> (Node.List, Instance.List)
> +                 -> DataCollector -> IO (Node.List, Instance.List)
> +queryAllMonDs m (nl, il) dc = do
> +  elems <- mapM (queryAMonD m dc) (Container.elems nl)
>    let elems' = catMaybes elems
>    if length elems == length elems'
>      then
> @@ -218,10 +248,23 @@ mkReport dc dcr =
>                     Bad _ -> Nothing
>               | otherwise -> Nothing
> 
> +-- | Get data report for the specified Data Collector and Node from the map.
> +fromFile :: DataCollector -> Node.Node -> MapMonDData -> Maybe DCReport
> +fromFile dc node m =
> +  case Map.lookup (Node.name node) m of
> +    Nothing -> Nothing
> +    Just reports ->
> +      let matchDCName dcr = dName dc == dcReportName dcr
> +      in L.find matchDCName reports
> 
> This Nothing -> Nothing doesn't look really nice, nor "haskelly".
> 
> Try to use the maybe function 
> (http://hackage.haskell.org/package/base-4.6.0.1/docs/Prelude.html#v:maybe)
> doing something like:
> let matchDCName dcr = dName dc == dcReportName dcr
> in maybe Nothing (L.find matchDCName) $ Map.lookup (Node.name node) m
> 
> (I didn't really check this code, it might fail lint or something: it's just 
> to give you a rough idea)

It works fine and hlint isn't complaining.

> 
>  
> +
>  -- | Query a MonD for a single Data Collector.
> -queryAMonD :: DataCollector -> Node.Node -> IO (Maybe Node.Node)
> -queryAMonD dc node = do
> -  dcReport <- fromCurl dc node
> +queryAMonD :: Maybe MapMonDData -> DataCollector -> Node.Node
> +              -> IO (Maybe Node.Node)
> +queryAMonD m dc node = do
> +  dcReport <-
> +    case m of
> +      Nothing -> fromCurl dc node
> +      Just m' -> return $ fromFile dc node m'
>    case mkReport dc dcReport of
>      Nothing -> return Nothing
>      Just report ->
> diff --git a/src/Ganeti/HTools/Program/Hbal.hs 
> b/src/Ganeti/HTools/Program/Hbal.hs
> index 776b10f..3fdc30a 100644
> --- a/src/Ganeti/HTools/Program/Hbal.hs
> +++ b/src/Ganeti/HTools/Program/Hbal.hs
> @@ -93,6 +93,7 @@ options = do
>      , oDynuFile
>      , oIgnoreDyn
>      , oMonD
> +    , oMonDDataFile
>      , oExTags
>      , oExInst
>      , oSaveCluster
> diff --git a/src/Ganeti/HTools/Program/Hinfo.hs 
> b/src/Ganeti/HTools/Program/Hinfo.hs
> index 1b45225..08db777 100644
> --- a/src/Ganeti/HTools/Program/Hinfo.hs
> +++ b/src/Ganeti/HTools/Program/Hinfo.hs
> @@ -63,6 +63,7 @@ options = do
>      , oOfflineNode
>      , oIgnoreDyn
>      , oMonD
> +    , oMonDDataFile
>      ]
> 
>  -- | The list of arguments supported by the program.
> --
> 1.7.10.4
> 
> 
> Rest LGTM, thanks.
> Michele

Thanks,
Spyros

> 
> -- 
> Google Germany GmbH
> Dienerstr. 12
> 80331 München
> 
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
> Geschäftsführer: Graham Law, Christine Elizabeth Flores

Reply via email to