From 7dbe4c72582f1a9f8905fc4cf9dbfd05546ca920 Mon Sep 17 00:00:00 2001
From: Klaus Aehlig <aehlig@google.com>
Date: Thu, 18 Apr 2013 14:53:45 +0200
Subject: [PATCH] Make hroller insist on finding precisely one master node

As people rely on the master node being the last node of the last
group, make hroller fail, if no master node could be found in the
cluster. This happens, e.g., if a backend format is used that does not
contain information about the master node. The error can be turned
into a warning by using the --force option.

Signed-off-by: Klaus Aehlig <aehlig@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>
---
 src/Ganeti/HTools/CLI.hs                    | 11 +++++++++++
 src/Ganeti/HTools/Program/Hroller.hs        |  9 +++++++++
 src/Ganeti/Utils.hs                         |  5 +++++
 test/hs/shelltests/htools-invalid.test      | 10 +++++++---
 test/hs/shelltests/htools-single-group.test |  6 ++++--
 5 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/src/Ganeti/HTools/CLI.hs b/src/Ganeti/HTools/CLI.hs
index 5e9bf0840..8f55f6fb4 100644
--- a/src/Ganeti/HTools/CLI.hs
+++ b/src/Ganeti/HTools/CLI.hs
@@ -52,6 +52,7 @@ module Ganeti.HTools.CLI
   , oExInst
   , oExTags
   , oExecJobs
+  , oForce
   , oGroup
   , oIAllocSrc
   , oInstMoves
@@ -117,6 +118,7 @@ data Options = Options
   , optExInst      :: [String]       -- ^ Instances to be excluded
   , optExTags      :: Maybe [String] -- ^ Tags to use for exclusion
   , optExecJobs    :: Bool           -- ^ Execute the commands via Luxi
+  , optForce       :: Bool           -- ^ Force the execution
   , optGroup       :: Maybe GroupID  -- ^ The UUID of the group to process
   , optIAllocSrc   :: Maybe FilePath -- ^ The iallocation spec
   , optSelInst     :: [String]       -- ^ Instances to be excluded
@@ -163,6 +165,7 @@ defaultOptions  = Options
   , optExInst      = []
   , optExTags      = Nothing
   , optExecJobs    = False
+  , optForce       = False
   , optGroup       = Nothing
   , optIAllocSrc   = Nothing
   , optSelInst     = []
@@ -319,6 +322,14 @@ oExecJobs =
    \ it for data gathering)",
    OptComplNone)
 
+oForce :: OptType
+oForce =
+  (Option "f" ["force"]
+   (NoArg (\ opts -> Ok opts {optForce = True}))
+   "force the execution of this program, even if warnings would\
+   \ otherwise prevent it",
+   OptComplNone)
+
 oGroup :: OptType
 oGroup =
   (Option "G" ["group"]
diff --git a/src/Ganeti/HTools/Program/Hroller.hs b/src/Ganeti/HTools/Program/Hroller.hs
index e55849003..72e863287 100644
--- a/src/Ganeti/HTools/Program/Hroller.hs
+++ b/src/Ganeti/HTools/Program/Hroller.hs
@@ -61,6 +61,7 @@ options = do
     , oNoHeaders
     , oSaveCluster
     , oGroup
+    , oForce
     ]
 
 -- | The list of arguments supported by the program.
@@ -112,11 +113,19 @@ main opts args = do
   unless (null args) $ exitErr "This program doesn't take any arguments."
 
   let verbose = optVerbose opts
+      maybeExit = if optForce opts then warn else exitErr
 
   -- Load cluster data. The last two arguments, cluster tags and ipolicy, are
   -- currently not used by this tool.
   ini_cdata@(ClusterData gl fixed_nl ilf _ _) <- loadExternalData opts
 
+  let master_names = map Node.name . filter Node.isMaster . IntMap.elems $
+                     fixed_nl
+  case master_names of
+    [] -> maybeExit "No master node found (maybe not supported by backend)."
+    [ _ ] -> return ()
+    _ -> exitErr $ "Found more than one master node: " ++  show master_names
+
   nlf <- setNodeStatus opts fixed_nl
 
   maybeSaveData (optSaveCluster opts) "original" "before hroller run" ini_cdata
diff --git a/src/Ganeti/Utils.hs b/src/Ganeti/Utils.hs
index b4a8c3cac..89a054d9e 100644
--- a/src/Ganeti/Utils.hs
+++ b/src/Ganeti/Utils.hs
@@ -49,6 +49,7 @@ module Ganeti.Utils
   , getCurrentTimeUSec
   , clockTimeToString
   , chompPrefix
+  , warn
   , wrap
   , trim
   , defaultHead
@@ -247,6 +248,10 @@ exitWhen False _  = return ()
 exitUnless :: Bool -> String -> IO ()
 exitUnless cond = exitWhen (not cond)
 
+-- | Print a warning, but do not exit.
+warn :: String -> IO ()
+warn = hPutStrLn stderr . (++) "Warning: "
+
 -- | Helper for 'niceSort'. Computes the key element for a given string.
 extractKey :: [Either Integer String]  -- ^ Current (partial) key, reversed
            -> String                   -- ^ Remaining string
diff --git a/test/hs/shelltests/htools-invalid.test b/test/hs/shelltests/htools-invalid.test
index eabce21be..c2bbe3e3e 100644
--- a/test/hs/shelltests/htools-invalid.test
+++ b/test/hs/shelltests/htools-invalid.test
@@ -52,10 +52,14 @@ Error: This program doesn't take any arguments.
 Error: This program doesn't take any arguments.
 >>>=1
 
-# hroller fails to build a graph for an empty cluster
+# hroller should notice the absence of a master node
 ./test/hs/hroller -t$TESTDATA_DIR/empty-cluster.data
->>>2
-Error: Cannot create node graph
+>>>2/Error: No master node found/
+>>>=1
+
+# hroller fails to build a graph for an empty cluster
+./test/hs/hroller -f -t$TESTDATA_DIR/empty-cluster.data
+>>>2/Error: Cannot create node graph/
 >>>=1
 
 # hbal doesn't accept invalid priority
diff --git a/test/hs/shelltests/htools-single-group.test b/test/hs/shelltests/htools-single-group.test
index f8e629cb5..0f271320e 100644
--- a/test/hs/shelltests/htools-single-group.test
+++ b/test/hs/shelltests/htools-single-group.test
@@ -28,10 +28,12 @@
 >>> /HCHECK_INIT_CLUSTER_NEED_REBALANCE=0/
 >>>= 0
 
+# FIXME: remove option -f once the text backend supports indicating
+#        the master node
 # hroller should be able to print the solution
-./test/hs/hroller -t$T/simu-onegroup.tiered
+./test/hs/hroller -f -t$T/simu-onegroup.tiered
 >>>= 0
 
 # hroller should be able to print the solution, in verbose mode as well
-./test/hs/hroller -t$T/simu-onegroup.tiered -v -v
+./test/hs/hroller -f -t$T/simu-onegroup.tiered -v -v
 >>>= 0
-- 
GitLab