Rhel support for mistral and create postgresql role (remove AXNS.postgresql dependency) #78

humblearner · 2017-01-11T23:37:30Z

Covers #76 and #11

humblearner · 2017-01-12T01:22:42Z

roles/mistral/tasks/main.yml

 - name: Install latest st2mistral
  become: yes
-  apt: 
+  package:
    name: st2mistral
    state: latest


Not sure if we should keep it latest or use present

Yeah, it's good.
The idea here to install latest mistral eg. get upgrades, but only when condition is met in this block.

arm4b

While this PR is still WIP, I left some early comments, - hopefully that'll help.

arm4b · 2017-01-12T11:14:56Z

roles/postgresql/tasks/postgresql_yum6.yml

+  yum:
+    name: http://yum.postgresql.org/9.4/redhat/rhel-6-x86_64/pgdg-redhat94-9.4-2.noarch.rpm
+    state: installed
+    update_cache: yes


update_cache: yes will break Ansible idempotence. Avoid using where possible

arm4b · 2017-01-12T11:20:01Z

roles/postgresql/tasks/postgresql_yum7.yml

+
+- name: yum | MD5-encrypted password for postgres 1
+  become: yes
+  command: 'sed -i "s/\(host.*all.*all.*127.0.0.1\/32.*\)ident/\1md5/" /var/lib/pgsql/data/pg_hba.conf'


Use http://docs.ansible.com/ansible/lineinfile_module.html instead

arm4b · 2017-01-12T11:21:00Z

roles/postgresql/tasks/postgresql_yum7.yml

+
+- name: yum | MD5-encrypted password for postgres 2
+  become: yes
+  command: 'sed -i "s/\(host.*all.*all.*::1\/128.*\)ident/\1md5/" /var/lib/pgsql/data/pg_hba.conf'


Same, Ansibe module http://docs.ansible.com/ansible/lineinfile_module.html can help to make sure the command is Ansible-idempotent

arm4b · 2017-01-12T11:22:51Z

stackstorm.yml

@@ -5,6 +5,7 @@
    - mongodb
    - rabbitmq
    - st2repos
+    - postgresql
    - mistral


Please don't forget to rename misral to st2mistral as requested in #11

So we'll use the following roles responsible for StackStorm:

st2

st2mistral

st2web

st2chatops in future

for consistency.

arm4b · 2017-01-12T11:23:25Z

roles/postgresql/tasks/postgresql_yum7.yml

+  service:
+    name: postgresql
+    state: started
+  tags: [db, postgresql]


Additional "enable" action is missing

arm4b · 2017-01-12T11:25:54Z

roles/postgresql/tasks/postgresql_yum7.yml

@@ -0,0 +1,34 @@
+---
+- name: yum | Initialize postgresql


Probably you meant Install postgresql, because next block is Initialize postgresql

arm4b · 2017-01-12T11:29:58Z

requirements.yml

-  connection: local
-  tasks:
-  - name: Install galaxy dependencies
-    command: ansible-galaxy install -r roles/mistral/requirements.yml


arm4b · 2017-01-12T11:35:27Z

roles/postgresql/tasks/postgresql_yum6.yml

+  when: ansible_distribution == "CentOS"
+  tags: [db, postgresql]
+
+- name: yum | Get pgdg-centos94-9.4-2 for {{ ansible_distribution }}


This might be confusing. Probably it means "Setup PostgreSQL repository" or something like that.

arm4b · 2017-01-12T11:36:26Z

roles/postgresql/tasks/postgresql_yum6.yml

+- name: yum | Get pgdg-centos94-9.4-2 for {{ ansible_distribution }}
+  become: yes
+  yum:
+    name: http://yum.postgresql.org/9.4/redhat/rhel-6-x86_64/pgdg-redhat94-9.4-2.noarch.rpm


Can you check if https:// version will work here (and in previous block)? for yum install from url

arm4b · 2017-01-12T11:37:54Z

roles/postgresql/tasks/postgresql_apt.yml

@@ -0,0 +1,14 @@
+---
+- name: apt | Initialize postgresql


Probably Install PostgreSQL and so Initialize PostgreSQL step is missing.

arm4b · 2017-01-13T13:22:11Z

README.md

@@ -26,8 +26,6 @@ ansible-playbook stackstorm.yml
 ```

 ## Dependencies
-Ansible Galaxy roles used by StackStorm installation:


Please also remove another requirements.yml occurrence in README.md.

arm4b · 2017-01-13T13:22:50Z

roles/postgresql/meta/main.yaml

+        - trusty
+        - xenial
+    - name: EL
+        - 6


versions: is missing

arm4b · 2017-01-13T13:29:24Z

roles/st2mistral/tasks/main.yml

+- name: Install st2mistral dependency
+  become: yes
+  package:
+    name: st2python


Good catch 👍
I assume that's the case why mistral didn't work for you.

We install st2python as package dependency for st2, see: https://github.com/StackStorm/st2-packages/blob/a46cc2267ac4b23c8f21a3471824c7c7f430cd1a/packages/st2/rpm/st2.spec#L15

Seems like we should add the same dependency for st2mistral as well in st2-packages.

Created an Issue for that: StackStorm/st2-packages#407 so we won't forget about it and fix the root cause.

In the bootstrap scripts we install st2 and its dependencies before st2mistral, that is why it was never caught: https://github.com/StackStorm/st2-packages/blob/master/scripts/st2bootstrap-el6.sh#L653. However, we may not have another use case, besides this and docker, where anyone installs st2mistral before st2.

arm4b · 2017-01-13T13:30:00Z

roles/st2mistral/tasks/main.yml

+  package:
+    name: st2python
+    state: present
+  when: ansible_distribution == "CentOS" and ansible_distribution_major_version == "6"


ansible_distribution == "CentOS"

Will it work on RHEL6?

arm4b · 2017-01-13T13:36:28Z

roles/postgresql/tasks/postgresql_yum6.yml

+- name: yum | Initialize Postgresql-9.4
+  become: yes
+  command: service postgresql-9.4 initdb
+  tags: [db, postgresql]


Same, this command needs a "file lock" to ensure that we run initdb only once per system and don't repeat it.

OR even better, - it's possible to run init only when previous block "changed", eg. only when package was installed.

See: https://raymii.org/s/tutorials/Ansible_-_Only-do-something-if-another-action-changed.html

arm4b · 2017-01-13T13:38:16Z

roles/postgresql/tasks/postgresql_yum6.yml

+  yum:
+    name: "{{ item }}"
+    state: installed
+    update_cache: yes


Can we avoid update_cache here to make this command Ansible-idempotent friendly?

With update_cache it will always have "changed" state.

arm4b · 2017-01-13T13:40:46Z

roles/st2mistral/meta/main.yml

+        - trusty
+        - xenial
+    - name: EL
+        - 6


versions: is missing

arm4b · 2017-01-13T13:50:41Z

Overall looks nice 👍

The final thing I would like you to address is Ansible Idempotence.
Eg. Ansible-playbook re-run should end with the following results:

changed=0.*failed=0

arm4b · 2017-01-13T20:19:10Z

roles/postgresql/tasks/postgresql_yum7.yml

+- name: yum | Initialize Postgresql
+  become: yes
+  command: postgresql-setup initdb
+  when: install_postgresql.changed


Nice indeed 👍

arm4b · 2017-01-16T13:15:43Z

README.md

@@ -19,15 +19,11 @@ At least 2GB of memory and 3.5GB of disk space is required, since StackStorm is

 ## Installation
 ```sh
-# ansible galaxy roles
-ansible-playbook requirements.yml
 # stackstorm
 ansible-playbook stackstorm.yml
 ```

 ## Dependencies


This line could be removed as well, take a look at resulting README.md.

arm4b

Nice PR indeed 👍
A lot of work and in-depth testing (especially +:100: for real RHEL testing).

Besides of comment ^^ above about README.md update everything looks good to me. Let's change and if you see it's Ansible-idempotent, I'm +1 to merge.

cc @bigmstone @Mierdin please take a look as well.

arm4b · 2017-01-16T18:26:33Z

ANXS.postgresql dependency started to fail for previously successful master builds:

TASK [ANXS.postgresql : PostgreSQL | Add PostgreSQL repository] ****************

       task path: /tmp/kitchen/roles/ANXS.postgresql/tasks/install.yml:17

       fatal: [localhost]: FAILED! => {"failed": true, "msg": "The conditional check 'postgresql_apt_repository' failed. The error was: expected token 'end of statement block', got 'http'\n  line 1\n\nThe error appears to have been in '/tmp/kitchen/roles/ANXS.postgresql/tasks/install.yml': line 17, column 3, but may\nbe elsewhere in the file depending on the exact syntax problem.\n\nThe offending line appears to be:\n\n\n- name: PostgreSQL | Add PostgreSQL repository\n  ^ here\n"}

https://travis-ci.org/StackStorm/ansible-st2/jobs/191748821

So this PR which removes external dependencies is just in time!

Anirudh Rekhi added 7 commits January 11, 2017 15:33

Rhel support: Mistral and create PostgreSQL role

0a2bca9

Remove AXNS.postgresql dependency for mistral

143915c

rhel support for mistral

b1fd33d

Fix for role/postgresql

60861dc

Remove postgresql/defaults/main.yml

3b42b08

Remove requirements_path: roles/mistral/requirements.yml

8df8570

Update path for postgresql tasks

b7d92f7

humblearner changed the title ~~[WIP] Rhel support: Mistral and create PostgreSQL role~~ [WIP] Rhel support for mistral and create postgresql role (remove AXNS.postgresql dependency) Jan 12, 2017

Anirudh Rekhi added 3 commits January 11, 2017 16:33

Rename postgresql tasks to make CI happy(use ansible_pkg_mgr)

e23443e

Add tags [db, postgresql]

83be2c5

Add tags: mistral

2a44c37

humblearner commented Jan 12, 2017

View reviewed changes

humblearner added the WIP label Jan 12, 2017

Change .yaml to .yml

c76230d

arm4b suggested changes Jan 12, 2017

View reviewed changes

Anirudh Rekhi added 5 commits January 12, 2017 12:35

Rename mistral to st2mistral and minor changes

8576179

Update stackstorm.yml

127ca77

Replace sed with replace module

cddfc9a

Fix for rhel6

eb24c61

Remove update_cache

697e31d

humblearner added RFR and removed WIP labels Jan 13, 2017

humblearner changed the title ~~[WIP] Rhel support for mistral and create postgresql role (remove AXNS.postgresql dependency)~~ Rhel support for mistral and create postgresql role (remove AXNS.postgresql dependency) Jan 13, 2017

arm4b reviewed Jan 13, 2017

View reviewed changes

roles/postgresql/meta/main.yaml

- trusty

- xenial

- name: EL

- 6

Copy link

Member

arm4b Jan 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

versions: is missing

arm4b reviewed Jan 13, 2017

View reviewed changes

roles/st2mistral/meta/main.yml

- trusty

- xenial

- name: EL

- 6

Copy link

Member

arm4b Jan 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

versions: is missing

arm4b mentioned this pull request Jan 13, 2017

EL6 st2mistral should depend on st2python StackStorm/st2-packages#407

Closed

Fix as per the comments

1426c6d

arm4b reviewed Jan 13, 2017

View reviewed changes

arm4b suggested changes Jan 16, 2017

View reviewed changes

arm4b approved these changes Jan 16, 2017

View reviewed changes

arm4b mentioned this pull request Jan 16, 2017

Enable CentOS7 CI in Travis #82

Merged

Anirudh Rekhi added 2 commits January 16, 2017 12:16

Update README and make tasks idempotent

2d18979

Minor cosmetic fix

d6144fd

humblearner merged commit f5df0b6 into master Jan 16, 2017

humblearner deleted the rhel_mistral_support branch January 16, 2017 20:39

This was referenced Jan 17, 2017

PostgreSQL Role [EL] #76

Closed

Rename st2mistral vars & Misc cosmetic improvements #94

Merged

arm4b added breaking change feature labels Jan 25, 2017

blag added the OS support Support/issues/PRs on a specific OS label Apr 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rhel support for mistral and create postgresql role (remove AXNS.postgresql dependency) #78

Rhel support for mistral and create postgresql role (remove AXNS.postgresql dependency) #78

humblearner commented Jan 11, 2017 •

edited

Loading

humblearner Jan 12, 2017 •

edited

Loading

arm4b Jan 12, 2017 •

edited

Loading

arm4b left a comment •

edited

Loading

arm4b Jan 12, 2017

arm4b Jan 12, 2017

arm4b Jan 12, 2017

arm4b Jan 12, 2017

arm4b Jan 12, 2017

arm4b Jan 12, 2017

arm4b Jan 12, 2017

arm4b Jan 12, 2017

arm4b Jan 12, 2017

arm4b Jan 12, 2017

arm4b Jan 13, 2017 •

edited

Loading

arm4b Jan 13, 2017

arm4b Jan 13, 2017

arm4b Jan 13, 2017 •

edited

Loading

humblearner Jan 16, 2017 •

edited

Loading

arm4b Jan 13, 2017 •

edited

Loading

arm4b Jan 13, 2017 •

edited

Loading

arm4b Jan 13, 2017

arm4b Jan 13, 2017

arm4b commented Jan 13, 2017

arm4b Jan 13, 2017

arm4b Jan 16, 2017

arm4b left a comment

arm4b commented Jan 16, 2017 •

edited

Loading

Rhel support for mistral and create postgresql role (remove AXNS.postgresql dependency) #78

Rhel support for mistral and create postgresql role (remove AXNS.postgresql dependency) #78

Conversation

humblearner commented Jan 11, 2017 • edited Loading

humblearner Jan 12, 2017 • edited Loading

Choose a reason for hiding this comment

arm4b Jan 12, 2017 • edited Loading

Choose a reason for hiding this comment

arm4b left a comment • edited Loading

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

arm4b Jan 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arm4b Jan 13, 2017 • edited Loading

Choose a reason for hiding this comment

humblearner Jan 16, 2017 • edited Loading

Choose a reason for hiding this comment

arm4b Jan 13, 2017 • edited Loading

Choose a reason for hiding this comment

arm4b Jan 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arm4b commented Jan 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arm4b left a comment

Choose a reason for hiding this comment

arm4b commented Jan 16, 2017 • edited Loading

humblearner commented Jan 11, 2017 •

edited

Loading

humblearner Jan 12, 2017 •

edited

Loading

arm4b Jan 12, 2017 •

edited

Loading

arm4b left a comment •

edited

Loading

arm4b Jan 13, 2017 •

edited

Loading

arm4b Jan 13, 2017 •

edited

Loading

humblearner Jan 16, 2017 •

edited

Loading

arm4b Jan 13, 2017 •

edited

Loading

arm4b Jan 13, 2017 •

edited

Loading

arm4b commented Jan 16, 2017 •

edited

Loading