[PR] [ISSUE #11385] fix(tools): read -a/--updateAllBroker instead of treating it as a switch [rocketmq]

["iloveeyjafjalla (via GitHub)" <gi...@apache.org>]

iloveeyjafjalla opened a new pull request, #11386:
URL: https://github.com/apache/rocketmq/pull/11386

   ### Which Issue(s) This PR Fixes
   
   - Fixes #11385
   
   ### Brief Description
   
   `mqadmin updateBrokerConfig -a false` updated the slaves anyway: the option is declared with the boolean as its argument
   (`new Option("a", "updateAllBroker", true, ...)`, help shows `-a,--updateAllBroker <arg>`), but the command only tested
   `commandLine.hasOption('a')`, so the value was never read and any presence of the flag selected the master+slave fan-out:
   
   ```java
   if (commandLine.hasOption('a')) {
       brokerAddrSet = CommandUtil.fetchMasterAndSlaveAddrByClusterName(defaultMQAdminExt, clusterName);
   ```
   
   The change reads the argument (`Boolean.parseBoolean(commandLine.getOptionValue('a').trim())`), so `-a false` — and a missing
   `-a` — update the masters only, while `-a true` keeps the documented behaviour. No documentation, script or other caller in
   the repository passes a bare `-a`, so the strict `true|false` argument stays exactly as the help describes it.
   
   ### How Did You Test This Change?
   
   Extended `UpdateBrokerConfigSubCommandTest` with a topology the command's cluster mode actually walks: a name server that
   reports one broker with a master and a slave, plus one counting mock broker per node (the test counts the requests each mock
   received, since the assertions are about *which* brokers are updated).
   
   - `testUpdateAllBrokerFalseSkipsSlaves`: `-c default-cluster -a false` → the master receives exactly one
     `UPDATE_BROKER_CONFIG`, the slave receives none.
   - `testUpdateAllBrokerTrueUpdatesSlaves`: `-a true` → both receive one (guards the documented behaviour).
   
   Results on Temurin JDK 8 (same JDK as CI), base `9ea2ccdc2` plus this patch:
   
   - before the fix: `java.lang.AssertionError: slave update count expected:<0> but was:<1>`
   - after the fix: `UpdateBrokerConfigSubCommandTest` — 3 tests, 0 failures
   - `mvn -B -ntp -pl tools package` with checkstyle + spotbugs enabled: `BUILD SUCCESS`, spotbugs `BugInstance size is 0`
   
   This change was prepared with AI assistance (analysis, patch and test drafting); the diff and every result above were
   reproduced locally by running the module's tests against the base commit and the patched tree.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org