[PATCH] tbot_contrib/utils.py: collect some usefull linux utils

[Harald Seiler]

Hello Heiko,

On Tue, 2020-04-21 at 06:15 +0200, Heiko Schocher wrote:
> Hello Harald,
> 
> Am 20.04.2020 um 17:41 schrieb Harald Seiler:
> > Hello Heiko,
> > 
> > thanks for the patch, this is quite useful!
> > 
> > On Mon, 2020-04-20 at 15:03 +0200, Heiko Schocher wrote:
> > > collect usefull linux utils functions. Start with
> > > 
> > > check_systemd_services_running(lnx: linux.LinuxShell, services: list)
> > 
> > Can we shorten the name a bit?  I'd suggest something like
> > `ensure_running` although feel free to use something else if you have
> > a better idea ...
> 
> Hmm.. hard to say. I think systemd should be in the name too.
> 
> ensure_sysd_service_run
> ensure_sysd_job_run
> check_sysd_job_run

systemd officially uses `sd` as the short form in their function names so
I think it'd make sense to do the same.  I dislike calling the function
`check` because it is doing more than checking whether the service is
running.  So I'd suggest

    ensure_sd_unit

meaning 'ensure that a systemd unit is active'. </bikeshed>

> > > check if all systemd services in list services run on linux
> > > machine lnx. If not, try to start them.
> > > 
> > > Signed-off-by: Heiko Schocher <hs at denx.de>
> > > ---
> > > 
> > >   tbot_contrib/utils.py | 31 +++++++++++++++++++++++++++++++
> > >   1 file changed, 31 insertions(+)
> > >   create mode 100644 tbot_contrib/utils.py
> > > 
> > > diff --git a/tbot_contrib/utils.py b/tbot_contrib/utils.py
> > > new file mode 100644
> > > index 0000000..8073f89
> > > --- /dev/null
> > > +++ b/tbot_contrib/utils.py
> > > @@ -0,0 +1,31 @@
> > > +# tbot, Embedded Automation Tool
> > > +# Copyright (C) 2020  Harald Seiler
> > 
> > I guess this shouldn't be me ;)
> 
> Heh, fast copy and paste ...
> 
> Hmm.. what about using SPDX License identifier ?
> 
> > > +#
> > > +# This program is free software: you can redistribute it and/or modify
> > > +# it under the terms of the GNU General Public License as published by
> > > +# the Free Software Foundation, either version 3 of the License, or
> > > +# (at your option) any later version.
> > > +#
> > > +# This program is distributed in the hope that it will be useful,
> > > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > +# GNU General Public License for more details.
> > > +#
> > > +# You should have received a copy of the GNU General Public License
> > > +# along with this program.  If not, see <https://www.gnu.org/licenses/>;;;;.
> > > +
> > > +from tbot.machine import linux
> > > +
> > > +
> > > +def check_systemd_services_running(lnx: linux.LinuxShell, services: list) -> None:
> > 
> > The correct type-annotation for services would be `typing.List[str]`.
> > You'll also need to import the typing module for that.
> 
> changed.
> 
> > > +    """
> > > +    check if all systemd services in list services run on linux machine lnx.
> > > +    If not, try to start them.
> > > +
> > > +    :param lnx: linux shell
> > > +    :param services: list of systemd services
> > > +    """
> > > +    for s in services:
> > > +        ret = lnx.exec("systemctl", "status", s, "--no-pager")
> > > +        if ret[0] != 0:
> > 
> > I think it is more robust to use `systemctl is-active` for this check.
> > Also, you can use `lnx.test()` which makes this a bit more concise:
> > 
> >      if lnx.test("systemctl", "is-active", s):
> > 
> 
> Thanks to Wolfgang for his reply, we found this ... I would say bug...
> yesterday, so "systemctl status" seems correct to me.
> 
> > > +            lnx.test("sudo", "systemctl", "start", s)
> > 
> > This one should be lnx.exec0().  test() will not raise an excepion in case
> > of an error so failure to start the unit would not be noticed.
> 
> Huh, than I msiunderstood you yesterday, fixed.
> 
> Thanks for the review. I send v2 if we solved the naming.
> 
> bye,
> Heiko

-- 
Harald