Fixing #751 -- the interface providers now have basic tests, at least to verify that prefetching and listing works. I think these resource types need to be largely rewritten, though, and they currently have no relationship to ifconfig, which seems strange.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@2747 980ebf18-57e1-0310-9a29-db15c13687c0

4 files changed, 172 insertions, 105 deletions

diff --git a/lib/puppet/provider/interface/redhat.rb b/lib/puppet/provider/interface/redhat.rb
index 8d7d09ca3..1f269f0ca 100644
--- a/lib/puppet/provider/interface/redhat.rb
+++ b/lib/puppet/provider/interface/redhat.rb
@@ -2,6 +2,8 @@ require 'puppet/provider/parsedfile'
 require 'erb'
 
 Puppet::Type.type(:interface).provide(:redhat) do
+	INTERFACE_DIR = "/etc/sysconfig/network-scripts"
+    confine :exists => INTERFACE_DIR
     defaultfor :operatingsystem => [:fedora, :centos, :redhat]
 
     # Create the setter/gettor methods to match the model.
@@ -32,14 +34,32 @@ LOOPBACKDUMMY
 	# maximum number of aliases per interface
 	MAX_ALIASES_PER_IFACE = 10
 
-	INTERFACE_DIR = "/etc/sysconfig/network-scripts"
 
 	@@dummies = []
 	@@aliases = Hash.new { |hash, key| hash[key] = [] }
 
+	# calculate which dummy interfaces are currently already in
+	# use prior to needing to call self.next_dummy later on.
+	def self.instances
+		# parse all of the config files at once
+		Dir.glob("%s/ifcfg-*" % INTERFACE_DIR).collect do |file|
+
+			record = parse(file)
+
+			# store the existing dummy interfaces
+			if record[:interface_type] == :dummy
+				@@dummies << record[:ifnum] unless @@dummies.include?(record[:ifnum])
+			end
+
+			if record[:interface_type] == :alias
+				@@aliases[record[:interface]] << record[:ifnum]
+			end
+            new(record)
+		end
+	end
+
 	# return the next avaliable dummy interface number, in the case where
 	# ifnum is not manually specified
-
 	def self.next_dummy
 		MAX_DUMMIES.times do |i|
 			unless @@dummies.include?(i.to_s)
@@ -60,42 +80,101 @@ LOOPBACKDUMMY
 		end
 	end
 
-	# calculate which dummy interfaces are currently already in
-	# use prior to needing to call self.next_dummy later on.
-	def self.prefetch
-		# parse all of the config files at once
-		Dir.glob("%s/ifcfg-*" % INTERFACE_DIR).each do |file|
+    # base the ifnum, for dummy / loopback interface in linux
+    # on the last octect of the IP address
 
-			record = parse(file)
+    # Parse the existing file.
+    def self.parse(file)
 
-			# store the existing dummy interfaces
-			if record[:interface_type] == :dummy
-				@@dummies << record[:ifnum]
-			end
+        opts = {}
+        return opts unless FileTest.exists?(file)
 
-			if record[:interface_type] == :alias
-				@@aliases[record[:interface]] << record[:ifnum]
-			end
+        File.open(file) do |f|
+            f.readlines.each do |line|
+                if line =~ /^(\w+)=(.+)$/
+                    opts[$1.downcase.intern] = $2
+                end
+            end
+        end
+
+        # figure out the "real" device information
+        case opts[:device]
+        when /:/:
+            if opts[:device].include?(":")
+                opts[:interface], opts[:ifnum] = opts[:device].split(":")
+            end
+
+            opts[:interface_type] = :alias
+        when /^dummy/:
+            opts[:interface_type] = :loopback
+			opts[:interface] = "dummy"
+
+			# take the number of the dummy interface, as this is used
+			# when working out whether to call next_dummy when dynamically
+			# creating these
+			opts[:ifnum] = opts[:device].sub("dummy",'')
+
+			@@dummies << opts[:ifnum].to_s unless @@dummies.include?(opts[:ifnum].to_s)
+        else
+			opts[:interface_type] = :normal
+			opts[:interface] = opts[:device]
+        end
+
+		# translate whether we come up on boot to true/false
+		case opts[:onboot].downcase
+		when "yes":
+			opts[:onboot] = :true
+		when "no":
+			opts[:onboot] = :false
+		else
+			# this case should never happen, but just in case
+			opts[:onboot] = false
 		end
-	end
+
+
+        # Remove any attributes we don't want.  These would be
+        # pretty easy to support.
+        [:bootproto, :broadcast, :netmask, :device].each do |opt|
+            if opts.include?(opt)
+                opts.delete(opt)
+            end
+        end
+
+        if opts.include?(:ipaddr)
+            opts[:name] = opts[:ipaddr]
+            opts.delete(:ipaddr)
+        end
+
+        return opts
+
+    end
+
+    # Prefetch our interface list, yo.
+    def self.prefetch(resources)
+        instances.each do |prov|
+            if resource = resources[prov.name]
+                resource.provider = prov
+            end
+        end
+    end
 
     def create
-        @model.class.validproperties.each do |property|
-            if value = @model.should(property)
+        @resource.class.validproperties.each do |property|
+            if value = @resource.should(property)
                 @property_hash[property] = value
             end
         end
-        @property_hash[:name] = @model.name
+        @property_hash[:name] = @resource.name
 
-        return (@model.class.name.to_s + "_created").intern
+        return (@resource.class.name.to_s + "_created").intern
     end
 
     def destroy
-        File.unlink(@model[:target])
+        File.unlink(@resource[:target])
     end
 
     def exists?
-        FileTest.exists?(@model[:target])
+        FileTest.exists?(@resource[:target])
     end
 
     # generate the content for the interface file, so this is dependent
@@ -105,7 +184,7 @@ LOOPBACKDUMMY
 	def generate
 		# choose which template to use for the interface file, based on
 		# the interface type
-        case @model.should(:interface_type)
+        case @resource.should(:interface_type)
         when :loopback
 			return LOOPBACK_TEMPLATE.result(binding)
         when :alias
@@ -117,20 +196,20 @@ LOOPBACKDUMMY
 	# This defaults to INTERFACE_DIR/ifcfg-<namevar>, but can have a
 	# more symbolic name by setting interface_desc in the type. 
     def file_path
-		@model[:interface_desc] ||= @model[:name]
-       	return File.join(INTERFACE_DIR, "ifcfg-" + @model[:interface_desc])
+		@resource[:interface_desc] ||= @resource[:name]
+       	return File.join(INTERFACE_DIR, "ifcfg-" + @resource[:interface_desc])
 
     end
 
 	# create the device name, so this based on the IP, and interface + type
 	def device
-		case @model.should(:interface_type)
+		case @resource.should(:interface_type)
 		when :loopback
 			@property_hash[:ifnum] ||= self.class.next_dummy
         	return "dummy" + @property_hash[:ifnum]
 		when :alias
-			@property_hash[:ifnum] ||= self.class.next_alias(@model[:interface])
-        	return @model[:interface] + ":" + @property_hash[:ifnum]
+			@property_hash[:ifnum] ||= self.class.next_alias(@resource[:interface])
+        	return @resource[:interface] + ":" + @property_hash[:ifnum]
 		end
     end
 
@@ -151,7 +230,7 @@ LOOPBACKDUMMY
     # Write the new file out.
     def flush
         # Don't flush to disk if we're removing the config.
-        return if @model.should(:ensure) == :absent
+        return if @resource.should(:ensure) == :absent
 
         @property_hash.each do |name, val|
             if val == :absent
@@ -159,82 +238,13 @@ LOOPBACKDUMMY
             end
         end
 
-        File.open(@model[:target], "w") do |f|
+        File.open(@resource[:target], "w") do |f|
             f.puts generate()
         end
     end
 
-    # base the ifnum, for dummy / loopback interface in linux
-    # on the last octect of the IP address
-
-    # Parse the existing file.
-    def self.parse(file)
-
-        opts = {}
-        return opts unless FileTest.exists?(file)
-
-        File.open(file) do |f|
-            f.readlines.each do |line|
-                if line =~ /^(\w+)=(.+)$/
-                    opts[$1.downcase.intern] = $2
-                end
-            end
-        end
-
-        # figure out the "real" device information
-        case opts[:device]
-        when /:/:
-            if opts[:device].include?(":")
-                opts[:interface], opts[:ifnum] = opts[:device].split(":")
-            end
-
-            opts[:interface_type] = :alias
-        when /^dummy/:
-            opts[:interface_type] = :loopback
-			opts[:interface] = "dummy"
-
-			# take the number of the dummy interface, as this is used
-			# when working out whether to call next_dummy when dynamically
-			# creating these
-			opts[:ifnum] = opts[:device].sub("dummy",'')
-
-			@@dummies << opts[:ifnum].to_s
-        else
-			opts[:interface_type] = :normal
-			opts[:interface] = opts[:device]
-        end
-
-		# translate whether we come up on boot to true/false
-		case opts[:onboot].downcase
-		when "yes":
-			opts[:onboot] = :true
-		when "no":
-			opts[:onboot] = :false
-		else
-			# this case should never happen, but just in case
-			opts[:onboot] = false
-		end
-
-
-        # Remove any attributes we don't want.  These would be
-        # pretty easy to support.
-        [:bootproto, :broadcast, :netmask, :device].each do |opt|
-            if opts.include?(opt)
-                opts.delete(opt)
-            end
-        end
-
-        if opts.include?(:ipaddr)
-            opts[:name] = opts[:ipaddr]
-            opts.delete(:ipaddr)
-        end
-
-        return opts
-
-    end
-
     def prefetch
-        @property_hash = self.class.parse(@model[:target])
+        @property_hash = self.class.parse(@resource[:target])
     end
 end
 
diff --git a/lib/puppet/provider/interface/sunos.rb b/lib/puppet/provider/interface/sunos.rb
index 0290aa209..abe9dd8c6 100644
--- a/lib/puppet/provider/interface/sunos.rb
+++ b/lib/puppet/provider/interface/sunos.rb
@@ -12,10 +12,7 @@ Puppet::Type.type(:interface).provide(:sunos,
     # Two types of lines:
     #   the first line does not start with 'addif'
     #   the rest do
-
-
 	record_line :sunos, :fields => %w{interface_type name ifopts onboot}, :rts => true, :absent => "", :block_eval => :instance do
-
         # Parse our interface line
         def process(line)
             details = {:ensure => :present}
@@ -80,6 +77,14 @@ Puppet::Type.type(:interface).provide(:sunos,
 		%{}
 	end
 
+    # Get a list of interface instances.
+    def self.instances
+		Dir.glob("/etc/hostname.*").collect do |file|
+            # We really only expect one record from each file
+            parse(file).shift
+        end.collect { |record| new(record) }
+    end
+
 	def self.match(hash)
 		# see if we can match the has against an existing object
 		if model.find { |obj| obj.value(:name) == hash[:name] }
@@ -89,10 +94,22 @@ Puppet::Type.type(:interface).provide(:sunos,
 		end	
 	end
 
+    # Prefetch our interface list, yo.
+    def self.prefetch(resources)
+        instances.each do |prov|
+            if resource = resources[prov.name]
+                resource.provider = prov
+            end
+        end
+    end
+
     # Where should the file be written out?  Can be overridden by setting
     # :target in the model.
     def file_path
-       	return File.join("/etc", "hostname." + @model[:interface])
+        unless @resource[:interface]
+            raise ArgumentError, "You must provide the interface name on Solaris"
+        end
+       	return File.join("/etc", "hostname." + @resource[:interface])
     end
 end
 
diff --git a/lib/puppet/type/interface.rb b/lib/puppet/type/interface.rb
index 206776260..e0ad34761 100644
--- a/lib/puppet/type/interface.rb
+++ b/lib/puppet/type/interface.rb
@@ -50,7 +50,7 @@ Puppet::Type.newtype(:interface) do
     newparam(:target) do
         desc "The path to the file this resource creates."
 
-        defaultto { @parent.provider.file_path }
+        defaultto { @resource.provider.file_path }
     end
 end
 
diff --git a/test/ral/types/interface.rb b/test/ral/types/interface.rb
new file mode 100755
index 000000000..66425deb2
--- /dev/null
+++ b/test/ral/types/interface.rb
@@ -0,0 +1,40 @@
+#!/usr/bin/env ruby
+
+$:.unshift("../../lib") if __FILE__ =~ /\.rb$/
+
+require 'puppettest'
+require 'mocha'
+
+class TestInterfaceType < PuppetTest::TestCase
+	include PuppetTest
+
+    def setup
+        super
+        @type = Puppet::Type.type(:interface)
+    end
+
+    def test_prefetch
+        interface = @type.create(:name => "127.0.0.1", :interface => "lo0", :check => :all)
+
+        @type.suitableprovider.each do |provider|
+            assert_nothing_raised("Could not prefetch interfaces from %s provider" % provider.name) do
+                provider.prefetch("eth0" => interface)
+            end
+        end
+    end
+
+    def test_instances
+        @type.suitableprovider.each do |provider|
+            list = nil
+            assert_nothing_raised("Could not get instance list from %s" % provider.name) do
+                list = provider.instances
+            end
+            assert(list.length > 0, "Did not get any instances from %s" % provider.name)
+            list.each do |interface|
+                assert_instance_of(provider, interface, "%s provider returned something other than a provider instance" % provider.name)
+            end
+        end
+    end
+end
+
+# $Id$