Description
Describe the Bug
Good morning. Maybe this is a bug, maybe a feature request, I'm not sure.
In peadm::subplans::install and in peadm::install we have the following parameter signature:
Optional[Boolean] $code_manager_auto_configure = true,
The value has a default parameter that's not undef. The datatype is undef. We cannot pass undef to it. When we call the plan and don't set the parameter, or if we pass undef, the default from the plan, true, is used. A fix would be to remove the Optional and make it a boolean:
Boolean $code_manager_auto_configure = true,
But does that make sense? $r10k_remote, $r10k_private_key_file and $r10k_private_key_content are optional. So by default we enable code manager but don't configure any repository. This will lock r10k, we're not able to call r10k directly anymore. But that might be desired for bootstrapping. We're now forced to add a repo via the PE console or disable the code manager.
Expected Behavior
I think the default value for $code_manager_auto_configure should be undef. People can explicitly enable or disable it. If $r10k_remote and $r10k_private_key_file or $r10k_private_key_content are set and $code_manager_auto_configure isn't false, we should enable code manager.
Activity
(puppetlabs#351) code_manager: Switch default to `undef`
bastelfreak commentedon May 19, 2023
I hacked a possible fix/enhancement in #352. Please let me know what you think about it
(puppetlabs#351) code_manager: Switch default to `undef`
(puppetlabs#351) code_manager: Switch default to `undef`
(puppetlabs#351) code_manager: Switch default to `undef`
ragingra commentedon May 19, 2023
Good evening Tim!
I’ve done a little asking around about this and believe that this is fine as is. The reason it's there is that you can't add a replica or a compiler unless code manager is on. When we made the param configurable we left the default as true to not interfere with other users. https://github.com/puppetlabs/puppetlabs-peadm/pull/341
If we were to set it to undef, the value would be automatically set to false which might cause issues. https://www.puppet.com/docs/puppet/8/lang_data_boolean.html#lang_data_boolean-automatic-conversion-boolean
Let me know if that’s of any help!
bastelfreak commentedon May 22, 2023
With the current setup I don't see an easy option for people to configure code manager later on. I cannot deploy a temporary control repo with r10k because it's locked. And values from the classifier overrule hiera. Even if I can bootstrap code somehow, I cannot change code manager because it's hardcoded in the node group.
And not every setup has a replica/compiler.
I think the logic in the plan could be reworked:
$code_manager_auto_configureto undef$r10k_remoteand$r10k_private_key_fileor$r10k_private_key_contentare set and$code_manager_auto_configureisn't false, enable it$code_manager_auto_configureis undef, enable it(puppetlabs#351) code_manager: Switch default to `undef`
(puppetlabs#351) code_manager: Switch default to `undef`
(puppetlabs#351) code_manager: Switch default to `undef`
CoMfUcIoS commentedon Sep 19, 2023
So to understand this right, we shouldn't configure code manager automatically if it isn't required by installation.
So a logic like the following is what we need here?
If DR is required configure code manager, else let the user decide through the
code_manager_auto_configureif he/she wants us to configure it(puppetlabs#351) code_manager: Switch default to `undef`
bastelfreak commentedon Sep 27, 2023
@CoMfUcIoS yes, that's essentially what my PR is about. Make the setup of code manager optional. The PR doesn't change the default behaviour. It just enables users to not setup code-manager during the peadm::install plan.
5 remaining items