So how exactly the immutable hashes are supposed to be used?

66 views
Skip to first unread message

Mariusz Gronczewski

unread,
Jul 14, 2017, 10:52:37 AM7/14/17
to Puppet Developers
Hi,

I've been slowly converting old 3.x codebase (which has seen days of 0.24...) to 4.x and there is a lot of following pattern used for hashes:'

    $hash = {
      "some" => "defaults"
    }
    
    if $thing_one == "is_true" {
        $hash["option1"] = $thing_one,
    }
    
    if $thing_two == "is_something" {
        $hash["option2"] = "something"
    }
    else {
        $hash["option2"] = "something else"
    }

etc. Now with immutable hashes I'm forced either to do:'

    $hash1 = {
      "some" => "defaults"
    }
    
    if $thing_one == "is_true" {
        $hash2 = {
            "option1" => $thing_one,
        }
    }
    else { $hash2 = {} }

    
    if $thing_two == "is_something" {
        $hash3 = {
            "option2" => "something"
        }
    }
    else {
        $hash3 = {
            "option2" => "something else"
    } 
    $hash4 = merge($hash1, $hash2, $hash3, $hash3)

or go the hacky route and go to erb and back to use ruby language to do it.

Am I missing something here ? is there a better way to do it? We have a lot of "get a hash, munge it, feed it to either puppet resource or to external config" pattern used in the code and immutability so far doesn't seem like a very useful property

John Bollinger

unread,
Jul 14, 2017, 2:52:05 PM7/14/17
to Puppet Developers
Well, the overall approach seems a bit questionable to me.  You've made the question too generic for me to offer any specific ways to alter your code to avoid hash use altogether, but in some cases that may be the best route.  You may also find that you can get a lot of mileage out of converting some of your code base to rely on Hiera for data, instead of building hashes inside a class and passing them around.

As for the actual code you presented, however, yes, there is a better way to do that specific thing, one that works in every Puppet version from 5.0 back to 2.6:

    $hash = {
     
'some'    => 'defaults',
     
'option1' => ( $thing_one ? { 'is_true' => $thing_one, default => undef } ),
     
'option2' => ( $thing_two ? { 'is_something' => 'something', default => 'something else' } )
   
}

That relies on selectors, a longtime Puppet language feature resembling a turbocharged ternary operator, and also on the trick of setting a value to undef, which has an effect equivalent to not setting it at all.  No multiple hashes or hash merging are required, and it's concise and fairly readable, too.

 
We have a lot of "get a hash, munge it, feed it to either puppet resource or to external config" pattern used in the code and immutability so far doesn't seem like a very useful property



If "munge it" means more than "add new entries" then I'm inclined to suggest scrapping the infected code altogether.  In particular, if hashes are used to try to work around the immutability of Puppet variables then I have no regard for the code at all.  Nevertheless, you seem already to be adept with the merge() function, and I'm sure you can see how that could be combined with the above approach to forming hash values in order to perform a wide variety of munging operations.


John

Mariusz Gronczewski

unread,
Jul 14, 2017, 10:23:11 PM7/14/17
to Puppet Developers

That particular case was basically adding some metadata so the server can be categorized correctly in monitoring system.

We have one resource that is "just" deploying host configuration that has a parameter of custom variables assigned to host (we use icinga) which is basically just a straightforward template deployment.

Then we have another one that takes care of any "logic" and defines first one as exported resource (that is then imported on monitoring server).
Rules in question were stuff like

* if server is a VM and it knows it parent host (via a fact), sanitize that name and then add add that name to host as custom attribute"
* if $project (each server is assigned to project via hiera) is in list of "which client owns which project" list, set custom attribute "client" to that client's name

 so nothing really complex but also not just 'if X exist, add it". I know *how* to hack it, just that having "if this and that then add to hash" seems to be much more readable, especially to some of my colleagues that only language they are semi-proficient with is bash and php...
 

As for the actual code you presented, however, yes, there is a better way to do that specific thing, one that works in every Puppet version from 5.0 back to 2.6:

    $hash = {
     
'some'    => 'defaults',
     
'option1' => ( $thing_one ? { 'is_true' => $thing_one, default => undef } ),
     
'option2' => ( $thing_two ? { 'is_something' => 'something', default => 'something else' } )
   
}

That relies on selectors, a longtime Puppet language feature resembling a turbocharged ternary operator, and also on the trick of setting a value to undef, which has an effect equivalent to not setting it at all.  No multiple hashes or hash merging are required, and it's concise and fairly readable, too.

undef is not "not setting value at all"; after passing to ruby it is still a key, just with undef as a value (so every .each loop have to check if value is defined), and in case of puppet 3.x it stringifies unknown facts to undef (not sure why...):
$a = {
 
"b" => $nonexisting_var
}
$z
= inline_template('<%= @a["b"].to_s %>')
notify
{"value: ${z}":;}..

results in "value: undef" under puppet 3.x and "value: " (and a nice warning) under 4.x

 

 
We have a lot of "get a hash, munge it, feed it to either puppet resource or to external config" pattern used in the code and immutability so far doesn't seem like a very useful property



If "munge it" means more than "add new entries" then I'm inclined to suggest scrapping the infected code altogether.  In particular, if hashes are used to try to work around the immutability of Puppet variables then I have no regard for the code at all.  Nevertheless, you seem already to be adept with the merge() function, and I'm sure you can see how that could be combined with the above approach to forming hash values in order to perform a wide variety of munging operations.
 
It's just that "make a bunch of hashes then push it to ruby function just to merge it and make another hash" seems a very hacky way to accomplish it. Languages that insist on immutability (like Clojure) also provide much better ways to manipulate those structures than "just merge a key with undef and pretend it disappeared", but Puppet seems to get it backward (add immutable state first, then start adding stuff to actually deal with it) and even basic stuff needs trip to ruby and back

We still have other... interesting code that basically takes stuff from PuppetDB, makes resource definitions out of it via a bit of ruby code in a template and puts it back into puppet (like "take all Yum repos used by servers and make a list of repos to be mirrored out of it) but that's adventure for another day :)

Henrik Lindberg

unread,
Jul 25, 2017, 8:55:45 AM7/25/17
to puppe...@googlegroups.com
You can do like what John Bollinger showed - or something like this
where you merge hash snippets using + operator.

$hash = { "some" => "defaults" }
+ if $thing_one == "is_true" {
{"option1" => $thing_one}
}
else { {} }
+ if $thing_two == "is_something" {
{"option2" => "something"}
}
else {
{"option2" => "something else"}
}

- henrik

Visit my Blog "Puppet on the Edge"
http://puppet-on-the-edge.blogspot.se/

Reply all
Reply to author
Forward
0 new messages