envoyproxy / gateway

Manages Envoy Proxy as a Standalone or Kubernetes-based Application Gateway
https://gateway.envoyproxy.io
Apache License 2.0
1.55k stars 333 forks source link

fix: some status updates are discarded by the status updater #4337

Closed zhaohuabing closed 1 day ago

zhaohuabing commented 2 days ago

Fix https://github.com/envoyproxy/gateway/issues/4336

codecov[bot] commented 2 days ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 65.96%. Comparing base (6eefb28) to head (1f79cc6). Report is 2 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #4337 +/- ## ========================================== - Coverage 65.98% 65.96% -0.02% ========================================== Files 197 197 Lines 23964 23958 -6 ========================================== - Hits 15813 15805 -8 - Misses 7025 7026 +1 - Partials 1126 1127 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

arkodg commented 1 day ago

thanks ! do we also need to change the order of status updater and k8s controller ?

zhaohuabing commented 1 day ago

thanks ! do we also need to change the order of status updater and k8s controller ?

I don't know if it's possible: status updater is added as a runnable in the k8s controller and start within it because status updater also needs leader election.

The current solution is removing the check for start and receive all the updates even it's not started yet. It's not very elegant but should solve this issue.

zhaohuabing commented 1 day ago

Should we cherry-pick this to v1.1 branch @guydc @arkodg ?

arkodg commented 1 day ago

Should we cherry-pick this to v1.1 branch @guydc @arkodg ?

Sure let's add a label and make sure to add it to v1.1 4