SLV-366 Make external database host optional #23
Conversation
plans/install.pp
Outdated
| @@ -94,7 +105,7 @@ | |||
| $master_pe_conf = epp('pe_xl/master-pe.conf.epp', | |||
| console_password => $console_password, | |||
| master_host => $master_host, | |||
| puppetdb_database_host => $puppetdb_database_host, | |||
| puppetdb_database_host => "$puppetdb_database_host", | |||
There was a problem hiding this comment.
and why double quotes on this one only?
There was a problem hiding this comment.
This parameter has to be a string. So, database_target fails because it is an array. It needs to be quoted in order to pass an empty string if it is undefined, otherwise it fails because the undef is passed rather than a string.
There was a problem hiding this comment.
I was wondering this as well. Could you please add a comment with this explanation?
There was a problem hiding this comment.
Comment added to clarify the purpose of the quotes. Thanks.
plans/install.pp
Outdated
| @@ -111,7 +122,7 @@ | |||
|
|
|||
| # Upload the pe.conf files to the hosts that need them | |||
| pe_xl::file_content_upload($master_pe_conf, '/tmp/pe.conf', $master_host) | |||
| pe_xl::file_content_upload($puppetdb_database_pe_conf, '/tmp/pe.conf', $puppetdb_database_host) | |||
| pe_xl::file_content_upload($puppetdb_database_pe_conf, '/tmp/pe.conf', $database_target) | |||
There was a problem hiding this comment.
This line looks like it would overwrite the master pe.conf if the database target was the master... Is that intended?
There was a problem hiding this comment.
database_target will be an empty array in the large deploy. So this command will not have any effect.
There was a problem hiding this comment.
database_target will be an empty array in the large deploy. So this command will not have any effect.
| **NOTE:** Currently, the module does not deploy a Large Architecture with HA. | ||
| The currently supported deployment architecture is shown below. | ||
|
|
||
|  |
johnduarte
force-pushed
the
slv-366-make-pdb-pg-optional
branch
from
df0506f
to
37db60a
Compare
This commit updates the plans, tasks, and templates to make the setting of `puppetdb_database_host` optional. Without this setting, a "large" environment will be deployed where PuppetDB resides on the master.
This commit adds a document for using the pe_xl module to deploy a "PE Large Architecture". An architecture diagram for the supported deployment is included for clarity. Currently, deploying the HA version of the Large Architecture is not supported by the tooling.
This commit makes some UX changes to varible names to new "target" variables in use, now that there are several of them. The changes aim to make each "target" clearly relate to the original input variable it was based on. This commit also tries to smooth over the epp template quoted string issue. It makes the puppetdb_database_host parameter to the template optional, so that undef is permissable to pass.
reidmv
force-pushed
the
slv-366-make-pdb-pg-optional
branch
from
37db60a
to
470f32b
Compare
|
@johnduarte I made a few changes to variable names and to try and smooth out the epp file template rough spot. Also rebased on master to incorporate some minor lint changes to keep the diff clean. Can you take a look at those changes and see if they look good to you? 470f32b. Other than that, I'm 👍 . It seems a little scope-creepy to add this to a module named "pe_xl", but the changes are small enough and the benefit big enough that it makes sense to not worry about that for now. 🙂 |
|
@reidmv thanks for the updates. They make perfect sense to me. I have confirmed that these changes do not break the deployment of an XL(HA) or a L(non-HA) architecture. 👍 |
|
Thanks @johnduarte !!! |
This PR makes the
puppetdb_database_hostan optional parameter for thepe_xlplan. This allows the plan to deploy a "PE Large Architecture".Note: This PR does not reconcile support for HA with the Large Architecture. This has been noted in the documentation.