Refact playbook #1727

guits · 2017-07-28T21:07:57Z

Refact all playbook

andrewschoen · 2017-07-31T17:20:01Z

roles/ceph-defaults/tasks/check_socket.yml

+# These checks are used to avoid running handlers at initial deployment.
+- name: check for a ceph socket
+  shell: |
+    {{ docker_exec_cmd }} bash -c 'stat {{ rbd_client_admin_socket_path }}/*.asok > /dev/null 2>&1'


Did you mean to have {{ docker_exec_cmd }} here? It's undefined unless a containerized_deployment is True.

@andrewschoen yes, docker_exec_cmd has a default value set to '' then it's set here so we can use this check for both ceph-common and ceph-docker-common, at least this was already used that way before that PR, I just moved all this from ceph-mon to ceph-defaults

leseb · 2017-08-01T09:04:05Z

roles/ceph-defaults/templates/restart_mon_daemon.sh.j2

@@ -9,15 +9,15 @@ SOCKET=/var/run/ceph/${CLUSTER}-mon.${MONITOR_NAME}.asok

 check_quorum() {
 while [ $RETRIES -ne 0 ]; do
-  MEMBERS=$(ceph --cluster ${CLUSTER} -s --format json | sed -r 's/.*"quorum_names":(\[[^]]+\]).*/\1/')
+  MEMBERS=$({{ docker_exec_cmd }} ceph --cluster ${CLUSTER} -s --format json | sed -r 's/.*"quorum_names":(\[[^]]+\]).*/\1/')


unrelated to the PR but I wonder if CLUSTER="{{ cluster }}" is really necessary, we can just have --cluster {{ cluster }}.

leseb · 2017-08-01T09:08:14Z

roles/ceph-defaults/templates/restart_osd_daemon.sh.j2

+
+wait_for_socket_in_docker() {
+  docker exec $1 timeout 10 bash -c "while [ ! -e /var/run/ceph/*.asok ]; do sleep 1 ; done"
+#TODO(guits): What if socket never shows up?


timeout will return 1 but we better have a message for this, this code should be better:

if ! docker exec $1 timeout 10 bash -c "while [ ! -e /var/run/ceph/*.asok ]; do sleep 1 ; done"; then log "Timed out while trying to look for a Ceph OSD socket." log "Abort mission!" exit 1 fi

will definitely try this

leseb · 2017-08-01T09:08:41Z

roles/ceph-defaults/templates/restart_osd_daemon.sh.j2

+  local count
+  count=10
+  while [ $count -ne 0 ]; do
+  id=$(docker ps -q -f "name=$1")


leseb · 2017-08-01T09:10:38Z

roles/ceph-defaults/templates/restart_osd_daemon.sh.j2

+  # We need to wait because it may take some time for the socket to actually exists
+  COUNT=10
+  # Wait and ensure the socket exists after restarting the daemon
+  {% if not containerized_deployment -%}


I'm not a big fan of reverse conditions so perhaps we should replace this with:

{% if containerized_deployment -%} ... {% else %} ... {% endif %}

ok i switched

leseb · 2017-08-01T09:11:18Z

roles/ceph-defaults/templates/restart_osd_daemon.sh.j2

+  {% endif %}
+  SOCKET=/var/run/ceph/test-osd.${osd_id}.asok
+  while [ $COUNT -ne 0 ]; do
+#    $docker_exec bash -c "test -S $SOCKET" && check_pgs && continue 2


Merge `ceph-docker-common` and `ceph-common` defaults vars in `ceph-defaults` role. Remove redundant variables declaration in `ceph-mon` and `ceph-osd` roles. Signed-off-by: Guillaume Abrioux <gabrioux@redhat.com>

Add a new role `ceph-defaults`. This role aims to handle all defaults vars for `ceph-docker-common` and `ceph-common` and set basic facts (eg. `fsid`) Signed-off-by: Guillaume Abrioux <gabrioux@redhat.com>

andrewschoen

I think this is looking good now. I do agree that this needs some documentation though. Before we merge should we create an issue or something so people are aware of the changes being made here? There will be implications for ansible-galaxy users at the least.

andrewschoen · 2017-08-02T14:47:11Z

infrastructure-playbooks/rolling_update.yml

+
+  roles:
+    - ceph-defaults
+    - ceph.ceph-common


Should this be just ceph-common?

I modified it

This will give us more flexibility and avoid a lot of useless when skipping all tasks from a non-desired role. Signed-off-by: Guillaume Abrioux <gabrioux@redhat.com>

Move `fsid`,`monitor_name`,`docker_exec_cmd` and `ceph_release` set_fact to `ceph-defaults` role. It will allow to reuse these facts without having to play `ceph-common` or `ceph-docker-common`. Signed-off-by: Guillaume Abrioux <gabrioux@redhat.com>

Until now, there is no handlers for containerized deployments. Signed-off-by: Guillaume Abrioux <gabrioux@redhat.com>

in containerized deployment, if you try to update your `ceph.conf` file it won't be actually updated on your nodes because it is overwritten by the copy of the file which is present in your fetch directory. Signed-off-by: Guillaume Abrioux <gabrioux@redhat.com>

Signed-off-by: Sébastien Han <seb@redhat.com>

A massive refactor of ceph-ansible merged about 18 hours ago in ceph/ceph-ansible#1727, which breaks our ceph gate. A patch already exists[1] which will adjust our ceph deployment according to the upstream changes, however there is some work to be done upstream[2] before our fix can merge. Until then, to keep the OSA master gate unblocked, we should pin the ceph-ansible roles immediately prior to the refactor commits. [1] https://review.openstack.org/#/c/490476/ [2] ceph/ceph-ansible#1740 Change-Id: Ie96984d207163404bb7f98cc10aa02a3035a362c

dillaman · 2017-08-11T14:47:48Z

roles/ceph-common/tasks/main.yml

@@ -93,10 +88,24 @@
  # Hard code this so we will skip the entire file instead of individual tasks (Default isn't Consistent)
  static: False

- include: facts.yml
- include: ./checks/check_socket.yml
+- name: get ceph version


This fails on nodes that have roles like "rbd_mirror" since the "ceph" command hasn't been installed yet

@dillaman this task was already executed in ceph-common before this refactor. Do you mean before this commit you didn't hit this issue?

Hmm -- looks like it never would have passed since PR #737 (which was merged long before support for rbd-mirror). Apparently the installation of rbd-mirror never worked.

I might have missed something but at the task you highlighted here, you should have ceph command available, the task which installs ceph packages are executed earlier in that same role but I'll investigate this.

If you have a VM/docker container that is just running the rbd-mirror role, nothing is installed at this point (at least on CentOS). There are some hooks to install ceph-base on CentOS if the client role is assigned to the host so I'd imagine that should be expanded to cover any host where Ceph ansible is running.

@dillaman see #1762

ceph-ansible has undergone significant refactoring in their v3 development, adding new required roles such as ceph-defaults and ceph-config, which are used to provide vars and configuration to the ceph service roles. These roles must be executed before the service roles to avoid missing vars[1][2]. During the v3 refactoring, ceph-common was removed as a galaxy-style role dependency in the service roles meta files[3]. This means we will need to explicitly execute ceph-common from now on also. This change adds the defaults and config roles and executes them. Also some minor cleanup such as alphabetizing the OpenStack roles list is done. Also added is an upgrade playbook, reno, and docs to assist in cleaning up the older galaxy-named ceph common roles which are no longer galaxy namespaced in our cloning configuration. [1] http://jenkins-logs.objects-us-dfw-1.cloud.lstn.net/osa-ci/490192/1/21/logs/console.log [2] ceph/ceph-ansible#1737 [3] ceph/ceph-ansible#1727 Change-Id: Ia8c0cb0a23f331fce7914afbfc05ef54ee3ffb0e

ceph-ansible has undergone significant refactoring in their v3 development, adding new required roles such as ceph-defaults and ceph-config, which are used to provide vars and configuration to the ceph service roles. These roles must be executed before the service roles to avoid missing vars[1][2]. During the v3 refactoring, ceph-common was removed as a galaxy-style role dependency in the service roles meta files[3]. This means we will need to explicitly execute ceph-common from now on also. This change adds the defaults and config roles and executes them. Also some minor cleanup such as alphabetizing the OpenStack roles list is done. Also added is an upgrade playbook, reno, and docs to assist in cleaning up the older galaxy-named ceph common roles which are no longer galaxy namespaced in our cloning configuration. [1] http://jenkins-logs.objects-us-dfw-1.cloud.lstn.net/osa-ci/490192/1/21/logs/console.log [2] ceph/ceph-ansible#1737 [3] ceph/ceph-ansible#1727 Conflicts: ansible-role-requirements.yml Change-Id: Ia8c0cb0a23f331fce7914afbfc05ef54ee3ffb0e (cherry picked from commit a53f1ae)

guits force-pushed the refact branch 17 times, most recently from 789b338 to 2167272 Compare July 31, 2017 13:59

leseb mentioned this pull request Jul 31, 2017

ceph-mds: Enable multimds under docker #1695

Merged

guits force-pushed the refact branch 4 times, most recently from 5657d68 to 7b5c0db Compare July 31, 2017 14:15

andrewschoen reviewed Jul 31, 2017

View reviewed changes

guits force-pushed the refact branch 2 times, most recently from a66af6f to 656c427 Compare August 1, 2017 04:20

leseb requested changes Aug 1, 2017

View reviewed changes

guits force-pushed the refact branch from 656c427 to cfc9f2c Compare August 1, 2017 12:43

leseb added the needs-docs label Aug 1, 2017

guits force-pushed the refact branch 2 times, most recently from 5d3e099 to 1f3d713 Compare August 1, 2017 13:40

guits force-pushed the refact branch 7 times, most recently from f1228a2 to 0b28ca1 Compare August 2, 2017 11:51

guits added 2 commits August 2, 2017 14:46

merge docker-common and common defaults vars

1d003aa

Merge `ceph-docker-common` and `ceph-common` defaults vars in `ceph-defaults` role. Remove redundant variables declaration in `ceph-mon` and `ceph-osd` roles. Signed-off-by: Guillaume Abrioux <gabrioux@redhat.com>

Add new role ceph-defaults

7322526

Add a new role `ceph-defaults`. This role aims to handle all defaults vars for `ceph-docker-common` and `ceph-common` and set basic facts (eg. `fsid`) Signed-off-by: Guillaume Abrioux <gabrioux@redhat.com>

guits force-pushed the refact branch from 0b28ca1 to d2d7095 Compare August 2, 2017 12:47

leseb approved these changes Aug 2, 2017

View reviewed changes

leseb force-pushed the refact branch from 120df95 to cb01b3f Compare August 2, 2017 14:07

leseb mentioned this pull request Aug 2, 2017

Support clients configuration with external Ceph clusters #1670

Closed

andrewschoen approved these changes Aug 2, 2017

View reviewed changes

guits and others added 6 commits August 2, 2017 17:12

Move role dependencies in site.yml/site-docker.yml

5adbf0f

This will give us more flexibility and avoid a lot of useless when skipping all tasks from a non-desired role. Signed-off-by: Guillaume Abrioux <gabrioux@redhat.com>

Move basics facts to ceph-defaults

fc6b6e9

Move `fsid`,`monitor_name`,`docker_exec_cmd` and `ceph_release` set_fact to `ceph-defaults` role. It will allow to reuse these facts without having to play `ceph-common` or `ceph-docker-common`. Signed-off-by: Guillaume Abrioux <gabrioux@redhat.com>

Add handlers for containerized deployment

7a333d0

Until now, there is no handlers for containerized deployments. Signed-off-by: Guillaume Abrioux <gabrioux@redhat.com>

infrastructure-playbooks: update with ceph-defaults roles

fdc6aeb

Signed-off-by: Sébastien Han <seb@redhat.com>

generate_group_vars_sample: update for ceph-defaults

3a8bf18

Signed-off-by: Sébastien Han <seb@redhat.com>

guits force-pushed the refact branch from cb01b3f to 3a8bf18 Compare August 2, 2017 15:13

guits changed the title ~~DNM: Refact~~ Refact playbook Aug 2, 2017

leseb merged commit 6e37915 into master Aug 2, 2017

leseb deleted the refact branch August 2, 2017 20:01

This was referenced Aug 4, 2017

Move get-fsid code to its own role, so it can be depended upon #1708

Closed

DNM: Refactor takeover #1639

Closed

dillaman reviewed Aug 11, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refact playbook #1727

Refact playbook #1727

guits commented Jul 28, 2017 •

edited by leseb

Loading

andrewschoen Jul 31, 2017

guits Aug 1, 2017

leseb Aug 1, 2017

guits Aug 1, 2017

leseb Aug 1, 2017

guits Aug 1, 2017

leseb Aug 1, 2017

guits Aug 1, 2017

leseb Aug 1, 2017

guits Aug 1, 2017

leseb Aug 1, 2017

guits Aug 1, 2017

andrewschoen left a comment

andrewschoen Aug 2, 2017

guits Aug 2, 2017

dillaman Aug 11, 2017

guits Aug 11, 2017

dillaman Aug 11, 2017

guits Aug 11, 2017

dillaman Aug 11, 2017

guits Aug 12, 2017

Refact playbook #1727

Refact playbook #1727

Conversation

guits commented Jul 28, 2017 • edited by leseb 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

Choose a reason for hiding this comment

andrewschoen 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

guits commented Jul 28, 2017 •

edited by leseb

Loading