fix: validation fails causing etcd events not to be handled correctly #11268
Conversation
bzp2010
changed the title
| ## } | ||
| ## } | ||
| ## After check, it only appears when watch recovers and returns events in bulk. | ||
| cat logs/error.log | grep "}, {" || (echo "failed: Log case 2 unexpected"; exit 1) |
There was a problem hiding this comment.
assert failed to check item data in error log too.
There was a problem hiding this comment.
test needs to fail if expectations are not met || gives a exit code 0 which is not detected as a failure by the cli testing suite.
EDIT: my bad, didn't notice the exit 1
There was a problem hiding this comment.
@shreemaan-abhishek Actually, the error doesn't cause the test suite to exit either, I'm not sure what's happening. But I've checked the log manually and it has no errors.
I.e. even if it exits with exit 1, the CLI test doesn't fail, which is too strange and should be checked and fixed. But not in this PR.
There was a problem hiding this comment.
@nic-6443 Good catch! Updated.
t/cli/test_etcd_sync_event_handle.sh
Outdated
| cat logs/error.log | grep "}, {" || (echo "failed: Log case 2 unexpected"; exit 1) | ||
|
|
||
| # Clean up | ||
| make stop |
There was a problem hiding this comment.
this last make stop is not needed, it is already run by default. Running twice give this error that pollutes the test output log.
There was a problem hiding this comment.
@shreemaan-abhishek Fixed, that because I add the first line . ./ci/common.sh finally.
bzp2010
requested review from
nic-6443,
shreemaan-abhishek,
membphis,
starsz and
moonming
Description
Fixes #11267
Ensure that all etcd events are handled, rather than jumping out of the execution flow when a checking error is encountered.
Current wrong behavior will cause changes in etcd to no longer take effect on the started data plane.
The issue has existed since at least APISIX 2.0, which was long, 4-5 years ago.
Checklist