PostgreSQL integration #11

ulvida · 2020-02-24T03:23:49Z

As previously discussed, this PR with PostgreSQL option for the sympa database.

As initially in our code we install Postgres in the role itself, in order to act similarly for pgsql and mysql, I added a flag that gives the option to install the database package within the role itself or outside. And, by the way, I added some requirements checks.

As far as tested, all the four options work, meaning that the role doesn't fail and let Sympa installed and running, with its database configured. However, as the role doesn't configure Apache with Sympa web interface, I couldn't fully test yet all functionalities. It would be good if you could test also.

I will be working in the integration of Apache configuration for Sympa, with robots and other features.

Also, it seems to me that the role still makes some assumptions about the host, particularly the presence of postfix or other sendmail-like MTA. I will be also working on it, as well as DKIM integration.

ulvida · 2020-02-24T03:30:51Z

Post Scriptum: I struggled a lot with debconf sympa parameters (see #4). Particularly, it seems that sympa/dbconfig-install has no effect here, as Sympa, when seeing a sympa.conf already present performs an upgrade, that configures the empty database provided.

In your initial code sympa/dbconfig-install was set to no, now it's set to true, but aparently it doesn't matter.

Scriptkiddi

Looks good already, I have to test it though.
Could you also please add the 2 new variables to the documentation.
Have you tested the postgresql capitalization for the config file? Can we transform it to a lowercase string?

Scriptkiddi · 2020-02-24T09:12:58Z

tasks/postgresql.yml

+  - name: check if postgres is installed
+    assert:
+      that: ansible_facts.packages['postgresql'] is defined
+      fail_msg: "No PostgreSQL pachage found. We stop, because we can't install sympa without its database. Sorry."


Scriptkiddi · 2020-02-24T09:13:18Z

tasks/mysql.yml

+        ansible_facts.packages['mysql-server'] is defined or
+        ansible_facts.packages['mariadb-server-10.1'] is defined   
+      ## The last condition could be replaced by a jinja2 json_query filter to match any version 
+      fail_msg: "No MySQL pachage found. We stop, because we can't install sympa without its database. Sorry."


Scriptkiddi · 2020-02-24T09:13:42Z

tasks/postgresql.yml

+
+## Debconf keys for sympa database
+
+- name: Definir opciones debconf de sympa, relativas a la base PostgreSQL


English please :)

Sorry... copy/paste... :)

Scriptkiddi · 2020-02-24T09:16:53Z

tasks/main.yml

+      ## set the database type
+  - { name: 'sympa', question: 'sympa/database-type', value: '{{ sympa_db_type }}' , vtype: select }
+      ## Configure the database at sympa installation with dbconfig
+  - { name: 'sympa', question: 'sympa/dbconfig-install', value: 'true' , vtype: boolean }


do we want this setting? Arent we configuring the database on our own

I tried again: for PostgreSQL, when I set this to false, sympa installation fails with the message:

Database sympa defined in sympa.conf has not the right structure or is unreachable. verify db_xxx parameters in sympa.conf

(and I tried again with postgresql or PostgreSQL for sympa parameter db_type, and that's the same).

For MySQL, when I tried yesterday there was no difference wether I put true or false. I'll try to figure out what is the difference between PostgreSQL and MySQL installation.

I'm too puzzled with this dbconfig parameter. I think my initial interpretation was right: seeing sympa.conf file, sympa tries an upgrade but, with PostreSQL, fails when running:

# /usr/lib/sympa/bin/sympa.pl --health_check

One strange thing is that I don't find any reference to db_passwd in database drivers code. dbconfig must do something that I don't identify, that solves this issue.

In order to avoid staying stucked, I propose you to set sympa/dbconfig-install to true for PostgreSQL and to maintain it set to false for MySQL.

Possible solution here. I'll test again.

Possible solution here. I'll test again.

Eureka! Yes, it did!

Scriptkiddi · 2020-02-24T09:17:45Z

defaults/main.yml

@@ -113,4 +117,5 @@ sympa_default_home: home
 sympa_edit_list: owner
 sympa_ldap_force_canonical_email: 1
 sympa_review_page_size: 25
+sympa_webserver_type: Other                        # 'Other' or 'Apache 2'


lets use lowercase strings without a space

Well, this variables is there to set a debconf parameter of select type. When you install sympa manually, and after checking the debconf db with debconf-get-selections it clearly proposes "Other" and "Apache 2".

I did some test, pre-setting the parameter with:

echo "sympa wwsympa/webserver_type select <value>" | debconf-set-selections

trying with <value> set to Other, other, apache2, Apache 2, etc.

Without changing debconf priorities, when setting the value to Other it silently preserves this value. When setting other values, it silently overwrites to the default value Apache 2.

So I think that there, we have to stick to the exact spelling proposed for the two debconf select options.

BTW, I struggled a lot with sympa debconf parameter. Sympa has a quite peculiar behaviour: most of the parameters are low o medium priority (so they aren't asked for), several are "internal", and most of all, they seem to be heavily overwritten at installation with default values, eventually built from linux hostname.

Scriptkiddi · 2020-02-24T09:18:36Z

tasks/postgresql.yml

+  loop:
+      ## Set the sympa database password
+  - { name: 'sympa', question: 'sympa/pgsql/app-pass', value: '{{ sympa_db_password | mandatory }}' , vtype: password }
+...


what are those dots for?

It's YAML syntax for the end of a YAML section. It's not needed by Ansible, but I prefer to stick to the upstream syntax over Ansible pidgin.

tasks/postgresql.yml

Scriptkiddi · 2020-02-24T09:19:19Z

tasks/postgresql.yml

+---
+## PostgreSQL installation and configuration for sympa
+
+- name: check PostgreSQL installation


Please start with a capital Letter

tasks/mysql.yml

Scriptkiddi · 2020-02-24T09:19:40Z

tasks/mysql.yml

+    package_facts:
+      manager: auto
+
+  - name: check if MySQL is installed


Please start with a capital letter

ulvida · 2020-02-24T14:54:34Z

Looks good already, I have to test it though.

Good! No hurry..

Could you also please add the 2 new variables to the documentation.

Right! (going to bed I realized I forgot to update the README... :)

Have you tested the postgresql capitalization for the config file? Can we transform it to a lowercase string?

I sticked to sympa documentation, considering also that letter case matters in Perl. However, I just tried with postgresql and it works, and the very same documentation mentions different letter cases for MySQL. So Ok, let's try to use lower case.

ulvida · 2020-02-24T15:38:04Z

PostgreSQL or postgresql?

My colleague @santiagomr tackles me arguing that proper names start with a capital case letter. And PostgreSQL project itself uses this ElephantC... sorry CamelCase wording.

On another hand, Debian packages' names are all lower case, and I understand the principle you propose to use only lower case in variable values.

So I propose you: let's use lower case (and no space) to refer to packages and, as far as possible, for variable values, but accept capital letters when refering to a proper name. Do you agree?

…debconf parameter

ulvida · 2020-02-24T17:20:11Z

tasks/main.yml

+    name:
+    - debconf
+    - debconf-utils
+    state: latest


I added this requirements' check, as suggested by Ansible documentation, even if it seems to work without debconf-utils package.

ulvida · 2020-02-25T18:25:46Z

Have you tested the postgresql capitalization for the config file? Can we transform it to a lowercase string?

I sticked to sympa documentation, considering also that letter case matters in Perl. However, I just tried with postgresql and it works, and the very same documentation mentions different letter cases for MySQL. So Ok, let's try to use lower case.

Well, despite the fact that on the first run of the playbook It works ok with lowercase postgresql, when I re-run it on a server installed, then sympa falls to a misconfigured state with systemctl giving the message:

feb 25 15:04:59 cazuela sympa_msg[17412]: err main::#87 > main::_load#314 > Sympa::DatabaseManager::instance#54 > Sympa::Database::new#62 Unable to use Sympa::DatabaseDriver::postgresql module: Can't locate Symp...

This Sympa::DatabaseDriver::postgresql is a reference to the Perl code files of sympa, so I think it needs the correct case spelling.

defaults/main.yml

…, should close PR stuvusIT#11, v0.2.1-pr11

ulvida · 2020-02-26T12:42:57Z

Have you tested the postgresql capitalization for the config file? Can we transform it to a lowercase string?
After my heavily checked last commit, I tried again with lower case postgresql for db_type sympa.conf parameter, and then sympa package installation clearly hangs with message:

Database sympa defined in sympa.conf has not the right structure or is unreachable. verify db_xxx parameters in sympa.conf

So precise spelling is clearly needed, as part of Perl code.

According to documentation (PosgreSQL and MySQL), Pg and mysql are deprecated/obsoleted values of this parameter.

ulvida · 2020-02-26T13:51:00Z

I think I'm done with this PR. Whenever you can review it, I have more proposals for you.

If you want, I can squash my commits, eventually preserving the versioned ones.

ulvida added 2 commits February 23, 2020 22:45

preparing pg integration, mysql check and optional installation

bf26fe6

PostgreSQL integration. For first PR of #4

555e70d

Scriptkiddi suggested changes Feb 24, 2020

View reviewed changes

According to @Scriptkiddi code review, except sympa/dbconfig-install …

ef2b37c

…debconf parameter

ulvida commented Feb 24, 2020

View reviewed changes

ulvida requested a review from Scriptkiddi February 24, 2020 17:24

ulvida force-pushed the issue2-upstream branch from 614368f to ef2b37c Compare February 24, 2020 18:26

value of sympa/dbconfig-install different for PostgreSQL and for MySQL

2d892ad

ulvida mentioned this pull request Feb 24, 2020

Integrating our code into upstream UdelaRInterior/ansible-sympa#4

Open

ulvida changed the title ~~PostgreSQL integration~~ WIP: PostgreSQL integration Feb 25, 2020

README update / database port - stuvusIT#11, for v0.2.0-pr11

e0ec6ac

ulvida force-pushed the issue2-upstream branch from 1077f85 to e0ec6ac Compare February 25, 2020 20:25

ulvida commented Feb 26, 2020

View reviewed changes

defaults/main.yml Show resolved Hide resolved

debconf sympa/dbconfig-install parameter solved, latest sympa package…

72f698b

…, should close PR stuvusIT#11, v0.2.1-pr11

ulvida changed the title ~~WIP: PostgreSQL integration~~ PostgreSQL integration Feb 26, 2020

db_type values, README, v0.2.2-pr11

2b5a25e

ulvida force-pushed the issue2-upstream branch from 0ef76c0 to 2b5a25e Compare February 26, 2020 14:07

ulvida mentioned this pull request Mar 5, 2020

Showing our work togather #12

Open

Scriptkiddi approved these changes Mar 7, 2020

View reviewed changes

Scriptkiddi merged commit 37f2ad6 into stuvusIT:master Mar 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PostgreSQL integration #11

PostgreSQL integration #11

ulvida commented Feb 24, 2020

ulvida commented Feb 24, 2020

Scriptkiddi left a comment

Scriptkiddi Feb 24, 2020

Scriptkiddi Feb 24, 2020

Scriptkiddi Feb 24, 2020

ulvida Feb 24, 2020

Scriptkiddi Feb 24, 2020

ulvida Feb 24, 2020

ulvida Feb 24, 2020

ulvida Feb 26, 2020

ulvida Feb 26, 2020

Scriptkiddi Feb 24, 2020

ulvida Feb 24, 2020

Scriptkiddi Feb 24, 2020

ulvida Feb 24, 2020

Scriptkiddi Feb 24, 2020

Scriptkiddi Feb 24, 2020

ulvida commented Feb 24, 2020 •

edited

Loading

ulvida commented Feb 24, 2020 •

edited

Loading

ulvida Feb 24, 2020

ulvida commented Feb 25, 2020

ulvida commented Feb 26, 2020

ulvida commented Feb 26, 2020 •

edited

Loading


		## Debconf keys for sympa database

		- name: Definir opciones debconf de sympa, relativas a la base PostgreSQL

PostgreSQL integration #11

PostgreSQL integration #11

Conversation

ulvida commented Feb 24, 2020

ulvida commented Feb 24, 2020

Scriptkiddi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ulvida commented Feb 24, 2020 • edited Loading

ulvida commented Feb 24, 2020 • edited Loading

Choose a reason for hiding this comment

ulvida commented Feb 25, 2020

ulvida commented Feb 26, 2020

ulvida commented Feb 26, 2020 • edited Loading

ulvida commented Feb 24, 2020 •

edited

Loading

ulvida commented Feb 24, 2020 •

edited

Loading

ulvida commented Feb 26, 2020 •

edited

Loading