If link or service enablement did not change, should not result be "No Changes" for an operation rather than "Success?"

[bogen85]

Describe the bug

Maybe this the chroot connection, not sure. But facts do work... Anyways, If I use openrc to enable a service in a chroot install, it works (Success). However, if I rerun I don't get "No Changes" I get "Success". So the same it enables or does not already have to enable due already been enabled.

I tried a symlink instead of openrc service enable, as i the end the result is supposed to be the same.

Same behavior, upon making the symlink, "Success". But when rerunning, "Success" (not "No Changes").

I'm use to ansible for these two things. (service enabling and symlinks).

I can collect facts and check them, and only make symlink if it is needed. However, if I have to do this, how is it any better than just running low level "automation" scripts. This is on an initial run (no symlink exists)

--> Loading config...
--> Loading inventory...
    The @chroot connector is in beta!

--> Connecting to hosts...
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Connected

--> Preparing operations...
    Loading: ./initial_prep.py
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Ready: ./initial_prep.py

--> Proposed changes:
    Groups: @chroot
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs]   Operations: 22   Commands: 26   

--> Beginning operation run...
--> Starting operation: Apk/Update 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Apk/Upgrade (available=True)
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Default packages 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Add devfs to runlevel sysinit 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Add dmesg to runlevel sysinit 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Add hwdrivers to runlevel sysinit 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Add mdev to runlevel sysinit 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Add bootmisc to runlevel boot 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Add hostname to runlevel boot 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Add hwclock to runlevel boot 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Add modules to runlevel boot 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Add networking to runlevel boot 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Add swap to runlevel boot 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Add sysctl to runlevel boot 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Add syslog to runlevel boot 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Add urandom to runlevel boot 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Add acpid to runlevel default 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Add crond to runlevel default 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Add dropbear to runlevel default 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Add killprocs to runlevel shutdown 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Add mount-ro to runlevel shutdown 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Add savecache to runlevel shutdown 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Results:
    Groups: @chroot
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs]   Successful: 22   Errors: 0   Commands: 26/26   

If I run it again with no check, then each symlink operation gets still gets "Success" (and I don't know if it had to create the symlink, or if it already existed)

This is on a rerun (symlink already exists) if I check before making the symlink:

--> Loading config...
--> Loading inventory...
    The @chroot connector is in beta!

--> Connecting to hosts...
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Connected

--> Preparing operations...
    Loading: ./initial_prep.py
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Ready: ./initial_prep.py

--> Proposed changes:
    Groups: @chroot
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs]   Operations: 3   Commands: 2   

--> Beginning operation run...
--> Starting operation: Apk/Update 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Apk/Upgrade (available=True)
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] Success

--> Starting operation: Default packages 
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs] No changes

--> Results:
    Groups: @chroot
    [@chroot<redacted>alpine/alpine-miniroot-extend/rootfs]   Successful: 3   Errors: 0   Commands: 2/2

@chroot<redacted>alpine/alpine-miniroot-extend/rootfs
    service devfs already in runlevel sysinit
    service dmesg already in runlevel sysinit
    service hwdrivers already in runlevel sysinit
    service mdev already in runlevel sysinit
    service bootmisc already in runlevel boot
    service hostname already in runlevel boot
    service hwclock already in runlevel boot
    service modules already in runlevel boot
    service networking already in runlevel boot
    service swap already in runlevel boot
    service sysctl already in runlevel boot
    service syslog already in runlevel boot
    service urandom already in runlevel boot
    service acpid already in runlevel default
    service crond already in runlevel default
    service dropbear already in runlevel default
    service killprocs already in runlevel shutdown
    service mount-ro already in runlevel shutdown
    service savecache already in runlevel shutdown

To Reproduce

Here is the python deployment script with the fact checking logic in it.

# vim: set expandtab tabstop=4 shiftwidth=4 filetype=python3 :

import click
from pyinfra import host
from pyinfra.operations import apk, files
from pyinfra.facts.files import FindLinks, Link
from pyinfra.api import BaseStateCallback

BASE_PACKAGES="mlocate mkinitfs openrc busybox-initscripts rsync-openrc dropbear rsync micro htop"

SERVICES = {
    "sysinit": "devfs dmesg hwdrivers mdev",
    "boot": "bootmisc hostname hwclock modules networking swap sysctl syslog urandom",
    "default": "acpid crond dropbear",
    "shutdown": "killprocs mount-ro savecache",
}

class Session(BaseStateCallback):
    def __init__(self):
        self.have = [host.name]
    def host_disconnect(self, state, host):
        if len(self.have) > 1:
            click.echo(click.style(f'\n{self.have[0]}\n', fg='white') + click.style('\n'.join(self.have[1:]), fg='blue'))
    def append(self, message):
        self.have.append(message)

def main():
    session = Session()
    host.state.add_callback_handler(session)

    apk.update()
    apk.upgrade(available=True)

    apk.packages(name='Default packages', packages=BASE_PACKAGES.split())

    for runlevel in SERVICES.keys():
        links = [x.strip() for x in host.get_fact(FindLinks, f'/etc/runlevels/{runlevel}', quote_path=False)]
        for service in SERVICES[runlevel].split():
            path, target = f'/etc/runlevels/{runlevel}/{service}', f'/etc/init.d/{service}'
            if (path in links) and (host.get_fact(Link, path)["link_target"].strip().removesuffix("'") == target):
                session.append(f'\tservice {service} already in runlevel {runlevel}')
            else:
                files.link(name = f'Add {service} to runlevel {runlevel}', path = path, target = target, force=True)

main()

Expected behavior

I should not have to put in a check for an operation. If I enable a service and rerun, it should be "no changes" if no steps are needed to get result. If I create a symlink and rerun, it should be "no changes" if no steps are needed to get result.

Is this valid expected behavior? Am I doing something wrong so I'm not getting "no changes" where applicable?

Meta

• Include output of pyinfra --support. --> Support information:

  If you are having issues with pyinfra or wish to make feature requests, please check out the GitHub issues at https://github.com/Fizzadar/pyinfra/issues . When adding an issue, be sure to include the following:

  System: Linux Platform: Linux-5.15.11-arch2-1-x86_64-with-glibc2.33 Release: 5.15.11-arch2-1 Machine: x86_64 pyinfra: v1.5 Executable: ./pyvenv/bin/pyinfra Python: 3.10.1 (CPython, GCC 11.1.0)

• How was pyinfra installed (source/pip)? Pip in a virtual env

Comments

I like what I see so far of pyinfra, but... It needs a lot of work to replace my use of ansible with it. I like it no python is needed on the target hosts. I like it that the "playbooks" are python source rather than a DSL in yaml.. However, unless I'm doing something wrong in regard to "no changes" vs "success" it is going to be a lot of work to bring pyinfra up to par. (correcting behavior for openrc service enablement and file symlink creation?)

[bogen85]

Here is a more exhaustive/comprehensive repro steps.

Needs to be run on a systemd Linux distro with host python/pip/venv installed.

User needs sudo.

Put these 4 files in your directory and type make basic-prep after renaming 3 of the files.

requirements.txt Makefile.txt initial_prep.py.txt chroot.py.txt

Rename 3 of the files

mv Makefile.txt Makefile
mv chroot.py.txt chroot.py
mv initial_prep.py.txt initial_prep.py
make basic-prep
# changes have now been made
# make basic-prep again...
make basic-prep

NOTE: Of course review what will happen first on your system, as sudo is in the mix...

Update: I updated the chroot wrapper (what is above works) chroot.py

#!/usr/bin/python3
# vim: set expandtab tabstop=4 shiftwidth=4 filetype=python3 :

import os, sys

PATH = '/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin'
SHELL = '/bin/sh'
cmd = [
        'sudo', 'systemd-nspawn', '--quiet', f'--setenv=PATH={PATH}',
        '--setenv=SHELL={SHELL}', '--directory',
    ]
cmd.extend(sys.argv[1:])
os.execvp(cmd[0], cmd)

[bogen85]

From initial_prep.py above:

    for runlevel in SERVICES.keys():
        links = [x.strip() for x in host.get_fact(FindLinks, f'/etc/runlevels/{runlevel}', quote_path=False)]
        for service in SERVICES[runlevel].split():
            path, target = f'/etc/runlevels/{runlevel}/{service}', f'/etc/init.d/{service}'
            if (path in links) and (host.get_fact(Link, path)["link_target"].strip().removesuffix("'") == target):
                session.append(f'\tservice {service} already in runlevel {runlevel}')
            else:
                files.link(name = f'Add {service} to runlevel {runlevel}', path = path, target = target, force=True)

Notice that I had to clean up the link_target fact for the Link path (there was and extraneous trailing single quote.)

I've not dug into the files.link code, it is possible if it is trying to check, that the check is failing. (As my initial check did as well, until I looked at what was being returned and cleaned it up).

[bogen85]

The equivalent in Ansible, while the output is cleaner, required me to removed the soft link of sh to busybox and make it a hard link. Also, I had to run ansible via sudo to get to work with the chroot. With pyinfra because I was able to use a chroot wrapper, so I did not need to run pyinfra via sudo.

Both Ansible and Pyinfra took about the same time. As I initially stated though, Ansible was more "correct" in determining what needed to be done.

That being said, Pyinfra shows a lot of promise and is by far more flexible.

[bogen85]

See #732 for discussion of the quoting issue that led the path link fact not being correct.

[bogen85]

For #729 @Fizzadar wrote:

@bogen85 Re: systemd-nspawn - are you using the @chroot connector at the moment? There's two approaches here:

* A new connector specific for `systemd-nspawn`

* Add a configuration field `host.data.X` to change the behaviour of the `@chroot` connector (this is probably nicer?)

Regarding systemd-nspawn, yes, I'm using the @chroot connector along with a chroot wrapper script in my path. (Details already discussed in this issue). I'm open to either (new connector or new configuration field) as long as I have the some functionality I'm getting from the wrapper (systemd-nspawn run with sudo, not all of pyinfra) (with ansible and chroot, all of ansible has to be run as root).

[Fizzadar]

I think this is fixed? At least the init.d operation. Please re-open if not!