xneelo / hetzner-needrestart

A puppet module for installing and configuring needrestart
3 stars 8 forks source link

needrestart override_rc overrides clobber existing values #24

Closed anarcat closed 4 months ago

anarcat commented 1 year ago

When you do what I did:

    class 'needrestart':
       package_ensure => $ensure,
       configs        => {
        'restart'     => 'a',
        'override_rc' => {
          'qr(^bacula-sd)'             => 0,
          # do not restart ifup because of dhclient, it may break
          'qr(^ifup@.+\.service)'      => 0,
        },
    }

... and naively think that the override_rc parameters are going to add to the existing configuration, you are mistaken. Soon you will be in a world of hurt as needrestart will aggressively restart critical things like gdm3 or dbus, crashing grahical sessions and generally wrecking havoc on your life.

This is a bit counter-intuitive. What we should be doing instead if add to the override_rc configuration, I think. The way I did it is with this:

  file { '/etc/needrestart/conf.d/safe-overrides.conf':
    content => @(EOF),
    # file managed by Puppet, local changes will be lost
    #
    # do not restart backup file server
    $nrconf{override_rc}{qr(^bacula-sd)} = 0,
    # do not restart ifup because of dhclient, it may break. should be
    # upstream, see https://github.com/liske/needrestart/issues/225
    $nrconf{override_rc}{qr(^ifup@.+\.service)} = 0;
    | EOF
    require => [File['/etc/needrestart/conf.d/'],Class['needrestart::install']],
  }

this adds to the override_rc hash instead of overwriting it.

I wonder if that should replace the class parameter or be a new one? Or maybe it's okay if i just bypass the module and setup my own file?