This is an automated email from the ASF dual-hosted git repository. guangning pushed a commit to branch branch-2.5 in repository https://gitbox.apache.org/repos/asf/pulsar.git
commit 28cb139222a61f85cf649aee3e92c698e97ec953 Author: Masahiro Sakamoto <[email protected]> AuthorDate: Thu Feb 13 02:00:21 2020 +0900 Fix bug that tenants whose allowed clusters include global cannot be created/updated (#6275) --- .../main/java/org/apache/pulsar/broker/admin/AdminResource.java | 5 ++--- .../java/org/apache/pulsar/broker/admin/impl/ClustersBase.java | 5 ++--- .../main/java/org/apache/pulsar/broker/admin/impl/TenantsBase.java | 6 +++++- .../test/java/org/apache/pulsar/broker/admin/AdminApiTest2.java | 7 ++++++- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java index b6330f6..2aa5ae7 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java @@ -522,10 +522,9 @@ public abstract class AdminResource extends PulsarWebResource { protected Set<String> clusters() { try { - Set<String> clusters = pulsar().getConfigurationCache().clustersListCache().get(); - // Remove "global" cluster from returned list - clusters.remove(Constants.GLOBAL_CLUSTER); + Set<String> clusters = pulsar().getConfigurationCache().clustersListCache().get().stream() + .filter(cluster -> !Constants.GLOBAL_CLUSTER.equals(cluster)).collect(Collectors.toSet()); return clusters; } catch (Exception e) { throw new RestException(e); diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ClustersBase.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ClustersBase.java index 417a68b..e6d2b6b 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ClustersBase.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ClustersBase.java @@ -84,10 +84,9 @@ public class ClustersBase extends AdminResource { }) public Set<String> getClusters() throws Exception { try { - Set<String> clusters = clustersListCache().get(); - // Remove "global" cluster from returned list - clusters.remove(Constants.GLOBAL_CLUSTER); + Set<String> clusters = clustersListCache().get().stream() + .filter(cluster -> !Constants.GLOBAL_CLUSTER.equals(cluster)).collect(Collectors.toSet()); return clusters; } catch (Exception e) { log.error("[{}] Failed to get clusters list", clientAppId(), e); diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/TenantsBase.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/TenantsBase.java index 223b06a..a6e1d35 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/TenantsBase.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/TenantsBase.java @@ -33,6 +33,7 @@ import javax.ws.rs.core.Response.Status; import org.apache.pulsar.broker.admin.AdminResource; import org.apache.pulsar.broker.web.RestException; +import org.apache.pulsar.common.naming.Constants; import org.apache.pulsar.common.naming.NamedEntity; import org.apache.pulsar.common.policies.data.TenantInfo; import org.apache.zookeeper.KeeperException; @@ -143,6 +144,9 @@ public class TenantsBase extends AdminResource { oldTenantAdmin.getAllowedClusters().removeAll(newTenantAdmin.getAllowedClusters()); log.debug("Following clusters are being removed : [{}]", oldTenantAdmin.getAllowedClusters()); for (String cluster : oldTenantAdmin.getAllowedClusters()) { + if (Constants.GLOBAL_CLUSTER.equals(cluster)) { + continue; + } List<String> activeNamespaces = Lists.newArrayList(); try { activeNamespaces = globalZk().getChildren(path(POLICIES, tenant, cluster), false); @@ -229,7 +233,7 @@ public class TenantsBase extends AdminResource { Set<String> availableClusters = clustersListCache().get(); Set<String> allowedClusters = info.getAllowedClusters(); nonexistentClusters = allowedClusters.stream() - .filter(cluster -> !availableClusters.contains(cluster)) + .filter(cluster -> !(availableClusters.contains(cluster) || Constants.GLOBAL_CLUSTER.equals(cluster))) .collect(Collectors.toList()); } catch (Exception e) { log.error("[{}] Failed to get available clusters", clientAppId(), e); diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTest2.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTest2.java index 4a54e2e..156ddf5 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTest2.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTest2.java @@ -622,9 +622,14 @@ public class AdminApiTest2 extends MockedPulsarServiceBaseTest { new ClusterData("http://broker.messaging.east2.example.com:8080")); admin.clusters().createCluster("global", new ClusterData()); + List<String> allClusters = admin.clusters().getClusters(); + Collections.sort(allClusters); + assertEquals(allClusters, + Lists.newArrayList("test", "us-east1", "us-east2", "us-west1", "us-west2", "us-west3", "us-west4")); + final String property = "peer-prop"; Set<String> allowedClusters = Sets.newHashSet("us-west1", "us-west2", "us-west3", "us-west4", "us-east1", - "us-east2"); + "us-east2", "global"); TenantInfo propConfig = new TenantInfo(Sets.newHashSet("test"), allowedClusters); admin.tenants().createTenant(property, propConfig);
