-
Notifications
You must be signed in to change notification settings - Fork 1k
Remove lldp as default service for Chassis #13115
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
Changes from all commits
febcacd
9582616
940ebe4
36feb13
ab4beee
8d59326
fa03609
532639a
0833669
f2dbf92
915dc97
1aacc7a
6ca8577
7ad4900
a6d5c16
03453b0
5fce19c
501698b
9d6f058
e7b6504
402c17c
fe770ed
dddc82a
c3f8665
c17e4f5
3878c7d
6266b90
37f880d
00a0dc6
a3a5999
9f1a2a7
f07b31e
03fe4fe
825c8b2
0b954ec
7293537
67a8074
5176515
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 |
|---|---|---|
|
|
@@ -21,7 +21,7 @@ class MultiAsicSonicHost(object): | |
| So, even a single asic pizza box is represented as a MultiAsicSonicHost with 1 SonicAsic. | ||
| """ | ||
|
|
||
| _DEFAULT_SERVICES = ["pmon", "snmp", "lldp", "database"] | ||
| _DEFAULT_SERVICES = ["pmon", "snmp", "database"] | ||
|
|
||
| def __init__(self, ansible_adhoc, hostname, duthosts, topo_type): | ||
| """ Initializing a MultiAsicSonicHost. | ||
|
|
@@ -64,13 +64,16 @@ def critical_services_tracking_list(self): | |
| """ | ||
| service_list = [] | ||
| active_asics = self.asics | ||
| if self.sonichost.is_supervisor_node() and self.get_facts()['asic_type'] != 'vs': | ||
| active_asics = [] | ||
| sonic_db_cli_out = self.command("sonic-db-cli CHASSIS_STATE_DB keys \"CHASSIS_FABRIC_ASIC_TABLE|asic*\"") | ||
| for a_asic_line in sonic_db_cli_out["stdout_lines"]: | ||
| a_asic_name = a_asic_line.split("|")[1] | ||
| a_asic_instance = self.asic_instance_from_namespace(namespace=a_asic_name) | ||
| active_asics.append(a_asic_instance) | ||
| if self.sonichost.is_supervisor_node(): | ||
| self._DEFAULT_SERVICES.append("lldp") | ||
|
Contributor
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. based on this condition, lldp will not be added in DEFAULT_SERVICES for VS supervisor, is that right?
Contributor
Author
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. @SuvarnaMeenakshi : updated. |
||
| if self.get_facts()['asic_type'] != 'vs': | ||
| active_asics = [] | ||
| sonic_db_cli_out = \ | ||
| self.command("sonic-db-cli CHASSIS_STATE_DB keys \"CHASSIS_FABRIC_ASIC_TABLE|asic*\"") | ||
| for a_asic_line in sonic_db_cli_out["stdout_lines"]: | ||
| a_asic_name = a_asic_line.split("|")[1] | ||
| a_asic_instance = self.asic_instance_from_namespace(namespace=a_asic_name) | ||
| active_asics.append(a_asic_instance) | ||
| service_list += self._DEFAULT_SERVICES | ||
|
|
||
| config_facts = self.config_facts(host=self.hostname, source="running")['ansible_facts'] | ||
|
|
@@ -94,6 +97,9 @@ def critical_services_tracking_list(self): | |
| self.sonichost.DEFAULT_ASIC_SERVICES.remove(service) | ||
| if config_facts['FEATURE'][service]['state'] == "disabled": | ||
| self.sonichost.DEFAULT_ASIC_SERVICES.remove(service) | ||
| else: | ||
| self._DEFAULT_SERVICES.append("lldp") | ||
|
Contributor
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. do we need this append here?
Contributor
Author
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. Yes . LLDP will be added as Default Service for Supervisor and Fixed Platoforms (else is for Fixed platforms) For Modular chassis LLDP will not be added to Default Service List which means for LC we will not add as per our requirement.
Contributor
Author
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. Am I missing something?
Contributor
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. got it, misunderstood the condition logic here. |
||
|
|
||
| for asic in active_asics: | ||
| service_list += asic.get_critical_services() | ||
| self.sonichost.reset_critical_services_tracking_list(service_list) | ||
|
|
||
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.
https://github.com/sonic-net/sonic-mgmt/blob/master/tests/common/devices/sonic.py#L43
and line #53 may also need to be fixed. Please check.
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.
It is not needed here. lldp still applies to Default asic service list