Fixed all the review comments from Daniel.

  Added a new file i.e. nic_base.py which will be used a base calls for all
  NIC related configuration.
  Modified some code in nic.py.

6 files changed, 222 insertions, 100 deletions

diff --git a/cloudinit/sources/helpers/vmware/imc/boot_proto.py b/cloudinit/sources/helpers/vmware/imc/boot_proto.py
index abfffd75..faba5887 100644
--- a/cloudinit/sources/helpers/vmware/imc/boot_proto.py
+++ b/cloudinit/sources/helpers/vmware/imc/boot_proto.py
@@ -18,7 +18,7 @@
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.

 

 

-class BootProto:

+class BootProtoEnum:

     """Specifies the NIC Boot Settings."""

 

     DHCP = 'dhcp'

diff --git a/cloudinit/sources/helpers/vmware/imc/config.py b/cloudinit/sources/helpers/vmware/imc/config.py
index 7eee47a5..aebc12a0 100644
--- a/cloudinit/sources/helpers/vmware/imc/config.py
+++ b/cloudinit/sources/helpers/vmware/imc/config.py
@@ -66,7 +66,8 @@ class Config:
     def name_servers(self):

         """Return the list of DNS servers."""

         res = []

-        for i in range(1, self._configFile.get_count(Config.DNS) + 1):

+        cnt = self._configFile.get_count_with_prefix(Config.DNS)

+        for i in range(1, cnt + 1):

             key = Config.DNS + str(i)

             res.append(self._configFile[key])

 

@@ -76,7 +77,8 @@ class Config:
     def dns_suffixes(self):

         """Return the list of DNS Suffixes."""

         res = []

-        for i in range(1, self._configFile.get_count(Config.SUFFIX) + 1):

+        cnt = self._configFile.get_count_with_prefix(Config.SUFFIX)

+        for i in range(1, cnt + 1):

             key = Config.SUFFIX + str(i)

             res.append(self._configFile[key])

 

diff --git a/cloudinit/sources/helpers/vmware/imc/config_file.py b/cloudinit/sources/helpers/vmware/imc/config_file.py
index e08a2a9a..7c47d14c 100644
--- a/cloudinit/sources/helpers/vmware/imc/config_file.py
+++ b/cloudinit/sources/helpers/vmware/imc/config_file.py
@@ -32,7 +32,8 @@ logger = logging.getLogger(__name__)
 class ConfigFile(ConfigSource, dict):
     """ConfigFile module to load the content from a specified source."""
 
-    def __init__(self):
+    def __init__(self, filename):
+        self._loadConfigFile(filename)
         pass
 
     def _insertKey(self, key, val):
@@ -48,9 +49,9 @@ class ConfigFile(ConfigSource, dict):
         val = val.strip()
 
         if key.startswith('-') or '|-' in key:
-            canLog = 0
+            canLog = False
         else:
-            canLog = 1
+            canLog = True
 
         # "sensitive" settings shall not be logged
         if canLog:
@@ -64,7 +65,7 @@ class ConfigFile(ConfigSource, dict):
         """Return the number of properties present."""
         return len(self)
 
-    def loadConfigFile(self, filename):
+    def _loadConfigFile(self, filename):
         """
         Parses properties from the specified config file.
 
@@ -87,22 +88,9 @@ class ConfigFile(ConfigSource, dict):
             logger.debug("FOUND CATEGORY = '%s'" % category)
 
             for (key, value) in config.items(category):
-                # "sensitive" settings shall not be logged
-                if key.startswith('-'):
-                    canLog = 0
-                else:
-                    canLog = 1
-
-                if canLog:
-                    logger.debug("Processing key, value: '%s':'%s'" %
-                                 (key, value))
-                else:
-                    logger.debug("Processing key, value : "
-                                 "'*********************'")
-
                 self._insertKey(category + '|' + key, value)
 
-    def keep_current_value(self, key):
+    def should_keep_current_value(self, key):
         """
         Determines whether a value for a property must be kept.
 
@@ -114,9 +102,9 @@ class ConfigFile(ConfigSource, dict):
         """
         # helps to distinguish from "empty" value which is used to indicate
         # "removal"
-        return not key in self
+        return key not in self
 
-    def remove_current_value(self, key):
+    def should_remove_current_value(self, key):
         """
         Determines whether a value for the property must be removed.
 
@@ -135,17 +123,11 @@ class ConfigFile(ConfigSource, dict):
         else:
             return False
 
-    def get_count(self, prefix):
+    def get_count_with_prefix(self, prefix):
         """
-        Return the total number of keys that start with the
-        specified prefix.
+        Return the total count of keys that start with the specified prefix.
 
         Keyword arguments:
         prefix -- prefix of the key
         """
-        res = 0
-        for key in self.keys():
-            if key.startswith(prefix):
-                res += 1
-
-        return res
+        return len([key for key in self if key.startswith(prefix)])
diff --git a/cloudinit/sources/helpers/vmware/imc/ipv4_mode.py b/cloudinit/sources/helpers/vmware/imc/ipv4_mode.py
index 28544e4f..33f88726 100644
--- a/cloudinit/sources/helpers/vmware/imc/ipv4_mode.py
+++ b/cloudinit/sources/helpers/vmware/imc/ipv4_mode.py
@@ -18,7 +18,7 @@
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 
-class Ipv4Mode:
+class Ipv4ModeEnum:
     """
     The IPv4 configuration mode which directly represents the user's goal.
 
diff --git a/cloudinit/sources/helpers/vmware/imc/nic.py b/cloudinit/sources/helpers/vmware/imc/nic.py
index bb45a9e6..a7594874 100644
--- a/cloudinit/sources/helpers/vmware/imc/nic.py
+++ b/cloudinit/sources/helpers/vmware/imc/nic.py
@@ -17,10 +17,11 @@
 #    You should have received a copy of the GNU General Public License
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-from .boot_proto import BootProto
+from .boot_proto import BootProtoEnum
+from .nic_base import NicBase, StaticIpv4Base, StaticIpv6Base
 
 
-class Nic:
+class Nic(NicBase):
     """
     Holds the information about each NIC specified
     in the customization specification file
@@ -31,10 +32,10 @@ class Nic:
         self._configFile = configFile
 
     def _get(self, what):
-        return self._configFile.get(self.name + what, None)
+        return self._configFile.get(self.name + '|' + what, None)
 
-    def _get_count(self, prefix):
-        return self._configFile.get_count(self.name + prefix)
+    def _get_count_with_prefix(self, prefix):
+        return self._configFile.get_count_with_prefix(self.name + prefix)
 
     @property
     def name(self):
@@ -42,41 +43,52 @@ class Nic:
 
     @property
     def mac(self):
-        return self._get('|MACADDR').lower()
+        return self._get('MACADDR').lower()
 
     @property
-    def bootProto(self):
-        return self._get('|BOOTPROTO').lower()
+    def primary(self):
+        value = self._get('PRIMARY').lower()
+        return value == 'yes' or value == 'true'
 
     @property
-    def ipv4(self):
-        """
-        Retrieves the DHCP or Static IPv6 configuration
-        based on the BOOTPROTO property associated with the NIC
-        """
-        if self.bootProto == BootProto.STATIC:
-            return StaticIpv4Conf(self)
+    def onboot(self):
+        value = self._get('ONBOOT').lower()
+        return value == 'yes' or value == 'true'
 
-        return DhcpIpv4Conf(self)
+    @property
+    def bootProto(self):
+        return self._get('BOOTPROTO').lower()
 
     @property
-    def ipv6(self):
-        cnt = self._get_count("|IPv6ADDR|")
+    def ipv4_mode(self):
+        return self._get('IPv4_MODE').lower()
 
-        if cnt != 0:
-            return StaticIpv6Conf(self)
+    @property
+    def staticIpv4(self):
+        """
+        Checks the BOOTPROTO property and returns StaticIPv4Addr
+        configuration object if STATIC configuration is set.
+        """
+        if self.bootProto == BootProtoEnum.STATIC:
+            return [StaticIpv4Addr(self)]
+        else:
+            return None
 
-        return DhcpIpv6Conf(self)
+    @property
+    def staticIpv6(self):
+        cnt = self._get_count_with_prefix('|IPv6ADDR|')
 
+        if not cnt:
+            return None
 
-class DhcpIpv4Conf:
-    """DHCP Configuration Setting."""
+        result = []
+        for index in range(1, cnt + 1):
+            result.append(StaticIpv6Addr(self, index))
 
-    def __init__(self, nic):
-        self._nic = nic
+        return result
 
 
-class StaticIpv4Addr:
+class StaticIpv4Addr(StaticIpv4Base):
     """Static IPV4  Setting."""
 
     def __init__(self, nic):
@@ -84,34 +96,22 @@ class StaticIpv4Addr:
 
     @property
     def ip(self):
-        return self._nic._get('|IPADDR')
+        return self._nic._get('IPADDR')
 
     @property
     def netmask(self):
-        return self._nic._get('|NETMASK')
+        return self._nic._get('NETMASK')
 
     @property
-    def gateway(self):
-        return self._nic._get('|GATEWAY')
+    def gateways(self):
+        value = self._nic._get('GATEWAY')
+        if value:
+            return [x.strip() for x in value.split(',')]
+        else:
+            return None
 
 
-class StaticIpv4Conf(DhcpIpv4Conf):
-    """Static IPV4 Configuration."""
-
-    @property
-    def addrs(self):
-        """Return the list of associated IPv4 addresses."""
-        return [StaticIpv4Addr(self._nic)]
-
-
-class DhcpIpv6Conf:
-    """DHCP IPV6 Configuration."""
-
-    def __init__(self, nic):
-        self._nic = nic
-
-
-class StaticIpv6Addr:
+class StaticIpv6Addr(StaticIpv6Base):
     """Static IPV6 Address."""
 
     def __init__(self, nic, index):
@@ -120,28 +120,12 @@ class StaticIpv6Addr:
 
     @property
     def ip(self):
-        return self._nic._get("|IPv6ADDR|" + str(self._index))
+        return self._nic._get('IPv6ADDR|' + str(self._index))
 
     @property
-    def prefix(self):
-        return self._nic._get("|IPv6NETMASK|" + str(self._index))
+    def netmask(self):
+        return self._nic._get('IPv6NETMASK|' + str(self._index))
 
     @property
     def gateway(self):
-        return self._nic._get("|IPv6GATEWAY|" + str(self._index))
-
-
-class StaticIpv6Conf(DhcpIpv6Conf):
-    """Static IPV6 Configuration."""
-
-    @property
-    def addrs(self):
-        """Return the list Associated IPV6 addresses."""
-        cnt = self._nic._get_count("|IPv6ADDR|")
-
-        res = []
-
-        for i in range(1, cnt + 1):
-            res.append(StaticIpv6Addr(self._nic, i))
-
-        return res
+        return self._nic._get('IPv6GATEWAY|' + str(self._index))
diff --git a/cloudinit/sources/helpers/vmware/imc/nic_base.py b/cloudinit/sources/helpers/vmware/imc/nic_base.py
new file mode 100644
index 00000000..030ba311
--- /dev/null
+++ b/cloudinit/sources/helpers/vmware/imc/nic_base.py
@@ -0,0 +1,154 @@
+# vi: ts=4 expandtab
+#
+#    Copyright (C) 2015 Canonical Ltd.
+#    Copyright (C) 2015 VMware Inc.
+#
+#    Author: Sankar Tanguturi <stanguturi@vmware.com>
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU General Public License version 3, as
+#    published by the Free Software Foundation.
+#
+#    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, see <http://www.gnu.org/licenses/>.
+
+
+class NicBase:
+    """
+    Define what are expected of each nic.
+    The following properties should be provided in an implementation class.
+    """
+
+    @property
+    def mac(self):
+        """
+        Retrieves the mac address of the nic
+        @return (str) : the MACADDR setting
+        """
+        raise NotImplementedError('MACADDR')
+
+    @property
+    def primary(self):
+        """
+        Retrieves whether the nic is the primary nic
+        Indicates whether NIC will be used to define the default gateway.
+        If none of the NICs is configured to be primary, default gateway won't
+        be set.
+        @return (bool): the PRIMARY setting
+        """
+        raise NotImplementedError('PRIMARY')
+
+    @property
+    def onboot(self):
+        """
+        Retrieves whether the nic should be up at the boot time
+        @return (bool) : the ONBOOT setting
+        """
+        raise NotImplementedError('ONBOOT')
+
+    @property
+    def bootProto(self):
+        """
+        Retrieves the boot protocol of the nic
+        @return (str): the BOOTPROTO setting, valid values: dhcp and static.
+        """
+        raise NotImplementedError('BOOTPROTO')
+
+    @property
+    def ipv4_mode(self):
+        """
+        Retrieves the IPv4_MODE
+        @return (str): the IPv4_MODE setting, valid values:
+        backwards_compatible, static, dhcp, disabled, as_is
+        """
+        raise NotImplementedError('IPv4_MODE')
+
+    @property
+    def staticIpv4(self):
+        """
+        Retrieves the static IPv4 configuration of the nic
+        @return (StaticIpv4Base list): the static ipv4 setting
+        """
+        raise NotImplementedError('Static IPv4')
+
+    @property
+    def staticIpv6(self):
+        """
+        Retrieves the IPv6 configuration of the nic
+        @return (StaticIpv6Base list): the static ipv6 setting
+        """
+        raise NotImplementedError('Static Ipv6')
+
+    def validate(self):
+        """
+        Validate the object
+        For example, the staticIpv4 property is required and should not be
+        empty when ipv4Mode is STATIC
+        """
+        raise NotImplementedError('Check constraints on properties')
+
+
+class StaticIpv4Base:
+    """
+    Define what are expected of a static IPv4 setting
+    The following properties should be provided in an implementation class.
+    """
+
+    @property
+    def ip(self):
+        """
+        Retrieves the Ipv4 address
+        @return (str): the IPADDR setting
+        """
+        raise NotImplementedError('Ipv4 Address')
+
+    @property
+    def netmask(self):
+        """
+        Retrieves the Ipv4 NETMASK setting
+        @return (str): the NETMASK setting
+        """
+        raise NotImplementedError('Ipv4 NETMASK')
+
+    @property
+    def gateways(self):
+        """
+        Retrieves the gateways on this Ipv4 subnet
+        @return (str list): the GATEWAY setting
+        """
+        raise NotImplementedError('Ipv4 GATEWAY')
+
+
+class StaticIpv6Base:
+    """Define what are expected of a static IPv6 setting
+    The following properties should be provided in an implementation class.
+    """
+
+    @property
+    def ip(self):
+        """
+        Retrieves the Ipv6 address
+        @return (str): the IPv6ADDR setting
+        """
+        raise NotImplementedError('Ipv6 Address')
+
+    @property
+    def netmask(self):
+        """
+        Retrieves the Ipv6 NETMASK setting
+        @return (str): the IPv6NETMASK setting
+        """
+        raise NotImplementedError('Ipv6 NETMASK')
+
+    @property
+    def gateway(self):
+        """
+        Retrieves the Ipv6 GATEWAY setting
+        @return (str): the IPv6GATEWAY setting
+        """
+        raise NotImplementedError('Ipv6 GATEWAY')