do not recreate log_dir if it is already a symlink

[level-a]

Pull Request (PR) description

do not recreate log_dir if it is already a symlink

[puppet-community-rangefinder]

nginx::config is a class

that may have no external impact to Forge modules.

This module is declared in 9 of 578 indexed public Puppetfiles.

---

These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

[smortex]

Hum… I would precisely want the module to replace the symlink with a directory if it existed 🤔…

Maybe add a parameter like Boolean $manage_logdir = true and conditionally manage this resource?

[smortex]

The unit tests take forever to run but maybe it's worth updating them to check this new parameter in spec/classes/nginx_spec.rb?

[level-a]

bundle exec rake parallel_spec finished for 11m for me
not sure if I know how to properly add it to nginx_spec.rb
smth like this to RedHat and Debian section?

context 'manage_log_dir => true' do
  let(:params) { { manage_log_dir: true } }
  it { is_expected.to contain_file('/var/log/nginx') }
end

[smortex]

@level-a near line 342 / 352 add the replace parameter to ensure it has the proper false default value, then add a test like above with a true value and check that the non-default value is reflected in the file resource parameters.

---

When done, you can combine your commits in a single one (squash):

From your working directory:

git rebase -i origin/master # Interactively rewrite history

This will bring your editor. Keep the first line unchanged, and replace pick with squash for all subsequent lines. Save and quit, your editor will pop again and allow you to combine the commit messages in a single commit message.

Then push your updated branch to update this PR:

git push -f            # Send the changes (-f is required because we re-wrote history)

[root-expert]

Maybe I'm missing something here, but why do we have to duplicate the spec on both Debian and RedHat?

Can't we add it to a common place? (for all supported OSes)

[root-expert]

LGTM