BREAKING: Change the default 'archive_path' to '/opt/consul/archives'.

[jmkeyes]

If another Puppet module already defines the archive directory then compiling a catalog leads to a duplicate resource error; this patch should fix that issue.

[solarkennedy]

This is the archive_path, I wouldn't expect people to be managing this outside of this module. Can you tell me more about your use-case and why this path is managed in your environment?

[jmkeyes]

This module defines the $archive_path as /opt/puppet-archive by default and manages that directory. That path can also be managed by other modules that use puppet/archive and doesn't seem to be consul-specific. This patch modifies the module to avoid any duplicate resource declarations.

[solarkennedy]

Hmm. You are right, that isn't really a good path for this module to be using.
But I don't think !defines are the way to go.

How about changing the default path to be /opt/consul/ and this module can assume it totally control this dir and doesn't have to share it?

[jmkeyes]

I've replaced the commit I made with one that updates archive_path to default to /opt/consul as requested. Will that work? Although this seems to work, I'd want to keep downloaded archives in a subfolder of /opt/consul. Maybe /opt/consul/archive or /opt/consul/pkg instead?

[solarkennedy]

Sure, but why do you want to keep downloaded packages in a subfolder of /opt/consul?

[jmkeyes]

The primary reason is for maintenance: I want to be able to automatically remove old archives that are no longer used after an installation or upgrade. Currently, the module stores the downloaded archives beside the folders that it extracts them into, which makes slightly harder to see what archives are unused. If the archives are in a separate folder then I can define a tidy resource to clean them up.

[solarkennedy]

sgtm, if we are going to change it, let's change it to something that can be purged automatically. Go ahead and set it to /opt/consul/archive(s) ?

[solarkennedy]

@jmkeyes although now we have a new problem: it is unlikely that users will have /opt/consul ready for puppet to make the archives folder in :( (right?)

[jmkeyes]

That is correct.

The examples within the README for this module specify /opt/consul as reasonable default for $::consul::config_hash['data_dir']. This hash key is then used to define $::consul::data_dir within the consul class, which is then used in the consul::install class as a base directory for the extracted archives.

Should I update my patch to store archives under a subdirectory of $::consul::data_dir instead?

[solarkennedy]

Ah, data_dir, right. Yes please. I think this is probably the most direct way to do it, as we know this folder must exist (eventually). 90% of the time that will be /opt/puppet anyway in the base, and then archives under that sounds very sane.

[jamtur01]

Looks like this branch was never updated to reflect this: Should I update my patch to store archives under a subdirectory of $::consul::data_dir instead?. See #307.