Moved r10k file and basedir

[WhatsARanjit]

No description provided.

[acidprime]

So this is different the pe_r10k's default value, can you explain why you made this decision in your commit ?

[WhatsARanjit]

Any PE was setup to provide a default environment directory of /etc/puppetlabs/puppet/environments by using the $::settings::confdir variable. I moved $r10k_basedir under each case so that we can do $::settings::codedir instead on PE 2015.x. This produces /etc/puppetlabs/code/environments instead. I then moved $modulepath underneath this because it uses $r10k_basedir to construct itself.

Next I moved $r10k_config_file under each each case so to avoid r10k's warning message that /etc/r10k.yaml is soon to be depricated. For PE 2015.x, it is set to /etc/puppetlabs/r10k/r10k.yaml instead. Other PEs and FOSS have the same values.

[acidprime]

Any PE was setup to provide a default environment directory of /etc/puppetlabs/puppet/environments by using the $::settings::confdir variable. I moved $r10k_basedir under each case so that we can do $::settings::codedir instead on PE 2015.x.
The diff shows this as $::settings::environmentpath which would be /etc/puppetlabs/puppet/environments on PE 3 and /etc/puppetlabs/code/environments on PE 2015 so I am not clear on the benefit of using codedir var here instead

This produces /etc/puppetlabs/code/environments instead. I then moved $modulepath underneath this because it uses $r10k_basedir to construct itself.

+    $r10k_config_file = '/etc/puppetlabs/r10k/r10k.yaml'
+    $modulepath       = "${r10k_basedir}/\$environment/modules:${pe_module_path}"

setting modulepath is not longer valid in Puppet 4 and so this support is really not needed in terms of this re-use trick, unless I am missing something.

Next I moved $r10k_config_file under each each case so to avoid r10k's warning message that /etc/r10k.yaml is soon to be depricated. For PE 2015.x, it is set to /etc/puppetlabs/r10k/r10k.yaml instead. Other PEs and FOSS have the same values.

So I have been avoiding this because of pe_r10k I did add 7f4bc3f which may mitigate this concern. I guess at this point the dealo is that this makes it not have the deprecation warning. Adrian has told me that this will be supported until r10k 3 so I guess this default is really about removing the warning, not really functionality needed until then. This was already discussed in #189 as well, but given 7f4bc3f I am tempted to actually do this as long as we test a scenario where a PE user uses the new answers in the installer https://docs.puppetlabs.com/pe/latest/install_complete_answer_file_reference.html#qpuppetmasterr10kremotegityourgitservercompuppetcontrolgit which will result in pe_r10k being classified. The check in 7f4bc3f should at least given them an error, but overall I have been wanting to just avoid the duplicate declaration error that might occur between both classes managing the same file. I guess we need to check if it makes it to the fail or if the duplicate declaration check runs first before I am ok merging this @WhatsARanjit can you give that a try and report back tomorrow?

[WhatsARanjit]

Yeah, so we do have a duplicate declaration. I used the answers file to provide the remote and then also added r10k myself.

Error: Could not retrieve catalog from remote server: Error 400 on SERVER: Evaluation Error: Error while evaluating a Resource Statement, Duplicate declaration: File[r10k.yaml] is already declared in file /etc/puppetlabs/code/environments/production/modules/r10k/manifests/config.pp:89; cannot redeclare at /opt/puppetlabs/puppet/modules/pe_r10k/manifests/config.pp:62 at /opt/puppetlabs/puppet/modules/pe_r10k/manifests/config.pp:62:3 on node master.puppetlabs.vm

[acidprime]

Yeah at this point I would rather not have the module try and manage the new path by default as it will cause upgrade headaches for pe customers and advanced users can pass it in