Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Branch-2.8] Fix validateGlobalNamespaceOwnership wrap exception issue. #14612

Merged
merged 2 commits into from
Mar 9, 2022

Conversation

Technoboy-
Copy link
Contributor

Cherry-pick #14269

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

@Technoboy- Technoboy- self-assigned this Mar 9, 2022
@codelipenghui codelipenghui merged commit ced5779 into apache:branch-2.8 Mar 9, 2022
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Mar 9, 2022
@Technoboy- Technoboy- deleted the cherry-pick-14269 branch August 10, 2022 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-2.8 Archived: 2.8 is end of life release/2.8.4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants