On Mon, Sep 16, 2013 at 11:05 AM, Jose A. Lopes <[email protected]>wrote:
> Haskell constants 'daemonsExtraLogfilesGanetiMondAccess' and > 'daemonsExtraLogfilesGanetiMondError' cannot be constants because > their Python counterparts are calculated through > 'pathutils.GetLogFilename', which indirectly depends on the > environment variable 'GANETI_ROOTDIR', as part of the virtual cluster > configuration. Instead, these paths must be computed at runtime, as > opposed to being computed at compile time through the Python to > Haskell constant generation, and must also depend on the same > environment variable. Fixes issue 575. > > Signed-off-by: Jose A. Lopes <[email protected]> > --- > src/Ganeti/Monitoring/Server.hs | 23 +++++++++++++++-------- > src/Ganeti/Runtime.hs | 20 ++++++++++++++++++++ > 2 files changed, 35 insertions(+), 8 deletions(-) > > diff --git a/src/Ganeti/Monitoring/Server.hs > b/src/Ganeti/Monitoring/Server.hs > index 0db1614..6c44936 100644 > --- a/src/Ganeti/Monitoring/Server.hs > +++ b/src/Ganeti/Monitoring/Server.hs > @@ -51,6 +51,7 @@ import qualified Ganeti.DataCollectors.InstStatus as > InstStatus > import qualified Ganeti.DataCollectors.Lv as Lv > import Ganeti.DataCollectors.Types > import qualified Ganeti.Constants as C > +import Ganeti.Runtime > > -- * Types and constants definitions > > @@ -99,11 +100,11 @@ collectors = > -- * Configuration handling > > -- | The default configuration for the HTTP server. > -defaultHttpConf :: Config Snap () > -defaultHttpConf = > - setAccessLog (ConfigFileLog C.daemonsExtraLogfilesGanetiMondAccess) . > +defaultHttpConf :: FilePath -> FilePath -> Config Snap () > +defaultHttpConf accessLog errorLog = > + setAccessLog (ConfigFileLog accessLog) . > setCompression False . > - setErrorLog (ConfigFileLog C.daemonsExtraLogfilesGanetiMondError) $ > + setErrorLog (ConfigFileLog errorLog) $ > setVerbose False > emptyConfig > > @@ -115,10 +116,16 @@ checkMain _ = return $ Right () > > -- | Prepare function for monitoring agent. > prepMain :: PrepFn CheckResult PrepResult > -prepMain opts _ = > - return $ > - setPort (maybe C.defaultMondPort fromIntegral (optPort opts)) > - defaultHttpConf > +prepMain opts _ = do > + mAccessLog <- daemonsExtraLogFile GanetiMond AccessLogReason > + mErrorLog <- daemonsExtraLogFile GanetiMond ErrorLogReason > + case (mAccessLog, mErrorLog) of > + (Just accessLog, Just errorLog) -> > + return $ > + setPort > + (maybe C.defaultMondPort fromIntegral (optPort opts)) > + (defaultHttpConf accessLog errorLog) > + _ -> fail "Failed to retrieve extra log filepaths for the monitoring > daemon" > > -- * Query answers > > diff --git a/src/Ganeti/Runtime.hs b/src/Ganeti/Runtime.hs > index bf24e94..3adafa5 100644 > --- a/src/Ganeti/Runtime.hs > +++ b/src/Ganeti/Runtime.hs > @@ -32,7 +32,9 @@ module Ganeti.Runtime > , daemonOnlyOnMaster > , daemonUser > , daemonGroup > + , ExtraLogReason(..) > , daemonLogFile > + , daemonsExtraLogFile > , daemonPidFile > , getEnts > , verifyDaemonUser > @@ -118,12 +120,30 @@ daemonGroup (DaemonGroup GanetiMond) = C.mondGroup > daemonGroup (ExtraGroup DaemonsGroup) = C.daemonsGroup > daemonGroup (ExtraGroup AdminGroup) = C.adminGroup > > +data ExtraLogReason = AccessLogReason | ErrorLogReason > nitpicking: ExtraLogReason is ok, but I think AccessLog and ErrorLog would be preferrable to AccessLogReason and ErrorLogReason: it sound much more natural (to me at least) to write something like: daemonsExtraLogFile GanetiMond AccessLog than daemonsExtraLogFile GanetiMond AccessLogReason What do you think? + > +daemonsExtraLogbase :: GanetiDaemon -> ExtraLogReason -> Maybe String > +daemonsExtraLogbase GanetiMond AccessLogReason = > + Just C.daemonsExtraLogbaseGanetiMondAccess > + > +daemonsExtraLogbase GanetiMond ErrorLogReason = > + Just C.daemonsExtraLogbaseGanetiMondError > + > +daemonsExtraLogbase _ _ = Nothing > + > -- | Returns the log file for a daemon. > daemonLogFile :: GanetiDaemon -> IO FilePath > daemonLogFile daemon = do > logDir <- Path.logDir > return $ logDir </> daemonLogBase daemon <.> "log" > > +daemonsExtraLogFile :: GanetiDaemon -> ExtraLogReason -> IO (Maybe > FilePath) > +daemonsExtraLogFile daemon logreason = do > + logDir <- Path.logDir > + case daemonsExtraLogbase daemon logreason of > + Nothing -> return Nothing > + Just logbase -> return . Just $ logDir </> logbase <.> "log" > + > -- | Returns the pid file name for a daemon. > daemonPidFile :: GanetiDaemon -> IO FilePath > daemonPidFile daemon = do > -- > 1.8.4 > > Rest LGTM. Thanks, Michele -- 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
