RHEL7/CentOS7 support + 5 more #30

clehene · 2015-04-02T00:52:51Z

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 · 2015-04-02T06:53:49Z

templates/master.erb

+MESOS_cluster="<%= @cluster %>"
+MESOS_log_dir="<%= @log_dir %>"
+MESOS_logbufsecs=1
+MESOS_logging_level="INFO"


I don't like default that you can't change. Though, in this case you could override this value in @env_var with same key.

@deric I hope I understand your comment correctly.
As this is a template it could be used to generate defaults or other files.
I'm suggesting it should be based on the environment variables and, hence could be sourced (e.g. by systemd).

The current one relies on variables that are replaced with environment variables by a script that is not versioned with mesos (not even source if it's available).

IMHO there are too many ways to configure things (this, defaults, configs, files, env vars and cli args) and this could be confusing and also unneeded in Puppet managed environments.

We should decide whether we want to configure Mesos via properties (/etc/mesos-master/* and /etc/mesos-slave/*) or by using ENV MESOS_{variables}. I think supporting both at the same time is very confusing. CLI arguments are out of questions, we expect people run Mesos as service using systemd, sysvinit or whatever else. When someone checks Mesos machine configuration will he look into /etc/default/mesos-slave or /etc/mesos-slave? Each Mesos version adds new parameters, it's problematic to add new variables each time this happens.

deric · 2015-04-02T07:08:10Z

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 · 2015-04-05T07:25:27Z

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

felixb · 2015-11-26T06:00:43Z

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

deric · 2015-11-26T23:09:29Z

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

felixb · 2015-11-27T08:15:57Z

@clehene: are you working on this?

clehene · 2015-12-01T23:17:04Z

@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 · 2015-12-12T22:05:12Z

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

felixb · 2016-01-13T09:46:22Z

any updates?

clehene · 2016-01-28T18:35:17Z

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

deric · 2016-01-28T19:52:08Z

manifests/init.pp

@@ -27,14 +27,15 @@
  $conf_dir       = '/etc/mesos',
  # e.g. zk://localhost:2181/mesos
  $zookeeper      = '',
+  $quorum         = 1,


Until now quorum variable could have been set via options:

options => { quorum => 4 }

But which one should take precedence? We don't want to break existing installations, so from this point of view it should be overridden by options value, if defined.

blsmth · 2016-03-16T14:32:14Z

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 · 2016-03-16T14:50:49Z

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

blsmth · 2016-03-16T15:03:45Z

thanks @deric see this issue

clehene · 2016-03-26T23:34:57Z

@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 · 2016-03-27T06:41:55Z

@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 · 2016-03-27T19:54:14Z

@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 · 2016-03-27T19:57:59Z

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

…le class settings

Ensure the directories are created with mkdir -p

This may cause conflicts, but it's better than relying on Mesosphere script that all it seems to do is to replace these with MESOS prefixed vars anyway.

clehene · 2016-03-28T00:49:09Z

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 · 2016-03-28T06:04:53Z

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

clehene · 2016-03-30T00:32:41Z

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

clehene · 2016-03-30T00:34:04Z

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 · 2016-03-30T07:54:22Z

@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 · 2016-03-30T22:50:24Z

@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 · 2016-03-31T08:13:11Z

Ok, sorry. I've overlooked that.

deric · 2016-03-31T08:23:30Z

templates/service/systemd/mesos.service.erb

+
+[Service]
+Delegate=yes
+ExecStart=/usr/sbin/mesos-<%= @name %> <% if @name=='slave' -%>-containerizers=docker,mesos<% end -%>


If you're using Mesosphere packages it would be easier to use:

ExecStart=/usr/bin/mesos-init-wrapper <%= @name %>

if you want to execute directly mesos binaries, we have to make sure, that all environment variables are defined.
Either way -containerizers=docker,mesos should not be hardcoded.

deric · 2016-03-31T08:33:13Z

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 · 2016-07-21T12:10:45Z

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
Minor refactoring to remove (some) duplication in .pp files Remove code duplication #75
Added / changed some configs/defaults
Forced yum cache expiry to actually get updates Expire yum cache #76
Changed configuration templates so that config files could be sourced directly by service units

deric reviewed Apr 2, 2015
View reviewed changes

deric reviewed Jan 28, 2016
View reviewed changes

clehene force-pushed the master branch 2 times, most recently from 4a81e73 to 15d3549 Compare March 26, 2016 23:33

clehene changed the title ~~5 PRs in one (for discussion)...~~ RHEL7/CentOS7 support + 5 more Mar 26, 2016

clehene force-pushed the master branch 3 times, most recently from 4c43b12 to fcdeb51 Compare March 27, 2016 21:23

Cosmin Lehene added 2 commits March 27, 2016 14:47

Systemd service units for RHEL/CENTOS 7+

a985edc

Refactor config: removed owner/group duplication by setting global Fi…

1c8b406

…le class settings

Cosmin Lehene added 4 commits March 27, 2016 14:47

Changed $work_dir defaults to /var/lib/mesos/{master, slave}

826c02c

Ensure the directories are created with mkdir -p

Added master.pp $quorum variable

a943007

Expire yum cache when setting the repo 

0a90409

Changed templates to set MESOS_ env vars

d70b90d

This may cause conflicts, but it's better than relying on Mesosphere script that all it seems to do is to replace these with MESOS prefixed vars anyway.

clehene force-pushed the master branch from fcdeb51 to d70b90d Compare March 27, 2016 21:47

deric reviewed Mar 31, 2016
View reviewed changes

This was referenced Jul 21, 2016

Remove code duplication #75

Closed

Expire yum cache #76

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RHEL7/CentOS7 support + 5 more #30

RHEL7/CentOS7 support + 5 more #30

clehene commented Apr 2, 2015

deric Apr 2, 2015

clehene Apr 14, 2015

deric Jan 28, 2016

deric commented Apr 2, 2015

clehene commented Apr 5, 2015

felixb commented Nov 26, 2015

deric commented Nov 26, 2015

felixb commented Nov 27, 2015

clehene commented Dec 1, 2015

clehene commented Dec 12, 2015

felixb commented Jan 13, 2016

clehene commented Jan 28, 2016

deric Jan 28, 2016

blsmth commented Mar 16, 2016

deric commented Mar 16, 2016

blsmth commented Mar 16, 2016

clehene commented Mar 26, 2016

deric commented Mar 27, 2016

clehene commented Mar 27, 2016

clehene commented Mar 27, 2016

clehene commented Mar 28, 2016

felixb commented Mar 28, 2016

clehene commented Mar 30, 2016

clehene commented Mar 30, 2016

deric commented Mar 30, 2016

clehene commented Mar 30, 2016

deric commented Mar 31, 2016

deric Mar 31, 2016

deric commented Mar 31, 2016

deric commented Jul 21, 2016 •

edited

Loading

RHEL7/CentOS7 support + 5 more #30

Are you sure you want to change the base?

RHEL7/CentOS7 support + 5 more #30

Conversation

clehene commented Apr 2, 2015

deric Apr 2, 2015

Choose a reason for hiding this comment

clehene Apr 14, 2015

Choose a reason for hiding this comment

deric Jan 28, 2016

Choose a reason for hiding this comment

deric commented Apr 2, 2015

clehene commented Apr 5, 2015

felixb commented Nov 26, 2015

deric commented Nov 26, 2015

felixb commented Nov 27, 2015

clehene commented Dec 1, 2015

clehene commented Dec 12, 2015

felixb commented Jan 13, 2016

clehene commented Jan 28, 2016

deric Jan 28, 2016

Choose a reason for hiding this comment

blsmth commented Mar 16, 2016

deric commented Mar 16, 2016

blsmth commented Mar 16, 2016

clehene commented Mar 26, 2016

deric commented Mar 27, 2016

clehene commented Mar 27, 2016

clehene commented Mar 27, 2016

clehene commented Mar 28, 2016

felixb commented Mar 28, 2016

clehene commented Mar 30, 2016

clehene commented Mar 30, 2016

deric commented Mar 30, 2016

clehene commented Mar 30, 2016

deric commented Mar 31, 2016

deric Mar 31, 2016

Choose a reason for hiding this comment

deric commented Mar 31, 2016

deric commented Jul 21, 2016 • edited Loading

deric commented Jul 21, 2016 •

edited

Loading