LGTM

Thanks,

Guido


On Mon, Feb 11, 2013 at 1:42 PM, Dimitris Aragiorgis <[email protected]> wrote:
> This slot was not used by Ganeti so the same info can be
> provided via tags. In order not to break configuration data
> we add a FromDict() method in Network config object that
> removes the deprecated network_type (if found) and then invoke
> the parent's method (which does the validation).
>
> Signed-off-by: Dimitris Aragiorgis <[email protected]>
> ---
>  doc/hooks.rst               |   11 +++++------
>  doc/rapi.rst                |    1 -
>  lib/cli.py                  |    5 -----
>  lib/client/gnt_network.py   |   13 +++++--------
>  lib/cmdlib.py               |   19 +------------------
>  lib/constants.py            |   10 ----------
>  lib/hypervisor/hv_kvm.py    |    6 ++----
>  lib/objects.py              |   15 ++++++++++++---
>  lib/opcodes.py              |    5 -----
>  lib/query.py                |    1 -
>  lib/rapi/client.py          |    3 +--
>  lib/rapi/rlib2.py           |    2 +-
>  man/ganeti-os-interface.rst |    3 ---
>  man/gnt-network.rst         |    4 ----
>  14 files changed, 27 insertions(+), 71 deletions(-)
>
> diff --git a/doc/hooks.rst b/doc/hooks.rst
> index a9f073e..3f27953 100644
> --- a/doc/hooks.rst
> +++ b/doc/hooks.rst
> @@ -224,7 +224,7 @@ Adds a network to the cluster.
>
>  :directory: network-add
>  :env. vars: NETWORK_NAME, NETWORK_SUBNET, NETWORK_GATEWAY, NETWORK_SUBNET6,
> -            NETWORK_GATEWAY6, NETWORK_TYPE, NETWORK_MAC_PREFIX, NETWORK_TAGS
> +            NETWORK_GATEWAY6, NETWORK_MAC_PREFIX, NETWORK_TAGS
>  :pre-execution: master node
>  :post-execution: master node
>
> @@ -247,7 +247,7 @@ Connects a network to a nodegroup.
>  :env. vars: GROUP_NAME, NETWORK_NAME,
>              GROUP_NETWORK_MODE, GROUP_NETWORK_LINK,
>              NETWORK_SUBNET, NETWORK_GATEWAY, NETWORK_SUBNET6,
> -            NETWORK_GATEWAY6, NETWORK_TYPE, NETWORK_MAC_PREFIX, NETWORK_TAGS
> +            NETWORK_GATEWAY6, NETWORK_MAC_PREFIX, NETWORK_TAGS
>  :pre-execution: nodegroup nodes
>  :post-execution: nodegroup nodes
>
> @@ -261,7 +261,7 @@ Disconnects a network from a nodegroup.
>  :env. vars: GROUP_NAME, NETWORK_NAME,
>              GROUP_NETWORK_MODE, GROUP_NETWORK_LINK,
>              NETWORK_SUBNET, NETWORK_GATEWAY, NETWORK_SUBNET6,
> -            NETWORK_GATEWAY6, NETWORK_TYPE, NETWORK_MAC_PREFIX, NETWORK_TAGS
> +            NETWORK_GATEWAY6, NETWORK_MAC_PREFIX, NETWORK_TAGS
>  :pre-execution: nodegroup nodes
>  :post-execution: nodegroup nodes
>
> @@ -273,7 +273,7 @@ Modifies a network.
>
>  :directory: network-modify
>  :env. vars: NETWORK_NAME, NETWORK_SUBNET, NETWORK_GATEWAY, NETWORK_SUBNET6,
> -            NETWORK_GATEWAY6, NETWORK_TYPE, NETWORK_MAC_PREFIX, NETWORK_TAGS
> +            NETWORK_GATEWAY6, NETWORK_MAC_PREFIX, NETWORK_TAGS
>  :pre-execution: master node
>  :post-execution: master node
>
> @@ -290,8 +290,7 @@ INSTANCE_NICn_NETWORK, INSTANCE_NICn_NETWORK_FAMILY,
>  INSTANCE_NICn_NETWORK_UUID, INSTANCE_NICn_NETWORK_SUBNET,
>  INSTANCE_NICn_NETWORK_GATEWAY, INSTANCE_NICn_NETWORK_SUBNET6,
>  INSTANCE_NICn_NETWORK_GATEWAY6, INSTANCE_NICn_NETWORK_MAC_PREFIX,
> -INSTANCE_NICn_NETWORK_TYPE, INSTANCE_DISK_COUNT, INSTANCE_DISKn_SIZE,
> -INSTANCE_DISKn_MODE.
> +INSTANCE_DISK_COUNT, INSTANCE_DISKn_SIZE, INSTANCE_DISKn_MODE.
>
>  The INSTANCE_NICn_* and INSTANCE_DISKn_* variables represent the
>  properties of the *n* -th NIC and disk, and are zero-indexed.
> diff --git a/doc/rapi.rst b/doc/rapi.rst
> index f70f8ca..11c64c1 100644
> --- a/doc/rapi.rst
> +++ b/doc/rapi.rst
> @@ -698,7 +698,6 @@ Example::
>          'name': 'nat',
>          'network': '10.0.0.0/28',
>          'network6': None,
> -        'network_type': 'private',
>          'reserved_count': 3,
>          'tags': ['nfdhcpd'],
>          …
> diff --git a/lib/cli.py b/lib/cli.py
> index 950578e..e5cd524 100644
> --- a/lib/cli.py
> +++ b/lib/cli.py
> @@ -117,7 +117,6 @@ __all__ = [
>    "NET_OPT",
>    "NETWORK_OPT",
>    "NETWORK6_OPT",
> -  "NETWORK_TYPE_OPT",
>    "NEW_CLUSTER_CERT_OPT",
>    "NEW_CLUSTER_DOMAIN_SECRET_OPT",
>    "NEW_CONFD_HMAC_KEY_OPT",
> @@ -1527,10 +1526,6 @@ REMOVE_RESERVED_IPS_OPT = 
> cli_option("--remove-reserved-ips",
>                                       help="Comma-delimited list of"
>                                       " reserved IPs to remove")
>
> -NETWORK_TYPE_OPT = cli_option("--network-type",
> -                              action="store", default=None, 
> dest="network_type",
> -                              help="Network type: private, public, None")
> -
>  NETWORK6_OPT = cli_option("--network6",
>                            action="store", default=None, dest="network6",
>                            help="IP network in CIDR notation")
> diff --git a/lib/client/gnt_network.py b/lib/client/gnt_network.py
> index 7210ab3..2407f39 100644
> --- a/lib/client/gnt_network.py
> +++ b/lib/client/gnt_network.py
> @@ -36,7 +36,7 @@ from ganeti import errors
>
>  #: default list of fields for L{ListNetworks}
>  _LIST_DEF_FIELDS = ["name", "network", "gateway",
> -                    "network_type", "mac_prefix", "group_list", "tags"]
> +                    "mac_prefix", "group_list", "tags"]
>
>
>  def _HandleReservedIPs(ips):
> @@ -77,7 +77,6 @@ def AddNetwork(opts, args):
>                              gateway6=opts.gateway6,
>                              network6=opts.network6,
>                              mac_prefix=opts.mac_prefix,
> -                            network_type=opts.network_type,
>                              add_reserved_ips=reserved_ips,
>                              conflicts_check=opts.conflicts_check,
>                              tags=tags)
> @@ -202,7 +201,7 @@ def ShowNetworkConfig(_, args):
>    cl = GetClient()
>    result = cl.QueryNetworks(fields=["name", "network", "gateway",
>                                      "network6", "gateway6",
> -                                    "mac_prefix", "network_type",
> +                                    "mac_prefix",
>                                      "free_count", "reserved_count",
>                                      "map", "group_list", "inst_list",
>                                      "external_reservations",
> @@ -210,7 +209,7 @@ def ShowNetworkConfig(_, args):
>                              names=args, use_locking=False)
>
>    for (name, network, gateway, network6, gateway6,
> -       mac_prefix, network_type, free_count, reserved_count,
> +       mac_prefix, free_count, reserved_count,
>         mapping, group_list, instances, ext_res, serial, uuid) in result:
>      size = free_count + reserved_count
>      ToStdout("Network name: %s", name)
> @@ -221,7 +220,6 @@ def ShowNetworkConfig(_, args):
>      ToStdout("  IPv6 Subnet: %s", network6)
>      ToStdout("  IPv6 Gateway: %s", gateway6)
>      ToStdout("  Mac Prefix: %s", mac_prefix)
> -    ToStdout("  Type: %s", network_type)
>      ToStdout("  Size: %d", size)
>      ToStdout("  Free: %d (%.2f%%)", free_count,
>               100 * float(free_count) / float(size))
> @@ -277,7 +275,6 @@ def SetNetworkParams(opts, args):
>      "add_reserved_ips": _HandleReservedIPs(opts.add_reserved_ips),
>      "remove_reserved_ips": _HandleReservedIPs(opts.remove_reserved_ips),
>      "mac_prefix": opts.mac_prefix,
> -    "network_type": opts.network_type,
>      "gateway6": opts.gateway6,
>      "network6": opts.network6,
>    }
> @@ -312,7 +309,7 @@ commands = {
>    "add": (
>      AddNetwork, ARGS_ONE_NETWORK,
>      [DRY_RUN_OPT, NETWORK_OPT, GATEWAY_OPT, ADD_RESERVED_IPS_OPT,
> -     MAC_PREFIX_OPT, NETWORK_TYPE_OPT, NETWORK6_OPT, GATEWAY6_OPT,
> +     MAC_PREFIX_OPT, NETWORK6_OPT, GATEWAY6_OPT,
>       NOCONFLICTSCHECK_OPT, TAG_ADD_OPT, PRIORITY_OPT, SUBMIT_OPT],
>      "<network_name>", "Add a new IP network to the cluster"),
>    "list": (
> @@ -331,7 +328,7 @@ commands = {
>    "modify": (
>      SetNetworkParams, ARGS_ONE_NETWORK,
>      [DRY_RUN_OPT, SUBMIT_OPT, ADD_RESERVED_IPS_OPT, REMOVE_RESERVED_IPS_OPT,
> -     GATEWAY_OPT, MAC_PREFIX_OPT, NETWORK_TYPE_OPT, NETWORK6_OPT, 
> GATEWAY6_OPT,
> +     GATEWAY_OPT, MAC_PREFIX_OPT, NETWORK6_OPT, GATEWAY6_OPT,
>       PRIORITY_OPT],
>      "<network_name>", "Alters the parameters of a network"),
>    "connect": (
> diff --git a/lib/cmdlib.py b/lib/cmdlib.py
> index 5ddecff..bb1e01d 100644
> --- a/lib/cmdlib.py
> +++ b/lib/cmdlib.py
> @@ -1385,7 +1385,7 @@ def _ExpandInstanceName(cfg, name):
>
>
>  def _BuildNetworkHookEnv(name, subnet, gateway, network6, gateway6,
> -                         network_type, mac_prefix, tags):
> +                         mac_prefix, tags):
>    """Builds network related env variables for hooks
>
>    This builds the hook environment from individual variables.
> @@ -1400,8 +1400,6 @@ def _BuildNetworkHookEnv(name, subnet, gateway, 
> network6, gateway6,
>    @param network6: the ipv6 subnet
>    @type gateway6: string
>    @param gateway6: the ipv6 gateway
> -  @type network_type: string
> -  @param network_type: the type of the network
>    @type mac_prefix: string
>    @param mac_prefix: the mac_prefix
>    @type tags: list
> @@ -1421,8 +1419,6 @@ def _BuildNetworkHookEnv(name, subnet, gateway, 
> network6, gateway6,
>      env["NETWORK_GATEWAY6"] = gateway6
>    if mac_prefix:
>      env["NETWORK_MAC_PREFIX"] = mac_prefix
> -  if network_type:
> -    env["NETWORK_TYPE"] = network_type
>    if tags:
>      env["NETWORK_TAGS"] = " ".join(tags)
>
> @@ -16226,7 +16222,6 @@ class LUNetworkAdd(LogicalUnit):
>        "network6": self.op.network6,
>        "gateway6": self.op.gateway6,
>        "mac_prefix": self.op.mac_prefix,
> -      "network_type": self.op.network_type,
>        "tags": self.op.tags,
>        }
>      return _BuildNetworkHookEnv(**args) # pylint: disable=W0142
> @@ -16241,7 +16236,6 @@ class LUNetworkAdd(LogicalUnit):
>                             network6=self.op.network6,
>                             gateway6=self.op.gateway6,
>                             mac_prefix=self.op.mac_prefix,
> -                           network_type=self.op.network_type,
>                             uuid=self.network_uuid,
>                             family=constants.IP4_VERSION)
>      # Initialize the associated address pool
> @@ -16383,7 +16377,6 @@ class LUNetworkSetParams(LogicalUnit):
>      """
>      self.network = self.cfg.GetNetwork(self.network_uuid)
>      self.gateway = self.network.gateway
> -    self.network_type = self.network.network_type
>      self.mac_prefix = self.network.mac_prefix
>      self.network6 = self.network.network6
>      self.gateway6 = self.network.gateway6
> @@ -16401,12 +16394,6 @@ class LUNetworkSetParams(LogicalUnit):
>                                       " reserved" % self.gateway,
>                                       errors.ECODE_STATE)
>
> -    if self.op.network_type:
> -      if self.op.network_type == constants.VALUE_NONE:
> -        self.network_type = None
> -      else:
> -        self.network_type = self.op.network_type
> -
>      if self.op.mac_prefix:
>        if self.op.mac_prefix == constants.VALUE_NONE:
>          self.mac_prefix = None
> @@ -16437,7 +16424,6 @@ class LUNetworkSetParams(LogicalUnit):
>        "network6": self.network6,
>        "gateway6": self.gateway6,
>        "mac_prefix": self.mac_prefix,
> -      "network_type": self.network_type,
>        "tags": self.tags,
>        }
>      return _BuildNetworkHookEnv(**args) # pylint: disable=W0142
> @@ -16497,9 +16483,6 @@ class LUNetworkSetParams(LogicalUnit):
>      if self.op.gateway6:
>        self.network.gateway6 = self.gateway6
>
> -    if self.op.network_type:
> -      self.network.network_type = self.network_type
> -
>      self.pool.Validate()
>
>      self.cfg.Update(self.network, feedback_fn)
> diff --git a/lib/constants.py b/lib/constants.py
> index b99b12c..2d7a722 100644
> --- a/lib/constants.py
> +++ b/lib/constants.py
> @@ -1213,16 +1213,6 @@ NIC_VALID_MODES = compat.UniqueFrozenset([
>  RESERVE_ACTION = "reserve"
>  RELEASE_ACTION = "release"
>
> -# An extra description of the network.
> -# Can be used by hooks/kvm-vif-bridge to apply different rules
> -NETWORK_TYPE_PRIVATE = "private"
> -NETWORK_TYPE_PUBLIC = "public"
> -
> -NETWORK_VALID_TYPES = compat.UniqueFrozenset([
> -  NETWORK_TYPE_PRIVATE,
> -  NETWORK_TYPE_PUBLIC,
> -  ])
> -
>  NICS_PARAMETER_TYPES = {
>    NIC_MODE: VTYPE_STRING,
>    NIC_LINK: VTYPE_STRING,
> diff --git a/lib/hypervisor/hv_kvm.py b/lib/hypervisor/hv_kvm.py
> index 26aacfd..2fe5b43 100644
> --- a/lib/hypervisor/hv_kvm.py
> +++ b/lib/hypervisor/hv_kvm.py
> @@ -149,7 +149,7 @@ def _OpenTap(vnet_hdr=True):
>
>
>  def _BuildNetworkEnv(name, network, gateway, network6, gateway6,
> -                     network_type, mac_prefix, tags, env):
> +                     mac_prefix, tags, env):
>    """Build environment variables concerning a Network.
>
>    """
> @@ -165,8 +165,6 @@ def _BuildNetworkEnv(name, network, gateway, network6, 
> gateway6,
>      env["NETWORK_GATEWAY6"] = gateway6
>    if mac_prefix:
>      env["NETWORK_MAC_PREFIX"] = mac_prefix
> -  if network_type:
> -    env["NETWORK_TYPE"] = network_type
>    if tags:
>      env["NETWORK_TAGS"] = " ".join(tags)
>
> @@ -857,7 +855,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
>      if nic.network:
>        n = objects.Network.FromDict(nic.netinfo)
>        _BuildNetworkEnv(nic.network, n.network, n.gateway,
> -                       n.network6, n.gateway6, n.network_type,
> +                       n.network6, n.gateway6,
>                         n.mac_prefix, n.tags, env)
>
>      if nic.nicparams[constants.NIC_MODE] == constants.NIC_MODE_BRIDGED:
> diff --git a/lib/objects.py b/lib/objects.py
> index 60f6afc..6c6c794 100644
> --- a/lib/objects.py
> +++ b/lib/objects.py
> @@ -2027,7 +2027,6 @@ class Network(TaggableObject):
>    __slots__ = [
>      "name",
>      "serial_no",
> -    "network_type",
>      "mac_prefix",
>      "family",
>      "network",
> @@ -2062,11 +2061,21 @@ class Network(TaggableObject):
>        result["%sNETWORK_GATEWAY6" % prefix] = self.gateway6
>      if self.mac_prefix:
>        result["%sNETWORK_MAC_PREFIX" % prefix] = self.mac_prefix
> -    if self.network_type:
> -      result["%sNETWORK_TYPE" % prefix] = self.network_type
>
>      return result
>
> +  @classmethod
> +  def FromDict(cls, val):
> +    """Custom function for networks.
> +
> +    Remove deprecated network_type. Still this info can be passed via tags.
> +
> +    """
> +    if "network_type" in val:
> +      del val["network_type"]
> +    obj = super(Network, cls).FromDict(val)
> +    return obj
> +
>
>  class SerializableConfigParser(ConfigParser.SafeConfigParser):
>    """Simple wrapper over ConfigParse that allows serialization.
> diff --git a/lib/opcodes.py b/lib/opcodes.py
> index 2c88363..f30ec41 100644
> --- a/lib/opcodes.py
> +++ b/lib/opcodes.py
> @@ -361,8 +361,6 @@ def _CheckStorageType(storage_type):
>  _PStorageType = ("storage_type", ht.NoDefault, _CheckStorageType,
>                   "Storage type")
>
> -_CheckNetworkType = ht.TElemOf(constants.NETWORK_VALID_TYPES)
> -
>
>  @ht.WithDesc("IPv4 network")
>  def _CheckCIDRNetNotation(value):
> @@ -2065,7 +2063,6 @@ class OpNetworkAdd(OpCode):
>    OP_DSC_FIELD = "network_name"
>    OP_PARAMS = [
>      _PNetworkName,
> -    ("network_type", None, ht.TMaybe(_CheckNetworkType), "Network type"),
>      ("network", ht.NoDefault, _TIpNetwork4, "IPv4 subnet"),
>      ("gateway", None, ht.TMaybe(_TIpAddress4), "IPv4 gateway"),
>      ("network6", None, ht.TMaybe(_TIpNetwork6), "IPv6 subnet"),
> @@ -2099,8 +2096,6 @@ class OpNetworkSetParams(OpCode):
>    OP_DSC_FIELD = "network_name"
>    OP_PARAMS = [
>      _PNetworkName,
> -    ("network_type", None, ht.TMaybeValueNone(_CheckNetworkType),
> -     "Network type"),
>      ("gateway", None, ht.TMaybeValueNone(_TIpAddress4), "IPv4 gateway"),
>      ("network6", None, ht.TMaybeValueNone(_TIpNetwork6), "IPv6 subnet"),
>      ("gateway6", None, ht.TMaybeValueNone(_TIpAddress6), "IPv6 gateway"),
> diff --git a/lib/query.py b/lib/query.py
> index dc07159..ef52d08 100644
> --- a/lib/query.py
> +++ b/lib/query.py
> @@ -2531,7 +2531,6 @@ _NETWORK_SIMPLE_FIELDS = {
>    "network6": ("IPv6Subnet", QFT_OTHER, 0, "IPv6 subnet"),
>    "gateway6": ("IPv6Gateway", QFT_OTHER, 0, "IPv6 gateway"),
>    "mac_prefix": ("MacPrefix", QFT_OTHER, 0, "MAC address prefix"),
> -  "network_type": ("NetworkType", QFT_OTHER, 0, "Network type"),
>    "serial_no": ("SerialNo", QFT_NUMBER, 0, _SERIAL_NO_DOC % "Network"),
>    "uuid": ("UUID", QFT_TEXT, 0, "Network UUID"),
>    }
> diff --git a/lib/rapi/client.py b/lib/rapi/client.py
> index 452fa95..cf79711 100644
> --- a/lib/rapi/client.py
> +++ b/lib/rapi/client.py
> @@ -1768,7 +1768,7 @@ class GanetiRapiClient(object): # pylint: disable=R0904
>                               None, None)
>
>    def CreateNetwork(self, network_name, network, gateway=None, network6=None,
> -                    gateway6=None, mac_prefix=None, network_type=None,
> +                    gateway6=None, mac_prefix=None,
>                      add_reserved_ips=None, tags=None, dry_run=False):
>      """Creates a new network.
>
> @@ -1797,7 +1797,6 @@ class GanetiRapiClient(object): # pylint: disable=R0904
>        "gateway6": gateway6,
>        "network6": network6,
>        "mac_prefix": mac_prefix,
> -      "network_type": network_type,
>        "add_reserved_ips": add_reserved_ips,
>        "tags": tags,
>        }
> diff --git a/lib/rapi/rlib2.py b/lib/rapi/rlib2.py
> index d748913..3a28a84 100644
> --- a/lib/rapi/rlib2.py
> +++ b/lib/rapi/rlib2.py
> @@ -94,7 +94,7 @@ N_FIELDS = ["name", "offline", "master_candidate", 
> "drained",
>
>  NET_FIELDS = ["name", "network", "gateway",
>                "network6", "gateway6",
> -              "mac_prefix", "network_type",
> +              "mac_prefix",
>                "free_count", "reserved_count",
>                "map", "group_list", "inst_list",
>                "external_reservations", "tags",
> diff --git a/man/ganeti-os-interface.rst b/man/ganeti-os-interface.rst
> index cd7c763..ed5872f 100644
> --- a/man/ganeti-os-interface.rst
> +++ b/man/ganeti-os-interface.rst
> @@ -144,9 +144,6 @@ NIC_%d_NETWORK_GATEWAY6
>  NIC_%d_NETWORK_MAC_PREFIX
>      (Optional) If a NIC network is specified, the network's mac prefix.
>
> -NIC_%d_NETWORK_TYPE
> -    (Optional) If a NIC network is specified, the network's type.
> -
>  NIC_%d_NETWORK_TAGS
>      (Optional) If a NIC network is specified, the network's tags, space
>      separated.
> diff --git a/man/gnt-network.rst b/man/gnt-network.rst
> index 3e4a551..69848cd 100644
> --- a/man/gnt-network.rst
> +++ b/man/gnt-network.rst
> @@ -32,7 +32,6 @@ ADD
>  | [--network6=*NETWORK6*]
>  | [--gateway6=*GATEWAY6*]
>  | [--mac-prefix=*MACPREFIX*]
> -| [--network-type=*NETWORKTYPE*]
>  | [--submit]
>  | {*network*}
>
> @@ -46,8 +45,6 @@ notation.
>  The ``--gateway`` option allows you to specify the default gateway for
>  this network.
>
> -The ``--network-type`` can be none, private or public.
> -
>  IPv6 semantics can be assigned to the network via the ``--network6`` and
>  ``--gateway6`` options. IP pool is meaningless for IPV6 so those two
>  values can be used for EUI64 generation from a NIC's MAC address.
> @@ -71,7 +68,6 @@ MODIFY
>  | [--network6=*NETWORK6*]
>  | [--gateway6=*GATEWAY6*]
>  | [--mac-prefix=*MACPREFIX*]
> -| [--network-type=*NETWORKTYPE*]
>  | [--submit]
>  | {*network*}
>
> --
> 1.7.10.4
>



--
Guido Trotter
Ganeti engineering
Google Germany

Reply via email to