Fix handling of docker restart policy

Docker has no restart policy named 'never'. It has 'no'.
This has bitten us already (see [1]) and might bite us again whenever
we want to change the restart policy to 'no'.

This patch makes our docker integration honor all valid restart policies
and only valid restart policies.
All relevant docker restart policy usages are patched as well.

I added some FIXMEs around which are relevant to kolla-ansible docker
integration. They are not fixed in here to not alter behavior.

[1] https://review.opendev.org/667363

Change-Id: I1c9764fb9bbda08a71186091aced67433ad4e3d6
Signed-off-by: Radosław Piliszek <radoslaw.piliszek@gmail.com>
This commit is contained in:
Radosław Piliszek 2019-07-12 17:15:42 +02:00
parent b7098faf88
commit 6a737b1968
59 changed files with 91 additions and 84 deletions

View File

@ -100,10 +100,10 @@ docker_runtime_directory: ""
docker_log_max_file: 5
docker_log_max_size: 50m
# Valid options are [ never, on-failure, always, unless-stopped ]
# Valid options are [ no, on-failure, always, unless-stopped ]
docker_restart_policy: "unless-stopped"
# '0' means unlimited retries
# '0' means unlimited retries (applies only to 'on-failure' policy)
docker_restart_policy_retry: "10"
# Common options used throughout Docker

View File

@ -14,6 +14,12 @@
# See the License for the specific language governing permissions and
# limitations under the License.
# FIXME(yoctozepto): this module does *not* validate "common_options" which are
# a hacky way to seed most usages of kolla_docker in kolla-ansible ansible
# playbooks - caution has to be exerted when setting "common_options"
# FIXME(yoctozepto): restart_policy is *not* checked in the container
import json
import os
import shlex
@ -156,17 +162,17 @@ options:
type: bool
restart_policy:
description:
- Determine what docker does when the container exits
- When docker restarts the container (does not affect checks)
required: False
type: str
choices:
- never
- no
- on-failure
- always
- unless-stopped
restart_retries:
description:
- How many times to attempt a restart if restart_policy is set
- How many times to attempt a restart if 'on-failure' policy is set
type: int
default: 10
volumes:
@ -675,16 +681,17 @@ class DockerWorker(object):
dimensions = self.parse_dimensions(dimensions)
options.update(dimensions)
if self.params.get('restart_policy') in ['on-failure',
'always',
'unless-stopped']:
policy = {'Name': self.params.get('restart_policy')}
restart_policy = self.params.get('restart_policy')
if restart_policy is not None:
restart_policy = {'Name': restart_policy}
# NOTE(Jeffrey4l): MaximumRetryCount is only needed for on-failure
# policy
if self.params.get('restart_policy') == 'on-failure':
if restart_policy['Name'] == 'on-failure':
retries = self.params.get('restart_retries')
policy['MaximumRetryCount'] = retries
options['restart_policy'] = policy
if retries is not None:
restart_policy['MaximumRetryCount'] = retries
options['restart_policy'] = restart_policy
if binds:
options['binds'] = binds
@ -917,7 +924,6 @@ def generate_module():
remove_on_exit=dict(required=False, type='bool', default=True),
restart_policy=dict(required=False, type='str', choices=[
'no',
'never',
'on-failure',
'always',
'unless-stopped']),

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_aodh"
restart_policy: "never"
restart_policy: no
volumes: "{{ aodh_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[aodh_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_barbican"
restart_policy: "never"
restart_policy: no
volumes: "{{ barbican_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[barbican_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_blazar"
restart_policy: "never"
restart_policy: no
volumes: "{{ blazar_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[blazar_api.group][0] }}"

View File

@ -15,7 +15,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_ceilometer"
restart_policy: "never"
restart_policy: no
volumes: "{{ ceilometer_notification.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[ceilometer_notification.group][0] }}"

View File

@ -78,7 +78,7 @@
BOOTSTRAP:
name: "bootstrap_osd_{{ item.0 }}"
privileged: True
restart_policy: "never"
restart_policy: no
volumes:
- "{{ node_config_directory }}/ceph-osd/:{{ container_config_directory }}/:ro"
- "/etc/localtime:/etc/localtime:ro"
@ -139,7 +139,7 @@
BOOTSTRAP:
name: "bootstrap_osd_cache_{{ item.0 }}"
privileged: True
restart_policy: "never"
restart_policy: no
volumes:
- "{{ node_config_directory }}/ceph-osd/:{{ container_config_directory }}/:ro"
- "/etc/localtime:/etc/localtime:ro"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_ceph_mon"
restart_policy: "never"
restart_policy: no
volumes:
- "{{ node_config_directory }}/ceph-mon/:{{ container_config_directory }}/:ro"
- "/etc/localtime:/etc/localtime:ro"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_cinder"
restart_policy: "never"
restart_policy: no
volumes: "{{ cinder_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[cinder_api.group][0] }}"

View File

@ -27,7 +27,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_cinder"
restart_policy: "never"
restart_policy: no
volumes: "{{ cinder_api.volumes }}"
run_once: True
delegate_to: "{{ groups[cinder_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_cloudkitty"
restart_policy: "never"
restart_policy: no
volumes: "{{ cloudkitty_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[cloudkitty_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_congress"
restart_policy: "never"
restart_policy: no
volumes: "{{ congress_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[congress_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_cyborg"
restart_policy: "never"
restart_policy: no
volumes: "{{ cyborg_api.volumes }}"
run_once: True
delegate_to: "{{ groups[cyborg_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_designate"
restart_policy: "never"
restart_policy: no
volumes: "{{ designate_central.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[designate_central.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_freezer"
restart_policy: "never"
restart_policy: no
volumes: "{{ freezer_api.volumes | reject('equalto', '') | list }}"
run_once: True
delegate_to: "{{ groups[freezer_api.group][0] }}"

View File

@ -32,7 +32,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_glance"
restart_policy: "never"
restart_policy: no
volumes: "{{ glance_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[glance_api.group][0] }}"

View File

@ -43,7 +43,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_glance"
restart_policy: "never"
restart_policy: no
volumes: "{{ glance_api.volumes }}"
run_once: True
delegate_to: "{{ groups[glance_api.group][0] }}"
@ -64,7 +64,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_glance"
restart_policy: "never"
restart_policy: no
volumes: "{{ glance_api.volumes }}"
run_once: True
delegate_to: "{{ groups[glance_api.group][0] }}"
@ -94,7 +94,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_glance"
restart_policy: "never"
restart_policy: no
volumes: "{{ glance_api.volumes }}"
run_once: True
delegate_to: "{{ groups[glance_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_gnocchi"
restart_policy: "never"
restart_policy: no
volumes: "{{ gnocchi_api.volumes }}"
run_once: True
delegate_to: "{{ groups[gnocchi_api.group][0] }}"

View File

@ -22,7 +22,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_heat"
restart_policy: "never"
restart_policy: no
volumes: "{{ heat_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[heat_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_horizon"
restart_policy: "never"
restart_policy: no
volumes: "{{ horizon.volumes }}"
run_once: True
delegate_to: "{{ groups[horizon.group][0] }}"

View File

@ -68,6 +68,6 @@
labels:
BOOTSTRAP:
name: "bootstrap_ironic_pxe"
restart_policy: "never"
restart_policy: no
volumes: "{{ ironic_pxe.volumes }}"
when: inventory_hostname in groups[ironic_pxe.group]

View File

@ -18,7 +18,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_ironic"
restart_policy: "never"
restart_policy: no
volumes: "{{ ironic_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[ironic_api.group][0] }}"
@ -39,7 +39,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_ironic_inspector"
restart_policy: "never"
restart_policy: no
volumes: "{{ ironic_inspector.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[ironic_inspector.group][0] }}"

View File

@ -36,7 +36,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_ironic"
restart_policy: "never"
restart_policy: no
volumes: "{{ ironic_api.volumes }}"
run_once: True
delegate_to: "{{ groups[ironic_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_karbor"
restart_policy: "never"
restart_policy: no
volumes: "{{ karbor_api.volumes }}"
run_once: True
delegate_to: "{{ groups[karbor_api.group][0] }}"

View File

@ -18,7 +18,7 @@
labels:
KOLLA_UPGRADE:
name: "init_upgrade_database"
restart_policy: "never"
restart_policy: no
volumes: "{{ service.volumes|reject('equalto', '')|list }}"
dimensions: "{{ service.dimensions }}"
run_once: True
@ -87,7 +87,7 @@
labels:
KOLLA_UPGRADE:
name: "finish_upgrade_database"
restart_policy: "never"
restart_policy: no
volumes: "{{ service.volumes|reject('equalto', '')|list }}"
dimensions: "{{ service.dimensions }}"
run_once: True

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_keystone"
restart_policy: "never"
restart_policy: no
volumes: "{{ keystone.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups['keystone'][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_magnum"
restart_policy: "never"
restart_policy: no
volumes: "{{ magnum_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[magnum_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_manila"
restart_policy: "never"
restart_policy: no
volumes: "{{ manila_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[manila_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "{{ service.container_name }}"
restart_policy: "never"
restart_policy: no
volumes: "{{ service.volumes }}"
dimensions: "{{ service.dimensions }}"
when:
@ -117,7 +117,7 @@
labels:
UPGRADE:
name: "upgrade_mariadb"
restart_policy: "never"
restart_policy: no
volumes: "{{ service.volumes }}"
no_log: true
when:
@ -183,7 +183,7 @@
labels:
UPGRADE:
name: "upgrade_mariadb"
restart_policy: "never"
restart_policy: no
volumes: "{{ service.volumes }}"
no_log: true
when:

View File

@ -6,7 +6,7 @@
common_options: "{{ docker_common_options }}"
image: "{{ xtrabackup_image_full }}"
name: "xtrabackup"
restart_policy: "never"
restart_policy: no
remove_on_exit: True
environment:
BACKUP_TYPE: "{{ mariadb_backup_type }}"

View File

@ -17,7 +17,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_mariadb"
restart_policy: "never"
restart_policy: no
volumes: "{{ service.volumes }}"
notify:
- Bootstrap MariaDB cluster

View File

@ -41,7 +41,7 @@
labels:
BOOTSTRAP:
name: mariadb_wsrep_recovery
restart_policy: "never"
restart_policy: no
volumes: "{{ mariadb_service.volumes }}"
- name: Copying MariaDB log file to /tmp
@ -135,7 +135,7 @@
labels:
BOOTSTRAP:
name: "{{ mariadb_service.container_name }}"
restart_policy: "never"
restart_policy: no
volumes: "{{ mariadb_service.volumes }}"
when:
- bootstrap_host is defined
@ -193,7 +193,7 @@
labels:
BOOTSTRAP:
name: "{{ mariadb_service.container_name }}"
restart_policy: "never"
restart_policy: no
volumes: "{{ mariadb_service.volumes }}"
when:
- bootstrap_host is defined

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_mistral"
restart_policy: "never"
restart_policy: no
volumes: "{{ mistral_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[mistral_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_monasca"
restart_policy: "never"
restart_policy: no
volumes: "{{ monasca_api.volumes }}"
run_once: True
delegate_to: "{{ groups[monasca_api.group][0] }}"

View File

@ -10,7 +10,7 @@
KOLLA_CONFIG_STRATEGY: "{{ config_strategy }}"
image: "{{ mongodb_image_full }}"
name: "bootstrap_mongodb"
restart_policy: "never"
restart_policy: no
volumes:
- "{{ node_config_directory }}/mongodb/:{{ container_config_directory }}/:ro"
- "/etc/localtime:/etc/localtime:ro"

View File

@ -12,7 +12,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_murano"
restart_policy: "never"
restart_policy: no
volumes:
- "{{ node_config_directory }}/murano-api/:{{ container_config_directory }}/:ro"
- "/etc/localtime:/etc/localtime:ro"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_neutron"
restart_policy: "never"
restart_policy: no
volumes: "{{ neutron_server.volumes }}"
run_once: True
delegate_to: "{{ groups[neutron_server.group][0] }}"
@ -36,7 +36,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_neutron_sfc"
restart_policy: "never"
restart_policy: no
volumes: "{{ neutron_server.volumes }}"
when:
- enable_neutron_sfc | bool

View File

@ -23,7 +23,7 @@
labels:
UPGRADE:
name: "bootstrap_neutron"
restart_policy: "never"
restart_policy: no
volumes: "{{ neutron_server.volumes }}"
run_once: True
delegate_to: "{{ groups['neutron-server'][0] }}"
@ -79,7 +79,7 @@
labels:
UPGRADE:
name: "bootstrap_neutron"
restart_policy: "never"
restart_policy: no
volumes: "{{ neutron_server.volumes }}"
run_once: True
delegate_to: "{{ groups['neutron-server'][0] }}"

View File

@ -18,7 +18,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_nova"
restart_policy: "never"
restart_policy: no
volumes: "{{ nova_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[nova_api.group][0] }}"

View File

@ -12,7 +12,7 @@
labels:
BOOTSTRAP:
name: "create_cell0_nova"
restart_policy: "never"
restart_policy: no
volumes: "{{ nova_api.volumes|reject('equalto', '')|list }}"
register: map_cell0
changed_when:
@ -39,7 +39,7 @@
labels:
BOOTSTRAP:
name: "list_cells_nova"
restart_policy: "never"
restart_policy: no
volumes: "{{ nova_api.volumes|reject('equalto', '')|list }}"
register: existing_cells_list
changed_when: false
@ -77,7 +77,7 @@
labels:
BOOTSTRAP:
name: "create_cell_nova"
restart_policy: "never"
restart_policy: no
volumes: "{{ nova_api.volumes|reject('equalto', '')|list }}"
register: base_cell
changed_when:
@ -103,7 +103,7 @@
labels:
BOOTSTRAP:
name: "create_cell_nova"
restart_policy: "never"
restart_policy: no
volumes: "{{ nova_api.volumes|reject('equalto', '')|list }}"
register: base_cell
changed_when:

View File

@ -40,7 +40,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_nova"
restart_policy: "never"
restart_policy: no
volumes: "{{ nova_api.volumes }}"
run_once: True
delegate_to: "{{ groups[nova_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_octavia"
restart_policy: "never"
restart_policy: no
volumes: "{{ octavia_api.volumes }}"
run_once: True
delegate_to: "{{ groups[octavia_api.group][0] }}"

View File

@ -15,7 +15,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_panko"
restart_policy: "never"
restart_policy: no
volumes: "{{ panko_api.volumes }}"
run_once: True
delegate_to: "{{ groups[panko_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_placement"
restart_policy: "never"
restart_policy: no
volumes: "{{ placement_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[placement_api.group][0] }}"

View File

@ -28,7 +28,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_placement"
restart_policy: "never"
restart_policy: no
volumes: "{{ placement_api.volumes }}"
run_once: True
delegate_to: "{{ groups[placement_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_qinling"
restart_policy: "never"
restart_policy: no
volumes: "{{ qinling_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[qinling_api.group][0] }}"

View File

@ -21,6 +21,6 @@
labels:
BOOTSTRAP:
name: "{{ project_name }}_bootstrap"
restart_policy: "never"
restart_policy: no
volumes: "{{ service.volumes }}"
when: rabbitmq_volume is changed

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_rally"
restart_policy: "never"
restart_policy: no
volumes: "{{ rally.volumes }}"
run_once: True
delegate_to: "{{ groups[rally.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_sahara"
restart_policy: "never"
restart_policy: no
volumes: "{{ sahara_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[sahara_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_searchlight"
restart_policy: "never"
restart_policy: no
volumes: "{{ searchlight_api.volumes }}"
run_once: True
delegate_to: "{{ groups[searchlight_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_senlin"
restart_policy: "never"
restart_policy: no
volumes: "{{ senlin_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[senlin_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_solum"
restart_policy: "never"
restart_policy: no
volumes: "{{ solum_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[solum_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_tacker"
restart_policy: "never"
restart_policy: no
volumes: "{{ tacker_server.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[tacker_server.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_trove"
restart_policy: "never"
restart_policy: no
volumes: "{{ trove_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[trove_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_vitrage"
restart_policy: "never"
restart_policy: no
volumes: "{{ vitrage_api.volumes | reject('equalto', '') | list }}"
run_once: True
delegate_to: "{{ groups[vitrage_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_watcher"
restart_policy: "never"
restart_policy: no
volumes: "{{ watcher_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[watcher_api.group][0] }}"

View File

@ -14,7 +14,7 @@
labels:
BOOTSTRAP:
name: "bootstrap_zun"
restart_policy: "never"
restart_policy: no
volumes: "{{ zun_api.volumes|reject('equalto', '')|list }}"
run_once: True
delegate_to: "{{ groups[zun_api.group][0] }}"

View File

@ -2,7 +2,7 @@
kolla_base_distro: "{{ base_distro }}"
kolla_install_type: "{{ install_type }}"
network_interface: "{{ api_interface_name }}"
docker_restart_policy: "never"
docker_restart_policy: "no"
# Use a random router id, otherwise it may result in the same router id
# in the CI gate.

View File

@ -13,6 +13,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
# FIXME(yoctozepto): tests do not imitate how ansible would handle module args
import copy
import imp
import os
@ -71,7 +73,6 @@ class ModuleArgsTest(base.BaseTestCase):
remove_on_exit=dict(required=False, type='bool', default=True),
restart_policy=dict(
required=False, type='str', choices=['no',
'never',
'on-failure',
'always',
'unless-stopped']),