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

[Heiko Schocher]

Hello Harald,

Am 22.04.2020 um 10:40 schrieb Harald Seiler:
> Hello Heiko,
> 
> On Wed, 2020-04-22 at 10:01 +0200, Heiko Schocher wrote:
>> Hello Harald,
>>
>> Am 22.04.2020 um 09:07 schrieb Harald Seiler:
>>> Hello Heiko,
>>>
>>> On Wed, 2020-04-22 at 07:01 +0200, Heiko Schocher wrote:
>>>> collect usefull linux utils functions. Start with
>>>>
>>>> ensure_sd_unit(lnx: linux.LinuxShell, services: typing.List[str])
>>>>
>>>> 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>
>>>> ---
>>>>
>>>> Changes in v3:
>>>> - fix commit message
>>>>     rename check_systemd_services_running() to ensure_sd_unit()
>>>>
>>>> Changes in v2:
>>>> - added comments from Harald
>>>>     - rename check_systemd_services_running() to ensure_sd_unit()
>>>>     - fix Copyright
>>>>     - correct type-annotation for services
>>>>     - use lnx.test()
>>>>     - use "is-active" instead "status" for getting state of service
>>>> - added a services cache, so we only check once per linux
>>>>     machine, if systemd services run.
>>>
>>> The cache is a great idea, thanks for adding that!  On that note, I've
>>> been playing with the idea of adding a new decorator for this kind of
>>> purpose.  It seems to happen quite often that some action needs to happen
>>> once for each machine it is called on.
>>
>> May a good idea, yes!
>>
>>>>    tbot_contrib/utils.py | 42 ++++++++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 42 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..ee91d00
>>>> --- /dev/null
>>>> +++ b/tbot_contrib/utils.py
>>>> @@ -0,0 +1,42 @@
>>>> +# tbot, Embedded Automation Tool
>>>> +# Copyright (C) 2020  Heiko Schocher
>>>> +#
>>>> +# 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
>>>> +import typing
>>>> +
>>>> +
>>>> +_SERVICES_CACHE: typing.Dict[str, typing.Dict[str, bool]] = {}
>>>> +
>>>> +
>>>> +def ensure_sd_unit(lnx: linux.LinuxShell, services: typing.List[str]) -> None:
>>>> +    """
>>>> +    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
>>>> +    """
>>>> +    if lnx.name not in _SERVICES_CACHE:
>>>> +        _SERVICES_CACHE[lnx.name] = {}
>>>
>>> Small nitpick:  Using the name as the key for your cache is not ideal
>>> because there is no guarantee that we won't see multiple machines with the
>>> same name.  Instead, you can use the machine itself as the key like I have
>>> done in check_for_tool [1].  This is possible because machines implement
>>> the hash() protocol (and cloned machines all yield the same hash).
>>
>> Hmm... I must try it, but I think, I did first:
>>
>> _SERVICES_CACHE[lnx] = {}
>>
>> and it did not worked ... Yes, just tried again:
> 
> Huh, this means the two machines which are created here are not proper
> clones of each other ... I'll have to take a closer look.  I guess this is
> an SSHConnector under the hood?

Yep!

> For now, I'd be fine with your version as is; this is not a big issue.
> I can adjust it later, once the other problem is fixed.

Ok, fine, thanks!

bye,
Heiko

> 
>> hs at lab-1:tbot-tbot2go  [wandboard-devel-messe] $ tbot @argswandboardlab1 interactive_uboot
>> tbot starting ...
>> ├─Flags:
>> │ 'lab-1-build'
>> ├─Calling interactive_uboot ...
>> │   ├─[local] ssh -o BatchMode=yes -i /home/hs/.ssh/id_rsa -p 22 hs at 192.168.1.109
>> │   ├─[lab1] systemctl is-active nfs-server.service --no-pager
>> │   │    ## active
>> │   ├─[lab1] systemctl is-active tftp.socket --no-pager
>> │   │    ## active
>> │   ├─[local] ssh -o BatchMode=yes -i /home/hs/.ssh/id_rsa -p 22 hs at 192.168.1.109
>> │   ├─[lab1] systemctl is-active nfs-server.service --no-pager
>> │   │    ## active
>> │   ├─[lab1] systemctl is-active tftp.socket --no-pager
>> │   │    ## active
>> │   ├─[lab1] kermit /home/hs/kermrc_wandboard
>>
>>>> +    for s in services:
>>>> +        if s in _SERVICES_CACHE[lnx.name]:
>>>> +            continue
>>>> +
>>>> +        if not lnx.test("systemctl", "is-active", s, "--no-pager"):
>>>
>>> You don't need `--no-pager` for is-active.
>>
>> Indeed.
>>
>>>> +            lnx.exec0("sudo", "systemctl", "start", s)
>>>> +
>>>> +        _SERVICES_CACHE[lnx.name][s] = True
>>>
>>> [1]: https://github.com/Rahix/tbot/blob/master/tbot/tc/shell.py#L173
>>>
>>
>> bye,
>> Heiko
> 
> Regards,
> 

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de