dns: fix defunct fd bug in apple resolver#13641
Merged
mattklein123 merged 9 commits intoenvoyproxy:masterfrom Oct 20, 2020
junr03:apple-dns-fixups
Merged
dns: fix defunct fd bug in apple resolver#13641mattklein123 merged 9 commits intoenvoyproxy:masterfrom junr03:apple-dns-fixups
mattklein123 merged 9 commits intoenvoyproxy:masterfrom
junr03:apple-dns-fixups
Conversation
added 4 commits
October 19, 2020 16:12
Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: Jose Nino <jnino@lyft.com>
Member
Author
My and @rebello95's thinking is that we can get this PR merged, and the bug fixed for users of this resolver. And I can work on the (kind of unorthodox) additional unit tests in parallel; I can probably get a rough draft of the PR tomorrow. |
junr03
commented
Oct 20, 2020
| ENVOY_LOG(debug, "DNS resolver file event ({})", events); | ||
| ASSERT(events & Event::FileReadyType::Read); | ||
| DNSServiceProcessResult(main_sd_ref_); | ||
| DNSServiceErrorType error = DNSServiceProcessResult(main_sd_ref_); |
Member
Author
There was a problem hiding this comment.
I verified that all other calls to the DNSService API that return DNSServiceErrorType are handled.
mattklein123
previously approved these changes
Oct 20, 2020
Member
mattklein123
left a comment
mattklein123
left a comment
There was a problem hiding this comment.
LGTM. Better tests would be appreciated.
Member
|
CI failure looks legit. /wait |
Signed-off-by: Jose Nino <jnino@lyft.com>
mattklein123
approved these changes
Oct 20, 2020
Member
Author
|
/retest |
|
Retrying Azure Pipelines. |
Member
Author
|
/retest |
|
🙀 Error while processing event: |
added 2 commits
October 20, 2020 13:28
Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: Jose Nino <jnino@lyft.com>
junr03
added a commit
to envoyproxy/envoy-mobile
that referenced
this pull request
Oct 21, 2020
Description: updates envoy to include: envoyproxy/envoy#13641 which fixes a corner case affecting iOS devices. Updates envoy mobile code to reflect changes in upstream envoy. Signed-off-by: Jose Nino <jnino@lyft.com>
mpuncel
added a commit
to mpuncel/envoy
that referenced
this pull request
Oct 21, 2020
* master: (22 commits) ci: various improvements (envoyproxy#13660) dns: fix defunct fd bug in apple resolver (envoyproxy#13641) build: support ppc64le with wasm (envoyproxy#13657) [fuzz] Added random load balancer fuzz (envoyproxy#13400) dependencies: compute and check release dates via GitHub API. (envoyproxy#13582) mac ci: try ignoring update failure (envoyproxy#13658) watchdog: Optimize WatchdogImpl::touch in preparation to more frequent petting of the watchdog. (envoyproxy#13103) typos: fix a couple 'enovy' mispellings (envoyproxy#13645) lua: Expose stream info downstreamLocalAddress and downstreamDirectRemoteAddress for Lua filter (envoyproxy#13536) tap: fix upstream streamed transport socket taps (envoyproxy#13638) Revert "delay health checks until transport socket secrets are ready. (envoyproxy#13516)" (envoyproxy#13639) Watchdog: use abort action as a default if killing is enabled. (envoyproxy#13523) [fuzz] Fixed divide by zero bug (envoyproxy#13545) wasm: flip the meaning of the "repository" in envoy_wasm_cc_binary(). (envoyproxy#13621) fix: record recovered local address (envoyproxy#13581) docs: fix incorrect compressor filter doc (envoyproxy#13611) docs: clean up docs for azp migration (envoyproxy#13558) wasm: fix building Wasm example. (envoyproxy#13619) test: Refactor flood tests into a separate test file (envoyproxy#13556) wasm: re-enable tests with precompiled modules. (envoyproxy#13583) ... Signed-off-by: Michael Puncel <mpuncel@squareup.com>
jpsim
pushed a commit
that referenced
this pull request
Nov 28, 2022
jpsim
pushed a commit
that referenced
this pull request
Nov 29, 2022
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Commit Message: fix defunct fd bug in apple resolver.
Risk Level: med - necessary fix for apple-based envoy deploys.
Testing: local testing in Envoy Mobile with lifecycle events that force fd failures. This could be better tested if the Apple API was wrapped and replaced in tests to ensure that the AppleDnsResolverImpl code has defensive logic against given API behavior. This could be done in a subsequent PR.
Signed-off-by: Jose Nino jnino@lyft.com