mirror of
https://github.com/CumulusNetworks/ifupdown2.git
synced 2024-05-06 15:54:50 +00:00
addons: vrf: fix check in vrf map initialization when no running vrfs present
Ticket: CM-10178 Review: trivial Testing: tested with failing testcase in the CM This patch fixes a check in vrf map initialization code which did not account for running vrfs correctly. This caused the case where there were no running vrfs but stale map file to fail. Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com> addons,ifupdown,sbin: adding ifquery --with-defaults option Ticket: CM-7840 Reviewed By: Roopa Prabhu Testing Done: yes, by installing ifupdown .deb file onto dell-s3000-02 This patch adds a new argument '--with-defaults' to 'ifquery' when 'ifquery --with-defaults' is executed, running states of all interface attributes are compared against respective configured attributes from /etc/network/interfaces file, if configured. Otherwise, compared against default attributes from policy file Signed-off-by: Nikhil <nikhil@cumulusnetworks.com>
This commit is contained in:
@@ -500,7 +500,8 @@ class address(moduleBase):
|
||||
if not self.ipcmd:
|
||||
self.ipcmd = iproute2()
|
||||
|
||||
def run(self, ifaceobj, operation, query_ifaceobj=None, ifaceobj_getfunc=None):
|
||||
def run(self, ifaceobj, operation, query_ifaceobj=None,
|
||||
ifaceobj_getfunc=None, **extra_args):
|
||||
""" run address configuration on the interface object passed as argument
|
||||
|
||||
Args:
|
||||
|
@@ -1369,8 +1369,7 @@ class bridge(moduleBase):
|
||||
if attrval:
|
||||
ifaceobjcurr.update_config_with_status('bridge-vids', attrval, -1)
|
||||
|
||||
def _query_check_bridge(self, ifaceobj, ifaceobjcurr,
|
||||
ifaceobj_getfunc=None):
|
||||
def _query_check_bridge(self, ifaceobj, ifaceobjcurr, withdefaults):
|
||||
if not self._is_bridge(ifaceobj):
|
||||
return
|
||||
if not self.brctlcmd.bridge_exists(ifaceobj.name):
|
||||
@@ -1379,6 +1378,9 @@ class bridge(moduleBase):
|
||||
|
||||
ifaceattrs = self.dict_key_subset(ifaceobj.config,
|
||||
self.get_mod_attrs())
|
||||
#Add default attributes if --with-defaults is set
|
||||
if withdefaults and 'bridge-stp' not in ifaceattrs:
|
||||
ifaceattrs.append('bridge-stp')
|
||||
if not ifaceattrs:
|
||||
return
|
||||
try:
|
||||
@@ -1396,7 +1398,10 @@ class bridge(moduleBase):
|
||||
# get the corresponding ifaceobj attr
|
||||
v = ifaceobj.get_attr_value_first(k)
|
||||
if not v:
|
||||
continue
|
||||
if withdefaults and k == 'bridge-stp':
|
||||
v = 'on' if self.default_stp_on else 'off'
|
||||
else:
|
||||
continue
|
||||
rv = runningattrs.get(k[7:])
|
||||
if k == 'bridge-mcqv4src':
|
||||
continue
|
||||
@@ -1412,14 +1417,14 @@ class bridge(moduleBase):
|
||||
# special case stp compare because it may
|
||||
# contain more than one valid values
|
||||
stp_on_vals = ['on', 'yes']
|
||||
stp_off_vals = ['off']
|
||||
stp_off_vals = ['off', 'no']
|
||||
if ((v in stp_on_vals and rv in stp_on_vals) or
|
||||
(v in stp_off_vals and rv in stp_off_vals)):
|
||||
ifaceobjcurr.update_config_with_status('bridge-stp',
|
||||
v, 0)
|
||||
rv, 0)
|
||||
else:
|
||||
ifaceobjcurr.update_config_with_status('bridge-stp',
|
||||
v, 1)
|
||||
rv, 1)
|
||||
elif k == 'bridge-ports':
|
||||
# special case ports because it can contain regex or glob
|
||||
running_port_list = rv.keys() if rv else []
|
||||
@@ -1601,9 +1606,10 @@ class bridge(moduleBase):
|
||||
except Exception, e:
|
||||
self.log_warn('%s: %s' %(ifaceobj.name, str(e)))
|
||||
|
||||
def _query_check(self, ifaceobj, ifaceobjcurr, ifaceobj_getfunc=None):
|
||||
def _query_check(self, ifaceobj, ifaceobjcurr, withdefaults,
|
||||
ifaceobj_getfunc=None):
|
||||
if self._is_bridge(ifaceobj):
|
||||
self._query_check_bridge(ifaceobj, ifaceobjcurr)
|
||||
self._query_check_bridge(ifaceobj, ifaceobjcurr, withdefaults)
|
||||
else:
|
||||
self._query_check_bridge_port(ifaceobj, ifaceobjcurr,
|
||||
ifaceobj_getfunc)
|
||||
@@ -1693,7 +1699,7 @@ class bridge(moduleBase):
|
||||
self.brctlcmd = brctl()
|
||||
|
||||
def run(self, ifaceobj, operation, query_ifaceobj=None,
|
||||
ifaceobj_getfunc=None):
|
||||
ifaceobj_getfunc=None, **extra_args):
|
||||
""" run bridge configuration on the interface object passed as
|
||||
argument. Can create bridge interfaces if they dont exist already
|
||||
|
||||
@@ -1718,6 +1724,7 @@ class bridge(moduleBase):
|
||||
self._flush_running_vidinfo()
|
||||
if operation == 'query-checkcurr':
|
||||
op_handler(self, ifaceobj, query_ifaceobj,
|
||||
extra_args['withdefaults'] if 'withdefaults' in extra_args else False,
|
||||
ifaceobj_getfunc=ifaceobj_getfunc)
|
||||
else:
|
||||
op_handler(self, ifaceobj, ifaceobj_getfunc=ifaceobj_getfunc)
|
||||
|
@@ -123,7 +123,7 @@ class ethtool(moduleBase,utilsBase):
|
||||
def _pre_down(self, ifaceobj):
|
||||
pass #self._post_up(ifaceobj,operation="_pre_down")
|
||||
|
||||
def _query_check(self, ifaceobj, ifaceobjcurr):
|
||||
def _query_check(self, ifaceobj, ifaceobjcurr, withdefaults):
|
||||
"""
|
||||
_query_check() needs to compare the configured (or running)
|
||||
attribute with the running attribute.
|
||||
@@ -133,11 +133,13 @@ class ethtool(moduleBase,utilsBase):
|
||||
This is because a reboot will lose their running attribute
|
||||
(the default will get set).
|
||||
"""
|
||||
# Add default attributes if --with-defaults is set
|
||||
for attr in ['speed', 'duplex', 'autoneg']:
|
||||
configured = ifaceobj.get_attr_value_first('link-%s'%attr)
|
||||
# if there is nothing configured, do not check
|
||||
if not configured:
|
||||
continue
|
||||
if not withdefaults:
|
||||
continue
|
||||
default = policymanager.policymanager_api.get_iface_default(
|
||||
module_name='ethtool',
|
||||
ifname=ifaceobj.name,
|
||||
@@ -266,6 +268,7 @@ class ethtool(moduleBase,utilsBase):
|
||||
self._init_command_handlers()
|
||||
|
||||
if operation == 'query-checkcurr':
|
||||
op_handler(self, ifaceobj, query_ifaceobj)
|
||||
op_handler(self, ifaceobj, query_ifaceobj,
|
||||
extra_args['withdefaults'] if 'withdefaults' in extra_args else False)
|
||||
else:
|
||||
op_handler(self, ifaceobj)
|
||||
|
@@ -744,7 +744,8 @@ class vrf(moduleBase):
|
||||
except Exception, e:
|
||||
self.log_warn(str(e))
|
||||
|
||||
def _query_check_vrf_dev(self, ifaceobj, ifaceobjcurr, vrf_table):
|
||||
def _query_check_vrf_dev(self, ifaceobj, ifaceobjcurr, vrf_table,
|
||||
withdefaults):
|
||||
try:
|
||||
if not self.ipcmd.link_exists(ifaceobj.name):
|
||||
self.logger.info('%s: vrf: does not exist' %(ifaceobj.name))
|
||||
@@ -767,15 +768,28 @@ class vrf(moduleBase):
|
||||
else:
|
||||
ifaceobjcurr.update_config_with_status('vrf-table',
|
||||
running_table, 0)
|
||||
if not withdefaults:
|
||||
return
|
||||
if self.vrf_helper:
|
||||
ret_vrfhelper = self.exec_command('%s verify %s %s'
|
||||
%(self.vrf_helper,
|
||||
ifaceobj.name, vrf_table))
|
||||
if 'default routes are installed' not in ret_vrfhelper:
|
||||
ifaceobjcurr.update_config_with_status('vrf-default-route',
|
||||
'no', 1)
|
||||
else:
|
||||
ifaceobjcurr.update_config_with_status('vrf-default-route',
|
||||
'yes',0)
|
||||
except Exception, e:
|
||||
self.log_warn(str(e))
|
||||
|
||||
def _query_check(self, ifaceobj, ifaceobjcurr):
|
||||
def _query_check(self, ifaceobj, ifaceobjcurr, withdefaults):
|
||||
try:
|
||||
vrf_table = ifaceobj.get_attr_value_first('vrf-table')
|
||||
if vrf_table:
|
||||
self._iproute2_vrf_map_initialize()
|
||||
self._query_check_vrf_dev(ifaceobj, ifaceobjcurr, vrf_table)
|
||||
self._query_check_vrf_dev(ifaceobj, ifaceobjcurr, vrf_table,
|
||||
withdefaults)
|
||||
else:
|
||||
vrf = ifaceobj.get_attr_value_first('vrf')
|
||||
if vrf:
|
||||
@@ -841,6 +855,7 @@ class vrf(moduleBase):
|
||||
return
|
||||
self._init_command_handlers()
|
||||
if operation == 'query-checkcurr':
|
||||
op_handler(self, ifaceobj, query_ifaceobj)
|
||||
op_handler(self, ifaceobj, query_ifaceobj,
|
||||
extra_args['withdefaults'] if 'withdefaults' in extra_args else False)
|
||||
else:
|
||||
op_handler(self, ifaceobj, ifaceobj_getfunc=ifaceobj_getfunc)
|
||||
|
@@ -895,13 +895,13 @@ class ifupdownMain(ifupdownBase):
|
||||
# continue reading
|
||||
pass
|
||||
|
||||
def _sched_ifaces(self, ifacenames, ops, skipupperifaces=False,
|
||||
followdependents=True, sort=False):
|
||||
def _sched_ifaces(self, ifacenames, ops, withdefaults=False,
|
||||
skipupperifaces=False, followdependents=True, sort=False):
|
||||
self.logger.debug('scheduling \'%s\' for %s'
|
||||
%(str(ops), str(ifacenames)))
|
||||
self._pretty_print_ordered_dict('dependency graph',
|
||||
self.dependency_graph)
|
||||
ifaceScheduler.sched_ifaces(self, ifacenames, ops,
|
||||
ifaceScheduler.sched_ifaces(self, ifacenames, ops, withdefaults,
|
||||
dependency_graph=self.dependency_graph,
|
||||
order=ifaceSchedulerFlags.INORDER
|
||||
if 'down' in ops[0]
|
||||
@@ -1209,7 +1209,7 @@ class ifupdownMain(ifupdownBase):
|
||||
def query(self, ops, auto=False, format_list=False, allow_classes=None,
|
||||
ifacenames=None,
|
||||
excludepats=None, printdependency=None,
|
||||
format='native', type=None):
|
||||
format='native', type=None, withdefaults=False):
|
||||
""" query an interface """
|
||||
|
||||
self.set_type(type)
|
||||
@@ -1273,7 +1273,7 @@ class ifupdownMain(ifupdownBase):
|
||||
elif ops[0] == 'query-raw':
|
||||
return self.print_ifaceobjs_raw(filtered_ifacenames)
|
||||
|
||||
ret = self._sched_ifaces(filtered_ifacenames, ops,
|
||||
ret = self._sched_ifaces(filtered_ifacenames, ops, withdefaults,
|
||||
followdependents=True
|
||||
if self.flags.WITH_DEPENDS else False)
|
||||
|
||||
|
@@ -49,7 +49,7 @@ class ifaceScheduler():
|
||||
cls._SCHED_STATUS = state
|
||||
|
||||
@classmethod
|
||||
def run_iface_op(cls, ifupdownobj, ifaceobj, op, cenv=None):
|
||||
def run_iface_op(cls, ifupdownobj, ifaceobj, op, withdefaults=False, cenv=None):
|
||||
""" Runs sub operation on an interface """
|
||||
ifacename = ifaceobj.name
|
||||
|
||||
@@ -80,11 +80,11 @@ class ifaceScheduler():
|
||||
continue
|
||||
ifupdownobj.logger.debug(msg)
|
||||
m.run(ifaceobj, op, query_ifaceobj,
|
||||
ifaceobj_getfunc=ifupdownobj.get_ifaceobjs)
|
||||
ifaceobj_getfunc=ifupdownobj.get_ifaceobjs, withdefaults=withdefaults)
|
||||
else:
|
||||
ifupdownobj.logger.debug(msg)
|
||||
m.run(ifaceobj, op,
|
||||
ifaceobj_getfunc=ifupdownobj.get_ifaceobjs)
|
||||
ifaceobj_getfunc=ifupdownobj.get_ifaceobjs, withdefaults=withdefaults)
|
||||
except Exception, e:
|
||||
if not ifupdownobj.ignore_error(str(e)):
|
||||
err = 1
|
||||
@@ -117,7 +117,7 @@ class ifaceScheduler():
|
||||
ifupdownobj.log_error(str(e))
|
||||
|
||||
@classmethod
|
||||
def run_iface_list_ops(cls, ifupdownobj, ifaceobjs, ops):
|
||||
def run_iface_list_ops(cls, ifupdownobj, ifaceobjs, ops, withdefaults=False):
|
||||
""" Runs all operations on a list of interface
|
||||
configurations for the same interface
|
||||
"""
|
||||
@@ -151,7 +151,7 @@ class ifaceScheduler():
|
||||
%(ifaceobjs[0].name, str(e)))
|
||||
pass
|
||||
for ifaceobj in ifaceobjs:
|
||||
cls.run_iface_op(ifupdownobj, ifaceobj, op,
|
||||
cls.run_iface_op(ifupdownobj, ifaceobj, op, withdefaults,
|
||||
cenv=ifupdownobj.generate_running_env(ifaceobj, op)
|
||||
if ifupdownobj.config.get('addon_scripts_support',
|
||||
'0') == '1' else None)
|
||||
@@ -212,8 +212,8 @@ class ifaceScheduler():
|
||||
return True
|
||||
|
||||
@classmethod
|
||||
def run_iface_graph(cls, ifupdownobj, ifacename, ops, parent=None,
|
||||
order=ifaceSchedulerFlags.POSTORDER,
|
||||
def run_iface_graph(cls, ifupdownobj, ifacename, ops, withdefaults=False,
|
||||
parent=None, order=ifaceSchedulerFlags.POSTORDER,
|
||||
followdependents=True):
|
||||
""" runs interface by traversing all nodes rooted at itself """
|
||||
|
||||
@@ -235,7 +235,7 @@ class ifaceScheduler():
|
||||
|
||||
# If inorder, run the iface first and then its dependents
|
||||
if order == ifaceSchedulerFlags.INORDER:
|
||||
cls.run_iface_list_ops(ifupdownobj, ifaceobjs, ops)
|
||||
cls.run_iface_list_ops(ifupdownobj, ifaceobjs, ops, False)
|
||||
|
||||
for ifaceobj in ifaceobjs:
|
||||
# Run lowerifaces or dependents
|
||||
@@ -254,11 +254,12 @@ class ifaceScheduler():
|
||||
if ifupdownobj.is_iface_noconfig(d)]
|
||||
if new_dlist:
|
||||
cls.run_iface_list(ifupdownobj, new_dlist, ops,
|
||||
ifacename, order, followdependents,
|
||||
withdefaults, ifacename, order,
|
||||
followdependents,
|
||||
continueonfailure=False)
|
||||
else:
|
||||
cls.run_iface_list(ifupdownobj, dlist, ops,
|
||||
ifacename, order,
|
||||
withdefaults, ifacename, order,
|
||||
followdependents,
|
||||
continueonfailure=False)
|
||||
except Exception, e:
|
||||
@@ -270,18 +271,17 @@ class ifaceScheduler():
|
||||
ifaceStatus.ERROR)
|
||||
raise
|
||||
if order == ifaceSchedulerFlags.POSTORDER:
|
||||
cls.run_iface_list_ops(ifupdownobj, ifaceobjs, ops)
|
||||
cls.run_iface_list_ops(ifupdownobj, ifaceobjs, ops, withdefaults)
|
||||
|
||||
@classmethod
|
||||
def run_iface_list(cls, ifupdownobj, ifacenames,
|
||||
ops, parent=None, order=ifaceSchedulerFlags.POSTORDER,
|
||||
def run_iface_list(cls, ifupdownobj, ifacenames, ops, withdefaults=False,
|
||||
parent=None, order=ifaceSchedulerFlags.POSTORDER,
|
||||
followdependents=True, continueonfailure=True):
|
||||
""" Runs interface list """
|
||||
|
||||
for ifacename in ifacenames:
|
||||
try:
|
||||
cls.run_iface_graph(ifupdownobj, ifacename, ops, parent,
|
||||
order, followdependents)
|
||||
cls.run_iface_graph(ifupdownobj, ifacename, ops, withdefaults,
|
||||
parent, order, followdependents)
|
||||
except Exception, e:
|
||||
if continueonfailure:
|
||||
if ifupdownobj.logger.isEnabledFor(logging.DEBUG):
|
||||
@@ -311,7 +311,7 @@ class ifaceScheduler():
|
||||
|
||||
if not skip_root:
|
||||
# run the iface first and then its upperifaces
|
||||
cls.run_iface_list_ops(ifupdownobj, ifaceobjs, ops)
|
||||
cls.run_iface_list_ops(ifupdownobj, ifaceobjs, ops, False)
|
||||
for ifaceobj in ifaceobjs:
|
||||
# Run upperifaces
|
||||
ulist = ifaceobj.upperifaces
|
||||
@@ -405,7 +405,7 @@ class ifaceScheduler():
|
||||
ifaceobjs = ifupdownobj.get_ifaceobjs(u)
|
||||
if not ifaceobjs:
|
||||
continue
|
||||
cls.run_iface_list_ops(ifupdownobj, ifaceobjs, ops)
|
||||
cls.run_iface_list_ops(ifupdownobj, ifaceobjs, ops, False)
|
||||
except Exception, e:
|
||||
if continueonfailure:
|
||||
self.logger.warn('%s' %str(e))
|
||||
@@ -434,7 +434,7 @@ class ifaceScheduler():
|
||||
return ifacenames_sorted
|
||||
|
||||
@classmethod
|
||||
def sched_ifaces(cls, ifupdownobj, ifacenames, ops,
|
||||
def sched_ifaces(cls, ifupdownobj, ifacenames, ops, withdefaults=False,
|
||||
dependency_graph=None, indegrees=None,
|
||||
order=ifaceSchedulerFlags.POSTORDER,
|
||||
followdependents=True, skipupperifaces=False, sort=False):
|
||||
@@ -529,7 +529,7 @@ class ifaceScheduler():
|
||||
run_queue.reverse()
|
||||
|
||||
# run interface list
|
||||
cls.run_iface_list(ifupdownobj, run_queue, ops,
|
||||
cls.run_iface_list(ifupdownobj, run_queue, ops, withdefaults,
|
||||
parent=None, order=order,
|
||||
followdependents=followdependents)
|
||||
if not cls.get_sched_status():
|
||||
|
@@ -107,6 +107,8 @@ def run_query(args):
|
||||
qop = 'query-dependency'
|
||||
elif args.printsavedstate:
|
||||
qop = 'query-savedstate'
|
||||
elif args.withdefaults:
|
||||
qop = 'query-withdefaults'
|
||||
else:
|
||||
qop='query'
|
||||
cachearg=(False if (iflist or args.nocache or
|
||||
@@ -130,7 +132,8 @@ def run_query(args):
|
||||
ifupdown_handle.query([qop], args.all, args.list, args.CLASS, iflist,
|
||||
excludepats=args.excludepats,
|
||||
printdependency=args.printdependency,
|
||||
format=args.format, type=args.type)
|
||||
format=args.format, type=args.type,
|
||||
withdefaults=args.withdefaults)
|
||||
except:
|
||||
raise
|
||||
|
||||
@@ -333,6 +336,11 @@ def update_ifquery_argparser(argparser):
|
||||
argparser.add_argument('-s', '--syntax-help', action='store_true',
|
||||
dest='syntaxhelp',
|
||||
help='print supported interface config syntax')
|
||||
argparser.add_argument('--with-defaults', action='store_true',
|
||||
dest='withdefaults',
|
||||
help='check policy default file contents, ' +
|
||||
'for unconfigured attributes, against ' +
|
||||
'running state of an interface')
|
||||
|
||||
def update_ifreload_argparser(argparser):
|
||||
""" parser for ifreload """
|
||||
|
Reference in New Issue
Block a user