Add role for installing ceph-ansible and its confiuration #23

mkudlej · 2017-01-11T12:38:17Z

No description provided.

mkudlej · 2017-01-11T12:39:51Z

@Tendrl/qe please review

dahorak

I have just few small notes/questions.

dahorak · 2017-01-11T13:10:33Z

roles/ceph-ansible/defaults/main.yml

+collocation: true
+
+# if collocation is false and journal devices are set,
+# create journals on journal devices.


What if collocation is false but journal_devices are not set? Does it use some autodetection, same as for empty devices, or it is not supported configuration?

If devices variable is empty devices are autodetected by ceph-ansible. If collocation is false, journal_devices should contain list of devices for journals.

dahorak · 2017-01-11T13:17:48Z

roles/ceph-ansible/tasks/main.yml

+    dest: /usr/share/ceph-ansible/group_vars/osds.yml
+    regexp: '^#devices:'
+    line: 'devices:'
+  when: "{{ devices }}"


The double curly brackets {{ and }} shouldn't be required here (see the Ansible doc).

It works with double curly brackets.

dahorak · 2017-01-11T13:22:25Z

roles/ceph-ansible/tasks/main.yml

+    dest: /usr/share/ceph-ansible/group_vars/osds.yml
+    regexp: "^#osd_auto_discovery: false"
+    line: "osd_auto_discovery: true"
+  when: "{{ not devices }}"


Does this construction works correctly? (maybe yes, I'm just asking, because the double quotes and double curly brackets are not usually used in such when construction.

Yes, it works.

dahorak · 2017-01-11T13:24:59Z

roles/ceph-ansible/tasks/main.yml

+    line: 'raw_journal_devices:'
+  when: "{{ (not collocation) and journal_devices }}"
+
+- name: Add devices for if they are set


I think, this should be:
Add devices for journal if they are set

You're right, I'll fix it.

dahorak

It is also required to prepare specific hosts file for ceph-ansible.

mbukatov · 2017-01-12T12:29:48Z

roles/ceph-ansible/tasks/main.yml

+# TODO do apt installation
+- name: Install ceph-ansible package
+  yum:
+    name=ceph-ansible


@mkudlej I see that you just install ceph-ansible package without enabling particular repository for it. That is good.

That said, I would like to know which ceph-ansible package you tested this role with?

mbukatov · 2017-01-12T13:25:14Z

roles/ceph-ansible/tasks/main.yml

+- name: Copy sample files
+  copy:
+    src: "/usr/share/ceph-ansible/{{ item.src }}"
+    dest: "/usr/share/ceph-ansible/{{ item.dst }}"


I would rather not install config files created from packaged samples into system directory.

mbukatov · 2017-01-12T13:50:41Z

install_ceph.yml

@@ -0,0 +1,6 @@
+---
+- name: Install ceph-ansible and install Ceph
+  hosts: usm_server


ceph-ansible should be installed and configured on qe_server instead

maybe we could just move installation of ceph-ansible package into qe_server role

mbukatov · 2017-01-12T14:41:29Z

I would suggest to move installation of ceph-ansible package into qe-server role, and then enable repo with this package in qe_server playbook.

mbukatov · 2017-01-12T14:43:32Z

Moreover it would be nice if it would be possible to run ceph-ansible playbooks via pytest ansible playbook plugin.

mbukatov · 2017-01-12T14:47:17Z

I'm dumping my notes here:

we are not testing ceph-ansible, we are just using it
we use it for setup of import cluster test case(s)
ceph-ansible will be installed, configured and used on qe_server
we install it from rpm package (which has a minor issue right now, because the package is not available in Storage SIG)
we have ansible role for configuration of ceph ansible (see tasks in this pull request)

- playbook is run on qe server node - ansible.cfg is used beause of ceph-ansible - files from ceph-ansible packages are copied and they are no more modified

mkudlej · 2017-01-18T13:49:01Z

@dahorak @mbukatov please review my changes

dahorak

I have few questions and notes.
And also it seems like there are by mistake some commits unrelated to the ceph-ansible.

dahorak · 2017-01-19T09:01:04Z

roles/ceph-ansible/README.rst

+As it is written in ``tasks/main.yml`` ceph-ansible package should be 
+preinstalled or available in enabled yum repository.
+
+Example usage of configured ceph-ansible:


Following lines sounds slightly strangely (or maybe I just misunderstand them). Could you please check it and at least fix the format, so it will be properly generated? (check the View button)

dahorak · 2017-01-19T09:08:30Z

roles/ceph-ansible/defaults/main.yml

@@ -0,0 +1,19 @@
+---
+# home directory which contains copy of files in ceph-ansible package
+home_dir: "{{ ansible_env['WORKSPACE'] | default(ansible_env['HOME']) }}"


The name home_dir sounds to me, like it should be the HOME dir of the user, I would name it maybe something like base_dir (if I understand the meaning correctly).
Also where should point the ansible_env['WORKSPACE']? (I wasn't able to find, what it means.)

WORKSPACE is variable in Jenkins environment for build workspace, right?

@mkudlej ah, yes you are right. Please, write there short description about that and consider the renaming of the home_dir to something like base_dir or work_dir.

dahorak · 2017-01-19T09:16:41Z

roles/ceph-ansible/defaults/main.yml

+journal_devices: []
+
+# path to ansible configuration which overrides default ansible configuration
+ansible_conf: /tmp/ansible.cfg


I'm not sure, if /tmp/ is the best place for the configuration file (it might not persist reboot of the machine, but this didn't apply to RHEL 7/CentOS 7 though).

I think that is not problem, because it is written every time you run this playbook. This is not configuration file which should persist after reboot.

Ok, I agree (I somehow thing it is launched only during the qe server configuration).

dahorak · 2017-01-19T09:19:13Z

roles/ceph-ansible/tasks/main.yml

+    regexp: '^#generate_fsid: true.*'
+    line: 'generate_fsid: true'
+
+# Disabled because this list in directly in ceph-ansible


Probably just typo? Disabled because this list is directly in ceph-ansible.

dahorak · 2017-01-19T09:20:58Z

roles/ceph-ansible/tasks/main.yml

+# edit mons.yml file
+
+# Disabled on CentOS because calamari is not available in SIG
+- name: Enable calamari on mons


Just question, it will be installed, configured and running on all mons, or only on one of them?

Ceph-ansible install and run calamari on all mons. Console 2 has no problem with this. It is OK to create user in Calamari only on one instance after installation for import purpose.

dahorak · 2017-01-19T09:26:54Z

roles/qe-server/README.rst

@@ -3,3 +3,78 @@
 ================

 This role configures the machine for execution of usmqe integration tests.
+


It seems like this and the following files are part of this pull request by mistake as it is about Xvfb server and other stuff and not about ceph-ansible.

mkudlej · 2017-01-19T11:02:01Z

Tested with ceph-ansible https://koji.fedoraproject.org/koji/buildinfo?buildID=831491

dahorak · 2017-01-19T11:25:58Z

roles/ceph-ansible/README.rst

+
+  # cd to ceph-ansible dir so relative paths can be used
+  # default value of variable $base_dir is in defaults/main.yml
+  $ cd $base_dir/ceph-ansible from defaults/main.yml


Thanks, now it make sense. Maybe just remove the from defaults/main.yml from the command.

mbukatov

I don't want to block your work, so I'm ok with merging it now.

That said, I would like to test this myself to check what needs to be done to run ceph-ansible playbooks via pytest-ansible-playbook fixture. Moreover, configuring ansible via ansible feels weird.

mbukatov · 2017-01-19T17:26:04Z

install_ceph.yml

@@ -0,0 +1,6 @@
+---


Could we rename this file to just ceph_ansible.yml or configura_ceph_ansible.yml?

mbukatov requested review from mbukatov and dahorak January 11, 2017 12:50

mbukatov assigned mkudlej Jan 11, 2017

dahorak reviewed Jan 11, 2017

View reviewed changes

dahorak requested changes Jan 11, 2017

View reviewed changes

mbukatov reviewed Jan 12, 2017

View reviewed changes

mbukatov requested changes Jan 12, 2017

View reviewed changes

mkudlej added 3 commits January 18, 2017 14:36

Add role for installing ceph-ansible and its confiuration

05c4ae0

fix requested issues

2fb0c07

Change required modifications

67e226f

- playbook is run on qe server node - ansible.cfg is used beause of ceph-ansible - files from ceph-ansible packages are copied and they are no more modified

dahorak requested changes Jan 19, 2017

View reviewed changes

mkudlej force-pushed the ceph-ansible branch from 818ebd7 to 67e226f Compare January 19, 2017 10:39

fix issues based on dahorak's comments

b7b11e1

dahorak reviewed Jan 19, 2017

View reviewed changes

dahorak approved these changes Jan 19, 2017

View reviewed changes

mbukatov approved these changes Jan 19, 2017

View reviewed changes

install_ceph.yml

@@ -0,0 +1,6 @@

---

Copy link

Contributor

mbukatov Jan 19, 2017

Choose a reason for hiding this comment

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

Could we rename this file to just ceph_ansible.yml or configura_ceph_ansible.yml?

reanme ceph-installer playbook, rename yml files according documentation

5bdcbbe

mbukatov merged commit 1f74c19 into usmqe:master Jan 20, 2017

		@@ -3,3 +3,78 @@
		================

		This role configures the machine for execution of usmqe integration tests.

Add role for installing ceph-ansible and its confiuration #23

Add role for installing ceph-ansible and its confiuration #23

Conversation

mkudlej commented Jan 11, 2017

mkudlej commented Jan 11, 2017

dahorak 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

dahorak left a comment

Choose a reason for hiding this comment

mbukatov Jan 12, 2017 • edited

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

mbukatov commented Jan 12, 2017

mbukatov commented Jan 12, 2017

mbukatov commented Jan 12, 2017

mkudlej commented Jan 18, 2017

dahorak 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

dahorak Jan 19, 2017 • edited

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

mkudlej commented Jan 19, 2017

Choose a reason for hiding this comment

mbukatov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbukatov Jan 12, 2017 •

edited

dahorak Jan 19, 2017 •

edited