Add Journald support #14

duritong · 2016-10-11T19:40:54Z

Make it possible to manage journald settings

raphink · 2016-10-31T10:08:41Z

manifests/init.pp

+  if !empty($journald_settings) {
+    service{'systemd-journald':
+      ensure => running,
+    }


Do we only want to manage journald if settings are passed, or always?

I'm totally open, I just did an implementation that was the most non-intrusive.

I'd say we might aswell manage it if we're going to.

@duritong can you make the change to always manage it then?

duritong · 2017-01-15T10:47:35Z

Ok, I pushed a rebased version that manages the journald service all the time.

thaiphv · 2017-07-18T00:41:39Z

When can we get this merged?

trevor-vaughan · 2017-07-18T01:41:18Z

I understand wanting to put all of the systemd subsystems under the systemd module, but the various subsystems really are probably complex enough, with enough non-linear changes, to warrant their own modules.

We forked https://github.com/cristifalcas/puppet-journald over at https://github.com/simp/pupmod-simp-journald to continue on with it, get it updated, and add our tests so I would recommend not adding journald support to the main systemd module.

ekohl · 2017-07-18T14:44:01Z

Given the size of those modules I wonder if a system::journald class would also work.

trevor-vaughan · 2017-07-18T16:49:04Z

@ekohl The issue isn't the size, it's the fact that the API for journald and systemd settings change independently and a change to either would cause a (potentially breaking) change to the API. You would basically end up with two forks running under this project to keep track of minute changes in either subsystem.

ekohl · 2017-07-22T11:10:04Z

@trevor-vaughan I'm not that familiar with it so I'll let you judge it then.

raphink · 2017-08-09T09:43:22Z

@mcanevet what do you think? Should we manage journald (and all its friends eventually) or let other modules do that (like @trevor-vaughan suggested)?

locknut · 2017-09-21T06:33:32Z

@raphink Our team is okay with (and prefers) one module for both systemd and journald,

locknut · 2017-09-21T06:40:02Z

We've been running @duritong 's journald patch for quite a while. Seeing a strange issue on CentOS 7.4 - when puppet refreshes the systemd-journald service, the agent run silently dies.

EDIT: FYI - this happens when the puppet agent run is launched by cloud-init.

trevor-vaughan · 2017-09-21T12:09:04Z

@raphink and @mcanevet I would like to point out that both systemd and journald are different subsystems and that an update to the API of one may introduce breaking changes in the module that do not affect the other. As such, I would like to petition again for keeping the two subsystems as separate modules.

raphink · 2017-10-16T07:57:46Z

@mcanevet what's your take on this?

bastelfreak · 2017-10-16T09:35:00Z

Personally I would be fine to have this PR in this module. Journald is, besides timesyncd, resolved and the actual init systemd, part of the systemd project. I see this module as something that managed the project, not only the init system.

@trevor-vaughan systemd itself (the init system) and systemd-journald have different options In a well designed code, breaking changes in one of those components shouldn't effect the other components, so I don't see an issue here. The module may need a major release to support those changes, but that should only affect this specific component (e.g. journald) and not the rest.

bastelfreak · 2018-03-13T08:47:03Z

This module already has support for managing networkd, resolved, timesyncd, so I think managing journald is totally fine as long as the default behavior is to not manage it. @duritong are you still interested in this PR? Can you rebase it?

@trevor-vaughan would you be okay with this?

bastelfreak · 2018-03-13T08:47:55Z

manifests/init.pp

@@ -1,7 +1,8 @@
 # -- Class systemd  
 # This module allows triggering systemd commands once for all modules 
 class systemd (
-  $service_limits = {}
+  $service_limits    = {},


Those parameters need datatypes

bastelfreak · 2018-03-13T08:48:27Z

spec/classes/init_spec.rb

+
+describe 'systemd' do
+
+  let(:facts) { {


could you use rspec-puppet-facts instead?

duritong · 2018-03-13T23:28:42Z

Sure, if it gives the chance to get it merged...

bastelfreak · 2018-03-14T08:18:00Z

manifests/journald.pp

+  }
+  $systemd::journald_settings.each |$option, $value| {
+    ini_setting{
+      $option:


interesting format, I haven't seen it this way in the past.

bastelfreak · 2018-03-14T08:18:40Z

metadata.json

+    },
+    {
+      "name": "puppetlabs/inifile",
+      "version_requirement": ">= 1.6.0 <2.0.0"


can you bump the version requirement to < 3.0.0? 2.2.0 is the latest release.

bastelfreak · 2018-03-14T08:20:17Z

Hi @duritong, thanks for rebasing it. I found one little issue that needs to be fixed. I'm going to merge this tomorrow unless somebody raises concerns.

elfchief · 2018-09-09T16:45:06Z

Any chance of actually getting this merged?

trevor-vaughan · 2018-09-09T16:52:59Z

@bastelfreak Arg...sorry that I missed this until now.

You said previously:

The module may need a major release to support those changes, but that should only affect this specific component (e.g. journald) and not the rest.

But this is exactly my issue. In this case, I can't reliably pin on an upper bound because features in two different subsystems can change independently and in potentially breaking changes. This increases the likelihood of forks if someone needs the fixes to the systemd part but can't use the breaking changes in the journald part.

Note that several of the major OS vendors take in subsystem patches at different rates so there is a relatively good chance that the module will end up in this scenario.

It's irritating, but systemd isn't a single thing, it's a big pluggable system that needs to be treated as such.

bastelfreak · 2018-09-09T17:12:17Z

@trevor-vaughan so we are testing against a fixed list operating systems, and it is pretty unlikely that for example RHEL 7.6 gets an update to 7.7 that pulls in a breaking change within the journald config. We do not even care about breaking changes within journald, right? As long as it still eats the config we provide. We have support for timesyncd/resolved and it is pretty generic and opt in. I think this pattern would be fine for journald as well. And a breaking change within this module for either the init part or any of the other components would result in another major release, so pinning to an upper version boundary works. Overall I think this would be fine?

@elfchief it cannot be merged because it has merge conflicts (and there is still a little debate going on). If you are interested in this PR, you can checkout the feature branch, rebase it, add a switch to enable/disable the management, like we do it here:
https://github.com/camptocamp/puppet-systemd/blob/10cdeb617478262caf05245f131652fa823fef22/manifests/init.pp#L83

Afterwards we can review it again.

trevor-vaughan · 2018-09-09T17:15:58Z

@bastelfreak I have no concerns about EL7 updates but I do worry about the same module trying to take care of EL and Ubuntu since they tend to pull in different changes over time.

I suppose that it can always be split in the future if necessary.

bastelfreak · 2018-09-09T17:20:43Z

As a side note: We (Vox Pupuli) are currently working on beaker tests no AWS. That will allow us to do acceptance tests within real virtual machines and not only docker instances on travis. My plan is to implement the same for this module, so we know it actually works. If anybody is familiar with the beaker-aws module and wants to help please reach out to me or @dhollinger.

duritong · 2018-10-16T19:51:13Z

I rebased it once again.

raphink · 2018-10-19T09:05:56Z

@duritong I'm sorry, I merged db56678 2 days ago and it created a conflict again 😢

duritong · 2018-10-19T12:01:15Z

So I rebased again.

duritong · 2018-10-20T12:26:29Z

/me dancing. thx!

Correct format of fixtures file.

raphink reviewed Oct 31, 2016

View reviewed changes

duritong force-pushed the journald_settings branch 2 times, most recently from 086f52e to 6528a20 Compare January 15, 2017 10:45

bastelfreak added the enhancement New feature or request label Mar 13, 2018

bastelfreak changed the title ~~Journald settings~~ Add Journald support Mar 13, 2018

bastelfreak added the needs-rebase label Mar 13, 2018

bastelfreak reviewed Mar 13, 2018

View reviewed changes

spec/classes/init_spec.rb Outdated

describe 'systemd' do

let(:facts) { {

Copy link

Member

bastelfreak Mar 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you use rspec-puppet-facts instead?

duritong force-pushed the journald_settings branch 2 times, most recently from a9a8603 to 155b374 Compare March 13, 2018 23:22

bastelfreak reviewed Mar 14, 2018

View reviewed changes

duritong force-pushed the journald_settings branch from 155b374 to 2a7b238 Compare March 15, 2018 06:11

duritong force-pushed the journald_settings branch from 2a7b238 to 4733e42 Compare October 16, 2018 19:50

make it possible to manage journald settings

d4944c0

duritong force-pushed the journald_settings branch from 4733e42 to d4944c0 Compare October 19, 2018 12:00

raphink merged commit 8dffbac into voxpupuli:master Oct 19, 2018

duritong deleted the journald_settings branch October 22, 2018 20:57

treydock mentioned this pull request Oct 23, 2018

Manage journald service and configuration #89

Merged

alexjfisher mentioned this pull request Apr 29, 2019

Include puppetlabs-inifile in the dependencies list #77

Closed

op-ct pushed a commit to op-ct/puppet-systemd that referenced this pull request Jun 17, 2022

Merge pull request voxpupuli#14 from traylenator/fixfixtures

40d7e99

Correct format of fixtures file.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Journald support #14

Add Journald support #14

duritong commented Oct 11, 2016

raphink Oct 31, 2016

duritong Oct 31, 2016

raphink Nov 2, 2016

raphink Jan 9, 2017

duritong commented Jan 15, 2017

thaiphv commented Jul 18, 2017

trevor-vaughan commented Jul 18, 2017

ekohl commented Jul 18, 2017

trevor-vaughan commented Jul 18, 2017

ekohl commented Jul 22, 2017

raphink commented Aug 9, 2017

locknut commented Sep 21, 2017

locknut commented Sep 21, 2017 •

edited

Loading

trevor-vaughan commented Sep 21, 2017

raphink commented Oct 16, 2017

bastelfreak commented Oct 16, 2017

bastelfreak commented Mar 13, 2018

bastelfreak Mar 13, 2018

bastelfreak Mar 13, 2018

duritong commented Mar 13, 2018

bastelfreak Mar 14, 2018

bastelfreak Mar 14, 2018

bastelfreak commented Mar 14, 2018

elfchief commented Sep 9, 2018

trevor-vaughan commented Sep 9, 2018

bastelfreak commented Sep 9, 2018

trevor-vaughan commented Sep 9, 2018

bastelfreak commented Sep 9, 2018

duritong commented Oct 16, 2018

raphink commented Oct 19, 2018

duritong commented Oct 19, 2018

duritong commented Oct 20, 2018

Add Journald support #14

Add Journald support #14

Conversation

duritong commented Oct 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

duritong commented Jan 15, 2017

thaiphv commented Jul 18, 2017

trevor-vaughan commented Jul 18, 2017

ekohl commented Jul 18, 2017

trevor-vaughan commented Jul 18, 2017

ekohl commented Jul 22, 2017

raphink commented Aug 9, 2017

locknut commented Sep 21, 2017

locknut commented Sep 21, 2017 • edited Loading

trevor-vaughan commented Sep 21, 2017

raphink commented Oct 16, 2017

bastelfreak commented Oct 16, 2017

bastelfreak commented Mar 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

duritong commented Mar 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bastelfreak commented Mar 14, 2018

elfchief commented Sep 9, 2018

trevor-vaughan commented Sep 9, 2018

bastelfreak commented Sep 9, 2018

trevor-vaughan commented Sep 9, 2018

bastelfreak commented Sep 9, 2018

duritong commented Oct 16, 2018

raphink commented Oct 19, 2018

duritong commented Oct 19, 2018

duritong commented Oct 20, 2018

locknut commented Sep 21, 2017 •

edited

Loading