On Mon, Mar 11, 2013 at 05:06:17PM +0100, Michele Tartara wrote:
> On Mon, Mar 11, 2013 at 4:34 PM, Iustin Pop <[email protected]> wrote:
>
> > On Mon, Mar 11, 2013 at 02:10:03PM +0100, Michele Tartara wrote:
> > > This commit adds the core infrastructure of the monitoring daemon,
> > > and integrates it in the build and test systems.
> > >
> > > The actual functionality of the monitoring daemon is still completely
> > missing.
> > >
> > > Signed-off-by: Michele Tartara <[email protected]>
> > > ---
> > > .gitignore | 1 +
> > > Makefile.am | 18 ++++++++++---
> > > configure.ac | 12 +++++----
> > > daemons/daemon-util.in | 12 +++++++++
> > > lib/constants.py | 2 ++
> > > man/ganeti-mond.rst | 13 +++++++++
> > > src/Ganeti/Monitoring/Server.hs | 57
> > +++++++++++++++++++++++++++++++++++++++
> > > src/ganeti-mond.hs | 47 ++++++++++++++++++++++++++++++++
> > > test/py/daemon-util_unittest.bash | 23 +++++++++++++---
> > > 9 files changed, 173 insertions(+), 12 deletions(-)
> > > create mode 100644 man/ganeti-mond.rst
> > > create mode 100644 src/Ganeti/Monitoring/Server.hs
> > > create mode 100644 src/ganeti-mond.hs
> > > @@ -205,7 +207,11 @@ HS_GENERATED_FILES =
> > > if WANT_HTOOLS
> > > HS_GENERATED_FILES += $(HS_PROGS)
> > > if ENABLE_CONFD
> > > -HS_GENERATED_FILES += src/hconfd src/ganeti-confd
> > > +HS_GENERATED_FILES += src/hconfd src/ganeti-confd
> >
> > Please don't ruin whitespace, here…
> >
> > > -# Haskell programs to compiled but not installed automatically
> > > -# Usually they have their own specific installation rules
> > > +# Haskell programs to be compiled by "make really-all"
> > > HS_COMPILE_PROGS= \
> > > + src/ganeti-mond \
> > > src/hconfd \
> > > src/rpc-test
> >
> > And here.
> >
>
> Both are whitespace/tabs problems. I didn't see them in vi, they just
> happend to be aligned correctly. I'll fix them.
Yes, but at least the first one you should have seen - the line
shouldn't have been modified in your diff, so its simple presence should
have warned you.
> > > @@ -529,6 +535,7 @@ HS_LIB_SRCS = \
> > > src/Ganeti/Jobs.hs \
> > > src/Ganeti/Logging.hs \
> > > src/Ganeti/Luxi.hs \
> > > + src/Ganeti/Monitoring/Server.hs \
> > > src/Ganeti/Network.hs \
> > > src/Ganeti/Objects.hs \
> > > src/Ganeti/OpCodes.hs \
> > > @@ -827,6 +834,10 @@ src/ganeti-confd: src/hconfd
> > > nodist_sbin_SCRIPTS += src/ganeti-confd
> > > endif
> > >
> > > +if ENABLE_MOND
> > > +nodist_sbin_SCRIPTS += src/ganeti-mond
> > > +endif
> > > +
> > > python_scripts = \
> > > tools/cfgshell \
> > > tools/cfgupgrade \
> > > @@ -932,6 +943,7 @@ man_MANS = \
> > > man/ganeti-confd.8 \
> > > man/ganeti-listrunner.8 \
> > > man/ganeti-masterd.8 \
> > > + man/ganeti-mond.8 \
> > > man/ganeti-noded.8 \
> > > man/ganeti-os-interface.7 \
> > > man/ganeti-extstorage-interface.7 \
> > > diff --git a/configure.ac b/configure.ac
> > > index ee2f524..a4cd78b 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -484,7 +484,7 @@ AC_ARG_ENABLE([confd],
> > > ENABLE_MOND=
> > > AC_ARG_ENABLE([mond],
> > > [AS_HELP_STRING([--enable-mond],
> > > - [enable the ganeti monitoring agent (default: check)])],
> > > + [enable the ganeti monitoring daemon (default: check)])],
> > > [],
> > > [enable_mond=check])
> > >
> > > @@ -558,17 +558,19 @@ fi
> > > AC_SUBST(ENABLE_CONFD, $has_confd)
> > > AM_CONDITIONAL([ENABLE_CONFD], [test x$has_confd = xTrue])
> > >
> > > -#extra modules for monitoring agent functionality
> > > +#extra modules for monitoring daemon functionality
> > > has_monitoring=False
> > > -if test "$enable_monitoring" != no; then
> > > +if test "$enable_mond" != no; then
> >
> > Hmm, I don't know if we should or not rename this. The flag is for the
> > entire monitoring, no matter how it's implemented (agent, monitor
> > daemon, etc.). Plus, it requires changes to build systems.
> >
> > Thoughts?
> >
>
> Sticking to "enable_monitoring" is fine for me. I was changing it to mond
> for improving the uniformity w.r.t. confd, but actually this is already in
> 2.7, so it's better not to change it. Plus, as you point out, it's for the
> entire monitoring, not just for the daemon.
>
>
> >
> > > MONITORING_PKG=
> > > AC_GHC_PKG_CHECK([attoparsec], [],
> > > [MONITORING_PKG="$MONITORING_PKG attoparsec"])
> > > + AC_GHC_PKG_CHECK([snap-server], [],
> > > + [MONITORING_PKG="$MONITORING_PKG snap-server"])
> > > if test -z "$MONITORING_PKG"; then
> > > has_monitoring=True
> > > elif test "$enable_monitoring" = check; then
> > > AC_MSG_WARN(m4_normalize([The required extra libraries for the
> > monitoring
> > > - agent were not found ($MONITORING_PKG),
> > > + daemon were not found ($MONITORING_PKG),
> > > monitoring disabled]))
> > > else
> > > AC_MSG_FAILURE(m4_normalize([The monitoring functionality was
> > requested, but
> > > @@ -577,7 +579,7 @@ if test "$enable_monitoring" != no; then
> > > fi
> > > fi
> > > if test "$has_monitoring" = True; then
> > > - AC_MSG_NOTICE([Enabling the monitoring agent usage])
> > > + AC_MSG_NOTICE([Enabling the monitoring daemon usage])
> > > fi
> > > AC_SUBST(ENABLE_MOND, $has_monitoring)
> > > AM_CONDITIONAL([ENABLE_MOND], [test "$has_monitoring" = True])
> > > diff --git a/daemons/daemon-util.in b/daemons/daemon-util.in
> > > index 786c4b0..fbabdfe 100644
> > > --- a/daemons/daemon-util.in
> > > +++ b/daemons/daemon-util.in
> > > @@ -41,10 +41,19 @@ if _confd_enabled; then
> > > DAEMONS+=( ganeti-confd )
> > > fi
> > >
> > > +_mond_enabled() {
> > > + [[ "@CUSTOM_ENABLE_MOND@" == True ]]
> > > +}
> > > +
> > > +if _mond_enabled; then
> > > + DAEMONS+=( ganeti-mond )
> > > +fi
> > > +
> > > NODED_ARGS=
> > > MASTERD_ARGS=
> > > CONFD_ARGS=
> > > RAPI_ARGS=
> > > +MOND_ARGS=
> > >
> > > # Read defaults file if it exists
> > > if [[ -s $defaults_file ]]; then
> > > @@ -79,6 +88,9 @@ _daemon_usergroup() {
> > > noded)
> > > echo "@GNTNODEDUSER@:@GNTDAEMONSGROUP@"
> > > ;;
> > > + mond)
> > > + echo "@GNTMONDUSER@:@GNTMONDGROUP@"
> > > + ;;
> > > *)
> > > echo "root:@GNTDAEMONSGROUP@"
> > > ;;
> > > diff --git a/lib/constants.py b/lib/constants.py
> > > index 979d1ba..eb1c414 100644
> > > --- a/lib/constants.py
> > > +++ b/lib/constants.py
> > > @@ -171,12 +171,14 @@ DAEMONS_PORTS = {
> > > # daemon-name: ("proto", "default-port")
> > > NODED: ("tcp", 1811),
> > > CONFD: ("udp", 1814),
> > > + MOND: ("tcp", 1815),
> > > RAPI: ("tcp", 5080),
> > > SSH: ("tcp", 22),
> > > }
> > >
> > > DEFAULT_NODED_PORT = DAEMONS_PORTS[NODED][1]
> > > DEFAULT_CONFD_PORT = DAEMONS_PORTS[CONFD][1]
> > > +DEFAULT_MOND_PORT = DAEMONS_PORTS[MOND][1]
> > > DEFAULT_RAPI_PORT = DAEMONS_PORTS[RAPI][1]
> > >
> > > FIRST_DRBD_PORT = 11000
> > > diff --git a/man/ganeti-mond.rst b/man/ganeti-mond.rst
> > > new file mode 100644
> > > index 0000000..c7006d8
> > > --- /dev/null
> > > +++ b/man/ganeti-mond.rst
> > > @@ -0,0 +1,13 @@
> > > +ganeti-mond(8) Ganeti | Version @GANETI_VERSION@
> > > +=================================================
> > > +
> > > +Name
> > > +----
> > > +
> > > +ganeti-mond - Ganeti monitoring agent
> > > +
> > > +.. vim: set textwidth=72 :
> > > +.. Local Variables:
> > > +.. mode: rst
> > > +.. fill-column: 72
> > > +.. End:
> > > diff --git a/src/Ganeti/Monitoring/Server.hs
> > b/src/Ganeti/Monitoring/Server.hs
> > > new file mode 100644
> > > index 0000000..2448d8b
> > > --- /dev/null
> > > +++ b/src/Ganeti/Monitoring/Server.hs
> > > @@ -0,0 +1,57 @@
> > > +{-# LANGUAGE OverloadedStrings #-}
> > > +
> > > +{-| Implementation of the Ganeti confd server functionality.
> > > +
> > > +-}
> > > +
> > > +{-
> > > +
> > > +Copyright (C) 2013 Google Inc.
> > > +
> > > +This program is free software; you can redistribute it and/or modify
> > > +it under the terms of the GNU General Public License as published by
> > > +the Free Software Foundation; either version 2 of the License, or
> > > +(at your option) any later version.
> > > +
> > > +This program is distributed in the hope that it will be useful, but
> > > +WITHOUT ANY WARRANTY; without even the implied warranty of
> > > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > > +General Public License for more details.
> > > +
> > > +You should have received a copy of the GNU General Public License
> > > +along with this program; if not, write to the Free Software
> > > +Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> > > +02110-1301, USA.
> > > +
> > > +-}
> > > +
> > > +module Ganeti.Monitoring.Server
> > > + ( main
> > > + , checkMain
> > > + , prepMain
> > > + ) where
> > > +
> > > +import Ganeti.Daemon
> > > +
> > > +-- * Types and constants definitions
> > > +
> > > +-- | Type alias for checkMain results.
> > > +type CheckResult = ()
> > > +
> > > +-- | Type alias for prepMain results.
> > > +type PrepResult = ()
> > > +
> > > +-- * Helper functions
> > > +
> > > +-- | Check function for the monitoring agent.
> > > +checkMain :: CheckFn CheckResult
> > > +checkMain _ = return $ Right ()
> > > +
> > > +-- | Prepare function for monitoring agent.
> > > +prepMain :: PrepFn CheckResult PrepResult
> > > +prepMain _ _ = return ()
> > > +
> > > +-- | Main function.
> > > +main :: MainFn CheckResult PrepResult
> > > +main _ _ _ =
> > > + return ()
> > > diff --git a/src/ganeti-mond.hs b/src/ganeti-mond.hs
> > > new file mode 100644
> > > index 0000000..d0fab0c
> > > --- /dev/null
> > > +++ b/src/ganeti-mond.hs
> > > @@ -0,0 +1,47 @@
> > > +{-| Ganeti monitoring agent daemon
> > > +
> > > +-}
> > > +
> > > +{-
> > > +
> > > +Copyright (C) 2013 Google Inc.
> > > +
> > > +This program is free software; you can redistribute it and/or modify
> > > +it under the terms of the GNU General Public License as published by
> > > +the Free Software Foundation; either version 2 of the License, or
> > > +(at your option) any later version.
> > > +
> > > +This program is distributed in the hope that it will be useful, but
> > > +WITHOUT ANY WARRANTY; without even the implied warranty of
> > > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > > +General Public License for more details.
> > > +
> > > +You should have received a copy of the GNU General Public License
> > > +along with this program; if not, write to the Free Software
> > > +Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> > > +02110-1301, USA.
> > > +
> > > +-}
> > > +
> > > +module Main (main) where
> > > +
> > > +import qualified Ganeti.Monitoring.Server
> > > +import Ganeti.Daemon
> > > +import Ganeti.Runtime
> > > +import qualified Ganeti.Constants as C
> > > +
> > > +-- | Options list and functions.
> > > +options :: [OptType]
> > > +options =
> > > + [ oNoDaemonize
> > > + , oNoUserChecks
> > > + , oDebug
> > > + , oPort C.defaultMondPort
> > > + ]
> > > +
> > > +-- | Main function.
> > > +main :: IO ()
> > > +main = genericMain GanetiMond options
> > > + Ganeti.Monitoring.Server.checkMain
> > > + Ganeti.Monitoring.Server.prepMain
> > > + Ganeti.Monitoring.Server.main
> >
> > Please indent genericMain to the next line. Again, only use the initial
> > line when all the code fits on a single line.
> >
>
> I'll do it. But this snippet is an exact copy (including spaces and
> newlines) of the existing code of src/hconfd.hs.
>
> Should I change that one as well?
Yes please.
> > Also, why do you import G.M.S qualified?
This as well, if it's the same.
> > > diff --git a/test/py/daemon-util_unittest.bash
> > b/test/py/daemon-util_unittest.bash
> > > index aa3eded..a2653b0 100755
> > > --- a/test/py/daemon-util_unittest.bash
> > > +++ b/test/py/daemon-util_unittest.bash
> > > @@ -32,12 +32,27 @@ if ! grep -q '^ENABLE_CONFD = ' lib/_autoconf.py;
> > then
> > > err "Please update $0, confd enable feature is missing"
> > > fi
> > >
> > > +if ! grep -q '^ENABLE_MOND = ' lib/_autoconf.py; then
> > > + err "Please update $0, mond enable feature is missing"
> > > +fi
> > > +
> > > +
> > > if grep -q '^ENABLE_CONFD = True' lib/_autoconf.py; then
> > > - DAEMONS="$(echo ganeti-{noded,masterd,rapi,confd})"
> > > - STOPDAEMONS="$(echo ganeti-{confd,rapi,masterd,noded})"
> > > + if grep -q '^ENABLE_MOND = True' lib/_autoconf.py; then
> > > + DAEMONS="$(echo ganeti-{noded,masterd,rapi,confd,mond})"
> > > + STOPDAEMONS="$(echo ganeti-{mond,confd,rapi,masterd,noded})"
> > > + else
> > > + DAEMONS="$(echo ganeti-{noded,masterd,rapi,confd})"
> > > + STOPDAEMONS="$(echo ganeti-{confd,rapi,masterd,noded})"
> > > + fi
> > > else
> > > - DAEMONS="$(echo ganeti-{noded,masterd,rapi})"
> > > - STOPDAEMONS="$(echo ganeti-{rapi,masterd,noded})"
> > > + if grep -q '^ENABLE_MOND = True' lib/_autoconf.py; then
> > > + DAEMONS="$(echo ganeti-{noded,masterd,rapi,mond})"
> > > + STOPDAEMONS="$(echo ganeti-{mond,rapi,masterd,noded})"
> > > + else
> > > + DAEMONS="$(echo ganeti-{noded,masterd,rapi})"
> > > + STOPDAEMONS="$(echo ganeti-{rapi,masterd,noded})"
> > > + fi
> >
> > Wow, this is very ugly code (not your patch, but well your patch extends
> > it).
> >
> >
> I agree. Should I try to refactor it all in some way?
I would appreciate it, yes.
thanks!
iustin
