Remove using redis proxy for redis protocol #1915
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Assign the PR to them by writing The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
Codecov Report
@@ Coverage Diff @@
## master #1915 +/- ##
==========================================
- Coverage 81.14% 80.98% -0.16%
==========================================
Files 187 186 -1
Lines 18883 18882 -1
==========================================
- Hits 15322 15291 -31
- Misses 3128 3157 +29
- Partials 433 434 +1
Continue to review full report at Codecov.
|
rshriram
reviewed
Nov 30, 2017
pilot/proxy/envoy/config.go
Outdated
| }, | ||
| }}, | ||
| } | ||
| } |
There was a problem hiding this comment.
Instead of removing all the code, can you just change in the main switch statement to consider redis as TCP? And add comments in functions like above, stating that redis support has to be enabled in the said switch statement..
I don't want to add this code again, when we fix this.
There was a problem hiding this comment.
Note that the removed code in git anyway. In the future you can just revert this commit. Anyway, submitting a version without redis code removed.
|
/test istio-presubmit |
rshriram
approved these changes
Nov 30, 2017
kyessenov
pushed a commit
to kyessenov/istio
that referenced
this pull request
Aug 14, 2018
Before:
$ bazel-bin/src/envoy/envoy --version
bazel-bin/src/envoy/envoy version: 0/1.8.0-dev//DEBUG
After:
$ bazel-bin/src/envoy/envoy --version
bazel-bin/src/envoy/envoy version: f315a32fc7c6f727fc9645cc1ca27d4160c1d0e0/1.8.0-dev/Clean/DEBUG
Fixes istio#1803.
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
PiotrSikora
added a commit
to PiotrSikora/istio
that referenced
this pull request
Aug 21, 2018
Pulling the following changes from github.com/istio/proxy: 1fc6253 add debug logs for collecting rbac attributes (istio#1922) c5282b6 Update Envoy SHA to latest with LcTrie optimizations. (istio#1918) 4ced9e7 Update clang to 6.0 and use it for release binaries. (istio#1914) 585abec fixed broken links to dev guide and contribution guide (istio#1913) c63d841 Provide source version information in the binary. (istio#1915) b49589a Install clang-format in the build image used by CircleCI. (istio#1917) 5d42471 Fix macOS build on CircleCI. (istio#1916) b1f4e7e add rbac filter to istio http integration test. (istio#1907) Pulling the following changes from github.com/envoyproxy/envoy: 73bd3d95c http_filter: add addEncodedTrailers and addDecodedTrailers (istio#3980) c3652aad5 rbac/fuzz: fix build (istio#4150) 07bc27c05 fix flaky RBAC integration test. (istio#4147) b150d61a9 header_map: copy constructor for HeaderMapImpl. (istio#4129) f345c8b23 test: moving websocket tests to using HTTP codec. (istio#4143) da500d20f upstream: init host hc value based on hc value from other priorities (istio#3959) da6194b94 test: add tests for corner-cases around sending requests before run() starts or after run() ends. (istio#4114) 3527f7799 perf: reduce the memory usage of LC Trie construction (istio#4117) b538e46d8 test: moving redundant code in websocket_integration_test to utilities (istio#4127) a3c55bf7b test: make YamlLoadFromStringFail less picky about error msg. (istio#4141) c283439b6 rbac: add rbac network filter. (istio#4083) 5a7152d21 fuzz: route lookup and header finalization fuzzer. (istio#4116) 589467360 Set content-type and content-length (istio#4113) 714ae130a fault: use FractionalPercent for percent (istio#3978) fde378705 test: Fix inverted exact match logic in IntegrationTcpClient::waitForData() (istio#4134) 794a00126 Added cluster_name to load assignment config for static cluster (istio#4123) 19f51e5e1 ssl: refactor ContextConfig to use TlsCertificateConfig (istio#4115) 0a4bffc5a syscall: refactor OsSysCalls for deeper errno latching (istio#4111) ec0d98e5e thrift_proxy: fix oneway bugs (istio#4025) 1381673ad Do not crash when converting YAML to JSON fails (istio#4110) 2662bf1f2 config: allow unknown fields flag (take 2) (istio#4096) 1ab839c1f Use a jittered backoff strategy for handling HdsDelegate stream/connection failures (istio#4108) 7309c14cf bazel: use GCS remote cache (istio#4050) 5fe4e14f0 Add thread local cache of overload action states (istio#4090) 3bb7fbc5f Added TCP healthcheck capabilities to the HdsDelegate (istio#4079) 98037ed37 secret: add secret provider interface and use it for TlsCertificates (istio#4086) 3e15c9490 upstream: allow custom extension protocol options (istio#4098) 9b33c49d1 Rename message types in hds.proto to improve readability (istio#4109) bb70b42bb fuzz: router header formatter/parser fuzz test. (istio#4105) fe57f6b33 fuzz: http parsing utility fuzzer. (istio#4107) 73dfedc95 ci: link ninja-buid to ninja for centos (istio#4106) 1cd509ef1 docs: add curl to Ubuntu deps (istio#4104) 45b900829 Handling updates from the management server on HDS (istio#4077) 510994c6a Don't use SIGTERM for admin /quitquitquit, just shut down directly. (istio#4099) 29b60291e fuzz: access log formatter fuzz test. (istio#4102) 765cac42f Destroy pending updates when updating a cluster (istio#4084) aafdf6037 authz_client_fix: fixed ext_authz http client when request contains content-length greater than 0 (istio#3888) 22ae0ab93 HttpConnectionManager and upstream counters for total completed requests (istio#3995) 04616d676 tcp_proxy: convert TCP proxy to use TCP connection pool (istio#4067) e759eab17 buffer: add prepend functions to Buffer::Instance (istio#4064) Fixes istio#7710, fixes istio#7817, and hopefully fixes istio#7759. Signed-off-by: Piotr Sikora <piotrsikora@google.com>
istio-testing
pushed a commit
that referenced
this pull request
Aug 22, 2018
Pulling the following changes from github.com/istio/proxy: 1fc6253 add debug logs for collecting rbac attributes (#1922) c5282b6 Update Envoy SHA to latest with LcTrie optimizations. (#1918) 4ced9e7 Update clang to 6.0 and use it for release binaries. (#1914) 585abec fixed broken links to dev guide and contribution guide (#1913) c63d841 Provide source version information in the binary. (#1915) b49589a Install clang-format in the build image used by CircleCI. (#1917) 5d42471 Fix macOS build on CircleCI. (#1916) b1f4e7e add rbac filter to istio http integration test. (#1907) Pulling the following changes from github.com/envoyproxy/envoy: 73bd3d95c http_filter: add addEncodedTrailers and addDecodedTrailers (#3980) c3652aad5 rbac/fuzz: fix build (#4150) 07bc27c05 fix flaky RBAC integration test. (#4147) b150d61a9 header_map: copy constructor for HeaderMapImpl. (#4129) f345c8b23 test: moving websocket tests to using HTTP codec. (#4143) da500d20f upstream: init host hc value based on hc value from other priorities (#3959) da6194b94 test: add tests for corner-cases around sending requests before run() starts or after run() ends. (#4114) 3527f7799 perf: reduce the memory usage of LC Trie construction (#4117) b538e46d8 test: moving redundant code in websocket_integration_test to utilities (#4127) a3c55bf7b test: make YamlLoadFromStringFail less picky about error msg. (#4141) c283439b6 rbac: add rbac network filter. (#4083) 5a7152d21 fuzz: route lookup and header finalization fuzzer. (#4116) 589467360 Set content-type and content-length (#4113) 714ae130a fault: use FractionalPercent for percent (#3978) fde378705 test: Fix inverted exact match logic in IntegrationTcpClient::waitForData() (#4134) 794a00126 Added cluster_name to load assignment config for static cluster (#4123) 19f51e5e1 ssl: refactor ContextConfig to use TlsCertificateConfig (#4115) 0a4bffc5a syscall: refactor OsSysCalls for deeper errno latching (#4111) ec0d98e5e thrift_proxy: fix oneway bugs (#4025) 1381673ad Do not crash when converting YAML to JSON fails (#4110) 2662bf1f2 config: allow unknown fields flag (take 2) (#4096) 1ab839c1f Use a jittered backoff strategy for handling HdsDelegate stream/connection failures (#4108) 7309c14cf bazel: use GCS remote cache (#4050) 5fe4e14f0 Add thread local cache of overload action states (#4090) 3bb7fbc5f Added TCP healthcheck capabilities to the HdsDelegate (#4079) 98037ed37 secret: add secret provider interface and use it for TlsCertificates (#4086) 3e15c9490 upstream: allow custom extension protocol options (#4098) 9b33c49d1 Rename message types in hds.proto to improve readability (#4109) bb70b42bb fuzz: router header formatter/parser fuzz test. (#4105) fe57f6b33 fuzz: http parsing utility fuzzer. (#4107) 73dfedc95 ci: link ninja-buid to ninja for centos (#4106) 1cd509ef1 docs: add curl to Ubuntu deps (#4104) 45b900829 Handling updates from the management server on HDS (#4077) 510994c6a Don't use SIGTERM for admin /quitquitquit, just shut down directly. (#4099) 29b60291e fuzz: access log formatter fuzz test. (#4102) 765cac42f Destroy pending updates when updating a cluster (#4084) aafdf6037 authz_client_fix: fixed ext_authz http client when request contains content-length greater than 0 (#3888) 22ae0ab93 HttpConnectionManager and upstream counters for total completed requests (#3995) 04616d676 tcp_proxy: convert TCP proxy to use TCP connection pool (#4067) e759eab17 buffer: add prepend functions to Buffer::Instance (#4064) Fixes #7710, fixes #7817, and hopefully fixes #7759. Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Currently, envoy does not support CDS clusters for redis proxy. Redis services become unaccessible on Istio when redis proxy is used.
The code in envoy that produces an error when CDS cluster is used for redis proxy:
https://github.com/envoyproxy/envoy/blob/8fee0f11f1d06abb1dae820a388ffe6d785274c0/source/common/redis/proxy_filter.cc#L21
calls
https://github.com/envoyproxy/envoy/blob/6b2823da5006e92bc4b365e9e8804a4f6a2eba37/source/common/config/utility.cc#L47
where an exception is thrown, resulting in listener on the port and the cluster not being added.
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #1763Special notes for your reviewer:
Release note: