cluster manager: avoid immediate activation for dynamic inserted cluster when initialize by Shikugawa · Pull Request #12783 · envoyproxy/envoy

Shikugawa · 2020-08-22T14:34:44Z

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:]

…ter when initialize Signed-off-by: Shikugawa <rei@tetrate.io>

mattklein123 · 2020-08-22T14:39:53Z

@lizan for first pass

lizan

Can you add a integration test to verify this fixes the issue described in #11120 ?

source/common/upstream/cluster_manager_impl.cc

stale · 2020-09-02T08:05:34Z

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!

Shikugawa · 2020-09-02T09:09:57Z

/nostale

Signed-off-by: Shikugawa <rei@tetrate.io>

source/common/upstream/cluster_manager_impl.cc

lizan

This LGTM, can you merge master? @mattklein123 for non-Tetrate pass.

mattklein123 · 2020-10-14T15:43:09Z

Please merge main and I will take a look.

/wait

mattklein123

Thanks!

Signed-off-by: Shikugawa <rei@tetrate.io>

mattklein123 · 2020-10-15T22:36:10Z

Merge main once #13598 merges. Thanks!

/wait

lizan · 2020-10-15T23:15:59Z

@mattklein123 FYI a /retest after #13598 would suffice.

lizan · 2020-10-15T23:41:40Z

/retest

repokitteh-read-only · 2020-10-15T23:41:45Z

Retrying Azure Pipelines, to retry CircleCI checks, use /retest-circle.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #12783 (comment) was created by @lizan.

see: more, trace.

* 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 <mpuncel@squareup.com>

rgs1 · 2020-10-19T19:16:26Z

FYI seeing some envoys stuck in PRE_INITIALIZING after syncing with upstream... so it might be this change or #13516... will know in a bit.

rgs1 · 2020-10-19T19:20:04Z

FYI seeing some envoys stuck in PRE_INITIALIZING after syncing with upstream... so it might be this change or #13516... will know in a bit.

this is when doing a hot restart

mattklein123 · 2020-10-19T19:48:28Z

Thanks @rgs1, it wouldn't surprise me if there was some bugs here. This code is complicated. If we need to revert we can.

rgs1 · 2020-10-19T19:51:00Z

I'll do a bisect in a bit and confirm the offending commit.

rgs1 · 2020-10-19T23:20:23Z

Ok this is change is clean, the issue is triggered by #13516 mixed with hot restarts.

cc: @mpuncel

…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 <rei@tetrate.io>

…ter when initialize (envoyproxy#12783) Signed-off-by: Shikugawa <rei@tetrate.io>

…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 <rei@tetrate.io>

* cluster manager: avoid immediate activation for dynamic inserted cluster when initialize (envoyproxy#12783) Signed-off-by: Shikugawa <rei@tetrate.io> * 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 <rei@tetrate.io> Co-authored-by: Rei Shimizu <rei@tetrate.io>

…ter when initialize (envoyproxy#12783) Signed-off-by: Shikugawa <rei@tetrate.io> Signed-off-by: Yuchen Dai <silentdai@gmail.com>

* cluster manager: avoid immediate activation for dynamic inserted cluster when initialize (envoyproxy#12783) Signed-off-by: Shikugawa <rei@tetrate.io> Signed-off-by: Yuchen Dai <silentdai@gmail.com> * cluster: unstuck cluster manager when update the initializing cluster (envoyproxy#13875) Signed-off-by: Yuchen Dai <silentdai@gmail.com> Co-authored-by: Rei Shimizu <rei@tetrate.io>

cluster manager: avoid immediate activation for dynamic inserted clus…

b882bde

…ter when initialize Signed-off-by: Shikugawa <rei@tetrate.io>

mattklein123 assigned lizan and mattklein123 Aug 22, 2020

lizan reviewed Aug 22, 2020

View reviewed changes

source/common/upstream/cluster_manager_impl.cc Show resolved Hide resolved

mattklein123 added the waiting label Aug 24, 2020

stale bot added the stale stalebot believes this issue/PR has not been touched recently label Sep 2, 2020

stale bot removed the stale stalebot believes this issue/PR has not been touched recently label Sep 2, 2020

lizan marked this pull request as draft September 7, 2020 02:38

update count

7e9332d

Signed-off-by: Shikugawa <rei@tetrate.io>

repokitteh-read-only bot removed the waiting label Sep 13, 2020

Shikugawa marked this pull request as ready for review September 13, 2020 12:01

Shikugawa mentioned this pull request Sep 15, 2020

sds: cluster not warming while certificates are being fetched; immediately marked active #11120

Closed

mattklein123 added the waiting:any label Sep 16, 2020

repokitteh-read-only bot removed the waiting:any label Sep 23, 2020

lizan added the waiting:any label Sep 23, 2020

cleanup

8e99e81

Signed-off-by: Shikugawa <rei@tetrate.io>

repokitteh-read-only bot removed the waiting:any label Sep 30, 2020

Merge branch 'master' into cds-warming

06aa174

Shikugawa mentioned this pull request Oct 1, 2020

sds: keep warming when dynamic inserted cluster can't be extracted secret entity #13344

Merged

mattklein123 added the waiting:any label Oct 1, 2020

JimmyCYJ reviewed Oct 6, 2020

View reviewed changes

source/common/upstream/cluster_manager_impl.cc Show resolved Hide resolved

repokitteh-read-only bot removed the waiting:any label Oct 6, 2020

mattklein123 added the waiting label Oct 7, 2020

lizan previously approved these changes Oct 14, 2020

View reviewed changes

mattklein123 removed the waiting label Oct 14, 2020

repokitteh-read-only bot added the waiting label Oct 14, 2020

repokitteh-read-only bot removed the waiting label Oct 15, 2020

mattklein123 approved these changes Oct 15, 2020

View reviewed changes

Kick CI

89ef612

Signed-off-by: Shikugawa <rei@tetrate.io>

repokitteh-read-only bot added the waiting label Oct 15, 2020

lizan removed the waiting label Oct 15, 2020

mattklein123 merged commit 0d15fee into envoyproxy:master Oct 16, 2020

lizan pushed a commit to lizan/envoy that referenced this pull request Oct 30, 2020

cluster manager: avoid immediate activation for dynamic inserted clus…

a1736e4

…ter when initialize (envoyproxy#12783) Signed-off-by: Shikugawa <rei@tetrate.io>

lizan mentioned this pull request Oct 30, 2020

Sds backport istio/envoy#281

Merged

howardjohn mentioned this pull request Nov 3, 2020

Cluster initialization issue. Envoy start could be blocked forever #13874

Closed

lambdai mentioned this pull request Nov 4, 2020

cluster: unstuck cluster manager when update the initializing cluster #13875

Merged

Shikugawa mentioned this pull request Nov 4, 2020

sds: keep warming when dynamic inserted cluster can't be extracted secret entity #13894

Closed

lambdai mentioned this pull request Nov 10, 2020

[1.8] fix cluster early warm up istio/envoy#285

Merged

Shikugawa deleted the cds-warming branch March 3, 2021 06:43

hzxuzhonghu mentioned this pull request Dec 13, 2021

dynamic_warming_clusters does not turn active #19262

Closed

Conversation

Shikugawa commented Aug 22, 2020

Uh oh!

mattklein123 commented Aug 22, 2020

Uh oh!

lizan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stale bot commented Sep 2, 2020

Uh oh!

Shikugawa commented Sep 2, 2020

Uh oh!

Uh oh!

lizan left a comment

Choose a reason for hiding this comment

Uh oh!

mattklein123 commented Oct 14, 2020

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

mattklein123 commented Oct 15, 2020

Uh oh!

lizan commented Oct 15, 2020

Uh oh!

lizan commented Oct 15, 2020

Uh oh!

repokitteh-read-only bot commented Oct 15, 2020

Uh oh!

rgs1 commented Oct 19, 2020

Uh oh!

rgs1 commented Oct 19, 2020

Uh oh!

mattklein123 commented Oct 19, 2020

Uh oh!

rgs1 commented Oct 19, 2020

Uh oh!

rgs1 commented Oct 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rgs1 commented Oct 19, 2020 •

edited

Loading