From d7ce147e3744a21d62631cfb2f5bcefeff705a5c Mon Sep 17 00:00:00 2001 From: David Ames Date: Thu, 19 Mar 2020 08:31:01 -0700 Subject: [PATCH] Use mysql-shell python Currently the mysql-shell snap in the snap store is deb based and defaults to JavaScript. This change enables the use of the python interpreter for msyql-shell. It also allows for a resource version of the mysql-shell snap for use during development cycles. Change-Id: I349cd154760f157a755cb9c29b07862fbb64793f --- .../charm/openstack/mysql_innodb_cluster.py | 147 ++++++++++-------- src/metadata.yaml | 6 + src/reactive/mysql_innodb_cluster_handlers.py | 2 +- src/tests/bundles/eoan.yaml | 1 - test-requirements.txt | 2 +- tox.ini | 5 + unit_tests/__init__.py | 1 + ...ib_charm_openstack_mysql_innodb_cluster.py | 138 ++++++++-------- 8 files changed, 165 insertions(+), 137 deletions(-) diff --git a/src/lib/charm/openstack/mysql_innodb_cluster.py b/src/lib/charm/openstack/mysql_innodb_cluster.py index ffa6a31..148ebb3 100644 --- a/src/lib/charm/openstack/mysql_innodb_cluster.py +++ b/src/lib/charm/openstack/mysql_innodb_cluster.py @@ -193,9 +193,13 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm): :returns: Path to binary mysqlsh :rtype: str """ - # The current upstream snap uses mysql-shell + # Allow for various versions of the msyql-shell snap # When we get the alias use /snap/bin/mysqlsh - # return "/snap/bin/mysqlsh" + if os.path.exists("/snap/bin/mysqlsh"): + return "/snap/bin/mysqlsh" + if os.path.exists("/snap/bin/mysql-shell.mysqlsh"): + return "/snap/bin/mysql-shell.mysqlsh" + # Default to the full path version return "/snap/mysql-shell/current/usr/bin/mysqlsh" @property @@ -478,13 +482,14 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm): ch_core.hookenv.log("Configuring instance for clustering: {}." .format(address), "INFO") - _script = """ - dba.configureInstance('{}:{}@{}'); - var myshell = shell.connect('{}:{}@{}'); - myshell.runSql("RESTART;"); - """.format( - self.cluster_user, self.cluster_password, address, - self.cluster_user, self.cluster_password, address) + _script = ( + "dba.configure_instance('{user}:{pw}@{addr}')\n" + "myshell = shell.connect('{user}:{pw}@{addr}')\n" + "myshell.run_sql('RESTART;')" + .format( + user=self.cluster_user, + pw=self.cluster_password, + addr=address)) try: output = self.run_mysqlsh_script(_script) except subprocess.CalledProcessError as e: @@ -529,12 +534,12 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm): "WARNING") return - _script = """ - shell.connect("{}:{}@{}") - var cluster = dba.createCluster("{}", {{"autoRejoinTries": "{}"}}); - """.format( - self.cluster_user, self.cluster_password, self.cluster_address, - self.options.cluster_name, self.options.auto_rejoin_tries) + _script = ( + "shell.connect('{}:{}@{}')\n" + "cluster = dba.create_cluster('{}', {{'autoRejoinTries': '{}'}})" + .format( + self.cluster_user, self.cluster_password, self.cluster_address, + self.options.cluster_name, self.options.auto_rejoin_tries)) ch_core.hookenv.log("Creating cluster: {}." .format(self.options.cluster_name), "INFO") try: @@ -564,13 +569,13 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm): :returns: This function is called for its side effect :rtype: None """ - _script = """ - shell.connect("{}:{}@{}") - var cluster = dba.getCluster("{}"); - cluster.setOption("{}", {}) - """.format( - self.cluster_user, self.cluster_password, self.cluster_address, - self.options.cluster_name, key, value) + _script = ( + "shell.connect('{}:{}@{}')\n" + "cluster = dba.get_cluster('{}')\n" + "cluster.set_option('{}', {})" + .format( + self.cluster_user, self.cluster_password, self.cluster_address, + self.options.cluster_name, key, value)) try: output = self.run_mysqlsh_script(_script).decode("UTF-8") return output @@ -602,18 +607,17 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm): ch_core.hookenv.log("Adding instance, {}, to the cluster." .format(address), "INFO") - _script = """ - shell.connect("{}:{}@{}") - var cluster = dba.getCluster("{}"); - - print("Adding instances to the cluster."); - cluster.addInstance( - {{user: "{}", host: "{}", password: "{}", port: "3306"}}, - {{recoveryMethod: "clone"}}); - """.format( - self.cluster_user, self.cluster_password, self.cluster_address, - self.options.cluster_name, - self.cluster_user, address, self.cluster_password) + _script = ( + "shell.connect('{user}:{pw}@{caddr}')\n" + "cluster = dba.get_cluster('{name}')\n" + "cluster.add_instance(" + "{{'user': '{user}', 'host': '{addr}', 'password': '{pw}', " + "'port': '3306'}}," + "{{'recoveryMethod': 'clone'}})" + .format( + user=self.cluster_user, pw=self.cluster_password, + caddr=self.cluster_address, + name=self.options.cluster_name, addr=address)) try: output = self.run_mysqlsh_script(_script) except subprocess.CalledProcessError as e: @@ -630,7 +634,7 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm): def reboot_cluster_from_complete_outage(self): """Reboot cluster from complete outage. - Execute the dba.rebootClusterFromCompleteOutage() after an outage. + Execute the dba.reboot_cluster_from_complete_outage() after an outage. This will rebootstrap the cluster and join this instance to the previously existing cluster. @@ -640,11 +644,12 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm): :returns: This function is called for its side effect :rtype: None """ - _script = """ - shell.connect("{}:{}@{}") - dba.rebootClusterFromCompleteOutage(); - """.format( - self.cluster_user, self.cluster_password, self.cluster_address) + _script = ( + "shell.connect('{}:{}@{}')\n" + "dba.reboot_cluster_from_complete_outage()" + .format( + self.cluster_user, self.cluster_password, + self.cluster_address)) try: output = self.run_mysqlsh_script(_script).decode("UTF-8") ch_core.hookenv.log( @@ -663,7 +668,7 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm): def rejoin_instance(self, address): """Rejoin Instance to the cluster - Execute the cluster.rejoinInstanc(address) to rejoin the specified + Execute the cluster.rejoin_instance(address) to rejoin the specified instance to the cluster. :param self: Self @@ -673,14 +678,14 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm): :rtype: None """ ch_core.hookenv.log("XXX: REjoin {}".format(address)) - _script = """ - shell.connect("{}:{}@{}") - var cluster = dba.getCluster("{}"); - cluster.rejoinInstance("{}:{}@{}") - """.format( - self.cluster_user, self.cluster_password, self.cluster_address, - self.cluster_name, - self.cluster_user, self.cluster_password, address) + _script = ( + "shell.connect('{user}:{pw}@{caddr}')\n" + "cluster = dba.get_cluster('{name}')\n" + "cluster.rejoin_instance('{user}:{pw}@{addr}')" + .format( + user=self.cluster_user, pw=self.cluster_password, + caddr=self.cluster_address, + name=self.cluster_name, addr=address)) try: output = self.run_mysqlsh_script(_script).decode("UTF-8") ch_core.hookenv.log( @@ -725,15 +730,16 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm): .format(e.output.decode("UTF-8")), "ERROR") return - _script = """ - shell.connect("{}:{}@{}") - var cluster = dba.getCluster("{}"); - - print(cluster.status()) - """.format(self.cluster_user, self.cluster_password, - self.cluster_address, self.cluster_name) + _script = ( + "shell.connect('{}:{}@{}')\n" + "cluster = dba.get_cluster('{}')\n" + "print(cluster.status())" + .format(self.cluster_user, self.cluster_password, + self.cluster_address, self.cluster_name)) try: - output = self.run_mysqlsh_script(_script) + # Do not output stderr to avoid json.loads errors + # on warnings. + output = self.run_mysqlsh_script(_script, stderr=None) except subprocess.CalledProcessError as e: ch_core.hookenv.log( "Failed checking cluster status: {}" @@ -1143,19 +1149,21 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm): :returns: This function is called for its side effect :rtype: None """ - _script = """ - shell.connect("{}:{}@{}") - var cluster = dba.getCluster("{}"); - """.format( - self.cluster_user, self.cluster_password, self.cluster_address, - self.cluster_name) + _script = ( + "shell.connect('{}:{}@{}')\n" + "cluster = dba.get_cluster('{}')" + .format( + self.cluster_user, self.cluster_password, self.cluster_address, + self.cluster_name)) self.run_mysqlsh_script(_script) - def run_mysqlsh_script(self, script): + def run_mysqlsh_script(self, script, stderr=subprocess.STDOUT): """Execute a MySQL shell script :param script: Mysqlsh script :type script: str + :param stderr: stderr to stdout or None + :type stderr: Union[subprocess.STDOUT, None] :side effect: Calls subprocess.check_output :raises subprocess.CalledProcessError: Raises CalledProcessError if the script gets a non-zero return @@ -1163,13 +1171,18 @@ class MySQLInnoDBClusterCharm(charms_openstack.charm.OpenStackCharm): :returns: subprocess output :rtype: UTF-8 byte string """ - with tempfile.NamedTemporaryFile(mode="w", suffix=".js") as _file: + # Use the /root dir because of the confined mysql-shell snap + with tempfile.NamedTemporaryFile( + mode="w", suffix=".py", dir="/root") as _file: _file.write(script) _file.flush() - cmd = ([self.mysqlsh_bin, "--no-wizard", "-f", _file.name]) + # Specify python as this is not the default in the deb version + # of the mysql-shell snap + cmd = [ + self.mysqlsh_bin, "--no-wizard", "--python", "-f", _file.name] return subprocess.check_output( - cmd, stderr=subprocess.STDOUT) + cmd, stderr=stderr) def write_root_my_cnf(self): """Write root my.cnf diff --git a/src/metadata.yaml b/src/metadata.yaml index 4cb67ff..6c4e1a0 100644 --- a/src/metadata.yaml +++ b/src/metadata.yaml @@ -17,3 +17,9 @@ provides: peers: cluster: interface: mysql-innodb-cluster +resources: + mysql-shell: + type: file + filename: 'mysql-shell.snap' + description: | + Snap for mysql-shell diff --git a/src/reactive/mysql_innodb_cluster_handlers.py b/src/reactive/mysql_innodb_cluster_handlers.py index f5efdbc..9d4042a 100644 --- a/src/reactive/mysql_innodb_cluster_handlers.py +++ b/src/reactive/mysql_innodb_cluster_handlers.py @@ -220,7 +220,7 @@ def config_changed(): with charm.provide_charm_instance() as instance: try: instance.wait_until_cluster_available() - except: + except Exception: ch_core.hookenv.log( "Cluster was not availble as expected.", "WARNING") ch_core.hookenv.log("Non-leader requst to restart.", "DEBUG") diff --git a/src/tests/bundles/eoan.yaml b/src/tests/bundles/eoan.yaml index 6f4d030..fa1004f 100644 --- a/src/tests/bundles/eoan.yaml +++ b/src/tests/bundles/eoan.yaml @@ -6,7 +6,6 @@ applications: series: eoan charm: ../../../mysql-innodb-cluster num_units: 3 - options: keystone: charm: cs:~openstack-charmers-next/keystone num_units: 1 diff --git a/test-requirements.txt b/test-requirements.txt index 0ab97f6..1c8aff7 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -4,7 +4,7 @@ # https://github.com/openstack-charmers/release-tools # # Lint and unit test requirements -flake8>=2.2.4,<=2.4.1 +flake8>=2.2.4 stestr>=2.2.0 requests>=2.18.4 charms.reactive diff --git a/tox.ini b/tox.ini index 20c843d..b39d344 100644 --- a/tox.ini +++ b/tox.ini @@ -50,6 +50,11 @@ basepython = python3.7 deps = -r{toxinidir}/test-requirements.txt commands = stestr run --slowest {posargs} +[testenv:py38] +basepython = python3.8 +deps = -r{toxinidir}/test-requirements.txt +commands = stestr run --slowest {posargs} + [testenv:pep8] basepython = python3 deps = -r{toxinidir}/test-requirements.txt diff --git a/unit_tests/__init__.py b/unit_tests/__init__.py index 87dfd20..e5a979f 100644 --- a/unit_tests/__init__.py +++ b/unit_tests/__init__.py @@ -26,6 +26,7 @@ def _add_path(path): if path not in sys.path: sys.path.insert(1, path) + _add_path(_src) _add_path(_lib) _add_path(_reactive) diff --git a/unit_tests/test_lib_charm_openstack_mysql_innodb_cluster.py b/unit_tests/test_lib_charm_openstack_mysql_innodb_cluster.py index 137faa7..f730353 100644 --- a/unit_tests/test_lib_charm_openstack_mysql_innodb_cluster.py +++ b/unit_tests/test_lib_charm_openstack_mysql_innodb_cluster.py @@ -78,7 +78,7 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper): self.config_data = {} self.data = {} self.stdin = mock.MagicMock() - self.filename = "script.js" + self.filename = "script.py" self.file = mock.MagicMock() self.file.name = self.filename self.ntf = mock.MagicMock() @@ -260,10 +260,11 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper): return databases def test_mysqlsh_bin(self): + self.patch_object(mysql_innodb_cluster.os.path, "exists") midbc = mysql_innodb_cluster.MySQLInnoDBClusterCharm() self.assertEqual( midbc.mysqlsh_bin, - "/snap/mysql-shell/current/usr/bin/mysqlsh") + "/snap/bin/mysqlsh") def test_mysql_password(self): midbc = mysql_innodb_cluster.MySQLInnoDBClusterCharm() @@ -441,13 +442,13 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper): midbc._get_password.side_effect = self._fake_data midbc.wait_until_connectable = mock.MagicMock() midbc.run_mysqlsh_script = mock.MagicMock() - _script = """ - dba.configureInstance('{}:{}@{}'); - var myshell = shell.connect('{}:{}@{}'); - myshell.runSql("RESTART;"); - """.format( - midbc.cluster_user, midbc.cluster_password, _addr, - midbc.cluster_user, midbc.cluster_password, _addr) + _script = ( + "dba.configure_instance('{}:{}@{}')\n" + "myshell = shell.connect('{}:{}@{}')\n" + "myshell.run_sql('RESTART;')" + .format( + midbc.cluster_user, midbc.cluster_password, _addr, + midbc.cluster_user, midbc.cluster_password, _addr)) midbc.configure_instance(_addr) self.is_flag_set.assert_called_once_with( @@ -475,12 +476,12 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper): midbc.run_mysqlsh_script = mock.MagicMock() midbc.options.cluster_name = _name midbc.options.auto_rejoin_tries = _tries - _script = """ - shell.connect("{}:{}@{}") - var cluster = dba.createCluster("{}", {{"autoRejoinTries": "{}"}}); - """.format( - midbc.cluster_user, midbc.cluster_password, - midbc.cluster_address, midbc.cluster_name, _tries) + _script = ( + "shell.connect('{}:{}@{}')\n" + "cluster = dba.create_cluster('{}', {{'autoRejoinTries': '{}'}})" + .format( + midbc.cluster_user, midbc.cluster_password, + midbc.cluster_address, midbc.cluster_name, _tries)) midbc.create_cluster() _is_flag_set_calls = [ @@ -509,18 +510,16 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper): midbc.wait_until_connectable = mock.MagicMock() midbc.run_mysqlsh_script = mock.MagicMock() midbc.options.cluster_name = _name - _script = """ - shell.connect("{}:{}@{}") - var cluster = dba.getCluster("{}"); - - print("Adding instances to the cluster."); - cluster.addInstance( - {{user: "{}", host: "{}", password: "{}", port: "3306"}}, - {{recoveryMethod: "clone"}}); - """.format( - midbc.cluster_user, midbc.cluster_password, - midbc.cluster_address, midbc.cluster_name, - midbc.cluster_user, _remote_addr, midbc.cluster_password) + _script = ( + "shell.connect('{}:{}@{}')\n" + "cluster = dba.get_cluster('{}')\n" + "cluster.add_instance(" + "{{'user': '{}', 'host': '{}', 'password': '{}', 'port': '3306'}}," + "{{'recoveryMethod': 'clone'}})" + .format( + midbc.cluster_user, midbc.cluster_password, + midbc.cluster_address, midbc.cluster_name, + midbc.cluster_user, _remote_addr, midbc.cluster_password)) midbc.add_instance_to_cluster(_remote_addr) self.is_flag_set.assert_called_once_with( @@ -856,18 +855,17 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper): midbc.run_mysqlsh_script = mock.MagicMock() midbc.run_mysqlsh_script.return_value = _json_string.encode("UTF-8") - _script = """ - shell.connect("{}:{}@{}") - var cluster = dba.getCluster("{}"); - - print(cluster.status()) - """.format( - midbc.cluster_user, midbc.cluster_password, - midbc.cluster_address, midbc.cluster_name) + _script = ( + "shell.connect('{}:{}@{}')\n" + "cluster = dba.get_cluster('{}')\n" + "print(cluster.status())" + .format( + midbc.cluster_user, midbc.cluster_password, + midbc.cluster_address, midbc.cluster_name)) self.assertEqual(_string, midbc.get_cluster_status()) midbc.wait_until_cluster_available.assert_called_once() - midbc.run_mysqlsh_script.assert_called_once_with(_script) + midbc.run_mysqlsh_script.assert_called_once_with(_script, stderr=None) # Cached data midbc.run_mysqlsh_script.reset_mock() @@ -879,7 +877,7 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper): midbc.run_mysqlsh_script.reset_mock() midbc._cached_cluster_status = _string self.assertEqual(_string, midbc.get_cluster_status(nocache=True)) - midbc.run_mysqlsh_script.assert_called_once_with(_script) + midbc.run_mysqlsh_script.assert_called_once_with(_script, stderr=None) def test_get_cluster_status_summary(self): _status_dict = {"defaultReplicaSet": {"status": "OK"}} @@ -1014,12 +1012,12 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper): midbc = mysql_innodb_cluster.MySQLInnoDBClusterCharm() midbc.options.cluster_name = _name midbc.run_mysqlsh_script = mock.MagicMock() - _script = """ - shell.connect("{}:{}@{}") - var cluster = dba.getCluster("{}"); - """.format( - midbc.cluster_user, midbc.cluster_password, - midbc.cluster_address, midbc.cluster_name) + _script = ( + "shell.connect('{}:{}@{}')\n" + "cluster = dba.get_cluster('{}')" + .format( + midbc.cluster_user, midbc.cluster_password, + midbc.cluster_address, midbc.cluster_name)) # Cluster available midbc.wait_until_cluster_available() @@ -1035,11 +1033,14 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper): def test_run_mysqlsh_script(self): _byte_string = "UTF-8 byte string".encode("UTF-8") self.subprocess.check_output.return_value = _byte_string - _script = """print("Hello World!")""" + _script = "print('Hello World!')" midbc = mysql_innodb_cluster.MySQLInnoDBClusterCharm() - self.assertEqual(_byte_string, midbc.run_mysqlsh_script(_script)) + self.assertEqual( + _byte_string, + midbc.run_mysqlsh_script(_script, stderr=self.stdin)) self.subprocess.check_output.assert_called_once_with( - [midbc.mysqlsh_bin, "--no-wizard", "-f", self.filename], + [midbc.mysqlsh_bin, "--no-wizard", "--python", "-f", + self.filename], stderr=self.stdin) self.file.write.assert_called_once_with(_script) @@ -1122,19 +1123,22 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper): _string = "status output" _key = "option_name" _value = "option_value" + _local_addr = "10.10.50.50" + self.get_relation_ip.return_value = _local_addr midbc = mysql_innodb_cluster.MySQLInnoDBClusterCharm() midbc.options.cluster_name = _name midbc.run_mysqlsh_script = mock.MagicMock() midbc.run_mysqlsh_script.return_value = _string.encode("UTF-8") - _script = """ - shell.connect("{}:{}@{}") - var cluster = dba.getCluster("{}"); - cluster.setOption("{}", {}) - """.format( - midbc.cluster_user, midbc.cluster_password, midbc.cluster_address, - midbc.options.cluster_name, _key, _value) + _script = ( + "shell.connect('{}:{}@{}')\n" + "cluster = dba.get_cluster('{}')\n" + "cluster.set_option('{}', {})" + .format( + midbc.cluster_user, midbc.cluster_password, + midbc.cluster_address, midbc.options.cluster_name, + _key, _value)) self.assertEqual(_string, midbc.set_cluster_option(_key, _value)) midbc.run_mysqlsh_script.assert_called_once_with(_script) @@ -1152,12 +1156,12 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper): midbc._get_password = mock.MagicMock() midbc._get_password.return_value = _pass - _script = """ - shell.connect("{}:{}@{}") - dba.rebootClusterFromCompleteOutage(); - """.format( - midbc.cluster_user, midbc.cluster_password, midbc.cluster_address, - midbc.options.cluster_name) + _script = ( + "shell.connect('{}:{}@{}')\n" + "dba.reboot_cluster_from_complete_outage()" + .format( + midbc.cluster_user, midbc.cluster_password, + midbc.cluster_address, midbc.options.cluster_name)) self.assertEqual(_string, midbc.reboot_cluster_from_complete_outage()) midbc.run_mysqlsh_script.assert_called_once_with(_script) @@ -1176,13 +1180,13 @@ class TestMySQLInnoDBClusterCharm(test_utils.PatchHelper): midbc._get_password = mock.MagicMock() midbc._get_password.return_value = _pass - _script = """ - shell.connect("{}:{}@{}") - var cluster = dba.getCluster("{}"); - cluster.rejoinInstance("{}:{}@{}") - """.format( - midbc.cluster_user, midbc.cluster_password, midbc.cluster_address, - midbc.options.cluster_name, - midbc.cluster_user, midbc.cluster_password, _remote_addr) + _script = ( + "shell.connect('{}:{}@{}')\n" + "cluster = dba.get_cluster('{}')\n" + "cluster.rejoin_instance('{}:{}@{}')" + .format( + midbc.cluster_user, midbc.cluster_password, + midbc.cluster_address, midbc.options.cluster_name, + midbc.cluster_user, midbc.cluster_password, _remote_addr)) self.assertEqual(_string, midbc.rejoin_instance(_remote_addr)) midbc.run_mysqlsh_script.assert_called_once_with(_script)