-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cluster manager: avoid immediate activation for dynamic inserted cluster when initialize #12783
Conversation
…ter when initialize Signed-off-by: Shikugawa <[email protected]>
@lizan for first pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a integration test to verify this fixes the issue described in #11120 ?
This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
/nostale |
Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Shikugawa <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM, can you merge master? @mattklein123 for non-Tetrate pass.
Please merge main and I will take a look. /wait |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Signed-off-by: Shikugawa <[email protected]>
Merge main once #13598 merges. Thanks! /wait |
@mattklein123 FYI a |
/retest |
Retrying Azure Pipelines, to retry CircleCI checks, use |
* master: (22 commits) delay health checks until transport socket secrets are ready. (envoyproxy#13516) test, oauth2: Make sure config test runs field validation (envoyproxy#13496) [http] swap codec implementations to default new (envoyproxy#13579) wasm: update proxy-wasm-cpp-host (envoyproxy#13606) postgres: do not copy and linearize received data when it is not going to be used (envoyproxy#13393) configs: Update configs v2 -> v3 (envoyproxy#13562) http2: Remove RELEASE_ASSERTs in sendPendingFrames() error handling (envoyproxy#13546) dependencies: track untracked implied dependencies, wrapup dashboard. (envoyproxy#13571) listener: add match all filter chain (envoyproxy#13449) fix mistakes in docstrings (envoyproxy#13603) ratelimit: add route entry metadata to ratelimit actions (envoyproxy#13269) cluster manager: avoid immediate activation for dynamic inserted cluster when initialize (envoyproxy#12783) ext_authz: Avoid calling check multiple times (envoyproxy#13288) docs: Unexclude remaining configs from validation (envoyproxy#13534) build: update rules_rust to allow Rustc in RBE (envoyproxy#13595) docs: Update sphinxext.rediraffe (envoyproxy#13589) Deprecate moonjit support on Windows before beta (envoyproxy#13541) dependencies: bump LuaJIT to 2.1 branch HEAD @ e9af1ab. (envoyproxy#13474) docs: add TLS stats to cluster stats doc (envoyproxy#13561) ci: stop building alpine-debug images in favor of ubuntu-based debug image (envoyproxy#13598) ... Signed-off-by: Michael Puncel <[email protected]>
FYI seeing some envoys stuck in |
this is when doing a hot restart |
Thanks @rgs1, it wouldn't surprise me if there was some bugs here. This code is complicated. If we need to revert we can. |
I'll do a bisect in a bit and confirm the offending commit. |
…cret entity (#13344) This PR highly depends on #12783. Changed to keep warming if dynamic inserted clusters (when initialize doesn't finished) failed to extract TLS certificate and certificate validation context. They shouldn't be indicated as ACTIVE cluster. Risk Level: Mid Testing: Unit Docs Changes: Release Notes: Added Fixes #11120, future work: #13777 Signed-off-by: Shikugawa <[email protected]>
…ter when initialize (envoyproxy#12783) Signed-off-by: Shikugawa <[email protected]>
…cret entity (envoyproxy#13344) This PR highly depends on envoyproxy#12783. Changed to keep warming if dynamic inserted clusters (when initialize doesn't finished) failed to extract TLS certificate and certificate validation context. They shouldn't be indicated as ACTIVE cluster. Risk Level: Mid Testing: Unit Docs Changes: Release Notes: Added Fixes envoyproxy#11120, future work: envoyproxy#13777 Signed-off-by: Shikugawa <[email protected]>
* cluster manager: avoid immediate activation for dynamic inserted cluster when initialize (envoyproxy#12783) Signed-off-by: Shikugawa <[email protected]> * sds: keep warming when dynamic inserted cluster can't be extracted secret entity (envoyproxy#13344) This PR highly depends on envoyproxy#12783. Changed to keep warming if dynamic inserted clusters (when initialize doesn't finished) failed to extract TLS certificate and certificate validation context. They shouldn't be indicated as ACTIVE cluster. Risk Level: Mid Testing: Unit Docs Changes: Release Notes: Added Fixes envoyproxy#11120, future work: envoyproxy#13777 Signed-off-by: Shikugawa <[email protected]> Co-authored-by: Rei Shimizu <[email protected]>
…ter when initialize (envoyproxy#12783) Signed-off-by: Shikugawa <[email protected]> Signed-off-by: Yuchen Dai <[email protected]>
* cluster manager: avoid immediate activation for dynamic inserted cluster when initialize (envoyproxy#12783) Signed-off-by: Shikugawa <[email protected]> Signed-off-by: Yuchen Dai <[email protected]> * cluster: unstuck cluster manager when update the initializing cluster (envoyproxy#13875) Signed-off-by: Yuchen Dai <[email protected]> Co-authored-by: Rei Shimizu <[email protected]>
Commit Message: To resolve #12670. This commit changes the cluster state change. In previous implementation, dynamically inserted cluster when initialization should be active immediately. This patch changes, from active immediately to warming -> active when initialization.
Additional Description:
Risk Level: Mid (core behavior change)
Testing: Unit
Docs Changes: Needed
Release Notes: Needed
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]