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

Complex system dependencies should not be managed in this module #102

Open
igalic opened this issue Dec 13, 2013 · 5 comments
Open

Complex system dependencies should not be managed in this module #102

igalic opened this issue Dec 13, 2013 · 5 comments
Labels

Comments

@igalic
Copy link
Collaborator

igalic commented Dec 13, 2013

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': }
}
@atomaka
Copy link
Collaborator

atomaka commented Dec 19, 2013

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.

@andyleejordan
Copy link
Collaborator

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.

brianvans added a commit to brianvans/puppet-gitlab that referenced this issue Mar 11, 2014
…al. removing these to work around duplicate declaration issues prior to sbadia#102 being merged
sbadia added a commit that referenced this issue Mar 28, 2014
This commit allow end users to easily disable/replace the dependency
management of GitLab module. Retro-compatibility is kept and by default
puppet-gitlab take care of GitLab dependency.

To override internal puppet-gitlab class, we can use this call:
  class {'gitlab':
    gitlab_dependency => 'mydepsclass'
  }

Refs: #35, #102
@atomaka
Copy link
Collaborator

atomaka commented Mar 28, 2014

@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.

@sbadia
Copy link
Owner

sbadia commented Apr 3, 2014

@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 :

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…).

@nrvale0
Copy link

nrvale0 commented Apr 3, 2014

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants