[BUG] plugin/ready: fix Reset list of readiness plugins#8035
Conversation
08cf249 to
6d4614d
Compare
|
I have open the PR with a fix for the readiness plugin list reset issue when using multiple server blocks. Summary of the bug: This PR corrects the reset logic in The change is small and targeted. I have tested it with the multi-block Corefile scenario, and readiness now behaves as expected even after reloads. Could you please take a look and review this PR? I would really appreciate it if you could approve the change so we can get this bug fixed and merged. Thanks! |
thevilledev
left a comment
There was a problem hiding this comment.
The fix itself looks correct. What's missing is a regression test for the multi-server-block scenario it's fixing. There's already a similar e2e test in test/reload_test.go called TestReloadUnreadyPlugin. I think a new test should follow the same pattern but with (at least) two server blocks. Could you add a test?
6d4614d to
2a00cb0
Compare
|
Hi, @thevilledev I've added the corresponding e2e test cases as requested. Could you please take another look and review it again? |
Signed-off-by: 杨军10092085 <yang.jun22@zte.com.cn>
2a00cb0 to
38b8647
Compare
|
I've made the changes as suggested. Would you mind taking another look? @thevilledev |
thevilledev
left a comment
There was a problem hiding this comment.
LGTM, thanks @yangjunmyfm192085!
1. Why is this pull request needed and what does it do?
When the Corefile contains multiple server blocks and each block enables the
readydirective, the readiness check from thekubernetesplugin is overwritten by plugins from subsequent blocks (e.g.erratic). As a result, the/readyendpoint only reports the readiness status of the last plugin in the list.Reproduction Corefile:
In the above configuration, querying the readiness endpoint (/ready) only checks the erratic plugin. The kubernetes plugin's readiness check is completely overwritten.
Root Cause:
The plugin/ready package collects plugins that implement the readiness interface into a list. However, the logic responsible for resetting this list (especially during configuration reload or when initializing multiple server blocks) was incorrect. The reset was not properly performed, causing later plugins to overwrite earlier ones. In the end, only the last registered readiness plugin remained in the list.
Fix:
Correct the reset logic of the readiness plugin list in plugin/ready.
Ensure that the list is properly cleared and repopulated on every reload and during initialization of multiple server blocks.
After this fix, readiness checks from all plugins (including kubernetes and erratic) in different server blocks will be respected and combined correctly.
2. Which issues (if any) are related?
3. Which documentation changes (if any) need to be made?
4. Does this introduce a backward incompatible change or deprecation?