Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EL6/7 and CentOS 6/7 support for st2 and st2repos #66

Merged
merged 45 commits into from
Jan 21, 2017
Merged
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
36329a6
Initial pass at adding RH-capable provisioning for 'st2' and 'st2repo…
Jan 4, 2017
8f1e76b
Minor cleanup; now using ansible module for yum config
Jan 4, 2017
6a4467f
Simplified include statements
Jan 5, 2017
5a1f2e4
Disabled GPG check for unstable as well
Jan 5, 2017
c40a809
Added {{ st2_pkg_repo }} for redhat install
Jan 5, 2017
f6ac495
Added GPG key ID for debian repo
Jan 5, 2017
f87c472
Merge branch 'master' into centos-st2-role
Jan 17, 2017
81af829
Added epel role
Jan 18, 2017
455c7f4
Removed reference to in yum config
Jan 18, 2017
8877d9d
Added hack to replace with simpler version number to work around issue
Jan 18, 2017
2c388f3
Added dependency on epel role for mongodb role
Jan 18, 2017
9f5c41e
Disabled GPG check for unsigned 'st2mistral' package
Jan 18, 2017
df414fe
updated readme with intended supported platforms
Jan 19, 2017
c3ee82f
Added supported platforms to meta/
Jan 19, 2017
3c1fd14
Used the 'package' module to cut down on clutter
Jan 19, 2017
025ed66
Removed update_cache
Jan 19, 2017
82f7072
Disabled GPG check for st2 package
Jan 19, 2017
7998048
Reordered the roles to make a bit more sense
Jan 19, 2017
6448f3b
Fixed all that GPG key nonsense
Jan 19, 2017
6984fae
I discovered ansible_distribution_major_version, so I'm using it
Jan 19, 2017
15ba22e
Removed EL from meta/, as it's not ready yet
Jan 19, 2017
80032d9
Removed redundant OS support in epel meta
Jan 19, 2017
09df69b
Improved var name in epel repo check
Jan 19, 2017
1904363
Updated epel task to avoid redundant 'get_url' task, and just pass UR…
Jan 19, 2017
7254866
Fixed name of repo config, and added step to update ca-certificates p…
Jan 19, 2017
acce004
Added libffi-devel installation
Jan 19, 2017
0ad201d
Merge branch 'master' of github.com:StackStorm/ansible-st2 into cento…
Jan 19, 2017
da000f6
Reordered conditional order to avoid issue with ubuntu
Jan 19, 2017
abb465e
Removed reference to closed issue
Jan 19, 2017
6abec30
Merge branch 'master' into centos-st2-role
Mierdin Jan 19, 2017
07b08bc
Force lowercase for ansible_os_family
Jan 19, 2017
709bc97
Merge branch 'centos-st2-role' of github.com:Mierdin/ansible-st2 into…
Jan 19, 2017
92f82ee
Fixed bug in epel check
Jan 20, 2017
f5bb4b5
Removed ansible_ssh_pipelining
Jan 20, 2017
edcb8a2
Merge branch 'master' of github.com:StackStorm/ansible-st2 into cento…
Jan 20, 2017
7fa3a70
Change 'latest' to 'present' to address ansible-lint error
Jan 20, 2017
9c52c7b
Implemented better fix for repo error
Jan 20, 2017
100ca71
Removed unnecessary error ignore statement
Jan 20, 2017
9988842
Added conditional check for including epel role; moved sudo inside
Jan 20, 2017
56ed120
Restored original role order
Jan 20, 2017
3c3f272
Clarified task names to more accurately reflect what they do
Jan 20, 2017
d8714bb
Reordered tasks in st2repos; now using yum module to install latest c…
Jan 20, 2017
e9ed597
Removed redundant epel installation, used epel role as dependency ins…
Jan 20, 2017
13d1d4c
Converted to http url for libffi-devel
Jan 21, 2017
968b9e5
Merge branch 'master' of github.com:StackStorm/ansible-st2 into cento…
Jan 21, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,20 @@ You might be interested in other methods to deploy StackStorm engine:
* [RHEL7/CentOS7](https://docs.stackstorm.com/install/rhel7.html)
* [RHEL6/CentOS6](https://docs.stackstorm.com/install/rhel6.html)

## Developing
Copy link
Member

Choose a reason for hiding this comment

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

👍


There are a few requirements when developing on `ansible-st2`:

These are the platforms we must support (must pass end-to-end testing):
- Xenial
- Trusty
- CentOS6
- CentOS7
- RHEL6 (via AWS)
- RHEL7 (via AWS)

Must also support Ansible Idempotence (Eg. Ansible-playbook re-run should end with the following results: changed=0.*failed=0)

## Help
If you're in stuck, our community always ready to help, feel free to:
* Ask questions in our [public Slack channel](https://stackstorm.com/community-signup)
Expand Down
18 changes: 18 additions & 0 deletions roles/epel/meta/main.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
galaxy_info:
description: Install EPEL repository
author: mierdin
company: StackStorm
license: Apache
min_ansible_version: 1.9
platforms:
- name: Ubuntu
versions:
- trusty
- precise
- name: EL
versions:
- 6
- 7
categories:
- system
12 changes: 12 additions & 0 deletions roles/epel/tasks/main.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
- name: Check if EPEL is installed
stat:
path: /etc/yum.repos.d/epel.repo
register: epel_installed
when: ansible_os_family == "RedHat"

- name: Install EPEL repo
yum:
name: "https://dl.fedoraproject.org/pub/epel/epel-release-latest-{{ ansible_distribution_major_version }}.noarch.rpm"
state: present
when: ansible_os_family == "RedHat" and not epel_installed.stat.exists
3 changes: 3 additions & 0 deletions roles/mongodb/meta/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,6 @@ galaxy_info:
- 7
categories:
- system
dependencies:
- role: epel
Copy link
Member

@arm4b arm4b Jan 20, 2017

Choose a reason for hiding this comment

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

Should have conditional check:
Include epel only when ansible_os_family == "RedHat".

Even despite there is a OS check in epel role, having condition here as well will look a bit better by logic.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in 9988842

become: yes
Copy link
Member

@arm4b arm4b Jan 20, 2017

Choose a reason for hiding this comment

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

Using become: yes when including anything external is usually a bad sign.

The good practice is to become: yes for every task in a role where it's 100% needed.
Eg. we do privilege escalation selectively per task, not per role.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ack, fixed in 9988842

22 changes: 20 additions & 2 deletions roles/st2/tasks/config_auth.yml
Original file line number Diff line number Diff line change
@@ -1,11 +1,29 @@
- name: Install auth pre-reqs
- name: Install auth pre-reqs (Debian)
become: yes
apt:
name: "{{ item }}"
state: present
with_items:
- python-passlib
- apache2-utils
when: ansible_os_family == 'Debian'

- name: Install epel-release repo (RedHat)
become: yes
yum:
name: epel-release
Copy link
Member

Choose a reason for hiding this comment

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

Will this work on real RHEL6?
You can test in AWS https://aws.amazon.com/marketplace/pp/B00CFQWLS6

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I will include both EL6 and EL7 in my testing.

Copy link
Member

@arm4b arm4b Jan 20, 2017

Choose a reason for hiding this comment

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

So here we install epel-release and still including epel role?
Needs improvement to avoid repeating similar tasks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in e9ed597

state: present
when: ansible_os_family == 'RedHat'

- name: Install auth pre-reqs (RedHat)
become: yes
yum:
name: "{{ item }}"
state: present
with_items:
- python-passlib
- httpd-tools
when: ansible_os_family == 'RedHat'

- name: Create htpasswd file
become: true
Expand All @@ -16,7 +34,7 @@
notify:
- restart st2api/st2stream

- name: Enable autentication
- name: Enable authentication
become: yes
ini_file:
dest: /etc/st2/st2.conf
Expand Down
16 changes: 12 additions & 4 deletions roles/st2/tasks/main.yml
Original file line number Diff line number Diff line change
@@ -1,17 +1,25 @@
---
- name: Install latest st2 package
- name: Install libffi-devel on EL6
become: yes
apt:
package:
name: ftp://fr2.rpmfind.net/linux/centos/6/os/x86_64/Packages/libffi-devel-3.0.5-3.2.el6.x86_64.rpm
state: present
when: ansible_os_family == "RedHat" and ansible_distribution_major_version == "6"
ignore_errors: yes # Ignore error when already installed
Copy link
Member

@arm4b arm4b Jan 20, 2017

Choose a reason for hiding this comment

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

That's a hack, - generally a sign of improvement. There should be a better way.

Additionally, I'm not sure if this block is idempotent.
Please double check that we don't download ftp://fr2.rpmfind.net/linux/centos/6/os/x86_64/Packages/libffi-devel-3.0.5-3.2.el6.x86_64.rpm on every playbook re-run.

Copy link
Member

@arm4b arm4b Jan 20, 2017

Choose a reason for hiding this comment

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

Per https://docs.stackstorm.com/install/rhel6.html#system-requirements did you check if it's possible to enable server-optional repository for RHEL6? That would be more elegant and preferable.
So this package dependency will be installed automatically by st2.

See libffi-devel is in st2 package requirements: https://packagecloud.io/StackStorm/stable/packages/el/6/st2-2.1.1-1.x86_64.rpm

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried that before, got this:

[root@ip-10-0-4-91 ~]# subscription-manager repos --enable rhel-6-server-optional-rpms
Error: rhel-6-server-optional-rpms is not a valid repository ID. Use --list option to see valid repositories.
[root@ip-10-0-4-91 ~]# subscription-manager repos --list
This system has no repositories available through subscriptions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Additionally, this task does run idempotently, now that I've changed to state: latest:

TASK [st2 : Install libffi-devel on EL6] ***************************************
ok: [10.0.4.91]

This should also make the ignore_errors statement pointless, so I have removed it in 100ca71. So as long as you don't prefer the server-optional approach (and know what I'm doing wrong that is preventing me from installing libffi-devel that way) then this should work better.

Copy link
Member

Choose a reason for hiding this comment

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

+1
Thanks for explanation!


- name: Install latest st2 package (stable)
Copy link
Member

@arm4b arm4b Jan 20, 2017

Choose a reason for hiding this comment

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

(stable) is not exactly what we install here.

Package repository (if you meant that) is controlled via st2_pkg_repo, see: https://github.com/StackStorm/ansible-st2#variables

So (stable) note could be removed since it's irrelevant for this particular block.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, my intention with changing this was for easier debugging. Since both tasks had the same name, it was hard to tell from the output which one actually ran.

This should work better: 3c3f272

become: yes
package:
name: st2
state: latest
when: st2_version == "latest"
notify:
- restart st2
tags: skip_ansible_lint

- name: Install latest st2 package
- name: Install latest st2 package (not stable)
Copy link
Member

@arm4b arm4b Jan 20, 2017

Choose a reason for hiding this comment

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

Same as https://github.com/StackStorm/ansible-st2/pull/66/files#r97132662

(not stable) note could be removed.

We can write name: Install pinned st2 package version or similar explanation instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in 3c3f272

become: yes
apt:
package:
name: st2={{ st2_version }}-{{ st2_revision }}
state: present
when: st2_version != "latest"
Expand Down
2 changes: 0 additions & 2 deletions roles/st2/tasks/user.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
tags: [st2, user]

- name: user | Authorize key-based access for system user
vars:
ansible_ssh_pipelining: true
become: yes
become_user: "{{ st2_system_user }}"
authorized_key:
Expand Down
3 changes: 1 addition & 2 deletions roles/st2mistral/tasks/main.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
---

- name: Install st2mistral dependency
become: yes
package:
Expand Down Expand Up @@ -44,8 +45,6 @@
tags: st2mistral

- name: Initiate database
vars:
ansible_ssh_pipelining: true
become: yes
become_user: postgres
shell: psql < /etc/mistral/init_mistral_db.SQL
Expand Down
23 changes: 23 additions & 0 deletions roles/st2repos/tasks/debian.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
- name: Install prereqs (Debian)
become: yes
apt:
name: "{{ item }}"
state: present
with_items:
- debian-archive-keyring
- apt-transport-https

- name: Add keys to keyring
become: yes
apt_key:
Copy link
Member

@arm4b arm4b Jan 5, 2017

Choose a reason for hiding this comment

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

I think it would be more solid if we check id (identifier of key) here as well.
It wasn't there before, but it's good time to add it.

See example with Mongo:

id: 42F3E95A2C4F08279C4960ADD68FA50FEA312927

Copy link
Member Author

Choose a reason for hiding this comment

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

Where would I go about finding that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nevermind, added in f6ac495

id: 418A7F2FB0E1E6E7EABF6FE8C2E73424D59097AB
url: https://packagecloud.io/StackStorm/{{ st2_pkg_repo }}/gpgkey
state: present

- name: Add StackStorm repos
become: yes
apt_repository:
repo: 'deb https://packagecloud.io/StackStorm/{{ st2_pkg_repo }}/{{ ansible_distribution|lower }}/ {{ ansible_distribution_release|lower }} main'
state: present
update_cache: yes
22 changes: 2 additions & 20 deletions roles/st2repos/tasks/main.yml
Original file line number Diff line number Diff line change
@@ -1,22 +1,4 @@
---
- name: Install prereqs
become: yes
apt:
name: "{{ item }}"
state: present
with_items:
- debian-archive-keyring
- apt-transport-https
# tasks file for st2repos

- name: Add keys to keyring
become: yes
apt_key:
url: https://packagecloud.io/StackStorm/{{ st2_pkg_repo }}/gpgkey
state: present

- name: Add StackStorm repos
become: yes
apt_repository:
repo: 'deb https://packagecloud.io/StackStorm/{{ st2_pkg_repo }}/{{ ansible_distribution|lower }}/ {{ ansible_distribution_release|lower }} main'
state: present
update_cache: yes
- include: "{{ ansible_os_family|lower }}.yml"
22 changes: 22 additions & 0 deletions roles/st2repos/tasks/redhat.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
- name: Add ST2 Repo
become: yes
yum_repository:
name: "StackStorm_{{ st2_pkg_repo }}"
description: "StackStorm_{{ st2_pkg_repo }}"
file: "StackStorm_{{ st2_pkg_repo }}"
baseurl: https://packagecloud.io/StackStorm/{{ st2_pkg_repo }}/el/{{ ansible_distribution_major_version }}/$basearch
Copy link
Member

Choose a reason for hiding this comment

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

So seems this is good enough and doesn't look as hack anymore 👍

I think we can close the #89 now, just add a note that we should always use {{ ansible_distribution_major_version }} instead of $releasever when adding any yum repo.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, cool. Closed #89

repo_gpgcheck: yes
gpgkey: "https://packagecloud.io/StackStorm/{{ st2_pkg_repo }}/gpgkey"
sslcacert: /etc/pki/tls/certs/ca-bundle.crt
metadata_expire: 300
gpgcheck: no
enabled: yes
sslverify: yes

# Fixes "Failure talking to yum: Cannot retrieve repository metadata (repomd.xml) for repository: StackStorm_stable. Please verify its path and try again" when installing st2
- name: Update ca-certificates package
Copy link
Member

@arm4b arm4b Jan 20, 2017

Choose a reason for hiding this comment

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

Since this fixes specific error for PackageCloud repository, maybe it's worth to move it before the
name: Add ST2 Repo block?

Copy link
Member Author

Choose a reason for hiding this comment

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

Reordered in d8714bb

become: yes
package:
Copy link
Member

Choose a reason for hiding this comment

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

Also, since this block is part of redhat.yml, I think it's more expected to use native yum module for installation, instead of cross-platform package module.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in d8714bb

name: ca-certificates
state: latest
Copy link
Member

Choose a reason for hiding this comment

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

[ANSIBLE0010] Package installs should not use latest

The ansible-lint warning is valid. Use state: installed where possible.
Installing latest package version on every playbook re-run is generally a bad practice (especially when ansible works in ansible-pull mode).

For st2 we use latest intentionally as option so user can specify st2_version=latest to have latest StackStorm.
Probably we should change default behavior there someday later since it could be dangerous for default installs.

For everything else the safe way is to upgrade software on the server by running specific playbook which focuses on OS package upgrades (or patches) to have full control and react accordingly if something went wrong.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just to be clear, for the st2 role, you're saying I should provide a var that controls the parameter passed there, defaulting to latest?

Copy link
Member

@arm4b arm4b Jan 20, 2017

Choose a reason for hiding this comment

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

Just change to state: installed here, as ansible-lint suggests.

[ANSIBLE0010] Package installs should not use latest

Sorry for my previous TL;DR comment ^^ 😄 Just wanted to give an explanation.

Copy link
Member Author

Choose a reason for hiding this comment

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

So the problem I had with this specific line (not other places where I made this linting error) was that unless I updated ca-certificates, I'd get this error when st2mistral runs:

Failure talking to yum: Cannot retrieve repository metadata (repomd.xml) for repository: StackStorm_stable. Please verify its path and try again

I just checked, and setting this to present instead of latest causes the error to come back up, as the package is already installed, so it doesn't do anything.

Copy link
Member

@arm4b arm4b Jan 20, 2017

Choose a reason for hiding this comment

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

Makes sense for an exception 👍

This looks like old EL6 version according to http://unix.stackexchange.com/questions/21310/yum-cannot-retrieve-repository-centos-6

If that happens on EL6 only, can you add conditional check to run this task only on EL6?
And we can add skip_ansible_lint tag for this block.

Copy link
Member Author

Choose a reason for hiding this comment

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

I may have a better fix for this, give me a few minutes.

Copy link
Member Author

Choose a reason for hiding this comment

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

@armab Okay, this seems to work: 9c52c7b Thoughts?

Copy link
Member

@arm4b arm4b Jan 20, 2017

Choose a reason for hiding this comment

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

@Mierdin It's not clear how 9c52c7b fixes the issue with outdated ca-certificates.

According to http://stackoverflow.com/questions/26734777/yum-error-cannot-retrieve-metalink-for-repository-epel-please-verify-its-path and http://unix.stackexchange.com/questions/21310/yum-cannot-retrieve-repository-centos-6 2 possible fixes for outdated EL6 are:

  • use http instead of https for package repo
  • update ca-certificates package

How yum -y repolist fixes that? I'm probably don't have enough context here.
Additionally, can you reproduce this issue? As I understood it happens in some outdated EL 6.5 distro.

I'd say, - we don't support some outdated RHEL6.5 which was released in 2013.

So after all, - I'm +1 to remove this block completely :) WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, per our slack conversation, I reverted back to the old approach and am now updating ca-certificates using the yum module, in d8714bb

2 changes: 1 addition & 1 deletion stackstorm.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
- postgresql
- nginx
- st2repos
- st2
- st2mistral
- st2
Copy link
Member

@arm4b arm4b Jan 20, 2017

Choose a reason for hiding this comment

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

Let's keep the original order.

We install st2 first (more significant) and then st2mistral.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in 56ed120

- st2web
- st2smoketests