Sorry, I missed that the misaligned code was actually being deleted. LGTM
On Thu, Mar 27, 2014 at 4:15 PM, Petr Pudlák <[email protected]> wrote: > > > > 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.) >
