migrated all code to envoyproxy/envoy and keep e2e only here

[wbpcode]

What this PR does / why we need it:

Now, all source code will be hold by the envoyproxy/envoy.

The are mainly two points I want to address by this change:

• For Istio self,to avoid constant merging and fixing when upgrading upstream Envoy. There's no stable ABI, so it can be breaking frequently.
• For users/developers who care Envoy based mesh and gateway, they now could follow the unified singe repo for data plane.

There are only 3-4 extensions in this repo and migrating them to upstream's repo have no any effect to our users or our CI/release procedures because extensions's position doesn't affect the compiled result.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

[istio-policy-bot]

🤔 🐛 You appear to be fixing a bug in Go code, yet your PR doesn't include updates to any test files. Did you forget to add a test?

Courtesy of your friendly test nag.

[istio-policy-bot]

😊 Welcome @wbpcode! This is either your first contribution to the Istio proxy repo, or it's been
a while since you've been here.

You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines
by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

[istio-testing]

Hi @wbpcode. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[wbpcode]

Waiting #6726 to be merged first because we need to ensure the CI works fine before this cleanup.

[wbpcode]

cc @kyessenov cc @zirain

[zirain]

/ok-to-test

[keithmattix]

Thanks for the PR. This is a pretty big change that I hadn’t been made aware of. We definitely will need some more discussion here

[wbpcode]

Thanks for the PR. This is a pretty big change that I hadn’t been made aware of. We definitely will need some more discussion here

Sure. This change comes from envoyproxy/envoy#29681 to free us/distributors out from constant merge and CI problems.

And at envoyproxy/envoy#42785, we have migrated all proxy extensions to envoy repo, without change any logic or API. (We updated Envoy's tool to make sure it could accept istio's API style.)

This PR try to build proxy with code in the Envoy repo and remove the code that have been migrated.

Ideally, there should no any aware-able change for users.

[wbpcode]

/retest

[kyessenov]

is this making some build changes?
CC @krinkinmu

[krinkinmu]

I'm probably overlooking something, but calling envoy_toolchains() + llvm_register_toolchains() should be enough here as far as I understand.

FWIW, I did the same in my PR migrating to hermetic toolchain, but I think it was just an artifact from my experiments that I didn't cleanup.

[kyessenov]

SG, I think it would be good to add some other people to contrib/istio owners in Envoy (@keithmattix ?). I don't think the location of the code matters that much.

[wbpcode]

SG, I think it would be good to add some other people to contrib/istio owners in Envoy (@keithmattix ?). I don't think the location of the code matters that much.

Sure. I will update the code owner list.

[howardjohn]

What is the benefit then?

We really need a design doc showing:

• Goals
• How the end to end build/release process will work
• How private builds will work
• Discuss user facing impacts, if any

[zirain]

Goals

@kyessenov share a lot at envoyproxy/envoy#29681

The most benifit I could image is that we won't be broken when refactor happen on upstream envoy.

How the end to end build/release process will work

Same as today.

How private builds will work

Same as today, target the private repo.

Discuss user facing impacts, if any

I think @wbpcode keep the api same as before as he can.

[kyessenov]

What is the benefit then?

• Goals

Reduce the churn of coordinating changes between upstream and C++ code here. There's no stable ABI, so it keeps breaking all the time.

• How the end to end build/release process will work

Same. This repo can be just a list of extensions used by Istio in the manifest, with no C++ code.

• How private builds will work

Same? Istio maintains an Envoy fork already.

• Discuss user facing impacts, if any

None? This only impacts development.

[dhawton]

• How private builds will work

Same? Istio maintains an Envoy fork already.

PSWG does not use/maintain an Envoy fork. We migrated to using the provided patches and define those in bazel for CVE fixes. IE: https://github.com/istio/proxy/blob/release-1.28.1-patch/WORKSPACE

[kyessenov]

• How private builds will work

Same? Istio maintains an Envoy fork already.

PSWG does not use/maintain an Envoy fork. We migrated to using the provided patches and define those in bazel for CVE fixes. IE: https://github.com/istio/proxy/blob/release-1.28.1-patch/WORKSPACE

OK, I wasn't aware of it. IMO that is still fine, that continues to work as before. You could use a fork and patch extension manifest instead to make it go the other way, but there's not much a difference.

[keithmattix]

@kyessenov Correct me if I'm wrong, but wouldn't moving to envoy repo mean that we'd now need an Envoy senior maintainer to approve every PR to the istio filters?

[kyessenov]

@kyessenov Correct me if I'm wrong, but wouldn't moving to envoy repo mean that we'd now need an Envoy senior maintainer to approve every PR to the istio filters?

No. Senior maintainer review is required for the core code only, not (most of) extension code. Contrib extensions only need extension owners for approval, so the bar is even lower, see https://github.com/envoyproxy/envoy/blob/main/EXTENSION_POLICY.md#contrib-extensions

[wbpcode]

Thanks so much @kyessenov, @zirain. From data plane's perspective, this PR should change nothing except the location of source code. but make us easier to maintain the code.

Will wait the #6726 to be merged first, then I will rebase the PR to resolve conflict.

[keithmattix]

Need a rebase here and need to discuss again with stakeholders. We talked about it in last week's community call, and I think the biggest questions were around the implications of having istio code owned by those who aren't istio maintainers plus having different degrees of testing in different places (istio/proxy, envoy, and istio/istio)

[zirain]

Need a rebase here and need to discuss again with stakeholders. We talked about it in last week's community call, and I think the biggest questions were around the implications of having istio code owned by those who aren't istio maintainers plus having different degrees of testing in different places (istio/proxy, envoy, and istio/istio)

having istio code owned by those who aren't istio maintainers

I think we already add all the guys from E&T WG into codeowners in envoy group.

having different degrees of testing in different places

I would say new feature and new test cases will be added in envoy repo, if there're some addtional e2e test case, it could be happen here.

[wbpcode]

/retest

[keithmattix]

Please let's bring this up one last time in a community meeting before moving forward

[zirain]

Please let's bring this up one last time in a community meeting before moving forward

don't worry, there's a DNM, it won't be merged.

[istio-testing]

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[keithmattix]

FYI, we discussed this in the latest WG meeting; I captured the high level points here: https://docs.google.com/document/d/1wsa06GGiq1LEGwhkiPP0FKIZJqdAiue-VeBonWAzAyk/edit?pli=1&tab=t.0#heading=h.gp28tf83n5tk

[wbpcode]

Consensus: need to answer if the problem we’re solving is worth it, but generally felt like folks were on board if the integration tests could be ported to envoy as well

cc @keithmattix

Although I think even we never move the e2e, this still make sense to simplify our and distributors' life. But sure I completely agree it would make more sense to avoid all these e2e here.

But I think we may needn't to do this in one punch? Rather than the migration, my plan is keep the e2e here, and at next 1-2 release cycle, we will add more unit/integration tests in the Envoy, to ensure all these filters in the Envoy could have similar test quality with other core filters (96.6% coverage).
Then, another 1-2 release cycles, we can remove all e2e tests here also?

By this way, this whole process will be more smoothy and we can have more confidence to the change?

[dhawton]

Consensus: need to answer if the problem we’re solving is worth it, but generally felt like folks were on board if the integration tests could be ported to envoy as well

cc @keithmattix

Although I think even we never move the e2e, this still make sense to simplify our and distributors' life. But sure I completely agree it would make more sense to avoid all these e2e here.

But I think we may needn't to do this in one punch? Rather than the migration, my plan is keep the e2e here, and at next 1-2 release cycle, we will add more unit/integration tests in the Envoy, to ensure all these filters in the Envoy could have similar test quality with other core filters (96.6% coverage). Then, another 1-2 release cycles, we can remove all e2e tests here also?

By this way, this whole process will be more smoothy and we can have more confidence to the change?

But this doesn't really identify the problem, at least for me. What problem are we solving?

From what John said, the original intent on this change was a step toward the effort to migrate to using a straight Envoy binary, but that is no longer the plan.

[keithmattix]

this still make sense to simplify our and distributors' life.

This feels like the problem statement; can we get more details on how this makes things simpler? And are there other benefits?

[wbpcode]

cc @dhawton

Hello, @dhawton, What problem are we solving? This problem actually has been discussed before in this PR. So I focused on the e2e.

But maybe there is too much context and I should put related information at the PR description.

The are mainly two points I want to address:

• For Istio self,to avoid constant merging and fixing when upgrading upstream Envoy. There's no stable ABI, so it can be breaking frequently.
• For users/developers who care Envoy based mesh and gateway, they now could follow the unified singe repo for data plane.

There are only 3-4 extensions in this repo and migrating them to upstream's repo have no any effect to our users or our CI/release procedures because extensions's position doesn't affect the compiled result.

This change could make the cooperation a little more smoothy and have no side effect. So, from my perspective, it's more like a why not?