Enhance gated template to have content of changes

Differential Revision: D55665988

fbshipit-source-id: 835ced0078917565168f4ce771f8e9d3df894371

cookbooks/fb_helpers/libraries/fb_helpers.rb

@@ -640,21 +640,23 @@ def self.get_hwaddr(interface)
       ::File.read(addrfile).strip.upcase
     end
 
-    def self._request_nw_changes_permission(run_context, new_resource)
+    def self._request_nw_changes_permission(run_context, new_resource, diff)
       run_context.node.default['fb_helpers']['_nw_perm_requested'] = true
       notification = Chef::Resource::Notification.new(
         'fb_helpers_request_nw_changes[manage]',
         :request_nw_changes,
         new_resource,
       )
+      run_context.node.default['fb_helpers']['_nw_perm_changes_requested'][
+        new_resource.name.to_s] = diff
       notification.fix_resource_reference(run_context.resource_collection)
       run_context.root_run_context.add_delayed_action(notification)
     end
 
     # readfile() safely reads file content in a variable,
     # removing the last line termination.
     # It is suitable to read a single-liners (sysctl settings or similar).
-    # It would return an empty string when the file is not avialable.
+    # It would return an empty string when the file is not available.
     #
     # Usage:
     #   readfile(path)

cookbooks/fb_helpers/resources/gated_template.rb

@@ -32,13 +32,15 @@
 default_action :manage
 
 action_class do
+  attr_reader :saved_why_run
+
   # Copied from lib/chef/runner.rb
   def forced_why_run
-    saved = Chef::Config[:why_run]
+    @saved_why_run = Chef::Config[:why_run]
     Chef::Config[:why_run] = true
     yield
   ensure
-    Chef::Config[:why_run] = saved
+    Chef::Config[:why_run] = @saved_why_run
   end
 end
 
@@ -69,10 +71,20 @@ def forced_why_run
         action new_resource.gated_action
       end
     else
-      Chef::Log.info('fb_helpers: not allowed to change configs for ' +
-                     new_resource.name.to_s)
-      Chef::Log.info('fb_helpers: requesting nw change permission')
-      FB::Helpers._request_nw_changes_permission(run_context, new_resource)
+      unless saved_why_run
+        if t.respond_to? :diff
+          # spec mocks respond_to? but return nil
+          diff = t.diff || ''
+          diff_msg = ' would have changed: ' + diff
+        else
+          diff = nil
+          diff_msg = ''
+        end
+        Chef::Log.info('fb_helpers: not allowed to change configs for ' +
+                      new_resource.name.to_s + diff_msg)
+        Chef::Log.info('fb_helpers: requesting nw change permission')
+        FB::Helpers._request_nw_changes_permission(run_context, new_resource, diff)
+      end
     end
   end
 end

cookbooks/fb_helpers/resources/request_nw_changes.rb

@@ -21,7 +21,15 @@
 
 action :request_nw_changes do
   file FB::Helpers::NW_CHANGES_NEEDED do
+    owner node.root_user
+    group node.root_group
+    mode '0644'
     action :touch
+    content lazy {
+              node['fb_helpers']['_nw_perm_changes_requested'].map do |resource, diff|
+                "#{resource} requesting to change:\n#{diff}"
+              end.join("\n").gsub(/\\n/, "\n").concat("\n")
+            }
   end
 end
 

cookbooks/fb_network_scripts/resources/redhat_interface.rb

@@ -190,7 +190,9 @@ def stop(interface)
       Chef::Log.info(
         "fb_network_scripts[#{interface}]: requesting nw change permission",
       )
-      FB::Helpers._request_nw_changes_permission(run_context, new_resource)
+      FB::Helpers._request_nw_changes_permission(
+        run_context, new_resource, "Would have enabled #{interface}"
+      )
     end
   end
 
@@ -392,7 +394,9 @@ def stop(interface)
     Chef::Log.info(
       "fb_network_scripts[#{interface}]: requesting nw change permission",
     )
-    FB::Helpers._request_nw_changes_permission(run_context, new_resource)
+    FB::Helpers._request_nw_changes_permission(
+      run_context, new_resource, "Would have started #{interface}"
+    )
   end
 end
 
@@ -412,7 +416,9 @@ def stop(interface)
                       interface.to_s)
     Chef::Log.info("fb_network_scripts[#{interface}]: requesting nw change " +
                       'permission')
-    FB::Helpers._request_nw_changes_permission(run_context, new_resource)
+    FB::Helpers._request_nw_changes_permission(
+      run_context, new_resource, "Would have stopped #{interface}"
+    )
   end
 end
 
@@ -449,6 +455,8 @@ def stop(interface)
     Chef::Log.info(
       "fb_network_scripts[#{interface}]: requesting nw change permission",
     )
-    FB::Helpers._request_nw_changes_permission(run_context, new_resource)
+    FB::Helpers._request_nw_changes_permission(
+      run_context, new_resource, "Would have disabled #{interface}"
+    )
   end
 end

cookbooks/fb_networkd/resources/default.rb

@@ -187,19 +187,16 @@ def validate_network_addresses(conf)
 
       on_host_networks.delete(path)
 
-      file path do
-        only_if { node.interface_change_allowed?(conf['name']) }
+      fb_helpers_gated_template path do
+        allow_changes node.interface_change_allowed?(conf['name'])
+        gated_action :delete
+        source 'networkd.conf.erb'
         owner node.root_user
         group node.root_group
         mode '0644'
-        action :delete
         notifies :run, 'execute[networkctl reload]', :immediately
         notifies :run, "execute[networkctl reconfigure #{conf['name']}]"
       end
-
-      if !node.interface_change_allowed?(conf['name'])
-        FB::Helpers._request_nw_changes_permission(run_context, new_resource)
-      end
     end
   end
 
@@ -266,18 +263,15 @@ def validate_network_addresses(conf)
     conflicting_links.each do |path|
       on_host_links.delete(path)
 
-      file path do
-        only_if { node.interface_change_allowed?(conf['name']) }
+      fb_helpers_gated_template path do
+        allow_changes node.interface_change_allowed?(conf['name'])
+        gated_action :delete
+        source 'networkd.conf.erb'
         owner node.root_user
         group node.root_group
         mode '0644'
-        action :delete
         notifies :run, "execute[udevadm trigger #{conf['name']}]"
       end
-
-      if !node.interface_change_allowed?(conf['name'])
-        FB::Helpers._request_nw_changes_permission(run_context, new_resource)
-      end
     end
   end
 
@@ -352,19 +346,16 @@ def validate_network_addresses(conf)
       # systemd-networkd.
       restart_for_new_vlan = false
 
-      file path do
-        only_if { node.interface_change_allowed?(conf['name']) }
+      fb_helpers_gated_template path do
+        allow_changes node.interface_change_allowed?(conf['name'])
+        gated_action :delete
+        source 'networkd.conf.erb'
         owner node.root_user
         group node.root_group
         mode '0644'
-        action :delete
         notifies :run, 'execute[networkctl reload]', :immediately
         notifies :run, "execute[networkctl reconfigure #{conf['name']}]"
       end
-
-      if !node.interface_change_allowed?(conf['name'])
-        FB::Helpers._request_nw_changes_permission(run_context, new_resource)
-      end
     end
 
     restart_networkd ||= restart_for_new_vlan
@@ -384,16 +375,16 @@ def validate_network_addresses(conf)
         action :nothing
       end
 
-      file path do
-        only_if { node.interface_change_allowed?(iface) }
-        action :delete
+      fb_helpers_gated_template path do
+        allow_changes node.interface_change_allowed?(iface)
+        gated_action :delete
+        source 'networkd.conf.erb'
+        owner node.root_user
+        group node.root_group
+        mode '0644'
         notifies :run, "execute[networkctl down #{iface}]", :immediately
         notifies :run, 'execute[networkctl reload]'
       end
-
-      unless node.interface_change_allowed?(iface)
-        FB::Helpers._request_nw_changes_permission(run_context, new_resource)
-      end
     end
   end
 
@@ -408,15 +399,15 @@ def validate_network_addresses(conf)
         action :nothing
       end
 
-      file path do
-        only_if { node.interface_change_allowed?(iface) }
-        action :delete
+      fb_helpers_gated_template path do
+        allow_changes node.interface_change_allowed?(iface)
+        gated_action :delete
+        source 'networkd.conf.erb'
+        owner node.root_user
+        group node.root_group
+        mode '0644'
         notifies :run, "execute[udevadm trigger #{iface}]"
       end
-
-      unless node.interface_change_allowed?(iface)
-        FB::Helpers._request_nw_changes_permission(run_context, new_resource)
-      end
     end
   end
 
@@ -431,16 +422,16 @@ def validate_network_addresses(conf)
         action :nothing
       end
 
-      file path do
-        only_if { node.interface_change_allowed?(iface) }
-        action :delete
+      fb_helpers_gated_template path do
+        allow_changes node.interface_change_allowed?(iface)
+        gated_action :delete
+        source 'networkd.conf.erb'
+        owner node.root_user
+        group node.root_group
+        mode '0644'
         notifies :run, "execute[networkctl delete #{iface}]", :immediately
         notifies :run, 'execute[networkctl reload]'
       end
-
-      unless node.interface_change_allowed?(iface)
-        FB::Helpers._request_nw_changes_permission(run_context, new_resource)
-      end
     end
   end