Skip to content
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

Add no_traffic_healthy_interval #13336

Merged
merged 6 commits into from
Oct 12, 2020

Conversation

Chuongv
Copy link
Contributor

@Chuongv Chuongv commented Sep 30, 2020

Adds a no_traffic_healthy_interval for when a cluster is marked healthy
and we want to use a different interval than no_traffic_interval

Risk level: Low
Testing: Unit test
Fixes: #13246
Release Notes: added option to use no_traffic_healthy_interval which allows a different no traffic interval when the cluster is healthy.

Signed-off-by: Chuong Vu chuongv@google.com

@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #13336 was opened by Chuongv.

see: more, trace.

Copy link
Contributor

@snowp snowp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks this makes sense to me. A few comments to get you started

api/envoy/config/core/v3/health_check.proto Outdated Show resolved Hide resolved
api/envoy/config/core/v3/health_check.proto Outdated Show resolved Hide resolved
api/envoy/config/core/v3/health_check.proto Outdated Show resolved Hide resolved
api/envoy/config/core/v3/health_check.proto Outdated Show resolved Hide resolved
docs/root/version_history/current.rst Outdated Show resolved Hide resolved
docs/root/version_history/current.rst Outdated Show resolved Hide resolved
test/common/upstream/health_checker_impl_test.cc Outdated Show resolved Hide resolved
test/common/upstream/health_checker_impl_test.cc Outdated Show resolved Hide resolved
@Chuongv Chuongv requested a review from snowp October 1, 2020 00:43
@Chuongv Chuongv force-pushed the no_traffic_healthy_interval branch 4 times, most recently from 6c041cf to a6145af Compare October 6, 2020 19:18
@Chuongv
Copy link
Contributor Author

Chuongv commented Oct 6, 2020

@snowp I think this is ready but I'm having trouble figuring out where this is occurring

/source/generated/rst/api-v3/config/core/v3/health_check.proto.rst:164: WARNING: Unexpected indentation.
looking for now-outdated files... none found

Where is this file outputted?

@snowp
Copy link
Contributor

snowp commented Oct 7, 2020

I think maybe it's not liking some of the trailing spaces in the proto comments? You can build docs locally with docs/build.sh

@Chuongv Chuongv force-pushed the no_traffic_healthy_interval branch 2 times, most recently from e461e8a to e0a8475 Compare October 8, 2020 05:23
Chuongv and others added 5 commits October 8, 2020 08:37
Adds a no_traffic_healthy_interval for when a cluster is marked healthy
and we want to use a different interval than no_traffic_interval

Fixes envoyproxy#13246

Risk level: Low
Testing: Unit test

Signed-off-by: Chuong Vu <chuongv@google.com>
Signed-off-by: Chuong Vu <chuongv@google.com>
Co-authored-by: Matt Klein <mattklein123@gmail.com>
Signed-off-by: Chuong Vu <chuongv@google.com>
Signed-off-by: Chuong Vu <chuongv@google.com>
Signed-off-by: Chuong Vu <chuongv@google.com>
@Chuongv Chuongv force-pushed the no_traffic_healthy_interval branch from e0a8475 to a35a4e2 Compare October 8, 2020 15:37
@Chuongv
Copy link
Contributor Author

Chuongv commented Oct 8, 2020

I think maybe it's not liking some of the trailing spaces in the proto comments? You can build docs locally with docs/build.sh

Thanks, that made it much easier to find out what it was 😄 . I think this is ready now.

Copy link
Contributor

@snowp snowp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks this LGTM.

@envoyproxy/api-shepherds for API review

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API LGTM, thanks!

@mattklein123 mattklein123 merged commit 593be22 into envoyproxy:master Oct 12, 2020
@Chuongv Chuongv deleted the no_traffic_healthy_interval branch October 13, 2020 18:12
mpuncel added a commit to mpuncel/envoy that referenced this pull request Oct 14, 2020
* master: (22 commits)
  http: using CONNECT_ERROR for HTTP/2 (envoyproxy#13519)
  listener: respect address.pipe.mode (it didn't work) (envoyproxy#13493)
  examples: Fix more deprecations/warnings in configs (envoyproxy#13529)
  overload: tcp connection refusal overload action (envoyproxy#13311)
  tcp: towards pluggable upstreams (envoyproxy#13331)
  conn_pool: fixing comments (envoyproxy#13520)
  Prevent SEGFAULT when disabling listener (envoyproxy#13515)
  Convert overload manager config literals to YAML (envoyproxy#13518)
  Fix runtime feature variable name (envoyproxy#13533)
  dependencies: refactor repository location schema utils, cleanups. (envoyproxy#13452)
  router:  fix an invalid ASSERT when encoding metadata frames in the router. (envoyproxy#13511)
  http2: Proactively disconnect connections flooded when resetting stream (envoyproxy#13482)
  ci use azp to sync filter example (envoyproxy#13501)
  mongo_proxy: support configurable command list for metrics (envoyproxy#13494)
  http local rate limit: note token bucket is shared (envoyproxy#13525)
  wasm/extensions: Wasm extension policy. (envoyproxy#13526)
  http: removing envoy.reloadable_features.http1_flood_protection (envoyproxy#13508)
  build: update ppc64le CI build status shield (envoyproxy#13521)
  dependencies: enforce dependency shepherd sign-off via RepoKitteh. (envoyproxy#13522)
  Add no_traffic_healthy_interval (envoyproxy#13336)
  ...

Signed-off-by: Michael Puncel <mpuncel@squareup.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

no_traffic_edge_interval
3 participants