Calling "pkg update -n" on Solaris too frequently causes problems with zones

[alanhargreaves]

Describe the Bug

On a Solaris system, running "pkg update -n" in the global zone at the same time that it is running in a non-global zone, can cause the command to fail in one or the other.

If we have puppet running this command, it causes the puppet run to fail for that specific resource.

On a T7-4 with more than 70 non-global zones, this happens, ... a lot.

Expected Behavior

insync? to return either true or false without having unexpected errors in the package subsystem.

Steps to Reproduce

Not so much steps as having an environment with a lot of non-global zones with puppet running on the global zone AND the non-global zones. If we find ourselves running pkg update -n in more than one place at once, one of them will fail.

Environment

• Version Currently 7.27
• Platform Solaris 11.4

Additional Context

The existence of puppet running in no-global zones as well as the global zone. I've been running something that addresses this now for about a year.

[alanhargreaves]

I've been running with the following now for about a year and it appears to address the issue.

% git diff main                             
diff --git a/lib/puppet/provider/package/pkg.rb b/lib/puppet/provider/package/pkg.rb
index cc9b34082a..5e7d3b795a 100644
--- a/lib/puppet/provider/package/pkg.rb
+++ b/lib/puppet/provider/package/pkg.rb
@@ -160,6 +160,13 @@ Puppet::Type.type(:package).provide :pkg, :parent => Puppet::Provider::Package d
         warning(_("Implicit version %{should} has %{n} possible matches") % { should: should, n: n })
       end
       potential_matches.each{ |p|
+
+        # If the version that we match is installed then we don't need
+        # to check if it's installed (or installable), just return true
+        if (is != 'absent') && (p[:status] == 'installed')
+          return true
+        end
+
         command = is == :absent ? 'install' : 'update'
         options = ['-n']
         options.concat(join_options(@resource[:install_options])) if @resource[:install_options]
@@ -227,6 +234,10 @@ Puppet::Type.type(:package).provide :pkg, :parent => Puppet::Provider::Package d
     if is[:ensure].to_sym == :absent
       command = 'install'
     else
+      # If the package is up to date, we don't need to do anything
+      if insync?(should)
+        return 0
+      end
       command = 'update'
     end
     args = ['--accept']

Limiting calls to this pkg command reduces the chances of this problem occurring. There are some optimisations that can be made in lib/puppet/provider/package/pkg.rb to achieve this.

In insync?, we have run a pkg list -Hvfa {pkg name and version string}. We save the installation status, including whether or not this particular version is installed. The most likely scenario is that the first version we match is the version installed. If this is the case, we don't need to run "pkg update -n" to repeat the check. We can just return true. Similar for any further iterations of the loop. If we see this version is installed, then we don't need to test any further.

An argument could be made that we could determine everything from the pkg list command, but the comments explain why we do the pkg update -n to verify that it can be installed. That being said, if the command returns any status other than 0 or 4, we loop, and test aginst the next version that matches our criterea. This could probably be spelled out better in the comments. Now given that we've already confirmed that we have the current version installed, we are not going to get a return code of 4, but I guess it doesn't hurt to leave it there.

Also in install, if the package is up to date, we can just return 0. We can check this with a call to insync?(should).

[alanhargreaves]

It's worth noting that the "normal" case for insync? is that the package is current. Returning true at the top of this function removes the majority of calls to pkg update -n {package@version}

[joshcooper]

It's important for insync? to return true consistently in all cases, as that is what puppet uses to generate a report of what changed during the run and it cause puppet to report that the system is out of compliance. Therefore, you'll need to adjust insync? logic to fit your case.

[alanhargreaves]

The block of code from insync? has been removed as unnecessary, which also addresses the Jan 31 comment.