Doing what I can to fix #1128, but just in preparation for removing 'interface'.

This type needs to be started again from scratch, and I'm not
going to do so for 0.24.5.

In particular, the model for red hat and sunos need to match --
they should both use the device name as the actual name.

Signed-off-by: Luke Kanies <luke@madstop.com>

3 files changed, 82 insertions, 27 deletions

diff --git a/lib/puppet/provider/interface/redhat.rb b/lib/puppet/provider/interface/redhat.rb
index 4a9fcb491..71f2a6327 100644
--- a/lib/puppet/provider/interface/redhat.rb
+++ b/lib/puppet/provider/interface/redhat.rb
@@ -57,14 +57,14 @@ LOOPBACKDUMMY
     def self.instances
         # parse all of the config files at once
         Dir.glob("%s/ifcfg-*" % INTERFACE_DIR).collect do |file|
-            record = parse(file)
+            instance = parse(file)
 
             # store the existing dummy interfaces
-            @@dummies << record[:ifnum] if (record[:interface_type] == :dummy and ! @@dummies.include?(record[:ifnum]))
+            @@dummies << instance.ifnum if (instance.interface_type == :dummy and ! @@dummies.include?(instance.ifnum))
 
-            @@aliases[record[:interface]] << record[:ifnum] if record[:interface_type] == :alias
+            @@aliases[instance.interface] << instance.ifnum if instance.interface_type == :alias
 
-            new(record)
+            instance
         end
     end
 
@@ -95,12 +95,18 @@ LOOPBACKDUMMY
 
     # Parse the existing file.
     def self.parse(file)
-        instance = new()
+        unless file =~ /-([^-]+)$/
+            Puppet.warning "Could not extract interface name from %s; skipping" % file
+            return nil
+        end
+        name = $1
+        instance = new(:name => name)
         return instance unless FileTest.exist?(file)
 
         File.readlines(file).each do |line|
             if line =~ /^(\w+)=(.+)$/
-                instance.send($1.downcase + "=", $2)
+                method = $1.downcase + "="
+                instance.send(method, $2) if instance.respond_to?(method)
             end
         end
 
@@ -197,11 +203,16 @@ LOOPBACKDUMMY
         end
     end
 
+    # LAK:NOTE This method is why this resource type has been disabled.
+    # The model is ridiculous -- the device name should be the interface
+    # name.  We're 90% of the way toward making this possible, but I'm not
+    # taking the time to fix it.
     # Set the name to our ip address.
     def ipaddr=(value)
         @property_hash[:name] = value
     end
 
+
     # whether the device is to be brought up on boot or not. converts
     # the true / false of the type, into yes / no values respectively
     # writing out the ifcfg-* files
diff --git a/spec/integration/provider/interface/redhat.rb b/spec/integration/provider/interface/redhat.rb
new file mode 100755
index 000000000..92b372dfb
--- /dev/null
+++ b/spec/integration/provider/interface/redhat.rb
@@ -0,0 +1,41 @@
+#!/usr/bin/env ruby
+
+# Find and load the spec file.
+Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/spec_helper.rb") }
+
+provider_class = Puppet::Type.type(:interface).provider(:redhat)
+
+describe provider_class do
+    describe "when returning instances" do
+        before do
+            Dir.stubs(:glob).with("/etc/sysconfig/network-scripts/ifcfg-*").returns(%w{/etc/sysconfig/network-scripts/ifcfg-eth0
+                /etc/sysconfig/network-scripts/ifcfg-lo})
+            FileTest.stubs(:exist?).returns true
+            File.stubs(:readlines).with("/etc/sysconfig/network-scripts/ifcfg-eth0").returns %w{DEVICE=eth0\n BOOTPROTO=dhcp\n ONBOOT=yes\n TYPE=Ethernet\n
+                USERCTL=yes\n PEERDNS=yes\n IPV6INIT=no\n }
+            File.stubs(:readlines).with("/etc/sysconfig/network-scripts/ifcfg-lo").returns %w{DEVICE=lo\n IPADDR=127.0.0.1\n NETMASK=255.0.0.0\n NETWORK=127.0.0.0\n
+                # If you're having problems with gated making 127.0.0.0/8 a martian,\n
+                # you can change this to something else (255.255.255.255, for example)\n
+                BROADCAST=127.255.255.255\n ONBOOT=yes\n NAME=loopback\n }
+        end
+
+        it "should succeed" do
+            instances = nil
+            lambda { instances = provider_class.instances }.should_not raise_error
+        end
+
+        it "should return provider instances for each file" do
+            provider_class.instances[0].should be_instance_of(provider_class)
+        end
+
+        it "should return provider instances for each file" do
+            provider_class.instances.length.should == 2
+        end
+
+        it "should set the name to the interface name extracted from the file" do
+            instances = provider_class.instances
+            instances[0].name.should == "eth0"
+            instances[1].name.should == "lo"
+        end
+    end
+end
diff --git a/spec/unit/provider/interface/redhat.rb b/spec/unit/provider/interface/redhat.rb
index 515c9f775..70830aab5 100755
--- a/spec/unit/provider/interface/redhat.rb
+++ b/spec/unit/provider/interface/redhat.rb
@@ -64,35 +64,38 @@ describe provider_class, "when determining the file path" do
     end
 end
 
-describe provider_class, "when returning instances" do
-    it "should consider each file in the network-scripts directory an interface instance" do
-        Dir.expects(:glob).with("/etc/sysconfig/network-scripts/ifcfg-*").returns(%w{one two})
-        one = {:name => "one"}
-        two = {:name => "two"}
-        Puppet::Type::Interface::ProviderRedhat.expects(:parse).with("one").returns(one)
-        Puppet::Type::Interface::ProviderRedhat.expects(:parse).with("two").returns(two)
-        Puppet::Type::Interface::ProviderRedhat.expects(:new).with(one).returns(:one)
-        Puppet::Type::Interface::ProviderRedhat.expects(:new).with(two).returns(:two)
-        Puppet::Type::Interface::ProviderRedhat.instances.should == [:one, :two]
+describe provider_class, "when parsing" do
+    before do
+        @provider = Puppet::Type::Interface::ProviderRedhat.new
+    end
+
+    it "should return nil if the file name does not include an interface" do
+        Puppet::Type::Interface::ProviderRedhat.parse("/my/file").should be_nil
     end
-end
 
-describe provider_class, "when parsing" do
     it "should return an unmodified provider if the file does not exist" do
-        FileTest.expects(:exist?).with("/my/file").returns(false)
+        FileTest.expects(:exist?).with("/my/ifcfg-eth0").returns(false)
         provider = mock 'provider'
         Puppet::Type::Interface::ProviderRedhat.expects(:new).returns(provider)
-        Puppet::Type::Interface::ProviderRedhat.parse("/my/file").should equal(provider)
+        Puppet::Type::Interface::ProviderRedhat.parse("/my/ifcfg-eth0").should equal(provider)
     end
 
     it "should set each attribute in the file on the provider" do
-        FileTest.expects(:exist?).with("/my/file").returns(true)
-        File.expects(:readlines).with("/my/file").returns(%w{one=two three=four})
-        provider = mock 'provider'
-        Puppet::Type::Interface::ProviderRedhat.expects(:new).returns(provider)
-        provider.expects(:one=).with('two')
-        provider.expects(:three=).with('four')
-        Puppet::Type::Interface::ProviderRedhat.parse("/my/file").should equal(provider)
+        FileTest.expects(:exist?).with("/my/ifcfg-eth0").returns(true)
+        File.expects(:readlines).with("/my/ifcfg-eth0").returns(%w{DEVICE=foo ONBOOT=yes})
+        Puppet::Type::Interface::ProviderRedhat.expects(:new).returns(@provider)
+        @provider.expects(:device=).with('foo')
+        @provider.expects(:onboot=).with('yes')
+        Puppet::Type::Interface::ProviderRedhat.parse("/my/ifcfg-eth0").should equal(@provider)
+    end
+
+    it "should not try to assign parameters that the provider does not support" do
+        Puppet::Type::Interface::ProviderRedhat.expects(:new).returns @provider
+
+        file = "/my/file/ifcfg-eth0"
+        FileTest.stubs(:exist?).returns true
+        File.expects(:readlines).with(file).returns %w{BOOTPROTO=foo\n DEVICE=eth0\n}
+        lambda { Puppet::Type::Interface::ProviderRedhat.parse(file) }.should_not raise_error
     end
 end