From 086b3e4335194cbe2bbfcd8c422fa21952124b44 Mon Sep 17 00:00:00 2001 From: Gregory Thiemonge Date: Fri, 9 Dec 2022 02:14:36 -0500 Subject: Fix parsing of zookeeper jobboard backend options Fix the zookeeper backend options when values are passed as strings, a "False" string is now treated as the False boolean. Closes-Bug: #1999174 Change-Id: I048faf06d89ebf980efe0598e647f2ec89f65ada --- taskflow/jobs/backends/impl_zookeeper.py | 4 +- taskflow/persistence/backends/impl_zookeeper.py | 4 +- taskflow/tests/unit/jobs/test_zk_job.py | 44 ++++++++++++++++ taskflow/tests/unit/test_utils_kazoo_utils.py | 67 +++++++++++++++++++++++++ taskflow/utils/kazoo_utils.py | 29 ++++++++--- 5 files changed, 138 insertions(+), 10 deletions(-) create mode 100644 taskflow/tests/unit/test_utils_kazoo_utils.py (limited to 'taskflow') diff --git a/taskflow/jobs/backends/impl_zookeeper.py b/taskflow/jobs/backends/impl_zookeeper.py index fc9399a..1e45f0f 100644 --- a/taskflow/jobs/backends/impl_zookeeper.py +++ b/taskflow/jobs/backends/impl_zookeeper.py @@ -28,6 +28,7 @@ from kazoo.protocol import states as k_states from kazoo.recipe import watchers from oslo_serialization import jsonutils from oslo_utils import excutils +from oslo_utils import strutils from oslo_utils import timeutils from oslo_utils import uuidutils @@ -829,7 +830,8 @@ class ZookeeperJobBoard(base.NotifyingJobBoard): excp.raise_with_cause(excp.JobFailure, "Failed to connect to zookeeper") try: - if self._conf.get('check_compatible', True): + if strutils.bool_from_string( + self._conf.get('check_compatible'), default=True): kazoo_utils.check_compatible(self._client, self.MIN_ZK_VERSION) if self._worker is None and self._emit_notifications: self._worker = futurist.ThreadPoolExecutor(max_workers=1) diff --git a/taskflow/persistence/backends/impl_zookeeper.py b/taskflow/persistence/backends/impl_zookeeper.py index 1189723..e75826e 100644 --- a/taskflow/persistence/backends/impl_zookeeper.py +++ b/taskflow/persistence/backends/impl_zookeeper.py @@ -20,6 +20,7 @@ import contextlib from kazoo import exceptions as k_exc from kazoo.protocol import paths from oslo_serialization import jsonutils +from oslo_utils import strutils from taskflow import exceptions as exc from taskflow.persistence import path_based @@ -161,7 +162,8 @@ class ZkConnection(path_based.PathBasedConnection): def validate(self): with self._exc_wrapper(): try: - if self._conf.get('check_compatible', True): + if strutils.bool_from_string( + self._conf.get('check_compatible'), default=True): k_utils.check_compatible(self._client, MIN_ZK_VERSION) except exc.IncompatibleVersion: exc.raise_with_cause(exc.StorageFailure, "Backend storage is" diff --git a/taskflow/tests/unit/jobs/test_zk_job.py b/taskflow/tests/unit/jobs/test_zk_job.py index d93f148..23d0505 100644 --- a/taskflow/tests/unit/jobs/test_zk_job.py +++ b/taskflow/tests/unit/jobs/test_zk_job.py @@ -293,3 +293,47 @@ class ZakeJobboardTest(test.TestCase, ZookeeperBoardTestMixin): self.assertRaises(excp.NotImplementedError, self.board.register_entity, entity_instance_2) + + def test_connect_check_compatible(self): + # Valid version + client = fake_client.FakeClient() + board = impl_zookeeper.ZookeeperJobBoard( + 'test-board', {'check_compatible': True}, + client=client) + self.addCleanup(board.close) + self.addCleanup(self.close_client, client) + + with base.connect_close(board): + pass + + # Invalid version, no check + client = fake_client.FakeClient(server_version=(3, 2, 0)) + board = impl_zookeeper.ZookeeperJobBoard( + 'test-board', {'check_compatible': False}, + client=client) + self.addCleanup(board.close) + self.addCleanup(self.close_client, client) + + with base.connect_close(board): + pass + + # Invalid version, check_compatible=True + client = fake_client.FakeClient(server_version=(3, 2, 0)) + board = impl_zookeeper.ZookeeperJobBoard( + 'test-board', {'check_compatible': True}, + client=client) + self.addCleanup(board.close) + self.addCleanup(self.close_client, client) + + self.assertRaises(excp.IncompatibleVersion, board.connect) + + # Invalid version, check_compatible='False' + client = fake_client.FakeClient(server_version=(3, 2, 0)) + board = impl_zookeeper.ZookeeperJobBoard( + 'test-board', {'check_compatible': 'False'}, + client=client) + self.addCleanup(board.close) + self.addCleanup(self.close_client, client) + + with base.connect_close(board): + pass diff --git a/taskflow/tests/unit/test_utils_kazoo_utils.py b/taskflow/tests/unit/test_utils_kazoo_utils.py new file mode 100644 index 0000000..a28b3e8 --- /dev/null +++ b/taskflow/tests/unit/test_utils_kazoo_utils.py @@ -0,0 +1,67 @@ +# -*- coding: utf-8 -*- + +# Copyright (C) Red Hat +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from unittest import mock + +from taskflow import test +from taskflow.utils import kazoo_utils + + +class MakeClientTest(test.TestCase): + + @mock.patch("kazoo.client.KazooClient") + def test_make_client_config(self, mock_kazoo_client): + conf = {} + expected = { + 'hosts': 'localhost:2181', + 'logger': mock.ANY, + 'read_only': False, + 'randomize_hosts': False, + 'keyfile': None, + 'keyfile_password': None, + 'certfile': None, + 'use_ssl': False, + 'verify_certs': True + } + + kazoo_utils.make_client(conf) + + mock_kazoo_client.assert_called_once_with(**expected) + + mock_kazoo_client.reset_mock() + + # With boolean passed as strings + conf = { + 'use_ssl': 'True', + 'verify_certs': 'False' + } + expected = { + 'hosts': 'localhost:2181', + 'logger': mock.ANY, + 'read_only': False, + 'randomize_hosts': False, + 'keyfile': None, + 'keyfile_password': None, + 'certfile': None, + 'use_ssl': True, + 'verify_certs': False + } + + kazoo_utils.make_client(conf) + + mock_kazoo_client.assert_called_once_with(**expected) + + mock_kazoo_client.reset_mock() diff --git a/taskflow/utils/kazoo_utils.py b/taskflow/utils/kazoo_utils.py index 505c101..2b9f7b9 100644 --- a/taskflow/utils/kazoo_utils.py +++ b/taskflow/utils/kazoo_utils.py @@ -17,6 +17,7 @@ from kazoo import client from kazoo import exceptions as k_exc from oslo_utils import reflection +from oslo_utils import strutils from taskflow import exceptions as exc from taskflow import logging @@ -24,6 +25,15 @@ from taskflow import logging LOG = logging.getLogger(__name__) +CONF_TRANSFERS = ( + ('read_only', strutils.bool_from_string, False), + ('randomize_hosts', strutils.bool_from_string, False), + ('keyfile', None, None), + ('keyfile_password', None, None), + ('certfile', None, None), + ('use_ssl', strutils.bool_from_string, False), + ('verify_certs', strutils.bool_from_string, True)) + def _parse_hosts(hosts): if isinstance(hosts, str): @@ -193,16 +203,19 @@ def make_client(conf): """ # See: https://kazoo.readthedocs.io/en/latest/api/client.html client_kwargs = { - 'read_only': bool(conf.get('read_only')), - 'randomize_hosts': bool(conf.get('randomize_hosts')), 'logger': LOG, - 'keyfile': conf.get('keyfile', None), - 'keyfile_password': conf.get('keyfile_password', None), - 'certfile': conf.get('certfile', None), - 'use_ssl': conf.get('use_ssl', False), - 'verify_certs': conf.get('verify_certs', True), - } + + for key, value_type_converter, default in CONF_TRANSFERS: + if key in conf: + if value_type_converter is not None: + client_kwargs[key] = value_type_converter(conf[key], + default=default) + else: + client_kwargs[key] = conf[key] + else: + client_kwargs[key] = default + # See: https://kazoo.readthedocs.io/en/latest/api/retry.html if 'command_retry' in conf: client_kwargs['command_retry'] = conf['command_retry'] -- cgit v1.2.1