Fixing #655 -- Solaris zones are again fully functional, from what I can tell

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

2 files changed, 119 insertions, 175 deletions

diff --git a/lib/puppet/type/zone.rb b/lib/puppet/type/zone.rb
index c38b212c0..54220a0fc 100644
--- a/lib/puppet/type/zone.rb
+++ b/lib/puppet/type/zone.rb
@@ -20,7 +20,7 @@ Puppet::Type.newtype(:zone) do
 
             unless current_value.is_a? Symbol
                 if current_value.is_a? Array
-                    list += current_vlue
+                    list += current_value
                 else
                     if current_value
                         list << current_value
@@ -41,6 +41,7 @@ Puppet::Type.newtype(:zone) do
 
             rms = []
             adds = []
+
             # Collect the modifications to make
             list.sort.uniq.collect do |obj|
                 # Skip objectories that are configured and should be
@@ -53,6 +54,7 @@ Puppet::Type.newtype(:zone) do
                 end
             end
 
+
             # And then perform all of the removals before any of the adds.
             (rms.collect { |o| rm(o) } + adds.collect { |o| add(o) }).join("\n")
         end
@@ -74,7 +76,14 @@ Puppet::Type.newtype(:zone) do
             only then can be ``running``.  Note also that ``halt`` is currently
             used to stop zones."
 
-        @properties = {}
+        @states = {}
+
+        def self.alias_state(values)
+            @state_aliases ||= {}
+            values.each do |nick, name|
+                @state_aliases[nick] = name
+            end
+        end
 
         def self.newvalue(name, hash)
             if @parametervalues.is_a? Hash
@@ -83,29 +92,39 @@ Puppet::Type.newtype(:zone) do
 
             @parametervalues << name
 
-            @properties[name] = hash
+            @states[name] = hash
             hash[:name] = name
         end
 
+        def self.state_name(name)
+            if other = @state_aliases[name]
+                other
+            else
+                name
+            end
+        end
+
         newvalue :absent, :down => :destroy
         newvalue :configured, :up => :configure, :down => :uninstall
         newvalue :installed, :up => :install, :down => :stop
         newvalue :running, :up => :start
 
+        alias_state :incomplete => :installed, :ready => :installed, :shutting_down => :running
+
         defaultto :running
 
-        def self.valueindex(value)
-            @parametervalues.index(value)
+        def self.state_index(value)
+            @parametervalues.index(state_name(value))
         end
 
         # Return all of the states between two listed values, exclusive
         # of the first item.
-        def self.valueslice(first, second)
+        def self.state_sequence(first, second)
             findex = sindex = nil
-            unless findex = @parametervalues.index(first)
+            unless findex = @parametervalues.index(state_name(first))
                 raise ArgumentError, "'%s' is not a valid zone state" % first
             end
-            unless sindex = @parametervalues.index(second)
+            unless sindex = @parametervalues.index(state_name(second))
                 raise ArgumentError, "'%s' is not a valid zone state" % first
             end
             list = nil
@@ -114,11 +133,11 @@ Puppet::Type.newtype(:zone) do
             # the range op twice.
             if findex > sindex
                 list = @parametervalues[sindex..findex].collect do |name|
-                    @properties[name]
+                    @states[name]
                 end.reverse
             else
                 list = @parametervalues[findex..sindex].collect do |name|
-                    @properties[name]
+                    @states[name]
                 end
             end
 
@@ -126,20 +145,24 @@ Puppet::Type.newtype(:zone) do
             list[1..-1]
         end
 
+        def retrieve
+            provider.properties[:ensure]
+        end
+
         def sync
             method = nil
             if up?
-                dir = :up
+                direction = :up
             else
-                dir = :down
+                direction = :down
             end
 
             # We need to get the state we're currently in and just call
             # everything between it and us.
-            properties.each do |prop|
-                if method = prop[dir]
+            self.class.state_sequence(self.retrieve, self.should).each do |state|
+                if method = state[direction]
                     warned = false
-                    while @resource.processing?
+                    while provider.processing?
                         unless warned
                             info "Waiting for zone to finish processing"
                             warned = true
@@ -149,7 +172,7 @@ Puppet::Type.newtype(:zone) do
                     provider.send(method)
                 else
                     raise Puppet::DevError, "Cannot move %s from %s" %
-                        [dir, st[:name]]
+                        [direction, st[:name]]
                 end
             end
 
@@ -159,7 +182,7 @@ Puppet::Type.newtype(:zone) do
         # Are we moving up the property tree?
         def up?
             current_value = self.retrieve
-            self.class.valueindex(current_value) < self.class.valueindex(self.should)
+            self.class.state_index(current_value) < self.class.state_index(self.should)
         end
     end
 
@@ -196,7 +219,7 @@ Puppet::Type.newtype(:zone) do
             end
         end
 
-        # Add a directory to our list of inherited directories.
+        # Add an interface.
         def add(str)
             interface, ip = ipsplit(str)
             "add net
@@ -212,6 +235,7 @@ end
             return interface, address
         end
 
+        # Remove an interface.
         def rm(str)
             interface, ip = ipsplit(str)
             # Reality seems to disagree with the documentation here; the docs
@@ -256,7 +280,7 @@ end
 
         validate do |value|
             unless value =~ /^\//
-                raise ArgumentError, "The zone base must be fully qualified"
+                raise ArgumentError, "Inherited filesystems must be fully qualified"
             end
         end
 
@@ -347,45 +371,11 @@ end
         end
     end
 
-    # Perform all of our configuration steps.
-    def configure
-        # If the thing is entirely absent, then we need to create the config.
-        str = %{create -b
-set zonepath=%s
-} % self[:path]
-
-        # Then perform all of our configuration steps.
-        properties().each do |property|
-            if property.is_a? ZoneConfigProperty and ! property.insync?
-                str += property.configtext + "\n"
-            end
-        end
-
-        str += "commit\n"
-        provider.setconfig(str)
-    end
-
-    # We need a way to test whether a zone is in process.  Our 'ensure'
-    # property models the static states, but we need to handle the temporary ones.
-    def processing?
-        if hash = provider.statushash()
-            case hash[:ensure]
-            when "incomplete", "ready", "shutting_down"
-                true
-            else
-                false
-            end
-        else
-            false
-        end
-    end
-
     def retrieve
-        if hash = provider.statushash()
-            prophash = setstatus(hash)
-
-            # Now retrieve the configuration itself and set appropriately.
-            return config2status(provider.getconfig(), prophash)
+        provider.flush
+        if hash = provider.properties() and hash[:ensure] != :absent
+            result = setstatus(hash)
+            result
         else
             return currentpropvalues(:absent)
         end
@@ -398,43 +388,16 @@ set zonepath=%s
             next if param == :name
             case self.class.attrtype(param)
             when :property:
-                prophash[self.property(param)] = value
+                # Only try to provide values for the properties we're managing
+                if prop = self.property(param)
+                    prophash[prop] = value
+                end
             else
                 self[param] = value
             end
         end
         return prophash
     end
-
-    private
-    # Turn the results of getconfig into status information.
-    def config2status(config, prophash)
-        config.each do |name, value|
-            case name
-            when :autoboot:
-                prophash[self.property(:autoboot)] = value.intern
-            when :zonepath:
-                # Nothing; this is set in the zoneadm list command
-            when :pool:
-                prophash[self.property(:pool)] = value
-            when :shares:
-                prophash[self.property(:shares)] = value
-            when "inherit-pkg-dir":
-                dirs = value.collect do |hash|
-                    hash[:dir]
-                end
-
-                prophash[self.property(:inherit)] = dirs
-            when "net":
-                vals = value.collect do |hash|
-                    "%s:%s" % [hash[:physical], hash[:address]]
-                end
-                prophash[self.proeprty(:ip)] = vals
-            end
-        end
-        
-        return prophash
-    end
 end
 
 # $Id$
diff --git a/test/ral/types/zone.rb b/test/ral/types/zone.rb
index abb042a04..d68306201 100755
--- a/test/ral/types/zone.rb
+++ b/test/ral/types/zone.rb
@@ -1,4 +1,3 @@
-
 #!/usr/bin/env ruby
 
 $:.unshift("../../lib") if __FILE__ =~ /\.rb$/
@@ -6,42 +5,14 @@ $:.unshift("../../lib") if __FILE__ =~ /\.rb$/
 require 'puppettest'
 require 'puppet/type/zone'
 
-class TestZone < Test::Unit::TestCase
-	include PuppetTest
-
-    def test_nothing
-    end
-
-    # Zones can only be tested on solaris.
-    if Facter["operatingsystem"].value == "Solaris"
+class TestZone < PuppetTest::TestCase
+    confine "Zones are only functional on Solaris" => (Facter["operatingsystem"].value == "Solaris")
 
     def setup
         super
         @@zones = []
     end
 
-    def teardown
-        current = %x{zoneadm list -cp}.split("\n").inject({}) { |h, line|
-            ary = line.split(":")
-            h[ary[1]] = ary[2]
-            h
-        }
-
-        Puppet::Type.type(:zone).clear
-
-        # Get rid of any lingering zones
-        @@zones.each do |zone|
-            next unless current.include? zone
-
-            obj = Puppet::Type.type(:zone).create(:name => zone)
-            obj[:ensure] = :absent
-            assert_apply(obj)
-        end
-
-        # We can't delete the temp files until the zones are stopped and removed.
-        super
-    end
-
     def mkzone(name)
         zone = nil
 
@@ -62,10 +33,10 @@ class TestZone < Test::Unit::TestCase
         return zone
     end
 
-    def test_list
+    def test_instances
         list = nil
         assert_nothing_raised {
-            list = Puppet::Type.type(:zone).list
+            list = Puppet::Type.type(:zone).instances
         }
 
         assert(! list.empty?, "Got no zones back")
@@ -73,14 +44,14 @@ class TestZone < Test::Unit::TestCase
         assert(list.find { |z| z[:name] == "global" }, "Could not find global zone")
     end
 
-    def test_valueslice
+    def test_state_sequence
         zone = mkzone("slicetest")
 
         property = zone.property(:ensure)
 
         slice = nil
         assert_nothing_raised {
-            slice = property.class.valueslice(:absent, :installed).collect do |o|
+            slice = property.class.state_sequence(:absent, :installed).collect do |o|
                 o[:name]
             end
         }
@@ -89,7 +60,7 @@ class TestZone < Test::Unit::TestCase
         assert_equal([:configured, :installed], slice)
 
         assert_nothing_raised {
-            slice = property.class.valueslice(:running, :installed).collect do |o|
+            slice = property.class.state_sequence(:running, :installed).collect do |o|
                 o[:name]
             end
         }
@@ -107,11 +78,12 @@ class TestZone < Test::Unit::TestCase
 
         assert(property, "Did not get ensure property")
 
+        values = nil
         assert_nothing_raised {
-            zone.retrieve
+            values = zone.retrieve
         }
 
-        assert(! property.insync?, "Property is somehow in sync")
+        assert(! property.insync?(values[property]), "Property is somehow in sync")
 
         assert(property.up?, "Property incorrectly thinks it is not moving up")
 
@@ -128,7 +100,7 @@ class TestZone < Test::Unit::TestCase
 
         property = zone.property(:ensure)
         assert_nothing_raised {
-            property.class.valueslice(:absent, :running).each do |st|
+            property.class.state_sequence(:absent, :running).each do |st|
                 [:up, :down].each do |m|
                     if st[m]
                         methods << st[m]
@@ -161,10 +133,10 @@ class TestZone < Test::Unit::TestCase
         assert_equal("add inherit-pkg-dir\nset dir=/usr\nend", property.configtext,
             "Got incorrect config text")
 
-#        property.is = "/usr"
-#
-#        assert_equal("", property.configtext,
-#            "Got incorrect config text")
+        zone.provider.inherit = "/usr"
+
+        assert_equal("", property.configtext,
+            "Got incorrect config text")
 
         # Now we want multiple directories
         property.should = %w{/usr /sbin /lib}
@@ -180,28 +152,50 @@ end"
         assert_equal(text, property.configtext,
             "Got incorrect config text")
 
- #       property.is = %w{/usr /sbin /lib}
- #       property.should = %w{/usr /sbin}
+        zone.provider.inherit = %w{/usr /sbin /lib}
+        property.should = %w{/usr /sbin}
 
-#        text = "remove inherit-pkg-dir dir=/lib"
+        text = "remove inherit-pkg-dir dir=/lib"
 
-#        assert_equal(text, property.configtext,
-#            "Got incorrect config text")
+        assert_equal(text, property.configtext,
+            "Got incorrect config text")
     end
+end
+
+class TestZoneAsRoot < TestZone
+    confine "Not running Zone creation tests" => Puppet.features.root?
+    confine "Zones are only functional on Solaris" => (Facter["operatingsystem"].value == "Solaris")
 
-    if Puppet::Util::SUIDManager.uid == 0
+    def teardown
+        current = %x{zoneadm list -cp}.split("\n").inject({}) { |h, line|
+            ary = line.split(":")
+            h[ary[1]] = ary[2]
+            h
+        }
+
+        Puppet::Type.type(:zone).clear
+
+        # Get rid of any lingering zones
+        @@zones.each do |zone|
+            next unless current.include? zone
+
+            obj = Puppet::Type.type(:zone).create(:name => zone)
+            obj[:ensure] = :absent
+            assert_apply(obj)
+        end
+
+        # We can't delete the temp files until the zones are stopped and removed.
+        super
+    end
     # Make sure our ensure process actually works.
     def test_ensure_sync
         zone = mkzone("ensuretesting")
 
         zone[:ensure] = :configured
 
-        zone.retrieve
         assert_apply(zone)
 
-        zone.retrieve
-
-        assert(zone.insync?, "Zone is not insync")
+        assert(zone.insync?(zone.retrieve), "Zone is not insync")
     end
 
     def test_getconfig
@@ -242,25 +236,27 @@ end
 
         assert_equal(0, $?, "Did not successfully create zone")
 
-        #@@zones << "configtesting"
-
         hash = nil
         assert_nothing_raised {
             hash = zone.provider.send(:getconfig)
         }
 
-        # Now set the configuration
-        assert_nothing_raised { zone.send(:config2status, hash) }
+        zone[:check] = [:inherit, :autoboot]
+
+        values = nil
+        assert_nothing_raised("Could not retrieve zone values") do
+            values = zone.retrieve.inject({}) { |result, newvals| result[newvals[0].name] = newvals[1]; result }
+        end
 
         # And make sure it gets set correctly.
         assert_equal(%w{/sbin /usr /opt/csw /lib /platform}.sort,
-            zone.property(:inherit).retrieve.sort, "Inherited dirs did not get collected correctly."
+            values[:inherit].sort, "Inherited dirs did not get collected correctly."
         )
 
-        assert_equal(["#{interface}:#{ip}"], zone.is(:ip),
+        assert_equal(["#{interface}:#{ip}"], values[:ip],
             "IP addresses did not get collected correctly.")
 
-        assert_equal(:true, zone.property(:autoboot).retrieve,
+        assert_equal(:true, values[:autoboot],
             "Autoboot did not get collected correctly.")
     end
 
@@ -274,12 +270,9 @@ end
 
         zone[:ensure] = :configured
 
-        zone.retrieve
         assert_apply(zone)
 
-        zone.retrieve
-
-        assert(zone.insync?, "Zone is not insync")
+        assert(zone.insync?(zone.retrieve), "Zone is not insync")
 
         # Now add a new directory to inherit
         assert_nothing_raised {
@@ -287,9 +280,7 @@ end
         }
         assert_apply(zone)
 
-        zone.retrieve
-
-        assert(zone.insync?, "Zone is not insync")
+        assert(zone.insync?(zone.retrieve), "Zone is not insync")
 
         assert(%x{/usr/sbin/zonecfg -z #{zone[:name]} info} =~ /dir: \/sbin/,
             "sbin was not added")
@@ -300,9 +291,7 @@ end
         }
         assert_apply(zone)
 
-        zone.retrieve
-
-        assert(zone.insync?, "Zone is not insync")
+        assert(zone.insync?(zone.retrieve), "Zone is not insync")
 
         assert(%x{/usr/sbin/zonecfg -z #{zone[:name]} info} !~ /dir: \/sbin/,
             "sbin was not removed")
@@ -311,18 +300,15 @@ end
         # that the interface exists.
         zone[:ip] = "hme0:192.168.0.1"
 
-        zone.retrieve
-        assert(! zone.insync?, "Zone is marked as in sync")
+        assert(! zone.insync?(zone.retrieve), "Zone is marked as in sync")
 
         assert_apply(zone)
-        zone.retrieve
-        assert(zone.insync?, "Zone is not in sync")
+        assert(zone.insync?(zone.retrieve), "Zone is not in sync")
         assert(%x{/usr/sbin/zonecfg -z #{zone[:name]} info} =~ /192.168.0.1/,
             "ip was not added")
         zone[:ip] = ["hme1:192.168.0.2", "hme0:192.168.0.1"]
         assert_apply(zone)
-        zone.retrieve
-        assert(zone.insync?, "Zone is not in sync")
+        assert(zone.insync?(zone.retrieve), "Zone is not in sync")
         assert(%x{/usr/sbin/zonecfg -z #{zone[:name]} info} =~ /192.168.0.2/,
             "ip was not added")
         zone[:ip] = ["hme1:192.168.0.2"]
@@ -348,14 +334,11 @@ end
 
         zone[:ensure] = :configured
 
-        assert(! zone.insync?, "Zone is incorrectly in sync")
+        assert(! zone.insync?(zone.retrieve), "Zone is incorrectly in sync")
 
         assert_apply(zone)
 
-        assert_nothing_raised {
-            zone.retrieve
-        }
-        assert(zone.insync?, "Zone is incorrectly out of sync")
+        assert(zone.insync?(zone.retrieve), "Zone is incorrectly out of sync")
 
         zone[:ensure] = :absent
 
@@ -385,8 +368,9 @@ end
             [:unconfigure, :absent]
         ].each do |method, property|
             Puppet.info "Testing %s" % method
+            current_values = nil
             assert_nothing_raised {
-                zone.retrieve
+                current_values = zone.retrieve
             }
             assert_nothing_raised {
                 zone.provider.send(method)
@@ -418,14 +402,13 @@ end
             assert_nothing_raised {
                 zone[:ensure] = value
             }
-            assert(! zone.insync?, "Zone is incorrectly in sync")
+            assert(! zone.insync?(zone.retrieve), "Zone is incorrectly in sync")
 
             assert_apply(zone)
 
             assert_nothing_raised {
-                zone.retrieve
+                assert(zone.insync?(zone.retrieve), "Zone is incorrectly out of sync")
             }
-            assert(zone.insync?, "Zone is incorrectly out of sync")
         end
 
         currentvalues = zone.retrieve
@@ -433,8 +416,6 @@ end
         assert_equal(:absent, currentvalues[zone.property(:ensure)], 
                      "Zone is not absent")
     end
-    end
-    end
 end
 
 # $Id$