RHEL7/CentOS7 support + 5 more

[clehene]

I'd like to discuss these changes on the PR and see how much we can integrate :)

• Add support for RHEL7/CentOS7
• Minor refactoring to remove (some) duplication in .pp files
• Added / changed some configs/defaults
• Forced yum cache expiry to actually get updates
• Changed configuration templates so that config files could be sourced directly by service units

[deric]

Generally I like the changes, don't be mistaken by too many comments. Definitely we have to fix the tests and try not to break current behaviour. If the changes would come in separate PR, it would be much easier to merge them. Anyway, we should probably bump the version at least to 0.6 or 1.0.

[clehene]

Thanks @deric I'll address the comments and make multiple PRs

[felixb]

Is there any update on this? I just wanted to implement the same thing. Native systemd support would be very nice.

[deric]

It would be nice to separate this into multiple PR, or create at least an issue for systemd support.

[felixb]

@clehene: are you working on this?

[clehene]

@felixb @deric yes, we use this internally and had very recently made a few more changes we'd like to push upstream. This said, @deric please see my questions above

ping @adragomir

[clehene]

We've rebased on top of master, need to cleanup patches and will make new PRs

[felixb]

any updates?

[clehene]

@felixb I rebased everything we have on top of current master and will make separate PRs for each one CentOS7 will come as a single commit/PR but I recommend we discuss the whole group.

@deric there are a few questions which you still haven't answered. I think it's worth discussing them, can you please take a look over the comments on the PR. Thanks!

[blsmth]

any status on this? I'm currently trying to use this module on CentOS7 since puppet forge advertises it as compatible, but i'm slowly finding it's not fully. some strange issues that may be resolved by this fix

[deric]

@blsmth What kind of issues you're having? Could you create a separate issue, so that we can address then individually?

[blsmth]

thanks @deric see this issue

[clehene]

@felixb @blsmth @deric ping rebased to master and updated Sorry it took forever, but every time I wanted to push this and rebased it got into another round of conflicts with master...

[deric]

@clehene cool, looks much better! What's the reasoning for introducing mesos::common::mkdir_p? Can't we just use:

file{'/some/path':
  ensure  => present,
  recurse => true,
} 

otherwise we have to handle various duplicate declaration conflicts. The exec should contain at least creates:

exec { "mkdir_p-${name}":
  creates => $name,
  command => "mkdir -p ${name}",
  unless  => "test -d ${name}",
  path    => '/bin:/usr/bin',
}

But it would be better if we could manage it just with default file resource.

[clehene]

@deric ensure => directory, recurse => true will only affect ownership (and probably other properties) but won't create the parent directories. It was decided that that's too complicated (https://projects.puppetlabs.com/issues/86) Note that mkdir_p should have a reference to the original author (did I lose it during rebase?) https://github.com/ghoneycutt/puppet-module-common/blob/master/manifests/mkdir_p.pp I'll add creates

[clehene]

The commit "Changed templates to set MESOS_ env vars" should have been squashed (at least partially) into the main RHEL7 support as it's required because SystemD EnvironmentFile doesn't work with export

[clehene]

Question: this is currently sourcing EnvironmentFile=/etc/default/mesos-<%= @name %> If you'd like to maintain the previous functionality based on env vars (not sure it's needed), we could probably have both files created and the template could add "export" optionally, based on a variable or something.

[felixb]

Imho, the config should live in /etc/sysconfig/ on centos.

[clehene]

thanks @felixb for pointing that. I'll fix it.

[clehene]

Quick note. I'm seeing some issues (yum list doesn't return anything because the repo is not being installed.) I'll dig into that and fix it. It's likely a missing dependency.

[deric]

@clehene What is the reason for removing all:

  owner  => $owner,
  group  => $group,

does it cause problems on RHEL7? Can't we just set $owner = undef, so that we don't break existing feature?

[clehene]

@deric I'm not removing them, I'm just setting them globally

  File {
    owner   => $owner,
    group   => $group,
  }

(resource default statements - https://docs.puppetlabs.com/puppet/latest/reference/lang_defaults.html) so that we avoid having them set everywhere

[deric]

Ok, sorry. I've overlooked that.

[deric]

We should introduce some parameter configure_via => env_vars (or files for using /etc/mesos-slave, /etc/mesos-master) which would avoid having same configuration at two places.

[deric]

This PR is getting too old and some of the issues mentioned are probably already solved, please let me know if any of the issues still persists. I'll try to track the discussion as separate issues:

• [ ] Add support for RHEL7/CentOS7
• [x] Minor refactoring to remove (some) duplication in .pp files #75
• [ ] Added / changed some configs/defaults
• [x] Forced yum cache expiry to actually get updates #76
• [ ] Changed configuration templates so that config files could be sourced directly by service units