Skip to content

[config/main.py] don't start/stop pmon during config reload #3

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

Closed
stephenxs wants to merge 5 commits into from
Closed

[config/main.py] don't start/stop pmon during config reload #3

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2871dea
[config/main.py] don't start/stop pmon during config reload since pmo…
Sep 10, 2019
ee74d59
[config/main.py] fix unnecessary blank
Sep 11, 2019
a9f2a27
[config/main.py] define seperated array of services on mellanox and o…
Sep 11, 2019
03d4c1c
[config/main.py] remove parenthesis in if statement
Sep 12, 2019
b6bff7c
[config/main.py]adjust code
Sep 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions config/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,10 @@ def _stop_services():
'hostcfgd',
]

# on Mellanox platform pmon is stopped by syncd
if (version_info and version_info.get('asic_type') == 'mellanox'):
Copy link
Copy Markdown

@stepanblyschak stepanblyschak Sep 11, 2019

Choose a reason for hiding this comment

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

Parenthesis not required here (lets remove to be more python-way consistent).
Also, in which cases bool(version_info) == False ? Let's teat it as undefined platform error, otherwise we hide the problem and keep pmon in the list

Copy link
Copy Markdown

@stepanblyschak stepanblyschak Sep 11, 2019

Choose a reason for hiding this comment

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

At the beginning of the file lets initialize asic_type like this:

try:
    asic_type = version_info['asic_type']
except KeyError, TypeError:
    click.abort('undefined ASIC type')

Any subsequent code that uses asic_type knows that it is initialized correctly

Copy link
Copy Markdown
Owner Author

@stephenxs stephenxs Sep 12, 2019

Choose a reason for hiding this comment

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

fixed.

services_to_stop.remove('pmon')

for service in services_to_stop:
try:
click.echo("Stopping service {} ...".format(service))
Expand Down Expand Up @@ -349,6 +353,10 @@ def _restart_services():
'hostcfgd',
]

# on Mellanox platform pmon is started by syncd
if (version_info and version_info.get('asic_type') == 'mellanox'):
Copy link
Copy Markdown

@stepanblyschak stepanblyschak Sep 11, 2019

Choose a reason for hiding this comment

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

same as above comment

Copy link
Copy Markdown
Owner Author

@stephenxs stephenxs Sep 13, 2019

Choose a reason for hiding this comment

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

fixed

services_to_restart.remove('pmon')

for service in services_to_restart:
try:
click.echo("Restarting service {} ...".format(service))
Expand Down