Repository: incubator-impala Updated Branches: refs/heads/master fc3ff1c52 -> 06abb5787
IMPALA-3664: S3A test_keys_do_not_work fails Previously when we created a new connection to S3 via the S3A connector, the S3AFileSystem object was created only then. However, recently it was realized that the object could be created even before we created a connection to the S3A file system; so the configuration values we pass through to the connection would be ignored. This is fixed by forcing the FS builder to return a new instance of the filesystem object, which will read the configuration changes we make. The test is also split up to test a DDL statement in a separate test because we would want that statement to succeed as it goes through the HMS and the SELECT statement to fail. Change-Id: I23b541eef747dd62e59390f8cc9ac6e5742ead40 Reviewed-on: http://gerrit.cloudera.org:8080/3392 Reviewed-by: Sailesh Mukil <[email protected]> Tested-by: Sailesh Mukil <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/06abb578 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/06abb578 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/06abb578 Branch: refs/heads/master Commit: 06abb57877e1aeab9e9fa54ffe8ae2f360603552 Parents: 38b18ea Author: Sailesh Mukil <[email protected]> Authored: Thu Jun 9 16:10:01 2016 -0700 Committer: Tim Armstrong <[email protected]> Committed: Tue Jun 21 17:38:12 2016 -0700 ---------------------------------------------------------------------- be/src/runtime/hdfs-fs-cache.cc | 6 +++++ tests/custom_cluster/test_s3a_access.py | 35 +++++++++++++++++----------- 2 files changed, 27 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/06abb578/be/src/runtime/hdfs-fs-cache.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/hdfs-fs-cache.cc b/be/src/runtime/hdfs-fs-cache.cc index 96ce723..59b74a1 100644 --- a/be/src/runtime/hdfs-fs-cache.cc +++ b/be/src/runtime/hdfs-fs-cache.cc @@ -85,6 +85,12 @@ Status HdfsFsCache::GetConnection(const string& path, hdfsFS* fs, hdfsBuilder* hdfs_builder = hdfsNewBuilder(); hdfsBuilderSetNameNode(hdfs_builder, namenode.c_str()); if (!s3a_access_key_.empty()) { + // Use a new instance of the filesystem object to be sure that it picks up the + // configuration changes we're going to make. Without this call, a cached + // filesystem object is used which is unaffected by calls to + // hdfsBuilderConfSetStr(). This is unexpected behavior in the HDFS API, but is + // unlikely to change. + hdfsBuilderSetForceNewInstance(hdfs_builder); hdfsBuilderConfSetStr(hdfs_builder, "fs.s3a.access.key", s3a_access_key_.c_str()); hdfsBuilderConfSetStr(hdfs_builder, "fs.s3a.secret.key", s3a_secret_key_.c_str()); } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/06abb578/tests/custom_cluster/test_s3a_access.py ---------------------------------------------------------------------- diff --git a/tests/custom_cluster/test_s3a_access.py b/tests/custom_cluster/test_s3a_access.py index be27443..6877811 100644 --- a/tests/custom_cluster/test_s3a_access.py +++ b/tests/custom_cluster/test_s3a_access.py @@ -43,14 +43,6 @@ class TestS3AAccess(CustomClusterTestSuite): def teardown_class(cls): os.remove(BAD_KEY_FILE) - def teardown_method(self, method): - self._drop_test_tbl() - - def _drop_test_tbl(self): - client = self._get_impala_client() - self.execute_query_expect_success(client, - "DROP TABLE IF EXISTS tinytable_s3") - def _get_impala_client(self): impalad = self.cluster.get_any_impalad() return impalad.service.create_beeswax_client() @@ -59,14 +51,29 @@ class TestS3AAccess(CustomClusterTestSuite): @CustomClusterTestSuite.with_args( "-s3a_access_key_cmd=\"%s\"\ -s3a_secret_key_cmd=\"%s\"" % (BAD_KEY_FILE, BAD_KEY_FILE)) - def test_keys_do_not_work(self): + def test_ddl_keys_ignored(self, unique_database): + '''DDL statements will ignore the S3 keys passed to Impala because the code path + that it exercises goes through the Hive Metastore which should have the correct keys + from the core-site configuration.''' + client = self._get_impala_client() + # This is repeated in the test below (because it's necessary there), but we still + # want to make sure that it is tested separately as it's a good test practice. + self.execute_query_expect_success(client, + "create external table if not exists {0}.tinytable_s3 like functional.tinytable \ + location '{1}/tinytable'".format(unique_database, WAREHOUSE)) + + @pytest.mark.execute_serially + @CustomClusterTestSuite.with_args( + "-s3a_access_key_cmd=\"%s\"\ + -s3a_secret_key_cmd=\"%s\"" % (BAD_KEY_FILE, BAD_KEY_FILE)) + def test_keys_do_not_work(self, unique_database): '''Test that using incorrect S3 access and secret keys will not allow Impala to - query S3. CREATE statements will work because that goes through the Hive Metastore - which should have the correct keys. + query S3. TODO: We don't have the test infrastructure in place yet to check if the keys do work in a custom cluster test. (See IMPALA-3422)''' client = self._get_impala_client() self.execute_query_expect_success(client, - "create external table if not exists tinytable_s3 like functional.tinytable \ - location '{0}/tinytable'".format(WAREHOUSE)) - self.execute_query_expect_failure(client, "select * from tinytable_s3") + "create external table if not exists {0}.tinytable_s3 like functional.tinytable \ + location '{1}/tinytable'".format(unique_database, WAREHOUSE)) + self.execute_query_expect_failure(client, "select * from {0}.tinytable_s3" + .format(unique_database))
