Skip to content

Skip gathering dualtor facts on single tor testbed#2646

Merged
bingwang-ms merged 1 commit intosonic-net:masterfrom
bingwang-ms:skip_dualtor_fact_for_single_tor
Dec 11, 2020
Merged

Skip gathering dualtor facts on single tor testbed#2646
bingwang-ms merged 1 commit intosonic-net:masterfrom
bingwang-ms:skip_dualtor_fact_for_single_tor

Conversation

@bingwang-ms
Copy link
Collaborator

Signed-off-by: bingwang [email protected]

Description of PR

Summary:
Fixes # (issue)
I noticed that the dual_tor_facts plugin costs around 5 minutes to gather dualtor facts when deploying or generating minigraph. The generated dualtor facts is not used on single tor testbed though.
This commit updates the task file for deploying minigraph to skip dual_tor_facts plugin on single tor testbed.

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Approach

What is the motivation for this PR?

This PR is to updates ansible/config_sonic_basedon_testbed.yml to skip dual_tor_facts plugin on single tor testbed.

How did you do it?

Add a condition check for dual_tor_facts tasks.

How did you verify/test it?

Verified by deploying minigraph on Arista7260. The task finished without exception, and the generated minigraph files are exactly the same.

Any platform specific information?

No.

Supported testbed topology if it's a new test case?

No.

Documentation

I noticed that the dual_tor_facts plugin costs around 5 minutes to
generate dualtor facts when deploying or generating minigraph.
The generated dualtor facts is not used on single tor testbed though.
This commit updates the task file for deploying minigraph to skip
dual_tor_facts plugin on single tor testbed.

Signed-off-by: bingwang <[email protected]>
- name: set default dualtor facts
set_fact:
dual_tor_facts: {}
when: "'dualtor' not in topo"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should it be like below?

when: "{{'dualtor' not in topo }}"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it OK to omit the {{ }} because it's not a template. And i verified it by deploying a minigraph.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please ignore this. I tested your code, it works. On the contrary, ansible complains with warning [WARNING]: conditional statements should not include jinja2 templating delimiters such as {{ }} or {% %}. Found: {{ 'dualtor' in topo }} if format when: "{{'dualtor' not in topo }}" is used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @wangxin

Copy link
Contributor

@theasianpianist theasianpianist left a comment

Choose a reason for hiding this comment

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

I find it interesting that the dual_tor_facts takes so long for you, it ran in a few seconds for me originally. I'll look into improving performance at some point.

@yxieca
Copy link
Collaborator

yxieca commented Dec 10, 2020

What was the time difference on 7260 before/after?

@bingwang-ms
Copy link
Collaborator Author

I find it interesting that the dual_tor_facts takes so long for you, it ran in a few seconds for me originally. I'll look into improving performance at some point.

The plugin will take a long time if we add multiple inventory files (such as str and veos). At such case, the hostvars will become huge, which means a long time is needed to transfer over network(by Ansible) and do memory copy. I was thinking to only pass neighbor info instead of the whole hostvars to dual_tor_facts, which may address the issue.

@bingwang-ms bingwang-ms merged commit 9927942 into sonic-net:master Dec 11, 2020
antoninamelnyk pushed a commit to antoninamelnyk/sonic-mgmt that referenced this pull request Dec 11, 2020
I noticed that the dual_tor_facts plugin costs around 5 minutes to
generate dualtor facts when deploying or generating minigraph.
The generated dualtor facts is not used on single tor testbed though.
This commit updates the task file for deploying minigraph to skip
dual_tor_facts plugin on single tor testbed.

Signed-off-by: bingwang <[email protected]>
antoninamelnyk added a commit to antoninamelnyk/sonic-mgmt that referenced this pull request Dec 11, 2020
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
Why I did it
submodule advance

b085b5f - [ci] Fix pipeline error about team5 not found. (Core dump in orchagent when assigning router interface to a vlan with untagged mode  sonic-net#2684) (3 hours ago) [Liu Shilong]
4549b4c - Fix issue: there is no retry while creating a RIF which is in removing state ([201811 sub-module] advance sub-modules: utilities, swss, swss-common sonic-net#2679) (3 hours ago) [Junchao-Mellanox]
980a45b - [FDB]Fixing FDB consolidated flush for Remote MACs (pmon to stretch sonic-net#2673) (3 hours ago) [Sudharsan Dhamal Gopalarathnam]
c646607 - Do not allow to add port to .1Q bridge while router port deletion is not completed (Update SDK, FW and SAI sonic-net#2669) (3 hours ago) [Lior Avramov]
4a321f0 - [orchagent]: Get bridge port ID from orchagent cache instead of SAI API ([201811 sub module] advance sairedis sub module sonic-net#2657) (3 hours ago) [Lawrence Lee]
f4b88f3 - [Dual-ToR] handle 'mux_tunnel_egress_acl' attrib in order to change ACL configuration (drop on ingress/egress) on standby ToR (lm75 doesn't support written alarm to syslog. sonic-net#2646) (3 hours ago) [Andriy Yurkiv]
a4f29c1 - [Workaround] EvpnRemoteVnip2pOrch warmboot check failure ([teamd]: wait for swss db flush done before starting teamd container sonic-net#2626) (3 hours ago) [jcaiMR]
53ee0a8 - Support for tc-dot1p and tc-dscp qosmap ([201803] [router-advertiser] Add templated script to wait for pertinent interfaces to be ready before starting radvd sonic-net#2559) (3 hours ago) [Divya Mukundan]
b953866 - [dual-tor] add missing SAI attribte in order to create IPNIP tunnel (Config reload/load_minigraph not clearing State DB sonic-net#2503) (3 hours ago) [Andriy Yurkiv]
How I did it
How to verify it
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants