sbadia/puppet-gitlab

Complex system dependencies should not be managed in this module

Opened this issue · 5 comments

We don't manage Ruby and Nginx in this module, and we shouldn't manage Postfix, and not even git and curl.

We should state up-front that these dependencies need to be in-place for a functioning installation.

Perhaps we can offer a hook right now, supplying our (mostly) working defaults, by placing all dependencies in a dependency class, which our users can replace:

class gitlab::dependency {
   ensure_packages ([
    'postfix',
    'redis',
    'nginx',
    'git',
    'curl',
   '...',
   ])
}

class gitlab (
  ...
  $dependency = 'gitlab::dependency',
  ..
) inherits gitlab::params {
  ... ->
  Class[$dependency] ->
  Class[gitlab::install] ~>
  Class[gitlab::config] ~>
  Class[gitlab::service] ->
  anchor{ 'gitlab::end': }
}

Issue #35 addresses this as well and we have slowly been moving requirements to puppet-gitlab-requirements. Abstracting required packages is relatively trivial and moving postfix to our requirements module is a non-issue.

But what do we do with configuration files? Adding the nginx configuration file is certainly part of the application setup but if someone decides they want to roll out their installation with Apache, then the file is no longer relevant and instead should be providing an Apache config file. And if we allow multiple web servers (which ones?) then do we take care of multiple application servers?

As I noted in PR #81, we probably need to determine our boundaries so we can make consistent decisions on things like this.

We shouldn't manage ruby, but with rbenv we can ensure that ruby1.9.3 is 'installed' and used for the gitlab user, without stepping on system ruby. I'm testing this right now in fact.

@sbadia I like this implementation, but how do you plan on implementing the Modulefile? Unless the dependencies portion uses no other modules, they need to be added for a complete install. However, legacy installs may be using those module names and depend on them not being installed by us.

@atomaka, @igalic, all, can you have a look to https://github.com/sbadia/puppet-gitlab/compare/bug;102;sbadia ? it's more a proof of concept (not perfect), in order to deal with our recurring issues (#35, #11, and all rvm/rbenv, …) (some examples here: 9bed304)

The idea is to :

  • Re-integrate gitlab_requirement external module into puppet-gitlab (Refs: #111, #109, #106, #35) and to be able to bootstrap a whole server with this module.
  • Move complex dependency to a separate class (Refs: #35, #102) in order to easily disable/replace it.

Of course keeping in mind to be as flexible as possible ...

@atomaka indeed, I don't know how to handle this for (modulefile level / fixtures, for now the poc integrate all), you think we should make two separate modules? (one with everything, and one without the «requirements» ? (the actual state ^^)).

The other problem I see in this poc, is the risk of duplication of resources if the user want to disable both classes (by using dummy one…).

I'm viewing this issue as it was referenced from another issue from an unrelated PR so I apologize if I'm missing the entire context...

One common approach to managing dependency packages when they might reasonably be dependencies of a class from another module and yet avoiding Dupe Resource Decl errors is the idempotent ensure_packages() function from puppetlabs/stdlib.