Guest facts shown with no useful information

[psss]

When running a simple provision/login/finish command I've noticed the guest facts are shown:

> tmt run provision -h virtual login finish
/var/tmp/tmt/run-014

/plans/default
    provision
        how: virtual
        facts: {'in_sync': False, 'arch': None, 'distro': None, 'kernel_release': None, 'package_manager': None, 'has_selinux': None, 'is_superuser': None, 'os_release_content': {}, 'lsb_release_content': {}}
        user: root
        image: fedora
        memory: 2048 MB
        disk: 40 GB
        arch: x86_64
        list_local_images: False
        progress: booting...
        multihost name: default-0
        arch: x86_64
        distro: Fedora Linux 38 (Cloud Edition)
        summary: 1 guest provisioned

They are shown at the very beginning, with no useful information. Let's show them later when we know something about the guest? And, perhaps, in verbose mode only?

[happz]

That does not look correct, TBH, more like a forgotten debug output or something.

[happz]

Aaah, right, it's testcloud.py being smart and printing all attributes of the guest.

[idorax]

The root cause is that the value of key facts is not None, and its value is printed by ProvisionTestcloud.go().

• https://github.com/teemtee/tmt/blob/main/tmt/steps/provision/testcloud.py#L655

$ python3 -m pdb ~/.virtualenvs/tmt/bin/tmt run provision -h virtual login
> /home/huanli/.virtualenvs/tmt/bin/tmt(3)<module>()
-> try:
(Pdb) b tmt/steps/provision/testcloud.py:642
Breakpoint 1 at /home/huanli/.virtualenvs/tmt/lib/python3.10/site-packages/tmt/steps/provision/testcloud.py:642
(Pdb) b tmt/steps/provision/testcloud.py:656
Breakpoint 2 at /home/huanli/.virtualenvs/tmt/lib/python3.10/site-packages/tmt/steps/provision/testcloud.py:656
(Pdb) c
/var/tmp/tmt/run-005

/plans/foo
    provision
        how: virtual
> /home/huanli/.virtualenvs/tmt/lib/python3.10/site-packages/tmt/steps/provision/testcloud.py(642)go()
-> for key, value in data.items():
(Pdb) ll
612         def go(self) -> None:
613             """ Provision the testcloud instance """
614             super().go()
...<snip>...
642 B->         for key, value in data.items():
643                 if key == 'memory':
644                     self.info('memory', f"{value} MB", 'green')
645                 elif key == 'disk':
646                     self.info('disk', f"{value} GB", 'green')
647                 elif key == 'connection':
648                     self.verbose('connection', value, 'green')
649                 elif key == 'key':
650                     if value:
651                         self.info('key', fmf.utils.listed(value), 'green')
652                 elif key == 'ssh_option':
653                     if value:
654                         self.info('ssh options', fmf.utils.listed(value), 'green')
655                 elif value is not None:
656 B                   self.info(key, value, 'green')
657     
...<snip>...
(Pdb) pp data
TestcloudGuestData(role=None,
                   guest=None,
                   facts=GuestFacts(in_sync=False,
                                    arch=None,
                                    distro=None,
                                    kernel_release=None,
                                    package_manager=None,
                                    has_selinux=None,
                                    is_superuser=None,
                                    os_release_content={},
                                    lsb_release_content={}),
                   port=None,
                   user='root',
                   key=[],
                   password=None,
                   ssh_option=[],
                   image='fedora',
                   memory=2048,
                   disk=40,
                   connection='session',
                   arch='x86_64',
                   image_url=None,
                   instance_name=None,
                   list_local_images=False)
(Pdb) c
> /home/huanli/.virtualenvs/tmt/lib/python3.10/site-packages/tmt/steps/provision/testcloud.py(642)go()
-> for key, value in data.items():
(Pdb) c
> /home/huanli/.virtualenvs/tmt/lib/python3.10/site-packages/tmt/steps/provision/testcloud.py(642)go()
-> for key, value in data.items():
(Pdb) c
> /home/huanli/.virtualenvs/tmt/lib/python3.10/site-packages/tmt/steps/provision/testcloud.py(656)go()
-> self.info(key, value, 'green')
(Pdb) p key, value
('facts', {'in_sync': False, 'arch': None, 'distro': None, 'kernel_release': None, 'package_manager': None, 'has_selinux': None, 'is_superuser': None, 'os_release_content': {}, 'lsb_release_content': {}})
(Pdb) c
        facts: {'in_sync': False, 'arch': None, 'distro': None, 'kernel_release': None, 'package_manager': None, 'has_selinux': None, 'is_superuser': None, 'os_release_content': {}, 'lsb_release_content': {}}
> /home/huanli/.virtualenvs/tmt/lib/python3.10/site-packages/tmt/steps/provision/testcloud.py(642)go()
-> for key, value in data.items():
(Pdb) q

To fix it, can we just ignore key facts? @psss and @happz

[happz]

That's the correct evaluation, and testcloud skipping facts field would help, but this would be quite a sad solution for me.

I'm not very fond of how these provision plugins print the info about guests, they often print fields shared among all guests, plus their own, and there's no single method that would take care of the shared set of keys. And then we get this, there's Guest.show() which prints facts, ignoring the field would have to be in all plugins, tomorrow we can spot the same in podman or artemis plugins... This calls for some "shared part" vs "custom part" solution. They all accept --image but artemis does not print it, but it has two lines to print out SSH options, which each plugin should print out anyway... Very unsatisfactory mess. Ignoring facts would indeed fix the immediate issue, but the underlying cause would be left untouched, ready to re-appear next week somewhere else :/

Maybe a GuestData.show() method to take care of this - print (or not...) fields from GuestData, then plugins can override the parent method and print only their fields as they wish. That could bring some peace, core code would take care of stuff like facts, role, name, and ssh options, testcloud plugin would print out its own fields like arch or memory.

Sometime after that, we could start moving stuff around - e.g. artemis also supports arch, all of them should one day support hardware... All plugins except connect accept --image, do we really need all of them to add their own info('image', image)? Some day, a new plugin will forget to add this line... I'd prefer leaving no space for such omissions, for that we might add mixing guest data classes with just fields from their respective area - beaker, artemis, testcloud, they all support some kind of arch/image/HW selection, we might add an "HW selectable guest" set of fields, and so on.

[psss]

The solution outline by @happz makes very good sense to me. Maybe two common show() methods will be needed, one before the actual provision steps are performed and one once we know everything about the guest, including the guest facts. Not sure whether, as a quick hot fix, we should not ignore the field for testcloud now to provide a nice output.

[psss]

Hotfix proposed in #2129, let's keep this issue to track the proper solution as suggested by @happz.

[psss]

/packit propose-downstream