Removed unless block from template

[genebean]

The unless broke if an empty array was passed in. The modifications are per recommendations from @danielparks

[genebean]

I need to edit this a little more...

[genebean]

Something is off with my testing here...

[genebean]

This error from Onceover is what I am trying to fix here.

role::pe::metrics
  using fact set server.example.net
F    should compile into a catalogue without dependency cycles (FAILED - 1)


Failures:

  1) role::pe::metrics using fact set server.example.net should compile into a catalogue without dependency cycles
     Failure/Error: it { should compile }

       error during compilation: Evaluation Error: Error while evaluating a Function Call, 'unique' expects one of:
         (String string, Callable[String] block?)
           rejected: parameter 'string' expects a String value, got Undef
         (Hash hash, Callable[Any] block?)
           rejected: parameter 'hash' expects a Hash value, got Undef
         (Array array, Callable[Any] block?)
           rejected: parameter 'array' expects an Array value, got Undef
         (Iterable iterable, Callable[Any] block?)
           rejected: parameter 'iterable' expects an Iterable value, got Undef (file: /Users/me/repos/my_repo/.onceover/etc/puppetlabs/code/environments/production/modules/puppet_metrics_dashboard/templates/influxdb.conf.epp, line: 23, column: 23) on node foo-c02r3abag8wp
     # ./spec/classes/role__pe__metrics_on_boot-pxe-prod-1.ops.puppetlabs.net_spec.rb:117:in `block (3 levels) in <top (required)>'

I am creating the list of masters and pdb servers via this in my manifest:

$master_query = "inventory[certname] {
    facts.classification.stage = '${facts['classification']['stage']}' and
    resources {
      type = 'Class' and
      title = 'Profile::Pe::Master'
    }
  }"

  $masters = puppetdb_query($master_query).map |$value| { $value["certname"] }

  $mom_query = "inventory[certname] {
    facts.classification.stage = '${facts['classification']['stage']}' and
    resources {
      type = 'Class' and
      title = 'Profile::Pe::Mom'
    }
  }"

  $moms = puppetdb_query($mom_query).map |$value| { $value["certname"] }

In my Onceover test run those pdb queries but naturally don't find anything... I thought this was retuning an empty array but I guess something else is going on :( The same code works fine in production when pdb actually returns stuff.

[jarretlavallee]

That is pretty weird. I am not sure how $serverarray would be undef given your changes. Is the oneover output after the proposed changes?

I suspect removing the unless $master_list.empty block and giving master_list a default value of [] should have alleviated the initial issue.

      Array $master_list  = []
| -%>

      $serverarray = $master_list.map |$host| { 
          $server_tag = join(split($host,"[.]").map | $element | { "server" },'.') 
          $wildcards = join(split($host,"[.]").map | $element | { "*" },'.') 
          "puppetlabs.${wildcards}.placeholder .${server_tag}.measurement*" 
     }

This is given that unique and map on an empty array would return an empty array:

# puppet apply -e 'notice(unique([].map |$a| {}))'
Notice: Scope(Class[main]): []
Notice: Compiled catalog for pe-201814-master.puppetdebug.vlan in environment production in 0.09 seconds
Notice: Applied catalog in 0.24 seconds

Oops, missed what was going on.

[danielparks]

Coincidently, unique was called on line 23 in the old code too. And that code would definitely set $serverarray to undef in the conditions you described.

I'd convert that if block to just $unique_serverarray = unique($serverarray), since that should be sufficient, AND it will put the unique call on line 22.

Then, when you run the test you'll be able to see if it's picking up the right code by looking at the line number.

[danielparks]

(I have tracked down waaayy too many bugs in PHP code by adding lines to change the line number in the error message. (Sometimes I would get a line number, but not a file name.))

[suckatrash]

@genebean I had a moment to look into this this morning. Based on the output from onceover it looks like the code is calling the new unique function that we added in PUP-7620. You can see your output is hitting the code block here:
https://github.com/hlindberg/puppet/blob/2052c4c1fbeab0aa77ddd3a4b427903e7ac0b286/lib/puppet/functions/unique.rb#L86-L105

I'm not sure how to solve this yet, though. I'm not familiar with onceover, would it somehow prefer the "native" unique function over the stdlib one? (I'm assuming when you run this in a puppet environment it uses the stdlib function still)

[genebean]

Guess that is why namespacing is important @suckatrash ... too bad the stdlib one is not namespaced by default

[genebean]

The oneceover issue was a PEBKAC one... aka, a local testing goof. This version passes tests.

[suckatrash]

@genebean @danielparks Is there really anything to fix here? If the new unique function returns undef when passed an empty array, then that's a breaking change that will be introduced when the stdlib function is deprecated.

I think we should submit a PUP ticket for that. I can pass an empty array to the (current) template and it will compile without errors as long as I use the stdlib function.

[genebean]

@suckatrash We can't assume the user will be using the stdlib version though as best as I can tell. The module needs to work regardless of which is used, right?

[genebean]

Oh, and that pebkac was just for the local onceover testing.

[suckatrash]

@genebean I'm going to merge this since it behaves better in either situation, but I still think this is a bug in the new function, and I opened a ticket here: PUP-9305

Thanks for the PR!

[genebean]

Thanks!

[hlindberg]

Make this Array[String] to avoid having to deal with bad stuff in the $master_list (later there are calls to split which will fail if there is garbage in $master_list

[hlindberg]

This is meaningless if typing the input as suggested above, and you know that the map function called above always returns an Array because the input to it is known to be Array[String].
The only thing that can happen is that the input can be empty, in which case the $unique_serverarray is also empty.

Also, note that the removal of any undef values in $serverarray on this line does not alter the $serverarray, it just affects the calculated length, so below, the call to unique could return a set that incudes undef when
there is no protection against that from the start.

You can write the entire thing (lines 15-26) like this:

$unique_serverarray = $master_list.map |$host| { 
  $split = $host.split("[.]")
  $server_tag = $split.map | $x | { "server" }.join('.') 
  $wildcards = $split.map | $x | { "*" }.join('.') 
  "puppetlabs.${wildcards}.placeholder .${server_tag}.measurement*" 
}.unique()