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

[sflow] Add mVRF support #1230

Merged
merged 2 commits into from
Nov 13, 2020
Merged

[sflow] Add mVRF support #1230

merged 2 commits into from
Nov 13, 2020

Conversation

GarrickHe
Copy link
Contributor

@GarrickHe GarrickHe commented Nov 11, 2020

  • Add mVRF support and options to CLICK CLI

Signed-off-by: Garrick He garrick_he@dell.com

- What I did
Add options to allow mVRF configuration for sFlow collectors

- How I did it
Update the show/main.py and confg/main.py scripts

- How to verify it
Check output of CLICK CLI:

admin@sonic:~$ sudo config sflow collector add col1 1.1.1.1 --port 1111 --vrf mgmt
admin@sonic:~$ show sflow

sFlow Global Information:
  sFlow Admin State:          down
  sFlow Polling Interval:     default
  sFlow AgentID:              default

  1 Collectors configured:
    Name: col1                IP addr: 1.1.1.1         UDP port: 1111   VRF: mgmt
admin@sonic:~$ sudo config sflow collector add col2 1.1.1.2
admin@sonic:~$ show sflow

sFlow Global Information:
  sFlow Admin State:          down
  sFlow Polling Interval:     default
  sFlow AgentID:              default

  2 Collectors configured:
    Name: col1                IP addr: 1.1.1.1         UDP port: 1111   VRF: mgmt
    Name: col2                IP addr: 1.1.1.2         UDP port: 6343   VRF: default
admin@sonic:~$ sudo config sflow collector add col1 1.1.1.1 --port 1111 --vrf wrong
Invalid VRF (only default or mgmt)
admin@sonic:~$

- Previous command output (if the output of a command-line utility has changed)

admin@sonic:~$ sudo config sflow collector add col1 1.1.1.1 --port 1111
admin@sonic:~$ show sflow

sFlow Global Information:
  sFlow Admin State:          down
  sFlow Polling Interval:     default
  sFlow AgentID:              default

  1 Collectors configured:
    Name: col1                IP addr: 1.1.1.1         UDP port: 1111
admin@sonic:~$ sudo config sflow collector add col2 1.1.1.2
admin@sonic:~$ show sflow

sFlow Global Information:
  sFlow Admin State:          down
  sFlow Polling Interval:     default
  sFlow AgentID:              default

  2 Collectors configured:
    Name: col1                IP addr: 1.1.1.1         UDP port: 1111
    Name: col2                IP addr: 1.1.1.2         UDP port: 6343

- New command output (if the output of a command-line utility has changed)

admin@sonic:~$ sudo config sflow collector add col1 1.1.1.1 --port 1111 --vrf mgmt
admin@sonic:~$ show sflow

sFlow Global Information:
  sFlow Admin State:          down
  sFlow Polling Interval:     default
  sFlow AgentID:              default

  1 Collectors configured:
    Name: col1                IP addr: 1.1.1.1         UDP port: 1111   VRF: mgmt
admin@sonic:~$ sudo config sflow collector add col2 1.1.1.2
admin@sonic:~$ show sflow

sFlow Global Information:
  sFlow Admin State:          down
  sFlow Polling Interval:     default
  sFlow AgentID:              default

  2 Collectors configured:
    Name: col1                IP addr: 1.1.1.1         UDP port: 1111   VRF: mgmt
    Name: col2                IP addr: 1.1.1.2         UDP port: 6343   VRF: default
admin@sonic:~$ sudo config sflow collector add col1 1.1.1.1 --port 1111 --vrf wrong
Invalid VRF (only default or mgmt)
admin@sonic:~$

* Add mVRF support and options to CLICK CLI

Signed-off-by: Garrick He <garrick_he@dell.com>
@GarrickHe
Copy link
Contributor Author

DO NOT MERGE UNTIL THIS PR IS MERGED:

sonic-net/sonic-buildimage#5904

prsunny
prsunny previously approved these changes Nov 12, 2020
@prsunny
Copy link
Contributor

prsunny commented Nov 12, 2020

retest this please

@prsunny
Copy link
Contributor

prsunny commented Nov 12, 2020

@GarrickHe , please fix the test failures on sflow

@GarrickHe
Copy link
Contributor Author

@prsunny - I looked in the details of the 'default' test. I don't see any information on what failed. Can you please tell me what am I suppose to fix? Thank you.

@prsunny
Copy link
Contributor

prsunny commented Nov 12, 2020

https://sonic-jenkins.westus2.cloudapp.azure.com/job/common/job/sonic-utilities-build-pr/2896/consoleFull

10:07:10      def test_show_sflow(self):
10:07:10          runner = CliRunner()
10:07:10          result = runner.invoke(show.cli.commands["sflow"], [], obj=Db())
10:07:10          print(result.exit_code, result.output)
10:07:10  >       assert result.exit_code == 0
10:07:10  E       AssertionError: assert 1 == 0
10:07:10  E        +  where 1 = <Result KeyError('collector_vrf',)>.exit_code

@GarrickHe
Copy link
Contributor Author

retest this please

1 similar comment
@GarrickHe
Copy link
Contributor Author

retest this please

* Update the sFlow test-cases to include VRF
* Update 'show sflow' command for backward-compatibility

Signed-off-by: Garrick He <garrick_he@dell.com>
@prsunny prsunny merged commit 3f15835 into sonic-net:master Nov 13, 2020
@harveylu-ruijie
Copy link

@GarrickHe @prsunny hi guys. I would wanna know why set this limitiion in is_valid_collector_info()
Snipaste_2024-09-27_11-16-39
Why the vrf here can not be another name , like 'Vrf1', 'Vrf2' ?
Thanks!

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.

3 participants