Skip to content
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

[config-engine] minigraph.py refactoring #448

Merged
merged 8 commits into from
Mar 30, 2017

Conversation

taoyl-ms
Copy link
Contributor

There is inconsistency among the interface of different field in current minigraph.py, and that is causing confusion and leads to misconfiguration in some of our j2 templates. This PR makes the following modification to the minigraph.py output:

  • Consolidate the port/interface output. Now we have minigraph_portchannels and minigraph_vlans which are dictionaries with port channel name / vlan name as key, and portchannel members / vlan members and vlan ids as value. The configuration that needs to be generated by iterating though all portchannels / vlans should use these fields.
  • On the other hand, we have minigraph_portchannel_interfaces and minigraph_vlan_interfaces for the ip address configured on portchannels and vlans. Those fields are formatted in lists and have the similar format with minigraph_interfaces. Those fields should be used when needs to iterate though all ip addresses / networks.
  • This PR also fixes several templates where they iterate through the wrong field and have potential issue when there are multiple addresses configured on a single port.
  • To keep it consistent, front panel ports now also have a minigraph_ports dictionary, together with the existing minigraph_interfaces list. The current values of this dictionary are port names and alias, and we can consider extending it with front panel indices.
  • In the previous version, name and alias in minigraph_interfaces are opposite from which in port_config.ini. This is fixed in the new minigraph_ports. (name - sonic name; alias - vendor-specific name)
  • minigraph_interfaces will no longer have name and alias, but only a attachto where port sonic name will be put into.
  • As alias information is included in minigraph_ports, alias_map field is no longer needed.

@taoyl-ms
Copy link
Contributor Author

taoyl-ms commented Mar 28, 2017

Sample outputs after this PR:

  • minigraph_ports:
    {'Ethernet0': {'alias': 'fortyGigE0/0', 'name': 'Ethernet0'}, 'Ethernet4': {'alias': 'fortyGigE0/4', 'name': 'Ethernet4'}}

  • minigraph_vlans:
    {'Vlan1000': {'name': 'Vlan1000', 'members': ['Ethernet0', 'Ethernet4']}}

  • minigraph_portchannels:
    {'PortChannel01': {'name': 'PortChannel13', 'members': ['Ethernet8']}}

  • minigraph_interfaces / minigraph_portchannel_interfaces / minigraph_vlan_interfaces (in the same format):
    [{'subnet': IPv4Network('10.0.0.56/31'), 'peer_addr': IPv4Address('10.0.0.57'), 'addr': IPv4Address('10.0.0.56'), 'mask': IPv4Address('255.255.255.254'), 'attachto': 'PortChannel01', 'prefixlen': 31}, {'subnet': IPv6Network('fc00::70/126'), 'peer_addr': IPv6Address('fc00::72'), 'addr': IPv6Address('fc00::71'), 'mask': '126', 'attachto': 'PortChannel01', 'prefixlen': 126}] #Closed

@stcheng
Copy link
Contributor

stcheng commented Mar 28, 2017

Based on the sample output, when will the SONiC alias used? when will the 'fortyGigE0/4' used? Will there be any replacement in the output? #Resolved

@stcheng
Copy link
Contributor

stcheng commented Mar 28, 2017

Could you also add some basic tests to ensure some output? and attach a generated t0 /etc/network/interfaces file here in the comment #Resolved

{% endfor %}
{% for vlan_interface in minigraph_vlan_interfaces|unique_name %}
Copy link
Contributor

@stcheng stcheng Mar 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you also remove the unique_name filter in sonic-cfggen? #WontFix

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel it's OK to keep it?


In reply to: 108321887 [](ancestors = 108321887)

{% for vlan in minigraph_vlans.keys()|sort %}
auto {{ vlan }}
{% endfor %}

Copy link
Contributor

@stcheng stcheng Mar 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't know if it will work or not to list all the vlan interfaces aside and the ip addresses aside. if doing in this way, move the 'bridge_ports none' to here so that even if we have multiple IPs, we don't have multiple 'bridge_ports none' lines. #Resolved

Copy link
Contributor

@stcheng stcheng Mar 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it turns out to be that bridge_ports should follow iface line. thanks @taoyl-ms . let's stick auto stanza with iface stanza together for better readability. #Resolved

{% for pc in minigraph_portchannels.keys()|sort %}
auto {{ pc }}
allow-hotplug {{ pc }}
{% endfor %}
Copy link
Contributor

@stcheng stcheng Mar 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't know if it is good to separate the iface stanza with the auto stanzas. #Resolved

Copy link
Contributor

@stcheng stcheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments left.

@lguohan
Copy link
Collaborator

lguohan commented Mar 29, 2017

does this pass the sonic-cfggen unit test? Is it running? #Resolved

@taoyl-ms
Copy link
Contributor Author

The only output where 'fortyGigE0/4' will present is in the "alias" field of minigraph_ports. All the other fields use SONiC name.


In reply to: 289641473 [](ancestors = 289641473)

@taoyl-ms
Copy link
Contributor Author

It seems the test is not running automatically after we moved from deb to whl. Just edited slave.mk to reenable it.


In reply to: 290018772 [](ancestors = 290018772)

@taoyl-ms
Copy link
Contributor Author

taoyl-ms commented Mar 29, 2017

Upload a sample /etc/network/interfaces file for T0 after this refactor.
interfaces.txt
#Resolved

@taoyl-ms
Copy link
Contributor Author

taoyl-ms commented Mar 29, 2017

All CR comments resolved. Please help review. #Closed

@taoyl-ms
Copy link
Contributor Author

taoyl-ms commented Mar 30, 2017

This will resolve #409

@stcheng
Copy link
Contributor

stcheng commented Mar 30, 2017

error: [Errno 2] No such file or directory: 'build/bdist.linux-x86_64/wheel/openconfig_acl.py'
do you know why this error appears?

@stcheng
Copy link
Contributor

stcheng commented Mar 30, 2017

resolve the conflicts?

@taoyl-ms taoyl-ms merged commit fed908f into sonic-net:master Mar 30, 2017
taoyl-ms added a commit that referenced this pull request Apr 3, 2017
taoyl-ms added a commit that referenced this pull request Apr 4, 2017
* [snmp] Fix a bug in SNMP alias mapping Which was introduced in #448.
rodnymolina pushed a commit to rodnymolina/sonic-buildimage that referenced this pull request Aug 17, 2017
    * Adjusting FRR's jinja template to meet latest sonic-cfgen requirements. Basically, i'm just extending sonic-net#448 changes into FRR.

    * Eliminate FRR's integrated-config file to prevent daemons from bypassing their own config files. FRR daemons now default to an integrated-config file for config-parsing purposes. But we are still interested in having each daemon looking in their specific config file (bgpd.conf, zebra.conf, etc). So here i'm just deleting this integrating-config file to prevent FRR from running from a bogus config-file.

RB=
G=lnos-reviewers
R=ntrianta,rjonnadu,rmolina,sfardeen,zxu
A=
lguohan pushed a commit that referenced this pull request Aug 17, 2017
#895)

* Adjusting FRR's jinja template to meet latest sonic-cfgen requirements. Basically, i'm just extending #448 changes into FRR.

    * Eliminate FRR's integrated-config file to prevent daemons from bypassing their own config files. FRR daemons now default to an integrated-config file for config-parsing purposes. But we are still interested in having each daemon looking in their specific config file (bgpd.conf, zebra.conf, etc). So here i'm just deleting this integrating-config file to prevent FRR from running from a bogus config-file.

RB=
G=lnos-reviewers
R=ntrianta,rjonnadu,rmolina,sfardeen,zxu
A=
stcheng pushed a commit that referenced this pull request Sep 22, 2017
…er (#903)

* Fixing a couple of issues to enable FRR to run with latest SONiC code.

* Adjusting FRR's jinja template to meet latest sonic-cfgen requirements. Basically, i'm just extending #448 changes into FRR.

* Eliminate FRR's integrated-config file to prevent daemons from bypassing their own config files. FRR daemons now default to an integrated-config file for config-parsing purposes. But we are still interested in having each daemon looking in their specific config file (bgpd.conf, zebra.conf, etc). So here i'm just deleting this integrating-config file to prevent FRR from running from a bogus config-file.

* Allows interactive session with the slave docker-container after conclusion of the building process.

Proposed changes provide a more direct access to the building pipeline,
by allowing users to launch consecutive compilation tasks from the same
slave container.
volodymyrsamotiy pushed a commit to volodymyrsamotiy/sonic-buildimage that referenced this pull request Apr 25, 2019
* 1b0d609 2019-04-24 [SAI] Advance submodule to v1.4.1 (sonic-net#450) [Marian Pritsak]
* 3d87ad1 2019-04-23 Add metadata serialization support for buffer pool stats (sonic-net#448) [Wenda Ni]
* a17e54e 2019-04-23 [SAI]: Move SAI pointer to v1.4 (sonic-net#447) [Shuotian Cheng]
* f8950b7 2019-04-23 Add warm-boot feature processing for wedge100bf_32x/65x platforms (sonic-net#434) [Pavlo Yadvichuk]
* e9e9dc3 2019-04-18 Add a comment to fix the docker issue (sonic-net#442) [pavel-shirshov]

Signed-off-by: Volodymyr Samotiy <volodymyrs@mellanox.com>
MichelMoriniaux pushed a commit to criteo-forks/sonic-buildimage that referenced this pull request May 28, 2019
* 1b0d609 2019-04-24 [SAI] Advance submodule to v1.4.1 (sonic-net#450) [Marian Pritsak]
* 3d87ad1 2019-04-23 Add metadata serialization support for buffer pool stats (sonic-net#448) [Wenda Ni]
* a17e54e 2019-04-23 [SAI]: Move SAI pointer to v1.4 (sonic-net#447) [Shuotian Cheng]
* f8950b7 2019-04-23 Add warm-boot feature processing for wedge100bf_32x/65x platforms (sonic-net#434) [Pavlo Yadvichuk]
* e9e9dc3 2019-04-18 Add a comment to fix the docker issue (sonic-net#442) [pavel-shirshov]

Signed-off-by: Volodymyr Samotiy <volodymyrs@mellanox.com>
madhanmellanox pushed a commit to madhanmellanox/sonic-buildimage that referenced this pull request Mar 23, 2020
* update to SAI 1.2

* set LAG PORT_VLAN_ID instead of member port VLAN id

* [test]: Add test for the CRM feature

Signed-off-by: Volodymyr Samotiy <volodymyrs@mellanox.com>

* [test]: Add test for the CRM feature

* Fix review comments

Signed-off-by: Volodymyr Samotiy <volodymyrs@mellanox.com>

* update to SAI 1.2

* set LAG PORT_VLAN_ID instead of member port VLAN id

* [test]: Add ACL test for the CRM feature

Signed-off-by: Volodymyr Samotiy <volodymyrs@mellanox.com>

* update to SAI 1.2

* set LAG PORT_VLAN_ID instead of member port VLAN id

* [test]: Add test for the CRM feature

* Use static neighbors instead of configuring server side
* Change polling interval value to 1 second
* Rename "nexthop_group_object" resource to "nexthop_group"

Signed-off-by: Volodymyr Samotiy <volodymyrs@mellanox.com>

* [crmorch]: Implement CRM feature (sonic-net#447)

Signed-off-by: Volodymyr Samotiy <volodymyrs@mellanox.com>

* [lua]: Only set the entry values when they are not nil (sonic-net#446)

* [lua]: Only set the entries when they are not nil

Signed-off-by: Sihui Han <sihan@microsoft.com>

* udpate

* further fix the detection time nil error

Signed-off-by: Sihui Han <sihan@microsoft.com>

* [test]: Add test for the CRM feature

* Fix fails in some test cases

Signed-off-by: Volodymyr Samotiy <volodymyrs@mellanox.com>

* [test]: Add test for the CRM feature

* Fix fails in IPv6 test cases

Signed-off-by: Volodymyr Samotiy <volodymyrs@mellanox.com>
dmytroxshevchuk pushed a commit to dmytroxshevchuk/sonic-buildimage that referenced this pull request Aug 31, 2020
tahmed-dev added a commit to tahmed-dev/sonic-buildimage that referenced this pull request Feb 18, 2021
Change in this update:
    b75aab7 [swss-common] Add LINKMGR CFG and MUX LINKMGR state table names (sonic-net#421)
    4a77d1c [ci]: add vstest (sonic-net#459)
    07258a6 [ci]: use build template (sonic-net#457)
    ddcae3e runRedisScript api to process integer returned by script run in the redis (sonic-net#447)
    33d89c7 [systemlag] Schema defs for system lag (sonic-net#448)
    af01f37 spell check fixes (sonic-net#456)
    7afd43d Update to make getNamespaces() API at par with the get_ns_list() swssdk-py API. (sonic-net#455)

signed-off-by: Tamer Ahmed <tamer.ahmed@microsoft.com>
tahmed-dev added a commit that referenced this pull request Feb 18, 2021
Change in this update:
    b75aab7 [swss-common] Add LINKMGR CFG and MUX LINKMGR state table names (#421)
    4a77d1c [ci]: add vstest (#459)
    07258a6 [ci]: use build template (#457)
    ddcae3e runRedisScript api to process integer returned by script run in the redis (#447)
    33d89c7 [systemlag] Schema defs for system lag (#448)
    af01f37 spell check fixes (#456)
    7afd43d Update to make getNamespaces() API at par with the get_ns_list() swssdk-py API. (#455)

signed-off-by: Tamer Ahmed <tamer.ahmed@microsoft.com>
daall pushed a commit that referenced this pull request Feb 25, 2021
Change in this update:
    b75aab7 [swss-common] Add LINKMGR CFG and MUX LINKMGR state table names (#421)
    4a77d1c [ci]: add vstest (#459)
    07258a6 [ci]: use build template (#457)
    ddcae3e runRedisScript api to process integer returned by script run in the redis (#447)
    33d89c7 [systemlag] Schema defs for system lag (#448)
    af01f37 spell check fixes (#456)
    7afd43d Update to make getNamespaces() API at par with the get_ns_list() swssdk-py API. (#455)

signed-off-by: Tamer Ahmed <tamer.ahmed@microsoft.com>
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
Change in this update:
    b75aab7 [swss-common] Add LINKMGR CFG and MUX LINKMGR state table names (sonic-net#421)
    4a77d1c [ci]: add vstest (sonic-net#459)
    07258a6 [ci]: use build template (sonic-net#457)
    ddcae3e runRedisScript api to process integer returned by script run in the redis (sonic-net#447)
    33d89c7 [systemlag] Schema defs for system lag (sonic-net#448)
    af01f37 spell check fixes (sonic-net#456)
    7afd43d Update to make getNamespaces() API at par with the get_ns_list() swssdk-py API. (sonic-net#455)

signed-off-by: Tamer Ahmed <tamer.ahmed@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants