Hi, I run tests on the patch, and there is another command that is missing the new *group_name* parameter: *gnt-instance recreate-disks*. This is the failed test: https://ganeti-buildbot.corp.google.com/ganeti/builders/qa-drbd84-new-quick/builds/261 Could you please have a look at it?
Thanks, Petr On Mon, May 5, 2014 at 3:28 PM, 'BSRK Aditya' via ganeti-devel < [email protected]> wrote: > This commit adds the feature requested in Issue 738. > The reporter asked for the ability to create an instance > to a specific group. > > Signed-off-by: BSRK Aditya <[email protected]> > --- > lib/cli.py | 2 + > lib/cmdlib/instance.py | 1 + > lib/cmdlib/test.py | 2 + > lib/masterd/iallocator.py | 2 + > man/gnt-instance.rst | 5 +++ > src/Ganeti/HTools/Backend/IAlloc.hs | 13 +++++-- > src/Ganeti/HTools/Cluster.hs | 78 > ++++++++++++++++++++++++++++++------- > src/Ganeti/HTools/Loader.hs | 13 ++++--- > src/Ganeti/OpCodes.hs | 2 + > test/hs/Test/Ganeti/OpCodes.hs | 4 +- > test/py/cmdlib/test_unittest.py | 2 + > 11 files changed, 99 insertions(+), 25 deletions(-) > > diff --git a/lib/cli.py b/lib/cli.py > index 04ee4e0..4d0632d 100644 > --- a/lib/cli.py > +++ b/lib/cli.py > @@ -1785,6 +1785,7 @@ COMMON_CREATE_OPTS = [ > IALLOCATOR_OPT, > NET_OPT, > NODE_PLACEMENT_OPT, > + NODEGROUP_OPT, > NOIPCHECK_OPT, > NOCONFLICTSCHECK_OPT, > NONAMECHECK_OPT, > @@ -2829,6 +2830,7 @@ def GenericInstanceCreate(mode, opts, args): > op = opcodes.OpInstanceCreate(instance_name=instance, > disks=disks, > disk_template=opts.disk_template, > + group_name=opts.nodegroup, > nics=nics, > conflicts_check=opts.conflicts_check, > pnode=pnode, snode=snode, > diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py > index 120b239..f363dda 100644 > --- a/lib/cmdlib/instance.py > +++ b/lib/cmdlib/instance.py > @@ -122,6 +122,7 @@ def _CreateInstanceAllocRequest(op, disks, nics, > beparams, node_name_whitelist): > spindle_use = beparams[constants.BE_SPINDLE_USE] > return iallocator.IAReqInstanceAlloc(name=op.instance_name, > disk_template=op.disk_template, > + group_name=op.group_name, > tags=op.tags, > os=op.os_type, > vcpus=beparams[constants.BE_VCPUS], > diff --git a/lib/cmdlib/test.py b/lib/cmdlib/test.py > index 2b10740..d0572e9 100644 > --- a/lib/cmdlib/test.py > +++ b/lib/cmdlib/test.py > @@ -396,6 +396,7 @@ class LUTestAllocator(NoHooksLU): > memory=self.op.memory, > disks=self.op.disks, > > disk_template=self.op.disk_template, > + group_name=self.op.group_name, > os=self.op.os, > tags=self.op.tags, > nics=self.op.nics, > @@ -419,6 +420,7 @@ class LUTestAllocator(NoHooksLU): > memory=self.op.memory, > disks=self.op.disks, > disk_template=disk_template, > + > group_name=self.op.group_name, > os=self.op.os, > tags=self.op.tags, > nics=self.op.nics, > diff --git a/lib/masterd/iallocator.py b/lib/masterd/iallocator.py > index 4ceae4a..c39da28 100644 > --- a/lib/masterd/iallocator.py > +++ b/lib/masterd/iallocator.py > @@ -158,6 +158,7 @@ class IAReqInstanceAlloc(IARequestBase): > ("spindle_use", ht.TNonNegativeInt), > ("disks", ht.TListOf(ht.TDict)), > ("disk_template", ht.TString), > + ("group_name", ht.TMaybe(ht.TNonEmptyString)), > ("os", ht.TString), > ("tags", _STRING_LIST), > ("nics", ht.TListOf(ht.TDict)), > @@ -188,6 +189,7 @@ class IAReqInstanceAlloc(IARequestBase): > return { > "name": self.name, > "disk_template": self.disk_template, > + "group_name": self.group_name, > "tags": self.tags, > "os": self.os, > "vcpus": self.vcpus, > diff --git a/man/gnt-instance.rst b/man/gnt-instance.rst > index 2cf6f64..3de3d42 100644 > --- a/man/gnt-instance.rst > +++ b/man/gnt-instance.rst > @@ -41,6 +41,7 @@ ADD > | [--os-parameters-secret *param*=*value*... ] > | [\--file-storage-dir *dir\_path*] [\--file-driver {loop \| blktap \| > blktap2}] > | {{-n|\--node} *node[:secondary-node]* \| {-I|\--iallocator} *name*} > +| [{-g|\--node-group} *nodegroup*] > | {{-o|\--os-type} *os-type*} > | [\--submit] [\--print-job-id] > | [\--ignore-ipolicy] > @@ -867,6 +868,10 @@ the allocator will select nodes for this instance > automatically, so you > don't need to pass them with the ``-n`` option. For more information > please refer to the instance allocator documentation. > > +The ``-g (--node-group)`` option can be used together with the ``-I`` > +option to instruct the instance allocator plugin to create the instance > +in a particular node group, specified by UUID or name. > + > The ``-t (--disk-template)`` options specifies the disk layout type > for the instance. If no disk template is specified, the default disk > template is used. The default disk template is the first in the list > diff --git a/src/Ganeti/HTools/Backend/IAlloc.hs > b/src/Ganeti/HTools/Backend/IAlloc.hs > index 866efe7..dc52eb7 100644 > --- a/src/Ganeti/HTools/Backend/IAlloc.hs > +++ b/src/Ganeti/HTools/Backend/IAlloc.hs > @@ -211,10 +211,11 @@ parseData now body = do > _ | optype == C.iallocatorModeAlloc -> > do > rname <- extrReq "name" > + rgn <- maybeFromObj request "group_name" > req_nodes <- extrReq "required_nodes" > inew <- parseBaseInstance rname request > let io = updateExclTags (extractExTags ctags) $ snd inew > - return $ Allocate io req_nodes > + return $ Allocate io (Cluster.AllocDetails req_nodes rgn) > | optype == C.iallocatorModeReloc -> > do > rname <- extrReq "name" > @@ -243,14 +244,16 @@ parseData now body = do > do > arry <- extrReq "instances" :: Result [JSObject JSValue] > let inst_reqs = map fromJSObject arry > - prqs <- mapM (\r -> > + prqs <- forM inst_reqs (\r -> > do > rname <- extrFromReq r "name" > + rgn <- maybeFromObj request > "group_name" > req_nodes <- extrFromReq r > "required_nodes" > inew <- parseBaseInstance rname r > let io = updateExclTags (extractExTags > ctags) > $ snd inew > - return (io, req_nodes)) inst_reqs > + return (io, Cluster.AllocDetails > + req_nodes rgn)) > return $ MultiAllocate prqs > | otherwise -> fail ("Invalid request type '" ++ optype ++ "'") > return (msgs, Request rqtype cdata) > @@ -399,8 +402,10 @@ processRequest :: Request -> Result IAllocResult > processRequest request = > let Request rqtype (ClusterData gl nl il _ _) = request > in case rqtype of > - Allocate xi reqn -> > + Allocate xi (Cluster.AllocDetails reqn Nothing) -> > Cluster.tryMGAlloc gl nl il xi reqn >>= formatAllocate il > + Allocate xi (Cluster.AllocDetails reqn (Just gn)) -> > + Cluster.tryGroupAlloc gl nl il gn xi reqn >>= formatAllocate il > Relocate idx reqn exnodes -> > processRelocate gl nl il idx reqn exnodes >>= formatRelocate > ChangeGroup gdxs idxs -> > diff --git a/src/Ganeti/HTools/Cluster.hs b/src/Ganeti/HTools/Cluster.hs > index a95c613..954b7e9 100644 > --- a/src/Ganeti/HTools/Cluster.hs > +++ b/src/Ganeti/HTools/Cluster.hs > @@ -29,7 +29,8 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, > Boston, MA > module Ganeti.HTools.Cluster > ( > -- * Types > - AllocSolution(..) > + AllocDetails(..) > + , AllocSolution(..) > , EvacSolution(..) > , Table(..) > , CStats(..) > @@ -63,6 +64,7 @@ module Ganeti.HTools.Cluster > -- * IAllocator functions > , genAllocNodes > , tryAlloc > + , tryGroupAlloc > , tryMGAlloc > , tryNodeEvac > , tryChangeGroup > @@ -77,8 +79,9 @@ module Ganeti.HTools.Cluster > , splitCluster > ) where > > -import Control.Applicative (liftA2) > +import Control.Applicative ((<$>), liftA2) > import Control.Arrow ((&&&)) > +import Control.Monad (unless) > import qualified Data.IntSet as IntSet > import Data.List > import Data.Maybe (fromJust, fromMaybe, isJust, isNothing) > @@ -99,6 +102,12 @@ import Ganeti.Types (EvacMode(..), mkNonEmpty) > > -- * Types > > +-- | Allocation details for an instance, specifying > +-- | required number of nodes, and > +-- | an optional group (name) to allocate to > +data AllocDetails = AllocDetails Int (Maybe String) > + deriving (Show) > + > -- | Allocation\/relocation solution. > data AllocSolution = AllocSolution > { asFailures :: [FailMode] -- ^ Failure counts > @@ -821,6 +830,12 @@ sortMGResults sols = > (extractScore . fromJust . asSolution) sol) > in sortBy (comparing solScore) sols > > +-- | Determines if a group is connected to the networks required by the > +-- | instance. > +hasRequiredNetworks :: Group.Group -> Instance.Instance -> Bool > +hasRequiredNetworks ng = all hasNetwork . Instance.nics > + where hasNetwork = maybe True (`elem` Group.networks ng) . Nic.network > + > -- | Removes node groups which can't accommodate the instance > filterValidGroups :: [(Group.Group, (Node.List, Instance.List))] > -> Instance.Instance > @@ -828,17 +843,32 @@ filterValidGroups :: [(Group.Group, (Node.List, > Instance.List))] > filterValidGroups [] _ = ([], []) > filterValidGroups (ng:ngs) inst = > let (valid_ngs, msgs) = filterValidGroups ngs inst > - hasNetwork nic = case Nic.network nic of > - Just net -> net `elem` Group.networks (fst ng) > - Nothing -> True > - hasRequiredNetworks = all hasNetwork (Instance.nics inst) > - in if hasRequiredNetworks > + in if hasRequiredNetworks (fst ng) inst > then (ng:valid_ngs, msgs) > else (valid_ngs, > ("group " ++ Group.name (fst ng) ++ > " is not connected to a network required by instance " ++ > Instance.name inst):msgs) > > +-- | Finds an allocation solution for an instance on a group > +findAllocation :: Group.List -- ^ The group list > + -> Node.List -- ^ The node list > + -> Instance.List -- ^ The instance list > + -> Gdx -- ^ The group to allocate to > + -> Instance.Instance -- ^ The instance to allocate > + -> Int -- ^ Required number of nodes > + -> Result (AllocSolution, [String]) > +findAllocation mggl mgnl mgil gdx inst cnt = do > + let belongsTo nl' nidx = nidx `elem` map Node.idx (Container.elems nl') > + nl = Container.filter ((== gdx) . Node.group) mgnl > + il = Container.filter (belongsTo nl . Instance.pNode) mgil > + group' = Container.find gdx mggl > + unless (hasRequiredNetworks group' inst) . failError > + $ "The group " ++ Group.name group' ++ " is not connected to\ > + \ a network required by instance " ++ Instance.name inst > + solution <- genAllocNodes mggl nl cnt False >>= tryAlloc nl il inst > + return (solution, solutionDescription (group', return solution)) > + > -- | Finds the best group for an instance on a multi-group cluster. > -- > -- Only solutions in @preferred@ and @last_resort@ groups will be > @@ -888,6 +918,19 @@ tryMGAlloc mggl mgnl mgil inst cnt = do > selmsg = "Selected group: " ++ group_name > return $ solution { asLog = selmsg:all_msgs } > > +-- | Try to allocate an instance to a group. > +tryGroupAlloc :: Group.List -- ^ The group list > + -> Node.List -- ^ The node list > + -> Instance.List -- ^ The instance list > + -> String -- ^ The allocation group (index) > + -> Instance.Instance -- ^ The instance to allocate > + -> Int -- ^ Required number of nodes > + -> Result AllocSolution -- ^ Solution > +tryGroupAlloc mggl mgnl ngil gn inst cnt = do > + gdx <- Group.idx <$> Container.findByName mggl gn > + (solution, msgs) <- findAllocation mggl mgnl ngil gdx inst cnt > + return $ solution { asLog = msgs } > + > -- | Calculate the new instance list after allocation solution. > updateIl :: Instance.List -- ^ The original instance list > -> Maybe Node.AllocElement -- ^ The result of the allocation > attempt > @@ -903,16 +946,21 @@ extractNl nl Nothing = nl > extractNl _ (Just (xnl, _, _, _)) = xnl > > -- | Try to allocate a list of instances on a multi-group cluster. > -allocList :: Group.List -- ^ The group list > - -> Node.List -- ^ The node list > - -> Instance.List -- ^ The instance list > - -> [(Instance.Instance, Int)] -- ^ The instance to allocate > - -> AllocSolutionList -- ^ Possible solution list > +allocList :: Group.List -- ^ The group list > + -> Node.List -- ^ The node list > + -> Instance.List -- ^ The instance > list > + -> [(Instance.Instance, AllocDetails)] -- ^ The instance > to > + -- allocate > + -> AllocSolutionList -- ^ Possible > solution > + -- list > -> Result (Node.List, Instance.List, > - AllocSolutionList) -- ^ The final solution list > + AllocSolutionList) -- ^ The final > solution > + -- list > allocList _ nl il [] result = Ok (nl, il, result) > -allocList gl nl il ((xi, xicnt):xies) result = do > - ares <- tryMGAlloc gl nl il xi xicnt > +allocList gl nl il ((xi, AllocDetails xicnt mgn):xies) result = do > + ares <- case mgn of > + Nothing -> tryMGAlloc gl nl il xi xicnt > + Just gn -> tryGroupAlloc gl nl il gn xi xicnt > let sol = asSolution ares > nl' = extractNl nl sol > il' = updateIl il sol > diff --git a/src/Ganeti/HTools/Loader.hs b/src/Ganeti/HTools/Loader.hs > index 7440ba3..60aa361 100644 > --- a/src/Ganeti/HTools/Loader.hs > +++ b/src/Ganeti/HTools/Loader.hs > @@ -79,11 +79,14 @@ request-specific fields. > > -} > data RqType > - = Allocate Instance.Instance Int -- ^ A new instance > allocation > - | Relocate Idx Int [Ndx] -- ^ Choose a new secondary > node > - | NodeEvacuate [Idx] EvacMode -- ^ node-evacuate mode > - | ChangeGroup [Gdx] [Idx] -- ^ Multi-relocate mode > - | MultiAllocate [(Instance.Instance, Int)] -- ^ Multi-allocate mode > + = Allocate Instance.Instance Cluster.AllocDetails -- ^ A new instance > + -- allocation > + | Relocate Idx Int [Ndx] -- ^ Choose a new > + -- secondary node > + | NodeEvacuate [Idx] EvacMode -- ^ node-evacuate > mode > + | ChangeGroup [Gdx] [Idx] -- ^ Multi-relocate > mode > + | MultiAllocate [(Instance.Instance, Cluster.AllocDetails)] > + -- ^ Multi-allocate > mode > deriving (Show) > > -- | A complete request, as received from Ganeti. > diff --git a/src/Ganeti/OpCodes.hs b/src/Ganeti/OpCodes.hs > index 0cb840c..672e597 100644 > --- a/src/Ganeti/OpCodes.hs > +++ b/src/Ganeti/OpCodes.hs > @@ -428,6 +428,7 @@ $(genOpCode "OpCode" > , pInstBeParams > , pInstDisks > , pOptDiskTemplate > + , pOptGroupName > , pFileDriver > , pFileStorageDir > , pInstHvParams > @@ -850,6 +851,7 @@ $(genOpCode "OpCode" > , pTargetGroups > , pIAllocatorSpindleUse > , pIAllocatorCount > + , pOptGroupName > ], > "iallocator") > , ("OpTestJqueue", > diff --git a/test/hs/Test/Ganeti/OpCodes.hs > b/test/hs/Test/Ganeti/OpCodes.hs > index da791ad..70bfc63 100644 > --- a/test/hs/Test/Ganeti/OpCodes.hs > +++ b/test/hs/Test/Ganeti/OpCodes.hs > @@ -268,6 +268,7 @@ instance Arbitrary OpCodes.OpCode where > <*> pure emptyJSObject -- beparams > <*> arbitrary -- disks > <*> arbitrary -- disk_template > + <*> genMaybe genNameNE -- group_name > <*> arbitrary -- file_driver > <*> genMaybe genNameNE -- file_storage_dir > <*> pure emptyJSObject -- hvparams > @@ -412,7 +413,8 @@ instance Arbitrary OpCodes.OpCode where > (genTags >>= mapM mkNonEmpty) <*> > arbitrary <*> arbitrary <*> genMaybe genNameNE <*> > arbitrary <*> genMaybe genNodeNamesNE <*> arbitrary <*> > - genMaybe genNamesNE <*> arbitrary <*> arbitrary > + genMaybe genNamesNE <*> arbitrary <*> arbitrary <*> > + genMaybe genNameNE > "OP_TEST_JQUEUE" -> > OpCodes.OpTestJqueue <$> arbitrary <*> arbitrary <*> > resize 20 (listOf genFQDN) <*> arbitrary > diff --git a/test/py/cmdlib/test_unittest.py > b/test/py/cmdlib/test_unittest.py > index 47fc761..bd2d68e 100644 > --- a/test/py/cmdlib/test_unittest.py > +++ b/test/py/cmdlib/test_unittest.py > @@ -126,6 +126,7 @@ class TestLUTestAllocator(CmdlibTestCase): > memory=0, > disk_template=constants.DT_DISKLESS, > os="mock_os", > + group_name="default", > vcpus=1) > self.valid_multi_alloc_op = \ > self.CopyOpCode(self.base_op, > @@ -134,6 +135,7 @@ class TestLUTestAllocator(CmdlibTestCase): > memory=0, > disk_template=constants.DT_DISKLESS, > os="mock_os", > + group_name="default", > vcpus=1) > self.valid_reloc_op = \ > self.CopyOpCode(self.base_op, > -- > 1.9.1.423.g4596e3a > >
