Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce the tmt+ansible subpackage #3465

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

bajertom
Copy link
Collaborator

Pull Request Checklist

Fixes #3425

  • implement the feature
  • include a release note

%endif
Requires: tmt == %{version}-%{release}
Requires: (ansible or ansible-core)
Requires: ansible-collection-containers-podman
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm not sure this is the correct combination. In the original scenario, after installing both plugins: if ansible is available, both would pull in just the ansible package; if ansible is not available, they would pull in the alternatives, ansible-core and ansible-collection-container-podman. With tmt+ansible, we get either ansible + ansible-collection-containers-podman, or ansible-core + ansible-collection-containers-podman. Which doesn't match the original situation.

Plus, installing tmt+ansible just to make --how local work, as requested in the issue, would pull in ansible-collection-container-podman` which is required for containers, and I did not plan to ever use containers :)

Maybe we should use tmt+ansible to pull in the generic parts - ansible or ansible-core, and leave ansible-collection-container-podman required just by tmt+-provision-container?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the ansible or ansible-core hack was only needed for a specific situation around fedora-36 where the package renaming happened. Now the ansible package always depends on ansible-core. So... I guess we can only require ansible-core now?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should use tmt+ansible to pull in the generic parts - ansible or ansible-core, and leave ansible-collection-container-podman required just by tmt+-provision-container?

That collection is really a small package, so not much extra content. But... at the second look... @happz is right, this is really closely related to the provision-container subpackage.

Copy link
Collaborator

@happz happz Jan 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the situation with ansible-collection-container-podman? Is ansible or ansible-collection-container-podman still needed?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For executing in a container I believe it is. Here's the change which was adding it: da225ee

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no such package on my Fedora 41 laptop. tmt+provision-container pulls in ansible which provides podman-related plugins. ansible-core is indeed unusable for containers, therefore depending on tmt+ansible which depends on ansible-core will not be enough. It seems to me tmt+provision-container should keep its ansible or ansible-collection-container-podman requirement, and gain new tmt+ansible just to pull in whatever "Ansible" means beyond podman integration.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary from the review session:

tmt+ansible
Require: ansible-core

tmt+provision-virtual
Require: tmt+ansible

tmt+provision-container
Require: tmt+ansible
Require: ansible-collection-containers-podman or ansible

@psss psss force-pushed the tbajer-tmt+ansible branch from 457ba99 to faf55aa Compare January 13, 2025 18:29
@@ -64,7 +64,7 @@ Obsoletes: tmt-provision-container < %{version}-%{release}
%endif
Requires: tmt == %{version}-%{release}
Requires: podman
Requires: (ansible or ansible-collection-containers-podman)
Requires: tmt-ansible == %{version}-%{release}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Requires: tmt-ansible == %{version}-%{release}
Requires: tmt+ansible == %{version}-%{release}

The plus sign should be used in the Requires sections as well, right?

@psss psss added the packaging Changes related to the rpm packaging label Jan 13, 2025
@psss psss changed the title tmt+ansible subpackage Introduce the tmt+ansible subpackage Jan 13, 2025
@bajertom bajertom modified the milestones: 1.41, 1.42 Jan 13, 2025
@thrix thrix added the priority | must high priority, must be included in the next release label Jan 14, 2025
@lukaszachy
Copy link
Collaborator

All (?) feature are implemented as ansible playbooks ,
so do I understand correctly that will not work with tmt minimal install? Could it be also documented then?
Or should we have a better name for this package?

@happz
Copy link
Collaborator

happz commented Jan 15, 2025

All (?) feature are implemented as ansible playbooks , so do I understand correctly that will not work with tmt minimal install? Could it be also documented then? Or should we have a better name for this package?

prepare/feature will work under the same conditions as prepare/ansible, so documenting the dependency on Ansible or tmt+ansible for prepare/ansible should also speak about prepare/feature. Whether both should work with the minimal installation, that I cannot answer, knowing very little about the expectations for the minimal installation.

@bajertom
Copy link
Collaborator Author

@lukaszachy As far as I understand it, it will not work with tmt minimal install. I will document this, for the prepare/feature too.

@lukaszachy
Copy link
Collaborator

@psss Is the 'prepare feature' a "core feature"? We say "Minimal tmt installation with core features only" as use case for minimal install - https://tmt.readthedocs.io/en/stable/stories/install.html#minimal

Do we even say which are those?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packaging Changes related to the rpm packaging priority | must high priority, must be included in the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unusable 'prepare -h ansible' in tmt minimal install
5 participants