Skip to content

Skip test_syslog_config_work_after_reboot#16416

Merged
StormLiangMS merged 1 commit intosonic-net:masterfrom
JibinBao:skip_ssip_reboot_case
Feb 14, 2025
Merged

Skip test_syslog_config_work_after_reboot#16416
StormLiangMS merged 1 commit intosonic-net:masterfrom
JibinBao:skip_ssip_reboot_case

Conversation

@JibinBao
Copy link
Contributor

@JibinBao JibinBao commented Jan 9, 2025

Description of PR

Skip test_syslog_config_work_after_reboot due to the bug sonic-net/sonic-buildimage#21201

Summary:
Fixes # (issue)

Type of change

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

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405
  • 202411

Approach

What is the motivation for this PR?

Skip test_syslog_config_work_after_reboot

How did you do it?

when dut_mgmt_network is a sub_network of forced_mgmt_routes, skip it

How did you verify/test it?

run test_syslog_config_work_after_reboot

Any platform specific information?

no

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

Documentation

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JibinBao JibinBao changed the title Skip test_syslog_config_work_after_reboot due to bug Skip test_syslog_config_work_after_reboot Jan 9, 2025
@ZhaohuiS
Copy link
Contributor

@JibinBao it's not a good way to skip test case like this.
You can use a github issue to skip the case, if the issue is closed, the case can continue to run.

syslog/test_syslog_source_ip.py::TestSSIP::test_syslog_config_work_after_reboot:
  skip:
    reason: "Testcase consistent failed, raised issue to track / Vs setup doesn't work when creating mgmt vrf"
    conditions_logical_operator: or
    conditions:
      - "https://github.com/sonic-net/sonic-mgmt/issues/16941"
      - "asic_type in ['vs']"

@JibinBao
Copy link
Contributor Author

@JibinBao it's not a good way to skip test case like this. You can use a github issue to skip the case, if the issue is closed, the case can continue to run.

syslog/test_syslog_source_ip.py::TestSSIP::test_syslog_config_work_after_reboot:
  skip:
    reason: "Testcase consistent failed, raised issue to track / Vs setup doesn't work when creating mgmt vrf"
    conditions_logical_operator: or
    conditions:
      - "https://github.com/sonic-net/sonic-mgmt/issues/16941"
      - "asic_type in ['vs']"

Hi @ZhaohuiS if we skip the test by a github issue , we will lose the test coverage, because only the setup with a special network has the problem.

@ZhaohuiS
Copy link
Contributor

ZhaohuiS commented Feb 13, 2025

@JibinBao in your change, if mgmt ip is in the subnet of one of forced_mgmt_routes, test_syslog_config_work_after_reboot will be skipped, for most of cases, it does hit this scenario, in my point, this case will be skipped eventually, what the difference between the skip in conditional mark and yours? And what test coverage will be lost?
On the second point, it's easy for us to forget remove the skip condition for this case, especially when this issue could not be fixed for long time.
But if we use a bug to enable the skip condition, we have regular meetings to go through those open issues, once it's closed, the case can be ran automatically, we don't have to submit a separated PR to remove your code to archive the same purpose.

What do you think about it?

@JibinBao
Copy link
Contributor Author

@JibinBao in your change, if mgmt ip is in the subnet of one of forced_mgmt_routes, test_syslog_config_work_after_reboot will be skipped, for most of cases, it does hit this scenario, in my point, this case will be skipped eventually, what the difference between the skip in conditional mark and yours? And what test coverage will be lost? On the second point, it's easy for us to forget remove the skip condition for this case, especially when this issue could not be fixed for long time. But if we use a bug to enable the skip condition, we have regular meetings to go through those open issues, once it's closed, the case can be ran automatically, we don't have to submit a separated PR to remove your code to archive the same purpose.

What do you think about it?

Hi @ZhaohuiS , I get your concern. 1. In our environment, we hit this scenario rarely,because we just server setups have the speical network config. So, we want to keep running the test case on the left setups.
2. If we skip it by github issue, we are worried that the issue of sonic-net/sonic-buildimage#21201 cannot be resolved quickly or set to not fix,so we will not run the case for a long time or forever.

Copy link
Collaborator

@StormLiangMS StormLiangMS left a comment

Choose a reason for hiding this comment

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

LGTM

@StormLiangMS StormLiangMS merged commit ee5388f into sonic-net:master Feb 14, 2025
mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Feb 14, 2025
…buildimage#21201 (sonic-net#16416)

What is the motivation for this PR?
Skip test_syslog_config_work_after_reboot

How did you do it?
when dut_mgmt_network is a sub_network of forced_mgmt_routes, skip it

How did you verify/test it?
run test_syslog_config_work_after_reboot

Any platform specific information?
no
mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Feb 14, 2025
…buildimage#21201 (sonic-net#16416)

What is the motivation for this PR?
Skip test_syslog_config_work_after_reboot

How did you do it?
when dut_mgmt_network is a sub_network of forced_mgmt_routes, skip it

How did you verify/test it?
run test_syslog_config_work_after_reboot

Any platform specific information?
no
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202405: #16966

@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202411: #16967

mssonicbld pushed a commit that referenced this pull request Feb 14, 2025
…buildimage#21201 (#16416)

What is the motivation for this PR?
Skip test_syslog_config_work_after_reboot

How did you do it?
when dut_mgmt_network is a sub_network of forced_mgmt_routes, skip it

How did you verify/test it?
run test_syslog_config_work_after_reboot

Any platform specific information?
no
mssonicbld pushed a commit that referenced this pull request Feb 17, 2025
…buildimage#21201 (#16416)

What is the motivation for this PR?
Skip test_syslog_config_work_after_reboot

How did you do it?
when dut_mgmt_network is a sub_network of forced_mgmt_routes, skip it

How did you verify/test it?
run test_syslog_config_work_after_reboot

Any platform specific information?
no
wangxin pushed a commit to wangxin/sonic-mgmt that referenced this pull request Feb 21, 2025
…buildimage#21201 (sonic-net#16416)

What is the motivation for this PR?
Skip test_syslog_config_work_after_reboot

How did you do it?
when dut_mgmt_network is a sub_network of forced_mgmt_routes, skip it

How did you verify/test it?
run test_syslog_config_work_after_reboot

Any platform specific information?
no
nnelluri-cisco pushed a commit to nnelluri-cisco/sonic-mgmt that referenced this pull request Mar 15, 2025
…buildimage#21201 (sonic-net#16416)

What is the motivation for this PR?
Skip test_syslog_config_work_after_reboot

How did you do it?
when dut_mgmt_network is a sub_network of forced_mgmt_routes, skip it

How did you verify/test it?
run test_syslog_config_work_after_reboot

Any platform specific information?
no
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants