From 757dc37afb0bfc75a99154544dc1d8fdd2563ed0 Mon Sep 17 00:00:00 2001 From: Andrew Purtell Date: Mon, 3 Jun 2019 14:05:14 -0700 Subject: [PATCH] HBASE-22533 TestAccessController3 is flaky (branch-1) Wait for created tables to be online before proceeding in access control tests. Fix a possible NPE in teardown if secure tests did not properly set up. --- .../hbase/security/access/SecureTestUtil.java | 19 ++++++++++++++----- .../access/TestAccessControlFilter.java | 1 + .../security/access/TestAccessController.java | 11 ++++++++++- .../access/TestAccessController2.java | 3 ++- .../access/TestAccessController3.java | 1 + .../TestCellACLWithMultipleVersions.java | 9 ++------- .../access/TestSecureBulkLoadEndpoint.java | 1 + .../security/access/TestTablePermissions.java | 2 ++ 8 files changed, 33 insertions(+), 14 deletions(-) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/SecureTestUtil.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/SecureTestUtil.java index 7a2671776b..92a44c4eec 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/SecureTestUtil.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/SecureTestUtil.java @@ -54,6 +54,7 @@ import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; import org.apache.hadoop.hbase.coprocessor.ObserverContext; import org.apache.hadoop.hbase.coprocessor.MasterCoprocessorEnvironment; import org.apache.hadoop.hbase.io.hfile.HFile; +import org.apache.hadoop.hbase.master.HMaster; import org.apache.hadoop.hbase.protobuf.ProtobufUtil; import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos; import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos.AccessControlService; @@ -768,17 +769,25 @@ public class SecureTestUtil { throws Exception { // NOTE: We need a latch because admin is not sync, // so the postOp coprocessor method may be called after the admin operation returned. - MasterSyncObserver observer = (MasterSyncObserver)testUtil.getHBaseCluster().getMaster() - .getMasterCoprocessorHost().findCoprocessor(MasterSyncObserver.class.getName()); - observer.tableDeletionLatch = new CountDownLatch(1); + MasterSyncObserver observer = null; + HMaster master = testUtil.getHBaseCluster().getMaster(); + if (master != null) { + observer = (MasterSyncObserver)master.getMasterCoprocessorHost() + .findCoprocessor(MasterSyncObserver.class.getName()); + if (observer != null) { + observer.tableDeletionLatch = new CountDownLatch(1); + } + } try { admin.disableTable(tableName); } catch (TableNotEnabledException e) { LOG.debug("Table: " + tableName + " already disabled, so just deleting it."); } admin.deleteTable(tableName); - observer.tableDeletionLatch.await(); - observer.tableDeletionLatch = null; + if (observer != null) { + observer.tableDeletionLatch.await(); + observer.tableDeletionLatch = null; + } } public static String convertToNamespace(String namespace) { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessControlFilter.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessControlFilter.java index f54d42c4a8..0c929184bf 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessControlFilter.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessControlFilter.java @@ -95,6 +95,7 @@ public class TestAccessControlFilter extends SecureTestUtil { @Test (timeout=180000) public void testQualifierAccess() throws Exception { final Table table = createTable(TEST_UTIL, TABLE, new byte[][] { FAMILY }); + TEST_UTIL.waitUntilAllRegionsAssigned(TABLE); try { doQualifierAccess(table); } finally { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java index 3d61bc699d..2b0c0f650b 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java @@ -270,6 +270,7 @@ public class TestAccessController extends SecureTestUtil { htd.addFamily(hcd); htd.setOwner(USER_OWNER); createTable(TEST_UTIL, htd, new byte[][] { Bytes.toBytes("s") }); + TEST_UTIL.waitUntilAllRegionsAssigned(htd.getTableName()); Region region = TEST_UTIL.getHBaseCluster().getRegions(TEST_TABLE).get(0); RegionCoprocessorHost rcpHost = region.getCoprocessorHost(); @@ -1282,6 +1283,7 @@ public class TestAccessController extends SecureTestUtil { htd.addFamily(new HColumnDescriptor(family1)); htd.addFamily(new HColumnDescriptor(family2)); createTable(TEST_UTIL, htd); + TEST_UTIL.waitUntilAllRegionsAssigned(htd.getTableName()); try { // create temp users User tblUser = @@ -1535,7 +1537,7 @@ public class TestAccessController extends SecureTestUtil { htd.addFamily(new HColumnDescriptor(family1)); htd.addFamily(new HColumnDescriptor(family2)); createTable(TEST_UTIL, htd); - + TEST_UTIL.waitUntilAllRegionsAssigned(htd.getTableName()); try { // create temp users User user = User.createUserForTesting(TEST_UTIL.getConfiguration(), "user", new String[0]); @@ -1640,6 +1642,7 @@ public class TestAccessController extends SecureTestUtil { htd.addFamily(new HColumnDescriptor(family2)); htd.setOwner(USER_OWNER); createTable(TEST_UTIL, htd); + TEST_UTIL.waitUntilAllRegionsAssigned(htd.getTableName()); try { List perms; Table acl = systemUserConnection.getTable(AccessControlLists.ACL_TABLE_NAME); @@ -2140,6 +2143,7 @@ public class TestAccessController extends SecureTestUtil { HTableDescriptor htd = new HTableDescriptor(TEST_TABLE2); htd.addFamily(new HColumnDescriptor(TEST_FAMILY)); createTable(TEST_UTIL, htd); + TEST_UTIL.waitUntilAllRegionsAssigned(htd.getTableName()); // Starting a new RegionServer. JVMClusterUtil.RegionServerThread newRsThread = hbaseCluster @@ -2297,6 +2301,7 @@ public class TestAccessController extends SecureTestUtil { htd.addFamily(hcd); htd.setOwner(USER_OWNER); createTable(TEST_UTIL, htd, new byte[][]{Bytes.toBytes("s")}); + TEST_UTIL.waitUntilAllRegionsAssigned(htd.getTableName()); } @Test (timeout=180000) @@ -2848,6 +2853,7 @@ public class TestAccessController extends SecureTestUtil { HTableDescriptor htd = new HTableDescriptor(table1); htd.addFamily(new HColumnDescriptor(family)); createTable(TEST_UTIL, htd); + TEST_UTIL.waitUntilAllRegionsAssigned(htd.getTableName()); // creating the ns and table in it String ns = "testNamespace"; @@ -2857,6 +2863,7 @@ public class TestAccessController extends SecureTestUtil { htd = new HTableDescriptor(table2); htd.addFamily(new HColumnDescriptor(family)); createTable(TEST_UTIL, htd); + TEST_UTIL.waitUntilAllRegionsAssigned(htd.getTableName()); // Verify that we can read sys-tables String aclTableName = AccessControlLists.ACL_TABLE_NAME.getNameAsString(); @@ -2936,6 +2943,8 @@ public class TestAccessController extends SecureTestUtil { TableName tname = TableName.valueOf("revoke"); try { TEST_UTIL.createTable(tname, TEST_FAMILY); + TEST_UTIL.waitUntilAllRegionsAssigned(tname); + User testUserPerms = User.createUserForTesting(conf, "revokePerms", new String[0]); Permission.Action[] actions = { Action.READ, Action.WRITE }; AccessControlClient.grant(TEST_UTIL.getConnection(), tname, testUserPerms.getShortName(), diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController2.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController2.java index ec8bc955f1..1beef164dc 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController2.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController2.java @@ -131,7 +131,7 @@ public class TestAccessController2 extends SecureTestUtil { createNamespace(TEST_UTIL, NamespaceDescriptor.create(namespace).build()); try (Table table = createTable(TEST_UTIL, tableName, new byte[][] { TEST_FAMILY, TEST_FAMILY_2 })) { - TEST_UTIL.waitTableEnabled(tableName); + TEST_UTIL.waitUntilAllRegionsAssigned(tableName); // Ingesting test data. table.put(Arrays.asList(new Put(TEST_ROW).addColumn(TEST_FAMILY, Q1, value1), @@ -504,6 +504,7 @@ public class TestAccessController2 extends SecureTestUtil { HTableDescriptor htd = new HTableDescriptor(table); htd.addFamily(new HColumnDescriptor(family)); createTable(TEST_UTIL, htd); + TEST_UTIL.waitUntilAllRegionsAssigned(htd.getTableName()); // Namespace needs this, as they follow the lazy creation of ACL znode. grantOnNamespace(TEST_UTIL, TESTGROUP1_USER1.getShortName(), ns, Action.ADMIN); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController3.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController3.java index d0038a1d5b..a421891041 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController3.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController3.java @@ -209,6 +209,7 @@ public class TestAccessController3 extends SecureTestUtil { htd.addFamily(hcd); htd.setOwner(USER_OWNER); createTable(TEST_UTIL, htd, new byte[][] { Bytes.toBytes("s") }); + TEST_UTIL.waitUntilAllRegionsAssigned(TEST_TABLE); Region region = TEST_UTIL.getHBaseCluster().getRegions(TEST_TABLE).get(0); RegionCoprocessorHost rcpHost = region.getCoprocessorHost(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLWithMultipleVersions.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLWithMultipleVersions.java index 3be7ef869f..c9a37d0e0b 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLWithMultipleVersions.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLWithMultipleVersions.java @@ -35,7 +35,6 @@ import org.apache.hadoop.hbase.HColumnDescriptor; import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.TableNotFoundException; -import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.ConnectionFactory; import org.apache.hadoop.hbase.client.Delete; @@ -145,12 +144,8 @@ public class TestCellACLWithMultipleVersions extends SecureTestUtil { htd.setOwner(USER_OWNER); htd.addFamily(hcd); // Create the test table (owner added to the _acl_ table) - try (Connection connection = ConnectionFactory.createConnection(TEST_UTIL.getConfiguration())) { - try (Admin admin = connection.getAdmin()) { - admin.createTable(htd, new byte[][] { Bytes.toBytes("s") }); - } - } - TEST_UTIL.waitTableEnabled(TEST_TABLE.getTableName()); + TEST_UTIL.createTable(htd, new byte[][] { Bytes.toBytes("s") }); + TEST_UTIL.waitUntilAllRegionsAssigned(TEST_TABLE.getTableName()); LOG.info("Sleeping a second because of HBASE-12581"); Threads.sleep(1000); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSecureBulkLoadEndpoint.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSecureBulkLoadEndpoint.java index d7a4a85539..4f07c2d92b 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSecureBulkLoadEndpoint.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSecureBulkLoadEndpoint.java @@ -146,6 +146,7 @@ public class TestSecureBulkLoadEndpoint { public void testForRaceCondition() throws Exception { /// create table testUtil.createTable(TABLE,FAMILY,Bytes.toByteArrays(SPLIT_ROWKEY)); + testUtil.waitUntilAllRegionsAssigned(TABLE); Consumer fsCreatedListener = new Consumer() { @Override diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestTablePermissions.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestTablePermissions.java index f8fad9f89d..26f127f9e7 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestTablePermissions.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestTablePermissions.java @@ -109,7 +109,9 @@ public class TestTablePermissions { "TestTablePermissions", ABORTABLE); UTIL.createTable(TEST_TABLE, TEST_FAMILY); + UTIL.waitUntilAllRegionsAssigned(TEST_TABLE); UTIL.createTable(TEST_TABLE2, TEST_FAMILY); + UTIL.waitUntilAllRegionsAssigned(TEST_TABLE2); } @AfterClass -- 2.21.0