Skip to content

Conversation

@lcndsmr
Copy link
Member

@lcndsmr lcndsmr commented May 22, 2023

Fixes Issue #157

Added task to install gpg if its not instaklled already
@lcndsmr
Copy link
Member Author

lcndsmr commented May 22, 2023

i put the task into the repo role in tasks/main.yml with the package module, since gpg is needed on all OS types

Copy link
Member

@widhalmt widhalmt left a comment

Choose a reason for hiding this comment

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

Hm... on rpm based systems the package is called gnupg2 . We just didn't install it because it's part of the base install. if we install it, we should care for both variants, though.

Since I started the roles that moved into this collection I changed my mind, I have to admit. In the meantime I'm not conviced that installing all dependencies with the role is good practice. As I see in many other roles from other people, packages that are not a direct dependency of installed software should be listed in README.md as dependency and not be touched by the role itself.

Of course, we need to make sure they are still installed via prepare.yml in Molecule checks.

Maybe we should use this discussion as an example on how we want to proceed with the roles and collections in our namespace? I'm ok with both ways, I just want a "default way" of doing it.

@widhalmt
Copy link
Member

The "nuke 'em from orbit" variant would be to install the packages but allow for deactivating this task via variable. Should make everybody happy. Ist just a significant bigger blob of code.

@widhalmt
Copy link
Member

@dgoetz had another idea: Manage all packages without alternatives inside the role. Especially if they are part of default repositories. Make sure there's no version pin. Everything else (optional packages, those that need extra repos etc.) added as task but disableable (is that a word?) via variable.

@lcndsmr
Copy link
Member Author

lcndsmr commented May 22, 2023

Just vomiting thoughts here:

I think we should add dependent packages in the role, as long as we know that they could be missing (like gpg), because maintaining ALL dependencies is nearly impossible and a lot of work, and if they are in the base repos. If they need extra repos or be downloaded or something, they should be diableable (it is a word now).

BUT: Imagine this scenario: We need a package that has its own repo and stuff (like something from hashicorp or something) and if we dont have the package, installation of other things fails and we cannot provide the goaöl of the role. Will we say in the readme "yeah you can disable it, but everything will fail"? I thinkwe should say something like "package x will be provided from source y, if you want to provide it from another source, you can set install_package_x to false. But make sure you have it installed before it runs"

@lcndsmr
Copy link
Member Author

lcndsmr commented May 22, 2023

i will change things here when we decide how to handle that :)

@lcndsmr lcndsmr marked this pull request as draft May 22, 2023 12:17
@widhalmt
Copy link
Member

I was just thinking of the, hopefully mostly hypthetical, situation where users want to install "their own(tm)" version of gpg. I've seen strange things in my consulting days... But to be honest, I guess, if someone really wants to do such a thing, they can always open an issue.

So +1 from my side for your and your and Dirks approach: Install everything we need if it's mandatory for the role to run and mandatory. And install everything else but make the task disableable (🎉 ).

@lcndsmr lcndsmr requested a review from widhalmt May 22, 2023 12:55
@lcndsmr lcndsmr marked this pull request as ready for review May 22, 2023 12:56
@widhalmt widhalmt enabled auto-merge May 22, 2023 13:00
Copy link
Member

@widhalmt widhalmt left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the changes.

I checked again, the package name on RedHat is gnupg2 currently but the package provides gnupg. So we even cover name changes etc. Great!

Approved.

@widhalmt widhalmt added this pull request to the merge queue May 30, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks May 30, 2023
@lcndsmr lcndsmr added this pull request to the merge queue May 30, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks May 30, 2023
@afeefghannam89 afeefghannam89 merged commit f081d36 into main May 30, 2023
@afeefghannam89 afeefghannam89 deleted the fix/missing-gpg-157 branch May 30, 2023 19:52
ivareri pushed a commit to ivareri/ansible-collection-elasticstack that referenced this pull request Jun 17, 2025
Fixes Issue NETWAYS#157

---------

Co-authored-by: Afeef Ghannam <39904920+afeefghannam89@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gpg package is missing as requirement for debian 11 (and possibly other deb based distros)

4 participants