On Thu, Mar 27, 2014 at 2:48 PM, Hrvoje Ribicic <[email protected]> wrote:
> The UUID/name switch avoided this particular bit of code, and as a > result the list-drbd command failed as it tried to compare UUIDs and > names. This patch fixes the retrieval, converts the newly returned > UUIDs to names, and modifies the QA to the results and not only the > invocation are checked. > > Signed-off-by: Gerard Oskamp <[email protected]> > Signed-off-by: Hrvoje Ribicic <[email protected]> > --- > qa/ganeti-qa.py | 3 ++- > qa/qa_node.py | 18 +++++++++++++++--- > src/Ganeti/Confd/Server.hs | 21 +++++++++++++++++---- > src/Ganeti/Config.hs | 3 ++- > 4 files changed, 36 insertions(+), 9 deletions(-) > > diff --git a/qa/ganeti-qa.py b/qa/ganeti-qa.py > index 11d129a..38659b8 100755 > --- a/qa/ganeti-qa.py > +++ b/qa/ganeti-qa.py > @@ -728,7 +728,8 @@ def RunInstanceTests(): > RunTestIf("cluster-epo", qa_cluster.TestClusterEpo) > RunDaemonTests(instance) > for node in inodes: > - RunTestIf("haskell-confd", qa_node.TestNodeListDrbd, node) > + RunTestIf("haskell-confd", qa_node.TestNodeListDrbd, node, > + templ == constants.DT_DRBD8) > if len(inodes) > 1: > RunTestIf("group-rwops", > qa_group.TestAssignNodesIncludingSplit, > constants.INITIAL_NODE_GROUP_NAME, > diff --git a/qa/qa_node.py b/qa/qa_node.py > index 16fdb15..4d820b8 100644 > --- a/qa/qa_node.py > +++ b/qa/qa_node.py > @@ -32,7 +32,7 @@ import qa_config > import qa_error > import qa_utils > > -from qa_utils import AssertCommand, AssertEqual > +from qa_utils import AssertCommand, AssertEqual, AssertIn, > GetCommandOutput > > > def _NodeAdd(node, readd=False): > @@ -451,9 +451,21 @@ def TestNodeListFields(): > qa_utils.GenericQueryFieldsTest("gnt-node", query.NODE_FIELDS.keys()) > > > -def TestNodeListDrbd(node): > +def TestNodeListDrbd(node, is_drbd): > """gnt-node list-drbd""" > - AssertCommand(["gnt-node", "list-drbd", node.primary]) > + master = qa_config.GetMasterNode() > + result_output = GetCommandOutput(master.primary, > + "gnt-node list-drbd --no-header %s" % > + node.primary) > + # Meaningful to note: there is but one instance, and the node is either > the > + # primary or one of the secondaries > + if is_drbd: > + # Invoked for both primary and secondary > + drbd_node, _, _, _, _, drbd_peer = result_output.split() > + AssertIn(node.primary, [drbd_node, drbd_peer]) > + else: > + # Output should be empty, barring newlines > + AssertEqual(result_output.strip(), "") > > > def _BuildSetESCmd(action, value, node_name): > diff --git a/src/Ganeti/Confd/Server.hs b/src/Ganeti/Confd/Server.hs > index 25cbb17..05275c4 100644 > --- a/src/Ganeti/Confd/Server.hs > +++ b/src/Ganeti/Confd/Server.hs > @@ -29,6 +29,7 @@ module Ganeti.Confd.Server > , prepMain > ) where > > +import Control.Applicative((<$>)) > import Control.Concurrent > import Control.Monad (forever, liftM) > import Data.IORef > @@ -112,6 +113,20 @@ getNodePipByInstanceIp cfg linkipmap link instip = > Bad _ -> queryUnknownEntry -- either instance or node not found > Ok node -> (ReplyStatusOk, J.showJSON (nodePrimaryIp node)) > > +-- | Returns a node name for a given UUID > +uuidToNodeName :: ConfigData -> String -> Result String > +uuidToNodeName cfg uuid = gntErrorToResult $ nodeName <$> getNode cfg uuid > + > +-- | Encodes a list of minors into a JSON representation, converting > UUIDs to > +-- names in the process > +encodeMinors :: ConfigData -> (String, Int, String, String, String, > String) > + -> Result J.JSValue > +encodeMinors cfg (node_uuid, a, b, c, d, peer_uuid) = do > + node_name <- uuidToNodeName cfg node_uuid > + peer_name <- uuidToNodeName cfg peer_uuid > + return . J.JSArray $ [J.showJSON node_name, J.showJSON a, J.showJSON b, > + J.showJSON c, J.showJSON d, J.showJSON peer_name] > + > -- | Builds the response to a given query. > buildResponse :: (ConfigData, LinkIpMap) -> ConfdRequest -> Result > StatusAnswer > buildResponse (cfg, _) (ConfdRequest { confdRqType = ReqPing }) = > @@ -186,11 +201,9 @@ buildResponse cdata req@(ConfdRequest { confdRqType > = ReqNodeDrbd }) = do > PlainQuery str -> return str > _ -> fail $ "Invalid query type " ++ show (confdRqQuery > req) > node <- gntErrorToResult $ getNode cfg node_name > - let minors = concatMap (getInstMinorsForNode (nodeName node)) . > + let minors = concatMap (getInstMinorsForNode (nodeUuid node)) . > M.elems . fromContainer . configInstances $ cfg > - encoded = [J.JSArray [J.showJSON a, J.showJSON b, J.showJSON c, > - J.showJSON d, J.showJSON e, J.showJSON f] | > Just nitpicking - the columns don't align. I'd suggest something like [ x, ... , y, ... z ] > - (a, b, c, d, e, f) <- minors] > + encoded <- mapM (encodeMinors cfg) minors > return (ReplyStatusOk, J.showJSON encoded) > > -- | Return the list of instances for a node (as ([primary], > [secondary])) given > diff --git a/src/Ganeti/Config.hs b/src/Ganeti/Config.hs > index 1f70836..fb0e2c6 100644 > --- a/src/Ganeti/Config.hs > +++ b/src/Ganeti/Config.hs > @@ -263,7 +263,8 @@ roleSecondary = "secondary" > > -- | Gets the list of DRBD minors for an instance that are related to > -- a given node. > -getInstMinorsForNode :: String -> Instance > +getInstMinorsForNode :: String -- ^ The UUID of a node. > + -> Instance > -> [(String, Int, String, String, String, String)] > getInstMinorsForNode node inst = > let role = if node == instPrimaryNode inst > -- > 1.9.1.423.g4596e3a > > Otherwise LGTM, thanks. (No need to resend.)
