React to `source` or `key` changes
After changing the `source` or `key` in the configuration, the `config-changed` hook was activated, but did not call `add_source` and `apt_update`. This logic has been added to the `config-changed` hook as well as the state changed to `blocked` if the addition of a new source or key fails. Closes-Bug: #1893034 Change-Id: I0e7afd5a06c4945341329dd96e8eef3016367386
This commit is contained in:
parent
a197911d6c
commit
b5726aa07c
|
@ -56,6 +56,7 @@ from charmhelpers.fetch import (
|
|||
apt_update,
|
||||
apt_install,
|
||||
add_source,
|
||||
SourceConfigError,
|
||||
)
|
||||
from charmhelpers.contrib.peerstorage import (
|
||||
peer_echo,
|
||||
|
@ -94,6 +95,7 @@ from charmhelpers.contrib.openstack.ha.utils import (
|
|||
update_hacluster_vip,
|
||||
update_hacluster_dns_ha,
|
||||
)
|
||||
from charmhelpers.core.unitdata import kv
|
||||
|
||||
from percona_utils import (
|
||||
determine_packages,
|
||||
|
@ -149,10 +151,10 @@ from percona_utils import (
|
|||
delete_replication_user,
|
||||
list_replication_users,
|
||||
check_mysql_connection,
|
||||
update_source,
|
||||
ADD_APT_REPOSITORY_FAILED,
|
||||
)
|
||||
|
||||
from charmhelpers.core.unitdata import kv
|
||||
|
||||
hooks = Hooks()
|
||||
|
||||
RES_MONITOR_PARAMS = ('params user="sstuser" password="%(sstpass)s" '
|
||||
|
@ -532,12 +534,29 @@ def config_changed():
|
|||
# leader or if the leader is bootstrapped and therefore ready for install.
|
||||
install_percona_xtradb_cluster()
|
||||
|
||||
# run a package update if the source or key has changed
|
||||
cfg = config()
|
||||
kvstore = kv()
|
||||
if cfg.changed("source") or cfg.changed("key"):
|
||||
status_set("maintenance", "Upgrading Percona packages")
|
||||
try:
|
||||
update_source(source=cfg["source"], key=cfg["key"])
|
||||
kvstore.set(ADD_APT_REPOSITORY_FAILED, False)
|
||||
except (subprocess.CalledProcessError, SourceConfigError):
|
||||
# NOTE (rgildein): Need to store the local state to prevent
|
||||
# `assess_status` from running, which changes the unit state
|
||||
# to "Unit is ready"
|
||||
kvstore.set(ADD_APT_REPOSITORY_FAILED, True)
|
||||
kvstore.flush()
|
||||
|
||||
if kvstore.get(ADD_APT_REPOSITORY_FAILED, False):
|
||||
return
|
||||
|
||||
if config('prefer-ipv6'):
|
||||
assert_charm_supports_ipv6()
|
||||
|
||||
hosts = get_cluster_hosts()
|
||||
leader_bootstrapped = is_leader_bootstrapped()
|
||||
leader_ip = leader_get('leader-ip')
|
||||
|
||||
# Cluster upgrade adds some complication
|
||||
cluster_series_upgrading = leader_get("cluster_series_upgrading")
|
||||
|
|
|
@ -49,9 +49,12 @@ from charmhelpers.core.hookenv import (
|
|||
)
|
||||
from charmhelpers.core.unitdata import kv
|
||||
from charmhelpers.fetch import (
|
||||
add_source,
|
||||
apt_install,
|
||||
apt_update,
|
||||
filter_installed_packages,
|
||||
get_upstream_version,
|
||||
SourceConfigError,
|
||||
)
|
||||
from charmhelpers.contrib.network.ip import (
|
||||
get_address_in_network,
|
||||
|
@ -88,6 +91,7 @@ HOSTS_FILE = '/etc/hosts'
|
|||
DEFAULT_MYSQL_PORT = 3306
|
||||
INITIAL_CLUSTERED_KEY = 'initial-cluster-complete'
|
||||
INITIAL_CLIENT_UPDATE_KEY = 'initial_client_update_done'
|
||||
ADD_APT_REPOSITORY_FAILED = "add-apt-repository-failed"
|
||||
|
||||
# NOTE(ajkavanagh) - this is 'required' for the pause/resume code for
|
||||
# maintenance mode, but is currently not populated as the
|
||||
|
@ -806,6 +810,16 @@ def assess_status(configs):
|
|||
@param configs: a templating.OSConfigRenderer() object
|
||||
@returns None - this function is executed for its side-effect
|
||||
"""
|
||||
kvstore = kv()
|
||||
if kvstore.get(ADD_APT_REPOSITORY_FAILED, False):
|
||||
# NOTE (rgildein): prevent unit status from changing from blocked
|
||||
# to "Unit is ready", if adding a new source failed
|
||||
log("skip assess_status, because adding new source failed", level=INFO)
|
||||
status_set(
|
||||
"blocked", "problem adding new source: {}".format(config("source"))
|
||||
)
|
||||
return
|
||||
|
||||
assess_status_func(configs)()
|
||||
if pxc_installed():
|
||||
# NOTE(fnordahl) ensure we do not call application_version_set with
|
||||
|
@ -1717,3 +1731,16 @@ def mysqldump(backup_dir, databases=None):
|
|||
gzcmd = ["gzip", _filename]
|
||||
subprocess.check_call(gzcmd)
|
||||
return "{}.gz".format(_filename)
|
||||
|
||||
|
||||
def update_source(source, key=None):
|
||||
"""Add source and run apt update."""
|
||||
try:
|
||||
add_source(source=source, key=key, fail_invalid=True)
|
||||
log("add new package source: {}".format(source))
|
||||
except (subprocess.CalledProcessError, SourceConfigError) as error:
|
||||
log(error, level=ERROR)
|
||||
raise
|
||||
else:
|
||||
apt_update() # run without retries
|
||||
log("apt update after adding a new package source")
|
||||
|
|
|
@ -402,10 +402,14 @@ class TestConfigChanged(CharmTestCase):
|
|||
'is_unit_paused_set',
|
||||
'is_unit_upgrading_set',
|
||||
'get_cluster_host_ip',
|
||||
'status_set',
|
||||
'kv',
|
||||
]
|
||||
|
||||
def setUp(self):
|
||||
CharmTestCase.setUp(self, hooks, self.TO_PATCH)
|
||||
self.test_config.set_previous("source", self.test_config["source"])
|
||||
self.test_config.set_previous("key", self.test_config["key"])
|
||||
self.config.side_effect = self.test_config.get
|
||||
self.is_unit_paused_set.return_value = False
|
||||
self.is_unit_upgrading_set.return_value = False
|
||||
|
@ -416,6 +420,9 @@ class TestConfigChanged(CharmTestCase):
|
|||
self.relation_ids.return_value = []
|
||||
self.is_relation_made.return_value = False
|
||||
self.get_cluster_hosts.return_value = []
|
||||
self.kvstore = mock.MagicMock()
|
||||
self.kvstore.get.return_value = False
|
||||
self.kv.return_value = self.kvstore
|
||||
|
||||
def _leader_get(key):
|
||||
settings = {'leader-ip': '10.10.10.10',
|
||||
|
@ -423,9 +430,36 @@ class TestConfigChanged(CharmTestCase):
|
|||
return settings.get(key)
|
||||
self.leader_get.side_effect = _leader_get
|
||||
|
||||
@mock.patch("percona_utils.add_source")
|
||||
@mock.patch("percona_utils.apt_update")
|
||||
def test_config_change_update_source(
|
||||
self, mock_apt_update, mock_add_source):
|
||||
"""Ensure add_source and apt_update is called after changing source"""
|
||||
self.test_config.set("source", "test-source")
|
||||
|
||||
hooks.config_changed()
|
||||
self.status_set.assert_any_call(
|
||||
"maintenance", "Upgrading Percona packages")
|
||||
mock_add_source.assert_called_once_with(
|
||||
source="test-source", key=None, fail_invalid=True)
|
||||
mock_apt_update.assert_called_once_with()
|
||||
|
||||
def test_config_change_update_source_failed(self):
|
||||
"""Ensure failure after configuring an invalid source"""
|
||||
self.test_config.set("source", "invalid-source")
|
||||
self.kvstore.get.return_value = True
|
||||
|
||||
hooks.config_changed()
|
||||
self.kvstore.set.assert_called_once_with(
|
||||
hooks.ADD_APT_REPOSITORY_FAILED, True)
|
||||
self.status_set.assert_has_calls([
|
||||
mock.call("maintenance", "Upgrading Percona packages"),
|
||||
])
|
||||
|
||||
def test_config_changed_open_port(self):
|
||||
'''Ensure open_port is called with MySQL default port'''
|
||||
self.is_leader_bootstrapped.return_value = True
|
||||
|
||||
hooks.config_changed()
|
||||
self.open_port.assert_called_with(3306)
|
||||
|
||||
|
@ -779,7 +813,7 @@ class TestConfigs(CharmTestCase):
|
|||
]
|
||||
|
||||
def setUp(self):
|
||||
CharmTestCase.setUp(self, hooks, TO_PATCH)
|
||||
CharmTestCase.setUp(self, hooks, self.TO_PATCH)
|
||||
self.config.side_effect = self.test_config.get
|
||||
self.default_config = self._get_default_config()
|
||||
for key, value in self.default_config.items():
|
||||
|
@ -987,11 +1021,6 @@ class TestConfigs(CharmTestCase):
|
|||
|
||||
class TestClusterRelation(CharmTestCase):
|
||||
|
||||
TO_PATCH = [
|
||||
'config',
|
||||
'is_leader',
|
||||
]
|
||||
|
||||
def setUp(self):
|
||||
CharmTestCase.setUp(self, hooks, TO_PATCH)
|
||||
self.config.side_effect = self.test_config.get
|
||||
|
|
|
@ -4,6 +4,8 @@ import tempfile
|
|||
|
||||
import mock
|
||||
|
||||
from charmhelpers.fetch import SourceConfigError
|
||||
|
||||
import percona_utils
|
||||
|
||||
from test_utils import CharmTestCase, patch_open
|
||||
|
@ -518,6 +520,24 @@ class UtilsTests(CharmTestCase):
|
|||
percona_utils.maybe_notify_bootstrapped()
|
||||
_notify_bootstrapped.assert_called_once_with(cluster_uuid=_uuid)
|
||||
|
||||
@mock.patch("percona_utils.add_source")
|
||||
@mock.patch("percona_utils.apt_update")
|
||||
def test_update_source(self, mock_apt_update, mock_add_source):
|
||||
"""Ensure that add_source and apt_update has been called"""
|
||||
percona_utils.update_source("test-source", key=None)
|
||||
|
||||
mock_add_source.assert_called_once_with(
|
||||
source="test-source", key=None, fail_invalid=True)
|
||||
mock_apt_update.assert_called_once_with()
|
||||
|
||||
@mock.patch("percona_utils.apt_update")
|
||||
def test_update_invalid_source(self, mock_apt_update):
|
||||
"""Ensure raise error and set blocked status after invalid source"""
|
||||
with self.assertRaises(SourceConfigError):
|
||||
percona_utils.update_source("invalid-source", key=None)
|
||||
|
||||
mock_apt_update.assert_not_called()
|
||||
|
||||
|
||||
class UtilsTestsStatus(CharmTestCase):
|
||||
|
||||
|
@ -606,11 +626,14 @@ class UtilsTestsCTC(CharmTestCase):
|
|||
'is_clustered',
|
||||
'distributed_wait',
|
||||
'clustered_once',
|
||||
'kv'
|
||||
'kv',
|
||||
]
|
||||
|
||||
def setUp(self):
|
||||
super(UtilsTestsCTC, self).setUp(percona_utils, self.TO_PATCH)
|
||||
kvstore = mock.MagicMock()
|
||||
kvstore.get.return_value = False
|
||||
self.kv.return_value = kvstore
|
||||
|
||||
@mock.patch.object(percona_utils, 'pxc_installed')
|
||||
@mock.patch.object(percona_utils, 'determine_packages')
|
||||
|
|
Loading…
Reference in New Issue