Merge pull request #55 from tvpartytonight/PA-5966

Stop using legacy facts

Author: joshcooper

lib/puppet/provider/mount.rb

@@ -26,7 +26,7 @@ def mount
   def remount
     # TRANSLATORS refers to remounting a file system
     info _('Remounting')
-    os = Facter.value(:operatingsystem)
+    os = Facter.value('os.name')
     supports_remounts = (resource[:remounts] == :true)
     if supports_remounts && os == 'AIX'
       remount_with_option('remount')

lib/puppet/provider/mount/parsed.rb

@@ -1,7 +1,7 @@
 require 'puppet/provider/parsedfile'
 require_relative '../mount'
 
-fstab = case Facter.value(:osfamily)
+fstab = case Facter.value('os.family')
         when 'Solaris' then '/etc/vfstab'
         when 'AIX' then '/etc/filesystems'
         else
@@ -20,14 +20,14 @@
 
   commands mountcmd: 'mount', umount: 'umount'
 
-  @fields = case Facter.value(:osfamily)
+  @fields = case Facter.value('os.family')
             when 'Solaris'
               [:device, :blockdevice, :name, :fstype, :pass, :atboot, :options]
             else
               [:device, :name, :fstype, :options, :dump, :pass]
             end
 
-  if Facter.value(:osfamily) == 'AIX'
+  if Facter.value('os.family') == 'AIX'
     # * is the comment character on AIX /etc/filesystems
     text_line :comment, match: %r{^\s*\*}
   else
@@ -43,7 +43,7 @@
   field_pattern = '(\s*(?>\S+))'
   text_line :incomplete, match: %r{^(?!#{field_pattern}{#{mandatory_fields.length}})}
 
-  case Facter.value(:osfamily)
+  case Facter.value('os.family')
   when 'AIX'
     # The only field that is actually ordered is :name. See `man filesystems` on AIX
     @fields = [:name, :account, :boot, :check, :dev, :free, :mount, :nodename,
@@ -272,7 +272,7 @@ def self.prefetch(resources = nil)
   end
 
   def self.mountinstances
-    regex = case Facter.value(:osfamily)
+    regex = case Facter.value('os.family')
             when 'Darwin'
               %r{ on (?:/private/var/automount)?(\S*)}
             when 'Solaris', 'HP-UX'

lib/puppet/type/mount.rb

@@ -145,7 +145,7 @@ def syncothers
 
     # Default to the device but with "dsk" replaced with "rdsk".
     defaultto do
-      if Facter.value(:osfamily) == 'Solaris'
+      if Facter.value('os.family') == 'Solaris'
         if (device = resource[:device]) && device.include?('/dsk/')
           device.sub(%r{/dsk/}, '/rdsk/')
         elsif (fstype = resource[:fstype]) && fstype == 'nfs'
@@ -193,7 +193,7 @@ def syncothers
 
     defaultto do
       if @resource.managed?
-        if Facter.value(:osfamily) == 'Solaris'
+        if Facter.value('os.family') == 'Solaris'
           '-'
         else
           0
@@ -220,7 +220,7 @@ def munge(value)
     desc "Whether to dump the mount.  Not all platform support this.
         Valid values are `1` or `0` (or `2` on FreeBSD). Default is `0`."
 
-    if Facter.value(:operatingsystem) == 'FreeBSD'
+    if Facter.value('os.name') == 'FreeBSD'
       newvalue(%r{(0|1|2)})
     else
       newvalue(%r{(0|1)})
@@ -267,7 +267,7 @@ def munge(value)
 
     newvalues(:true, :false)
     defaultto do
-      case Facter.value(:operatingsystem)
+      case Facter.value('os.name')
       when 'FreeBSD', 'Darwin', 'DragonFly', 'OpenBSD'
         false
       when 'AIX'

spec/integration/provider/mount_spec.rb

@@ -5,8 +5,6 @@
 describe 'mount provider (integration)', unless: Puppet.features.microsoft_windows? do
   include PuppetSpec::Files
 
-  family = Facter.value(:osfamily)
-
   def create_fake_fstab(initially_contains_entry)
     File.open(@fake_fstab, 'w') do |f|
       if initially_contains_entry
@@ -24,8 +22,8 @@ def create_fake_fstab(initially_contains_entry)
     allow(Facter).to receive(:value).with(:hostname).and_return('some_host')
     allow(Facter).to receive(:value).with(:domain).and_return('some_domain')
     allow(Facter).to receive(:value).with(:kernel).and_return('Linux')
-    allow(Facter).to receive(:value).with(:operatingsystem).and_return('RedHat')
-    allow(Facter).to receive(:value).with(:osfamily).and_return('RedHat')
+    allow(Facter).to receive(:value).with('os.name').and_return('RedHat')
+    allow(Facter).to receive(:value).with('os.family').and_return('RedHat')
     allow(Facter).to receive(:value).with(:fips_enabled).and_return(false)
     Puppet::Util::ExecutionStub.set do |command, _options|
       case command[0]
@@ -111,9 +109,7 @@ def run_in_catalog(settings)
               ['local', 'journaled', '', nil].each do |options_setting|
                 describe "When setting options => '#{options_setting}'" do
                   it "leaves the system in the #{expected_final_state ? 'mounted' : 'unmounted'} state, #{expected_fstab_data ? 'with' : 'without'} data in /etc/fstab" do
-                    if family == 'Solaris'
-                      skip('Solaris: The double :operatingsystem value does not get changed in lib/puppet/provider/mount/parsed.rb')
-                    elsif options_setting && options_setting.empty?
+                    if options_setting && options_setting.empty?
                       expect { run_in_catalog(ensure: ensure_setting, options: options_setting) }.to raise_error Puppet::ResourceError
                     else
                       if options_setting

spec/unit/provider/mount/parsed_spec.rb

@@ -21,20 +21,20 @@
 
   # LAK:FIXME I can't double Facter because this test happens at parse-time.
   it 'defaults to /etc/vfstab on Solaris' do
-    if Facter.value(:osfamily) != 'Solaris'
+    if Facter.value('os.family') != 'Solaris'
       skip('This test only works on Solaris')
     else
       expect(described_class.default_target).to eq('/etc/vfstab')
     end
   end
 
   it 'defaults to /etc/filesystems on AIX' do
-    pending 'This test only works on AIX' unless Facter.value(:osfamily) == 'AIX'
+    pending 'This test only works on AIX' unless Facter.value('os.family') == 'AIX'
     expect(described_class.default_target).to eq('/etc/filesystems')
   end
 
   it 'defaults to /etc/fstab on anything else' do
-    if Facter.value(:osfamily) == 'Solaris'
+    if Facter.value('os.family') == 'Solaris'
       skip('This test only does not work on Solaris')
     else
       expect(described_class.default_target).to eq('/etc/fstab')
@@ -53,7 +53,7 @@
     #   it_should_behave_like "all parsedfile providers",
     #     provider_class, my_fixtures('*.fstab')
 
-    describe 'on Solaris', if: Facter.value(:osfamily) == 'Solaris' do
+    describe 'on Solaris', if: Facter.value('os.family') == 'Solaris' do
       it 'extracts device from the first field' do
         expect(described_class.parse_line(vfstab_sample)[:device]).to eq('/dev/dsk/c0d0s0')
       end
@@ -83,7 +83,7 @@
       end
     end
 
-    describe 'on other platforms than Solaris', if: Facter.value(:osfamily) != 'Solaris' do
+    describe 'on other platforms than Solaris', if: Facter.value('os.family') != 'Solaris' do
       it 'extracts device from the first field' do
         expect(described_class.parse_line(fstab_sample)[:device]).to eq('/dev/vg00/lv01')
       end
@@ -112,7 +112,7 @@
 
   describe 'mountinstances' do
     it 'gets name from mountoutput found on Solaris' do
-      allow(Facter).to receive(:value).with(:osfamily).and_return 'Solaris'
+      allow(Facter).to receive(:value).with('os.family').and_return 'Solaris'
       allow(described_class).to receive(:mountcmd).and_return(File.read(my_fixture('solaris.mount')))
       mounts = described_class.mountinstances
       expect(mounts.size).to eq(6)
@@ -125,7 +125,7 @@
     end
 
     it 'gets name from mountoutput found on HP-UX' do
-      allow(Facter).to receive(:value).with(:osfamily).and_return 'HP-UX'
+      allow(Facter).to receive(:value).with('os.family').and_return 'HP-UX'
       allow(described_class).to receive(:mountcmd).and_return(File.read(my_fixture('hpux.mount')))
       mounts = described_class.mountinstances
       expect(mounts.size).to eq(17)
@@ -149,7 +149,7 @@
     end
 
     it 'gets name from mountoutput found on Darwin' do
-      allow(Facter).to receive(:value).with(:osfamily).and_return 'Darwin'
+      allow(Facter).to receive(:value).with('os.family').and_return 'Darwin'
       allow(described_class).to receive(:mountcmd).and_return(File.read(my_fixture('darwin.mount')))
       mounts = described_class.mountinstances
       expect(mounts.size).to eq(6)
@@ -162,7 +162,7 @@
     end
 
     it 'gets name from mountoutput found on Linux' do
-      allow(Facter).to receive(:value).with(:osfamily).and_return 'Gentoo'
+      allow(Facter).to receive(:value).with('os.family').and_return 'Gentoo'
       allow(described_class).to receive(:mountcmd).and_return(File.read(my_fixture('linux.mount')))
       mounts = described_class.mountinstances
       expect(mounts[0]).to eq(name: '/', mounted: :yes)
@@ -175,7 +175,7 @@
     end
 
     it 'gets name from mountoutput found on AIX' do
-      allow(Facter).to receive(:value).with(:osfamily).and_return 'AIX'
+      allow(Facter).to receive(:value).with('os.family').and_return 'AIX'
       allow(described_class).to receive(:mountcmd).and_return(File.read(my_fixture('aix.mount')))
       mounts = described_class.mountinstances
       expect(mounts[0]).to eq(name: '/', mounted: :yes)
@@ -196,8 +196,8 @@
   end
 
   it "supports AIX's paragraph based /etc/filesystems" do
-    pending 'This test only works on AIX' unless Facter.value(:osfamily) == 'AIX'
-    allow(Facter).to receive(:value).with(:osfamily).and_return 'AIX'
+    pending 'This test only works on AIX' unless Facter.value('os.family') == 'AIX'
+    allow(Facter).to receive(:value).with('os.family').and_return 'AIX'
     allow(described_class).to receive(:default_target).and_return my_fixture('aix.filesystems')
     allow(described_class).to receive(:mountcmd).and_return File.read(my_fixture('aix.mount'))
     instances = described_class.instances
@@ -220,8 +220,8 @@
       end
 
       before :each do
-        allow(Facter).to receive(:value).with(:osfamily).and_return platform
-        if Facter[:osfamily] == 'Solaris'
+        allow(Facter).to receive(:value).with('os.family').and_return platform
+        if Facter.value('os.family') == 'Solaris'
           platform == 'solaris' ||
             skip("We need to double the operatingsystem fact at load time, but can't")
         else
@@ -244,7 +244,7 @@
       end
 
       # Following mountpoint are present in all fstabs/mountoutputs
-      describe 'on other platforms than Solaris', if: Facter.value(:osfamily) != 'Solaris' do
+      describe 'on other platforms than Solaris', if: Facter.value('os.family') != 'Solaris' do
         it 'includes unmounted resources' do
           expect(instances).to include(name: '/', ensure: :mounted)
         end
@@ -275,8 +275,8 @@
 
       before :each do
         allow(Facter).to receive(:value).with(:kernel).and_return 'Linux'
-        allow(Facter).to receive(:value).with(:operatingsystem).and_return 'RedHat'
-        allow(Facter).to receive(:value).with(:osfamily).and_return 'RedHat'
+        allow(Facter).to receive(:value).with('os.name').and_return 'RedHat'
+        allow(Facter).to receive(:value).with('os.family').and_return 'RedHat'
         begin
           mount = my_fixture('linux.mount')
           allow(described_class).to receive(:mountcmd).and_return File.read(mount)
@@ -331,11 +331,11 @@
       end
 
       before :each do
-        [:osfamily, :operatingsystem, :kernel].each do |fact|
+        ['os.family', 'os.name', :kernel].each do |fact|
           allow(Facter).to receive(:value).with(fact).and_return platform
         end
 
-        if Facter[:osfamily] == 'Solaris'
+        if Facter.value('os.family') == 'Solaris'
           platform == 'solaris' ||
             skip("We need to double the operatingsystem fact at load time, but can't")
         else
@@ -357,7 +357,7 @@
         allow(described_class).to receive(:default_target).and_return fstab
       end
 
-      describe 'on other platforms than Solaris', if: Facter.value(:osfamily) != 'Solaris' do
+      describe 'on other platforms than Solaris', if: Facter.value('os.family') != 'Solaris' do
         it 'sets :ensure to :unmounted if found in fstab but not mounted' do
           described_class.prefetch(resource_hash)
           expect(res_unmounted.provider.get(:ensure)).to eq(:unmounted)

spec/unit/provider/mount_spec.rb

@@ -75,7 +75,7 @@
           allow(mounter).to receive(:info)
           allow(mounter).to receive(:options).and_return('ro')
           allow(resource).to receive(:[]).with(:remounts).and_return(:true)
-          expect(Facter).to receive(:value).with(:operatingsystem).and_return 'AIX'
+          expect(Facter).to receive(:value).with('os.name').and_return 'AIX'
           expect(mounter).to receive(:mountcmd).with('-o', 'ro,remount', name)
           mounter.remount
         end
@@ -93,7 +93,7 @@
       allow(mounter).to receive(:info)
       allow(mounter).to receive(:options)
       allow(resource).to receive(:[]).with(:remounts).and_return(false)
-      expect(Facter).to receive(:value).with(:operatingsystem).and_return 'OpenBSD'
+      expect(Facter).to receive(:value).with('os.name').and_return 'OpenBSD'
       expect(mounter).to receive(:mountcmd).with('-o', 'update', name)
       mounter.remount
     end
@@ -102,7 +102,7 @@
       allow(mounter).to receive(:info)
       allow(mounter).to receive(:options)
       allow(resource).to receive(:[]).with(:remounts).and_return(false)
-      expect(Facter).to receive(:value).with(:operatingsystem).and_return 'AIX'
+      expect(Facter).to receive(:value).with('os.name').and_return 'AIX'
       expect(mounter).to receive(:mount)
       expect(mounter).to receive(:unmount)
       mounter.remount

spec/unit/type/mount_spec.rb

@@ -167,7 +167,7 @@ def mounted?
     describe 'for blockdevice' do
       before :each do
         # blockdevice is only used on Solaris
-        [:osfamily, :operatingsystem, :kernel].each do |fact|
+        ['os.family', 'os.name', :kernel].each do |fact|
           allow(Facter).to receive(:value).with(fact).and_return 'Solaris'
         end
       end
@@ -252,21 +252,21 @@ def mounted?
       end
 
       it 'supports - on Solaris' do
-        [:osfamily, :operatingsystem, :kernel].each do |fact|
+        ['os.family', 'os.name', :kernel].each do |fact|
           allow(Facter).to receive(:value).with(fact).and_return 'Solaris'
         end
         expect { described_class.new(name: '/foo', ensure: :present, pass: '-') }.not_to raise_error
       end
 
       it 'defaults to 0 on non Solaris' do
-        [:osfamily, :operatingsystem, :kernel].each do |fact|
+        ['os.family', 'os.name', :kernel].each do |fact|
           allow(Facter).to receive(:value).with(fact).and_return 'HP-UX'
         end
         expect(described_class.new(name: '/foo', ensure: :present)[:pass]).to eq(0)
       end
 
       it 'defaults to - on Solaris' do
-        [:osfamily, :operatingsystem, :kernel].each do |fact|
+        ['os.family', 'os.name', :kernel].each do |fact|
           allow(Facter).to receive(:value).with(fact).and_return 'Solaris'
         end
         expect(described_class.new(name: '/foo', ensure: :present)[:pass]).to eq('-')
@@ -283,11 +283,11 @@ def mounted?
       end
 
       # Unfortunately the operatingsystem is evaluatet at load time so I am unable to double operatingsystem
-      it 'supports 2 as a value for dump on FreeBSD', if: Facter.value(:operatingsystem) == 'FreeBSD' do
+      it 'supports 2 as a value for dump on FreeBSD', if: Facter.value('os.name') == 'FreeBSD' do
         expect { described_class.new(name: '/foo', ensure: :present, dump: '2') }.not_to raise_error
       end
 
-      it 'does not support 2 as a value for dump when not on FreeBSD', if: Facter.value(:operatingsystem) != 'FreeBSD' do
+      it 'does not support 2 as a value for dump when not on FreeBSD', if: Facter.value('os.name') != 'FreeBSD' do
         expect { described_class.new(name: '/foo', ensure: :present, dump: '2') }.to raise_error Puppet::Error, %r{Invalid value}
       end