On Mon, Oct 14, 2013 at 10:25:31AM +0200, Hrvoje Ribicic wrote:
> The instance queries have a field called console which requires an
> extra RPC call. This commit allows extra parameters to be passed on
> to the query logic, which allows us to determine when to make the
> extra call.
>
> Signed-off-by: Hrvoje Ribicic <[email protected]>
> ---
> src/Ganeti/Query/Instance.hs | 21 ++++++++++++++-------
> src/Ganeti/Query/Query.hs | 31 ++++++++++++++++++++-----------
> 2 files changed, 34 insertions(+), 18 deletions(-)
>
> diff --git a/src/Ganeti/Query/Instance.hs b/src/Ganeti/Query/Instance.hs
> index 6dc74f2..0651442 100644
> --- a/src/Ganeti/Query/Instance.hs
> +++ b/src/Ganeti/Query/Instance.hs
> @@ -819,20 +819,27 @@ getHypervisorSpecs cfg instances =
> in zip hvs . map ((Map.!) hvParamMap . hypervisorToRaw) $ hvs
>
> -- | Collect live data from RPC query if enabled.
> -collectLiveData :: Bool -> ConfigData -> [Instance] -> IO [(Instance,
> Runtime)]
> -collectLiveData False _ instances =
> +collectLiveData :: Bool -- ^ Live queries allowed
> + -> ConfigData -- ^ The cluster config
> + -> [String] -- ^ The requested fields
> + -> [Instance] -- ^ The instance objects
> + -> IO [(Instance, Runtime)]
> +collectLiveData False _ _ instances =
> return $ zip instances
> (repeat (Left $ RpcResultError "Live data disabled"))
> -collectLiveData True cfg instances = do
> +collectLiveData True cfg fields instances = do
> let hvSpecs = getHypervisorSpecs cfg instances
> instanceNodes = nub . justOk $
> map (getNode cfg . instPrimaryNode) instances
> goodNodes = nodesWithValidConfig cfg instanceNodes
> instInfoRes <- executeRpcCall goodNodes
> (RpcCallAllInstancesInfo hvSpecs)
> - consInfoRes <- case getAllConsoleParams cfg instances of
> - Bad _ -> return . zip goodNodes . repeat . Left $ RpcResultError
> - "Cannot construct parameters for console info call"
> - Ok p -> executeRpcCalls $ consoleParamsToCalls p
> + consInfoRes <-
> + if "console" `elem` fields
> + then case getAllConsoleParams cfg instances of
> + Bad _ -> return . zip goodNodes . repeat . Left $ RpcResultError
> + "Cannot construct parameters for console info call"
> + Ok p -> executeRpcCalls $ consoleParamsToCalls p
> + else return [] -- The information is not necessary
> return . zip instances .
> map (extractLiveInfo instInfoRes consInfoRes) $ instances
> diff --git a/src/Ganeti/Query/Query.hs b/src/Ganeti/Query/Query.hs
> index 2752fd4..758aee1 100644
> --- a/src/Ganeti/Query/Query.hs
> +++ b/src/Ganeti/Query/Query.hs
> @@ -80,6 +80,12 @@ import Ganeti.Path
> import Ganeti.Types
> import Ganeti.Utils
>
> +
Extra empty line.
> +-- | Collector type
> +data CollectorType a b =
> + CollectorSimple (Bool -> ConfigData -> [a] -> IO [(a, b)]) |
> + CollectorFieldAware (Bool -> ConfigData -> [String] -> [a] -> IO [(a, b)])
Just to share my personal preference, I like to tie the constructor
names to the type name. I especially like the way Template Haskell
does it, for example,
data Exp
= ConE
| LitE
| VarE
| ...
Anyway, leave the choice of names up to you. In any case,
indentation:
data ...
= ...
| ...
> +
> -- * Helper functions
>
> -- | Builds an unknown field definition.
> @@ -155,7 +161,7 @@ getRequestedJobIDs qfilter =
> -- | Generic query implementation for resources that are backed by
> -- some configuration objects.
> genericQuery :: FieldMap a b -- ^ Field map
> - -> (Bool -> ConfigData -> [a] -> IO [(a, b)]) -- ^ Collector
> + -> CollectorType a b -- ^ Collector
Saying that the 'FieldMap' is the 'Field map' and the 'CollectorType'
is the 'Collector' isn't very helpful.
> -> (a -> String) -- ^ Object to name function
> -> (ConfigData -> Container a) -- ^ Get all objects from config
> -> (ConfigData -> String -> ErrorResult a) -- ^ Lookup object
> @@ -181,7 +187,9 @@ genericQuery fieldsMap collector nameFn configFn getFn cfg
> fobjects <- resultT $ filterM (\n -> evaluateFilter cfg Nothing n cfilter)
> objects
> -- here run the runtime data gathering...
This comment is weird. Why not just 'Run the runtime data gathering' ?
> - runtimes <- lift $ collector live' cfg fobjects
> + runtimes <- case collector of
> + CollectorSimple collFn -> lift $ collFn live' cfg fobjects
> + CollectorFieldAware collFn -> lift $ collFn live' cfg fields fobjects
> -- ... then filter again the results, based on gathered runtime data
Weird comment again. 'Filter again the results, ...'
Thanks,
Jose
> let fdata = map (\(obj, runtime) ->
> map (execGetter cfg runtime obj) fgetters)
> @@ -205,25 +213,26 @@ queryInner :: ConfigData -- ^ The current
> configuration
> -> IO (ErrorResult QueryResult) -- ^ Result
>
> queryInner cfg live (Query (ItemTypeOpCode QRNode) fields qfilter) wanted =
> - genericQuery Node.fieldsMap Node.collectLiveData nodeName configNodes
> getNode
> - cfg live fields qfilter wanted
> + genericQuery Node.fieldsMap (CollectorSimple Node.collectLiveData) nodeName
> + configNodes getNode cfg live fields qfilter wanted
>
> queryInner cfg live (Query (ItemTypeOpCode QRInstance) fields qfilter)
> wanted =
> - genericQuery Instance.fieldsMap Instance.collectLiveData instName
> - configInstances getInstance cfg live fields qfilter wanted
> + genericQuery Instance.fieldsMap (CollectorFieldAware
> Instance.collectLiveData)
> + instName configInstances getInstance cfg live fields qfilter
> + wanted
>
> queryInner cfg live (Query (ItemTypeOpCode QRGroup) fields qfilter) wanted =
> - genericQuery Group.fieldsMap Group.collectLiveData groupName
> configNodegroups
> - getGroup cfg live fields qfilter wanted
> + genericQuery Group.fieldsMap (CollectorSimple Group.collectLiveData)
> groupName
> + configNodegroups getGroup cfg live fields qfilter wanted
>
> queryInner cfg live (Query (ItemTypeOpCode QRNetwork) fields qfilter) wanted
> =
> - genericQuery Network.fieldsMap Network.collectLiveData
> + genericQuery Network.fieldsMap (CollectorSimple Network.collectLiveData)
> (fromNonEmpty . networkName)
> configNetworks getNetwork cfg live fields qfilter wanted
>
> queryInner cfg live (Query (ItemTypeOpCode QRExport) fields qfilter) wanted =
> - genericQuery Export.fieldsMap Export.collectLiveData nodeName configNodes
> - getNode cfg live fields qfilter wanted
> + genericQuery Export.fieldsMap (CollectorSimple Export.collectLiveData)
> + nodeName configNodes getNode cfg live fields qfilter wanted
>
> queryInner _ _ (Query qkind _ _) _ =
> return . Bad . GenericError $ "Query '" ++ show qkind ++ "' not supported"
> --
> 1.8.4
>
--
Jose Antonio Lopes
Ganeti Engineering
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
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370
