Skip to content

Conversation

@Jason918
Copy link
Contributor

Cherry pick #14269 to branch 2.7

Motivation

When Rest API call AdminResource#validateGlobalNamespaceOwnership, broker will execute PulsarWebResource#checkLocalOrGetPeerReplicationCluster.
In PulsarWebResource#checkLocalOrGetPeerReplicationCluster:

if (policiesResult.isPresent()) {
Policies policies = policiesResult.get();
if (policies.replication_clusters.isEmpty()) {
String msg = String.format(
"Namespace does not have any clusters configured : local_cluster=%s ns=%s",
localCluster, namespace.toString());
log.warn(msg);
validationFuture.completeExceptionally(new RestException(Status.PRECONDITION_FAILED, msg));
} else if (!policies.replication_clusters.contains(localCluster)) {
ClusterDataImpl ownerPeerCluster = getOwnerFromPeerClusterList(pulsarService,
policies.replication_clusters);
if (ownerPeerCluster != null) {
// found a peer that own this namespace
validationFuture.complete(ownerPeerCluster);
return;
}
String msg = String.format(
"Namespace missing local cluster name in clusters list: local_cluster=%s ns=%s clusters=%s",
localCluster, namespace.toString(), policies.replication_clusters);
log.warn(msg);
validationFuture.completeExceptionally(new RestException(Status.PRECONDITION_FAILED, msg));
} else {
validationFuture.complete(null);
}
} else {
String msg = String.format("Policies not found for %s namespace", namespace.toString());
log.warn(msg);
validationFuture.completeExceptionally(new RestException(Status.NOT_FOUND, msg));
}

Line 780, 794, and 801 has thrown RestException.
But validateGlobalNamespaceOwnership has wrapped the exception :

protected void validateGlobalNamespaceOwnership() {
try {
validateGlobalNamespaceOwnership(this.namespaceName);
} catch (IllegalArgumentException e) {
throw new RestException(Status.PRECONDITION_FAILED, "Tenant name or namespace is not valid");
} catch (RestException re) {
if (re.getResponse().getStatus() == Status.NOT_FOUND.getStatusCode()) {
throw new RestException(Status.NOT_FOUND, "Namespace not found");
}
throw new RestException(Status.PRECONDITION_FAILED, "Namespace does not have any clusters configured");
} catch (Exception e) {
log.warn("Failed to validate global cluster configuration : ns={} emsg={}", namespaceName, e.getMessage());
throw new RestException(Status.SERVICE_UNAVAILABLE, "Failed to validate global cluster configuration");
}
}

This could make the user confused that the log printed is not matched with the REST API.

Modification

  • Add a relative test.

Documentation

  • no-need-doc

@Jason918 Jason918 requested a review from Technoboy- July 28, 2022 11:12
@Jason918 Jason918 self-assigned this Jul 28, 2022
@Technoboy- Technoboy- merged commit c9e3a23 into apache:branch-2.7 Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants