swisscom / puppet-scaleio

MIT License
9 stars 6 forks source link

Remove Package_verifiable dependency from the module #2

Open ggeldenhuis opened 8 years ago

ggeldenhuis commented 8 years ago

It is my understanding that Package_verifiable is currently being used to allow a certain workflow in Swisscom. Specifically it allows that ScaleIO can be configured using puppet and that OS upgrades can be done using yum update whilst preventing ScaleIO modules from being updated together with other packages. I fully understand the use case for this but the problem that this creates, is that it is specific to Swisscom and not necessarily desirable behaviour for other companies. Puppet modules should be as standalone as possible and written to be loosely coupled. The dependency on https://github.com/swisscom/puppet-package_verifiable makes it less loosely coupled, its add user specific functionality and currently limits functionality to one OS.

An alternative proposition would be to remove Package_verifiable functionality from the module in favour for the package resource. The requirement to lock packages could then be moved to an internal puppet profile class. Eg some sudo code:

class profiles::mdm (
  $locked_packages,
}{
  include mdm
  package_verifiable { $locked_packages:}
}

Doing so would allow for much greater easy in extending the module and make it more transportable across multiple environments because you are not forced into a specific workflow.

duritong commented 8 years ago

One could also make the usage oprional. Meaning by default the current bahavior/workflow with package_verifiable is kept, but if turned off or on platforms not supporting it, the module goes with the package resource. There are always helper module that add some coupling and it's always hatd to draw a line. A feature flag certainly relaxes your concerns.

If you look at the module there are plenty of other parts where one would need to make things more modular (e.g. direct yum calls) to support other platforms. So it anyway needs some refactoring work supporting multi-distro support and making package_verifiable becomes then a minor part.

I'm pretty sure PRs doing that work and suggesting possible solutions will be considered.

nordewal commented 8 years ago

By using the package_verifiable module we actually achieve two things:

  1. All ScaleIO packages are version locked in the package manager, so that they don't get updated using yum update
  2. If ScaleIO is already installed, we don't manage the package resource, to prevent ScaleIO from being updated by just changing the version number in the scaleio::version parameter.

While the first point could be solved by using an internal wrapper module, the second point is not. Thus, I actually had the same idea that @duritong already suggested this morning. Introducing a feature flag should solve both our use-cases.

As already talked about it over Skype, the module itself for sure needs some refactoring also in other places. I'd be very happy to accept such PRs that make this module working on multiple distros.