Fixes #32078 - explicitly notify dynflow core service on changes

[evgeni]

No description provided.

[ekohl]

I kind of feel like this would be redundant because ~> is already a notification. You can check this with the .that_notifies() matcher.

[evgeni]

I kind of feel like this would be redundant because ~> is already a notification. You can check this with the .that_notifies() matcher.

Well, no. A ~> B ~> C doesn't make a notify from A to C, and that's what was missing here.

[evgeni]

The interesting thing is: that_notifies doesn't fail w/o my patch, and yet there is no notification if you look at the puppet logs.

[ekohl]

Well, no. A ~> B ~> C doesn't make a notify from A to C, and that's what was missing here.

It was always my understanding that it does make that notify. Let's see if I can create a minimal reproducer.

[ekohl]

Given the following code:

file { '/tmp/file.txt':
  content => "Hello World!\n",
}
~> file { '/tmp/directory':
  ensure => directory,
}
~> service { 'postfix':
  ensure => running,
  enable => true,
}

Then if you run:

# puppet apply test.pp
...
# echo something > /tmp/file.txt
# puppet apply --debug test.pp
...
Info: Applying configuration version '1615809989'
Debug: /Stage[main]/Main/File[/tmp/file.txt]/notify: notify to File[/tmp/directory]
Debug: /Stage[main]/Main/File[/tmp/directory]/notify: notify to Service[postfix]
Notice: /Stage[main]/Main/File[/tmp/file.txt]/content: content changed '{sha256}625275dca783645310e6a62741564055bb0a9d9c6cc01847ace82ec215a3273f' to '{sha256}03ba204e50d126e4674c005e04d82e84c21366780af1f43bd54a37816b6ab340'
Debug: /Stage[main]/Main/File[/tmp/file.txt]: The container Class[Main] will propagate my refresh event
Debug: Executing: '/bin/systemctl is-active -- postfix'
Debug: Executing: '/bin/systemctl is-enabled -- postfix'
Debug: Class[Main]: The container Stage[main] will propagate my refresh event
Debug: Finishing transaction 12100
Debug: Storing state
Debug: Pruned old state cache entries in 0.00 seconds
Debug: Stored state in 0.01 seconds
Notice: Applied catalog in 0.05 seconds
...

So that supports your observation. However, as you also observed, rspec-puppet doesn't make it easy to verify. https://rspec-puppet.com/documentation/classes/#testing-relationships-between-resources doesn't really go into it.

Now I'm questioning if this is a bug in Puppet or a bug rspec-puppet.

[evgeni]

Now I'm questioning if this is a bug in Puppet or a bug rspec-puppet.

Or both? ;)

Let me challenge the "but we know someone over there!" and ping @DavidS ;)

[ekohl]

I've submitted rodjek/rspec-puppet#821 as a minimal reproducer.

[evgeni]

FWIW, I tried puppet agent 4.0.0, 5.0.0, 6.0.0 and 7.4.1, neither of them add a A~>C notification automatically with the above code.

[evgeni]

@ekohl should I add back the "with_notify" test, as that at least correctly expresses the intent of the change with actually failing if it's not present?

[ekohl]

I'm not sure if with_notify is that useful to test here. It's hardcoded there and there are no variables. I question how many bugs we would catch with it. Let's merge it as is now.