On Thu, May 23, 2013 at 1:03 PM, Guido Trotter <[email protected]> wrote:
> > > > On Wed, May 22, 2013 at 5:28 PM, Thomas Thrainer <[email protected]>wrote: > >> Add a NIC type and extend the Instance type by a list of NIC's. Parse >> the NIC's in allocation requests and store them for now. Later patches >> will make use of this field in order to ensure that the requested >> instance is only placed in node groups wich are connected to those >> networks. >> >> Signed-off-by: Thomas Thrainer <[email protected]> >> --- >> Makefile.am | 1 + >> src/Ganeti/HTools/Backend/IAlloc.hs | 23 ++++++- >> src/Ganeti/HTools/Backend/Luxi.hs | 2 +- >> src/Ganeti/HTools/Backend/Rapi.hs | 4 +- >> src/Ganeti/HTools/Backend/Text.hs | 2 +- >> src/Ganeti/HTools/Instance.hs | 7 ++- >> src/Ganeti/HTools/Nic.hs | 77 >> +++++++++++++++++++++++ >> src/Ganeti/HTools/Program/Hspace.hs | 4 +- >> test/data/htools/hail-alloc-invalid-twodisks.json | 3 +- >> test/data/htools/hail-alloc-twodisks.json | 3 +- >> test/hs/Test/Ganeti/HTools/Instance.hs | 2 +- >> test/hs/Test/Ganeti/TestHTools.hs | 2 +- >> 12 files changed, 117 insertions(+), 13 deletions(-) >> create mode 100644 src/Ganeti/HTools/Nic.hs >> >> diff --git a/Makefile.am b/Makefile.am >> index ffdee66..507ffbe 100644 >> --- a/Makefile.am >> +++ b/Makefile.am >> @@ -548,6 +548,7 @@ HS_LIB_SRCS = \ >> src/Ganeti/HTools/Group.hs \ >> src/Ganeti/HTools/Instance.hs \ >> src/Ganeti/HTools/Loader.hs \ >> + src/Ganeti/HTools/Nic.hs \ >> src/Ganeti/HTools/Node.hs \ >> src/Ganeti/HTools/PeerMap.hs \ >> src/Ganeti/HTools/Program/Hail.hs \ >> diff --git a/src/Ganeti/HTools/Backend/IAlloc.hs >> b/src/Ganeti/HTools/Backend/IAlloc.hs >> index 823e0c9..22dad86 100644 >> --- a/src/Ganeti/HTools/Backend/IAlloc.hs >> +++ b/src/Ganeti/HTools/Backend/IAlloc.hs >> @@ -44,6 +44,7 @@ import qualified Ganeti.HTools.Container as Container >> import qualified Ganeti.HTools.Group as Group >> import qualified Ganeti.HTools.Node as Node >> import qualified Ganeti.HTools.Instance as Instance >> +import qualified Ganeti.HTools.Nic as Nic >> import qualified Ganeti.Constants as C >> import Ganeti.HTools.CLI >> import Ganeti.HTools.Loader >> @@ -56,6 +57,22 @@ import Ganeti.Utils >> -- | Type alias for the result of an IAllocator call. >> type IAllocResult = (String, JSValue, Node.List, Instance.List) >> >> +-- | Parse a NIC within an instance (in a creation request) >> +parseNic :: String -> JSRecord -> Result Nic.Nic >> +parseNic n a = do >> + mac <- maybeFromObj a "mac" >> + ip <- maybeFromObj a "ip" >> + mode <- maybeFromObj a "mode" >>= \m -> case m of >> + Just "bridged" -> Ok $ Just Nic.Bridged >> + Just "routed" -> Ok $ Just Nic.Routed >> + Just "openvswitch" -> Ok $ Just Nic.OpenVSwitch >> + Nothing -> Ok Nothing >> + _ -> Bad $ "invalid NIC mode in instance " ++ n >> + link <- maybeFromObj a "link" >> + bridge <- maybeFromObj a "bridge" >> + network <- maybeFromObj a "network" >> + return (Nic.create mac ip mode link bridge network) >> + >> -- | Parse the basic specifications of an instance. >> -- >> -- Instances in the cluster instance list and the instance in an >> @@ -75,7 +92,11 @@ parseBaseInstance n a = do >> tags <- extract "tags" >> dt <- extract "disk_template" >> su <- extract "spindle_use" >> - return (n, Instance.create n mem disk disks vcpus Running tags True 0 >> 0 dt su) >> + nics <- extract "nics" >>= toArray >>= asObjectList >>= >> + mapM (parseNic n . fromJSObject) >> + return >> + (n, >> + Instance.create n mem disk disks vcpus Running tags True 0 0 dt su >> nics) >> >> -- | Parses an instance as found in the cluster instance list. >> parseInstance :: NameAssoc -- ^ The node name-to-index association list >> diff --git a/src/Ganeti/HTools/Backend/Luxi.hs >> b/src/Ganeti/HTools/Backend/Luxi.hs >> index b36376f..7227844 100644 >> --- a/src/Ganeti/HTools/Backend/Luxi.hs >> +++ b/src/Ganeti/HTools/Backend/Luxi.hs >> @@ -173,7 +173,7 @@ parseInstance ktn [ name, disk, mem, vcpus >> xdt <- convert "disk_template" disk_template >> xsu <- convert "be/spindle_use" su >> let inst = Instance.create xname xmem xdisk [xdisk] xvcpus >> - xrunning xtags xauto_balance xpnode snode xdt xsu >> + xrunning xtags xauto_balance xpnode snode xdt xsu [] >> return (xname, inst) >> >> parseInstance _ v = fail ("Invalid instance query result: " ++ show v) >> diff --git a/src/Ganeti/HTools/Backend/Rapi.hs >> b/src/Ganeti/HTools/Backend/Rapi.hs >> index aa8ab1b..5049198 100644 >> --- a/src/Ganeti/HTools/Backend/Rapi.hs >> +++ b/src/Ganeti/HTools/Backend/Rapi.hs >> @@ -140,7 +140,7 @@ parseInstance ktn a = do >> dt <- extract "disk_template" a >> su <- extract "spindle_use" beparams >> let inst = Instance.create name mem disk disks vcpus running tags >> - auto_balance pnode snode dt su >> + auto_balance pnode snode dt su [] >> return (name, inst) >> >> -- | Construct a node from a JSON object. >> @@ -230,7 +230,7 @@ parseData (group_body, node_body, inst_body, >> info_body) = do >> let (node_names, node_idx) = assignIndices node_data >> inst_data <- inst_body >>= getInstances node_names >> let (_, inst_idx) = assignIndices inst_data >> - (tags, ipolicy, master) <- >> + (tags, ipolicy, master) <- >> info_body >>= >> (fromJResult "Parsing cluster info" . decodeStrict) >>= >> parseCluster >> diff --git a/src/Ganeti/HTools/Backend/Text.hs >> b/src/Ganeti/HTools/Backend/Text.hs >> index 17674a7..06e1a74 100644 >> --- a/src/Ganeti/HTools/Backend/Text.hs >> +++ b/src/Ganeti/HTools/Backend/Text.hs >> @@ -245,7 +245,7 @@ loadInst ktn [ name, mem, dsk, vcpus, status, >> auto_bal, pnode, snode >> " has same primary and secondary node - " ++ pnode >> let vtags = commaSplit tags >> newinst = Instance.create name vmem vdsk [vdsk] vvcpus vstatus >> vtags >> - auto_balance pidx sidx disk_template spindle_use >> + auto_balance pidx sidx disk_template spindle_use [] >> return (name, newinst) >> >> loadInst ktn [ name, mem, dsk, vcpus, status, auto_bal, pnode, snode >> diff --git a/src/Ganeti/HTools/Instance.hs b/src/Ganeti/HTools/Instance.hs >> index 4ba3e91..00c3d9a 100644 >> --- a/src/Ganeti/HTools/Instance.hs >> +++ b/src/Ganeti/HTools/Instance.hs >> @@ -60,6 +60,7 @@ module Ganeti.HTools.Instance >> import Ganeti.BasicTypes >> import qualified Ganeti.HTools.Types as T >> import qualified Ganeti.HTools.Container as Container >> +import Ganeti.HTools.Nic (Nic) >> >> import Ganeti.Utils >> >> @@ -85,6 +86,7 @@ data Instance = Instance >> , allTags :: [String] -- ^ List of all instance tags >> , exclTags :: [String] -- ^ List of instance exclusion tags >> , arPolicy :: T.AutoRepairPolicy -- ^ Instance's auto-repair policy >> + , nics :: [Nic] -- ^ NICs of the instance >> } deriving (Show, Eq) >> >> instance T.Element Instance where >> @@ -165,9 +167,9 @@ type List = Container.Container Instance >> -- later (via 'setIdx' for example). >> create :: String -> Int -> Int -> [Int] -> Int -> T.InstanceStatus >> -> [String] -> Bool -> T.Ndx -> T.Ndx -> T.DiskTemplate -> Int >> - -> Instance >> + -> [Nic] -> Instance >> create name_init mem_init dsk_init disks_init vcpus_init run_init >> tags_init >> - auto_balance_init pn sn dt su = >> + auto_balance_init pn sn dt su nics_init = >> Instance { name = name_init >> , alias = name_init >> , mem = mem_init >> @@ -186,6 +188,7 @@ create name_init mem_init dsk_init disks_init >> vcpus_init run_init tags_init >> , allTags = tags_init >> , exclTags = [] >> , arPolicy = T.ArNotEnabled >> + , nics = nics_init >> } >> >> -- | Changes the index. >> diff --git a/src/Ganeti/HTools/Nic.hs b/src/Ganeti/HTools/Nic.hs >> new file mode 100644 >> index 0000000..5919239 >> --- /dev/null >> +++ b/src/Ganeti/HTools/Nic.hs >> @@ -0,0 +1,77 @@ >> +{-| Module describing an NIC. >> + >> +The NIC data type only holds data about a NIC, but does not provide any >> +logic. >> + >> +-} >> + >> +{- >> + >> +Copyright (C) 2009, 2010, 2011, 2012, 2013 Google Inc. >> + >> +This program is free software; you can redistribute it and/or modify >> +it under the terms of the GNU General Public License as published by >> +the Free Software Foundation; either version 2 of the License, or >> +(at your option) any later version. >> + >> +This program is distributed in the hope that it will be useful, but >> +WITHOUT ANY WARRANTY; without even the implied warranty of >> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> +General Public License for more details. >> + >> +You should have received a copy of the GNU General Public License >> +along with this program; if not, write to the Free Software >> +Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA >> +02110-1301, USA. >> + >> +-} >> + >> +module Ganeti.HTools.Nic >> + ( Nic(..) >> + , Mode(..) >> + , List >> + , create >> + ) where >> + >> +import qualified Ganeti.HTools.Container as Container >> + >> +import qualified Ganeti.HTools.Types as T >> + >> +-- * Type declarations >> + >> +data Mode = Bridged | Routed | OpenVSwitch deriving (Show, Eq) >> + >> +-- | The NIC type. >> +data Nic = Nic >> + { mac :: Maybe String -- ^ MAC address of the NIC >> + , ip :: Maybe String -- ^ IP address of the NIC >> + , mode :: Maybe Mode -- ^ the mode the NIC operates in >> + , link :: Maybe String -- ^ the link of the NIC >> + , bridge :: Maybe String -- ^ the bridge this NIC is connected >> to if >> + -- the mode is Bridged >> + , network :: Maybe T.NetworkID -- ^ network UUID if this NIC is >> connected >> + -- to a network >> + } deriving (Show, Eq) >> + >> > > I wonder if it would make sense to use Objects.PartialNic for this. > True, we don't in other parts of htools, but this was (in part) because we > didn't have Objects implemented then. > Maybe using those we can remove some duplicate code? (later, not in this > patch series) > > I tried to explain the rational for the new type in the comment, and added a TODO for reworking it when Objects.* are touched anyways. Interdiff: diff --git a/src/Ganeti/HTools/Nic.hs b/src/Ganeti/HTools/Nic.hs index 5919239..02f634d 100644 --- a/src/Ganeti/HTools/Nic.hs +++ b/src/Ganeti/HTools/Nic.hs @@ -42,6 +42,17 @@ import qualified Ganeti.HTools.Types as T data Mode = Bridged | Routed | OpenVSwitch deriving (Show, Eq) -- | The NIC type. +-- +-- It holds the data for a NIC as it is provided via the IAllocator protocol +-- for an instance creation request. All data in those request is optional, +-- that's why all fields are Maybe's. +-- +-- TODO: Another name might be more appropriate for this type, as for example +-- RequestedNic. But this type is used as a field in the Instance type, which +-- is not named RequestedInstance, so such a name would be weird. PartialNic +-- already exists in Objects, but doesn't fit the bill here, as it contains +-- a required field (mac). Objects and the types therein are subject to being +-- reworked, so until then this type is left as is. data Nic = Nic { mac :: Maybe String -- ^ MAC address of the NIC , ip :: Maybe String -- ^ IP address of the NIC > +-- | A simple name for an instance map. >> +type List = Container.Container Nic >> + >> +-- * Initialization >> + >> +-- | Create a NIC. >> +-- >> +create :: Maybe String >> + -> Maybe String >> + -> Maybe Mode >> + -> Maybe String >> + -> Maybe String >> + -> Maybe T.NetworkID >> + -> Nic >> +create mac_init ip_init mode_init link_init bridge_init network_init = >> + Nic { mac = mac_init >> + , ip = ip_init >> + , mode = mode_init >> + , link = link_init >> + , bridge = bridge_init >> + , network = network_init >> + } >> diff --git a/src/Ganeti/HTools/Program/Hspace.hs >> b/src/Ganeti/HTools/Program/Hspace.hs >> index 78f0687..42c0aab 100644 >> --- a/src/Ganeti/HTools/Program/Hspace.hs >> +++ b/src/Ganeti/HTools/Program/Hspace.hs >> @@ -394,9 +394,9 @@ runAllocation cdata stop_allocation actual_result >> spec dt mode opts = do >> -- of the disk space to individual disks), sensible defaults are guessed >> (e.g., >> -- having a single disk). >> instFromSpec :: RSpec -> DiskTemplate -> Int -> Instance.Instance >> -instFromSpec spx = >> +instFromSpec spx dt su = >> Instance.create "new" (rspecMem spx) (rspecDsk spx) [rspecDsk spx] >> - (rspecCpu spx) Running [] True (-1) (-1) >> + (rspecCpu spx) Running [] True (-1) (-1) dt su [] >> >> combineTiered :: Maybe Int -> Cluster.AllocNodes -> Cluster.AllocResult >> -> >> Instance.Instance -> Result Cluster.AllocResult >> diff --git a/test/data/htools/hail-alloc-invalid-twodisks.json >> b/test/data/htools/hail-alloc-invalid-twodisks.json >> index 3914a5d..4a15e64 100644 >> --- a/test/data/htools/hail-alloc-invalid-twodisks.json >> +++ b/test/data/htools/hail-alloc-invalid-twodisks.json >> @@ -85,7 +85,8 @@ >> "spindle_use": 2, >> "tags": [], >> "type": "allocate", >> - "vcpus": 1 >> + "vcpus": 1, >> + "nics": [] >> }, >> "version": 2 >> } >> diff --git a/test/data/htools/hail-alloc-twodisks.json >> b/test/data/htools/hail-alloc-twodisks.json >> index e4614a7..5e9e196 100644 >> --- a/test/data/htools/hail-alloc-twodisks.json >> +++ b/test/data/htools/hail-alloc-twodisks.json >> @@ -85,7 +85,8 @@ >> "spindle_use": 2, >> "tags": [], >> "type": "allocate", >> - "vcpus": 1 >> + "vcpus": 1, >> + "nics": [] >> }, >> "version": 2 >> } >> diff --git a/test/hs/Test/Ganeti/HTools/Instance.hs >> b/test/hs/Test/Ganeti/HTools/Instance.hs >> index ca8f682..5e6e36a 100644 >> --- a/test/hs/Test/Ganeti/HTools/Instance.hs >> +++ b/test/hs/Test/Ganeti/HTools/Instance.hs >> @@ -62,7 +62,7 @@ genInstanceSmallerThan lim_mem lim_dsk lim_cpu = do >> sn <- arbitrary >> vcpus <- choose (0, lim_cpu) >> dt <- arbitrary >> - return $ Instance.create name mem dsk [dsk] vcpus run_st [] True pn sn >> dt 1 >> + return $ Instance.create name mem dsk [dsk] vcpus run_st [] True pn sn >> dt 1 [] >> >> -- | Generates an instance smaller than a node. >> genInstanceSmallerThanNode :: Node.Node -> Gen Instance.Instance >> diff --git a/test/hs/Test/Ganeti/TestHTools.hs >> b/test/hs/Test/Ganeti/TestHTools.hs >> index b51df04..eca4f57 100644 >> --- a/test/hs/Test/Ganeti/TestHTools.hs >> +++ b/test/hs/Test/Ganeti/TestHTools.hs >> @@ -100,7 +100,7 @@ defGroupAssoc = Map.singleton (Group.uuid defGroup) >> (Group.idx defGroup) >> createInstance :: Int -> Int -> Int -> Instance.Instance >> createInstance mem dsk vcpus = >> Instance.create "inst-unnamed" mem dsk [dsk] vcpus Types.Running [] >> True (-1) >> - (-1) Types.DTDrbd8 1 >> + (-1) Types.DTDrbd8 1 [] >> >> -- | Create a small cluster by repeating a node spec. >> makeSmallCluster :: Node.Node -> Int -> Node.List >> -- >> 1.8.2.1 >> >> > Thanks, > > Guido > > -- Thomas Thrainer | Software Engineer | [email protected] | 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
