Unit_file: Add notify_service

[fe80]

Pull Request (PR) description

Hello,

I had add new parameters for Unit_file define, notify_service. This params I can use for disable the service notify. Very useful for not restart an in memory service for space or a new comment.

I've try to add spec but that not work. I've try something like that:

context 'notify_service => false' do
  let(:params) { { enable: true, notify_service: false } }
  
  it do
    expect(subject).to contain_service(title).
      without_subscribes_to("File[/etc/systemd/system/#{title}]")
  end
end

or

context 'notify_service => false' do
  let(:params) { { enable: true, notify_service: false } }
  
  it do
    expect(subject).to contain_file("/etc/systemd/system/#{title}").
      without_notifies("Service[#{title}]")
  end
end

Can you help me for this part ?

This Pull Request (PR) fixes the following issues

Regards,

[ekohl]

I think there's no such thing as without_notifies or without_subscribes_to. The closest I can think of is

it { is_expected.not_to contain_service(title).with_subscribes_to("File[/etc/systemd/system/#{title}]")

Slight nuance is that it's also true if the service is not contained in the catalog. Perhaps a possible enhancement for https://github.com/puppetlabs/rspec-puppet?

[fe80]

Hello @ekohl

Thank it's look good !

I've also create an issue here puppetlabs/rspec-puppet#28

Regards,

[ekohl]

with_subscribes_to expects a property to be set, like

service { $title:
  subscribes_to => File[$file],
}

That's never going to be true. This checks the compiled catalog (which mirrors the check on line 210 for example):

Suggested change

	is_expected.not_to contain_service(title).with_subscribes_to(
	is_expected.not_to contain_service(title).that_subscribes_to(

[fe80]

That not work, but something like this look ok:

           let(:params) { { enable: true, notify_service: false } }
 
           it do
-            is_expected.not_to contain_service(title).with_subscribes_to(
-              "File[/etc/systemd/system/#{title}]"
+            is_expected.not_to contain_file("/etc/systemd/system/#{title}").with(
+              notify: /Service\[#{title}\]/
             )
           end
         end

It's good for you ?

[ekohl]

No, that's not the same. Please see https://rspec-puppet.com/documentation/classes/#testing-relationships-between-resources for the background. Where with doesn't doesn't take ~> into account, that_... does.

Your suggestion would fail on this:

file { "/etc/systemd/system/${title}":
  notify => Service[$title],
}

It would not fail on:

file { "/etc/systemd/system/${title}":
}

File["/etc/systemd/system/${title}"] ~> Service[$title]

The reason it now fails is a transitive property. See rodjek/rspec-puppet#821 for more discussion, but there's a chain:

File["/etc/systemd/system/${name}"] ~> Systemd::Daemon_reload[$name] ~> Service[$name]

rspec-puppet thinks this implies:

File["/etc/systemd/system/${name}"] ~> Service[$name]

https://puppet.com/docs/puppet/7/lang_relationships.html#lang_rel_chaining_arrows is unclear about this. However, now in practice it probably is. Because the daemon reload is notified, that probably triggers a service restart anyway. So if $daemon_reload is true (default value), I don't think your patch is valid.

[fe80]

I've try before, with my path when it's true that fail correctly:

diff --git a/spec/defines/unit_file_spec.rb b/spec/defines/unit_file_spec.rb
index a0521aa..6dde40a 100644
--- a/spec/defines/unit_file_spec.rb
+++ b/spec/defines/unit_file_spec.rb
@@ -272,8 +272,18 @@ describe 'systemd::unit_file' do
           let(:params) { { enable: true, notify_service: false } }
 
           it do
-            is_expected.not_to contain_service(title).with_subscribes_to(
-              "File[/etc/systemd/system/#{title}]"
+            is_expected.not_to contain_file("/etc/systemd/system/#{title}").with(
+              notify: /Service\[#{title}\]/
+            )
+          end
+        end
+
+        context 'notify_service => true' do
+          let(:params) { { enable: true, notify_service: true } }
+
+          it do
+            is_expected.not_to contain_file("/etc/systemd/system/#{title}").with(
+              notify: /Service\[#{title}\]/
             )
           end
         end

...
  21) systemd::unit_file supported operating systems on centos-8-x86_64 notify_service => true is expected not to contain File[/etc/systemd/system/test.service] with notify =~ /Service\[test.service\]/
      Failure/Error:
        is_expected.not_to contain_file("/etc/systemd/system/#{title}").with(
          notify: /Service\[#{title}\]/
        )
      
        expected that the catalogue would not contain File[/etc/systemd/system/test.service]
      # ./spec/defines/unit_file_spec.rb:285:in `block (6 levels) in <top (required)>'

Finished in 1 minute 49.83 seconds (files took 54.25 seconds to load)
609 examples, 21 failures
...

[fe80]

Hello,

Any update/idea about this test ?

[ekohl]

I still don't know what the proper behavior is: puppetlabs/rspec-puppet#35 is still open. It may well be that with your current implementation it's impossible to prevent a notification to propagate to the service.

[TheMeier]

implemented in #433