Duplicate declaration with nested create_resources

499 views
Skip to first unread message

Joao Morais

unread,
Jul 10, 2014, 10:03:14 AM7/10/14
to puppet...@googlegroups.com

Hello list, I have some services that may be duplicated in some machines. They are much like an Apache vhost. In order to remove details from my manifests, I moved the service names to ENC because they are machine-dependent. Here are my datasources and manifests:

ENC:
parameters:
  myservices:
    host1:
      myperserviceconfig
    host2:
      myperserviceconfig

Hiera:
myservice_host:
  myproblematicconfig:
    config1:
      data
    config2:
      data
  myotherconfig:
  ...

Manifests:
  class profile::my::myservice {
    # ENC: $myservices
    $hiera_myservice_host = hiera("myservice_host")
    # ENC has per-host/service and hiera has per-environment configurations
    create_resources(::myservice::host, $myservices, $hiera_myservice_host)
  }

  define myservice::host (myconfigs...) {
    ...
    if $myproblematicconfig {
      create_resources(::myservice::myconfig, $myproblematicconfig)
    }
  }

Each myproblematicconfig key is a new resource and I can have zero to N keys. This configuration is working nice if I have only one service, but if I have two or more services, Puppet Master correctly says:

  Error: Could not retrieve catalog from remote server: Error 400 on SERVER: Duplicate declaration: myservice::myconfig[config1] is already declared

Tried prefix (from puppetlabs/stdlib) in order to change the resource name but it only supports arrays. Changing create_resources to $var.each should help but it requires parser=future which I cannot use right now. Do you see another approach I could try?

jcbollinger

unread,
Jul 11, 2014, 10:11:17 AM7/11/14
to puppet...@googlegroups.com


On Thursday, July 10, 2014 9:03:14 AM UTC-5, Joao Morais wrote:

Hello list, I have some services that may be duplicated in some machines. They are much like an Apache vhost. In order to remove details from my manifests, I moved the service names to ENC because they are machine-dependent. Here are my datasources and manifests:

ENC:
parameters:
  myservices:
    host1:
      myperserviceconfig
    host2:
      myperserviceconfig

Hiera:
myservice_host:
  myproblematicconfig:
    config1:
      data
    config2:
      data
  myotherconfig:
  ...



So, I'm interpreting the hiera data as providing configuration details that, if present, apply to every 'myservice', at least as defaults.  Furthermore, from the data and manifests I judge that 'myproblematicconfig' is a distinguished identifier, handled in a manner specific to it.

 
Manifests:
  class profile::my::myservice {
    # ENC: $myservices
    $hiera_myservice_host = hiera("myservice_host")
    # ENC has per-host/service and hiera has per-environment configurations
    create_resources(::myservice::host, $myservices, $hiera_myservice_host)
  }

  define myservice::host (myconfigs...) {
    ...
    if $myproblematicconfig {
      create_resources(::myservice::myconfig, $myproblematicconfig)
    }
  }

Each myproblematicconfig key is a new resource


No, that's only true within the scope one myservice:host.  That is, in fact, the crux of your problem.

 
and I can have zero to N keys. This configuration is working nice if I have only one service, but if I have two or more services, Puppet Master correctly says:

  Error: Could not retrieve catalog from remote server: Error 400 on SERVER: Duplicate declaration: myservice::myconfig[config1] is already declared



Quite.

 
Tried prefix (from puppetlabs/stdlib) in order to change the resource name but it only supports arrays. Changing create_resources to $var.each should help but it requires parser=future which I cannot use right now. Do you see another approach I could try?



The basic problem is that members of your $hiera_myservice_host, such as the one keyed by 'myproblematicconfig' do not represent collections of resources.  Instead, they represent only a partial characterization of the resources you want to create.  They aren't suitable for direct use with create_resources(), at least in your context.

A different structure for your data might work better, but you can also work with the data structure you already have.  In particular, create_resources() can often be replaced by an array-titled resource declaration with the array drawn from the keys() of your resource hash.  Combinatorial resource declarations are a bit trickier, but they can be done, even without the future parser.

Here's one way you could approach it:

define myservice::host ($myconfigs...) {
  ...
  if $myproblematicconfig {
    $config_names = keys($myproblematicconfig)
    $config_names_qual = regsubst($config_names, '^', "${title}::")
    myservice::host::myproblematicconfighelper { $config_names_qual:
      configs => $myproblematicconfig
    }
  }
}

define myservice::host::myproblematicconfighelper($all_configs) {
  $selected_config_name = regsubst($title, '^.*::')
  $selected_config_hash = {
    $title => $all_configs[$selected_config_name]
  }
  # Could use a normal declaration of a myservice::myconfig, too:
  create_resources(::myservice::myconfig, $selected_config_hash)
}


That's a bit roundabout; it is intended to fit into your current scheme with a minimum of changes.  If the myservice::myconfig type does not use its $title (or $name) then you could probably use the above without any other changes.  Otherwise, myservice::myconfig would need to be adjusted to extract its unqualified name (as, for example, the helper type does).

You may be able to make that cleaner, simpler, or otherwise better by applying some of your knowledge of the problem space to the structure of the defined types.


John

Joao Morais

unread,
Jul 14, 2014, 9:08:51 AM7/14/14
to puppet...@googlegroups.com

Em sexta-feira, 11 de julho de 2014 11h11min17s UTC-3, jcbollinger escreveu:

So, I'm interpreting the hiera data as providing configuration details that, if present, apply to every 'myservice', at least as defaults.  Furthermore, from the data and manifests I judge that 'myproblematicconfig' is a distinguished identifier, handled in a manner specific to it.

Almost that. I have quite some configurations like that but almost all of them are applied on a single ERB template. The "problematicconfig", on the other hand, will manage different files. So AFAICS the iteration need to be at the Puppet manifest side. create_resources() is the way I have found to iterate a hash without parser=future.


A different structure for your data might work better

Share your thoughts! No problem with some refactor.


, but you can also work with the data structure you already have.  In particular, create_resources() can often be replaced by an array-titled resource declaration with the array drawn from the keys() of your resource hash.  Combinatorial resource declarations are a bit trickier, but they can be done, even without the future parser.

Got the idea, nice approach with keys+regsubst, thank you very much.


You may be able to make that cleaner, simpler, or otherwise better by applying some of your knowledge of the problem space to the structure of the defined types.

Digging a bit more into the problem I found another approach: define a local variable with the $title just before the hiera_hash call, and use such local variable in the hash declaration. Something like this:

Manifest:
  class profile::my::myservice {
    $mytitle_hiera = $title

    # ENC: $myservices
    $hiera_myservice_host = hiera("myservice_host")
    # ENC has per-host/service and hiera has per-environment configurations
    create_resources(::myservice::host, $myservices, $hiera_myservice_host)
  }

Hiera:
  myservice_host:
    myproblematicconfig:
      %{mytitle_hiera}_config1:
        data
      %{mytitle_hiera}_config2:
        data
    myotherconfig:
    ...

It worked like a charm in my configuration. On the other hand Puppet docs[1] says this is not a good practice. I follow the doc in the sense that the lookup will be dependent of a manifest configuration. Best practices, straightforward code and fit the needs are really challenging.

[1] http://docs.puppetlabs.com/hiera/latest/variables.html#best-practices


Joao Morais




jcbollinger

unread,
Jul 15, 2014, 9:45:27 AM7/15/14
to puppet...@googlegroups.com


On Monday, July 14, 2014 8:08:51 AM UTC-5, Joao Morais wrote:

Em sexta-feira, 11 de julho de 2014 11h11min17s UTC-3, jcbollinger escreveu:

So, I'm interpreting the hiera data as providing configuration details that, if present, apply to every 'myservice', at least as defaults.  Furthermore, from the data and manifests I judge that 'myproblematicconfig' is a distinguished identifier, handled in a manner specific to it.

Almost that. I have quite some configurations like that but almost all of them are applied on a single ERB template. The "problematicconfig", on the other hand, will manage different files. So AFAICS the iteration need to be at the Puppet manifest side. create_resources() is the way I have found to iterate a hash without parser=future.


A different structure for your data might work better

Share your thoughts! No problem with some refactor.


I didn't have a specific idea there, I'm afraid.  I don't have a sufficiently firm grasp of what you're trying to do, or of the overall structure of [this part of] your manifest set to make a specific recommendation.  My main point there is what I said elsewhere: your data don't give complete details of the resources you want to create, so they are not well structured for use with create_resources().

 


, but you can also work with the data structure you already have.  In particular, create_resources() can often be replaced by an array-titled resource declaration with the array drawn from the keys() of your resource hash.  Combinatorial resource declarations are a bit trickier, but they can be done, even without the future parser.

Got the idea, nice approach with keys+regsubst, thank you very much.


You may be able to make that cleaner, simpler, or otherwise better by applying some of your knowledge of the problem space to the structure of the defined types.

Digging a bit more into the problem I found another approach: define a local variable with the $title just before the hiera_hash call, and use such local variable in the hash declaration. Something like this:

Manifest:
  class profile::my::myservice {
    $mytitle_hiera = $title
    # ENC: $myservices
    $hiera_myservice_host = hiera("myservice_host")
    # ENC has per-host/service and hiera has per-environment configurations
    create_resources(::myservice::host, $myservices, $hiera_myservice_host)
  }

Hiera:
  myservice_host:
    myproblematicconfig:
      %{mytitle_hiera}_config1:
        data
      %{mytitle_hiera}_config2:
        data
    myotherconfig:
    ...

It worked like a charm in my configuration. On the other hand Puppet docs[1] says this is not a good practice. I follow the doc in the sense that the lookup will be dependent of a manifest configuration.


I'm glad the docs say that, because the approach you describe makes me queasy.  I'm sure it does work, but a substantial part of the reason for externalizing data is to decouple it from your manifests.  Interpolating manifest-based data -- especially context-sensitive manifest-based data -- into your external data runs directly counter to that objective.  Furthermore, your data are no longer independently meaningful, and you assume an additional burden of including the correct interpolation expression in your data everywhere one is needed.

 
Best practices, straightforward code and fit the needs are really challenging.


Yes, they can be.  The future parser will make that a bit easier.  You could also consider whether you could improve your situation with a custom function or two.  More generally, I think people really need to adopt the correct mindset to be effective at developing Puppet manifests and modules.  Specifically, it is best to recognize that when you work on manifests, you are engaging in modelling, not programming.  That applies even when you engage the more programming-like features available in the future parser.


John

Reply all
Reply to author
Forward
0 new messages