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)


> +-- | 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

Reply via email to