puppetlabs / puppetlabs-zfs_core

Apache License 2.0
0 stars 37 forks source link

Handling of integer values causes puppet to always apply properties #71

Open sbraz opened 7 months ago

sbraz commented 7 months ago

Describe the Bug

Hi, Because of the way ZFS properties can be passed and are returned by the zfs get commands, Puppet applies properties even though they already have the correct values, e.g.

quota changed '4.88T' to '5000G' (corrective)`
reservation changed '6T' to '6597069766656' (corrective)

Expected Behavior

When the values match, the module should detect it and not attempt to set them again.

Steps to Reproduce

  1. Set a quota of 5000G
  2. Set a reservation of 6597069766656

Environment

Additional Context

I thought of the following patch:

diff --git a/lib/puppet/provider/zfs/zfs.rb b/lib/puppet/provider/zfs/zfs.rb
index 976d9bf..b97ec0c 100644
--- a/lib/puppet/provider/zfs/zfs.rb
+++ b/lib/puppet/provider/zfs/zfs.rb
@@ -82,7 +82,7 @@ Puppet::Type.type(:zfs).provide(:zfs) do
    :secondarycache, :setuid, :sharenfs, :sharesmb,
    :snapdir, :sync, :version, :volsize, :vscan, :xattr].each do |field|
     define_method(field) do
-      zfs(:get, '-H', '-o', 'value', field, @resource[:name]).strip
+      zfs(:get, '-p', '-H', '-o', 'value', field, @resource[:name]).strip
     end

     define_method(field.to_s + '=') do |should|

It would prevent issues when the Puppet configuration uses exact values: But it doesn't help when we pass Puppet human-readable values, e.g.

quota changed '5368709120000' to '5000G' (corrective)

Maybe the solution would be to:

joshcooper commented 2 months ago

The munge method needs to implemented for properties whose values can take multiple forms. That way the desired and current values are canonicalized before being compared.