Skip to content

[logAnalyzer] Avoid logAnalyzer logs being overwritten#822

Merged
liat-grozovik merged 1 commit intosonic-net:masterfrom
wangxin:log-analyzer-pr
Mar 13, 2019
Merged

[logAnalyzer] Avoid logAnalyzer logs being overwritten#822
liat-grozovik merged 1 commit intosonic-net:masterfrom
wangxin:log-analyzer-pr

Conversation

@wangxin
Copy link
Collaborator

@wangxin wangxin commented Mar 8, 2019

Description of PR

Summary:
Fixes # (issue)
In logAnalyzer related scripts, ansible_date_time is used for generating
testname_unique which will be used as folder name for storing
logAnalyzer results. The drawback is that ansible_date_time is fixed
in single ansible playbook execution. If logAnalyzer is called
multiple times, the earlier results could be overwritten by the latest
results.

The fix is to replace ansible_date_time with:
{{lookup('pipe','date +%Y-%m-%d-%H:%M:%S')}}

There are two differences:

  1. Each time the lookup plugin is called, the returned date and time
    would be different (interval of two calls longer than 1 second)
  2. The lookup plugin gets date and time of the sonic-mgmt container,
    not date and time of DUT.

After this change, filename of logAnalyzer result files would be almost the same. The only tiny difference is the separator char in timestamp. An example filename:
summary.loganalysis.functional_test.2019-03-08-10:30:18.log

Usually sonic-mgmt container and DUT are synched to same NTP server.
The timestamps should still be useful for debugging.

Type of change

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

Approach

How did you do it?

Replaced occurance of ansible_date_time with "{{lookup('pipe','date +%Y-%m-%d-%H:%M:%S')}}"

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

In logAnalyzer related scripts, ansible_date_time is used for generating
testname_unique which will be used as folder name for storing
logAnalyzer results. The drawback is that ansible_date_time is fixed
in single ansible playbook execution. If logAnalyzer is called
multiple times, the earlier results could be overwritten by the latest
results.

The fix is to replace ansible_date_time with:
{{lookup('pipe','date +%Y-%m-%d-%H:%M:%S')}}

There are two differences:
1. Each time the lookup plugin is called, the returned date and time
   would be different (interval of two calls longer than 1 second)
2. The lookup plugin gets date and time of the sonic-mgmt container,
   not date and time of DUT.

Signed-off-by: Xin Wang <[email protected]>
@liat-grozovik liat-grozovik merged commit e1ce368 into sonic-net:master Mar 13, 2019
@wangxin wangxin deleted the log-analyzer-pr branch March 18, 2019 05:21
yxieca pushed a commit that referenced this pull request Mar 18, 2019
In logAnalyzer related scripts, ansible_date_time is used for generating
testname_unique which will be used as folder name for storing
logAnalyzer results. The drawback is that ansible_date_time is fixed
in single ansible playbook execution. If logAnalyzer is called
multiple times, the earlier results could be overwritten by the latest
results.

The fix is to replace ansible_date_time with:
{{lookup('pipe','date +%Y-%m-%d-%H:%M:%S')}}

There are two differences:
1. Each time the lookup plugin is called, the returned date and time
   would be different (interval of two calls longer than 1 second)
2. The lookup plugin gets date and time of the sonic-mgmt container,
   not date and time of DUT.

Signed-off-by: Xin Wang <[email protected]>
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]>
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]>
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.

4 participants