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

feat: alloy installation via repository + suse support #309

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

Conversation

pkretzer
Copy link

  • Added variables to install the alloy agent via a repository or url (defaults to url).
  • Added support for installing on openSUSE
  • Added molecule configuration for testing.

@CLAassistant
Copy link

CLAassistant commented Jan 13, 2025

CLA assistant check
All committers have signed the CLA.

@ishanjainn
Copy link
Member

@voidquark any thoughts on this?

@voidquark
Copy link
Collaborator

will check this

@voidquark
Copy link
Collaborator

voidquark commented Feb 13, 2025

Hello, Thanks for contributing! 🚀

Here are my findings and some missing pieces:

  • When using the repository, the latest version is always installed. This might not be what users expect when they specify a particular version.
  • The README definitely needs documentation for the new variables.
  • I don’t think it's necessary to modify the Molecule file since the supported distros are already listed here.
  • I'm also not sold to idea that repository management should be part of this role. It make sense to support installation via the repository, but I believe managing the repository itself should be outside the scope of this role.

Why I think the repository should be managed separately:

  • You can handle repository management in your pre_tasks or with a dedicated role that manages repositories system-wide.
  • Alloy Uninstall tasks and removing the repository is risky. What if the repository was set up differently and is now unexpectedly removed? The purpose of uninstalling should be to clean up the role’s resources, not system-level resources like repositories.

Therefore, I believe this PR should be adjusted to:

  • Include possibility to install package via the repository.
  • Allow users to select a specific version for installation via the repository.
  • Make it clear in the README that this role does not manage the repository itself. The repository must already be present on the system.

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.

4 participants