Function lookup doesn't behave like ruby puppet implementation

[jfroche]

we have a function plone::varnish::backend that calls ::varnish::backend.

define plone::varnish::backend (
  $backend_name,
  $host='localhost',
  $port='8080') {

  $varnish_probe = {
    url       => '/',
    interval  => '5s',
    timeout   => '10s',
    window    => '5',
    threshold => '3'
  }

  ::varnish::backend {$backend_name:
    host  => $host,
    port  => $port,
    probe => $varnish_probe
  }
}

As you see, we are explicit that we want to start from the top to prevent accidental scoping issues. language-puppet doesn't seem to like function this kind of notation. Error is that the varnish::backend is not defined:

Unknown resource Varnish::Backend[client1] used in the following relationships: Varnish::Backend[client1] -> Concat::Fragment[client1]

If I drop the leading "::", it will work for language-puppet but fails for puppet (tested on 3.5.1):

Error: Invalid parameter probe at /root/puppet-stack-plone/modules/plone/manifests/varnish/backend.pp:22 on node ...

Any advice ?

[jfroche]

Also tested using puppet 3.8.3, same problem:

Error: Invalid parameter probe on Plone::Varnish::Backend[client2] at /root/puppet-stack-plone/modules/plone/manifests/varnish/backend.pp:22 on node ...

[bartavelle]

I am not sure I understand what the bug is. What's surprising is that you get a relationship error, which means it should have figured how to resolve ::varnish::backend ! Can you produce a minimal test case so that I can investigate ?

[jfroche]

Did a simple structure here: https://s3-eu-west-1.amazonaws.com/affinitic-pub/issue-140.tar

cd issue-140 && puppet apply --modulepath=modules site.pp

It works for puppet because I am explicit using "::b::foo"

[bartavelle]

I don't get any error here ... which version are you using ? I suspect this is something that is already fixed, but perhaps not released !

[bartavelle]

(It looks like a pair of bugs that got recently squashed)

[jfroche]

Using docker image: pierrer/puppetresources:v1.1.4-g

[PierreR]

So pierrer/puppetresources:v1.1.4-g is actually master

[bartavelle]

Will this be updated to the latest commit automagically, or will the actual version be dependent on when the docker container was built ?

[PierreR]

Not I built the docker image manually. I know -g was built against master (or quite near of it).

I will try the sample against master and confirm.

[bartavelle]

But can you reproduce the error with the given tar file ?

[PierreR]

That's what I am going to try right now ;-)

[PierreR]

So I can reproduce the error with the tar file but I had to add a resource in the "body" of the last define. Something like

  notify{'b::foo':}

I know that this is failing because we globally call a defined resources (using the defined keyword). I believe this has been failing all along.

[PierreR]

At the very least I know it has been failing for a while because I have submitted a couple of PR to some puppetlabs modules requesting the removal of global (::) namespace for defined resources.

[bartavelle]

Ah I can reproduce the problem too, I will try to look into it, but this looks annoying to fix !

[PierreR]

Pour info: About using :: with defined resources:

This is ugly and should be unnecessary, but is occasionally required due to an outstanding design issue.

from https://docs.puppetlabs.com/puppet/3.8/reference/lang_namespaces.html#syntax

I believe (but I am not sure 100%) that a :: in front of a defined resources will fail in Puppet 4. That's why I have tried to PR and asked for the removal of such namespace for foreman, nginx (PR accepted) and apache (PR refused) puppetlabs modules. The arguments for the refusal of the PR by apache were not convincing (https://github.com/puppetlabs/puppetlabs-apache/pull/1191)

[bartavelle]

Yeah but the simple fix that would be ignoring the leading :: will produce erroneous clashes ...

[PierreR]

Sure. I guess it is too early to go and just support Puppet 4 autoloader behavior. If I understand it correctly, relative namespacing is not allowed anymore:

https://docs.puppetlabs.com/puppet/latest/reference/lang_namespaces.html

Does that mean that most puppetlabs modules won't work in Puppet 4 ?

[PierreR]

Would it make sense to just ignore clashes with relative defined resources. Just emit a warning about the clash but choose the absolute (basically ignore ::) at least for defined resources.

[bartavelle]

I do normalize all resource names now, so the leading :: are ignored. On the other hand, the topmost define is selected instead of the local one. So this should do the right thing in this instance. Can anyone find a regression with this ?

[PierreR]

It works fine on my side with my nodes.

What's the f in fridentifier ? Why not normalizeRIdent of normRIdentifier or something alike ?

[bartavelle]

Yeah that would be a much better name !

[PierreR]

I can change that which one do you prefer ?

[bartavelle]

yes that would be nice, thanks !

[bartavelle]

(any would do)