Catalog compilation can fail when managing the same map file via multiple autofs::map resources

[jcbollinger]

Affected Puppet, Ruby, OS and module versions/distributions

• Puppet: 5.4.0
• Ruby: 2.1
• Distribution: RedHat Enterprise Linux 7.4
• Module version: 4.3.0, 4.3.1rc0

How to reproduce (e.g Puppet code you use)

This is a generalization of issue #104. The underlying problem is that the module relies on autofs::map to declare an appropriate Concat resource for its map file, which it does indirectly by means of ensure_resource() in an attempt to avoid duplicate resource declarations when multiple autofs::map resources target the same map file. But this strategy is only partially effective: it works only as long as all autofs::map resources contributing to the same map file pass identical parameter lists to ensure_resource().

The example in issue #104 demonstrates this problem, and so does this:

autofs::map { 'foo':
  mapfile => '/etc/auto.test',
  replace => false,
}

autofs::map { 'bar':
  mapfile => '/etc/auto.test',
  mapcontents => ['bar -rw example.com:/export/bar'],
}

(That can also be rewritten to avoid the using the undocumented $autofs::map::replace parameter directly by using an autofs::mount resource with $replace => false in place of Autofs::Map[foo].)

What are you seeing

Catalog compilation fails with a duplicate resource declaration error.

What behaviour did you expect instead

Either catalog compilation should succeed, somehow merging map file properties from multiple autofs::map declarations, or it should fail with a user-actionable error message (i.e. one describing a problem with the resources declared explicitly, not those declared internally by the module).

But really, the module design ought not to allow the problem to arise in the first place. Or at bare minimum, the module documentation should present the relevant usage constraints.

Output log

Error: Evaluation Error: Error while evaluating a Resource Statement, Evaluation Error: Error while evaluating a Function Call, Duplicate declaration: Concat[/etc/auto.test] is already declared at (file: /home/jbolling/.puppetlabs/etc/code/modules/autofs/manifests/map.pp, line: 54); cannot redeclare (file: /home/jbolling/.puppetlabs/etc/code/modules/autofs/manifests/map.pp, line: 54) (file: /home/jbolling/.puppetlabs/etc/code/modules/autofs/manifests/map.pp, line: 54, column: 3) (file: /home/jbolling/tmp/map_err.pp, line: 7) on node xxx.yyy.org

Any additional information you'd like to impart

I account the issue to a design flaw: defined type autofs::map is trying to do too many things. It represents a chunk of contents of a map file, and with the possibility of multiple resources of that type contributing to the same map file, it is too much for autofs::map to have overall responsibility for the map file itself. That ought to be the job of autofs::mount.

Making autofs::mount responsible for the overall map file (via that file's master Concat resource) would mean that one could not use autofs::map without autofs::mount (or equivalent data), but that's already halfway the case. Without an autofs::mount to manage an appropriate entry in the master map (leaving aside the map file for a moment) the use cases for managing map files are relatively few. And these could be addressed by giving autofs::mount the capability of managing only the specified map file, leaving the master map alone.

On the other hand, Autofs does not require every mount point to have a distinct map file, though it's unclear how useful it is for different mount points to share, or how common it is for them to do so. This module can accommodate mapfile sharing as-is, but it would make for a cleaner separation of concerns to add a separate resource to manage map files.

[dhollinger]

@jcbollinger Did #111 or #110 solved this issue? If so, I'll go ahead and close the issue

[jcbollinger]

@dhollinger, no, neither #110 nor #111 fixes this. I anticipate that it will be fixed together with #104, because the two have the same underlying cause. I am waiting on your response to my latest comments on that issue to decide how to proceed.