From ca80b266ed831813a2a9ecec84d288ec3f8f5104 Mon Sep 17 00:00:00 2001 From: David Ames Date: Thu, 8 Oct 2020 22:55:30 +0000 Subject: [PATCH] Run db sync manually by leader only Without db_auto_create = False each barbican-worker attempts to run alembic upgrades on the database whenever they start. This leads to race conditions that can leave the sate of the database broken (multiple alembic version that "overlap"). This change runs the barbican-manage db upgrade by the leader only avoiding the race condition. charms.openstack also handles the openstack upgrade process with a call to instance.db_sync. Change-Id: I6b9498059c7057b73b1c3db0e355456c38b0510e Closes-Bug: #1827690 --- src/layer.yaml | 1 + src/lib/charm/openstack/barbican.py | 3 +++ src/reactive/barbican_handlers.py | 14 ++++++++++++++ src/templates/rocky/barbican.conf | 5 +---- unit_tests/test_barbican_handlers.py | 5 +++++ 5 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/layer.yaml b/src/layer.yaml index 71ca0b4..a8ec1ab 100644 --- a/src/layer.yaml +++ b/src/layer.yaml @@ -1,4 +1,5 @@ includes: + - layer:leadership - layer:openstack-api - interface:mysql-shared - interface:rabbitmq diff --git a/src/lib/charm/openstack/barbican.py b/src/lib/charm/openstack/barbican.py index 322c8bb..8575623 100644 --- a/src/lib/charm/openstack/barbican.py +++ b/src/lib/charm/openstack/barbican.py @@ -145,6 +145,9 @@ class BarbicanCharm(charms_openstack.charm.HAOpenStackCharm): group = "barbican" + # This is the command to sync the database + sync_cmd = ['sudo', '-u', 'barbican', 'barbican-manage', 'db', 'upgrade'] + def get_amqp_credentials(self): """Provide the default amqp username and vhost as a tuple. diff --git a/src/reactive/barbican_handlers.py b/src/reactive/barbican_handlers.py index ff855bc..96afbb9 100644 --- a/src/reactive/barbican_handlers.py +++ b/src/reactive/barbican_handlers.py @@ -60,6 +60,20 @@ def render_stuff(*args): 'secrets.available')) barbican_charm.configure_ssl() barbican_charm.assess_status() + reactive.set_flag('first-render') + + +@reactive.when('leadership.is_leader') +@reactive.when('charm.installed') +@reactive.when('shared-db.available') +@reactive.when('first-render') +@reactive.when_not('db.synced') +def run_db_migration(): + with charm.provide_charm_instance() as barbican_charm: + barbican_charm.db_sync() + barbican_charm.restart_all() + reactive.set_state('db.synced') + barbican_charm.assess_status() @reactive.when('secrets.new-plugin') diff --git a/src/templates/rocky/barbican.conf b/src/templates/rocky/barbican.conf index 75f6bb3..745c2c5 100644 --- a/src/templates/rocky/barbican.conf +++ b/src/templates/rocky/barbican.conf @@ -3,10 +3,7 @@ debug = {{ options.debug }} bind_host = {{ options.service_listen_info.barbican_worker.ip }} bind_port = {{ options.service_listen_info.barbican_worker.port }} host_href = {{ options.external_endpoints.barbican_worker.url }} - -# Create the Barbican database on service startup. This is `true` by default -# up to Ussuri and `false` by default from Victoria on: -db_auto_create = true +db_auto_create = False {% include "parts/section-transport-url" %} diff --git a/unit_tests/test_barbican_handlers.py b/unit_tests/test_barbican_handlers.py index e37f460..3b87f91 100644 --- a/unit_tests/test_barbican_handlers.py +++ b/unit_tests/test_barbican_handlers.py @@ -37,9 +37,14 @@ class TestRegisteredHooks(test_utils.TestRegisteredHooks): 'amqp.available',), 'secrets_plugin_configure': ('secrets.new-plugin',), 'cluster_connected': ('ha.connected',), + 'run_db_migration': ('leadership.is_leader', + 'charm.installed', + 'shared-db.available', + 'first-render',), }, 'when_not': { 'cluster_connected': ('ha.available',), + 'run_db_migration': ('db.synced',), }, } # test that the hooks were registered via the