Support multiple envs_dir directories

[optiz0r]

This module adds support for the puppet environmentpath setting to be configured
using multiple directories. An example use-case is where r10k is in use to
deploy some but not all environments, to avoid unmanaged environments being
purged by r10k.

• Each listed environmentpath directory will be created and managed by puppet
• The default post-receive hook script uses the first listed directory in
  envs_dir to create initial environments to maintain backward compatibility.
• The config_version_cmd is updated to use only the first listed directory from
  envs_dir, this is only used in the environment.conf.erb template which does
  not appear to be deployed anywhere in the current version of this module

Tests are included to ensure that all listed directories are created and
managed, and that any other references to the envs_dir parameter behave the
same as if only a single directory were specified to maintain backward
compatibility.

Fixes #708

[ekohl]

I think keeping the data type of envs_dir on Stdlib::Absolutepath is misleading. If it's an array, really make it an array: Variant[Array[Stdlib::Absolutepath, 1], Stdlib::Absolutepath].

[optiz0r]

I think keeping the data type of envs_dir on Stdlib::Absolutepath is misleading. If it's an array, really make it an array: Variant[Array[Stdlib::Absolutepath, 1], Stdlib::Absolutepath].

A wrinkle: this is going to need corresponding changes to the foreman-installer too, which currently takes a single string parameter and passes it straight through. This also introduces a potential compatibility break between newer versions of the foreman-installer and older versions of this puppet module, where a newer installer would pass an array but an older puppet module would only accept a string. Would that be a major concern?

[ekohl]

That's why I suggested a Variant. That would accept both a string and an array.

[optiz0r]

I didn't mean on the puppet module side, the actual foreman installer which uses this module only knows how to pass a string at the moment. It does not know how to pass an array. The change in behaviour to pass an array of strings instead might be breaking, particularly considering both forwards and backwards compatibility.

[ekohl]

I think it should handle a Variant already but it's a good point that it should be tested. We can also only accept an array and make write a migration. That may be easier in the code too and we're doing a backwards incompatible release next anyway.

[ekohl]

I'm very close to releasing a major version of the module. Would you like to update this so it's an array? I can write the migration on the installer side.

[optiz0r]

@ekohl Do you mean switch from Variant[Array[Stdlib::Absolutepath, 1], Stdlib::Absolutepath] to just Array[Stdlib::Absolutepath, 1]? If so yes, I can do that this weekend.

[ekohl]

That is indeed what I mean. I can wait with a major version until Monday.

[optiz0r]

Updated to drop taking a single path as a string (breaking change), and rebased against current master.

I've been monkey patching a variant of this change that takes multiple paths as colon-separated strings, since the foreman-installer only passes the arg as a string. I am running foreman nightly, so if you are able to update the installer to pass an array, I'm happy to try testing that against this branch before you roll a release to confirm all working as expected.

[ekohl]

Very minor thing, other than that 👍

[ekohl]

You don't need to modify this since you can pass an array to file.

[ekohl]

I think theforeman/foreman-installer#740 would be the installer migration.

[optiz0r]

Will fix in the morning. This is leftover from handling the more complex variant case. I will revert this hunk and force push.

…

On Fri, 4 Feb 2022, 22:34 Ewoud Kohl van Wijngaarden, < ***@***.***> wrote: ***@***.**** requested changes on this pull request. Very minor thing, other than that 👍 ------------------------------ In manifests/server/config.pp <#803 (comment)> : > + $puppet::server::envs_dir.each |$dir| { + file { $dir: + ensure => $ensure, + owner => $puppet::server::environments_owner, + group => $puppet::server::environments_group, + mode => $puppet::server::environments_mode, + target => $puppet::server::envs_target, + force => true, + } } You don't need to modify this since you can pass an array to file. — Reply to this email directly, view it on GitHub <#803 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAH3VL523OXEJPHX56U7ZQTUZRIAZANCNFSM5EM5JPJA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[ekohl]

Thanks!