On Mon, Feb 04, 2013 at 10:44:42AM +0100, Michele Tartara wrote:
> In order to fetch information about the uptime of the VMs running in Xen,
> we need to analyze the output of the "xm uptime" command.
>
> This commit adds the parser to do that, and its tests.
>
> Signed-off-by: Michele Tartara <[email protected]>
> ---
> Makefile.am | 3 +
> src/Ganeti/Hypervisor/Xen/Types.hs | 13 ++++
> src/Ganeti/Hypervisor/Xen/XmUptimeParser.hs | 52 +++++++++++++
> test/data/xen-xm-uptime-3.0.txt | 3 +
> .../Test/Ganeti/Hypervisor/Xen/XmUptimeParser.hs | 88
> ++++++++++++++++++++++
> test/hs/htest.hs | 2 +
> 6 files changed, 161 insertions(+)
> create mode 100644 src/Ganeti/Hypervisor/Xen/XmUptimeParser.hs
> create mode 100644 test/data/xen-xm-uptime-3.0.txt
> create mode 100644 test/hs/Test/Ganeti/Hypervisor/Xen/XmUptimeParser.hs
>
> diff --git a/Makefile.am b/Makefile.am
> index 9af1f87..f98802a 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -521,6 +521,7 @@ HS_LIB_SRCS = \
> src/Ganeti/HTools/Program/Main.hs \
> src/Ganeti/HTools/Types.hs \
> src/Ganeti/Hypervisor/Xen/XmListParser.hs \
> + src/Ganeti/Hypervisor/Xen/XmUptimeParser.hs \
> src/Ganeti/Hypervisor/Xen/Types.hs \
> src/Ganeti/Hash.hs \
> src/Ganeti/JQueue.hs \
> @@ -572,6 +573,7 @@ HS_TEST_SRCS = \
> test/hs/Test/Ganeti/HTools/PeerMap.hs \
> test/hs/Test/Ganeti/HTools/Types.hs \
> test/hs/Test/Ganeti/Hypervisor/Xen/XmListParser.hs \
> + test/hs/Test/Ganeti/Hypervisor/Xen/XmUptimeParser.hs \
> test/hs/Test/Ganeti/JSON.hs \
> test/hs/Test/Ganeti/Jobs.hs \
> test/hs/Test/Ganeti/JQueue.hs \
> @@ -1053,6 +1055,7 @@ TEST_FILES = \
> test/data/xen-xm-list-4.0.1-dom0-only.txt \
> test/data/xen-xm-list-4.0.1-four-instances.txt \
> test/data/xen-xm-list-long-3.0.txt \
> + test/data/xen-xm-uptime-3.0.txt \
> test/py/ganeti-cli.test \
> test/py/gnt-cli.test \
> test/py/import-export_unittest-helper
> diff --git a/src/Ganeti/Hypervisor/Xen/Types.hs
> b/src/Ganeti/Hypervisor/Xen/Types.hs
> index 3b10b30..b2e5745 100644
> --- a/src/Ganeti/Hypervisor/Xen/Types.hs
> +++ b/src/Ganeti/Hypervisor/Xen/Types.hs
> @@ -27,8 +27,11 @@ module Ganeti.Hypervisor.Xen.Types
> , Domain(..)
> , FromConfig(..)
> , isAlmostEqual
> + , UptimeInfo(..)
> ) where
>
> +import Text.Printf
> +
> import Ganeti.BasicTypes
> import Ganeti.Objects
>
> @@ -104,3 +107,13 @@ instance FromConfig [Config] where
> fromConfig c =
> Bad $ "Unable to extract a List from this configuration: "
> ++ show c
> +
> +-- Data type representing the information that can be obtained from "xm
> uptime"
> +data UptimeInfo = UptimeInfo
> + { uInfoName :: String
> + , uInfoID :: Int
> + , uInfoUptime :: String
> + } deriving (Eq)
> +
> +instance Show UptimeInfo where
> + show (UptimeInfo name idNum uptime) = printf "%s\t%d\t%s" name idNum uptime
Again, does this need to be Show or do you simply want a pretty-print
format?
> diff --git a/src/Ganeti/Hypervisor/Xen/XmUptimeParser.hs
> b/src/Ganeti/Hypervisor/Xen/XmUptimeParser.hs
> new file mode 100644
> index 0000000..bf2e8f7
> --- /dev/null
> +++ b/src/Ganeti/Hypervisor/Xen/XmUptimeParser.hs
> @@ -0,0 +1,52 @@
I'm a bit… not convinced here that one-xm-command-per-file model is
good. This file, for example, has only 52 lines, which IMHO is too short
to warrant a new module.
Wouldn't it make more sense to keep all xm-related functionality
(remember that xl will change things yet again, I think) into a single
Xm.hs file?
> diff --git a/test/hs/Test/Ganeti/Hypervisor/Xen/XmUptimeParser.hs
> b/test/hs/Test/Ganeti/Hypervisor/Xen/XmUptimeParser.hs
> new file mode 100644
> index 0000000..71e5aad
> --- /dev/null
> +++ b/test/hs/Test/Ganeti/Hypervisor/Xen/XmUptimeParser.hs
> @@ -0,0 +1,88 @@
> +{-# LANGUAGE TemplateHaskell #-}
> +{-# OPTIONS_GHC -fno-warn-orphans #-}
> +
> +{-| Unittests for "xm list --long" parser -}
:)
> +
> +{-
> +
> +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 Test.Ganeti.Hypervisor.Xen.XmUptimeParser
> (testHypervisor_Xen_XmUptimeParser) where
> +
> +import Test.HUnit
> +import Test.QuickCheck as QuickCheck hiding (Result)
> +
> +import Test.Ganeti.TestHelper
> +import Test.Ganeti.TestCommon
> +
> +import qualified Data.Attoparsec.Text as A
> +import qualified Data.Map as Map
> +import Data.Text (pack)
> +import Text.Printf
> +
> +import Ganeti.Hypervisor.Xen.Types
> +import Ganeti.Hypervisor.Xen.XmUptimeParser
> +
> +{-# ANN module "HLint: ignore Use camelCase" #-}
> +
> +-- | Generates an arbitrary "xm uptime" output
> +instance Arbitrary UptimeInfo where
> + arbitrary = do
> + name <- genFQDN
> + idNum <- (arbitrary :: Gen Int) `suchThat` (>= 0)
> + days <- (arbitrary :: Gen Int) `suchThat` (>= 0)
You can/should use the NonNegative instances here:
NonNegative idNum <- arbitrary
NonNegative days <- arbitrary
> + hours <- choose (0, 23) :: Gen Int
> + mins <- choose (0, 59) :: Gen Int
> + secs <- choose (0, 59) :: Gen Int
> + let uptime :: String
> + uptime =
> + if days == 0
> + then printf "%d days, %d:%d:%d" days hours mins secs
> + else printf "%d:%d:%d" hours mins secs
Wrong/reverted if test? if days ≠ 0 … ?
> + return $ UptimeInfo name idNum uptime
thanks,
iustin