IMPALA-4839: Remove implicit 'localhost' for KUDU_MASTER_HOSTS The Kudu query tests were failing on a remote cluster because the Kudu master was always set to '127.0.0.1', with no way to override it.
This patch corrects the issue with a number of changes: - Add a pytest command line option to specify an arbitrary Kudu master - Consolidate the place where the default Kudu master is derived. It had been stored both in the env and in tests/common/__init__.py, with different files looking to different places. For now, just look to the env, and remove the value from __init__.py. - The kudu_client test fixture in conftest.py was using the connect() method from impala.dbapi (part of the Impyla library), without specifying the host param. In the absence of that, the default value is 'localhost', so add the host param to the connect() call. - Define the various defaults for pytest config as constants at the top of conftest.py. Change-Id: I9df71480a165f4ce21ae3edab6ce7227fbf76f77 Reviewed-on: http://gerrit.cloudera.org:8080/5877 Reviewed-by: Matthew Jacobs <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/894bb778 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/894bb778 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/894bb778 Branch: refs/heads/master Commit: 894bb7785519f4f00d1af52e9c6602c43396503c Parents: 9e36088 Author: David Knupp <[email protected]> Authored: Wed Feb 1 16:03:28 2017 -0800 Committer: Impala Public Jenkins <[email protected]> Committed: Tue Feb 14 21:51:39 2017 +0000 ---------------------------------------------------------------------- bin/impala-config.sh | 2 +- bin/start-impala-cluster.py | 9 ++-- testdata/bin/generate-schema-statements.py | 2 +- tests/common/__init__.py | 2 - tests/conftest.py | 69 +++++++++++++++---------- tests/custom_cluster/test_kudu.py | 2 +- tests/query_test/test_kudu.py | 5 +- 7 files changed, 52 insertions(+), 39 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/894bb778/bin/impala-config.sh ---------------------------------------------------------------------- diff --git a/bin/impala-config.sh b/bin/impala-config.sh index 2cc0429..4a89432 100755 --- a/bin/impala-config.sh +++ b/bin/impala-config.sh @@ -326,7 +326,7 @@ else fi export NUM_CONCURRENT_TESTS="${NUM_CONCURRENT_TESTS-${CORES}}" -export KUDU_MASTER="${KUDU_MASTER:-127.0.0.1}" +export KUDU_MASTER_HOSTS="${KUDU_MASTER_HOSTS:-127.0.0.1}" export KUDU_MASTER_PORT="${KUDU_MASTER_PORT:-7051}" export IMPALA_FE_DIR="$IMPALA_HOME/fe" http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/894bb778/bin/start-impala-cluster.py ---------------------------------------------------------------------- diff --git a/bin/start-impala-cluster.py b/bin/start-impala-cluster.py index df7dea6..b276b08 100755 --- a/bin/start-impala-cluster.py +++ b/bin/start-impala-cluster.py @@ -27,9 +27,8 @@ from getpass import getuser from time import sleep, time from optparse import OptionParser from testdata.common import cgroups -from tests.common import KUDU_MASTER_HOSTS - +KUDU_MASTER_HOSTS = os.getenv('KUDU_MASTER_HOSTS', '127.0.0.1') DEFAULT_IMPALA_MAX_LOG_FILES = os.environ.get('IMPALA_MAX_LOG_FILES', 10) @@ -72,7 +71,7 @@ parser.add_option("--log_level", type="int", dest="log_level", default=1, help="Set the impalad backend logging level") parser.add_option("--jvm_args", dest="jvm_args", default="", help="Additional arguments to pass to the JVM(s) during startup.") -parser.add_option("--kudu_masters", default=KUDU_MASTER_HOSTS, +parser.add_option("--kudu_master_hosts", default=KUDU_MASTER_HOSTS, help="The host name or address of the Kudu master. Multiple masters " "can be specified using a comma separated list.") @@ -237,9 +236,9 @@ def start_impalad_instances(cluster_size): (mem_limit, # Goes first so --impalad_args will override it. build_impalad_logging_args(i, service_name), build_jvm_args(i), build_impalad_port_args(i), param_args) - if options.kudu_masters: + if options.kudu_master_hosts: # Must be prepended, otherwise the java options interfere. - args = "-kudu_master_hosts %s %s" % (options.kudu_masters, args) + args = "-kudu_master_hosts %s %s" % (options.kudu_master_hosts, args) stderr_log_file_path = os.path.join(options.log_dir, '%s-error.log' % service_name) exec_impala_process(IMPALAD_PATH, args, stderr_log_file_path) http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/894bb778/testdata/bin/generate-schema-statements.py ---------------------------------------------------------------------- diff --git a/testdata/bin/generate-schema-statements.py b/testdata/bin/generate-schema-statements.py index 07e722a..a214822 100755 --- a/testdata/bin/generate-schema-statements.py +++ b/testdata/bin/generate-schema-statements.py @@ -226,7 +226,7 @@ def build_table_template(file_format, columns, partition_columns, row_format, partitioned_by = "partition by hash partitions 3" # Fetch KUDU host and port from environment - kudu_master = os.getenv("KUDU_MASTER_ADDRESS", "127.0.0.1") + kudu_master = os.getenv("KUDU_MASTER_HOSTS", "127.0.0.1") kudu_master_port = os.getenv("KUDU_MASTER_PORT", "7051") row_format_stmt = str() tblproperties["kudu.master_addresses"] = \ http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/894bb778/tests/common/__init__.py ---------------------------------------------------------------------- diff --git a/tests/common/__init__.py b/tests/common/__init__.py index 665bfe2..e69de29 100644 --- a/tests/common/__init__.py +++ b/tests/common/__init__.py @@ -1,2 +0,0 @@ -# These values should match the impalad startup flag -kudu_master_hosts -KUDU_MASTER_HOSTS = "127.0.0.1" http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/894bb778/tests/conftest.py ---------------------------------------------------------------------- diff --git a/tests/conftest.py b/tests/conftest.py index 44b00ff..351117d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -27,7 +27,6 @@ import logging import os import pytest -from common import KUDU_MASTER_HOSTS from common.test_result_verifier import QueryTestResult from tests.common.patterns import is_valid_impala_identifier from tests.comparison.db_connection import ImpalaConnection @@ -37,17 +36,23 @@ logging.basicConfig(level=logging.INFO, format='%(threadName)s: %(message)s') LOG = logging.getLogger('test_configuration') DEFAULT_CONN_TIMEOUT = 45 - -def _get_default_nn_http_addr(): - """Return the namenode ip and webhdfs port if the default shouldn't be used""" - if FILESYSTEM == 'isilon': - return "%s:%s" % (os.getenv("ISILON_NAMENODE"), ISILON_WEBHDFS_PORT) - return None +DEFAULT_EXPLORATION_STRATEGY = 'core' +DEFAULT_HDFS_XML_CONF = os.path.join(os.environ['HADOOP_CONF_DIR'], "hdfs-site.xml") +DEFAULT_HIVE_SERVER2 = 'localhost:11050' +DEFAULT_IMPALAD_HS2_PORT = '21050' +DEFAULT_IMPALADS = "localhost:21000,localhost:21001,localhost:21002" +DEFAULT_KUDU_MASTER_HOSTS = os.getenv('KUDU_MASTER_HOSTS', '127.0.0.1') +DEFAULT_KUDU_MASTER_PORT = os.getenv('KUDU_MASTER_PORT', '7051') +DEFAULT_METASTORE_SERVER = 'localhost:9083' +DEFAULT_NAMENODE_ADDR = None +if FILESYSTEM == 'isilon': + DEFAULT_NAMENODE_ADDR = "{node}:{port}".format(node=os.getenv("ISILON_NAMENODE"), + port=ISILON_WEBHDFS_PORT) def pytest_addoption(parser): """Adds a new command line options to py.test""" - parser.addoption("--exploration_strategy", default="core", + parser.addoption("--exploration_strategy", default=DEFAULT_EXPLORATION_STRATEGY, help="Default exploration strategy for all tests. Valid values: core, " "pairwise, exhaustive.") @@ -55,25 +60,27 @@ def pytest_addoption(parser): help="Override exploration strategy for specific workloads using the " "format: workload:exploration_strategy. Ex: tpch:core,tpcds:pairwise.") - parser.addoption("--impalad", default="localhost:21000,localhost:21001,localhost:21002", + parser.addoption("--impalad", default=DEFAULT_IMPALADS, help="A comma-separated list of impalad host:ports to target. Note: " "Not all tests make use of all impalad, some tests target just the " "first item in the list (it is considered the 'default'") - parser.addoption("--impalad_hs2_port", default="21050", + parser.addoption("--impalad_hs2_port", default=DEFAULT_IMPALAD_HS2_PORT, help="The impalad HiveServer2 port.") - parser.addoption("--metastore_server", default="localhost:9083", + parser.addoption("--metastore_server", default=DEFAULT_METASTORE_SERVER, help="The Hive Metastore server host:port to connect to.") - parser.addoption("--hive_server2", default="localhost:11050", + parser.addoption("--hive_server2", default=DEFAULT_HIVE_SERVER2, help="Hive's HiveServer2 host:port to connect to.") - default_xml_path = os.path.join(os.environ['HADOOP_CONF_DIR'], "hdfs-site.xml") - parser.addoption("--minicluster_xml_conf", default=default_xml_path, + parser.addoption("--kudu_master_hosts", default=DEFAULT_KUDU_MASTER_HOSTS, + help="Kudu master. Can be supplied as hostname, or hostname:port.") + + parser.addoption("--minicluster_xml_conf", default=DEFAULT_HDFS_XML_CONF, help="The full path to the HDFS xml configuration file") - parser.addoption("--namenode_http_address", default=_get_default_nn_http_addr(), + parser.addoption("--namenode_http_address", default=DEFAULT_NAMENODE_ADDR, help="The host:port for the HDFS Namenode's WebHDFS interface. Takes" " precedence over any configuration read from --minicluster_xml_conf") @@ -298,20 +305,21 @@ def kudu_client(): """Provides a new Kudu client as a pytest fixture. The client only exists for the duration of the method it is used in. """ - if "," in KUDU_MASTER_HOSTS: + kudu_master = pytest.config.option.kudu_master_hosts + + if "," in kudu_master: raise Exception("Multi-master not supported yet") - if ":" in KUDU_MASTER_HOSTS: - host, port = KUDU_MASTER_HOSTS.split(":") + if ":" in kudu_master: + host, port = kudu_master.split(":") else: - host, port = KUDU_MASTER_HOSTS, 7051 + host, port = kudu_master, DEFAULT_KUDU_MASTER_PORT kudu_client = kudu_connect(host, port) + yield kudu_client + try: - yield kudu_client - finally: - try: - kudu_client.close() - except Exception as e: - LOG.warn("Error closing Kudu client: %s", e) + kudu_client.close() + except Exception as e: + LOG.warn("Error closing Kudu client: %s", e) @pytest.yield_fixture(scope="class") @@ -387,12 +395,17 @@ def __unique_conn(db_name=None, timeout=DEFAULT_CONN_TIMEOUT): @contextlib.contextmanager def __auto_closed_conn(db_name=None, timeout=DEFAULT_CONN_TIMEOUT): - """Returns a connection to Impala. This is intended to be used in a "with" block. The - connection will be closed upon exiting the block. + """Returns a connection to Impala. This is intended to be used in a "with" block. + The connection will be closed upon exiting the block. The returned connection will have a 'db_name' property. """ - conn = impala_connect(database=db_name, timeout=timeout) + default_impalad = pytest.config.option.impalad.split(',')[0] + impalad_host = default_impalad.split(':')[0] + hs2_port = pytest.config.option.impalad_hs2_port + + conn = impala_connect(host=impalad_host, port=hs2_port, database=db_name, + timeout=timeout) try: conn.db_name = db_name yield conn http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/894bb778/tests/custom_cluster/test_kudu.py ---------------------------------------------------------------------- diff --git a/tests/custom_cluster/test_kudu.py b/tests/custom_cluster/test_kudu.py index bd1584b..9377863 100644 --- a/tests/custom_cluster/test_kudu.py +++ b/tests/custom_cluster/test_kudu.py @@ -19,10 +19,10 @@ import logging import pytest from kudu.schema import INT32 -from tests.common import KUDU_MASTER_HOSTS from tests.common.custom_cluster_test_suite import CustomClusterTestSuite from tests.common.kudu_test_suite import KuduTestSuite +KUDU_MASTER_HOSTS = pytest.config.option.kudu_master_hosts LOG = logging.getLogger(__name__) class TestKuduOperations(CustomClusterTestSuite, KuduTestSuite): http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/894bb778/tests/query_test/test_kudu.py ---------------------------------------------------------------------- diff --git a/tests/query_test/test_kudu.py b/tests/query_test/test_kudu.py index 5b28120..b7235e8 100644 --- a/tests/query_test/test_kudu.py +++ b/tests/query_test/test_kudu.py @@ -32,11 +32,12 @@ import logging import pytest import textwrap -from tests.common import KUDU_MASTER_HOSTS from tests.common.kudu_test_suite import KuduTestSuite from tests.common.impala_cluster import ImpalaCluster from tests.verifiers.metric_verifier import MetricVerifier +KUDU_MASTER_HOSTS = pytest.config.option.kudu_master_hosts + LOG = logging.getLogger(__name__) class TestKuduOperations(KuduTestSuite): @@ -62,6 +63,8 @@ class TestKuduOperations(KuduTestSuite): def test_kudu_partition_ddl(self, vector, unique_database): self.run_test_case('QueryTest/kudu_partition_ddl', vector, use_db=unique_database) + @pytest.mark.skipif(pytest.config.option.testing_remote_cluster, + reason="Test references hardcoded hostnames: IMPALA-4873") @pytest.mark.execute_serially def test_kudu_alter_table(self, vector, unique_database): self.run_test_case('QueryTest/kudu_alter', vector, use_db=unique_database)
