Jira (PUP-9792) Merge of lookup_options not working

0 views
Skip to first unread message

Matt Dainty (JIRA)

unread,
Jun 21, 2019, 7:10:03 AM6/21/19
to puppe...@googlegroups.com
Matt Dainty created an issue
 
Puppet / Bug PUP-9792
Merge of lookup_options not working
Issue Type: Bug Bug
Affects Versions: PUP 5.5.14
Assignee: Unassigned
Components: Hiera & Lookup
Created: 2019/06/21 4:09 AM
Priority: Normal Normal
Reporter: Matt Dainty

Puppet Version: 5.5.14
Puppet Server Version: 5.3.8
OS Name/Version: RHEL 7.x

In our Hiera data, we have made use of lookup_options to control the merge behaviour, so in our catch-all common.yaml there are entries such as:

---
 
lookup_options:
 
  profile::autofs::mounts:
 
    merge: hash

This works as desired. However we have some exceptions where we want to override this on a per-role basis back to the original "first" behaviour, so as we have a higher priority role level, I added the following in a role/foo.yaml:

---
 
lookup_options:
 
  profile::autofs::mounts:
 
    merge: first

If I use the "puppet lookup" command against the correct host, this appears to work as desired, using `--explain-options` I get the following result:

  Merged result: {
 
    "profile::autofs::mounts" => {
 
      "merge" => "first"
 
    }
 
  }

For any other hosts I still get the behaviour defined at the common level:

  Merged result: {
 
    "profile::autofs::mounts" => {
 
      "merge" => "hash"
 
    }
 
  }

All good. However this doesn't seem to actually apply to the lookup itself:

Using merge options from "lookup_options" hash
 
Searching for "profile::autofs::mounts"
 
  ...

It only ever picks up the behaviour defined at the common level.

Desired Behavior:

I would expect to see:

Using merge options from "lookup_options" first
 
Searching for "profile::autofs::mounts"
 
  ...

Actual Behavior:

One bit of information that might be relevant is that in order for our lookups to work from the CLI, I have to use the --compile option, so basically something like:

puppet lookup --compile --environment <env> --node <node> profile::autofs::mounts --explain(-options)

We have multiple backends/levels configured in Hiera, a mix of YAML/eYAML and using the HTTP backend too.

 

I noticed there was PUP-7037 which was raised for similar behaviour but was ultimately traced to a typo. I'm confident this isn't down to a typo.
Add Comment Add Comment
 
This message was sent by Atlassian JIRA (v7.7.1#77002-sha1:e75ca93)
Atlassian logo

Matt Dainty (JIRA)

unread,
Jun 21, 2019, 7:31:03 AM6/21/19
to puppe...@googlegroups.com
Matt Dainty updated an issue
Change By: Matt Dainty
*Puppet Version: 5.5.14*
*Puppet Server Version: 5.3.8*
*OS Name/Version: RHEL 7.x*


In our Hiera data, we have made use of lookup_options to control the merge behaviour, so in our catch-all common.yaml there are entries such as:
{noformat}
---
lookup_options:
  profile::autofs::mounts:
    merge: hash {noformat}

This works as desired. However we have some exceptions where we want to override this on a per-role basis back to the original "first" behaviour, so as we have a higher priority role level, I added the following in a role/foo.yaml:
{noformat}
---
lookup_options:
  profile::autofs::mounts:
    merge: first {noformat}

If I use the "puppet lookup" command against the correct host, this appears to work as desired, using `--explain-options` I get the following result:
{noformat}
  Merged result: {
    "profile::autofs::mounts" => {
      "merge" => "first"
    }
  }{noformat}

For any other hosts I still get the behaviour defined at the common level:
{noformat}
  Merged result: {
    "profile::autofs::mounts" => {
      "merge" => "hash"
    }
  } {noformat}

All good. However this doesn't seem to actually apply to the lookup itself:
{noformat}
Using merge options from "lookup_options" hash
Searching for "profile::autofs::mounts"   Merge strategy hash
  ... {noformat}

It only ever picks up the behaviour defined at the common level.

*Desired Behavior:*


I would expect to see:
{noformat}
Using merge options from "lookup_options" first hash
Searching for "profile::autofs::mounts"

   ... {noformat}
 i.e. (no merge strategy displayed here)

 *Actual Behavior:*


One bit of information that might be relevant is that in order for our lookups to work from the CLI, I have to use the --compile option, so basically something like:
{noformat}
puppet lookup --compile --environment <env> --node <node> profile::autofs::mounts --explain(-options){noformat}

We have multiple backends/levels configured in Hiera, a mix of YAML/eYAML and using the HTTP backend too.

 

I noticed there was PUP-7037 which was raised for similar behaviour but was ultimately traced to a typo. I'm confident this isn't down to a typo.


 

Edit: After a bit more experimenting, I was a bit confused about where "hash" should be printed vs other merge strategies, the 'Using merge options from "lookup_options" hash' is pretty much a fixed string, it's the presence/lack of "Merge strategy ..." that is the identifier.

Henrik Lindberg (JIRA)

unread,
Jun 21, 2019, 2:11:03 PM6/21/19
to puppe...@googlegroups.com
Henrik Lindberg commented on Bug PUP-9792
 
Re: Merge of lookup_options not working

IIRC, the lookup_options are looked up once for everything. If the hierarchy then depends on changing variables leading to different paths being looked up, it is supposed to evict the cache, maybe that does not happen for lookup_options.

Henrik Lindberg (JIRA)

unread,
Jun 21, 2019, 2:14:02 PM6/21/19
to puppe...@googlegroups.com

The lookup_options merge is not a deep merge - if overriding, all information for the specific key the options are for must be given. Not sure if that has any bearing here.

Jorie Tappa (JIRA)

unread,
Jul 15, 2019, 1:30:03 PM7/15/19
to puppe...@googlegroups.com
Jorie Tappa commented on Bug PUP-9792

ping Henrik Lindberg does this ticket need action, or more information from Matt Dainty?

Henrik Lindberg (JIRA)

unread,
Jul 15, 2019, 5:54:03 PM7/15/19
to puppe...@googlegroups.com

Jorie Tappa Someone needs to set up and try to reproduce his case. I added some notes about things to look at - for example, does this work if the hierarchy does not depend on compilation - something that Matt Dainty could try as well as someone at Puppet. This could also be a bug that has been fixed as he is using Puppet 5.5.14.

Jorie Tappa (JIRA)

unread,
Jul 22, 2019, 1:05:06 PM7/22/19
to puppe...@googlegroups.com
Jorie Tappa commented on Bug PUP-9792

Matt Dainty can you please help us with a simple example of how we can reproduce this on our end?

Matt Dainty (JIRA)

unread,
Sep 12, 2019, 11:36:03 AM9/12/19
to puppe...@googlegroups.com
Matt Dainty commented on Bug PUP-9792

Sorry, I didn't see your comments. I think I've managed to reduce this down to the following contrived example:

hiera.yaml

---
 
version: 5
 
defaults:
 
  datadir: data
 
  data_hash: yaml_data
 
hierarchy:
 
  - name: YAML
 
    paths:
 
      - "%{bar}.yaml"
 
      - "common.yaml"

data/common.yaml

---
 
lookup_options:
 
  foo:
 
    merge: hash
 
foo:
 
  foo: 'bar'

data/bar.yaml

---
 
lookup_options:
 
  foo:
 
    merge: first
 
foo:
 
  baz: 'quux'

manifests/site.pp

notice(lookup('foo'))
 
 
 
$bar = 'bar'
 
 
 
notice(lookup('foo'))

If I run `puppet lookup --compile --environment ... --node ... --explain foo`, I get:

Warning: Undefined variable 'bar'; \n   (file & line not available)
 
Notice: Scope(Class[main]): {foo => bar}
 
Notice: Scope(Class[main]): {foo => bar, baz => quux}
 
Using merge options from "lookup_options" hash
 
Searching for "foo"
 
  Merge strategy hash
 
    Global Data Provider (hiera configuration version 5)
 
      Using configuration "/etc/puppetlabs/code/hiera.yaml"
 
      Hierarchy entry "Common"
 
        Path "/etc/puppetlabs/code/data/common.yaml"
 
          Original path: "common.yaml"
 
          Path not found
 
    Environment Data Provider (hiera configuration version 5)
 
      Using configuration "/etc/puppetlabs/code/environments/.../hiera.yaml"
 
      Merge strategy hash
 
        Hierarchy entry "YAML"
 
          Merge strategy hash
 
            Path "/etc/puppetlabs/code/environments/.../data/bar.yaml"
 
              Original path: "%{bar}.yaml"
 
              Found key: "foo" value: {
 
                "baz" => "quux"
 
              }
 
            Path "/etc/puppetlabs/code/environments/.../data/common.yaml"
 
              Original path: "common.yaml"
 
              Found key: "foo" value: {
 
                "foo" => "bar"
 
              }
 
            Merged result: {
 
              "foo" => "bar",
 
              "baz" => "quux"
 
            }
 
        Merged result: {
 
          "foo" => "bar",
 
          "baz" => "quux"
 
        }
 
    Merged result: {
 
      "foo" => "bar",
 
      "baz" => "quux"
 
    }

I would be expecting to just get '{ "baz" => "quux" }'.

If I change hiera.yaml to:

---
 
version: 5
 
defaults:
 
  datadir: data
 
  data_hash: yaml_data
 
hierarchy:
 
  - name: YAML
 
    paths:
 
      - "%{facts.bar}.yaml"
 
      - "common.yaml"

So I use a fact instead of a top-scope variable and then create a facts.yaml with:

---
 
bar: 'bar'

When I run `puppet lookup --environment ... --node ... --facts ./facts.yaml --explain foo` (no --compile) I get:

Searching for "lookup_options"
 
  Global Data Provider (hiera configuration version 5)
 
    Using configuration "/etc/puppetlabs/code/hiera.yaml"
 
    Hierarchy entry "Common"
 
      Path "/etc/puppetlabs/code/data/common.yaml"
 
        Original path: "common.yaml"
 
        Path not found
 
  Environment Data Provider (hiera configuration version 5)
 
    Using configuration "/etc/puppetlabs/code/environments/.../hiera.yaml"
 
    Merge strategy hash
 
      Hierarchy entry "Normal YAML hierarchy levels"
 
        Merge strategy hash
 
          Path "/etc/puppetlabs/code/environments/.../data/bar.yaml"
 
            Original path: "%{facts.bar}.yaml"
 
            Found key: "lookup_options" value: {
 
              "foo" => {
 
                "merge" => "first"
 
              }
 
            }
 
          Path "/etc/puppetlabs/code/environments/.../data/common.yaml"
 
            Original path: "common.yaml"
 
            Found key: "lookup_options" value: {
 
              "foo" => {
 
                "merge" => "hash"
 
              }
 
            }
 
          Merged result: {
 
            "foo" => {
 
              "merge" => "first"
 
            }
 
          }
 
      Merged result: {
 
        "foo" => {
 
          "merge" => "first"
 
        }
 
      }
 
Using merge options from "lookup_options" hash
 
Searching for "foo"
 
  Global Data Provider (hiera configuration version 5)
 
    Using configuration "/etc/puppetlabs/code/hiera.yaml"
 
    Hierarchy entry "Common"
 
      Path "/etc/puppetlabs/code/data/common.yaml"
 
        Original path: "common.yaml"
 
        Path not found
 
  Environment Data Provider (hiera configuration version 5)
 
    Using configuration "/etc/puppetlabs/code/environments/.../hiera.yaml"
 
    Hierarchy entry "YAML"
 
      Path "/etc/puppetlabs/code/environments/.../data/bar.yaml"
 
        Original path: "%{facts.bar}.yaml"
 
        Found key: "foo" value: {
 
          "baz" => "quux"
 
        }

This returns the expected result. So it looks specifically to do with using variables, which unfortunately I can't get away from.

Since reporting this, we're now running Puppet 5.5.16 and Puppet Server 5.3.9. I can't (easily) upgrade to 6.x due to us relying on MCollective currently.

Hopefully this helps.

 

Henrik Lindberg (JIRA)

unread,
Sep 13, 2019, 1:44:04 PM9/13/19
to puppe...@googlegroups.com

In order to use top scope variables in your hiera.yaml and get the correct answer from puppet lookup CLI, you must use the --evaluate option.

Matt Dainty (JIRA)

unread,
Sep 16, 2019, 10:14:03 AM9/16/19
to puppe...@googlegroups.com
Matt Dainty commented on Bug PUP-9792

There's no such option, (on 5.5.16 at least):

Error: Could not parse application options: invalid option: --evaluate

I'm already using --compile as part of my tests, however I don't get the correct answer with the puppet lookup CLI, nor within a normal catalog compile.

Your first comment suggested a lack of cache eviction, my example does suggest the lookup options aren't re-evaluated once the top scope variables are set.

Matt Dainty (JIRA)

unread,
Sep 16, 2019, 11:23:02 AM9/16/19
to puppe...@googlegroups.com
Matt Dainty commented on Bug PUP-9792

I've managed to get my example to work by changing manifests/site.pp to just the following:

$bar = 'bar'
 
 
 
notice(lookup('foo'))

Basically by ensuring the top scope variable is set before any lookup() is called, it gets the correct merge behaviour. This does suggest that lookup_options is getting cached for the lifetime of the compile.

I'm trying to find where in the code this caching is done but I'm not having much luck so far.

Matt Dainty (JIRA)

unread,
Sep 17, 2019, 5:30:03 AM9/17/19
to puppe...@googlegroups.com
Matt Dainty commented on Bug PUP-9792

Eventually I managed to unpick the multiple levels of caching and the following crude diff fixes the behaviour:

diff --git a/lib/puppet/pops/lookup/lookup_adapter.rb b/lib/puppet/pops/lookup/lookup_adapter.rb
 
index 4014aea..d531705 100644
 
--- a/lib/puppet/pops/lookup/lookup_adapter.rb
 
+++ b/lib/puppet/pops/lookup/lookup_adapter.rb
 
@@ -233,12 +233,12 @@ class LookupAdapter < DataAdapter
 
     module_name = key.module_name     # Retrieve the options for the module. We use nil as a key in case we have no module
 
-    if !@lookup_options.include?(module_name)
 
+    #if !@lookup_options.include?(module_name)
 
       options = retrieve_lookup_options(module_name, lookup_invocation, MergeStrategy.strategy(HASH))
 
-      @lookup_options[module_name] = options
 
-    else
 
-      options = @lookup_options[module_name]
 
-    end
 
+    #  @lookup_options[module_name] = options
 
+    #else
 
+    #  options = @lookup_options[module_name]
 
+    #end
 
     extract_lookup_options_for_key(key, options)
 
   end
 
@@ -377,7 +377,7 @@ class LookupAdapter < DataAdapter
 
   # Retrieve and cache lookup options specific to the environment of the compiler that this adapter is attached to (i.e. a merge
 
   # of global and environment lookup options).
 
   def env_lookup_options(lookup_invocation, merge_strategy)
 
-    if !instance_variable_defined?(:@env_lookup_options)
 
+    #if !instance_variable_defined?(:@env_lookup_options)
 
       global_options = global_lookup_options(lookup_invocation, merge_strategy)
 
       @env_only_lookup_options = nil
 
       catch(:no_such_key) { @env_only_lookup_options = validate_lookup_options(lookup_in_environment(LookupKey::LOOKUP_OPTIONS, lookup_invocation, merge_strategy), nil) }
 
@@ -388,7 +388,7 @@ class LookupAdapter < DataAdapter
 
       else
 
         @env_lookup_options = merge_strategy.merge(global_options, @env_only_lookup_options)
 
       end
 
-    end
 
+    #end
 
     @env_lookup_options
 
   end

Would a tidied up PR be acceptable?

Matt Dainty (JIRA)

unread,
Sep 17, 2019, 10:54:02 AM9/17/19
to puppe...@googlegroups.com
Matt Dainty commented on Bug PUP-9792
Reply all
Reply to author
Forward
0 new messages