Skip to content

Update homebrew ansible modules to use log rotation#4446

Merged
yutongzhang-microsoft merged 5 commits intosonic-net:masterfrom
yutongzhang-microsoft:master
Oct 13, 2021
Merged

Update homebrew ansible modules to use log rotation#4446
yutongzhang-microsoft merged 5 commits intosonic-net:masterfrom
yutongzhang-microsoft:master

Conversation

@yutongzhang-microsoft
Copy link
Copy Markdown
Contributor

Description of PR

Some homebrew ansible modules may generate log files to /tmp folder on the host that it was executed. These modules manage the log files by their own and do not support log rotation. A new log rotation capable module utility was added in #4302

Update below ansible modules to use the new tool for logging:
ansible/roles/vm_set/library/sonic_kickstart.py
ansible/roles/vm_set/library/ceos_network.py
ansible/roles/vm_set/library/kickstart.py
ansible/roles/vm_set/library/vm_topology.py

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?

Use the new log rotation capable module utility which was added in #4302 to support log rotation.

How did you do it?

I import the new log rotation capable module utility at the front of each file and use it to support log rotation. Meanwhile, I delete the previous functions which were related to the log.

How did you verify/test it?

I use ./testbed-cli.sh add-topo, deploy-mg and remove-topo to test it.

Any platform specific information?

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

Documentation

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Oct 9, 2021

This pull request introduces 8 alerts when merging 2b82690 into c517cb9 - view on LGTM.com

new alerts:

  • 3 for Clear-text logging of sensitive information
  • 2 for Unused local variable
  • 2 for Unused import
  • 1 for Module is imported more than once

@wangxin
Copy link
Copy Markdown
Collaborator

wangxin commented Oct 9, 2021

Can you address the LGTM alerts?

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Oct 9, 2021

This pull request introduces 5 alerts when merging 4dae88c into 6c0a8c2 - view on LGTM.com

new alerts:

  • 3 for Clear-text logging of sensitive information
  • 2 for Unused import

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Oct 9, 2021

This pull request introduces 3 alerts when merging f447ec3 into 6c0a8c2 - view on LGTM.com

new alerts:

  • 3 for Clear-text logging of sensitive information

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Oct 12, 2021

This pull request introduces 3 alerts when merging 5fe3d73 into 33d44e6 - view on LGTM.com

new alerts:

  • 3 for Clear-text logging of sensitive information

@yutongzhang-microsoft yutongzhang-microsoft merged commit be27c6a into sonic-net:master Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants