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.

Sign up for GitHub

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

Jump to bottom

[config] Add Table hard dependency check #3159

Merged
merged 3 commits into from
Mar 7, 2024
Merged

[config] Add Table hard dependency check #3159

merged 3 commits into from
Mar 7, 2024
+67 −0

Conversation

wen587
Copy link
Contributor

@wen587 wen587 commented Feb 8, 2024 •
edited
Loading

ADO: 26732148

What I did

Add YANG hard depdency check for AAA and TACPLUS table

How I did it

Add a special check

How to verify it

Unit test

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)

@qiluo-msft qiluo-msft requested a review from liuh-80 February 9, 2024 01:35
qiluo-msft
qiluo-msft reviewed Feb 9, 2024
View reviewed changes
config/main.py Outdated Show resolved Hide resolved
qiluo-msft
qiluo-msft reviewed Feb 9, 2024
View reviewed changes
config/main.py Outdated Show resolved Hide resolved
qiluo-msft
qiluo-msft reviewed Feb 9, 2024
View reviewed changes
config/main.py Outdated
@@ -1999,6 +2002,20 @@ def override_config_db(config_db, config_input):
click.echo("Overriding completed. No service is restarted.")


def yang_hard_dependency_check(config_json):
Copy link
Contributor

@qiluo-msft qiluo-msft Feb 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

yang_hard_dependency_check

Just carious if there is a generic solution which using yang-model to parse the ConfigDB, and detect the missing dependency issues only. #Pending

@wen587 wen587 changed the title [config] Add YANG hard dependency check [config] Add Table hard dependency check Feb 18, 2024
@wen587 wen587 marked this pull request as ready for review February 18, 2024 11:13
qiluo-msft
qiluo-msft previously approved these changes Feb 21, 2024
View reviewed changes
@qiluo-msft
Copy link
Contributor

@liuh-80 Could you help review?

@xumia
Copy link
Collaborator

xumia commented Feb 22, 2024

/azp run Azure.sonic-utilities

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

liuh-80
liuh-80 reviewed Feb 23, 2024
View reviewed changes
config/main.py Outdated Show resolved Hide resolved
@wen587 wen587 requested review from qiluo-msft and liuh-80 February 28, 2024 12:02
@qiluo-msft qiluo-msft merged commit 9aa9eaa into sonic-net:master Mar 7, 2024
5 checks passed
@StormLiangMS
Copy link
Contributor

hi @wen587 could you run test with 202305 with this PR?

wen587 added a commit to wen587/sonic-utilities that referenced this pull request Mar 12, 2024
@wen587
[config] Add Table hard dependency check (sonic-net#3159)
df68884 
ADO: 26732148 
#### What I did
Add YANG hard depdency check for AAA and TACPLUS table
#### How I did it
Add a special check
#### How to verify it
Unit test
@wen587
Copy link
Contributor Author

wen587 commented Mar 12, 2024

Hi @StormLiangMS , please see below 202305 test.

tests/config_override_test.py::TestConfigOverride::test_broken_json PASSED [ 7%]
tests/config_override_test.py::TestConfigOverride::test_json_is_not_dict PASSED [ 7%]
tests/config_override_test.py::TestConfigOverride::test_dry_run PASSED [ 7%]
tests/config_override_test.py::TestConfigOverride::test_golden_config_db_empty PASSED [ 7%]
tests/config_override_test.py::TestConfigOverride::test_golden_config_db_partial PASSED [ 7%]
tests/config_override_test.py::TestConfigOverride::test_golden_config_db_new_feature PASSED [ 8%]
tests/config_override_test.py::TestConfigOverride::test_golden_config_db_full PASSED [ 8%]
tests/config_override_test.py::TestConfigOverride::test_golden_config_db_port_config PASSED [ 8%]
tests/config_override_test.py::TestConfigOverride::test_golden_config_db_empty_table_removal PASSED [ 8%]
tests/config_override_test.py::TestConfigOverride::test_aaa_yang_hard_depdency_check_failure PASSED [ 8%]
tests/config_override_test.py::TestConfigOverride::test_yang_verification_enabled PASSED [ 8%]
tests/config_override_test.py::TestConfigOverride::test_running_config_yang_failure PASSED [ 8%]
tests/config_override_test.py::TestConfigOverride::test_golden_input_yang_failure PASSED [ 8%]
tests/config_override_test.py::TestConfigOverride::test_final_config_yang_failure PASSED [ 8%]
tests/config_override_test.py::TestConfigOverrideMultiasic::test_macsec_override PASSED [ 8%]
tests/config_override_test.py::TestConfigOverrideMultiasic::test_device_metadata_table_rm PASSED [ 8%]
tests/config_override_test.py::TestConfigOverrideMultiasic::test_device_metadata_keep_sysinfo PASSED [ 8%]
tests/config_override_test.py::TestConfigOverrideMultiasic::test_device_metadata_gen_sysinfo PASSED [ 8%]
tests/config_override_test.py::TestConfigOverrideMultiasic::test_masic_missig_localhost_override PASSED [ 8%]
tests/config_override_test.py::TestConfigOverrideMultiasic::test_masic_missig_asic_override PASSED [ 8%]

mssonicbld pushed a commit to mssonicbld/sonic-utilities that referenced this pull request Mar 12, 2024
@wen587 @mssonicbld
[config] Add Table hard dependency check (sonic-net#3159)
67a4b16 
ADO: 26732148 
#### What I did
Add YANG hard depdency check for AAA and TACPLUS table
#### How I did it
Add a special check
#### How to verify it
Unit test
@mssonicbld mssonicbld mentioned this pull request Mar 12, 2024
Merged
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202305: #3210

liushilongbuaa pushed a commit to mssonicbld/sonic-utilities that referenced this pull request Mar 13, 2024
@wen587 @liushilongbuaa
[config] Add Table hard dependency check (sonic-net#3159)
4fb8ea6 
ADO: 26732148 
#### What I did
Add YANG hard depdency check for AAA and TACPLUS table
#### How I did it
Add a special check
#### How to verify it
Unit test
liushilongbuaa pushed a commit to mssonicbld/sonic-utilities that referenced this pull request Mar 13, 2024
@wen587 @liushilongbuaa
[config] Add Table hard dependency check (sonic-net#3159)
3ef4290 
ADO: 26732148 
#### What I did
Add YANG hard depdency check for AAA and TACPLUS table
#### How I did it
Add a special check
#### How to verify it
Unit test
mssonicbld pushed a commit that referenced this pull request Mar 13, 2024
@wen587 @mssonicbld
[config] Add Table hard dependency check (#3159)
45c4409 
ADO: 26732148 
#### What I did
Add YANG hard depdency check for AAA and TACPLUS table
#### How I did it
Add a special check
#### How to verify it
Unit test
mssonicbld pushed a commit to mssonicbld/sonic-utilities that referenced this pull request Mar 21, 2024
@wen587 @mssonicbld
[config] Add Table hard dependency check (sonic-net#3159)
a30ef25 
ADO: 26732148 
#### What I did
Add YANG hard depdency check for AAA and TACPLUS table
#### How I did it
Add a special check
#### How to verify it
Unit test
@mssonicbld mssonicbld mentioned this pull request Mar 21, 2024
Merged
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202311: #3225

mssonicbld pushed a commit that referenced this pull request Mar 21, 2024
@wen587 @mssonicbld
[config] Add Table hard dependency check (#3159)
fb4a090 
ADO: 26732148 
#### What I did
Add YANG hard depdency check for AAA and TACPLUS table
#### How I did it
Add a special check
#### How to verify it
Unit test
qiluo-msft
qiluo-msft reviewed May 3, 2024
View reviewed changes
config/main.py
AAA_TABLE = config_json.get("AAA", {})
TACPLUS_TABLE = config_json.get("TACPLUS", {})

aaa_authentication_login = AAA_TABLE.get("authentication", {}).get("login", "")
Copy link
Contributor

Choose a reason for hiding this comment

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

Please check authorization field.

Copy link
Contributor

Choose a reason for hiding this comment

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

Offline discussed, the issue is that the checking logic here is not consistent with yang models. We need to fix either to make them aligned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qiluo-msft qiluo-msft qiluo-msft approved these changes

@liuh-80 liuh-80 liuh-80 approved these changes

Assignees
No one assigned
Labels
Approved for 202305 Branch Approved for 202311 Branch Included in 202305 Branch Included in 202311 Branch Request for 202205 Branch Request for 202305 Branch Request for 202311 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@wen587 @qiluo-msft @xumia @StormLiangMS @mssonicbld @liuh-80 @yxieca @liushilongbuaa