LGTM, Thanks,
Guido On Fri, Apr 19, 2013 at 1:26 PM, Klaus Aehlig <[email protected]> wrote: >> > + let master_names = map Node.name . filter Node.isMaster . IntMap.elems $ >> > + fixed_nl >> > + case master_names of >> > + [] -> maybeExit "No master node found." >> > + [ _ ] -> return () >> > + _ -> maybeExit $ "Found more than one master node: " ++ show >> > master_names >> > + >> >> These two are not exactly the same. >> (one is a "backend doesn't support master detection", while the other >> one is a strange error condition) >> I propose allowing force to override just the first, and specifying >> that it "can" happen. > > Done. > >> > --- a/test/hs/shelltests/htools-single-group.test >> > +++ b/test/hs/shelltests/htools-single-group.test >> > @@ -29,9 +29,9 @@ >> > >>>= 0 >> > >> > # 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 >> >> Please add # FIXME: comments there specifying to remove -f once the >> files and text backends are updated for master node support. >> I also suggest adding a shelltest making sure a file "as of today" >> fails without -f, and succeeds with -f (and of course we shouldn't fix >> that one). :) > > Done. > > Interdiff > > diff --git a/src/Ganeti/HTools/Program/Hroller.hs > b/src/Ganeti/HTools/Program/Hroller.hs > index 24f4479..72e8632 100644 > --- a/src/Ganeti/HTools/Program/Hroller.hs > +++ b/src/Ganeti/HTools/Program/Hroller.hs > @@ -122,9 +122,9 @@ main opts args = do > let master_names = map Node.name . filter Node.isMaster . IntMap.elems $ > fixed_nl > case master_names of > - [] -> maybeExit "No master node found." > + [] -> maybeExit "No master node found (maybe not supported by backend)." > [ _ ] -> return () > - _ -> maybeExit $ "Found more than one master node: " ++ show > master_names > + _ -> exitErr $ "Found more than one master node: " ++ show master_names > > nlf <- setNodeStatus opts fixed_nl > > diff --git a/test/hs/shelltests/htools-invalid.test > b/test/hs/shelltests/htools-invalid.test > index 8d557c3..c2bbe3e 100644 > --- a/test/hs/shelltests/htools-invalid.test > +++ b/test/hs/shelltests/htools-invalid.test > @@ -52,6 +52,11 @@ Error: This program doesn't take any arguments. > Error: This program doesn't take any arguments. > >>>=1 > > +# hroller should notice the absence of a master node > +./test/hs/hroller -t$TESTDATA_DIR/empty-cluster.data > +>>>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/ > diff --git a/test/hs/shelltests/htools-single-group.test > b/test/hs/shelltests/htools-single-group.test > index d4db336..0f27132 100644 > --- a/test/hs/shelltests/htools-single-group.test > +++ b/test/hs/shelltests/htools-single-group.test > @@ -28,6 +28,8 @@ > >>> /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 -f -t$T/simu-onegroup.tiered > >>>= 0 > > > -- > Klaus Aehlig > Google Germany GmbH, Dienerstr. 12, 80331 Muenchen > Registergericht und -nummer: Hamburg, HRB 86891 > Sitz der Gesellschaft: Hamburg > Geschaeftsfuehrer: Graham Law, Katherine Stephens -- Guido Trotter 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, Katherine Stephens Steuernummer: 48/725/00206 Umsatzsteueridentifikationsnummer: DE813741370
