Skip to content
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.

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

[multi-asic]: Update reload of systemd services to support multi-asic platforms #856

Merged

Conversation

SuvarnaMeenakshi
Copy link
Contributor

- What I did
For multi-asic platform, there can be multiple instances of some systemd services. currently config reload CLI stops and restarts only single instance of systemd service.
Modified stop/reset failed-status and restart system services function to perform action on multiple instances of systemd services.

- How I did it

  1. Read plaform/asic.conf file to get the number of ASICs for a platform.
  2. Get the services which are multiple instances from generated_services.conf
  3. For single ASIC platform, continue to stop and restart services as it is done today.
  4. For multiple ASIC platform, stop and restart each instance for multi-instance service. stop and restart of single instance will be done the same way for single and multi-asic platforms.

- How to verify it
Bring up a single ASIC VS image.
Load config_db.json/minigraph.xml
Update config_db.json or load a new config_db.json.
Do a "config reload".
Verify that all required services are stopped and restarted.
Verify all services are up.
Verify if the configuration/interfaces of previous configuration is completely removed and new configuration is applied.

- Previous command output (if the output of a command-line utility has changed)

- New command output (if the output of a command-line utility has changed)

config/main.py Outdated
except SystemExit as e:
log_error("Restart {} failed with error {}".format(service, e))
raise
if ((service + '.service' in generated_services_list) or
Copy link
Contributor

Choose a reason for hiding this comment

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

duplicated code as above. please use function to reduce the duplicated code.

Copy link
Contributor

Choose a reason for hiding this comment

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

looks like the code is duplicated three times

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated code as per comment.

jleveque
jleveque previously approved these changes Mar 24, 2020
config/main.py Outdated
except SystemExit as e:
log_error("Stopping {} failed with error {}".format(service, e))
raise
execute_systemctl(services_to_stop, SYSTEMCTL_STOP)
Copy link
Contributor

Choose a reason for hiding this comment

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

please check the return results and act accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

execute_systemctl() - this function prints error message or handles exception itself, should we further get return value and act?

config/main.py Outdated Show resolved Hide resolved
config/main.py Outdated Show resolved Hide resolved
config/main.py Outdated Show resolved Hide resolved
Copy link
Contributor

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

as comments

@jleveque jleveque dismissed their stale review March 24, 2020 20:13

Accidental approval.

config/main.py Outdated Show resolved Hide resolved
config/main.py Outdated Show resolved Hide resolved
config/main.py Outdated Show resolved Hide resolved
config/main.py Outdated Show resolved Hide resolved
lguohan
lguohan previously approved these changes Mar 25, 2020
judyjoseph
judyjoseph previously approved these changes Mar 25, 2020
@SuvarnaMeenakshi SuvarnaMeenakshi merged commit 798ce2f into sonic-net:master Mar 25, 2020
@SuvarnaMeenakshi SuvarnaMeenakshi deleted the support_multi_asic branch March 25, 2020 19:28
rlhui pushed a commit that referenced this pull request Mar 30, 2020
… platforms (#856)

* Update stop, reset failed status and restart of systemd services
to support multi-asic platforms.

* Create function to avoid code duplication.

* Fixed errors due to pervious commit and review comments.

* Minor update to fix spacing.

* Minor update to fix spacing.

* Minor update to fix spacing.

* For multi asic platform updated logic of stopping/restarting of
services to ensure that the right instances are stopped and
restarted if a service is both global and multi-instance.

* Fixed log error message with incorrect number of parameterts.
abdosi pushed a commit to abdosi/sonic-utilities that referenced this pull request Aug 4, 2020
… platforms (sonic-net#856)

* Update stop, reset failed status and restart of systemd services
to support multi-asic platforms.

* Create function to avoid code duplication.

* Fixed errors due to pervious commit and review comments.

* Minor update to fix spacing.

* Minor update to fix spacing.

* Minor update to fix spacing.

* For multi asic platform updated logic of stopping/restarting of
services to ensure that the right instances are stopped and
restarted if a service is both global and multi-instance.

* Fixed log error message with incorrect number of parameterts.
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.

6 participants