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

[202205] Fix issue: out of range sflow polling interval is accepted and stored in config_db (#2847) #3123

Merged
merged 1 commit into from
Jan 22, 2024

Conversation

stephenxs
Copy link
Collaborator

This is to backport #2847 to 202205

Fixed issue: out of range sflow polling interval is accepted and stored in config_db.

Reproduce step:

1. Enable sflow feature:   config feature state sflow enabled
2. Enable sflow itself:   config sflow enable
3. Configure out of range polling interval:  config sflow polling-interval 1. Error message is shown as expected
4. Save config:    config save -y
5. Check "SFLOW" section inside config_db

As the interval is invalid, the expected behavior is that the interval is not saved to redis. But we see the invalid value was written to redis.

Change click.echo to ctx.fail

  1. Manual test
  2. Add a check in existing unit test case to cover the change

What I did

How I did it

How to verify it

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

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

… in config_db (sonic-net#2847)

Fixed issue: out of range sflow polling interval is accepted and stored in config_db.

Reproduce step:
```
1. Enable sflow feature:   config feature state sflow enabled
2. Enable sflow itself:   config sflow enable
3. Configure out of range polling interval:  config sflow polling-interval 1. Error message is shown as expected
4. Save config:    config save -y
5. Check "SFLOW" section inside config_db
```

As the interval is invalid, the expected behavior is that the interval is not saved to redis. But we see the invalid value was written to redis.

Change `click.echo` to `ctx.fail`

1. Manual test
2. Add a check in existing unit test case to cover the change
@stephenxs
Copy link
Collaborator Author

Failed due to unable to download swss-common package, which isn't relevant to the PR.
@yxieca who can help on this? thanks

Starting: Download sonic swss common deb packages
==============================================================================
Task         : Download Pipeline Artifacts
Description  : Download build and pipeline artifacts
Version      : 2.198.0
Author       : Microsoft Corporation
Help         : https://docs.microsoft.com/azure/devops/pipelines/tasks/utility/download-pipeline-artifact
==============================================================================
##[error]No builds currently exist in the pipeline definition supplied.
Finishing: Download sonic swss common deb packages

@stephenxs
Copy link
Collaborator Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@saiarcot895
Copy link
Contributor

/AzurePipeline run Azure.sonic-utilities

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

@saiarcot895 @yxieca could you please help to merge?

@yxieca yxieca merged commit 7a7305e into sonic-net:202205 Jan 22, 2024
5 checks passed
@stephenxs stephenxs deleted the 202205-fix-sflow-out-of-range branch January 23, 2024 01:26
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.

7 participants