Skip to content

[fix][admin] Fix delete tenant#19925

Merged
tisonkun merged 2 commits into
apache:masterfrom
nodece:fix-delete-tenant
Mar 27, 2023
Merged

[fix][admin] Fix delete tenant#19925
tisonkun merged 2 commits into
apache:masterfrom
nodece:fix-delete-tenant

Conversation

@nodece

@nodece nodece commented Mar 25, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Fixes #19921

Motivation

Failed to delete tenant, see log:

2023-03-25T18:40:15,174+0800 [pulsar-web-48-12] ERROR org.apache.pulsar.broker.admin.impl.TenantsBase - [null] Failed to delete tenant newtenant
org.apache.pulsar.metadata.api.MetadataStoreException$NotFoundException: path /admin/local-policies/newtenant not found.
        at org.apache.pulsar.metadata.impl.RocksdbMetadataStore.storeDelete(RocksdbMetadataStore.java:480) ~[pulsar-metadata.jar:3.0.0-SNAPSHOT]
        at org.apache.pulsar.metadata.impl.AbstractMetadataStore.deleteInternal(AbstractMetadataStore.java:386) ~[pulsar-metadata.jar:3.0.0-SNAPSHOT]
        at org.apache.pulsar.metadata.impl.AbstractMetadataStore.delete(AbstractMetadataStore.java:373) ~[pulsar-metadata.jar:3.0.0-SNAPSHOT]
        at org.apache.pulsar.metadata.cache.impl.MetadataCacheImpl.delete(MetadataCacheImpl.java:248) ~[pulsar-metadata.jar:3.0.0-SNAPSHOT]
        at org.apache.pulsar.broker.resources.BaseResources.deleteAsync(BaseResources.java:160) ~[pulsar-broker-common.jar:3.0.0-SNAPSHOT]
        at org.apache.pulsar.broker.resources.LocalPoliciesResources.deleteLocalPoliciesTenantAsync(LocalPoliciesResources.java:88) ~[pulsar-broker-common.jar:3.0.0-SNAPSHOT]
        at org.apache.pulsar.broker.admin.impl.TenantsBase.lambda$internalDeleteTenantAsync$33(TenantsBase.java:238) ~[pulsar-broker.jar:3.0.0-SNAPSHOT]

When deleting a tenant, we delete a path that does not exist, so throw an exception.

Modifications

  • Check whether the /admin/local-policies/{tenant} exists, and then delete it

Verifying this change

  • Make sure that the change passes the CI checks.
    Added AdminApiTenantTest.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@nodece
[fix][admin] Fix delete tenant
63a0b8e 
Signed-off-by: nodece <nodeces@gmail.com>
@nodece nodece requested a review from tisonkun March 25, 2023 11:51
@nodece nodece self-assigned this Mar 25, 2023
@nodece nodece added type/bug The PR fixed a bug or issue reported a bug area/admin labels Mar 25, 2023
@nodece nodece added this to the 3.0.0 milestone Mar 25, 2023
@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Mar 25, 2023
@nodece

nodece commented Mar 25, 2023

Copy link
Copy Markdown
Member Author

/pulsarbot rerun-failure-checks

@nodece nodece changed the title [fix][admin]: Fix delete tenant [fix][admin] Fix delete tenant Mar 25, 2023
@nodece nodece mentioned this pull request Mar 25, 2023
Closed
2 tasks
tisonkun
tisonkun approved these changes Mar 25, 2023
View reviewed changes

@tisonkun tisonkun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locally verified. LGTM.

Comments inline.

Comment thread pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTenantTest.java Outdated
import java.util.List;
import java.util.UUID;

import static org.testng.Assert.*;

@tisonkun tisonkun Mar 25, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expand star import and sort by:

pulsar/src/idea-code-style.xml

Lines 30 to 31 in 329e80b

<package name="" withSubpackages="true" static="true" />
<package name="" withSubpackages="true" static="false" />

?

@nodece nodece Mar 26, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I should use the static import.

Comment thread pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTenantTest.java
@nodece
Fix import
aafd7e9 
Signed-off-by: nodece <nodeces@gmail.com>
@nodece

nodece commented Mar 26, 2023

Copy link
Copy Markdown
Member Author

/pulsarbot rerun-failure-checks

@tisonkun

tisonkun commented Mar 27, 2023

Copy link
Copy Markdown
Member

Merging...

@nodece Thanks for preparing this patch :)

@tisonkun tisonkun merged commit 32ad906 into apache:master Mar 27, 2023
nodece added a commit that referenced this pull request Mar 27, 2023
@nodece
[fix][admin] Delete tenant local policy only if exist (#19925)
8d6afba 
Signed-off-by: nodece <nodeces@gmail.com>
(cherry picked from commit 32ad906)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tisonkun tisonkun tisonkun approved these changes

@michaeljmarshall michaeljmarshall Awaiting requested review from michaeljmarshall

Assignees

@nodece nodece

Labels

area/admin doc-not-needed Your PR changes do not impact docs ready-to-test type/bug The PR fixed a bug or issue reported a bug

Projects

None yet

Milestone

3.0.0

Development

Successfully merging this pull request may close these issues.

[Bug] Deleting a tenant results in a HTTP 500 error, but deletes the tenant

2 participants

@nodece @tisonkun