Rename st2mistral vars & Misc cosmetic improvements #94

arm4b · 2017-01-20T19:35:20Z

We renamed mistral -> st2mistral role for consistency with other packages we install via roles (st2, st2web), but forgot to do so for respective variables.

This PR is a follow-up for #78 which actualizes variable names with a role name (it's good practice to prefix variable name with a role name).

mistral_version -> st2mistral_version
mistral_db -> st2mistral_db
mistral_db_username -> st2mistral_db_username
mistral_db_password -> st2mistral_db_password

Additionally tiny improvements:

Cosmetic fixes, more clear Task descriptions
Fix some st2mistral DB migration corner cases with "file locks" we create
Ensure mistral service is enabled and running

… role

This could be important when user changes DB connection settings from DB1 to DB2 and we change mistral.conf only when new DB is 100% ready

Configure mistral after DB creation

arm4b · 2017-01-23T15:26:14Z

So after playing a bit and taking closer look at st2mistral role, I created a new issue to improve it in some better times:

Move DB creation logic from st2mistral to postgresql role #95

arm4b · 2017-01-23T15:34:18Z

roles/st2mistral/tasks/main.yml

  tags: st2mistral

- name: Initiate database
+- name: Initiate mistral database
  become: yes
  become_user: postgres
  shell: psql < /etc/mistral/init_mistral_db.SQL


Here we have a problem when user will consider to change any of st2mistral_ database settings in future, - the SQL script will not run twice. That means, new DB is not created.

We can workaround with additional lock files, but #95 should solve it in a right way.

Mierdin · 2017-01-23T18:45:07Z

roles/st2mistral/tasks/main.yml

@@ -55,9 +42,29 @@
    - restart mistral
  tags: st2mistral

- name: Setup Mistral DB tables, etc
+- name: Make sure "Initiate mistral database" doesn't run twice


You're creating the file here, but I don't see where you're checking for the presence of the file in the previous task (i.e. no when clause)

See: creates: /etc/mistral/init_mistral_db.SQL.ansible.has.run in previous task.
If file from the creates exists, - the command won't be executed.

See: http://docs.ansible.com/ansible/shell_module.html for more info.

Ah, okay I get it now.

Mierdin · 2017-01-23T18:46:18Z

roles/st2mistral/tasks/main.yml

+    - restart mistral
+  tags: st2mistral
+
+- name: Setup mistral DB tables, etc


Same as above - don't currently see how this is idempotent, since there is no when clause

Similar example with creates: https://github.com/StackStorm/ansible-st2/pull/94/files/2b9a44658672559fff187610578832c22a6c2a0c#r97388431

Mierdin · 2017-01-23T18:46:52Z

roles/st2mistral/tasks/main.yml

@@ -67,11 +74,19 @@

 - name: Register mistral actions
  become: yes
-  shell: /opt/stackstorm/mistral/bin/mistral-db-manage --config-file /etc/mistral/mistral.conf populate && touch /etc/mistral/mistral-db-manage.upgrade.head.ansible.has.run
+  shell: /opt/stackstorm/mistral/bin/mistral-db-manage --config-file /etc/mistral/mistral.conf populate && touch /etc/mistral/mistral-db-manage.populate.ansible.has.run


We touch the file first, find: && touch /etc/mistral/mistral-db-manage.populate.ansible.has.run
and later in block don't execute it if touch exists:

creates: /etc/mistral/mistral-db-manage.populate.ansible.has.run

Mierdin

👍 LGTM

humblearner

👍

arm4b · 2017-01-23T19:51:44Z

@humblearner I just moved that mistral.conf task after DB creation stage.

So DB is created first and then we change the mistral.conf setting if DB creation was successful.
Logically, it looks a bit better order for me.

arm4b added breaking change enhancement labels Jan 20, 2017

arm4b added this to the v0.6.0 milestone Jan 20, 2017

arm4b added the WIP label Jan 20, 2017

arm4b force-pushed the fix/rename-mistral-vars branch from a2b7eca to 5f4e848 Compare January 20, 2017 19:36

armab added 6 commits January 23, 2017 14:44

Rename var mistral_version -> st2mistral_version for st2mistral role

7d71a86

Rename DB-related vars mistral_db -> st2mistral_db for st2mistral…

1c9623c

… role

Add comments for default st2mistral vars

0a4a531

Adjust st2mistral meta

d1ebb5a

Improve st2mistral task descriptions

52727e7

Deploying database init script shouldn't restart mistral

7060d20

arm4b force-pushed the fix/rename-mistral-vars branch from da8bd52 to 7060d20 Compare January 23, 2017 12:45

armab added 8 commits January 23, 2017 15:23

Change mistral.conf connection only after DB full setup

e750492

This could be important when user changes DB connection settings from DB1 to DB2 and we change mistral.conf only when new DB is 100% ready

Fix correct lock files for mistral DB migrations

bad365b

Fix idempotence for "Create mistral database" task

2e34d64

Configure mistral after DB creation

Ensure mistral service is enabled and running

ad0a1b4

Add missing st2mistral tag

b05d27f

Fix idempotence for "Initiate mistral database" task once again

f36ff60

Skip ansible lint for st2mistral lock files

fe51af7

Skip ansible lint for st2mistral lock files

7658acf

arm4b added RFR and removed WIP labels Jan 23, 2017

arm4b requested review from Mierdin and humblearner January 23, 2017 15:26

arm4b changed the title ~~Rename st2mistral vars~~ Rename st2mistral vars & Misc cosmetic improvements Jan 23, 2017

arm4b commented Jan 23, 2017

View reviewed changes

Use 'ansible_os_family' to cover both CentOS & RHEL

2b9a446

Mierdin reviewed Jan 23, 2017

View reviewed changes

Mierdin approved these changes Jan 23, 2017

View reviewed changes

humblearner self-requested a review January 23, 2017 19:50

humblearner approved these changes Jan 23, 2017

View reviewed changes

arm4b merged commit 156a084 into master Jan 23, 2017

arm4b deleted the fix/rename-mistral-vars branch January 23, 2017 19:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename st2mistral vars & Misc cosmetic improvements #94

Rename st2mistral vars & Misc cosmetic improvements #94

arm4b commented Jan 20, 2017 •

edited

Loading

arm4b commented Jan 23, 2017

arm4b Jan 23, 2017 •

edited

Loading

Mierdin Jan 23, 2017

arm4b Jan 23, 2017 •

edited

Loading

Mierdin Jan 23, 2017

Mierdin Jan 23, 2017

arm4b Jan 23, 2017

Mierdin Jan 23, 2017

arm4b Jan 23, 2017 •

edited

Loading

Mierdin left a comment

humblearner left a comment

arm4b commented Jan 23, 2017

Rename st2mistral vars & Misc cosmetic improvements #94

Rename st2mistral vars & Misc cosmetic improvements #94

Conversation

arm4b commented Jan 20, 2017 • edited Loading

arm4b commented Jan 23, 2017

arm4b Jan 23, 2017 • edited Loading

Choose a reason for hiding this comment

Mierdin Jan 23, 2017

Choose a reason for hiding this comment

arm4b Jan 23, 2017 • edited Loading

Choose a reason for hiding this comment

Mierdin Jan 23, 2017

Choose a reason for hiding this comment

Mierdin Jan 23, 2017

Choose a reason for hiding this comment

arm4b Jan 23, 2017

Choose a reason for hiding this comment

Mierdin Jan 23, 2017

Choose a reason for hiding this comment

arm4b Jan 23, 2017 • edited Loading

Choose a reason for hiding this comment

Mierdin left a comment

Choose a reason for hiding this comment

humblearner left a comment

Choose a reason for hiding this comment

arm4b commented Jan 23, 2017

arm4b commented Jan 20, 2017 •

edited

Loading

arm4b Jan 23, 2017 •

edited

Loading

arm4b Jan 23, 2017 •

edited

Loading

arm4b Jan 23, 2017 •

edited

Loading