From e301d7f3a95f15463aa0f512d95da8745ce03297 Mon Sep 17 00:00:00 2001 From: Elyse Salberg Date: Tue, 10 Oct 2017 21:02:43 -0400 Subject: [PATCH] Enable using ifscripts for alias resources; allow setting ifcfg resources to absent (remove file) --- README.md | 12 ++++ manifests/alias.pp | 10 +++ manifests/alias/range.pp | 2 +- manifests/init.pp | 50 +++++++++++++-- spec/defines/network_alias_range_spec.rb | 2 +- spec/defines/network_alias_spec.rb | 81 +++++++++++++++++++++++- spec/spec_helper.rb | 4 ++ 7 files changed, 154 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 3d167aad..b0c645d0 100644 --- a/README.md +++ b/README.md @@ -293,6 +293,18 @@ By default, all changes notify the network service, thus triggering a restart of restart => false, } +Use ifscripts instead of restarting the network services (alias only): + +Alias resources can trigger using the ifdown / ifup scripts instead of doing a full network restart. They also can be set to ensure => 'absent' to remove the ifcfg-${interface} file (as well as 'up' and 'down'). The ifscripts parameter is a boolean that defaults +to false. + + network::alias { 'eth0:1': + ensure => 'up', + ipaddress => '1.2.3.4', + netmask => '255.255.255.0', + ifscripts => true, + } + Hiera ----- diff --git a/manifests/alias.pp b/manifests/alias.pp index a9e9e82f..319c8351 100644 --- a/manifests/alias.pp +++ b/manifests/alias.pp @@ -14,6 +14,7 @@ # $zone - optional # $metric - optional # $restart - optional - defaults to true +# $ifscripts - optional # # === Actions: # @@ -48,12 +49,20 @@ $zone = undef, $metric = undef, $restart = true, + $ifscripts = false, ) { # Validate our data if ! is_ip_address($ipaddress) { fail("${ipaddress} is not an IP address.") } # Validate our booleans validate_bool($noaliasrouting) validate_bool($userctl) + validate_bool($restart) + validate_bool($ifscripts) + # Validate our regular expressions + $states = [ '^up$', '^down$', '^absent$' ] + validate_re($ensure, $states, '$ensure must be "up", "down", or "absent".') + + include '::network' network_if_base { $title: ensure => $ensure, @@ -72,5 +81,6 @@ zone => $zone, metric => $metric, restart => $restart, + ifscripts => $ifscripts, } } # define network::alias diff --git a/manifests/alias/range.pp b/manifests/alias/range.pp index 4746d94d..26539660 100644 --- a/manifests/alias/range.pp +++ b/manifests/alias/range.pp @@ -59,7 +59,7 @@ validate_bool($arpcheck) # Validate our regular expressions $states = [ '^up$', '^down$', '^absent$' ] - validate_re($ensure, $states, '$ensure must be either "up", "down", or "absent".') + validate_re($ensure, $states, '$ensure must be "up", "down", or "absent".') include '::network' diff --git a/manifests/init.pp b/manifests/init.pp index 6e3798fd..785a11f1 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -79,6 +79,7 @@ # $promisc - optional - defaults to false # $restart - optional - defaults to true # $arpcheck - optional - defaults to true +# $ifscripts - optional (alias only) # # === Actions: # @@ -140,6 +141,7 @@ $promisc = false, $restart = true, $arpcheck = true, + $ifscripts = false, ) { # Validate our booleans validate_bool($noaliasrouting) @@ -155,14 +157,23 @@ validate_bool($promisc) validate_bool($restart) validate_bool($arpcheck) + validate_bool($ifscripts) # Validate our regular expressions - $states = [ '^up$', '^down$' ] - validate_re($ensure, $states, '$ensure must be either "up" or "down".') + $states = [ '^up$', '^down$', '^absent$' ] + validate_re($ensure, $states, '$ensure must be "up", "down", or "absent".') include '::network' $interface = $name + # Handle ensure => 'absent' + $file_ensure = $ensure ? { + 'up' => 'present', + 'down' => 'present', + 'absent' => 'absent', + default => 'present', + } + # Deal with the case where $dns2 is non-empty and $dns1 is empty. if $dns2 { if !$dns1 { @@ -193,6 +204,15 @@ $iftemplate = template('network/ifcfg-eth.erb') } + if $ifscripts { + if ! ($interface in $::interfaces) { + $refreshonly_ifscripts = undef + } + else { + $refreshonly_ifscripts = true + } + } + if $flush { exec { 'network-flush': user => 'root', @@ -204,8 +224,30 @@ } } + if $ifscripts { + case $ensure { + 'up': { + exec { "Refresh ${interface}": + command => "/sbin/ifdown ${interface} && /sbin/ifup ${interface}", + refreshonly => $refreshonly_ifscripts, + subscribe => File["ifcfg-${interface}"], + } + } + 'down','absent': { + if ($interface in $::interfaces) { + exec { "ifdown ${interface}": + command => "/sbin/ifdown ${interface}", + before => File["ifcfg-${interface}"], + } + } + } + default: { + } + } + } + file { "ifcfg-${interface}": - ensure => 'present', + ensure => $file_ensure, mode => '0644', owner => 'root', group => 'root', @@ -213,7 +255,7 @@ content => $iftemplate, } - if $restart { + if $restart and ! $ifscripts { File["ifcfg-${interface}"] { notify => Service['network'], } diff --git a/spec/defines/network_alias_range_spec.rb b/spec/defines/network_alias_range_spec.rb index 3abc9512..b8305163 100644 --- a/spec/defines/network_alias_range_spec.rb +++ b/spec/defines/network_alias_range_spec.rb @@ -14,7 +14,7 @@ } end it 'should fail' do - expect {should contain_file('ifcfg-bond77-range0')}.to raise_error(Puppet::Error, /\$ensure must be either "up", "down", or "absent"./) + expect {should contain_file('ifcfg-bond77-range0')}.to raise_error(Puppet::Error, /\$ensure must be "up", "down", or "absent"./) end end diff --git a/spec/defines/network_alias_spec.rb b/spec/defines/network_alias_spec.rb index b5624604..981f9983 100644 --- a/spec/defines/network_alias_spec.rb +++ b/spec/defines/network_alias_spec.rb @@ -13,7 +13,7 @@ } end it 'should fail' do - expect {should contain_file('ifcfg-eth1:1')}.to raise_error(Puppet::Error, /\$ensure must be either "up" or "down"./) + expect {should contain_file('ifcfg-eth1:1')}.to raise_error(Puppet::Error, /\$ensure must be "up", "down", or "absent"./) end end @@ -62,6 +62,26 @@ it { should contain_service('network') } end + context 'required parameters: ensure => absent' do + let(:title) { 'bond2:1' } + let :params do { + :ensure => 'absent', + :ipaddress => '1.2.3.6', + :netmask => '255.255.255.0', + :restart => true, + } + end + let :facts do { + :osfamily => 'RedHat', + :interfaces => 'eth0,bond2:1', + } + end + it { is_expected.to contain_file('ifcfg-bond2:1').with( + :ensure => 'absent', + )} + it { should contain_service('network') } + end + context 'optional parameters' do let(:title) { 'bond3:2' } let :params do { @@ -103,4 +123,63 @@ it { should contain_service('network') } end + context 'optional parameters: ifscripts => true, ensure => up' do + let(:title) { 'bond2:1' } + let :params do { + :ensure => 'up', + :ipaddress => '1.2.3.6', + :netmask => '255.255.255.0', + :restart => true, + :ifscripts => true, + } + end + let :facts do { + :osfamily => 'RedHat', + :interfaces => 'eth0', + } + end + it { should contain_file('ifcfg-bond2:1').with( + :ensure => 'present', + :mode => '0644', + :owner => 'root', + :group => 'root', + :path => '/etc/sysconfig/network-scripts/ifcfg-bond2:1', + )} + it 'should contain File[ifcfg-bond2:1] with required contents' do + verify_contents(catalogue, 'ifcfg-bond2:1', [ + 'DEVICE=bond2:1', + 'BOOTPROTO=none', + 'ONPARENT=yes', + 'TYPE=Ethernet', + 'IPADDR=1.2.3.6', + 'NETMASK=255.255.255.0', + 'NO_ALIASROUTING=no', + 'NM_CONTROLLED=no', + ]) + end + it { is_expected.to contain_file('ifcfg-bond2:1') } + it { is_expected.to contain_exec('Refresh bond2:1') } + end + + context 'optional parameters: ifscripts => true, ensure => absent' do + let(:title) { 'bond2:1' } + let :params do { + :ensure => 'absent', + :ipaddress => '1.2.3.6', + :netmask => '255.255.255.0', + :restart => true, + :ifscripts => true, + } + end + let :facts do { + :osfamily => 'RedHat', + :interfaces => 'eth0,bond2:1', + } + end + it { is_expected.to contain_file('ifcfg-bond2:1').with( + :ensure => 'absent', + )} + it { is_expected.to contain_exec('ifdown bond2:1') } + end + end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2c6f5664..00c3e573 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1 +1,5 @@ require 'puppetlabs_spec_helper/module_spec_helper' + +RSpec.configure do |config| + config.before(:all) {Facter.clear} +end