From d37da2cfa9fa28dee1ab4187bc1b3e86572a68e8 Mon Sep 17 00:00:00 2001
From: Jeffrey Zhang <zhang.lei.fly@gmail.com>
Date: Sun, 25 Sep 2016 20:13:54 +0800
Subject: [PATCH] Optimize reconfigure action for glance

Partically-implements: blueprint better-reconfigure
Change-Id: I89e30e8b87f24a621c521d915842a4af0042d6fe
---
 ansible/library/kolla_docker.py               | 41 +++++++++---
 ansible/roles/glance/defaults/main.yml        | 24 ++++++-
 ansible/roles/glance/handlers/main.yml        | 46 +++++++++++++
 .../roles/glance/tasks/bootstrap_service.yml  | 12 ++--
 ansible/roles/glance/tasks/config.yml         | 66 ++++++++++++++-----
 ansible/roles/glance/tasks/deploy.yml         |  5 +-
 ansible/roles/glance/tasks/precheck.yml       |  8 +--
 ansible/roles/glance/tasks/pull.yml           | 16 ++---
 ansible/roles/glance/tasks/reconfigure.yml    | 64 +-----------------
 ansible/roles/glance/tasks/start.yml          | 25 -------
 ansible/roles/glance/tasks/upgrade.yml        |  3 +-
 tests/test_kolla_docker.py                    | 51 +++++++++++---
 12 files changed, 207 insertions(+), 154 deletions(-)
 create mode 100644 ansible/roles/glance/handlers/main.yml
 mode change 100644 => 120000 ansible/roles/glance/tasks/reconfigure.yml
 delete mode 100644 ansible/roles/glance/tasks/start.yml

diff --git a/ansible/library/kolla_docker.py b/ansible/library/kolla_docker.py
index 28f752a2e..1b9b1d467 100644
--- a/ansible/library/kolla_docker.py
+++ b/ansible/library/kolla_docker.py
@@ -33,6 +33,7 @@ options:
     required: True
     type: str
     choices:
+      - compare_container
       - compare_image
       - create_volume
       - get_container_env
@@ -40,6 +41,7 @@ options:
       - pull_image
       - remove_container
       - remove_volume
+      - recreate_or_restart_container
       - restart_container
       - start_container
       - stop_container
@@ -267,6 +269,12 @@ class DockerWorker(object):
             return None
         return self.dc.inspect_container(self.params.get('name'))
 
+    def compare_container(self):
+        container = self.check_container()
+        if not container or self.check_container_differs():
+            self.changed = True
+        return self.changed
+
     def check_container_differs(self):
         container_info = self.get_container_info()
         return (
@@ -554,6 +562,21 @@ class DockerWorker(object):
         options = self.build_container_options()
         self.dc.create_container(**options)
 
+    def recreate_or_restart_container(self):
+        self.changed = True
+        container = self.check_container()
+        # get config_strategy from env
+        environment = self.params.get('environment')
+        config_strategy = environment.get('KOLLA_CONFIG_STRATEGY')
+
+        if not container:
+            self.start_container()
+        elif container and config_strategy == 'COPY_ONCE':
+            self.remove_container()
+            self.start_container()
+        elif container and config_strategy == 'COPY_ALWAYS':
+            self.restart_container()
+
     def start_container(self):
         if not self.check_image():
             self.pull_image()
@@ -651,16 +674,14 @@ class DockerWorker(object):
 def generate_module():
     argument_spec = dict(
         common_options=dict(required=False, type='dict', default=dict()),
-        action=dict(required=True, type='str', choices=['compare_image',
-                                                        'create_volume',
-                                                        'get_container_env',
-                                                        'get_container_state',
-                                                        'pull_image',
-                                                        'remove_container',
-                                                        'remove_volume',
-                                                        'restart_container',
-                                                        'start_container',
-                                                        'stop_container']),
+        action=dict(required=True, type='str',
+                    choices=['compare_container', 'compare_image',
+                             'create_volume', 'get_container_env',
+                             'get_container_state', 'pull_image',
+                             'recreate_or_restart_container',
+                             'remove_container', 'remove_volume',
+                             'restart_container', 'start_container',
+                             'stop_container']),
         api_version=dict(required=False, type='str', default='auto'),
         auth_email=dict(required=False, type='str'),
         auth_password=dict(required=False, type='str'),
diff --git a/ansible/roles/glance/defaults/main.yml b/ansible/roles/glance/defaults/main.yml
index 6a7a5985d..0f6fede03 100644
--- a/ansible/roles/glance/defaults/main.yml
+++ b/ansible/roles/glance/defaults/main.yml
@@ -1,8 +1,26 @@
 ---
 project_name: "glance"
-glance_service_groups:
-  - { name: glance_api, service: glance-api, group: glance-api }
-  - { name: glance_registry, service: glance-registry, group: glance-registry }
+
+glance_services:
+  glance-api:
+    container_name: glance_api
+    group: glance-api
+    enabled: true
+    image: "{{ glance_api_image_full }}"
+    volumes:
+      - "{{ node_config_directory }}/glance-api/:{{ container_config_directory }}/:ro"
+      - "/etc/localtime:/etc/localtime:ro"
+      - "glance:/var/lib/glance/"
+      - "kolla_logs:/var/log/kolla/"
+  glance-registry:
+    container_name: glance_registry
+    group: glance-registry
+    enabled: true
+    image: "{{ glance_registry_image_full }}"
+    volumes:
+      - "{{ node_config_directory }}/glance-registry/:{{ container_config_directory }}/:ro"
+      - "/etc/localtime:/etc/localtime:ro"
+      - "kolla_logs:/var/log/kolla/"
 
 
 ####################
diff --git a/ansible/roles/glance/handlers/main.yml b/ansible/roles/glance/handlers/main.yml
new file mode 100644
index 000000000..d2ec351ae
--- /dev/null
+++ b/ansible/roles/glance/handlers/main.yml
@@ -0,0 +1,46 @@
+---
+- name: Restart glance-api container
+  vars:
+    service_name: "glance-api"
+    service: "{{ glance_services[service_name] }}"
+    config_json: "{{ glance_config_jsons.results|selectattr('item.key', 'equalto', service_name)|first }}"
+    glance_conf: "{{ glance_confs.results|selectattr('item.key', 'equalto', service_name)|first }}"
+    policy_json: "{{ glance_policy_jsons.results|selectattr('item.key', 'equalto', service_name)|first }}"
+    glance_api_container: "{{ check_glance_containers.results|selectattr('item.key', 'equalto', service_name)|first }}"
+  kolla_docker:
+    action: "recreate_or_restart_container"
+    common_options: "{{ docker_common_options }}"
+    name: "{{ service.container_name }}"
+    image: "{{ service.image }}"
+    volumes: "{{ service.volumes }}"
+  when:
+    - inventory_hostname in groups[service.group]
+    - service.enabled | bool
+    - config_json.changed | bool
+      or glance_conf.changed | bool
+      or policy_json.changed | bool
+      or glance_api_container.changed | bool
+
+
+- name: Restart glance-registry container
+  vars:
+    service_name: "glance-registry"
+    service: "{{ glance_services[service_name] }}"
+    config_json: "{{ glance_config_jsons.results|selectattr('item.key', 'equalto', service_name)|first }}"
+    glance_conf: "{{ glance_confs.results|selectattr('item.key', 'equalto', service_name)|first }}"
+    policy_json: "{{ glance_policy_jsons.results|selectattr('item.key', 'equalto', service_name)|first }}"
+    glance_registry_container: "{{ check_glance_containers.results|selectattr('item.key', 'equalto', service_name)|first }}"
+  kolla_docker:
+    action: "recreate_or_restart_container"
+    common_options: "{{ docker_common_options }}"
+    name: "{{ service.container_name }}"
+    image: "{{ service.image }}"
+    volumes: "{{ service.volumes }}"
+  when:
+    - inventory_hostname in groups[service.group]
+    - service.enabled | bool
+    - config_json.changed | bool
+      or glance_conf.changed | bool
+      or policy_json.changed | bool
+      or glance_registry_container.changed | bool
+
diff --git a/ansible/roles/glance/tasks/bootstrap_service.yml b/ansible/roles/glance/tasks/bootstrap_service.yml
index decc23177..ae311e427 100644
--- a/ansible/roles/glance/tasks/bootstrap_service.yml
+++ b/ansible/roles/glance/tasks/bootstrap_service.yml
@@ -1,5 +1,7 @@
 ---
 - name: Running Glance bootstrap container
+  vars:
+    glance_api: "{{ glance_services['glance-api'] }}"
   kolla_docker:
     action: "start_container"
     common_options: "{{ docker_common_options }}"
@@ -7,15 +9,11 @@
     environment:
       KOLLA_BOOTSTRAP:
       KOLLA_CONFIG_STRATEGY: "{{ config_strategy }}"
-    image: "{{ glance_api_image_full }}"
+    image: "{{ glance_api.image }}"
     labels:
       BOOTSTRAP:
     name: "bootstrap_glance"
     restart_policy: "never"
-    volumes:
-      - "{{ node_config_directory }}/glance-api/:{{ container_config_directory }}/:ro"
-      - "/etc/localtime:/etc/localtime:ro"
-      - "glance:/var/lib/glance/"
-      - "kolla_logs:/var/log/kolla/"
+    volumes: "{{ glance_api.volumes }}"
   run_once: True
-  delegate_to: "{{ groups['glance-api'][0] }}"
+  delegate_to: "{{ groups[glance_api.group][0] }}"
diff --git a/ansible/roles/glance/tasks/config.yml b/ansible/roles/glance/tasks/config.yml
index d88993353..4575558ed 100644
--- a/ansible/roles/glance/tasks/config.yml
+++ b/ansible/roles/glance/tasks/config.yml
@@ -1,34 +1,46 @@
 ---
 - name: Ensuring config directories exist
   file:
-    path: "{{ node_config_directory }}/{{ item.service }}"
+    path: "{{ node_config_directory }}/{{ item.key }}"
     state: "directory"
     recurse: yes
-  when: inventory_hostname in groups[item.group]
-  with_items: "{{ glance_service_groups }}"
+  when: inventory_hostname in groups[item.value.group]
+  with_dict: "{{ glance_services }}"
 
 - name: Copying over config.json files for services
   template:
-    src: "{{ item.service }}.json.j2"
-    dest: "{{ node_config_directory }}/{{ item.service }}/config.json"
-  when: inventory_hostname in groups[item.group]
-  with_items: "{{ glance_service_groups }}"
+    src: "{{ item.key }}.json.j2"
+    dest: "{{ node_config_directory }}/{{ item.key }}/config.json"
+  register: glance_config_jsons
+  when:
+    - item.value.enabled | bool
+    - inventory_hostname in groups[item.value.group]
+  with_dict: "{{ glance_services }}"
+  notify:
+    - Restart glance-api container
+    - Restart glance-registry container
 
 - name: Copying over glance-*.conf
   merge_configs:
     vars:
-      service_name: "{{ item.service }}"
+      service_name: "{{ item.key }}"
     sources:
-      - "{{ role_path }}/templates/{{ item.service }}.conf.j2"
+      - "{{ role_path }}/templates/{{ item.key }}.conf.j2"
       - "{{ node_custom_config }}/global.conf"
       - "{{ node_custom_config }}/database.conf"
       - "{{ node_custom_config }}/messaging.conf"
       - "{{ node_custom_config }}/glance.conf"
-      - "{{ node_custom_config }}/glance/{{ item.service }}.conf"
-      - "{{ node_custom_config }}/glance/{{ inventory_hostname }}/{{ item.service }}.conf"
-    dest: "{{ node_config_directory }}/{{ item.service }}/{{ item.service }}.conf"
-  when: inventory_hostname in groups[item.group]
-  with_items: "{{ glance_service_groups }}"
+      - "{{ node_custom_config }}/glance/{{ item.key }}.conf"
+      - "{{ node_custom_config }}/glance/{{ inventory_hostname }}/{{ item.key }}.conf"
+    dest: "{{ node_config_directory }}/{{ item.key }}/{{ item.key }}.conf"
+  register: glance_confs
+  when:
+    - item.value.enabled | bool
+    - inventory_hostname in groups[item.value.group]
+  with_dict: "{{ glance_services }}"
+  notify:
+    - Restart glance-api container
+    - Restart glance-registry container
 
 - name: Check if policies shall be overwritten
   local_action: stat path="{{ node_custom_config }}/glance/policy.json"
@@ -37,8 +49,28 @@
 - name: Copying over existing policy.json
   template:
     src: "{{ node_custom_config }}/glance/policy.json"
-    dest: "{{ node_config_directory }}/{{ item.service }}/policy.json"
+    dest: "{{ node_config_directory }}/{{ item.key }}/policy.json"
+  register: glance_policy_jsons
   when:
-    - inventory_hostname in groups[item.group]
     - glance_policy.stat.exists
-  with_items: "{{ glance_service_groups }}"
+    - inventory_hostname in groups[item.value.group]
+  with_dict: "{{ glance_services }}"
+  notify:
+    - Restart glance-api container
+    - Restart glance-registry container
+
+- name: Check glance containers
+  kolla_docker:
+    action: "compare_container"
+    common_options: "{{ docker_common_options }}"
+    name: "{{ item.value.container_name }}"
+    image: "{{ item.value.image }}"
+    volumes: "{{ item.value.volumes }}"
+  register: check_glance_containers
+  when:
+    - inventory_hostname in groups[item.value.group]
+    - item.value.enabled | bool
+  with_dict: "{{ glance_services }}"
+  notify:
+    - Restart glance-api container
+    - Restart glance-registry container
diff --git a/ansible/roles/glance/tasks/deploy.yml b/ansible/roles/glance/tasks/deploy.yml
index 0704c65bb..bc589e3fa 100644
--- a/ansible/roles/glance/tasks/deploy.yml
+++ b/ansible/roles/glance/tasks/deploy.yml
@@ -22,9 +22,8 @@
 - include: bootstrap.yml
   when: inventory_hostname in groups['glance-api']
 
-- include: start.yml
-  when: inventory_hostname in groups['glance-api'] or
-        inventory_hostname in groups['glance-registry']
+- name: Flush handlers
+  meta: flush_handlers
 
 - include: check.yml
   when: inventory_hostname in groups['glance-api'] or
diff --git a/ansible/roles/glance/tasks/precheck.yml b/ansible/roles/glance/tasks/precheck.yml
index 37876d4f4..687fd0d9d 100644
--- a/ansible/roles/glance/tasks/precheck.yml
+++ b/ansible/roles/glance/tasks/precheck.yml
@@ -1,9 +1,7 @@
 ---
 - name: Get container facts
   kolla_container_facts:
-    name:
-      - glance_api
-      - glance_registry
+    name: "{{ glance_services.keys() }}"
   register: container_facts
 
 - name: Checking free port for Glance API
@@ -13,7 +11,7 @@
     connect_timeout: 1
     state: stopped
   when:
-    - inventory_hostname in groups['glance-api']
+    - inventory_hostname in groups[glance_services['glance-api']['group']]
     - container_facts['glance_api'] is not defined
 
 - name: Checking free port for Glance Registry
@@ -23,5 +21,5 @@
     connect_timeout: 1
     state: stopped
   when:
-    - inventory_hostname in groups['glance-registry']
+    - inventory_hostname in groups[glance_services['glance-api']['group']]
     - container_facts['glance_registry'] is not defined
diff --git a/ansible/roles/glance/tasks/pull.yml b/ansible/roles/glance/tasks/pull.yml
index 0a3ebeb58..4bc7a313e 100644
--- a/ansible/roles/glance/tasks/pull.yml
+++ b/ansible/roles/glance/tasks/pull.yml
@@ -1,14 +1,10 @@
 ---
-- name: Pulling glance-api image
+- name: Pulling glance images
   kolla_docker:
     action: "pull_image"
     common_options: "{{ docker_common_options }}"
-    image: "{{ glance_api_image_full }}"
-  when: inventory_hostname in groups['glance-api']
-
-- name: Pulling glance-registry image
-  kolla_docker:
-    action: "pull_image"
-    common_options: "{{ docker_common_options }}"
-    image: "{{ glance_registry_image_full }}"
-  when: inventory_hostname in groups['glance-registry']
+    image: "{{ item.value.image }}"
+  when:
+    - inventory_hostname in groups[item.value.group]
+    - item.value.enabled | bool
+  with_dict: "{{ glance_services }}"
diff --git a/ansible/roles/glance/tasks/reconfigure.yml b/ansible/roles/glance/tasks/reconfigure.yml
deleted file mode 100644
index 22f0eb79b..000000000
--- a/ansible/roles/glance/tasks/reconfigure.yml
+++ /dev/null
@@ -1,63 +0,0 @@
----
-- name: Ensuring the containers up
-  kolla_docker:
-    name: "{{ item.name }}"
-    action: "get_container_state"
-  register: container_state
-  failed_when: container_state.Running == false
-  when: inventory_hostname in groups[item.group]
-  with_items: "{{ glance_service_groups }}"
-
-- include: config.yml
-
-- name: Check the configs
-  command: docker exec {{ item.name }} /usr/local/bin/kolla_set_configs --check
-  changed_when: false
-  failed_when: false
-  register: check_results
-  when: inventory_hostname in groups[item.group]
-  with_items: "{{ glance_service_groups }}"
-
-# NOTE(jeffrey4l): when config_strategy == 'COPY_ALWAYS'
-# and container env['KOLLA_CONFIG_STRATEGY'] == 'COPY_ONCE',
-# just remove the container and start again
-- name: Containers config strategy
-  kolla_docker:
-    name: "{{ item.name }}"
-    action: "get_container_env"
-  register: container_envs
-  when: inventory_hostname in groups[item.group]
-  with_items: "{{ glance_service_groups }}"
-
-- name: Remove the containers
-  kolla_docker:
-    name: "{{ item[0]['name'] }}"
-    action: "remove_container"
-  register: remove_containers
-  when:
-    - inventory_hostname in groups[item[0]['group']]
-    - config_strategy == "COPY_ONCE" or item[1]['KOLLA_CONFIG_STRATEGY'] == 'COPY_ONCE'
-    - item[2]['rc'] == 1
-  with_together:
-    - "{{ glance_service_groups }}"
-    - "{{ container_envs.results }}"
-    - "{{ check_results.results }}"
-
-- include: bootstrap_service.yml
-
-- include: start.yml
-  when: remove_containers.changed
-
-- name: Restart containers
-  kolla_docker:
-    name: "{{ item[0]['name'] }}"
-    action: "restart_container"
-  when:
-    - inventory_hostname in groups[item[0]['group']]
-    - config_strategy == 'COPY_ALWAYS'
-    - item[1]['KOLLA_CONFIG_STRATEGY'] != 'COPY_ONCE'
-    - item[2]['rc'] == 1
-  with_together:
-    - "{{ glance_service_groups }}"
-    - "{{ container_envs.results }}"
-    - "{{ check_results.results }}"
diff --git a/ansible/roles/glance/tasks/reconfigure.yml b/ansible/roles/glance/tasks/reconfigure.yml
new file mode 120000
index 000000000..0412f9220
--- /dev/null
+++ b/ansible/roles/glance/tasks/reconfigure.yml
@@ -0,0 +1 @@
+deploy.yml
\ No newline at end of file
diff --git a/ansible/roles/glance/tasks/start.yml b/ansible/roles/glance/tasks/start.yml
deleted file mode 100644
index bec16bbee..000000000
--- a/ansible/roles/glance/tasks/start.yml
+++ /dev/null
@@ -1,25 +0,0 @@
----
-- name: Starting glance-registry container
-  kolla_docker:
-    action: "start_container"
-    common_options: "{{ docker_common_options }}"
-    image: "{{ glance_registry_image_full }}"
-    name: "glance_registry"
-    volumes:
-      - "{{ node_config_directory }}/glance-registry/:{{ container_config_directory }}/:ro"
-      - "/etc/localtime:/etc/localtime:ro"
-      - "kolla_logs:/var/log/kolla/"
-  when: inventory_hostname in groups['glance-registry']
-
-- name: Starting glance-api container
-  kolla_docker:
-    action: "start_container"
-    common_options: "{{ docker_common_options }}"
-    image: "{{ glance_api_image_full }}"
-    name: "glance_api"
-    volumes:
-      - "{{ node_config_directory }}/glance-api/:{{ container_config_directory }}/:ro"
-      - "/etc/localtime:/etc/localtime:ro"
-      - "glance:/var/lib/glance/"
-      - "kolla_logs:/var/log/kolla/"
-  when: inventory_hostname in groups['glance-api']
diff --git a/ansible/roles/glance/tasks/upgrade.yml b/ansible/roles/glance/tasks/upgrade.yml
index 308053080..c38db1adf 100644
--- a/ansible/roles/glance/tasks/upgrade.yml
+++ b/ansible/roles/glance/tasks/upgrade.yml
@@ -3,4 +3,5 @@
 
 - include: bootstrap_service.yml
 
-- include: start.yml
+- name: Flush handlers
+  meta: flush_handlers
diff --git a/tests/test_kolla_docker.py b/tests/test_kolla_docker.py
index efcf16c0d..c9f055cc5 100644
--- a/tests/test_kolla_docker.py
+++ b/tests/test_kolla_docker.py
@@ -43,16 +43,13 @@ class ModuleArgsTest(base.BaseTestCase):
         argument_spec = dict(
             common_options=dict(required=False, type='dict', default=dict()),
             action=dict(
-                required=True, type='str', choices=['compare_image',
-                                                    'create_volume',
-                                                    'get_container_env',
-                                                    'get_container_state',
-                                                    'pull_image',
-                                                    'remove_container',
-                                                    'remove_volume',
-                                                    'restart_container',
-                                                    'start_container',
-                                                    'stop_container']),
+                required=True, type='str',
+                choices=['compare_container', 'compare_image', 'create_volume',
+                         'get_container_env', 'get_container_state',
+                         'pull_image', 'recreate_or_restart_container',
+                         'remove_container', 'remove_volume',
+                         'restart_container', 'start_container',
+                         'stop_container']),
             api_version=dict(required=False, type='str', default='auto'),
             auth_email=dict(required=False, type='str'),
             auth_password=dict(required=False, type='str'),
@@ -372,6 +369,40 @@ class TestContainer(base.BaseTestCase):
         self.dw.module.fail_json.assert_called_once_with(
             msg="No such container: fake_container")
 
+    def test_recreate_or_restart_container_not_container(self):
+        self.dw = get_DockerWorker({
+            'environment': dict(KOLLA_CONFIG_STRATEGY='COPY_ALWAYS')})
+        self.dw.check_container = mock.Mock(return_value=None)
+        self.dw.start_container = mock.Mock()
+
+        self.dw.recreate_or_restart_container()
+
+        self.dw.start_container.assert_called_once_with()
+
+    def test_recreate_or_restart_container_container_copy_always(self):
+        self.dw = get_DockerWorker({
+            'environment': dict(KOLLA_CONFIG_STRATEGY='COPY_ALWAYS')})
+        self.dw.check_container = mock.Mock(
+            return_value=self.fake_data['containers'][0])
+        self.dw.restart_container = mock.Mock()
+
+        self.dw.recreate_or_restart_container()
+
+        self.dw.restart_container.assert_called_once_with()
+
+    def test_recreate_or_restart_container_container_copy_once(self):
+        self.dw = get_DockerWorker({
+            'environment': dict(KOLLA_CONFIG_STRATEGY='COPY_ONCE')})
+        self.dw.check_container = mock.Mock(
+            return_value=self.fake_data['containers'][0])
+        self.dw.start_container = mock.Mock()
+        self.dw.remove_container = mock.Mock()
+
+        self.dw.recreate_or_restart_container()
+
+        self.dw.remove_container.assert_called_once_with()
+        self.dw.start_container.assert_called_once_with()
+
 
 class TestImage(base.BaseTestCase):
 
-- 
GitLab