Στις 24 Σεπ 2013, 12:24 μ.μ., ο/η Michele Tartara <[email protected]> έγραψε:
> On Fri, Sep 20, 2013 at 3:14 PM, Spyros Trigazis <[email protected]> wrote: > Contact all MonDs from HTools to fetch data from its Data > Collectors (only CPUload Data Collector is queried at the > moment). This information is available to all HTools and can be > ignored if the --ignore-dynu option is enabled. This > functionality is implemented in ExtLoader.hs. > > The semantics should be changed according to Klaus' suggestion in the other > email thread. ack > > > Signed-off-by: Spyros Trigazis <[email protected]> > --- > src/Ganeti/HTools/Cluster.hs | 15 ++++- > src/Ganeti/HTools/ExtLoader.hs | 129 > ++++++++++++++++++++++++++++++++++++- > src/Ganeti/HTools/Program/Hail.hs | 11 +++- > 3 files changed, 147 insertions(+), 8 deletions(-) > > diff --git a/src/Ganeti/HTools/Cluster.hs b/src/Ganeti/HTools/Cluster.hs > index 88891a4..cf1705e 100644 > --- a/src/Ganeti/HTools/Cluster.hs > +++ b/src/Ganeti/HTools/Cluster.hs > @@ -496,6 +496,17 @@ applyMove nl inst (FailoverAndReplace new_sdx) = > new_inst, old_sdx, new_sdx) > in new_nl > > +-- | Update the instance utilization according to the utilization of the > +-- given node and the requested number of cpus. > +uInstUtil :: Node.Node -> Instance.Instance -> Instance.Instance > +uInstUtil node inst = > + let cpus = Node.uCpu node + Instance.vcpus inst > + cpuLoad = cpuWeight . Node.utilLoad $ node > + perCpu = cpuLoad / fromIntegral cpus > + load = fromIntegral (Instance.vcpus inst) * perCpu > + oldUtil = Instance.util inst > + in inst { Instance.util = oldUtil { cpuWeight = load }} > + > -- | Tries to allocate an instance on one given node. > allocateOnSingle :: Node.List -> Instance.Instance -> Ndx > -> OpResult Node.AllocElement > @@ -504,7 +515,7 @@ allocateOnSingle nl inst new_pdx = > new_inst = Instance.setBoth inst new_pdx Node.noSecondary > in do > Instance.instMatchesPolicy inst (Node.iPolicy p) (Node.exclStorage p) > - new_p <- Node.addPri p inst > + new_p <- Node.addPri p (uInstUtil p inst) > let new_nl = Container.add new_pdx new_p nl > new_score = compCV new_nl > return (new_nl, new_inst, [new_p], new_score) > @@ -518,7 +529,7 @@ allocateOnPair nl inst new_pdx new_sdx = > in do > Instance.instMatchesPolicy inst (Node.iPolicy tgt_p) > (Node.exclStorage tgt_p) > - new_p <- Node.addPri tgt_p inst > + new_p <- Node.addPri tgt_p (uInstUtil tgt_p inst) > new_s <- Node.addSec tgt_s inst new_pdx > > Why isn't the utilization information added to the secondary node as well? ack > > let new_inst = Instance.setBoth inst new_pdx new_sdx > new_nl = Container.addTwo new_pdx new_p new_sdx new_s nl > diff --git a/src/Ganeti/HTools/ExtLoader.hs b/src/Ganeti/HTools/ExtLoader.hs > index 488345b..d42c833 100644 > --- a/src/Ganeti/HTools/ExtLoader.hs > +++ b/src/Ganeti/HTools/ExtLoader.hs > @@ -27,31 +27,47 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, > Boston, MA > > -} > > +{-# LANGUAGE BangPatterns #-} > + > module Ganeti.HTools.ExtLoader > ( loadExternalData > , commonSuffix > , maybeSaveData > + , queryAllMonDDCs > ) where > > import Control.Monad > import Control.Exception > -import Data.Maybe (isJust, fromJust) > +import Data.Char > +import Data.Maybe (isJust, fromJust, catMaybes) > +import Network.Curl > import System.FilePath > import System.IO > import System.Time (getClockTime) > import Text.Printf (hPrintf) > > +import qualified Text.JSON as J > + > +import qualified Ganeti.Constants as C > +import qualified Ganeti.DataCollectors.CPUload as CPUload > +import qualified Ganeti.HTools.Container as Container > import qualified Ganeti.HTools.Backend.Luxi as Luxi > import qualified Ganeti.HTools.Backend.Rapi as Rapi > import qualified Ganeti.HTools.Backend.Simu as Simu > import qualified Ganeti.HTools.Backend.Text as Text > import qualified Ganeti.HTools.Backend.IAlloc as IAlloc > +import qualified Ganeti.HTools.Node as Node > +import qualified Ganeti.HTools.Instance as Instance > import Ganeti.HTools.Loader (mergeData, checkData, ClusterData(..) > , commonSuffix, clearDynU) > > import Ganeti.BasicTypes > +import Ganeti.Cpu.Types > +import Ganeti.DataCollectors.Types > import Ganeti.HTools.Types > import Ganeti.HTools.CLI > +import Ganeti.JSON > +import Ganeti.Logging (logWarning) > import Ganeti.Utils (sepSplit, tryRead, exitIfBad, exitWhen) > > -- | Error beautifier. > @@ -115,11 +131,12 @@ loadExternalData opts = do > ldresult = input_data >>= (if ignoreDynU then clearDynU else return) > >>= mergeData eff_u exTags selInsts exInsts now > cdata <- exitIfBad "failed to load data, aborting" ldresult > - let (fix_msgs, nl) = checkData (cdNodes cdata) (cdInstances cdata) > + cdata' <- if ignoreDynU then return cdata else queryAllMonDDCs cdata > > I guess the change of semantic (from using MonD as the default, to using it > only when an option is provided, will have to be performed here. ack > > + let (fix_msgs, nl) = checkData (cdNodes cdata') (cdInstances cdata') > > unless (optVerbose opts == 0) $ maybeShowWarnings fix_msgs > > - return cdata {cdNodes = nl} > + return cdata' {cdNodes = nl} > > -- | Function to save the cluster data to a file. > maybeSaveData :: Maybe FilePath -- ^ The file prefix to save to > @@ -134,3 +151,109 @@ maybeSaveData (Just path) ext msg cdata = do > writeFile out_path adata > hPrintf stderr "The cluster state %s has been written to file '%s'\n" > msg out_path > + > +-- | Type describing a data collector basic information > +data DataCollector = DataCollector > + { dName :: String -- ^ Name of the data collector > + , dCategory :: Maybe DCCategory -- ^ The name of the category > + } > + > +-- | The actual data types for MonD's Data Collectors. > +data Report = CPUavgloadReport CPUavgload > + > +-- | The list of Data Collectors used by hail and hbal. > +collectors :: [DataCollector] > +collectors = > + [ DataCollector CPUload.dcName CPUload.dcCategory ] > + > +-- | Query all MonDs for all Data Collector. > +queryAllMonDDCs :: ClusterData -> IO ClusterData > +queryAllMonDDCs cdata = do > + let (ClusterData _ nl il _ _) = cdata > + (nl', il') <- foldM queryAllMonDs (nl, il) collectors > + 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) > + let elems' = catMaybes elems > + if length elems == length elems' > + then do > + let il' = foldl updateUtilData il elems' > + nl' = zip (Container.keys nl) elems' > + return (Container.fromList nl', il') > + else return (nl,il) > > If I'm not mistaken, here you are using the data from MonD only if you got > all of them. Which is good, but the user should also get some message saying > that the data are not being used because they are incomplete. > I see, in the following part of the code, that there is a message when there > was a failure contacting a node, but that doesn't say that ALL the data are > being ignored. I can add to this message that the data provided by the specified data collector are not going to be used at all. > > + > +-- | Query a specified MonD for a Data Collector. > +fromCurl :: DataCollector -> Node.Node -> IO (Maybe DCReport) > +fromCurl dc node = do > + (code, !body) <- curlGetString (prepareUrl dc node) [] > + case code of > + CurlOK -> do > + case J.decodeStrict body :: J.Result DCReport of > + J.Ok r -> return $ Just r > + J.Error _ -> return Nothing > + _ -> do > + logWarning $ "Failed to contact node's " ++ Node.name node > + ++ " MonD for DC " ++ dName dc > + return Nothing > + > +-- | Return the data from correct combination of a Data Collector > +-- and a DCReport. > +mkReport :: DataCollector -> Maybe DCReport -> Maybe Report > +mkReport dc dcr = > + case dcr of > + Nothing -> Nothing > + Just dcr' -> > + case () of > + _ | CPUload.dcName == dName dc -> > > This is a really unusual syntax. Can't you just use an "if" instead of "case > () of _"? I could do that, but in this way it is easier to add more collectors: _ | FirstCollector.dcName == dName dc -> ... _ | SecondCollector.dcName == dName dc -> ... What do you think? > > + case fromJVal (dcReportData dcr') :: Result CPUavgload of > + Ok cav -> Just $ CPUavgloadReport cav > + Bad _ -> Nothing > + | otherwise -> Nothing > + > +-- | Query a MonD for a single Data Collector. > +queryAMonD :: DataCollector -> Node.Node -> IO (Maybe Node.Node) > +queryAMonD dc node = do > + dcReport <- fromCurl dc node > + case mkReport dc dcReport of > + Nothing -> return Nothing > + Just report -> > + case report of > + CPUavgloadReport cav -> > + let ct = cavCpuTotal cav > + du = Node.utilLoad node > + du' = du {cpuWeight = ct} > + in return $ Just node {Node.utilLoad = du'} > + > +-- | Update utilization data. > +updateUtilData :: Instance.List -> Node.Node -> Instance.List > +updateUtilData il node = > + let ct = cpuWeight (Node.utilLoad node) > + n_uCpu = Node.uCpu node > + upd inst = > + if Node.idx node == Instance.pNode inst > + then > + let i_vcpus = Instance.vcpus inst > + i_util = ct / fromIntegral n_uCpu * fromIntegral i_vcpus > + i_du = Instance.util inst > + i_du' = i_du {cpuWeight = i_util} > + in inst {Instance.util = i_du'} > + else inst > > Beware the indentation. We use "then" and "else" one level more indented than > "if": > > if .... > then > ... > else > ... ack > > > + in Container.map upd il > + > +-- | Prepare url to query a single collector. > +prepareUrl :: DataCollector -> Node.Node -> URLString > +prepareUrl dc node = > + Node.name node ++ ":" ++ show C.defaultMondPort ++ "/" > + ++ "1" ++ "/report/" ++ > + getDCCName (dCategory dc) ++ "/" ++ dName dc > > Don't use "1" as a "magic number". It's defined as "latestAPIVersion" in > src/Ganeti/Monitoring/Server.hs. It's better to take it from there. > If importing that file is unconvenient, just move it to the constants file > and import it from there in both places. I'll move it to lib/constants.py. > > + > +-- | Get Category Name. > +getDCCName :: Maybe DCCategory -> String > +getDCCName dcc = > + case dcc of > + Nothing -> "default" > + Just c -> map toLower . drop 2 . show $ c > > Duplicating the function for extracting the name of a category here is not > elegant. It's already part of the DCCategory definition, in > src/Ganeti/DataCollector/Types.hs. Currently, it's part of the JSON instance. > You can either obtain the name asking for the JSON representation of the > category (which isn't really elegant but works) or, much better, extract the > code from there as an independent function, and then use it in the definition > of showJSON and here. CPUload data collector doesn't have a category. Can I still use showJSON in order to construct the url? We need "default" as an answer. > > diff --git a/src/Ganeti/HTools/Program/Hail.hs > b/src/Ganeti/HTools/Program/Hail.hs > index 50009a3..507192c 100644 > --- a/src/Ganeti/HTools/Program/Hail.hs > +++ b/src/Ganeti/HTools/Program/Hail.hs > @@ -39,7 +39,8 @@ import Ganeti.Common > import Ganeti.HTools.CLI > import Ganeti.HTools.Backend.IAlloc > import Ganeti.HTools.Loader (Request(..), ClusterData(..)) > -import Ganeti.HTools.ExtLoader (maybeSaveData, loadExternalData) > +import Ganeti.HTools.ExtLoader (maybeSaveData, loadExternalData > + , queryAllMonDDCs) > import Ganeti.Utils > > -- | Options list and functions. > @@ -51,6 +52,7 @@ options = > , oDataFile > , oNodeSim > , oVerbose > + , oIgnoreDyn > ] > > -- | The list of arguments supported by the program. > @@ -69,8 +71,11 @@ wrapReadRequest opts args = do > cdata <- loadExternalData opts > let Request rqt _ = r1 > return $ Request rqt cdata > - else return r1 > - > + else do > + let Request rqt cdata = r1 > + cdata' <- > + if optIgnoreDynu opts then return cdata else queryAllMonDDCs cdata > + return $ Request rqt cdata' > > -- | Main function. > main :: Options -> [String] -> IO () > -- > 1.7.10.4 > > > 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
