From 103e8404cdea70dad486940f209b9683f1c7b936 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Sun, 20 Sep 2020 15:18:50 +0200 Subject: ifconfig: T2653: remove duplicates of get_config() A lot of derived classes from Interface implemented their own get_config() method which more or less was the same everywhere. We also hat different qualifiers like @staticmethod or @classmethod. This is now changed to only have the @classmethod in Interface base class which will return the necessary dictionary keys for the required interfaces. This change is a mid reduction in lines of code which is always a very nice thing! --- python/vyos/ifconfig/geneve.py | 13 ----------- python/vyos/ifconfig/interface.py | 25 +++++++++------------ python/vyos/ifconfig/l2tpv3.py | 34 +++++++++-------------------- python/vyos/ifconfig/macsec.py | 16 -------------- python/vyos/ifconfig/macvlan.py | 15 +------------ python/vyos/ifconfig/vxlan.py | 14 ------------ python/vyos/ifconfig/wireless.py | 16 -------------- src/conf_mode/interfaces-geneve.py | 11 +++++----- src/conf_mode/interfaces-l2tpv3.py | 10 ++++----- src/conf_mode/interfaces-macsec.py | 12 +++++----- src/conf_mode/interfaces-pseudo-ethernet.py | 11 +++++----- src/conf_mode/interfaces-vxlan.py | 10 ++++----- src/conf_mode/interfaces-wireless.py | 10 ++++----- 13 files changed, 51 insertions(+), 146 deletions(-) diff --git a/python/vyos/ifconfig/geneve.py b/python/vyos/ifconfig/geneve.py index 0a13043cc..5c4597be8 100644 --- a/python/vyos/ifconfig/geneve.py +++ b/python/vyos/ifconfig/geneve.py @@ -13,7 +13,6 @@ # You should have received a copy of the GNU Lesser General Public # License along with this library. If not, see . -from copy import deepcopy from vyos.ifconfig.interface import Interface @Interface.register @@ -51,18 +50,6 @@ class GeneveIf(Interface): # interface is always A/D down. It needs to be enabled explicitly self.set_admin_state('down') - @classmethod - def get_config(cls): - """ - GENEVE interfaces require a configuration when they are added using - iproute2. This static method will provide the configuration dictionary - used by this class. - - Example: - >> dict = GeneveIf().get_config() - """ - return deepcopy(cls.default) - def update(self, config): """ General helper function which works on a dictionary retrived by get_config_dict(). It's main intention is to consolidate the scattered diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index 4e420dc1d..807191b3d 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -174,6 +174,15 @@ class Interface(Control): def exists(cls, ifname): return os.path.exists(f'/sys/class/net/{ifname}') + @classmethod + def get_config(cls): + """ + Some but not all interfaces require a configuration when they are added + using iproute2. This method will provide the configuration dictionary + used by this class. + """ + return deepcopy(cls.default) + def __init__(self, ifname, **kargs): """ This is the base interface class which supports basic IP/MAC address @@ -1084,7 +1093,7 @@ class VLANIf(Interface): def _create(self): # bail out early if interface already exists - if os.path.exists(f'/sys/class/net/{self.ifname}'): + if self.exists(f'{self.ifname}'): return cmd = 'ip link add link {source_interface} name {ifname} type vlan id {vlan_id}' @@ -1100,20 +1109,6 @@ class VLANIf(Interface): # interface is always A/D down. It needs to be enabled explicitly self.set_admin_state('down') - @staticmethod - def get_config(): - """ - MACsec interfaces require a configuration when they are added using - iproute2. This static method will provide the configuration dictionary - used by this class. - - Example: - >> dict = VLANIf().get_config() - """ - config = deepcopy(__class__.default) - del config['type'] - return config - def set_admin_state(self, state): """ Set interface administrative state to be 'up' or 'down' diff --git a/python/vyos/ifconfig/l2tpv3.py b/python/vyos/ifconfig/l2tpv3.py index 33740921e..5fd90f9cf 100644 --- a/python/vyos/ifconfig/l2tpv3.py +++ b/python/vyos/ifconfig/l2tpv3.py @@ -13,7 +13,6 @@ # You should have received a copy of the GNU Lesser General Public # License along with this library. If not, see . -import os from vyos.ifconfig.interface import Interface @Interface.register @@ -28,6 +27,15 @@ class L2TPv3If(Interface): default = { 'type': 'l2tp', + 'peer_tunnel_id': '', + 'local_port': 0, + 'remote_port': 0, + 'encapsulation': 'udp', + 'local_address': '', + 'remote_address': '', + 'session_id': '', + 'tunnel_id': '', + 'peer_session_id': '' } definition = { **Interface.definition, @@ -73,7 +81,7 @@ class L2TPv3If(Interface): >>> i.remove() """ - if os.path.exists('/sys/class/net/{}'.format(self.config['ifname'])): + if self.exists(self.config['ifname']): # interface is always A/D down. It needs to be enabled explicitly self.set_admin_state('down') @@ -86,25 +94,3 @@ class L2TPv3If(Interface): cmd = 'ip l2tp del tunnel tunnel_id {tunnel_id}' self._cmd(cmd.format(**self.config)) - @staticmethod - def get_config(): - """ - L2TPv3 interfaces require a configuration when they are added using - iproute2. This static method will provide the configuration dictionary - used by this class. - - Example: - >> dict = L2TPv3If().get_config() - """ - config = { - 'peer_tunnel_id': '', - 'local_port': 0, - 'remote_port': 0, - 'encapsulation': 'udp', - 'local_address': '', - 'remote_address': '', - 'session_id': '', - 'tunnel_id': '', - 'peer_session_id': '' - } - return config diff --git a/python/vyos/ifconfig/macsec.py b/python/vyos/ifconfig/macsec.py index 6f570d162..456686ea6 100644 --- a/python/vyos/ifconfig/macsec.py +++ b/python/vyos/ifconfig/macsec.py @@ -56,22 +56,6 @@ class MACsecIf(Interface): # interface is always A/D down. It needs to be enabled explicitly self.set_admin_state('down') - @staticmethod - def get_config(): - """ - MACsec interfaces require a configuration when they are added using - iproute2. This static method will provide the configuration dictionary - used by this class. - - Example: - >> dict = MACsecIf().get_config() - """ - config = { - 'security_cipher': '', - 'source_interface': '', - } - return config - def update(self, config): """ General helper function which works on a dictionary retrived by get_config_dict(). It's main intention is to consolidate the scattered diff --git a/python/vyos/ifconfig/macvlan.py b/python/vyos/ifconfig/macvlan.py index 9c1d09c1c..2447fec77 100644 --- a/python/vyos/ifconfig/macvlan.py +++ b/python/vyos/ifconfig/macvlan.py @@ -1,4 +1,4 @@ -# Copyright 2019 VyOS maintainers and contributors +# Copyright 2019-2020 VyOS maintainers and contributors # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -13,7 +13,6 @@ # You should have received a copy of the GNU Lesser General Public # License along with this library. If not, see . -from copy import deepcopy from vyos.ifconfig.interface import Interface @Interface.register @@ -53,18 +52,6 @@ class MACVLANIf(Interface): cmd = f'ip link set dev {ifname} type macvlan mode {mode}' return self._cmd(cmd) - @classmethod - def get_config(cls): - """ - MACVLAN interfaces require a configuration when they are added using - iproute2. This method will provide the configuration dictionary used - by this class. - - Example: - >> dict = MACVLANIf().get_config() - """ - return deepcopy(cls.default) - def update(self, config): """ General helper function which works on a dictionary retrived by get_config_dict(). It's main intention is to consolidate the scattered diff --git a/python/vyos/ifconfig/vxlan.py b/python/vyos/ifconfig/vxlan.py index dba62b61a..ad1f605ed 100644 --- a/python/vyos/ifconfig/vxlan.py +++ b/python/vyos/ifconfig/vxlan.py @@ -13,8 +13,6 @@ # You should have received a copy of the GNU Lesser General Public # License along with this library. If not, see . -from copy import deepcopy - from vyos import ConfigError from vyos.ifconfig.interface import Interface @@ -97,18 +95,6 @@ class VXLANIf(Interface): self._cmd(cmd) - @classmethod - def get_config(cls): - """ - VXLAN interfaces require a configuration when they are added using - iproute2. This static method will provide the configuration dictionary - used by this class. - - Example: - >> dict = VXLANIf().get_config() - """ - return deepcopy(cls.default) - def update(self, config): """ General helper function which works on a dictionary retrived by get_config_dict(). It's main intention is to consolidate the scattered diff --git a/python/vyos/ifconfig/wireless.py b/python/vyos/ifconfig/wireless.py index 346577119..37703d242 100644 --- a/python/vyos/ifconfig/wireless.py +++ b/python/vyos/ifconfig/wireless.py @@ -50,22 +50,6 @@ class WiFiIf(Interface): .format(**self.config) self._cmd(cmd) - @staticmethod - def get_config(): - """ - WiFi interfaces require a configuration when they are added using - iw (type/phy). This static method will provide the configuration - ictionary used by this class. - - Example: - >> conf = WiFiIf().get_config() - """ - config = { - 'phy': 'phy0' - } - return config - - def update(self, config): """ General helper function which works on a dictionary retrived by get_config_dict(). It's main intention is to consolidate the scattered diff --git a/src/conf_mode/interfaces-geneve.py b/src/conf_mode/interfaces-geneve.py index cc2cf025a..af7c121f4 100755 --- a/src/conf_mode/interfaces-geneve.py +++ b/src/conf_mode/interfaces-geneve.py @@ -17,7 +17,6 @@ import os from sys import exit -from copy import deepcopy from netifaces import interfaces from vyos.config import Config @@ -62,7 +61,6 @@ def verify(geneve): def generate(geneve): return None - def apply(geneve): # Check if GENEVE interface already exists if geneve['ifname'] in interfaces(): @@ -72,10 +70,11 @@ def apply(geneve): g.remove() if 'deleted' not in geneve: - # GENEVE interface needs to be created on-block - # instead of passing a ton of arguments, I just use a dict - # that is managed by vyos.ifconfig - conf = deepcopy(GeneveIf.get_config()) + # This is a special type of interface which needs additional parameters + # when created using iproute2. Instead of passing a ton of arguments, + # use a dictionary provided by the interface class which holds all the + # options necessary. + conf = GeneveIf.get_config() # Assign GENEVE instance configuration parameters to config dict conf['vni'] = geneve['vni'] diff --git a/src/conf_mode/interfaces-l2tpv3.py b/src/conf_mode/interfaces-l2tpv3.py index 144cee5fe..2653ff19c 100755 --- a/src/conf_mode/interfaces-l2tpv3.py +++ b/src/conf_mode/interfaces-l2tpv3.py @@ -17,7 +17,6 @@ import os from sys import exit -from copy import deepcopy from netifaces import interfaces from vyos.config import Config @@ -88,10 +87,11 @@ def generate(l2tpv3): return None def apply(l2tpv3): - # L2TPv3 interface needs to be created/deleted on-block, instead of - # passing a ton of arguments, I just use a dict that is managed by - # vyos.ifconfig - conf = deepcopy(L2TPv3If.get_config()) + # This is a special type of interface which needs additional parameters + # when created using iproute2. Instead of passing a ton of arguments, + # use a dictionary provided by the interface class which holds all the + # options necessary. + conf = L2TPv3If.get_config() # Check if L2TPv3 interface already exists if l2tpv3['ifname'] in interfaces(): diff --git a/src/conf_mode/interfaces-macsec.py b/src/conf_mode/interfaces-macsec.py index 2866ccc0a..73c80866a 100755 --- a/src/conf_mode/interfaces-macsec.py +++ b/src/conf_mode/interfaces-macsec.py @@ -16,7 +16,6 @@ import os -from copy import deepcopy from sys import exit from vyos.config import Config @@ -102,12 +101,11 @@ def apply(macsec): os.unlink(wpa_suppl_conf.format(**macsec)) else: - # MACsec interfaces require a configuration when they are added using - # iproute2. This static method will provide the configuration - # dictionary used by this class. - - # XXX: subject of removal after completing T2653 - conf = deepcopy(MACsecIf.get_config()) + # This is a special type of interface which needs additional parameters + # when created using iproute2. Instead of passing a ton of arguments, + # use a dictionary provided by the interface class which holds all the + # options necessary. + conf = MACsecIf.get_config() conf['source_interface'] = macsec['source_interface'] conf['security_cipher'] = macsec['security']['cipher'] diff --git a/src/conf_mode/interfaces-pseudo-ethernet.py b/src/conf_mode/interfaces-pseudo-ethernet.py index 59edca1cc..98397b28f 100755 --- a/src/conf_mode/interfaces-pseudo-ethernet.py +++ b/src/conf_mode/interfaces-pseudo-ethernet.py @@ -14,9 +14,6 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -import os - -from copy import deepcopy from sys import exit from vyos.config import Config @@ -101,9 +98,11 @@ def apply(peth): if 'mode_old' in peth: MACVLANIf(peth['ifname']).remove() - # MACVLAN interface needs to be created on-block instead of passing a ton - # of arguments, I just use a dict that is managed by vyos.ifconfig - conf = deepcopy(MACVLANIf.get_config()) + # This is a special type of interface which needs additional parameters + # when created using iproute2. Instead of passing a ton of arguments, + # use a dictionary provided by the interface class which holds all the + # options necessary. + conf = MACVLANIf.get_config() # Assign MACVLAN instance configuration parameters to config dict conf['source_interface'] = peth['source_interface'] diff --git a/src/conf_mode/interfaces-vxlan.py b/src/conf_mode/interfaces-vxlan.py index bea3aa25b..a00c58608 100755 --- a/src/conf_mode/interfaces-vxlan.py +++ b/src/conf_mode/interfaces-vxlan.py @@ -17,7 +17,6 @@ import os from sys import exit -from copy import deepcopy from netifaces import interfaces from vyos.config import Config @@ -95,10 +94,11 @@ def apply(vxlan): v.remove() if 'deleted' not in vxlan: - # VXLAN interface needs to be created on-block - # instead of passing a ton of arguments, I just use a dict - # that is managed by vyos.ifconfig - conf = deepcopy(VXLANIf.get_config()) + # This is a special type of interface which needs additional parameters + # when created using iproute2. Instead of passing a ton of arguments, + # use a dictionary provided by the interface class which holds all the + # options necessary. + conf = VXLANIf.get_config() # Assign VXLAN instance configuration parameters to config dict for tmp in ['vni', 'group', 'source_address', 'source_interface', 'remote', 'port']: diff --git a/src/conf_mode/interfaces-wireless.py b/src/conf_mode/interfaces-wireless.py index c6c843e7b..be59b72b5 100755 --- a/src/conf_mode/interfaces-wireless.py +++ b/src/conf_mode/interfaces-wireless.py @@ -18,7 +18,6 @@ import os from sys import exit from re import findall -from copy import deepcopy from netaddr import EUI, mac_unix_expanded from vyos.config import Config @@ -233,10 +232,11 @@ def apply(wifi): if 'deleted' in wifi: WiFiIf(interface).remove() else: - # WiFi interface needs to be created on-block (e.g. mode or physical - # interface) instead of passing a ton of arguments, I just use a dict - # that is managed by vyos.ifconfig - conf = deepcopy(WiFiIf.get_config()) + # This is a special type of interface which needs additional parameters + # when created using iproute2. Instead of passing a ton of arguments, + # use a dictionary provided by the interface class which holds all the + # options necessary. + conf = WiFiIf.get_config() # Assign WiFi instance configuration parameters to config dict conf['phy'] = wifi['physical_device'] -- cgit v1.2.3