Changes to add template support for copp.json. #5053
Conversation
This is needed so that we can install differnt type of Traps based on Device Role (Tor/Leaf/Mgmt/etc...). Initial use case is to install DHCP/DHCPv6 tarp only for tor router. Signed-off-by: Abhishek Dosi <[email protected]>
| }, | ||
| "OP": "SET" | ||
| }, | ||
| {% if DEVICE_METADATA['localhost']['type'] == "ToRRouter" %} |
There was a problem hiding this comment.
It would be better to check for the existence of type. If not present, the template might crash - Ref
There was a problem hiding this comment.
Could you please ensure if the type is not defined, the trap is installed? Type is not a mandatory parameter and this might cause backward compatibility issues in cases where type field is not defined
There was a problem hiding this comment.
@prsunny @dgsudharsan Updated to add default else condition for backward compatibility
if DEVICE_METADATA and it sub-parameters are not define.
|
@dgsudharsan, please review |
Signed-off-by: Abhishek Dosi <[email protected]>
| }, | ||
| "OP": "SET" | ||
| }, | ||
| {% if DEVICE_METADATA is defined and DEVICE_METADATA['localhost'] is defined and DEVICE_METADATA['localhost']['type'] is defined and DEVICE_METADATA['localhost']['type'] == "ToRRouter" %} |
There was a problem hiding this comment.
IMO, device_metadata and device_metadata['localhost'] check is kind of redundant. Our initialization is highly dependent on this.
There was a problem hiding this comment.
@prsunny yes that is correct and few places we are not even checking type also.
However just made it to make sure we can avoid any exception at least for this template
| { | ||
| "COPP_TABLE:trap.group.lldp.udld": { | ||
| "trap_ids": "lldp,udld", | ||
| "trap_action":"trap", |
There was a problem hiding this comment.
If type (if defined) is not ToRRouter have the trap-ids only for lldp & udlp and else case have ""trap_ids": "lldp,dhcp,dhcpv6,udld","
There was a problem hiding this comment.
@venkatmahalingam yes this is the behavior
There was a problem hiding this comment.
@venkatmahalingam got your point. Updated accordingly.
|
retest vsimage please |
|
retest vsimage please |
Removed 00-copp.config.json from swss debian package Changes to make copp.json as j2 template based based on device role. copp.json.j2 is moved into docker-orchagent. docker-init.sh of orchagent will create json file form j2 template. This PR is dependent on : sonic-net/sonic-buildimage#5053 Signed-off-by: Abhishek Dosi <[email protected]>
Removed 00-copp.config.json from swss debian package Changes to make copp.json as j2 template based based on device role. copp.json.j2 is moved into docker-orchagent. docker-init.sh of orchagent will create json file form j2 template. This PR is dependent on : sonic-net/sonic-buildimage#5053 Signed-off-by: Abhishek Dosi <[email protected]>
* Changes to add template support for copp.json. This is needed so that we can install differnt type of Traps based on Device Role (Tor/Leaf/Mgmt/etc...). Initial use case is to install DHCP/DHCPv6 tarp only for tor router. Signed-off-by: Abhishek Dosi <[email protected]> * Fixed based on review comments. Signed-off-by: Abhishek Dosi <[email protected]> * Fixed based on review comment.
|
hi @abdosi |
* Changes to add template support for copp.json. This is needed so that we can install differnt type of Traps based on Device Role (Tor/Leaf/Mgmt/etc...). Initial use case is to install DHCP/DHCPv6 tarp only for tor router. Signed-off-by: Abhishek Dosi <[email protected]> * Fixed based on review comments. Signed-off-by: Abhishek Dosi <[email protected]> * Fixed based on review comment.
Changes:=
Changes to add template support for copp.json.
This is needed so that we can install different type of
Traps based on Device Role (Tor/Leaf/Mgmt/etc...).
Initial use case is to install DHCP/DHCPv6 trap only
for Tor router.
This PR is dependent on sonic-net/sonic-swss#1366.
Currently this change is not compatible with warm-reboot from previous image version.
Need to handle warm-reboot case with this change. This will done in separate PR
Testing:
Manually Verified based on Device Role json file getting generated. Also verfied if Device_Metadata or it's sub-param are not present then no exception is generated and correct template is formed.
Dump APP DB to have correct traps based on generated json file.