22259529 pkg_mediator fails on previously set implementation parameter
authorShawn Ferry <shawn.ferry@oracle.com>
Thu, 21 Apr 2016 12:46:25 -0400
changeset 5861 720358c56a5d
parent 5860 afd31ba91ee9
child 5862 ce31ce66da02
22259529 pkg_mediator fails on previously set implementation parameter
components/ruby/puppet-modules/oracle-solaris_providers/files/etc/puppet/modules/solaris_providers/lib/puppet/provider/pkg_mediator/solaris.rb
components/ruby/puppet-modules/oracle-solaris_providers/files/etc/puppet/modules/solaris_providers/lib/puppet/type/pkg_mediator.rb
components/ruby/puppet-modules/oracle-solaris_providers/files/etc/puppet/modules/solaris_providers/spec/fixtures/unit/provider/pkg/pkg_mediator/pkg_mediator.txt
components/ruby/puppet-modules/oracle-solaris_providers/files/etc/puppet/modules/solaris_providers/spec/fixtures/unit/provider/pkg/pkg_mediator/pkg_mediator_foo.txt
components/ruby/puppet-modules/oracle-solaris_providers/files/etc/puppet/modules/solaris_providers/spec/unit/provider/pkg/pkg_mediator_spec.rb
components/ruby/puppet-modules/oracle-solaris_providers/files/etc/puppet/modules/solaris_providers/spec/unit/type/pkg_mediator_spec.rb
components/ruby/puppet-modules/oracle-solaris_providers/oracle-solaris_providers-local.frag
--- a/components/ruby/puppet-modules/oracle-solaris_providers/files/etc/puppet/modules/solaris_providers/lib/puppet/provider/pkg_mediator/solaris.rb	Wed Apr 27 14:55:10 2016 -0700
+++ b/components/ruby/puppet-modules/oracle-solaris_providers/files/etc/puppet/modules/solaris_providers/lib/puppet/provider/pkg_mediator/solaris.rb	Thu Apr 21 12:46:25 2016 -0400
@@ -20,7 +20,7 @@
 #
 
 #
-# Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2013, 2016, Oracle and/or its affiliates. All rights reserved.
 #
 
 Puppet::Type.type(:pkg_mediator).provide(:pkg_mediator) do
@@ -28,23 +28,38 @@
     confine :operatingsystem => [:solaris]
     defaultfor :osfamily => :solaris, :kernelrelease => ['5.11', '5.12']
     commands :pkg => '/usr/bin/pkg'
+    mk_resource_methods
+
+    def initialize(value={})
+      super(value)
+      @property_flush = { :set_args => [], :unset_args => [] }
+    end
+
+    def self.parse_mediator(line)
+            name, _ver_src, version, _impl_src, impl, _impl_ver = line.split("\t")
+
+            # Neither Implementation nor Version are required
+            if impl.nil? || impl.empty?
+                impl = :None
+            end
+            if version.nil? || version.empty?
+                version = :None
+            end
+
+            return { :name => name,
+                :ensure => :present,
+                :implementation => impl,
+                :version => version }
+    end
+
+    def self.get_mediator(name)
+      return self.parse_mediator(pkg(:mediator, "-H", "-F", "tsv", name))
+    end
 
     def self.instances
         pkg(:mediator, "-H", "-F", "tsv").split("\n").collect do |line|
-            name, ver_src, version, impl_src, impl, impl_ver = line.split("\t")
-            
-            # Because implementation is an optional parameter, it may not be set.
-            # If the implementation is not set, that needs to be captured in 
-            # the output.
-            if not impl
-                impl = 'None'
-            end
-
-            new(:name => name,
-                :ensure => :present,
-                :implementation => impl,
-                :version => version)
-        end 
+            new(self.parse_mediator(line))
+        end
     end
 
     def self.prefetch(resources)
@@ -60,40 +75,62 @@
     end
 
     def exists?
-        # only compare against @resource if one is provided via manifests
-        if @property_hash[:ensure] == :present and @resource[:version] != nil
-            return (@property_hash[:ensure] == :present and \
-                   @property_hash[:version] == @resource[:version])
+        if @property_hash[:ensure] == :present and not
+          # Don't check values unless they are set in the manifest/resource
+          (  @resource[:version].nil? && @resource[:implementation].nil? )
+            # Both Version and Implementation must be expected or unspecified
+            return ((version == @resource[:version]) ||
+                     @resource[:version].nil? ) \
+                   &&
+                   ((implementation == @resource[:implementation] ||
+                     @resource[:implementation].nil?))
         end
         @property_hash[:ensure] == :present
     end
 
-    def version
-        @property_hash[:version]
-    end
+    def build_flags
+        if version == @resource[:version]
+          # Current State is Correct, noop
+        elsif @resource[:version] == :None && version != :None
+          # version is set and should not be
+          @property_flush[:unset_args] << '-V'
+        elsif ! @resource[:version].nil?
+          @property_flush[:set_args] << '-V' << @resource[:version]
+        end
 
-    def implementation
-        @property_hash[:implementation]
+        if implementation == @resource[:implementation]
+          # Current State is Correct, noop
+        elsif @resource[:implementation] == :None && implementation != :None
+          # implementation is set and should not be
+            @property_flush[:unset_args] << '-I'
+        elsif ! @resource[:implementation].nil?
+            @property_flush[:set_args] << '-I' << @resource[:implementation]
+        end
+
+        # If there is no pre-existing resource there will be no properties
+        # defined. If we got here and set_args is 0 we have unset_args
+        # otherwise there would be no changes
+        if @property_hash[:ensure].nil? && @property_flush[:set_args].size == 0
+          raise Puppet::ResourceError.new(
+            "Cannot unset absent mediator; use ensure => :absent instead of <property> => None")
+        end
     end
 
-    def build_flags
-        flags = []
-        if version = @resource[:version] and version != nil
-            flags << "-V" << @resource[:version]
-        end
-
-        if implementation = @resource[:implementation] and implementation != nil
-            flags << "-I" << @resource[:implementation]
-        end
-        flags
+    def flush
+        pkg("set-mediator", @property_flush[:set_args], @resource[:name]) if
+          @property_flush[:set_args].size > 0
+        pkg("unset-mediator", @property_flush[:unset_args], @resource[:name]) if
+          @property_flush[:unset_args].size > 0
+        @property_hash = self.class.get_mediator(resource[:name])
     end
 
     # required puppet functions
     def create
-        pkg("set-mediator", build_flags, @resource[:name])
+        build_flags
     end
 
     def destroy
-        pkg("unset-mediator", build_flags, @resource[:name])
+        # Absent mediators don't require any flag parsing, just remove them
+        pkg("unset-mediator", @resource[:name])
     end
 end
--- a/components/ruby/puppet-modules/oracle-solaris_providers/files/etc/puppet/modules/solaris_providers/lib/puppet/type/pkg_mediator.rb	Wed Apr 27 14:55:10 2016 -0700
+++ b/components/ruby/puppet-modules/oracle-solaris_providers/files/etc/puppet/modules/solaris_providers/lib/puppet/type/pkg_mediator.rb	Thu Apr 21 12:46:25 2016 -0400
@@ -35,9 +35,25 @@
 
     newproperty(:version) do
         desc "The version of the mediated interface to use"
+        newvalues(/none/io,/\A\d+(?:\.\d+){0,}\Z/)
+        munge do |value|
+          return value.downcase.capitalize.to_sym if value.match(/none/i)
+          value
+        end
     end
 
     newproperty(:implementation) do
         desc "The implementation of the mediated interface to use"
+        newvalues(/none/io,/\A[[:alnum:]]+\Z/,/\A[[:alnum:]][email protected](?:\d+(?:\.\d+){0,})\Z/)
+        munge do |value|
+          return value.downcase.capitalize.to_sym if value.match(/none/i)
+          value
+        end
     end
+
+    validate {
+      if self[:version] == :None && self[:implementation] == :None
+        fail("Version and Implementation cannot both be :None use ensure => :absent instead")
+      end
+    }
 end
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/components/ruby/puppet-modules/oracle-solaris_providers/files/etc/puppet/modules/solaris_providers/spec/fixtures/unit/provider/pkg/pkg_mediator/pkg_mediator.txt	Thu Apr 21 12:46:25 2016 -0400
@@ -0,0 +1,5 @@
+foo	local	1.1.1	local	[email protected]
+fooVer	local	1.1.1	local	
+fooImp	local		local	[email protected]
+gcc	system	5.3	system
+java	system	1.8	system
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/components/ruby/puppet-modules/oracle-solaris_providers/files/etc/puppet/modules/solaris_providers/spec/fixtures/unit/provider/pkg/pkg_mediator/pkg_mediator_foo.txt	Thu Apr 21 12:46:25 2016 -0400
@@ -0,0 +1,1 @@
+foo	local	1.1.1	local	[email protected]
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/components/ruby/puppet-modules/oracle-solaris_providers/files/etc/puppet/modules/solaris_providers/spec/unit/provider/pkg/pkg_mediator_spec.rb	Thu Apr 21 12:46:25 2016 -0400
@@ -0,0 +1,71 @@
+#!/usr/bin/env ruby
+
+require 'spec_helper'
+provider_class = Puppet::Type.type(:pkg_mediator).provider(:pkg_mediator)
+
+describe provider_class do
+
+  before(:each) do
+    # Create a mock resource
+    @resource = Puppet::Type.type(:pkg_mediator).new(
+      :name             => "foo",
+      :ensure           => :present,
+      :version          => "1.1.1",
+      :implementation   => "[email protected]")
+    @provider = provider_class.new(@resource)
+
+    FileTest.stubs(:file?).with('/usr/bin/pkg').returns true
+    FileTest.stubs(:executable?).with('/usr/bin/pkg').returns true
+  end
+
+  [ "version", "exists?", "implementation", "build_flags",
+    "create", "destroy" ].each do |method|
+    it "should have a #{method} method" do
+      expect(@provider.class.method_defined?(method)).to be true
+    end
+  end
+
+  describe ".instances" do
+    it "should have an instances method" do
+      expect(provider_class).to respond_to :instances
+    end
+
+    describe "should get a list of mediators" do
+      provider_class.expects(:pkg).with(:mediator, "-H", "-F", "tsv").returns
+      File.read(my_fixture('pkg_mediator.txt'))
+      instances = provider_class.instances.map { |p| {
+        :name             => p.get(:name),
+        :ensure           => p.get(:ensure),
+        :version          => p.get(:version),
+        :implementation   => p.get(:implementation) } }
+
+      it "with the expected number of instances" do
+        expect(instances.size).to eq(5)
+      end
+
+      it "with version and implementation" do
+        expect(instances[0]).to eq({
+          :name             => 'foo',
+          :ensure           => :present,
+          :version          => '1.1.1',
+          :implementation   => '[email protected]'})
+      end
+
+      it "with version only" do
+        expect(instances[1]).to eq({
+          :name             => 'fooVer',
+          :ensure           => :present,
+          :version          => '1.1.1',
+          :implementation   => :None})
+      end
+
+      it "with implementation only" do
+        expect(instances[2]).to eq({
+          :name             => 'fooImp',
+          :ensure           => :present,
+          :version          => :None,
+          :implementation   => '[email protected]'})
+      end
+    end
+  end
+end
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/components/ruby/puppet-modules/oracle-solaris_providers/files/etc/puppet/modules/solaris_providers/spec/unit/type/pkg_mediator_spec.rb	Thu Apr 21 12:46:25 2016 -0400
@@ -0,0 +1,78 @@
+#!/usr/bin/env rspec
+require 'spec_helper'
+
+describe Puppet::Type.type(:pkg_mediator) do
+
+  before do
+    @class = described_class
+    @profile_name = "pkgmediator"
+  end
+
+  it "should have :name as its keyattribute" do
+    expect( @class.key_attributes).to be == [:name]
+  end
+
+  describe "when validating attributes" do
+    [ :ensure, :version, :implementation ].each do |prop|
+      it "should have a #{prop} property" do
+        expect(@class.attrtype(prop)).to be == :property
+      end
+    end
+  end
+
+  describe "when validating values" do
+
+    describe "for ensure" do
+      error_pattern = /Invalid value/m
+      def validate(ens)
+         @class.new(:name => @profile_name, :ensure => ens)
+      end
+
+      [ "present", "absent" ].each do |newval|
+        it "should accept a value of #{newval}" do
+          expect { validate(newval) }.not_to raise_error
+        end
+      end
+    end
+
+    describe "for version" do
+      def validate(ver)
+         @class.new(:name => @profile_name, :version => ver)
+      end
+
+      [ "none", "None", "1", "1.2", "1.2.3" ].each do |v|
+        it "should accept #{v}" do
+          expect { validate v }.not_to raise_error
+        end
+      end
+
+      [ "A", "1a", "1.2b" ].each do |v|
+        it "should not accept #{v}" do
+          expect { validate v }.to raise_error(Puppet::ResourceError)
+        end
+      end
+    end  # version
+
+    describe "for implementation" do
+      def validate(imp)
+         @class.new(:name => @profile_name, :implementation => imp)
+      end
+
+      [ "none", "None", "foo", "[email protected]", "[email protected]" ].each do |v|
+        it "should accept #{v}" do
+          expect { validate v }.not_to raise_error
+        end
+      end
+        [ "foo bar", "foo 1", "[email protected]" ].each do |v|
+      it "should not accept #{v}" do
+          expect { validate v }.to raise_error(Puppet::ResourceError)
+        end
+      end
+    end  # implementation
+  end # validating values
+  describe "when validating resource" do
+    it "should not accept None for both implementation and version" do
+         expect { @class.new(:name => @profile_name, :version => 'None', :implementation => "None") }.to raise_error(Puppet::ResourceError)
+    end
+  end
+end
--- a/components/ruby/puppet-modules/oracle-solaris_providers/oracle-solaris_providers-local.frag	Wed Apr 27 14:55:10 2016 -0700
+++ b/components/ruby/puppet-modules/oracle-solaris_providers/oracle-solaris_providers-local.frag	Thu Apr 21 12:46:25 2016 -0400
@@ -77,3 +77,7 @@
 file path=solaris_providers/lib/puppet/type/svccfg.rb
 file path=solaris_providers/lib/puppet/type/vni_interface.rb
 file path=solaris_providers/lib/puppet/type/vnic.rb
+file path=solaris_providers/spec/fixtures/unit/provider/pkg/pkg_mediator/pkg_mediator.txt
+file path=solaris_providers/spec/fixtures/unit/provider/pkg/pkg_mediator/pkg_mediator_foo.txt
+file path=solaris_providers/spec/unit/provider/pkg/pkg_mediator_spec.rb
+file path=solaris_providers/spec/unit/type/pkg_mediator_spec.rb