test: cockpit-pcp is deprecated - use cockpit-machines

[richm]

The tests_packages_full test is failing on el9.6 and later - cockpit-pcp
is missing - use cockpit-machines instead which seems to be available
on all platforms.

Signed-off-by: Rich Megginson [email protected]

[richm]

[citest]

[richm]

[citest]

[martinpitt]

OK -- Note that you will have to replace the "cockpit-pcp" fallback with something else eventually, it's gone in current Ubuntu version as well.

[richm]

OK -- Note that you will have to replace the "cockpit-pcp" fallback with something else eventually, it's gone in current Ubuntu version as well.

Ok - I guess we'll need to do that when we add a CI test for Integration tests / ubuntu (ubuntu-24.04)

[richm]

[citest]

[spetrosi]

[citest bad]

[martinpitt]

Argh! I did a review three days ago, but apparently actually sending it failed. Sorry! (I was already wondering why I was being ignored..)

[richm]

@martinpitt seems cockpit-podman is not available on all Ubuntu platforms

[martinpitt]

@richm weird -- it does exist in 22.04, 24.04, and latest: https://launchpad.net/ubuntu/+source/cockpit-podman

[martinpitt]

I'm not sure I understand this -- what actually tells the role to install cockpit-podman? Shouldn't that be here? As far as I can see, this only changes the check after the role ran, but not the role invocation itself?

Update: That should happen through

__cockpit_packages_full:
  - cockpit-*

So this feels like a bug.

[richm]

I'm not sure I understand this -- what actually tells the role to install cockpit-podman?

The test tells the cockpit role to install all packages specified in __cockpit_packages_full.

On RedHat family https://github.com/linux-system-roles/cockpit/blob/main/vars/RedHat-10.yml#L24
and by default if no OS customization is specified https://github.com/linux-system-roles/cockpit/blob/main/vars/default.yml#L16
the role installs cockpit-*

Shouldn't that be here?

Yes.

As far as I can see, this only changes the check after the role ran, but not the role invocation itself?

Right. So the problem is that installing cockpit-* on Ubuntu is not pulling in the cockpit-podman package.

Update: That should happen through

__cockpit_packages_full:
  - cockpit-*

So this feels like a bug.

[richm]

yes, this is a bug - note this from the Ubuntu log:

TASK [linux-system-roles.cockpit : Set version specific variables] *************
task path: /home/runner/work/cockpit/cockpit/tests/roles/linux-system-roles.cockpit/tasks/set_vars.yml:32
ok: [localhost] => (item=/home/runner/work/cockpit/cockpit/tests/roles/linux-system-roles.cockpit/vars/Debian.yml) => {"ansible_facts": {"__cockpit_packages": {"default": "{{ __cockpit_packages_minimal + __cockpit_packages_default }}", "full": "{{ __cockpit_packages_minimal + __cockpit_packages_default + __cockpit_packages_full }}", "minimal": "{{ __cockpit_packages_minimal }}"}, "__cockpit_packages_default": ["cockpit", "cockpit-networkmanager", "cockpit-packagekit", "cockpit-storaged"], "__cockpit_packages_full": ["cockpit-doc", "cockpit-machines", "cockpit-pcp"], "__cockpit_packages_minimal": ["cockpit-system", "cockpit-ws"]}, "ansible_included_var_files": ["/home/runner/work/cockpit/cockpit/tests/roles/linux-system-roles.cockpit/vars/Debian.yml"], "ansible_loop_var": "item", "changed": false, "item": "/home/runner/work/cockpit/cockpit/tests/roles/linux-system-roles.cockpit/vars/Debian.yml"}

It is using Debian specific values on Ubuntu

[richm]

@martinpitt ah - because Ubuntu is in Debian os_family and because there is a Debian.yml vars file, this code pulls in those variable definitions for Debian on Ubuntu - https://github.com/linux-system-roles/cockpit/blob/main/tasks/set_vars.yml#L32

so what should we do here? Should I create a vars/Ubuntu.yml which uses cockpit-* for __cockpit_packages_full? Also note that doing so would be unrelated to this PR, so I would do that in a separate PR.

[martinpitt]

That seems right in general -- Ubuntu as a derivative of Debian should be similar enough to Debian to not warrant another copy I think. So is the problem that https://github.com/linux-system-roles/cockpit/blob/main/vars/Debian.yml#L10 doesn't support a "cockpit-*" glob?

As a minimal fix for this PR you may perhaps use "cockpit-machines"? That exists everywhere aside from RHEL 7 (not sure if you still care about that), and it's part of the Debian __full definition. And perhaps in a follow-up add cockpit-podman there, if the globbing doesn't work?

Thanks!

[richm]

That seems right in general -- Ubuntu as a derivative of Debian should be similar enough to Debian to not warrant another copy I think. So is the problem that https://github.com/linux-system-roles/cockpit/blob/main/vars/Debian.yml#L10 doesn't support a "cockpit-*" glob?

correct - and I don't know enough about Debian to know if it should be cockpit-*, or a discrete list of packages.

As a minimal fix for this PR you may perhaps use "cockpit-machines"? That exists everywhere aside from RHEL 7 (not sure if you still care about that), and it's part of the Debian __full definition. And perhaps in a follow-up add cockpit-podman there,

Where is "there"? In Debian.yml in the __cockpit_packages_full list?

if the globbing doesn't work?

ok - but we don't have testing on Debian, so I don't know if cockpit-* will work for all Debian os_family platforms.

Thanks!

[martinpitt]

Where is "there"? In Debian.yml in the __cockpit_packages_full list?

Yes, sorry.

I don't know if cockpit-* will work for all Debian os_family platforms.

This should largely be a function of Ansible and apt/python-apt, not of the specific distro (that seems highly unlikely to patch in a derivative), so if it works on Debian stable or Ubuntu 22.04, it ought to work everywhere.

[richm]

Where is "there"? In Debian.yml in the __cockpit_packages_full list?

Yes, sorry.

I don't know if cockpit-* will work for all Debian os_family platforms.

This should largely be a function of Ansible and apt/python-apt, not of the specific distro (that seems highly unlikely to patch in a derivative), so if it works on Debian stable or Ubuntu 22.04, it ought to work everywhere.

ok - I'll do that in a separate PR

[richm]

[citest]

[martinpitt]

Thanks! LGTM, if that goes green.

[martinpitt]

(although it could just use c-machines everywhere and be simplified, but I don't mind much)

[richm]

(although it could just use c-machines everywhere and be simplified, but I don't mind much)

Yes, much simpler

[richm]

[citest]

[martinpitt]

Yay!