Revert "Fixes #2943 - implements shared grpc connection per multiple …#5272
Conversation
|
Please fill out the PR as described in https://github.com/envoyproxy/envoy/pull/PULL_REQUESTS.md -- I'm not sure why you want to revert this commit. |
adilhafeez
changed the title
|
@zuercher updated description |
|
@adilhafeez please amend your commit to have the DCO signoff. Adding text in the form |
|
@adilhafeez @lita @junr03 @tonya11en has this commit been verified to be faulty via bisecting/revert? Has any basic debugging been done? If we know this commit is faulty we can revert but it would we should 100% confirm this commit is at fault. |
|
@mattklein123 this commit caused a stream of 503 no healthy upstream hosts which stopped when we reverted this commit. |
|
@adilhafeez per previous comment please fix DCO. Thanks! |
mattklein123
changed the title
|
I added text that @zuercher recommended. Isnt that enough? |
No, as the DCO bot is still failing. Please see the bot failure and the contributing guide to see how to work with DCO. |
|
Will fix the DCO. Thanks |
|
Is there a failing test case for this? |
… multiple EDS subscriptions. (envoyproxy#5026)" This reverts commit a085974. Signed-off-by: Adil Hafeez <[email protected]>
adilhafeez
force-pushed
the
revert_a08597467917c3d982755ea45075e05106b92dd7
branch
from
44206cf to
d3cc24f
Compare
@dmitri-d no, but we should have test case to ensure that we don't break EDS fetch next time |
|
@mattklein123 DCO bot is happy now |
|
@adilhafeez: I'm happy to help with a testcase(s), but would need more details re: how EDS fetch fails. |
|
@adilhafeez: yes, I think reverting the commit is the way to go. Let me know if I can help with the investigation. |
|
@dmitri-d, @junr03 will be able to help to help debug and describe to you the problem next week so that you can fix it. @adilhafeez Yes, we will obviously have a test case for whatever is broken when it is fixed. |
|
@mattklein123 can you please merge? all tests passed. |
|
OK, we should definitely have a dedicated EDS gRPC xDS integration test; today we have a combined gRPC ADS integration test and an EDS xDS from filesystem. We could build on the work done in #5228 to add a dedicated EDS gRPC test here. |
|
I'll add EDS integration test while I'm at it. |
|
@dmitri-d yeah I would go ahead and just do that as a change on top of current master. It's possible that test will fail when you un-revert the change and then we don't need to debug at Lyft. |
… multiple EDS subscriptions. (envoyproxy#5026)" (envoyproxy#5272) This reverts commit a085974. Signed-off-by: Adil Hafeez <[email protected]> Signed-off-by: Fred Douglas <[email protected]>
6 participants
Description:
While rolling out envoy/master at Lyft we found out that xDS was having issues making outbound grpc calls to fetch cluster information. We are still investigating the root case. This commit seems to be the suspect.
This reverts commit a085974.
Risk Level: Low
Testing: tested on lyft staging environment
Docs Changes: n/a
Release Notes: n/a
Signed-off-by: Adil Hafeez [email protected]