Fix lint (remove redundant 'do's)
Interdiff:
diff --git a/src/Ganeti/Ssconf.hs b/src/Ganeti/Ssconf.hs
index b4483f9..2e8b6e8 100644
--- a/src/Ganeti/Ssconf.hs
+++ b/src/Ganeti/Ssconf.hs
@@ -154,7 +154,7 @@ getPrimaryIPFamily optpath = do
-- | Parse the nodes vm capable value from a 'String'.
parseNodesVmCapable :: String -> Result [(String, Bool)]
-parseNodesVmCapable str = do
+parseNodesVmCapable str =
forM (lines str) $ \line -> do
(key, val) <- parseKeyValue "Parsing node_vm_capable" line
val' <- tryRead "Parsing value of node_vm_capable" val
@@ -184,7 +184,7 @@ parseHypervisorList str =
-- | Read and parse the list of enabled hypervisors.
getHypervisorList :: Maybe FilePath -> IO (Result [Hypervisor])
-getHypervisorList optPath = do
+getHypervisorList optPath =
(parseHypervisorList =<<) <$>
readSSConfFile optPath Nothing SSHypervisorList
Thanks,
Jose
On Jun 10 11:17, Helga Velroyen wrote:
> LGTM, thanks (including the tests for hypervisor list)
>
>
> On Tue, Jun 10, 2014 at 10:25 AM, 'Jose A. Lopes' via ganeti-devel <
> [email protected]> wrote:
>
> > While we're at it, also add a test for hypervisor list.
> >
> > diff --git a/src/Ganeti/Ssconf.hs b/src/Ganeti/Ssconf.hs
> > index 60848ad..b4483f9 100644
> > --- a/src/Ganeti/Ssconf.hs
> > +++ b/src/Ganeti/Ssconf.hs
> > @@ -34,6 +34,7 @@ module Ganeti.Ssconf
> > , getNodesVmCapable
> > , getMasterCandidatesIps
> > , getMasterNode
> > + , parseHypervisorList
> > , getHypervisorList
> > , parseEnabledUserShutdown
> > , getEnabledUserShutdown
> > @@ -176,11 +177,16 @@ getMasterNode optPath = do
> > result <- readSSConfFile optPath Nothing SSMasterNode
> > return $ liftM rStripSpace result
> >
> > --- | Read the list of enabled hypervisors.
> > +-- | Parse the list of enabled hypervisors from a 'String'.
> > +parseHypervisorList :: String -> Result [Hypervisor]
> > +parseHypervisorList str =
> > + mapM Types.hypervisorFromRaw $ lines str
> > +
> > +-- | Read and parse the list of enabled hypervisors.
> > getHypervisorList :: Maybe FilePath -> IO (Result [Hypervisor])
> > getHypervisorList optPath = do
> > - result <- readSSConfFile optPath Nothing SSHypervisorList
> > - return $ mapM Types.hypervisorFromRaw =<< liftM lines result
> > + (parseHypervisorList =<<) <$>
> > + readSSConfFile optPath Nothing SSHypervisorList
> >
> > -- | Parse whether user shutdown is enabled from a 'String'.
> > parseEnabledUserShutdown :: String -> Result Bool
> > diff --git a/test/hs/Test/Ganeti/Ssconf.hs b/test/hs/Test/Ganeti/Ssconf.hs
> > index a305d37..325cfbc 100644
> > --- a/test/hs/Test/Ganeti/Ssconf.hs
> > +++ b/test/hs/Test/Ganeti/Ssconf.hs
> > @@ -36,6 +36,7 @@ import Data.List
> > import Test.Ganeti.TestHelper
> >
> > import qualified Ganeti.Ssconf as Ssconf
> > +import qualified Ganeti.Types as Types
> >
> > -- * Ssconf tests
> >
> > @@ -56,6 +57,12 @@ caseParseNodesVmCapable = do
> > ]
> > HUnit.assertEqual "Mismatch in parsed and expected result" expected
> > result
> >
> > +caseParseHypervisorList :: HUnit.Assertion
> > +caseParseHypervisorList = do
> > + let result = Ssconf.parseHypervisorList "kvm\nxen-pvm\nxen-hvm"
> > + expected = return [Types.Kvm, Types.XenPvm, Types.XenHvm]
> > + HUnit.assertEqual "Mismatch in parsed and expected result" expected
> > result
> > +
> > caseParseEnabledUserShutdown :: HUnit.Assertion
> > caseParseEnabledUserShutdown = do
> > let result1 = Ssconf.parseEnabledUserShutdown "True"
> > @@ -68,5 +75,6 @@ caseParseEnabledUserShutdown = do
> > testSuite "Ssconf"
> > [ 'prop_filename
> > , 'caseParseNodesVmCapable
> > + , 'caseParseHypervisorList
> > , 'caseParseEnabledUserShutdown
> >
> > On Jun 06 18:37, Hrvoje Ribicic wrote:
> > > On Fri, Jun 6, 2014 at 10:51 AM, 'Jose A. Lopes' via ganeti-devel <
> > > [email protected]> wrote:
> > >
> > > > Add list of enabled hypervisors, cluster wide parameter for user
> > > > shutdown, and a list of nodes and their respective vm capable
> > > > attributes to Ssconf. This will be needed by the KVM daemon to
> > > > determine whether it should start on a particular node.
> > > >
> > > > Signed-off-by: Jose A. Lopes <[email protected]>
> > > > ---
> > > > lib/config.py | 5 +++++
> > > > lib/ssconf.py | 22 ++++++++++++++++++++++
> > > > src/Ganeti/Constants.hs | 6 ++++++
> > > > src/Ganeti/Ssconf.hs | 40 +++++++++++++++++++++++++++++++++++++++-
> > > > src/Ganeti/Types.hs | 1 +
> > > > 5 files changed, 73 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/lib/config.py b/lib/config.py
> > > > index dd38a22..6ea2083 100644
> > > > --- a/lib/config.py
> > > > +++ b/lib/config.py
> > > > @@ -2614,6 +2614,8 @@ class ConfigWriter(object):
> > > > for ninfo in node_infos]
> > > > node_snd_ips = ["%s %s" % (ninfo.name, ninfo.secondary_ip)
> > > > for ninfo in node_infos]
> > > > + node_vm_capable = ["%s=%s" % (ninfo.name, str(ninfo.vm_capable))
> > > > + for ninfo in node_infos]
> > > >
> > > > instance_data = fn(instance_names)
> > > > off_data = fn(node.name for node in node_infos if node.offline)
> > > > @@ -2624,6 +2626,7 @@ class ConfigWriter(object):
> > > > node_data = fn(node_names)
> > > > node_pri_ips_data = fn(node_pri_ips)
> > > > node_snd_ips_data = fn(node_snd_ips)
> > > > + node_vm_capable_data = fn(node_vm_capable)
> > > >
> > > > cluster = self._config_data.cluster
> > > > cluster_tags = fn(cluster.GetTags())
> > > > @@ -2660,6 +2663,7 @@ class ConfigWriter(object):
> > > > constants.SS_NODE_LIST: node_data,
> > > > constants.SS_NODE_PRIMARY_IPS: node_pri_ips_data,
> > > > constants.SS_NODE_SECONDARY_IPS: node_snd_ips_data,
> > > > + constants.SS_NODE_VM_CAPABLE: node_vm_capable_data,
> > > > constants.SS_OFFLINE_NODES: off_data,
> > > > constants.SS_ONLINE_NODES: on_data,
> > > > constants.SS_PRIMARY_IP_FAMILY: str(cluster.primary_ip_family),
> > > > @@ -2670,6 +2674,7 @@ class ConfigWriter(object):
> > > > constants.SS_UID_POOL: uid_pool,
> > > > constants.SS_NODEGROUPS: nodegroups_data,
> > > > constants.SS_NETWORKS: networks_data,
> > > > + constants.SS_ENABLED_USER_SHUTDOWN:
> > > > str(cluster.enabled_user_shutdown),
> > > > }
> > > > ssconf_values = self._ExtendByAllHvparamsStrings(ssconf_values,
> > > > all_hvparams)
> > > > diff --git a/lib/ssconf.py b/lib/ssconf.py
> > > > index a2a660f..55f9df1 100644
> > > > --- a/lib/ssconf.py
> > > > +++ b/lib/ssconf.py
> > > > @@ -57,6 +57,7 @@ _VALID_KEYS = compat.UniqueFrozenset([
> > > > constants.SS_NODE_LIST,
> > > > constants.SS_NODE_PRIMARY_IPS,
> > > > constants.SS_NODE_SECONDARY_IPS,
> > > > + constants.SS_NODE_VM_CAPABLE,
> > > > constants.SS_OFFLINE_NODES,
> > > > constants.SS_ONLINE_NODES,
> > > > constants.SS_PRIMARY_IP_FAMILY,
> > > > @@ -73,6 +74,7 @@ _VALID_KEYS = compat.UniqueFrozenset([
> > > > constants.SS_HVPARAMS_XEN_KVM,
> > > > constants.SS_HVPARAMS_XEN_CHROOT,
> > > > constants.SS_HVPARAMS_XEN_LXC,
> > > > + constants.SS_ENABLED_USER_SHUTDOWN,
> > > > ])
> > > >
> > > > #: Maximum size for ssconf files
> > > > @@ -321,6 +323,20 @@ class SimpleStore(object):
> > > > nl = data.splitlines(False)
> > > > return nl
> > > >
> > > > + def GetNodesVmCapable(self):
> > > > + """Return the cluster nodes' vm capable value.
> > > > +
> > > > + @rtype: dict of string to bool
> > > > + @return: mapping of node names to vm capable values
> > > > +
> > > > + """
> > > > + data = self._ReadFile(constants.SS_NODE_VM_CAPABLE)
> > > > + vm_capable = {}
> > > > + for line in data.splitlines(False):
> > > > + (node_uuid, node_vm_capable) = line.split("=")
> > > > + vm_capable[node_uuid] = node_vm_capable
> > > >
> > >
> > > This will get you a str to str map that might be interpreted as all True
> > > later.
> > >
> > >
> > > > + return vm_capable
> > > > +
> > > > def GetNodegroupList(self):
> > > > """Return the list of nodegroups.
> > > >
> > > > @@ -415,6 +431,12 @@ class SimpleStore(object):
> > > > raise errors.ConfigurationError("Error while trying to parse
> > > > primary IP"
> > > > " family: %s" % err)
> > > >
> > > > + def GetEnabledUserShutdown(self):
> > > > + """Return whether user shutdown is enabled.
> > > > +
> > > > + """
> > > > + return self._ReadFile(constants.SS_ENABLED_USER_SHUTDOWN) ==
> > "True"
> > > >
> > >
> > > Why not a cast to bool here?
> > >
> > >
> > > > +
> > > >
> > > > def WriteSsconfFiles(values, dry_run=False):
> > > > """Update all ssconf files.
> > > > diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs
> > > > index c13d6d6..c87510b 100644
> > > > --- a/src/Ganeti/Constants.hs
> > > > +++ b/src/Ganeti/Constants.hs
> > > > @@ -3570,6 +3570,9 @@ ssNodePrimaryIps = "node_primary_ips"
> > > > ssNodeSecondaryIps :: String
> > > > ssNodeSecondaryIps = "node_secondary_ips"
> > > >
> > > > +ssNodeVmCapable :: String
> > > > +ssNodeVmCapable = "node_vm_capable"
> > > > +
> > > > ssOfflineNodes :: String
> > > > ssOfflineNodes = "offline_nodes"
> > > >
> > > > @@ -3637,6 +3640,9 @@ validSsHvparamsKeys =
> > > > ssFilePerms :: Int
> > > > ssFilePerms = 0o444
> > > >
> > > > +ssEnabledUserShutdown :: String
> > > > +ssEnabledUserShutdown = "enabled_user_shutdown"
> > > > +
> > > > -- | Cluster wide default parameters
> > > > defaultEnabledHypervisor :: String
> > > > defaultEnabledHypervisor = htXenPvm
> > > > diff --git a/src/Ganeti/Ssconf.hs b/src/Ganeti/Ssconf.hs
> > > > index 3f7a64b..3704c36 100644
> > > > --- a/src/Ganeti/Ssconf.hs
> > > > +++ b/src/Ganeti/Ssconf.hs
> > > > @@ -30,8 +30,11 @@ module Ganeti.Ssconf
> > > > , sSKeyToRaw
> > > > , sSKeyFromRaw
> > > > , getPrimaryIPFamily
> > > > + , getNodesVmCapable
> > > > , getMasterCandidatesIps
> > > > , getMasterNode
> > > > + , getHypervisorList
> > > > + , getEnabledUserShutdown
> > > > , keyToFilename
> > > > , sSFilePrefix
> > > > ) where
> > > > @@ -48,6 +51,8 @@ import Ganeti.BasicTypes
> > > > import qualified Ganeti.Constants as C
> > > > import qualified Ganeti.Path as Path
> > > > import Ganeti.THH
> > > > +import Ganeti.Types (Hypervisor)
> > > > +import qualified Ganeti.Types as Types
> > > > import Ganeti.Utils
> > > >
> > > > -- | Maximum ssconf file size we support.
> > > > @@ -73,6 +78,7 @@ $(declareSADT "SSKey"
> > > > , ("SSNodeList", 'C.ssNodeList)
> > > > , ("SSNodePrimaryIps", 'C.ssNodePrimaryIps)
> > > > , ("SSNodeSecondaryIps", 'C.ssNodeSecondaryIps)
> > > > + , ("SSNodeVmCapable", 'C.ssNodeVmCapable)
> > > > , ("SSOfflineNodes", 'C.ssOfflineNodes)
> > > > , ("SSOnlineNodes", 'C.ssOnlineNodes)
> > > > , ("SSPrimaryIpFamily", 'C.ssPrimaryIpFamily)
> > > > @@ -82,6 +88,7 @@ $(declareSADT "SSKey"
> > > > , ("SSMaintainNodeHealth", 'C.ssMaintainNodeHealth)
> > > > , ("SSUidPool", 'C.ssUidPool)
> > > > , ("SSNodegroups", 'C.ssNodegroups)
> > > > + , ("SSEnabledUserShutdown", 'C.ssEnabledUserShutdown)
> > > > ])
> > > >
> > > > -- | Convert a ssconf key into a (full) file path.
> > > > @@ -118,6 +125,14 @@ readSSConfFile optpath def key = do
> > > > keyToFilename (fromMaybe dpath optpath) $ key
> > > > return (liftM (take maxFileSize) result)
> > > >
> > > > +-- | Parses a key-value pair of the form "key=value" from 'str', fails
> > > > +-- with 'desc' otherwise.
> > > > +parseKeyValue :: Monad m => String -> String -> m (String, String)
> > > > +parseKeyValue desc str =
> > > > + case sepSplit '=' str of
> > > > + [key, value] -> return (key, value)
> > > > + _ -> fail $ "Failed to parse key-value pair for " ++ desc
> > > > +
> > > > -- | Parses a string containing an IP family
> > > > parseIPFamily :: Int -> Result Socket.Family
> > > > parseIPFamily fam | fam == AutoConf.pyAfInet4 = Ok Socket.AF_INET
> > > > @@ -133,6 +148,17 @@ getPrimaryIPFamily optpath = do
> > > > return (liftM rStripSpace result >>=
> > > > tryRead "Parsing af_family" >>= parseIPFamily)
> > > >
> > > > +-- | Read the nodes vm capable value.
> > > > +getNodesVmCapable :: Maybe FilePath -> IO (Result [(String, Bool)])
> > > > +getNodesVmCapable optPath = do
> > > > + result <- readSSConfFile optPath Nothing SSNodeVmCapable
> > > > + return $
> > > > + liftM lines result >>=
> > > > + mapM (parseKeyValue "Parsing node_vm_capable") >>=
> > > > + mapM (\(x, y) ->
> > > > + do val <- tryRead "Parsing value of node_vm_capable" y
> > > > + return (x, val))
> > > > +
> > > > -- | Read the list of IP addresses of the master candidates of the
> > > > cluster.
> > > > getMasterCandidatesIps :: Maybe FilePath -> IO (Result [String])
> > > > getMasterCandidatesIps optPath = do
> > > > @@ -143,4 +169,16 @@ getMasterCandidatesIps optPath = do
> > > > getMasterNode :: Maybe FilePath -> IO (Result String)
> > > > getMasterNode optPath = do
> > > > result <- readSSConfFile optPath Nothing SSMasterNode
> > > > - return (liftM rStripSpace result)
> > > > + return $ liftM rStripSpace result
> > > > +
> > > > +-- | Read the list of enabled hypervisors
> > > > +getHypervisorList :: Maybe FilePath -> IO (Result [Hypervisor])
> > > > +getHypervisorList optPath = do
> > > > + result <- readSSConfFile optPath Nothing SSHypervisorList
> > > > + return $ mapM Types.hypervisorFromRaw =<< liftM lines result
> > > > +
> > > > +-- | Read whether user shutdown is enabled
> > > > +getEnabledUserShutdown :: Maybe FilePath -> IO (Result Bool)
> > > > +getEnabledUserShutdown optPath = do
> > > > + result <- readSSConfFile optPath Nothing SSEnabledUserShutdown
> > > > + return $ tryRead "Parsing enabled_user_shutdown" =<< liftM
> > rStripSpace
> > > > result
> > > > diff --git a/src/Ganeti/Types.hs b/src/Ganeti/Types.hs
> > > > index 38d71b3..e61b983 100644
> > > > --- a/src/Ganeti/Types.hs
> > > > +++ b/src/Ganeti/Types.hs
> > > > @@ -75,6 +75,7 @@ module Ganeti.Types
> > > > , CVErrorCode(..)
> > > > , cVErrorCodeToRaw
> > > > , Hypervisor(..)
> > > > + , hypervisorFromRaw
> > > > , hypervisorToRaw
> > > > , OobCommand(..)
> > > > , oobCommandToRaw
> > > > --
> > > > 2.0.0.526.g5318336
> > > >
> > > >
> >
> > --
> > 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
> >
>
>
>
> --
> --
> Helga Velroyen | Software Engineer | [email protected] |
>
> 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
--
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
