Skip to content

[logAnalyzer] Fix the issue of unmatched start and end mark#831

Merged
yxieca merged 1 commit intosonic-net:masterfrom
wangxin:la-fix-pr
Mar 20, 2019
Merged

[logAnalyzer] Fix the issue of unmatched start and end mark#831
yxieca merged 1 commit intosonic-net:masterfrom
wangxin:la-fix-pr

Conversation

@wangxin
Copy link
Collaborator

@wangxin wangxin commented Mar 19, 2019

Description of PR

Summary:
Fixes # (issue)
Previous PR #822 broke the log analyzer. This commit is to fix the issue
introduced by PR #822.

PR #822 is to replace the method of getting timestamp from
ansible_date_time to the pipe plugin. Log analyzer needs a variable
testname_unique to have timestamp in its value.

Script run_command_with_log_analyzer.yml uses include_vars to include
variables defined in vars/run_config_test_vars.yml. Ansible includes
variables dynamically when 'include_vars' is used. Consequence is that
the testname_unique variable defined in the vars/run_config_test_vars.yml
is re-evaluated when it is referenced. This caused log analyzer to use
inconsistent start and end mark.

To fix this issue, a unique timestamp is always generated before
include_vars. Then the timestamp is feed to the testname_unique
defined in the vars file. This approach can guarantee consistent
run id for each log analyzer execution. And the run id would also
be different for different log analyzer execution.

The other scripts changed in this commit have the similar issue.

Type of change

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

Approach

How did you do it?

Ansible include_vars is dynamic. To avoid the pipe for variable testname_unique in vars file being repeatedly evaluated, a unique_timestamp variable is initialized before include_vars. And in vars file, replace the pipe plugin with the variable unique_timestamp just initialized.

How did you verify/test it?

Tested on Mellanox platform.

Any platform specific information?

No

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

Documentation

Previous PR #822 broke the log analyzer. This commit is to fix the issue
introduced by PR #822.

PR #822 is to replace the method of getting timestamp from
ansible_date_time to the pipe plugin. Log analyzer needs a variable
testname_unique to have timestamp in its value.

Script run_command_with_log_analyzer.yml uses include_vars to include
variables defined in vars/run_config_test_vars.yml. Ansible includes
variables dynamically when 'include_vars' is used. Consequence is that
the testname_unique variable defined in the vars/run_config_test_vars.yml
is re-evaluated when it is referenced. This caused log analyzer to use
inconsistent start and end mark.

To fix this issue, a unique timestamp is always generated before
include_vars. Then the timestamp is feed to the testname_unique
defined in the vars file. This approach can guarantee consistent
run id for each log analyzer execution. And the run id would also
be different for different log analyzer execution.

The other scripts changed in this commit have the similar issue.

Signed-off-by: Xin Wang <[email protected]>
@yxieca yxieca merged commit 75c1fe0 into sonic-net:master Mar 20, 2019
yxieca pushed a commit that referenced this pull request Mar 20, 2019
Previous PR #822 broke the log analyzer. This commit is to fix the issue
introduced by PR #822.

PR #822 is to replace the method of getting timestamp from
ansible_date_time to the pipe plugin. Log analyzer needs a variable
testname_unique to have timestamp in its value.

Script run_command_with_log_analyzer.yml uses include_vars to include
variables defined in vars/run_config_test_vars.yml. Ansible includes
variables dynamically when 'include_vars' is used. Consequence is that
the testname_unique variable defined in the vars/run_config_test_vars.yml
is re-evaluated when it is referenced. This caused log analyzer to use
inconsistent start and end mark.

To fix this issue, a unique timestamp is always generated before
include_vars. Then the timestamp is feed to the testname_unique
defined in the vars file. This approach can guarantee consistent
run id for each log analyzer execution. And the run id would also
be different for different log analyzer execution.

The other scripts changed in this commit have the similar issue.

Signed-off-by: Xin Wang <[email protected]>
lguohan pushed a commit that referenced this pull request Mar 28, 2019
…cted (#844)

PR #831 does not fully fix the issue introduced by PR #822. Ansible's
include_vars module could not override variable value previous defined
by set_fact. Variables in vars/run_config_test_vars.yml may still have
old value.

The change is to avoid using include_vars. The variables defined in
run_config_test_vars.yml are moved into script
run_command_with_log_analyzer.yml. The vars files are deleted.

The same change is made to other scripts using the same pattern.

Signed-off-by: Xin Wang <[email protected]>
yxieca pushed a commit that referenced this pull request Mar 28, 2019
…cted (#844)

PR #831 does not fully fix the issue introduced by PR #822. Ansible's
include_vars module could not override variable value previous defined
by set_fact. Variables in vars/run_config_test_vars.yml may still have
old value.

The change is to avoid using include_vars. The variables defined in
run_config_test_vars.yml are moved into script
run_command_with_log_analyzer.yml. The vars files are deleted.

The same change is made to other scripts using the same pattern.

Signed-off-by: Xin Wang <[email protected]>
@wangxin wangxin deleted the la-fix-pr branch May 24, 2019 03:32
deerao02 pushed a commit to deerao02/sonic-mgmt that referenced this pull request Dec 18, 2025
<!--
Please make sure you've read and understood our contributing guidelines;
https://github.com/sonic-net/SONiC/blob/gh-pages/CONTRIBUTING.md

Please provide following information to help code review process a bit easier:
-->
### Description of PR
<!--
- Please include a summary of the change and which issue is fixed.
- Please also include relevant motivation and context. Where should reviewer start? background context?
- List any dependencies that are required for this change.
-->

Summary:
Fixes # (issue)

### Type of change

<!--
- Fill x for your type of change.
- e.g.
- [x] Bug fix
-->

- [x] Bug fix
- [ ] Testbed and Framework(new/improvement)
- [ ] New Test case
 - [ ] Skipped for non-supported platforms
- [x] Test case improvement

### Back port request
- [ ] 202205
- [ ] 202305
- [ ] 202311
- [ ] 202405
- [x] 202411
- [x] 202505

### Approach
#### What is the motivation for this PR?
Enable vlan_ping test to work with ipv6-only testbed.

#### How did you do it?
Added ability to deal with ipv6-only within existing test.

#### How did you verify/test it?
Ran test on ipv6-only testbed.
<img width="1938" height="522" alt="image" src="https://github.com/user-attachments/assets/7c2318e9-c616-409b-a7e1-5bac169c3ae9" />

#### Any platform specific information?

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

### Documentation
<!--
(If it's a new feature, new test case)
Did you update documentation/Wiki relevant to your implementation?
Link to the wiki page?
-->
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.

2 participants