puppetlabs / puppetlabs-concat

File concatenation system for Puppet
Apache License 2.0
171 stars 303 forks source link

concat::fragment does not seem to notify properly #740

Open faxm0dem opened 2 years ago

faxm0dem commented 2 years ago

Describe the Bug

When using a notify meta parameter in a concat::fragment, the target resource doesn't get notified.

Expected Behavior

When a concat::fragment's state changes, it should properly notify the target resource.

Steps to Reproduce

Example code:

service { 'foo':
  ensure => 'running',
}

concat { '/tmp/foo':
  ensure => 'present',
}

concat::fragment { '/tmp/foo bar':
  target  => '/tmp/foo',
  content => 'bar',
  order   => '10',
  notify  => Service['foo'],
}
  1. Make sure service foo is running
  2. Make sure file /tmp/foo is not present
  3. Run puppet apply on the above code
Notice: Compiled catalog for localhost in environment production in 0.08 seconds
Notice: /Stage[main]/Main/Concat[/tmp/foo]/File[/tmp/foo]/ensure: defined content as '{sha256}fcde2b2edba56bf408601fb721fe9b5c338d10ee429ea04fae5511b68fbf8fb9'
Notice: Applied catalog in 0.03 seconds

As you can see, even though the file changed, it didn't trigger restart of the service.

Environment

b4ldr commented 2 years ago

looks like this could be the same issue as MODULES-7533 (also see #430)

ekohl commented 2 years ago

For some reason these meta parameters are explicitly not copied here: https://github.com/puppetlabs/puppetlabs-concat/blob/4148c4b21d84a4663d291d17700ac6063e678c8c/lib/puppet/type/concat_file.rb#L344

It was introduced in https://github.com/puppetlabs/puppetlabs-concat/commit/dd88b1a4eee853cfff60b99096c05ce85a9d556e but it doesn't explain why those were excluded.

vchepkov commented 1 year ago

IMHO, one should use concat's resource notify, not an single fragment - result is the same, isn't it?

faxm0dem commented 1 year ago

I can imagine cases where one fragment should notify the service, but not another. For instance, a changed comment in a config file shouldn't restart the service.

b4ldr commented 1 year ago

IMHO, one should use concat's resource notify, not an single fragment - result is the same, isn't it?

A real world example we have for this is the puppet.conf file on the puppetserver. most changes to this file are only read by the agent and dont require any notify. however changes to the server section require the puppetserver to be reloaded