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

Don't throw exception when getting supervisord service status #2681 #2682

Conversation

FroggyFlox
Copy link
Member

Fixes #2681
@phillxnet, @Hooverdan96: ready for review.

We currently throw an Exception when checking the status of a surpervisor-controlled service such as replication. This leads to an exception being thrown even when expected (service is off), which can itself interrupt other parts such as our get_services websocket.

This pull-request adds a new arg to superctl() allowing for the possibility to not throw an exception. Note that run_command()'s default for throw is True, so we respect this here.

Functional testing

The Samba service was toggled ON.
Within 15 sec (websocket "refresh" interval), the status of all services was refreshed and the toggle button was re-aligned with the others, and stayed ON.
The Samba service was then toggled back OFF and the opposite was observed: button switched to OFF and was re-aligned to the other buttons within 15 secs.

Unit testing

All tests still pass. Note that this was tested on Leap 15.4 only:

buildvm:/opt/rockstor # cd src/rockstor/ && poetry run django-admin test ; cd -
Creating test database for alias 'default'...
Creating test database for alias 'smart_manager'...
System check identified no issues (0 silenced).
......................................................................................................................................................................................................................................................................
----------------------------------------------------------------------
Ran 262 tests in 15.827s

OK
Destroying test database for alias 'default'...
Destroying test database for alias 'smart_manager'...

…r#2681

We currently throw an Exception when checking the status of a
surpervisor-controlled service such as replication. This leads to an
exception being thrown even when expected (service is off), which can
itself interrupt other parts such as our get_services websocket.

This commit adds a new arg to the superctl() function to allow for the
possibility to not throw an exception. Note that run_command's default
for throw is True, so respect this here.
@FroggyFlox FroggyFlox linked an issue Sep 30, 2023 that may be closed by this pull request
Copy link
Member

@phillxnet phillxnet left a comment

Choose a reason for hiding this comment

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

@FroggyFlox This is a nice find. Thanks.
As discussed we need to move all our stuff to systemd only, rather than our current service manger inside another arrangement.
Re: "Replace supervisord with further systemd intergration" #2650
and your follow-up of:
"Avoid system calls to interact with systemd services" #2680
But all in good time. And my apologies for missing this earlier in our more recent refactoring re our move to Poetry.
I't been bugging me for a while now that we kept seeing that exception; I just hadn't realised the ramifications.

And this is a nice fix for what we have currently.

Cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Services] Websocket failure to update service status
2 participants