Configuration-based installation of OpenShift and Cloud Pak for Data/Integration/Watson AIOps on various private and public cloud infrastructure providers. Deployment attempts to achieve the end-state defined in the configuration. If something fails along the way, you only need to restart the process to continue the deployment.
As I was working on PR #826 , I noticed a lot of possible improvements in the scripts creating the Postgres DB.
Here are my submissions ; I think this should improve the global readability of the ansible scripts.
I also have a question about a part of the scripts, cf. to this other question at end of this PR description.
TL;DR
This patch fix multiple discrepancies between some identical pg SQL snippets, some incoherence/useless code and simplify the global readability of those scrips (in my opinion — Feel free to debate with me on this! 😄 )
List of Changes
57d7142eb7820c0fc24283ecd21fa84656552b40 : Remove a duplicated ansible script for creating the DB proj2 for ADP
972a60aa9a0ad838ee2eeffcf3da12569e127e7f : Replace all SQLCREATE ROLE with the simpler CREATE USER that has better intent on the expected usage for this ROLE
b1c47daee2f2105df4842f47b1e43646ac312d97 : Remove some unnecessary GRANT as the database/tablespace OWNERs already have those permission
f992230ab54a80c7b59e2a23a94660723c559dd3 : Fix some CREATE TABLESPACE not used during database creation
8d706c2fbcdccfcabe005ccf6c2a16f5849bc095 : Normalize other database scripts using default tablespace
bf4b9dc4b612888747d5a95bc3c4ed02729ca1fc : Remove unnecessary AUTHORIZATION for basdb users owning the created schema
719097c41db8343bfad5cfdaf0eeab101e5c66af : Revoke all connection rights on databases for public users.
In my opinion, it would be clearer to remove the _tbs suffix from the role and directly add it on each item in the with_items: list for these reasons:
Ease of reading/less mental mapping for maintainers that don't know/forgot that this suffix is automatically added to each item name passed to the role remove-postgresql-tablespace.
Coherence with the db creation syntax that is referencing the full name of the tablespace :
E.g.: CREATE DATABASE aeos OWNER aeos TEMPLATE template0 ENCODING UTF8 TABLESPACE aeos_tbs;
Hello again cpd team!
As I was working on PR #826 , I noticed a lot of possible improvements in the scripts creating the Postgres DB. Here are my submissions ; I think this should improve the global readability of the ansible scripts.
I also have a question about a part of the scripts, cf. to this other question at end of this PR description.
TL;DR
This patch fix multiple discrepancies between some identical pg SQL snippets, some incoherence/useless code and simplify the global readability of those scrips (in my opinion — Feel free to debate with me on this! 😄 )
List of Changes
CREATE ROLEwith the simplerCREATE USERthat has better intent on the expected usage for thisROLEGRANTas the database/tablespace OWNERs already have those permissionCREATE TABLESPACEnot used during database creationAUTHORIZATIONforbasdbusers owning the created schemapublicusers.This other question
As I was going through the scripts, I noticed that the ansible task for removing the created pg tablespaces use the name of the database instead of the real name of the tablespace: e.g.:
devos1instead ofdevos1_tbs. https://github.com/IBM/cloud-pak-deployer/blob/08acfccba46afbc5421c0dcf1f7875eb7d1b7852/automation-roles/50-install-cloud-pak/cp4ba/cp4ba-core/tasks/remove.yml#L328-L347The
_tbspart is added under the hood in this ansible Role (l.27) : https://github.com/IBM/cloud-pak-deployer/blob/08acfccba46afbc5421c0dcf1f7875eb7d1b7852/automation-roles/50-install-cloud-pak/cp4ba/common/tasks/remove-postgresql-tablespace.yml#L20-L30In my opinion, it would be clearer to remove the
_tbssuffix from the role and directly add it on each item in thewith_items:list for these reasons:remove-postgresql-tablespace.CREATE DATABASE aeos OWNER aeos TEMPLATE template0 ENCODING UTF8 TABLESPACE aeos_tbs;I'm waiting for your feedback! 😄