voxpupuli / puppet-augeasproviders_grub

Augeas-based grub type and providers
Apache License 2.0
9 stars 33 forks source link

Hard dependency on grub2-tools on CentOS7 missing #20

Closed vide closed 6 years ago

vide commented 8 years ago

Hi

it seems that by default grub2-tools is not installed on CentOS7 (it comes with grubby), but the kernel_parameter provider (and probably all the other ones too) needs grub2-mkconfig to properly update Grub2 config, nad it comes with the grub2-tools package. So, what's your opinion on this issue? If you are ok with creating manifests to ensure that the package is there in all the supported platforms, I could help you out and prepare a PR. If you think that this should be managed outside this module, then you can simply close this issue :-)

Thanks in advance!

trevor-vaughan commented 8 years ago

@vide I don't think that the modules should manage the package since this is very site dependent and will likely conflict with existing resources in someone's environment.

It could be baked into one of the providers to manage if not present but it gets difficult to correctly test on all OSs and it a little bit of spooky action at a distance (catalog resource injection).

The providers should be confined on the proper commands at this point and should simply fail stating that they can't find a suitable provider.

I'll let the others chime in but I think that the safest action would be to simply document the requirement and leave it up to the user how they manage packages on their system.

vide commented 8 years ago

@trevor-vaughan yeah, I think you're right. I didn't know that all these augeasproviders_* existed, and it makes perfectly sense to leave package dependencies outside them. But it should mentioned in the README, as you say.

raphink commented 8 years ago

I'll happily take a PR on either the README or a confinement on the command in the providers, but like @trevor-vaughan I don't think a manifest to ensure the tools' installation belongs in this module.

trevor-vaughan commented 6 years ago

Closed the issue based on general consensus.