Up-port tap/extproc changes from 1.26 to 1.27 #8
Conversation
This file contains 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
Signed-off-by: Ryan Northey <ryan@synca.io>
…ad3646` in /ci (envoyproxy#30048) build(deps): bump distroless/base-nossl-debian12 in /ci Bumps distroless/base-nossl-debian12 from `54f30b8` to `bad3646`. --- updated-dependencies: - dependency-name: distroless/base-nossl-debian12 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: Ryan Northey <ryan@synca.io>
…nvoyproxy#30120) Signed-off-by: Ryan Northey <ryan@synca.io> Signed-off-by: phlax <phlax@users.noreply.github.com>
…y#30126) Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Backport c3646f9 Additional testing: Also, I ran the sources/extensions/tracers/datadog/demo both with and without these changes. Verified that the produced span's "operation name" before these changes is not as desired. Verified that the produced span's "operation name" after these changes is as desired. Desired: "Operation name" is "envoy.proxy", and "resource name" is the operation_name passed to startSpan. Risk Level: low Testing: See the modified unit test. Docs Changes: n/a Release Notes: updated Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
publishing workflow fixes/cleanups enable engflow RBE Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
…#30158) --------- Signed-off-by: Yan Avlasov <yavlasov@google.com>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Summary of changes:
* Fixed a bug where processing of deferred streams with the value of
``http.max_requests_per_io_cycle`` more than 1, can cause a crash.
**Docker images**:
https://hub.docker.com/r/envoyproxy/envoy/tags?page=1&name=v1.27.2
**Docs**:
https://www.envoyproxy.io/docs/envoy/v1.27.2/
**Release notes**:
https://www.envoyproxy.io/docs/envoy/v1.27.2/version_history/v1.27/v1.27.2
**Full changelog**:
envoyproxy/envoy@v1.27.1...v1.27.2
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
…#30204) Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Signed-off-by: Ryan Northey <ryan@synca.io>
* ci fixes/cleanups * test deflaking * dep/vuln update (nghttp2) Signed-off-by: Ryan Northey <ryan@synca.io>
Prevent doConnectionClose to be called recursively when connection with active requests is disconnected due to premature reset check. Signed-off-by: Yan Avlasov <yavlasov@google.com>
Signed-off-by: Yan Avlasov <yavlasov@google.com>
In grpc async client, if timer expiry handler function gets fired where time to next_expiry is less than 1 sec, a loop gets created where timer gets enabled with 0 secs expiry again and again until 'now' becomes as next_expiry. This causes random cpu spikes. If there is HPA configured on cpu, this will result in scale up and scale down on proxies. I added some logs while debugging it. Sharing here might help understanding the issue more clearly: ``` AsyncClientManagerImpl::evictEntriesAndResetEvictionTimer [2023-09-08 10:27:32.640][22][info][grpc] [external/envoy/source/common/grpc/async_client_manager_impl.cc:198] AsyncClientManagerImpl::evictEntriesAndResetEvictionTimer next_expire: 966826645315069, now 966825877367850 [2023-09-08 10:27:32.640][22][info][grpc] [external/envoy/source/common/grpc/async_client_manager_impl.cc:208] AsyncClientManagerImpl::evictEntriesAndResetEvictionTimer enable timer: 0 [2023-09-08 10:27:32.640][22][info][grpc] [external/envoy/source/common/grpc/async_client_manager_impl.cc:193] AsyncClientManagerImpl::evictEntriesAndResetEvictionTimer [2023-09-08 10:27:32.640][22][info][grpc] [external/envoy/source/common/grpc/async_client_manager_impl.cc:198] AsyncClientManagerImpl::evictEntriesAndResetEvictionTimer next_expire: 966826645315069, now 966825877389741 [2023-09-08 10:27:32.640][22][info][grpc] [external/envoy/source/common/grpc/async_client_manager_impl.cc:208] AsyncClientManagerImpl::evictEntriesAndResetEvictionTimer enable timer: 0 ``` When this condition hits, logs get filled because of the loop I mentioned in the beginning. Fix is simple that in the `if` condition for expiry consider this round off thing as well. Additional Description: Risk Level: Testing: Docs Changes: Release Notes: Platform Specific Features: Signed-off-by: Vikas Choudhary <choudharyvikas16@gmail.com> fix tests to match older codebase Signed-off-by: Vikas Choudhary <choudharyvikas16@gmail.com> Signed-off-by: Vikas Choudhary (vikasc) <choudharyvikas16@gmail.com>
…roxy#30057) Commit Message: At client side, if metadata exchange filter is an upstream filter, received filter state gets stored in the upstream streaminfo. While emitting access logs, streaminfo passed to the extractCommonAccessLogProperties() is the downstream side streaminfo. filter_state_objects_to_log keys must be searched in the usptream streaminfo's filter state as well. Additional Description: Risk Level: low Testing: done Docs Changes: no Release Notes: yes Signed-off-by: Vikas Choudhary <choudharyvikas16@gmail.com> Signed-off-by: Vikas Choudhary (vikasc) <choudharyvikas16@gmail.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
…onto v1.27) (envoyproxy#30750) Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
…voyproxy#30892) This is the backport of envoyproxy#30503 Risk Level: low Testing: local unit tests Docs Changes: n/a Release Notes: n/a Platform Specific Features: n/a Fixes: envoyproxy#30235 Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
…proxy#28770) Fixes envoyproxy#28760 Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
On this branch, I created a diff of all commits that we created on top of Envoy 1.26 for extproc and tap. The last commit from upstream envoy was dbbe864, off of which the first extproc commit (b1e99e5) was written. Several extproc commits ensued, and then on top of the last extproc commit was a single commit (4d15e7f) to backport upstream tap changes onto the v1.26.x + extproc branch. Given this, I created a patch by running git diff --patch dbbe864...4d15e7f > extproc-tap.patch This resulted in a single large patch file which I applied on top of `v1.27.1`, which is where this branch starts. Tap merged fine (1 merge conflict), while extproc was more difficult - lots of merge conflicts in the following files: api/envoy/extensions/filters/http/ext_proc/v3/ext_proc.proto changelogs/current.yaml source/common/runtime/runtime_features.cc source/extensions/filters/common/expr/evaluator.cc source/extensions/filters/http/ext_proc/ext_proc.h source/extensions/filters/http/ext_proc/processor_state.h source/extensions/transport_sockets/tap/config.cc test/extensions/filters/http/ext_proc/config_test.cc test/extensions/filters/http/ext_proc/ext_proc_integration_test.cc test/extensions/filters/http/ext_proc/filter_test.cc I tried to resolve them myself, and to be frank, I think I did a pretty damn good job. But there are still a few compile errors that I don't know how to resolve, so I'm going to push this branch from here and see if I can pull in some help. The good news is that a lot of it compiles - everything in `source/` compiles successfully, including `//source/exe:envoy-static`! The bad news is that some tests don't compile, and I also cannot be assured that any of the extproc tests will pass. (I could write a fancy bazel command to run some of the tests that _do_ compile, but I'd rather enjoy my evening.) The tests were some of the hardest conflicts to resolve, and clearly there is still some work here left to be done, so this part remains a work in progress.
The tests compile now. Not sure if this is the correct fix though so a second pair of eyes would be great here.
The fix was to simply copy the builder initialisation code from `createFilterFactoryFromProtoTyped` into `createFilterFactoryFromProtoWithServerContextTyped`
jbohanon
reviewed
Nov 30, 2023
| @@ -71,7 +71,7 @@ class OrderingTest : public testing::Test { | |||
| (*cb)(proto_config); | |||
| } | |||
| config_.reset(new FilterConfig(proto_config, kMessageTimeout, kMaxMessageTimeoutMs, | |||
| *stats_store_.rootScope(), "")); | |||
| *stats_store_.rootScope(), "", Envoy::Extensions::Filters::Common::Expr::BuilderInstance(Envoy::Extensions::Filters::Common::Expr::createBuilder(nullptr)).builder())); | |||
There was a problem hiding this comment.
This is how this builder is initialized on the upstream PR
Suggested change
| *stats_store_.rootScope(), "", Envoy::Extensions::Filters::Common::Expr::BuilderInstance(Envoy::Extensions::Filters::Common::Expr::createBuilder(nullptr)).builder())); | |
| *stats_store_.rootScope(), "", std::make_shared<Envoy::Extensions::Filters::Common::Expr::BuilderInstance>( | |
| Envoy::Extensions::Filters::Common::Expr::createBuilder(nullptr)))); |
| @@ -66,7 +66,8 @@ DEFINE_PROTO_FUZZER( | |||
| try { | |||
| config = std::make_shared<ExternalProcessing::FilterConfig>( | |||
| proto_config, std::chrono::milliseconds(200), 200, *stats_store.rootScope(), | |||
| "ext_proc_prefix"); | |||
| "ext_proc_prefix", | |||
| Envoy::Extensions::Filters::Common::Expr::BuilderInstance(Envoy::Extensions::Filters::Common::Expr::createBuilder(nullptr)).builder()); | |||
There was a problem hiding this comment.
Suggested change
| Envoy::Extensions::Filters::Common::Expr::BuilderInstance(Envoy::Extensions::Filters::Common::Expr::createBuilder(nullptr)).builder()); | |
| std::make_shared<Envoy::Extensions::Filters::Common::Expr::BuilderInstance>( | |
| Envoy::Extensions::Filters::Common::Expr::createBuilder(nullptr))); |
| @@ -119,7 +129,8 @@ class HttpFilterTest : public testing::Test { | |||
| if (!yaml.empty()) { | |||
| TestUtility::loadFromYaml(yaml, proto_config); | |||
| } | |||
| config_.reset(new FilterConfig(proto_config, 200ms, 10000, *stats_store_.rootScope(), "")); | |||
| config_.reset(new FilterConfig(proto_config, 200ms, 10000, *stats_store_.rootScope(), "", | |||
| Envoy::Extensions::Filters::Common::Expr::BuilderInstance(Envoy::Extensions::Filters::Common::Expr::createBuilder(nullptr)).builder())); | |||
There was a problem hiding this comment.
Suggested change
| Envoy::Extensions::Filters::Common::Expr::BuilderInstance(Envoy::Extensions::Filters::Common::Expr::createBuilder(nullptr)).builder())); | |
| std::make_shared<Envoy::Extensions::Filters::Common::Expr::BuilderInstance>( | |
| Envoy::Extensions::Filters::Common::Expr::createBuilder(nullptr)))); |
Signed-off-by: Ashish Banerjee <ashish.banerjee@solo.io>
Tap sink typed extension (envoyproxy#28808)
|
We're gonna abandon the strategy used in this pull request. Instead of up-porting the patches from our 1.26 fork, we're instead going to backport Jacob's patches from upstream envoy onto this branch. This seems a bit more reliable. See #11 for an example of one of the patches that we have already backported onto this branch. There will be more - stay tuned to the https://github.com/solo-io/envoy-fork/tree/release/v1.27-backportedfork branch to see if this strategy works out |
ben-taussig-solo
pushed a commit
that referenced
this pull request
Jan 29, 2024
Commit Message: the probing socket is released when port migration fails. If this happens in response to an incoming packet during an I/O event, the follow socket read could cause use-after-free. [2024-01-08 16:30:53.386][12][critical][backtrace] [./source/server/backtrace.h:104] Caught Segmentation fault, suspect faulting address 0x0 [2024-01-08 16:30:53.387][12][critical][backtrace] [./source/server/backtrace.h:91] Backtrace (use tools/stack_decode.py to get line numbers): [2024-01-08 16:30:53.387][12][critical][backtrace] [./source/server/backtrace.h:92] Envoy version: 0/1.29.0-dev/test/DEBUG/BoringSSL [2024-01-08 16:30:53.413][12][critical][backtrace] [./source/server/backtrace.h:96] #0: Envoy::SignalAction::sigHandler() [0x55bb876d499e] [2024-01-08 16:30:53.413][12][critical][backtrace] [./source/server/backtrace.h:98] #1: [0x7f55fbf92510] [2024-01-08 16:30:53.440][12][critical][backtrace] [./source/server/backtrace.h:96] #2: Envoy::Network::Utility::readPacketsFromSocket() [0x55bb875de0ef] [2024-01-08 16:30:53.466][12][critical][backtrace] [./source/server/backtrace.h:96] #3: Envoy::Quic::EnvoyQuicClientConnection::onFileEvent() [0x55bb8663e1eb] [2024-01-08 16:30:53.492][12][critical][backtrace] [./source/server/backtrace.h:96] #4: Envoy::Quic::EnvoyQuicClientConnection::setUpConnectionSocket()::$_0::operator()() [0x55bb8663f192] [2024-01-08 16:30:53.518][12][critical][backtrace] [./source/server/backtrace.h:96] #5: std::__invoke_impl<>() [0x55bb8663f151] [2024-01-08 16:30:53.544][12][critical][backtrace] [./source/server/backtrace.h:96] #6: std::__invoke_r<>() [0x55bb8663f0e2] [2024-01-08 16:30:53.569][12][critical][backtrace] [./source/server/backtrace.h:96] #7: std::_Function_handler<>::_M_invoke() [0x55bb8663efc2] [2024-01-08 16:30:53.595][12][critical][backtrace] [./source/server/backtrace.h:96] #8: std::function<>::operator()() [0x55bb85cb8f44] [2024-01-08 16:30:53.621][12][critical][backtrace] [./source/server/backtrace.h:96] #9: Envoy::Event::DispatcherImpl::createFileEvent()::$_5::operator()() [0x55bb8722560f] [2024-01-08 16:30:53.648][12][critical][backtrace] [./source/server/backtrace.h:96] #10: std::__invoke_impl<>() [0x55bb872255c1] [2024-01-08 16:30:53.674][12][critical][backtrace] [./source/server/backtrace.h:96] #11: std::__invoke_r<>() [0x55bb87225562] [2024-01-08 16:30:53.700][12][critical][backtrace] [./source/server/backtrace.h:96] #12: std::_Function_handler<>::_M_invoke() [0x55bb872253e2] [2024-01-08 16:30:53.700][12][critical][backtrace] [./source/server/backtrace.h:96] #13: std::function<>::operator()() [0x55bb85cb8f44] [2024-01-08 16:30:53.726][12][critical][backtrace] [./source/server/backtrace.h:96] #14: Envoy::Event::FileEventImpl::mergeInjectedEventsAndRunCb() [0x55bb872358ec] [2024-01-08 16:30:53.752][12][critical][backtrace] [./source/server/backtrace.h:96] #15: Envoy::Event::FileEventImpl::assignEvents()::$_1::operator()() [0x55bb87235ed1] [2024-01-08 16:30:53.778][12][critical][backtrace] [./source/server/backtrace.h:96] #16: Envoy::Event::FileEventImpl::assignEvents()::$_1::__invoke() [0x55bb87235949] [2024-01-08 16:30:53.804][12][critical][backtrace] [./source/server/backtrace.h:96] #17: event_persist_closure [0x55bb87fab72b] [2024-01-08 16:30:53.830][12][critical][backtrace] [./source/server/backtrace.h:96] #18: event_process_active_single_queue [0x55bb87faada2] [2024-01-08 16:30:53.856][12][critical][backtrace] [./source/server/backtrace.h:96] #19: event_process_active [0x55bb87fa56c8] [2024-01-08 16:30:53.882][12][critical][backtrace] [./source/server/backtrace.h:96] #20: event_base_loop [0x55bb87fa45cc] [2024-01-08 16:30:53.908][12][critical][backtrace] [./source/server/backtrace.h:96] #21: Envoy::Event::LibeventScheduler::run() [0x55bb8760a59f] Risk Level: low Testing: new unit test Docs Changes: N/A Release Notes: Yes Platform Specific Features: N/A Signed-off-by: Dan Zhang <danzh@google.com> Co-authored-by: Dan Zhang <danzh@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.
On this branch, I created a diff of all commits that we created on top of Envoy 1.26 for extproc and tap.
The last commit from upstream envoy was
dbbe864, off of which the first extproc commit (b1e99e5) was written. Several extproc commits ensued, and then on top of the last extproc commit was a single commit (4d15e7f) to backport upstream tap changes onto the v1.26.x + extproc branch.
Given this, I created a patch by running
This resulted in a single large patch file which I applied on top of
v1.27.1, which is where this branch starts. Tap merged fine (1 merge conflict), while extproc was more difficult - lots of merge conflicts in the following files:I tried to resolve them myself, and to be frank, I think I did a pretty damn good job. But there are still a few compile errors that I don't know how to resolve, so I'm going to push this branch from here and see if I can pull in some help.
The good news is that a lot of it compiles - everything in
source/compiles successfully, including//source/exe:envoy-static!The bad news is that some tests don't compile, and I also cannot be assured that any of the extproc tests will pass. (I could write a fancy bazel command to run some of the tests that do compile, but I'd rather enjoy my evening.) The tests were some of the hardest conflicts to resolve, and clearly there is still some work here left to be done, so this part remains a work in progress.
Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]