Skip to content
This repository has been archived by the owner on Jun 14, 2018. It is now read-only.

Decouple proxy from pilot agent #968

Closed
andraxylia opened this issue Jul 28, 2017 · 10 comments
Closed

Decouple proxy from pilot agent #968

andraxylia opened this issue Jul 28, 2017 · 10 comments
Assignees

Comments

@andraxylia
Copy link
Contributor

andraxylia commented Jul 28, 2017

Right now, proxy and agent are bundled in the same container. Their lifecycle cannot be controller by k8s native, it's the agent controlling the proxy process, and it's flaky.

Proxy should be the same container in ingress, sidecar or middle proxy.

This will also simplify the build.

@andraxylia andraxylia added this to the Istio 0.3 milestone Jul 28, 2017
@kyessenov
Copy link
Contributor

  1. k8s cannot coordinate envoy restarts (passing TCP sockets between restarts)

  2. Ingress controller is separated from proxy. Discovery service implements k8s ingress controller.

  3. Done already.

@andraxylia
Copy link
Contributor Author

Does pilot agent still triggers envoy restart even after LDS? In what scenarios?

@andraxylia
Copy link
Contributor Author

Ingress controller is not separate from the proxy, it runs in the same container. Does ingress controller need to restart the envoy process also? If yes, in what scenarios?

@andraxylia andraxylia self-assigned this Jul 29, 2017
@andraxylia andraxylia reopened this Jul 29, 2017
@kyessenov
Copy link
Contributor

kyessenov commented Jul 29, 2017

I feel that there is some confusion here.

Yes, pilot agent is triggering restarts since LDS does not cover TLS contexts (yet). We can't do TLS cert rotation without it at the moment. Lyft uses a similar set-up running a little agent next to the proxy.

Ingress controller is separate from the proxy. Discovery service computes the ingress routes and then sends them to the proxy. Again, TLS changes require restarts.

If we never need to restart Envoy then there is simply no need for the agent. If we do need to restart Envoy then it has to be in the same container to coordinate socket exchange.

Not sure what the point of this issue.

@andraxylia
Copy link
Contributor Author

andraxylia commented Jul 29, 2017

I thought we no longer need to restart Envoy. If this will be true in the future, let's keep this issue open and re-visit it for 0.3 release.

Envoy restart for TLS changes has been brought up here:
envoyproxy/envoy#95

@PiotrSikora @myidpt @wattli are there any chances Envoy will support TLS certificates rotation without requiring restart?

@andraxylia
Copy link
Contributor Author

Also here:
envoyproxy/envoy#1194

@andraxylia
Copy link
Contributor Author

So the ingress controller is bundled with the pilot discovery?

@andraxylia andraxylia changed the title Decouple proxy from pilot agent and from the ingress controller Decouple proxy from pilot agent Jul 29, 2017
@rshriram
Copy link
Member

rshriram commented Jul 30, 2017 via email

@andraxylia
Copy link
Contributor Author

I do not believe in the "always" in the above sentence. Let's just wait from input from the people working in envoy on that.
And the restart has been flaky, pilot e2e routing tests fail randomly when run independently. It's good LDS will solve 95% of the problem, but even with LDS I see random failures. I am looking for an architecture where we solve the problem completely. If it is possible, let's do it.

In early February I asked Kuat about these frequent restarts and why the communication between agent and envoy is done via config, and in general, why is this so complicated, and the answer I got was "this is what it is, all proxies work this way, there is nothing we can do". We could do xDS little time after. So I am not buying the "always" unless there is a technical impossibility or a solid reason for not being able to do it.

@kyessenov
Copy link
Contributor

Issue moved to istio/istio #1393 via ZenHub

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants