fix: prevent DescribeLogDirs hang in admin client

[zerowidth]

This fixes a goroutine/waitgroup bug in the admin client. If an unknown broker ID is passed to DescribeLogDirs, it will hang forever.

It's a bug with waitgroups, where a waitgroup can be incremented without an associated decrement leading to an infinite wait. I think it's simple enough that it can be fixed with just visual inspection, but I included a test case just to be sure.

It's tricky to get a codebase into this scenario, but has occurred in our systems a few times. In summary, two goroutines using a shared sarama.Client:

• One goroutine does various things to get a list of (then valid) broker IDs
• Another goroutine runs a PartitionConsumer
• A broker goes offline
• The consumer goroutine, as part of its PartitionConsumer error recovery, eventually calls RefreshMetadata, updating the internal brokers list in the Client to remove the broker that's offline.
• The first goroutine calls DescribeLogDirs with the original broker list, which still includes the dead broker
• DescribeLogDirs
  • increments a waitgroup
  • calls findBroker, which doesn't find the dead broker in client.Brokers()
  • logs the error then continues the loop without firing off a goroutine with a wg.Done() to decrement
  • the final wg.Wait() hangs.

This was noted in the original PR introducing this admin API call (#1646 (comment)), but the bug occurs before any network IO is involved.

[dnwe]

@zerowidth 👋🏻 thanks for your contribution and the detailed description. Your changes look good — are you in a position to sign the Shopify CLA?

[zerowidth]

are you in a position to sign the Shopify CLA?

I’ve signed the CLA but wasn’t able to rerun the probot check. Once that’s done we should be good to go. Thanks!

[dnwe]

LGTM, thanks for your contribution!