Skip to content

Generic Config Updater cacl test#4775

Merged
wen587 merged 2 commits intosonic-net:masterfrom
wen587:gu_acl
Dec 3, 2021
Merged

Generic Config Updater cacl test#4775
wen587 merged 2 commits intosonic-net:masterfrom
wen587:gu_acl

Conversation

@wen587
Copy link
Contributor

@wen587 wen587 commented Nov 29, 2021

Description of PR

Summary:
Fixes # (issue)

Type of change

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

Back port request

  • 201911

Approach

What is the motivation for this PR?

End to End test support for Generic Updater apply-patch
This PR is to verify the usage of 'config apply-patch' works on acl control plane

How did you do it?

First we setup clean ACL env. Then make some config apply change. And check if the ACL is changed as expected.

How did you verify/test it?

Run test of sonic-mgmt/tests/generic_config_updater/test_acl.py on KVM

Any platform specific information?

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

Documentation

@wen587 wen587 requested a review from qiluo-msft November 29, 2021 10:18
from tests.generic_config_updater.gu_utils import generate_tmpfile, delete_tmpfile

pytestmark = [
pytest.mark.topology('t0'),
Copy link
Contributor

@qiluo-msft qiluo-msft Nov 30, 2021

Choose a reason for hiding this comment

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

t0

control plane acls are applicable to any topology, not just t0. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

@@ -0,0 +1,360 @@
import logging
Copy link
Contributor

@qiluo-msft qiluo-msft Nov 30, 2021

Choose a reason for hiding this comment

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

import

Since this test case is design for control plane acl, let's name the script test_cacl.py ? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed

duthost.shell("sudo cp /etc/sonic/config_db.json {}".format(config_tmpfile))

# Cleanup acl config
duthost.shell('sonic-db-cli CONFIG_DB keys "ACL_RULE|*" | xargs sonic-db-cli CONFIG_DB del',
Copy link
Contributor

@qiluo-msft qiluo-msft Nov 30, 2021

Choose a reason for hiding this comment

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

xargs

Did you test with empty ACL_RULE table? I guess you need the option --no-run-if-empty. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested with empty and the command failed. So I added module_ignore_errors=True to get that error ignored.
Seems yours is the better way to handle an empty table. I will remove module_ignore_error

"""
cmds = "show acl table"
output = duthost.shell(cmds)
pytest_assert(not output['rc'], "'{}' is not running successfully".format(cmds))
Copy link
Contributor

@qiluo-msft qiluo-msft Nov 30, 2021

Choose a reason for hiding this comment

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

is not running

technically, the show command is executed and finished running. It is not running. You may use the words like "failed with rc={}" #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed. Thanks for the advice

def expect_res_success_acl_rule(duthost, expected_content_list, unexpected_content_list):
"""Check if acl rule added as expected
"""
cmds = "sudo iptables -S"
Copy link
Contributor

@qiluo-msft qiluo-msft Nov 30, 2021

Choose a reason for hiding this comment

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

-S

What is the reason to use -S instead of -nL ? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wen587 wen587 marked this pull request as ready for review December 1, 2021 07:05
@wen587 wen587 requested a review from a team as a code owner December 1, 2021 07:05
@wen587 wen587 changed the title GU acl test Generic Config Updater acl test Dec 1, 2021
@wen587 wen587 changed the title Generic Config Updater acl test Generic Config Updater cacl test Dec 1, 2021
@wangxin
Copy link
Collaborator

wangxin commented Dec 2, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@wen587 wen587 merged commit 188e828 into sonic-net:master Dec 3, 2021
AntonHryshchuk pushed a commit to AntonHryshchuk/sonic-mgmt that referenced this pull request Jan 4, 2022
…g updater (sonic-net#4775)

Summary:
Testcase of ACL control plane for generic updater apply-patch

What is the motivation for this PR?
End to End test support for Generic Updater apply-patch
This PR is to verify the usage of 'config apply-patch' works on ACL control plane

How did you do it?
First, we set up a clean ACL env. Then make some config apply change. And check if the ACL is changed as expected.

How did you verify/test it?
Run test of sonic-mgmt/tests/generic_config_updater/test_acl.py on KVM
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