From 2a67003e47def740859cb6f42c78b70de02d199b Mon Sep 17 00:00:00 2001 From: Stanley Zhang Date: Fri, 9 Sep 2016 11:49:09 +1200 Subject: [PATCH 1/8] can't believe I didn't commit --- README.md | 5 ++ manifests/rotational.pp | 64 +++++++++++++++++++++++++ spec/acceptance/rotational_spec.rb | 30 ++++++++++++ spec/defines/disk_rotational_spec.rb | 70 ++++++++++++++++++++++++++++ 4 files changed, 169 insertions(+) create mode 100644 manifests/rotational.pp create mode 100644 spec/acceptance/rotational_spec.rb create mode 100644 spec/defines/disk_rotational_spec.rb diff --git a/README.md b/README.md index 0f0616c..0fe3459 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,11 @@ Configure xvde1 with a readahead: disk::readahead { 'xvde1': readahead => 2048 } ``` +Configure xvde1 with a rotational: +```puppet +disk::rotational { 'xvde1': rotational => true } +``` + Known Issues: ------------- diff --git a/manifests/rotational.pp b/manifests/rotational.pp new file mode 100644 index 0000000..15ee224 --- /dev/null +++ b/manifests/rotational.pp @@ -0,0 +1,64 @@ +# == Define: disk::rotational +# +# This definition allows the setting of a disk rotational in linux. +# +# +# === Parameters +# +# [*name*] +# String. The device to set the rotational on +# Required +# +# [*rotational*] +# String. The rotational to use +# Default: 1 +# +# +# === Examples +# +# disk::rotational { 'xvde1': +# rotational => 1 +# } +# +# +# === Authors +# +# * Stanley Zhang +# +# +define disk::rotational ( + $rotational = 1 +) { + + if ! defined(Class['disk']) { + fail('You must include the disk base class before using any disk defined resources') + } + + $has_device = inline_template("<%= '${::blockdevices}'.split(',').include?('${name}') %>") + + if str2bool($has_device) == false and $::disk::fail_on_missing_device { + fail("Device ${name} does not exist") + } + + $rotational_value = bool2num($rotational) + if str2bool($has_device) == true { + $maybe_set_rotational = join([ + "test -d /sys/block/${name}", + "echo ${rotational_value} > /sys/block/${name}/queue/rotational", + ], ' && ') + + disk::persist_setting { "disk_rotational_for_${name}": + command => $maybe_set_rotational, + path => $::disk::bin_path, + match => "/sys/block/${name}/queue/rotational", + } + + exec { "disk_rotational_for_${name}": + command => $maybe_set_rotational, + path => $::disk::bin_path, + unless => "grep -q '\\[${rotational_value}\\]' /sys/block/${name}/queue/rotational", + } + + } + +} diff --git a/spec/acceptance/rotational_spec.rb b/spec/acceptance/rotational_spec.rb new file mode 100644 index 0000000..c76571e --- /dev/null +++ b/spec/acceptance/rotational_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper_acceptance' + +describe 'disk tuning' do + context 'set rotational' do + + it 'should work idempotently with no errors' do + pp = <<-EOS + class { 'disk': } + disk::rotational { 'sda': rotational => 1 } + EOS + + # ensure rotational is changed + apply_manifest(pp, :catch_failures => true) + + pp = <<-EOS + class { 'disk': } + disk::rotational { 'sda': rotational => false } + EOS + + # Run it twice and test for idempotency + apply_manifest(pp, :catch_failures => true) + apply_manifest(pp, :catch_changes => true) + end + + describe file('/sys/block/sda/queue/rotational') do + its(:content) { should equal 0 } + end + + end +end diff --git a/spec/defines/disk_rotational_spec.rb b/spec/defines/disk_rotational_spec.rb new file mode 100644 index 0000000..e367629 --- /dev/null +++ b/spec/defines/disk_rotational_spec.rb @@ -0,0 +1,70 @@ +require 'spec_helper' + +describe 'disk::rotational', :type => :define do + let(:params) { { :rotational => 'true' } } + let(:pre_condition) { 'include disk '} + + context 'single device' do + let(:facts) { { :blockdevices => 'xvde1' } } + + context 'valid device' do + let(:title) { 'xvde1' } + let(:expected_cmd) { + [ + 'test -d /sys/block/xvde1', + 'echo 1 > /sys/block/xvde1/queue/rotational' + ].join(' && ') + } + it { should contain_class('disk') } + it { + should contain_exec('disk_rotational_for_xvde1').with({ + 'command' => expected_cmd + }) + } + it { + should contain_disk__persist_setting('disk_rotational_for_xvde1').with({ + 'command' => expected_cmd, + 'match' => "/sys/block/xvde1/queue/rotational" + }) + } + end + + context 'invalid device' do + let(:title) { 'sda' } + + context 'set fail_on_missing_device false' do + let(:pre_condition) { 'class { disk: fail_on_missing_device => false }' } + it { should_not contain_exec('disk_rotational_for_sda') } + it { should_not contain_disk__persist_setting('disk_rotational_for_sda') } + end + + context 'set fail_on_missing_device true' do + let(:pre_condition) { 'class { disk: fail_on_missing_device => true }' } + it { + expect { + should_not contain_exec('disk_rotational_for_sda') + }.to raise_error(Puppet::Error, /Device sda does not exist/) + } + end + end + + end + + context 'multiple devices' do + let(:facts) { { :blockdevices => 'xvde1,xvdf' } } + + context 'valid device' do + let(:title) { 'xvde1' } + it { should contain_exec('disk_rotational_for_xvde1') } + it { should contain_disk__persist_setting('disk_rotational_for_xvde1') } + end + + context 'invalid device' do + let(:title) { 'sda' } + it { should_not contain_exec('disk_rotational_for_sda') } + it { should_not contain_disk__persist_setting('disk_rotational_for_sda') } + end + end + +end + From 5a8927f9317fda59731803b3006e47a9d1a87119 Mon Sep 17 00:00:00 2001 From: Stanley Zhang Date: Sat, 17 Sep 2016 22:10:25 +1200 Subject: [PATCH 2/8] obviously the is_integer function in 3.2.0 doesn't work relys soly on "value == value.to_i.to_s" which can only determinate if a string is integer --- .fixtures.yml | 2 +- metadata.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.fixtures.yml b/.fixtures.yml index 7ad541e..08ee23d 100644 --- a/.fixtures.yml +++ b/.fixtures.yml @@ -2,7 +2,7 @@ fixtures: forge_modules: stdlib: repo: "puppetlabs/stdlib" - ref: "3.2.0" + ref: "3.2.1" symlinks: disk: "#{source_dir}" diff --git a/metadata.json b/metadata.json index 8f3eead..8db4f67 100644 --- a/metadata.json +++ b/metadata.json @@ -14,6 +14,6 @@ { "operatingsystem": "Ubuntu", "operatingsystemrelease": [ "12.04", "14.04" ] } ], "dependencies": [ - { "name": "puppetlabs/stdlib", "version_requirement": ">=3.2.0 <5.0.0" } + { "name": "puppetlabs/stdlib", "version_requirement": ">=3.2.1 <5.0.0" } ] } From 9c52484ae1cd131b8cd72735f137cee52ff9cd4d Mon Sep 17 00:00:00 2001 From: Stanley Zhang Date: Sun, 18 Sep 2016 09:18:12 +1200 Subject: [PATCH 3/8] there seems no centos-65 any more replaced with centos-66 --- .../nodesets/{centos-65-x64.yml => centos-66-x64.yml} | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) rename spec/acceptance/nodesets/{centos-65-x64.yml => centos-66-x64.yml} (73%) diff --git a/spec/acceptance/nodesets/centos-65-x64.yml b/spec/acceptance/nodesets/centos-66-x64.yml similarity index 73% rename from spec/acceptance/nodesets/centos-65-x64.yml rename to spec/acceptance/nodesets/centos-66-x64.yml index 0af2873..c68a329 100644 --- a/spec/acceptance/nodesets/centos-65-x64.yml +++ b/spec/acceptance/nodesets/centos-66-x64.yml @@ -1,10 +1,10 @@ HOSTS: - centos-65-x64: + centos-66-x64: roles: - master platform: el-6-x86_64 - box : puppetlabs/centos-6.5-64-nocm - box_url : https://vagrantcloud.com/puppetlabs/boxes/centos-6.5-64-nocm + box : puppetlabs/centos-6.6-64-nocm + box_url : https://vagrantcloud.com/puppetlabs/boxes/centos-6.6-64-nocm hypervisor : vagrant CONFIG: log_level: verbose From 0756b321a80af94957097ba7ca9ab994ea07ea96 Mon Sep 17 00:00:00 2001 From: Stanley Zhang Date: Sun, 18 Sep 2016 09:18:50 +1200 Subject: [PATCH 4/8] a copied issue and use value comparison --- manifests/rotational.pp | 2 +- spec/acceptance/rotational_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/manifests/rotational.pp b/manifests/rotational.pp index 15ee224..d2af225 100644 --- a/manifests/rotational.pp +++ b/manifests/rotational.pp @@ -56,7 +56,7 @@ exec { "disk_rotational_for_${name}": command => $maybe_set_rotational, path => $::disk::bin_path, - unless => "grep -q '\\[${rotational_value}\\]' /sys/block/${name}/queue/rotational", + unless => "grep -q '${rotational_value}' /sys/block/${name}/queue/rotational", } } diff --git a/spec/acceptance/rotational_spec.rb b/spec/acceptance/rotational_spec.rb index c76571e..f88c7f4 100644 --- a/spec/acceptance/rotational_spec.rb +++ b/spec/acceptance/rotational_spec.rb @@ -23,7 +23,7 @@ class { 'disk': } end describe file('/sys/block/sda/queue/rotational') do - its(:content) { should equal 0 } + its(:content) { should eql "0\n" } end end From a88448bcc601c2c570923196e2990873c77e4de2 Mon Sep 17 00:00:00 2001 From: Stanley Zhang Date: Tue, 13 Dec 2016 13:18:19 +1300 Subject: [PATCH 5/8] use hdparm to control writecache --- .gitignore | 2 + Gemfile | 4 +- manifests/init.pp | 10 ++++- manifests/params.pp | 4 +- manifests/writecache.pp | 62 ++++++++++++++++++++++++++++ spec/acceptance/writecache_spec.rb | 36 ++++++++++++++++ spec/classes/disk_spec.rb | 15 +++++++ spec/defines/disk_writecache_spec.rb | 40 ++++++++++++++++++ spec/spec_helper_acceptance.rb | 3 -- 9 files changed, 169 insertions(+), 7 deletions(-) create mode 100644 manifests/writecache.pp create mode 100644 spec/acceptance/writecache_spec.rb create mode 100644 spec/classes/disk_spec.rb create mode 100644 spec/defines/disk_writecache_spec.rb diff --git a/.gitignore b/.gitignore index 00de2b6..1699278 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,5 @@ spec/fixtures/ .vagrant/ .bundle/ coverage/ +junit/ +log/ diff --git a/Gemfile b/Gemfile index 8f93270..66e2917 100644 --- a/Gemfile +++ b/Gemfile @@ -2,10 +2,10 @@ source ENV['GEM_SOURCE'] || "https://rubygems.org" group :unit_tests do gem 'rake', :require => false - gem 'rspec', '~> 3.1.0', :require => false + gem 'rspec', :require => false gem 'rspec-puppet', :require => false gem 'puppetlabs_spec_helper', :require => false - gem 'puppet-lint', '1.0.1', :require => false + gem 'puppet-lint', :require => false gem 'puppet-syntax', :require => false gem 'metadata-json-lint', :require => false gem 'json', :require => false diff --git a/manifests/init.pp b/manifests/init.pp index addeb34..1284441 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -33,9 +33,17 @@ class disk ( $fail_on_missing_device = $::disk::params::fail_on_missing_device, $persist_file = $::disk::params::persist_file, - $bin_path = $::disk::params::bin_path + $bin_path = $::disk::params::bin_path, + $hdparm_package_name = $::disk::params::hdparm_package_name, + $hdparm_package_ensure = $::disk::params::hdparm_package_ensure, ) inherits disk::params { validate_bool($fail_on_missing_device) validate_absolute_path($persist_file) + + unless defined(Package[$hdparm_package_name]) { + package { $hdparm_package_name: + ensure => $hdparm_package_ensure, + } + } } diff --git a/manifests/params.pp b/manifests/params.pp index 3b58fc6..382da77 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -14,5 +14,7 @@ $fail_on_missing_device = false $persist_file = '/etc/rc.local' $bin_path = ['/bin', '/usr/bin', '/sbin'] - + + $hdparm_package_name = 'hdparm' + $hdparm_package_ensure = 'present' } diff --git a/manifests/writecache.pp b/manifests/writecache.pp new file mode 100644 index 0000000..fbeca46 --- /dev/null +++ b/manifests/writecache.pp @@ -0,0 +1,62 @@ +# == Define: disk::writecache +# +# This definition allows change of write cache of a disk by using hdparm +# +# +# === Parameters +# +# [*name*] +# String. The device to set the writecache on +# Required +# +# [*writecache*] +# Boolean. Whether to turn on write cache +# Default: 1 +# +# +# === Examples +# +# disk::writecache { 'xvde1': +# writecache => 1 +# } +# +# +# === Authors +# +# * Stanley Zhang +# +# +define disk::writecache ( + $writecache = 1 +) { + + if ! defined(Class['disk']) { + fail('You must include the disk base class before using any disk defined resources') + } + + $has_device = inline_template("<%= '${::blockdevices}'.split(',').include?('${name}') %>") + + if str2bool($has_device) == false and $::disk::fail_on_missing_device { + fail("Device ${name} does not exist") + } + + if str2bool($has_device) { + $writecache_value = bool2num($writecache) + $set_writecache_cmd = "hdparm -W${writecache_value} /dev/${name}" + $set_writecache_match = "hdparm -W[0,1] /dev/${name}" + + disk::persist_setting { "disk_writecache_for_${name}": + command => $set_writecache_cmd, + path => $::disk::bin_path, + match => $set_writecache_match, + require => Package[$::disk::hdparm_package_name], + } + + exec { "disk_writecache_for_${name}": + command => $set_writecache_cmd, + path => $::disk::bin_path, + unless => "hdparm -W /dev/${name} | grep write-caching | grep ${writecache_value}", + require => Package[$::disk::hdparm_package_name], + } + } +} diff --git a/spec/acceptance/writecache_spec.rb b/spec/acceptance/writecache_spec.rb new file mode 100644 index 0000000..a9a7894 --- /dev/null +++ b/spec/acceptance/writecache_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper_acceptance' + +describe 'disk tuning' do + context 'set writecache' do + it 'should work idempotently with no errors' do + pp = <<-EOS + class { 'disk': } + disk::writecache { 'sda': writecache => 'yes' } + EOS + + # Run it twice and test for idempotency + apply_manifest(pp, :catch_failures => true) + apply_manifest(pp, :catch_changes => true) + end + + it 'should work as expected' do + pp = <<-EOS + class { 'disk': } + disk::writecache { 'sda': writecache => 'no' } + EOS + # ensure writecache is changed + apply_manifest(pp, :catch_failures => true) + + # Check running settings + # Disable running check because Virtualbox doesn't allow + # diskcache to be disabled + # cache_status = shell( + # 'hdparm -W /dev/sda | grep write-caching | awk \'{print $3}\'') + # expect(cache_status.output.chomp.to_i).to eq 0 + + # Verify persistent settings + rc_local_content = shell('cat /etc/rc.local') + expect(rc_local_content.output).to match /hdparm -W0 \/dev\/sda/ + end + end +end diff --git a/spec/classes/disk_spec.rb b/spec/classes/disk_spec.rb new file mode 100644 index 0000000..1000ca2 --- /dev/null +++ b/spec/classes/disk_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe 'disk', :type => :class do + context 'hdparm' do + context 'pre-defined' do + let(:pre_condition) { 'package { hdparm: ensure => 0.2 }' } + + it { is_expected.to compile.with_all_deps } + it { is_expected.to contain_package('hdparm').with_ensure('0.2') } + end + context 'not defined' do + it { is_expected.to contain_package('hdparm').with_ensure('present') } + end + end +end diff --git a/spec/defines/disk_writecache_spec.rb b/spec/defines/disk_writecache_spec.rb new file mode 100644 index 0000000..d4e49d7 --- /dev/null +++ b/spec/defines/disk_writecache_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' + +describe 'disk::writecache', :type => :define do + let(:facts) { { :blockdevices => 'sda,xvda' } } + let(:pre_condition) { 'include disk' } + let(:hdparm_package_name) { 'hdparm' } + + context 'valid device' do + let(:title) { 'xvda' } + let(:params) { { :writecache => 'yes' } } + let(:exec_title) { "disk_writecache_for_#{title}" } + + it { is_expected.to contain_class('disk') } + it { is_expected.to contain_disk__persist_setting(exec_title) + .with({ + :command => 'hdparm -W1 /dev/xvda', + :match => 'hdparm -W[0,1] /dev/xvda' + }).that_requires("Package[#{hdparm_package_name}]")} + it { is_expected.to contain_exec(exec_title) + .with({ + :command => 'hdparm -W1 /dev/xvda', + :unless => 'hdparm -W /dev/xvda | grep write-caching | grep 1' + }).that_requires("Package[#{hdparm_package_name}]")} + end + + context 'invalid device' do + let(:title) { 'xvdb' } + let(:params) { { :writecache => false } } + + context 'with fail_on_missing_device to false' do + let(:pre_condition) { 'class { disk: fail_on_missing_device => false }' } + it { is_expected.not_to raise_error Puppet::Error, /xvdb does not exist/ } + end + + context 'with fail_on_missing_device to true' do + let(:pre_condition) { 'class { disk: fail_on_missing_device => true }' } + it { is_expected.to raise_error Puppet::Error, /xvdb does not exist/ } + end + end +end diff --git a/spec/spec_helper_acceptance.rb b/spec/spec_helper_acceptance.rb index b828ddb..ac81e39 100644 --- a/spec/spec_helper_acceptance.rb +++ b/spec/spec_helper_acceptance.rb @@ -5,9 +5,6 @@ hosts.each do |host| install_puppet end - - hosts.each do |host| - end end RSpec.configure do |c| From 5152dfe34c33bd8b3e85574021e484ab37b4df58 Mon Sep 17 00:00:00 2001 From: Stanley Zhang Date: Tue, 13 Dec 2016 15:17:38 +1300 Subject: [PATCH 6/8] CentOS 7 requires the rc.local to be executable --- manifests/init.pp | 5 +++++ spec/classes/disk_spec.rb | 6 +++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/manifests/init.pp b/manifests/init.pp index 1284441..9cea305 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -46,4 +46,9 @@ ensure => $hdparm_package_ensure, } } + + # CentOS 7 requires this file be executable + file { $persist_file: + mode => '0755', + } } diff --git a/spec/classes/disk_spec.rb b/spec/classes/disk_spec.rb index 1000ca2..53cf913 100644 --- a/spec/classes/disk_spec.rb +++ b/spec/classes/disk_spec.rb @@ -1,11 +1,15 @@ require 'spec_helper' describe 'disk', :type => :class do + context 'should compile without errors' do + it { is_expected.to compile.with_all_deps } + it { is_expected.to contain_file('/etc/rc.local').with_mode('0755') } + end + context 'hdparm' do context 'pre-defined' do let(:pre_condition) { 'package { hdparm: ensure => 0.2 }' } - it { is_expected.to compile.with_all_deps } it { is_expected.to contain_package('hdparm').with_ensure('0.2') } end context 'not defined' do From 8bb22aceb0a988b90313b4368485473641e7fd8a Mon Sep 17 00:00:00 2001 From: Stanley Zhang Date: Tue, 13 Dec 2016 15:24:31 +1300 Subject: [PATCH 7/8] use rc.d/rc.local cause /etc/rc.local is just a symlink --- README.md | 2 +- manifests/init.pp | 4 ++-- manifests/params.pp | 4 ++-- manifests/persist_setting.pp | 2 +- spec/acceptance/writecache_spec.rb | 2 +- spec/classes/disk_spec.rb | 2 +- spec/defines/disk_persist_setting_spec.rb | 6 +++--- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 0fe3459..5f77b67 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ Configure some defaults: ```puppet class { 'disk': - persist_file => "/etc/rc.local" + persist_file => "/etc/rc.d/rc.local" } ``` diff --git a/manifests/init.pp b/manifests/init.pp index 9cea305..047f3c4 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -11,7 +11,7 @@ # # [*persist_file*] # String. Where to write commands to persist non-persistent settings -# Default: /etc/rc.local +# Default: /etc/rc.d/rc.local # # [*bin_path*] # Array|String. ??? @@ -20,7 +20,7 @@ # # class { disk': # fail_on_missing_device => true, -# persist_file => '/etc/rc.local', +# persist_file => '/etc/rc.d/rc.local', # bin_path => ["/bin", "/usr/bin", "/sbin"] # } # diff --git a/manifests/params.pp b/manifests/params.pp index 382da77..731895e 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -12,9 +12,9 @@ class disk::params { $fail_on_missing_device = false - $persist_file = '/etc/rc.local' + $persist_file = '/etc/rc.d/rc.local' $bin_path = ['/bin', '/usr/bin', '/sbin'] - + $hdparm_package_name = 'hdparm' $hdparm_package_ensure = 'present' } diff --git a/manifests/persist_setting.pp b/manifests/persist_setting.pp index 78781df..545faa2 100644 --- a/manifests/persist_setting.pp +++ b/manifests/persist_setting.pp @@ -19,7 +19,7 @@ # # [*persist_file*] # String. Path to a file where the command will become persisted -# Default: /etc/rc.local +# Default: /etc/rc.d/rc.local # # # === Authors diff --git a/spec/acceptance/writecache_spec.rb b/spec/acceptance/writecache_spec.rb index a9a7894..85fc7e0 100644 --- a/spec/acceptance/writecache_spec.rb +++ b/spec/acceptance/writecache_spec.rb @@ -29,7 +29,7 @@ class { 'disk': } # expect(cache_status.output.chomp.to_i).to eq 0 # Verify persistent settings - rc_local_content = shell('cat /etc/rc.local') + rc_local_content = shell('cat /etc/rc.d/rc.local') expect(rc_local_content.output).to match /hdparm -W0 \/dev\/sda/ end end diff --git a/spec/classes/disk_spec.rb b/spec/classes/disk_spec.rb index 53cf913..b555197 100644 --- a/spec/classes/disk_spec.rb +++ b/spec/classes/disk_spec.rb @@ -3,7 +3,7 @@ describe 'disk', :type => :class do context 'should compile without errors' do it { is_expected.to compile.with_all_deps } - it { is_expected.to contain_file('/etc/rc.local').with_mode('0755') } + it { is_expected.to contain_file('/etc/rc.d/rc.local').with_mode('0755') } end context 'hdparm' do diff --git a/spec/defines/disk_persist_setting_spec.rb b/spec/defines/disk_persist_setting_spec.rb index a090010..1422419 100644 --- a/spec/defines/disk_persist_setting_spec.rb +++ b/spec/defines/disk_persist_setting_spec.rb @@ -6,7 +6,7 @@ let(:params) { { :path => [ "/bin", "/usr/bin", "/sbin" ], - :persist_file => "/etc/rc.local" + :persist_file => "/etc/rc.d/rc.local" } } @@ -16,7 +16,7 @@ should contain_file_line('test').with({ 'line' => "PATH=/bin:/usr/bin:/sbin false", 'match' => "^PATH=/bin:/usr/bin:/sbin false", - 'path' => "/etc/rc.local" + 'path' => "/etc/rc.d/rc.local" }) } end @@ -32,7 +32,7 @@ should contain_file_line('test').with({ 'line' => "PATH=/bin:/usr/bin:/sbin echo noop > /sys/block/sda/queue/scheduler", 'match' => "/sys/block/sda/queue/scheduler", - 'path' => "/etc/rc.local" + 'path' => "/etc/rc.d/rc.local" }) } end From df5500af78ee78b01f40e9b369b559da6c69b74e Mon Sep 17 00:00:00 2001 From: Stanley Zhang Date: Thu, 6 Apr 2017 14:41:21 +1200 Subject: [PATCH 8/8] make it work with right idea of fail_on_missing --- manifests/readahead.pp | 32 ++++++++++++---------------- manifests/rotational.pp | 32 ++++++++++++---------------- manifests/scheduler.pp | 32 ++++++++++++---------------- manifests/writecache.pp | 30 ++++++++++++-------------- metadata.json | 2 +- spec/defines/disk_readahead_spec.rb | 14 ++++++++---- spec/defines/disk_rotational_spec.rb | 14 ++++++++---- spec/defines/disk_scheduler_spec.rb | 14 ++++++++---- 8 files changed, 87 insertions(+), 83 deletions(-) diff --git a/manifests/readahead.pp b/manifests/readahead.pp index bb68ef5..42143b2 100644 --- a/manifests/readahead.pp +++ b/manifests/readahead.pp @@ -43,24 +43,20 @@ fail("Device ${name} does not exist") } - if str2bool($has_device) == true { - $maybe_set_readahead = join([ - "test -d /sys/block/${name}", - "blockdev --setra ${readahead} /dev/${name}", - ], ' && ') - - disk::persist_setting { "disk_readahead_for_${name}": - command => $maybe_set_readahead, - path => $::disk::bin_path, - match => "blockdev\\s--setra\\s[0-9]+\\s/dev/${name}", - } - - exec { "disk_readahead_for_${name}": - command => $maybe_set_readahead, - path => $::disk::bin_path, - unless => "blockdev --getra /dev/${name} | grep -q ${readahead}", - } - + $maybe_set_readahead = join([ + "test -d /sys/block/${name}", + "blockdev --setra ${readahead} /dev/${name}", + ], ' && ') + + disk::persist_setting { "disk_readahead_for_${name}": + command => $maybe_set_readahead, + path => $::disk::bin_path, + match => "blockdev\\s--setra\\s[0-9]+\\s/dev/${name}", } + exec { "disk_readahead_for_${name}": + command => $maybe_set_readahead, + path => $::disk::bin_path, + unless => "blockdev --getra /dev/${name} | grep -q ${readahead}", + } } diff --git a/manifests/rotational.pp b/manifests/rotational.pp index d2af225..53f71ed 100644 --- a/manifests/rotational.pp +++ b/manifests/rotational.pp @@ -41,24 +41,20 @@ } $rotational_value = bool2num($rotational) - if str2bool($has_device) == true { - $maybe_set_rotational = join([ - "test -d /sys/block/${name}", - "echo ${rotational_value} > /sys/block/${name}/queue/rotational", - ], ' && ') - - disk::persist_setting { "disk_rotational_for_${name}": - command => $maybe_set_rotational, - path => $::disk::bin_path, - match => "/sys/block/${name}/queue/rotational", - } - - exec { "disk_rotational_for_${name}": - command => $maybe_set_rotational, - path => $::disk::bin_path, - unless => "grep -q '${rotational_value}' /sys/block/${name}/queue/rotational", - } - + $maybe_set_rotational = join([ + "test -d /sys/block/${name}", + "echo ${rotational_value} > /sys/block/${name}/queue/rotational", + ], ' && ') + + disk::persist_setting { "disk_rotational_for_${name}": + command => $maybe_set_rotational, + path => $::disk::bin_path, + match => "/sys/block/${name}/queue/rotational", } + exec { "disk_rotational_for_${name}": + command => $maybe_set_rotational, + path => $::disk::bin_path, + unless => "grep -q '${rotational_value}' /sys/block/${name}/queue/rotational", + } } diff --git a/manifests/scheduler.pp b/manifests/scheduler.pp index cd9878d..f2e2106 100644 --- a/manifests/scheduler.pp +++ b/manifests/scheduler.pp @@ -41,24 +41,20 @@ fail("Device ${name} does not exist") } - if str2bool($has_device) == true { - $maybe_set_scheduler = join([ - "test -d /sys/block/${name}", - "echo ${scheduler} > /sys/block/${name}/queue/scheduler", - ], ' && ') - - disk::persist_setting { "disk_scheduler_for_${name}": - command => $maybe_set_scheduler, - path => $::disk::bin_path, - match => "/sys/block/${name}/queue/scheduler", - } - - exec { "disk_scheduler_for_${name}": - command => $maybe_set_scheduler, - path => $::disk::bin_path, - unless => "grep -q '\\[${scheduler}\\]' /sys/block/${name}/queue/scheduler", - } - + $maybe_set_scheduler = join([ + "test -d /sys/block/${name}", + "echo ${scheduler} > /sys/block/${name}/queue/scheduler", + ], ' && ') + + disk::persist_setting { "disk_scheduler_for_${name}": + command => $maybe_set_scheduler, + path => $::disk::bin_path, + match => "/sys/block/${name}/queue/scheduler", } + exec { "disk_scheduler_for_${name}": + command => $maybe_set_scheduler, + path => $::disk::bin_path, + unless => "grep -q '\\[${scheduler}\\]' /sys/block/${name}/queue/scheduler", + } } diff --git a/manifests/writecache.pp b/manifests/writecache.pp index fbeca46..0e1ada9 100644 --- a/manifests/writecache.pp +++ b/manifests/writecache.pp @@ -40,23 +40,21 @@ fail("Device ${name} does not exist") } - if str2bool($has_device) { - $writecache_value = bool2num($writecache) - $set_writecache_cmd = "hdparm -W${writecache_value} /dev/${name}" - $set_writecache_match = "hdparm -W[0,1] /dev/${name}" + $writecache_value = bool2num($writecache) + $set_writecache_cmd = "hdparm -W${writecache_value} /dev/${name}" + $set_writecache_match = "hdparm -W[0,1] /dev/${name}" - disk::persist_setting { "disk_writecache_for_${name}": - command => $set_writecache_cmd, - path => $::disk::bin_path, - match => $set_writecache_match, - require => Package[$::disk::hdparm_package_name], - } + disk::persist_setting { "disk_writecache_for_${name}": + command => $set_writecache_cmd, + path => $::disk::bin_path, + match => $set_writecache_match, + require => Package[$::disk::hdparm_package_name], + } - exec { "disk_writecache_for_${name}": - command => $set_writecache_cmd, - path => $::disk::bin_path, - unless => "hdparm -W /dev/${name} | grep write-caching | grep ${writecache_value}", - require => Package[$::disk::hdparm_package_name], - } + exec { "disk_writecache_for_${name}": + command => $set_writecache_cmd, + path => $::disk::bin_path, + unless => "hdparm -W /dev/${name} | grep write-caching | grep ${writecache_value}", + require => Package[$::disk::hdparm_package_name], } } diff --git a/metadata.json b/metadata.json index 8db4f67..b790527 100644 --- a/metadata.json +++ b/metadata.json @@ -14,6 +14,6 @@ { "operatingsystem": "Ubuntu", "operatingsystemrelease": [ "12.04", "14.04" ] } ], "dependencies": [ - { "name": "puppetlabs/stdlib", "version_requirement": ">=3.2.1 <5.0.0" } + { "name": "puppetlabs/stdlib", "version_requirement": ">=3.2.1 <4.16.0" } ] } diff --git a/spec/defines/disk_readahead_spec.rb b/spec/defines/disk_readahead_spec.rb index 7683dc8..c826a39 100644 --- a/spec/defines/disk_readahead_spec.rb +++ b/spec/defines/disk_readahead_spec.rb @@ -34,8 +34,8 @@ context 'set fail_on_missing_device false' do let(:pre_condition) { 'class { disk: fail_on_missing_device => false }' } - it { should_not contain_exec('disk_readahead_for_sda') } - it { should_not contain_disk__persist_setting('disk_readahead_for_sda') } + it { should contain_exec('disk_readahead_for_sda') } + it { should contain_disk__persist_setting('disk_readahead_for_sda') } end context 'set fail_on_missing_device true' do @@ -43,6 +43,7 @@ it { expect { should_not contain_exec('disk_readahead_for_sda') + should_not contain_disk__persist_setting('disk_readahead_for_sda') }.to raise_error(Puppet::Error, /Device sda does not exist/) } end @@ -60,9 +61,14 @@ end context 'invalid device' do + let(:pre_condition) { 'class { disk: fail_on_missing_device => true }' } let(:title) { 'sda' } - it { should_not contain_exec('disk_readahead_for_sda') } - it { should_not contain_disk__persist_setting('disk_readahead_for_sda') } + it { + expect { + should_not contain_exec('disk_readahead_for_sda') + should_not contain_disk__persist_setting('disk_readahead_for_sda') + }.to raise_error(Puppet::Error, /Device sda does not exist/) + } end end diff --git a/spec/defines/disk_rotational_spec.rb b/spec/defines/disk_rotational_spec.rb index e367629..2ffc56c 100644 --- a/spec/defines/disk_rotational_spec.rb +++ b/spec/defines/disk_rotational_spec.rb @@ -34,8 +34,8 @@ context 'set fail_on_missing_device false' do let(:pre_condition) { 'class { disk: fail_on_missing_device => false }' } - it { should_not contain_exec('disk_rotational_for_sda') } - it { should_not contain_disk__persist_setting('disk_rotational_for_sda') } + it { should contain_exec('disk_rotational_for_sda') } + it { should contain_disk__persist_setting('disk_rotational_for_sda') } end context 'set fail_on_missing_device true' do @@ -43,6 +43,7 @@ it { expect { should_not contain_exec('disk_rotational_for_sda') + should_not contain_disk__persist_setting('disk_rotational_for_sda') }.to raise_error(Puppet::Error, /Device sda does not exist/) } end @@ -60,9 +61,14 @@ end context 'invalid device' do + let(:pre_condition) { 'class { disk: fail_on_missing_device => true }' } let(:title) { 'sda' } - it { should_not contain_exec('disk_rotational_for_sda') } - it { should_not contain_disk__persist_setting('disk_rotational_for_sda') } + it { + expect { + should_not contain_exec('disk_rotational_for_sda') + should_not contain_disk__persist_setting('disk_rotational_for_sda') + }.to raise_error(Puppet::Error, /Device sda does not exist/) + } end end diff --git a/spec/defines/disk_scheduler_spec.rb b/spec/defines/disk_scheduler_spec.rb index 4f2517a..d787391 100644 --- a/spec/defines/disk_scheduler_spec.rb +++ b/spec/defines/disk_scheduler_spec.rb @@ -34,8 +34,8 @@ context 'set fail_on_missing_device false' do let(:pre_condition) { 'class { disk: fail_on_missing_device => false }' } - it { should_not contain_exec('disk_scheduler_for_sda') } - it { should_not contain_disk__persist_setting('disk_scheduler_for_sda') } + it { should contain_exec('disk_scheduler_for_sda') } + it { should contain_disk__persist_setting('disk_scheduler_for_sda') } end context 'set fail_on_missing_device true' do @@ -43,6 +43,7 @@ it { expect { should_not contain_exec('disk_scheduler_for_sda') + should_not contain_disk__persist_setting('disk_scheduler_for_sda') }.to raise_error(Puppet::Error, /Device sda does not exist/) } end @@ -60,9 +61,14 @@ end context 'invalid device' do + let(:pre_condition) { 'class { disk: fail_on_missing_device => true }' } let(:title) { 'sda' } - it { should_not contain_exec('disk_scheduler_for_sda') } - it { should_not contain_disk__persist_setting('disk_scheduler_for_sda') } + it { + expect { + should_not contain_exec('disk_scheduler_for_sda') + should_not contain_disk__persist_setting('disk_scheduler_for_sda') + }.to raise_error(Puppet::Error, /Device sda does not exist/) + } end end