LGTM, thanks On Mon, 1 Jun 2015 at 12:49 'Klaus Aehlig' via ganeti-devel < [email protected]> wrote:
> > > commit 9cd96c47fdf6a1e4152ab26c39c0f87416f78319 > Merge: f241763 f9139cd > Author: Klaus Aehlig <[email protected]> > Date: Mon Jun 1 12:36:50 2015 +0200 > > Merge branch 'stable-2.13' into stable-2.14 > > * stable-2.13 > (no changes) > > * stable-2.14 > Make WConfD's updateLocksWaiting safe > Tests specifying safeUpdateLocksWaiting > Provide a repeatable version of updateLocksWaiting > Verify that updateLocks is idempotent > Always accept no-op requests > Allow unconditional failovers off offline nodes > Remove now unused variable > Fix bug in ssconf comparison, disable it for vcluster > QA: test renewing the cluster certificate only > QA: Assert equality of ssconf_master_candidate_certs > QA: Add more verify steps in renew crypto QA > > * stable-2.11 > (no changes) > > * stable-2.10 > Substitute 'suffix' for 'revision' > > Conflicts: > src/Ganeti/HTools/Node.hs (apply condition at new code location) > > diff --cc src/Ganeti/HTools/Node.hs > index e92df67,cb17f24..f47a45f > --- a/src/Ganeti/HTools/Node.hs > +++ b/src/Ganeti/HTools/Node.hs > @@@ -936,93 -608,52 +937,101 @@@ addSec = addSecEx Fals > > -- | Adds a secondary instance (extended version). > addSecEx :: Bool -> Node -> Instance.Instance -> T.Ndx -> T.OpResult Node > - addSecEx force t inst pdx = -- trace "addSecEx" $ > + addSecEx = addSecExEx False > + > + -- | Adds a secondary instance (doubly extended version). The first > parameter > + -- tells `addSecExEx` to ignore disks completly. There is only one > legitimate > + -- use case for this, and this is failing over a DRBD instance where the > primary > + -- node is offline (and hence will become the secondary afterwards). > + addSecExEx :: Bool > + -> Bool -> Node -> Instance.Instance -> T.Ndx -> T.OpResult > Node > + addSecExEx ignore_disks force t inst pdx = > let iname = Instance.idx inst > + forthcoming = Instance.forthcoming inst > old_peers = peers t > - old_mem = fMem t > - new_dsk = fDsk t - Instance.dsk inst > - new_free_sp = calcNewFreeSpindles True t inst > - new_inst_sp = calcSpindleUse True t inst > + strict = not force > + > secondary_needed_mem = if Instance.usesSecMem inst > then Instance.mem inst > else 0 > new_peem = P.find pdx old_peers + secondary_needed_mem > new_peers = P.add pdx new_peem old_peers > - new_rmem = max (rMem t) new_peem > - new_prem = fromIntegral new_rmem / tMem t > - new_failn1 = old_mem <= new_rmem > - new_dp = computeNewPDsk t new_free_sp new_dsk > - old_load = utilLoad t > - new_load = old_load { T.dskWeight = T.dskWeight old_load + > - T.dskWeight (Instance.util > inst) } > - strict = not force > - in case () of > - _ | not (Instance.hasSecondary inst) -> Bad T.FailDisk > - | not ignore_disks && new_dsk <= 0 -> Bad T.FailDisk > - | new_dsk < loDsk t && strict -> Bad T.FailDisk > - | exclStorage t && new_free_sp < 0 -> Bad T.FailSpindles > - | new_inst_sp > hiSpindles t && strict -> Bad T.FailDisk > - | secondary_needed_mem >= old_mem && strict -> Bad T.FailMem > - | new_failn1 && not (failN1 t) && strict -> Bad T.FailMem > - | otherwise -> > - let new_slist = iname:sList t > - r = t { sList = new_slist, fDsk = new_dsk > - , peers = new_peers, failN1 = new_failn1 > - , rMem = new_rmem, pDsk = new_dp > - , pRem = new_prem, utilLoad = new_load > - , instSpindles = new_inst_sp > - , fSpindles = new_free_sp > - } > - in Ok r > + > + old_mem_forth = fMemForth t > + new_dsk_forth = fDskForth t - Instance.dsk inst > + new_free_sp_forth = calcNewFreeSpindlesForth True t inst > + new_inst_sp_forth = calcSpindleUseForth True t inst > + new_dp_forth = computeNewPDsk t new_free_sp_forth new_dsk_forth > + old_load_forth = utilLoadForth t > + new_load_forth = old_load_forth > + { T.dskWeight = T.dskWeight old_load_forth + > + T.dskWeight (Instance.util inst) > + } > + new_slist_forth = iname:sListForth t > + > + updateForthcomingFields n = > + n { sListForth = new_slist_forth > + , fDskForth = new_dsk_forth > + , pDskForth = new_dp_forth > + , utilLoadForth = new_load_forth > + , instSpindlesForth = new_inst_sp_forth > + , fSpindlesForth = new_free_sp_forth > + > + -- TODO Set failN1Forth, rMemForth, pRemForth > + } > + > + checkForthcomingViolation > + | not (Instance.hasSecondary inst) = Bad T.FailDisk > + | new_dsk_forth <= 0 = Bad T.FailDisk > + | new_dsk_forth < loDsk t = Bad T.FailDisk > + | exclStorage t && new_free_sp_forth < 0 = Bad T.FailSpindles > + | new_inst_sp_forth > hiSpindles t = Bad T.FailDisk > + | secondary_needed_mem >= old_mem_forth = Bad T.FailMem > + -- TODO Check failN1 including forthcoming instances > + | otherwise = Ok () > + > + in if forthcoming > + then case strict of > + True | Bad err <- checkForthcomingViolation -> Bad err > + > + _ -> Ok $ updateForthcomingFields t > + else let > + old_mem = fMem t > + new_dsk = fDsk t - Instance.dsk inst > + new_free_sp = calcNewFreeSpindles True t inst > + new_inst_sp = calcSpindleUse True t inst > + new_rmem = max (rMem t) new_peem > + new_prem = fromIntegral new_rmem / tMem t > + new_failn1 = old_mem <= new_rmem > + new_dp = computeNewPDsk t new_free_sp new_dsk > + old_load = utilLoad t > + new_load = old_load > + { T.dskWeight = T.dskWeight old_load + > + T.dskWeight (Instance.util > inst) > + } > + new_slist = iname:sList t > + in case () of > + _ | not (Instance.hasSecondary inst) -> Bad T.FailDisk > - | new_dsk <= 0 -> Bad T.FailDisk > ++ | not ignore_disks && new_dsk <= 0 -> Bad T.FailDisk > + | strict && new_dsk < loDsk t -> Bad T.FailDisk > + | exclStorage t && new_free_sp < 0 -> Bad T.FailSpindles > + | strict && new_inst_sp > hiSpindles t -> Bad T.FailDisk > + | strict && secondary_needed_mem >= old_mem -> Bad T.FailMem > + | strict && new_failn1 && not (failN1 t) -> Bad T.FailMem > + > + -- When strict also check forthcoming limits, but after normal > checks > + | strict, Bad err <- checkForthcomingViolation -> Bad err > + > + | otherwise -> > + Ok . updateForthcomingFields $ > + t { sList = new_slist, fDsk = new_dsk > + , peers = new_peers, failN1 = new_failn1 > + , rMem = new_rmem, pDsk = new_dp > + , pRem = new_prem, utilLoad = new_load > + , instSpindles = new_inst_sp > + , fSpindles = new_free_sp > + } > + > > -- | Predicate on whether migration is supported between two nodes. > checkMigration :: Node -> Node -> T.OpResult () > diff --cc test/hs/Test/Ganeti/Locking/Waiting.hs > index 40ee0c5,3950663..de863b2 > --- a/test/hs/Test/Ganeti/Locking/Waiting.hs > +++ b/test/hs/Test/Ganeti/Locking/Waiting.hs > @@@ -245,9 -244,27 +245,27 @@@ prop_PendingJustified > let isJustified (_, b, req) = > genericResult (const False) (not . S.null) . snd > . L.updateLocks b req $ getAllocation state > - in printTestCase "Pebding requests must be good and not fulfillable" > + in counterexample "Pending requests must be good and not fulfillable" > . all isJustified . S.toList $ getPendingRequests state > > + -- | Verify that `updateLocks` is idempotent, except that in the > repetition, > + -- no waiters are notified. > + prop_UpdateIdempotent :: Property > + prop_UpdateIdempotent = > + forAll (arbitrary :: Gen (LockWaiting TestLock TestOwner Integer)) $ > \state -> > + forAll (arbitrary :: Gen TestOwner) $ \owner -> > + forAll (arbitrary :: Gen [LockRequest TestLock]) $ \req -> > + let (state', (answer', _)) = updateLocks owner req state > + (state'', (answer'', nfy)) = updateLocks owner req state' > + in conjoin [ printTestCase ("repeated updateLocks waiting gave > different\ > + \ answers: " ++ show answer' ++ " /= " > + ++ show answer'') $ answer' == answer'' > + , printTestCase "updateLocks not idempotent" > + $ extRepr state' == extRepr state'' > + , printTestCase ("notifications (" ++ show nfy ++ ") on > replay") > + $ S.null nfy > + ] > + > -- | Verify that extRepr . fromExtRepr = id for all valid extensional > -- representations. > prop_extReprPreserved :: Property > > -- > Klaus Aehlig > Google Germany GmbH, Dienerstr. 12, 80331 Muenchen > Registergericht und -nummer: Hamburg, HRB 86891 > Sitz der Gesellschaft: Hamburg > Geschaeftsfuehrer: Graham Law, Christine Elizabeth Flores >
