-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[cfg engine] Add support to read device description xml #775
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,11 +42,33 @@ | |
|
||
class minigraph_encoder(json.JSONEncoder): | ||
def default(self, obj): | ||
if isinstance(obj, | ||
(ipaddress.IPv4Network, ipaddress.IPv6Network, ipaddress.IPv4Address, ipaddress.IPv6Address)): | ||
if isinstance(obj, ( | ||
ipaddress.IPv4Network, ipaddress.IPv6Network, | ||
ipaddress.IPv4Address, ipaddress.IPv6Address | ||
)): | ||
return str(obj) | ||
return json.JSONEncoder.default(self, obj) | ||
|
||
def parse_device(device): | ||
lo_prefix = None | ||
mgmt_prefix = None | ||
d_type = None # don't shadow type() | ||
hwsku = None | ||
name = None | ||
if str(QName(ns3, "type")) in device.attrib: | ||
d_type = device.attrib[str(QName(ns3, "type"))] | ||
|
||
for node in device: | ||
if node.tag == str(QName(ns, "Address")): | ||
lo_prefix = node.find(str(QName(ns2, "IPPrefix"))).text | ||
elif node.tag == str(QName(ns, "ManagementAddress")): | ||
mgmt_prefix = node.find(str(QName(ns2, "IPPrefix"))).text | ||
elif node.tag == str(QName(ns, "Hostname")): | ||
name = node.text | ||
elif node.tag == str(QName(ns, "HwSku")): | ||
hwsku = node.text | ||
return (lo_prefix, mgmt_prefix, name, hwsku, d_type) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
name may be not itialized. #Closed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
||
def parse_png(png, hname): | ||
neighbors = {} | ||
devices = {} | ||
|
@@ -77,24 +99,9 @@ def parse_png(png, hname): | |
|
||
if child.tag == str(QName(ns, "Devices")): | ||
for device in child.findall(str(QName(ns, "Device"))): | ||
lo_addr = None | ||
# don't shadow type() | ||
d_type = None | ||
mgmt_addr = None | ||
hwsku = None | ||
if str(QName(ns3, "type")) in device.attrib: | ||
d_type = device.attrib[str(QName(ns3, "type"))] | ||
|
||
for node in device: | ||
if node.tag == str(QName(ns, "Address")): | ||
lo_addr = node.find(str(QName(ns2, "IPPrefix"))).text.split('/')[0] | ||
elif node.tag == str(QName(ns, "ManagementAddress")): | ||
mgmt_addr = node.find(str(QName(ns2, "IPPrefix"))).text.split('/')[0] | ||
elif node.tag == str(QName(ns, "Hostname")): | ||
name = node.text | ||
elif node.tag == str(QName(ns, "HwSku")): | ||
hwsku = node.text | ||
|
||
(lo_prefix, mgmt_prefix, name, hwsku, d_type) = parse_device(device) | ||
lo_addr = None if not lo_prefix else lo_prefix.split('/')[0] | ||
mgmt_addr = None if not mgmt_prefix else mgmt_prefix.split('/')[0] | ||
devices[name] = {'lo_addr': lo_addr, 'type': d_type, 'mgmt_addr': mgmt_addr, 'hwsku': hwsku} | ||
|
||
if child.tag == str(QName(ns, "DeviceInterfaceLinks")): | ||
|
@@ -270,7 +277,7 @@ def parse_cpg(cpg, hname): | |
myasn = int(asn) | ||
peers = router.find(str(QName(ns1, "Peers"))) | ||
for bgpPeer in peers.findall(str(QName(ns, "BGPPeer"))): | ||
addr = bgpPeer.find(str(QName(ns, "Address"))).text | ||
addr = bgpPeer.find(str(QName(ns, "Address"))).text | ||
if bgpPeer.find(str(QName(ns1, "PeersRange"))) is not None: | ||
name = bgpPeer.find(str(QName(ns1, "Name"))).text | ||
ip_range = bgpPeer.find(str(QName(ns1, "PeersRange"))).text | ||
|
@@ -382,7 +389,6 @@ def parse_port_config(hwsku, platform=None, port_config_file=None): | |
port_alias_map[alias] = name | ||
return ports | ||
|
||
|
||
def parse_xml(filename, platform=None, port_config_file=None): | ||
root = ET.parse(filename).getroot() | ||
mini_graph_path = filename | ||
|
@@ -432,9 +438,7 @@ def parse_xml(filename, platform=None, port_config_file=None): | |
elif child.tag == str(QName(ns, "MetadataDeclaration")): | ||
(syslog_servers, dhcp_servers, ntp_servers, mgmt_routes, erspan_dst, deployment_id) = parse_meta(child, hostname) | ||
|
||
Tree = lambda: defaultdict(Tree) | ||
|
||
results = Tree() | ||
results = {} | ||
results['minigraph_hwsku'] = hwsku | ||
# sorting by lambdas are not easily done without custom filters. | ||
# TODO: add jinja2 filter to accept a lambda to sort a list of dictionaries by attribute. | ||
|
@@ -484,6 +488,38 @@ def parse_xml(filename, platform=None, port_config_file=None): | |
|
||
return results | ||
|
||
def parse_device_desc_xml(filename): | ||
root = ET.parse(filename).getroot() | ||
(lo_prefix, mgmt_prefix, hostname, hwsku, d_type) = parse_device(root) | ||
|
||
results = {} | ||
results['minigraph_hwsku'] = hwsku | ||
results['minigraph_hostname'] = hostname | ||
results['inventory_hostname'] = hostname | ||
|
||
lo_intfs = [] | ||
ipn = ipaddress.IPNetwork(lo_prefix) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
handle exception? #WontFix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm feeling letting the exception to be thrown out and crashing at the outermost layer is the best behavior if exception happens here. In reply to: 125150698 [](ancestors = 125150698) |
||
ipaddr = ipn.ip | ||
prefix_len = ipn.prefixlen | ||
ipmask = ipn.netmask | ||
lo_intf = {'name': None, 'addr': ipaddr, 'prefixlen': prefix_len} | ||
if isinstance(ipn, ipaddress.IPv4Network): | ||
lo_intf['mask'] = ipmask | ||
else: | ||
lo_intf['mask'] = str(prefix_len) | ||
lo_intfs.append(lo_intf) | ||
results['minigraph_lo_interfaces'] = lo_intfs | ||
|
||
mgmt_intf = None | ||
mgmt_ipn = ipaddress.IPNetwork(mgmt_prefix) | ||
ipaddr = mgmt_ipn.ip | ||
prefix_len = str(mgmt_ipn.prefixlen) | ||
ipmask = mgmt_ipn.netmask | ||
gwaddr = ipaddress.IPAddress(int(mgmt_ipn.network) + 1) | ||
mgmt_intf = {'addr': ipaddr, 'prefixlen': prefix_len, 'mask': ipmask, 'gwaddr': gwaddr} | ||
results['minigraph_mgmt_interface'] = mgmt_intf | ||
return results | ||
|
||
port_alias_map = {} | ||
|
||
def print_parse_xml(filename): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ import netaddr | |
import json | ||
from minigraph import minigraph_encoder | ||
from minigraph import parse_xml | ||
from minigraph import parse_device_desc_xml | ||
from sonic_platform import get_machine_info | ||
from sonic_platform import get_platform_info | ||
|
||
|
@@ -47,7 +48,9 @@ def unique_name(l): | |
|
||
def main(): | ||
parser=argparse.ArgumentParser(description="Render configuration file from minigraph data and jinja2 template.") | ||
parser.add_argument("-m", "--minigraph", help="minigraph xml file") | ||
group = parser.add_mutually_exclusive_group() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If one is required, you may use 'required=True' #Closed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
group.add_argument("-m", "--minigraph", help="minigraph xml file") | ||
group.add_argument("-M", "--device-description", help="device description xml file") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
instead of adding a new option, can we treat the device xml as a variant(subset) of minigraph which only contains a subset of information? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't see your point. Those are different files with different schema. I already make the output format consistence so you can use same tool and same template to generate, for example, |
||
parser.add_argument("-p", "--port-config", help="port config file, used with -m") | ||
parser.add_argument("-y", "--yaml", help="yaml file that contains addtional variables", action='append', default=[]) | ||
parser.add_argument("-a", "--additional-data", help="addition data, in json string") | ||
|
@@ -79,6 +82,9 @@ def main(): | |
else: | ||
data.update(parse_xml(minigraph)) | ||
|
||
if args.device_description != None: | ||
data.update(parse_device_desc_xml(args.device_description)) | ||
|
||
for yaml_file in args.yaml: | ||
with open(yaml_file, 'r') as stream: | ||
additional_data = yaml.load(stream) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
<Device i:type="ToRRouter" xmlns="Microsoft.Search.Autopilot.Evolution" xmlns:i="http://www.w3.org/2001/XMLSchema-instance"> | ||
<ElementType>ToRRouter</ElementType> | ||
<Address xmlns:a="Microsoft.Search.Autopilot.NetMux"> | ||
<a:IPPrefix>10.10.0.12/32</a:IPPrefix> | ||
</Address> | ||
<AddressV6 xmlns:a="Microsoft.Search.Autopilot.NetMux"> | ||
<a:IPPrefix>::/0</a:IPPrefix> | ||
</AddressV6> | ||
<ManagementAddress xmlns:a="Microsoft.Search.Autopilot.NetMux"> | ||
<a:IPPrefix>10.0.1.5/28</a:IPPrefix> | ||
</ManagementAddress> | ||
<ManagementAddressV6 xmlns:a="Microsoft.Search.Autopilot.NetMux"> | ||
<a:IPPrefix>::/0</a:IPPrefix> | ||
</ManagementAddressV6> | ||
<Hostname>switch1</Hostname> | ||
<HwSku>ACS-MSN2700</HwSku> | ||
</Device> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggest init as empty string, so later conditional split can be simplified. #ByDesign
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep it None as returning an empty string might make template debug mroe difficult than None.
In reply to: 125150571 [](ancestors = 125150571)