Revert "addons: mstpctl: reset (default) bridge mstpctl options when users remove settings"

This reverts commit a3d36dd86d.
Ticket CM-14516 has been filed to track this

Signed-off-by: Nikhil <nikhil@cumulusnetworks.com>

addons/mstpctl.py

@@ -144,7 +144,7 @@ class mstpctl(moduleBase):
                           'validrange' : ['0', '255'],
                           'default' : '2',
                           'required' : False,
-                          'jsonAttr': 'helloTime',
+                          'jsonAttr': 'portHelloTime',
                           'example' : ['mstpctl-hello 2']},
                     'mstpctl-portnetwork' : 
                         { 'help' : 'enable/disable bridge assurance capability for a port',
@@ -329,32 +329,16 @@ class mstpctl(moduleBase):
         try:
             # set bridge attributes
             for attrname, dstattrname in self._attrs_map.items():
-                config_val = ifaceobj.get_attr_value_first(attrname)
-                default_val = policymanager.policymanager_api.get_iface_default(module_name=self.__class__.__name__, ifname=ifaceobj.name, attr=attrname)
-                if not default_val:
-                    default_val = self.get_mod_subattr(attrname,'default')
-                jsonAttr =  self.get_mod_subattr(attrname, 'jsonAttr')
-                try:
-                    running_val = self.mstpctlcmd.get_bridge_attr(
-                                    ifaceobj.name, jsonAttr)
-                except:
-                    self.logger.info('%s: could not get running %s value'
-                                     %(ifaceobj.name, attrname))
-                    running_val = None
-                if (not config_val and default_val and (running_val != default_val)):
-                    # this happens when users remove an attribute from a port
-                    # and expect the default to be restored with ifreload.
-                    config_val = default_val
-                elif not config_val:
-                    # there is nothing configured and no default to reset
-                    continue
                 try:
+                    v = ifaceobj.get_attr_value_first(attrname)
+                    if not v:
+                       continue
                     if attrname == 'mstpctl-treeprio':
                        self.mstpctlcmd.set_bridge_treeprio(ifaceobj.name,
-                                config_val, check)
+                                v, check)
                     else:
                        self.mstpctlcmd.set_bridge_attr(ifaceobj.name,
-                                dstattrname, config_val, check)
+                                dstattrname, v, check)
                 except Exception, e:
                     self.logger.warn('%s' %str(e))
                     pass
@@ -431,10 +415,7 @@ class mstpctl(moduleBase):
             else:
                 return 'yes'
         else:
-            default_val = policymanager.policymanager_api.get_iface_default(module_name=self.__class__.__name__, ifname=ifaceobj.name, attr=attr)
+            return self.get_mod_subattr(attr,'default')
-            if not default_val:
-                return self.get_mod_subattr(attr,'default')
-            return default_val
 
     def _apply_bridge_port_settings(self, ifaceobj, bridgename=None,
                                     bridgeifaceobj=None,

ifupdown/policymanager.py

@@ -210,5 +210,4 @@ class policymanager():
 
         return mod_array
 
-
 policymanager_api = policymanager()

ifupdownaddons/mstpctlutil.py

@@ -28,17 +28,6 @@ class mstpctlutil(utilsBase):
                      'hello' : 'hello-time',
                      'forcevers' : 'force-protocol-version'}
 
-    _bridge_jsonAttr_map = {
-                            'treeprio': 'bridgeId',
-                            'maxage': 'maxAge',
-                            'fdelay': 'fwdDelay',
-                            'txholdcount': 'txHoldCounter',
-                            'maxhops': 'maxHops',
-                            'ageing': 'ageingTime',
-                            'hello': 'helloTime',
-                            'forcevers': 'forceProtocolVersion',
-                            }
-
     _bridgeportattrmap = {'portadminedge' : 'admin-edge-port',
                      'portp2p' : 'admin-point-to-point',
                      'portrestrrole' : 'restricted-role',
@@ -94,33 +83,6 @@ class mstpctlutil(utilsBase):
             self.logger.info('%s: cannot fetch mstpctl bridge port attributes: %s' % str(e))
         return mstpctl_bridgeport_attrs_dict
 
-    def _get_bridge_attrs_from_cache(self, bridgename):
-        attrs = MSTPAttrsCache.get(bridgename)
-        if attrs:
-            return attrs
-        mstpctl_bridge_attrs_dict = {}
-        try:
-            cmd = ['/sbin/mstpctl', 'showbridge', 'json', bridgename]
-            output = utils.exec_commandl(cmd)
-            if not output:
-                return mstpctl_bridge_attrs_dict
-        except Exception as e:
-            self.logger.info(str(e))
-            return mstpctl_bridge_attrs_dict
-        try:
-            mstpctl_bridge_cache = json.loads(output.strip('\n'))
-            for jsonAttr in mstpctl_bridge_cache[bridgename].keys():
-                mstpctl_bridge_attrs_dict[jsonAttr] = (
-                    str(mstpctl_bridge_cache[bridgename][jsonAttr]))
-            mstpctl_bridge_attrs_dict['treeprio'] = '%d' %(
-                                   int(mstpctl_bridge_attrs_dict.get('bridgeId',
-                                   '').split('.')[0], base=16) * 4096)
-            del mstpctl_bridge_attrs_dict['bridgeId']
-            MSTPAttrsCache.set(bridgename, mstpctl_bridge_attrs_dict)
-        except Exception as e:
-            self.logger.info('%s: cannot fetch mstpctl bridge attributes: %s' % str(e))
-        return mstpctl_bridge_attrs_dict
-
     def get_bridge_ports_attrs(self, bridgename):
         return self._get_bridge_port_attrs_from_cache(bridgename)
 
@@ -131,7 +93,7 @@ class mstpctlutil(utilsBase):
             return 'yes'
         return str(value)
 
-    def update_bridge_port_cache(self, bridgename, portname, attrname, value):
+    def update_cache(self, bridgename, portname, attrname, value):
         attrs = self.get_bridge_ports_attrs(bridgename)
         if not attrs:
             attrs = {}
@@ -140,13 +102,6 @@ class mstpctlutil(utilsBase):
         attrs[portname][attrname] = value
         MSTPAttrsCache.set(bridgename, attrs)
 
-    def update_bridge_cache(self, bridgename, attrname, value):
-        attrs = self.get_bridge_ports_attrs(bridgename)
-        if not attrs:
-            attrs = {}
-        attrs[attrname] = value
-        MSTPAttrsCache.set(bridgename, attrs)
-
     def set_bridge_port_attr(self, bridgename, portname, attrname, value, json_attr=None):
         cache_value = self.get_bridge_port_attr(bridgename, portname, json_attr)
         if cache_value and cache_value == value:
@@ -158,13 +113,16 @@ class mstpctlutil(utilsBase):
             utils.exec_commandl(['/sbin/mstpctl', 'set%s' % attrname,
                                  bridgename, portname, value])
         if json_attr:
-            self.update_bridge_port_cache(bridgename, portname, json_attr, value)
+            self.update_cache(bridgename, portname, json_attr, value)
 
     def get_bridge_attrs(self, bridgename):
         bridgeattrs = {}
         try:
-            bridgeattrs = dict((k, self.get_bridge_attr(bridgename, v))
+            bridgeattrs = dict((k, self.get_bridge_attr(bridgename, k))
-                                 for k,v in self._bridge_jsonAttr_map.items())
+                                 for k in self._bridgeattrmap.keys())
+            bridgeattrs['treeprio'] = '%d' %(int(bridgeattrs.get('bridgeid',
+                                     '').split('.')[0], base=16) * 4096)
+            del bridgeattrs['bridgeid']
         except Exception, e:
             self.logger.debug(bridgeattrs)
             self.logger.debug(str(e))
@@ -172,32 +130,28 @@ class mstpctlutil(utilsBase):
         return bridgeattrs
 
     def get_bridge_attr(self, bridgename, attrname):
-        if attrname == 'bridgeId':
+        try:
-            attrname = 'treeprio'
+            cmdl = ['/sbin/mstpctl', 'showbridge', bridgename,
-        return self._get_bridge_attrs_from_cache(bridgename).get(attrname)
+                    self._bridgeattrmap[attrname]]
+            return utils.exec_commandl(cmdl).strip('\n')
+        except Exception, e:
+            pass
+        return None
 
     def set_bridge_attr(self, bridgename, attrname, attrvalue, check=True):
 
         if check:
-            if attrname == 'treeprio':
+            attrvalue_curr = self.get_bridge_attr(bridgename, attrname)
-                attrvalue_curr = self.get_bridge_attr(bridgename, attrname)
-            else:
-                attrvalue_curr = self.get_bridge_attr(bridgename,
-                                        self._bridge_jsonAttr_map[attrname])
             if attrvalue_curr and attrvalue_curr == attrvalue:
                 return
         if attrname == 'treeprio':
             utils.exec_commandl(['/sbin/mstpctl', 'set%s' % attrname,
                                  '%s' % bridgename, '0', '%s' % attrvalue],
                                 stdout=False, stderr=None)
-            self.update_bridge_cache(bridgename, attrname, str(attrvalue))
         else:
             utils.exec_commandl(['/sbin/mstpctl', 'set%s' % attrname,
                                  '%s' % bridgename, '%s' % attrvalue],
                                 stdout=False, stderr=None)
-            self.update_bridge_cache(bridgename,
-                                     self._bridge_jsonAttr_map[attrname],
-                                     str(attrvalue))
 
     def set_bridge_attrs(self, bridgename, attrdict, check=True):
         for k, v in attrdict.iteritems():
@@ -209,7 +163,17 @@ class mstpctlutil(utilsBase):
                 self.logger.warn('%s: %s' %(bridgename, str(e)))
 
     def get_bridge_treeprio(self, bridgename):
-        return self.get_bridge_attr(bridgename, 'treeprio')
+        try:
+            cmdl = ['/sbin/mstpctl',
+                    'showbridge',
+                    bridgename,
+                    self._bridgeattrmap['bridgeid']]
+
+            bridgeid = utils.exec_commandl(cmdl).strip('\n')
+            return '%d' %(int(bridgeid.split('.')[0], base=16) * 4096)
+        except:
+            pass
+        return None
 
     def set_bridge_treeprio(self, bridgename, attrvalue, check=True):
         if check:
@@ -218,7 +182,6 @@ class mstpctlutil(utilsBase):
                 return
         utils.exec_commandl(['/sbin/mstpctl', 'settreeprio', bridgename, '0',
                              str(attrvalue)])
-        self.update_bridge_cache(bridgename, 'treeprio', str(attrvalue))
 
     def showbridge(self, bridgename=None):
         if bridgename: