[improve][cli] Add --cleanupSubscription to pulsar-admin

[jiangpengcheng]

Fixes #20027

Master Issue: #xyz

PIP: #xyz

Motivation

pulsar-admin can't set cleanupSubscription for functions/sinks

Modifications

add a --cleanupSubscription parameter

Verifying this change

• Make sure that the change passes the CI checks.

(Please pick either of the following options)

• This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

• Added integration tests for end-to-end deployment with large payloads (10MB)
• Extended integration test for recovery after broker failure

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

• Dependencies (add or upgrade a dependency)
• The public API
• The schema
• The default values of configurations
• The threading model
• The binary protocol
• The REST endpoints
• The admin CLI options
• The metrics
• Anything that affects deployment

Documentation

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

Matching PR in forked repository

PR in forked repository: jiangpengcheng#11

[tisonkun]

Removals on this file seems breaking change?

[jiangpengcheng]

It's duplicated in lines 196~200

other deletions in this file also because of duplicate code

[tisonkun]

Thanks for your explanation! Running tests...

[codecov-commenter]

Codecov Report

Merging #20028 (e35f427) into master (6036dcc) will decrease coverage by 0.03%.
The diff coverage is 53.84%.

@@             Coverage Diff              @@
##             master   #20028      +/-   ##
============================================
- Coverage     72.93%   72.90%   -0.03%     
- Complexity    31934    31943       +9     
============================================
  Files          1868     1868              
  Lines        138449   138457       +8     
  Branches      15235    15235              
============================================
- Hits         100978   100946      -32     
- Misses        29438    29475      +37     
- Partials       8033     8036       +3     

Flag	Coverage Δ
inttests	24.20% <0.00%> (+0.09%)	⬆️
systests	24.75% <0.00%> (-0.18%)	⬇️
unittests	72.21% <53.84%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...java/org/apache/pulsar/admin/cli/CmdFunctions.java	46.71% <0.00%> (-0.25%)	⬇️
...ain/java/org/apache/pulsar/admin/cli/CmdSinks.java	43.18% <0.00%> (-0.29%)	⬇️
...apache/pulsar/functions/utils/SinkConfigUtils.java	72.74% <ø> (-0.39%)	⬇️
...mon/policies/data/stats/SubscriptionStatsImpl.java	69.91% <50.00%> (-0.74%)	⬇️
...pulsar/broker/admin/impl/PersistentTopicsBase.java	64.12% <58.33%> (-0.28%)	⬇️
...ensions/strategy/LeastResourceUsageWithWeight.java	76.66% <100.00%> (ø)

... and 59 files with indirect coverage changes

[tisonkun]

Merging...

Thank you @jiangpengcheng!

[tisonkun]

cc @nlu90 @cbornet for peer reviewing - although this patch changes CLI options, I think it reflects a former decision on adding the option in configs so that we can merge it as is.

[Anonymitaet]

Hi @jiangpengcheng thanks for introducing this great improvement!

We're trying to align docs with code for each release and I have some questions as below. Can you please help answer? TIA!

1. Which version does this improvement go in? Pulsar 3.0 or 3.1?

2. Double-check: is this improvement only added to pulsar-admin (not to REST API and Java admin API)?

[poorbarcode]

@jiangpengcheng @Anonymitaet

If it should be cherry-picked into branch-3.0, please ping me

[Anonymitaet]

Hi @jiangpengcheng thanks for introducing this great improvement!

We're trying to align docs with code for each release and I have some questions as below. Can you please help answer? TIA!

1. Which version does this improvement go in? Pulsar 3.0 or 3.1?
2. Double-check: is this improvement only added to pulsar-admin (not to REST API and Java admin API)?

ping @jiangpengcheng, can you answer the questions?

[Anonymitaet]

@jiangpengcheng for the doc side, the flag has been shown on the Reference site, but language support info is missing.

Can you add that? Thanks!

[Anonymitaet]

For the doc side, we need to add it to:

• Pulsar reference site: this reference is generated from code files automatically, so can you add the language support to

  pulsar/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdFunctions.java

  Line 202 in 1687244

  @Parameter(names = "--cleanup-subscription", description = "Whether delete the subscription "

  ? Thanks!

• Pulsar Feature Matrix (Client + Function): I've added this flag and language support to https://docs.google.com/spreadsheets/d/1YHYTkIXR8-Ql103u-IMI18TXLlGStK8uJjDsOOA0T20/edit#gid=1588411801&range=90:90

[jiangpengcheng]

@Anonymitaet added here: #20836