LGTM

Thanks,

Guido

On Tue, Mar 6, 2012 at 11:59 PM, Iustin Pop <[email protected]> wrote:
> Currently, these presume that the instance being generated always is
> DRBD/relocatable (but won't be in the future), and also presume that
> all relocation modes can be used (hint: not :).
>
> So we add a couple of helper functions to restrict the used disk
> templates/relocation modes, and these two tests are ready for shared
> storage.
> ---
>  htools/Ganeti/HTools/QC.hs |   19 ++++++++++++++++---
>  1 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/htools/Ganeti/HTools/QC.hs b/htools/Ganeti/HTools/QC.hs
> index 8e1b413..dadc09e 100644
> --- a/htools/Ganeti/HTools/QC.hs
> +++ b/htools/Ganeti/HTools/QC.hs
> @@ -232,6 +232,17 @@ genUniquesList cnt =
>            newelem <- arbitrary `suchThat` (`notElem` lst)
>            return (newelem:lst)) [] [1..cnt]
>
> +-- | Checks if an instance is mirrored.
> +isMirrored :: Instance.Instance -> Bool
> +isMirrored =
> +  (/= Types.MirrorNone) . Types.templateMirrorType . Instance.diskTemplate
> +
> +-- | Returns the possible change node types for a disk template.
> +evacModeOptions :: Types.MirrorType -> [Types.EvacMode]
> +evacModeOptions Types.MirrorNone     = []
> +evacModeOptions Types.MirrorInternal = [minBound..maxBound] -- DRBD can do 
> all
> +evacModeOptions Types.MirrorExternal = [Types.ChangePrimary, Types.ChangeAll]
> +
>  -- * Arbitrary instances
>
>  -- | Defines a DNS name.
> @@ -1261,13 +1272,15 @@ check_EvacMode grp inst result =
>  prop_ClusterAllocEvacuate =
>   forAll (choose (4, 8)) $ \count ->
>   forAll (genOnlineNode `suchThat` (isNodeBig 4)) $ \node ->
> -  forAll (genInstanceSmallerThanNode node) $ \inst ->
> +  forAll (genInstanceSmallerThanNode node `suchThat` isMirrored) $ \inst ->
>   case genClusterAlloc count node inst of
>     Types.Bad msg -> failTest msg
>     Types.Ok (nl, il, inst') ->
>       conjoin $ map (\mode -> check_EvacMode defGroup inst' $
>                               Cluster.tryNodeEvac defGroupList nl il mode
> -                                [Instance.idx inst']) [minBound..maxBound]
> +                                [Instance.idx inst']) .
> +                              evacModeOptions . Types.templateMirrorType .
> +                              Instance.diskTemplate $ inst'
>
>  -- | Checks that on a 4-8 node cluster with two node groups, once we
>  -- allocate an instance on the first node group, we can also change
> @@ -1275,7 +1288,7 @@ prop_ClusterAllocEvacuate =
>  prop_ClusterAllocChangeGroup =
>   forAll (choose (4, 8)) $ \count ->
>   forAll (genOnlineNode `suchThat` (isNodeBig 4)) $ \node ->
> -  forAll (genInstanceSmallerThanNode node) $ \inst ->
> +  forAll (genInstanceSmallerThanNode node `suchThat` isMirrored) $ \inst ->
>   case genClusterAlloc count node inst of
>     Types.Bad msg -> failTest msg
>     Types.Ok (nl, il, inst') ->
> --
> 1.7.9.1
>



-- 
Guido Trotter
Google - Corporate Computing Services SRE

Google Ireland Ltd. : Registered in Ireland with company number 368047.
Gordon House, Barrow Street, Dublin 4, Ireland.

Reply via email to