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

[Issue 9790] Support disabled the tenants/namespaces force deletion in the broker.conf #9819

Merged
merged 4 commits into from
Mar 10, 2021

Conversation

murong00
Copy link
Contributor

@murong00 murong00 commented Mar 5, 2021

Motivation

Fixes #9790

Modifications

  • Add new configs below and keep the default value to false:
forceDeleteTenantAllowed=false
forceDeleteNamespaceAllowed=false
  • Add unit tests for this change.

@murong00 murong00 self-assigned this Mar 5, 2021
@@ -290,6 +290,12 @@ protected void internalDeleteTenant(AsyncResponse asyncResponse, String tenant)
}

protected void internalDeleteTenantForcefully(AsyncResponse asyncResponse, String tenant) {
if (!pulsar().getConfiguration().isForceDeleteTenantAllowed()) {
asyncResponse.resume(
new RestException(Status.FORBIDDEN, "Broker doesn't allow forced deletion of tenants"));
Copy link
Contributor

Choose a reason for hiding this comment

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

The @ApiReponse use 403 to indicate if the request has admin permission, I think 405 is more reasonable here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the tips, done.

@murong00
Copy link
Contributor Author

murong00 commented Mar 8, 2021

/pulsarbot run-failure-checks

@sijie sijie added doc-required Your PR changes impact docs and you will update later. type/feature The PR added a new feature or issue requested a new feature labels Mar 8, 2021
@sijie sijie added this to the 2.8.0 milestone Mar 8, 2021
@Anonymitaet
Copy link
Member

@murong00 thanks for your great work! Shall the changes be documented to the broker -configuration section?
If so, could you please help add the docs accordingly? Thanks

@murong00
Copy link
Contributor Author

murong00 commented Mar 8, 2021

@Anonymitaet Added the docs accordingly, please help to take a look when you are available.

@@ -185,6 +185,8 @@ internalListenerName|Specify the internal listener name for the broker.<br><br>*
|brokerDeleteInactiveTopicsFrequencySeconds| How often to check for inactive topics |60|
| brokerDeleteInactiveTopicsMode | Set the mode to delete inactive topics. <li> `delete_when_no_subscriptions`: delete the topic which has no subscriptions or active producers. <li> `delete_when_subscriptions_caught_up`: delete the topic whose subscriptions have no backlogs and which has no active producers or consumers. | `delete_when_no_subscriptions` |
| brokerDeleteInactiveTopicsMaxInactiveDurationSeconds | Set the maximum duration for inactive topics. If it is not specified, the `brokerDeleteInactiveTopicsFrequencySeconds` parameter is adopted. | N/A |
|forceDeleteTenantAllowed| Allow forced deletion of tenants.|false|
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
|forceDeleteTenantAllowed| Allow forced deletion of tenants.|false|
|forceDeleteTenantAllowed| Allow you to delete a tenant forcefully. |false|

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

conf/broker.conf Outdated
@@ -165,6 +165,12 @@ brokerDeleteInactivePartitionedTopicMetadataEnabled=false
# Topics that are inactive for longer than this value will be deleted
brokerDeleteInactiveTopicsMaxInactiveDurationSeconds=

# Allow forced deletion of tenants.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Allow forced deletion of tenants.
# Allow you to delete a tenant forcefully.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

conf/broker.conf Outdated
# Allow forced deletion of tenants.
forceDeleteTenantAllowed=false

# Allow forced deletion of namespaces.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Allow forced deletion of namespaces.
# Allow you to delete a namespace forcefully.

@@ -185,6 +185,8 @@ internalListenerName|Specify the internal listener name for the broker.<br><br>*
|brokerDeleteInactiveTopicsFrequencySeconds| How often to check for inactive topics |60|
| brokerDeleteInactiveTopicsMode | Set the mode to delete inactive topics. <li> `delete_when_no_subscriptions`: delete the topic which has no subscriptions or active producers. <li> `delete_when_subscriptions_caught_up`: delete the topic whose subscriptions have no backlogs and which has no active producers or consumers. | `delete_when_no_subscriptions` |
| brokerDeleteInactiveTopicsMaxInactiveDurationSeconds | Set the maximum duration for inactive topics. If it is not specified, the `brokerDeleteInactiveTopicsFrequencySeconds` parameter is adopted. | N/A |
|forceDeleteTenantAllowed| Allow forced deletion of tenants.|false|
|forceDeleteNamespaceAllowed| Allow forced deletion of namespaces.|false|
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
|forceDeleteNamespaceAllowed| Allow forced deletion of namespaces.|false|
|forceDeleteNamespaceAllowed| Allow you to delete a namespace forcefully. |false|

@Anonymitaet Anonymitaet added doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. and removed doc-required Your PR changes impact docs and you will update later. labels Mar 9, 2021
@murong00
Copy link
Contributor Author

murong00 commented Mar 9, 2021

/pulsarbot run-failure-checks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. type/feature The PR added a new feature or issue requested a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support disabled the tenants/namespaces force deletion in the broker.conf
4 participants