proxy: slim Istio agent dependencies #50212
Conversation
Change-Id: Ib8ca017c2b86b84c9bff9b818ef6fa531e5e46a8 Signed-off-by: Kuat Yessenov <kuat@google.com>
Change-Id: Iff0c64bb7b806734be402f95129bf788d332525a Signed-off-by: Kuat Yessenov <kuat@google.com>
istio-testing
added
the
size/L
kyessenov
added
the
release-notes-none
|
/retest |
pilot/cmd/pilot-agent/app/cmd.go
Outdated
| @@ -222,7 +222,7 @@ func initStatusServer( | |||
| agent *istio_agent.Agent, | |||
| shutdown context.CancelFunc, | |||
| ) error { | |||
| o := options.NewStatusServerOptions(proxy, proxyConfig, agent) | |||
| o := options.NewStatusServerOptions(proxy.IsIPv6(), proxy.Type, proxyConfig, agent) | |||
There was a problem hiding this comment.
Are we only trying to slim the library, not the cmd itself? If so, why? If not, how does it help to just make the libraries slim?
There was a problem hiding this comment.
I can revert this change. I can't finish changing pilot-agent because of that SDS import. Yes, I tried to move model.Proxy into pkg/model but that causes too much of a change if we embed a struct. So I think it's better to copy just the functions we need.
Change-Id: Ia3ab68417f892843e948e6e91f2730568e54e4d0 Signed-off-by: Kuat Yessenov <kuat@google.com>
Change-Id: I3377a1783b813d50032fe1df9fe8e40f4843ea41 Signed-off-by: Kuat Yessenov <kuat@google.com>
| @@ -71,6 +73,9 @@ func NewAgentOptions(proxy *model.Proxy, cfg *meshconfig.ProxyConfig) *istioagen | |||
| DualStack: features.EnableDualStack, | |||
| UseExternalWorkloadSDS: useExternalWorkloadSDSEnv, | |||
| MetadataDiscovery: enableWDSEnv, | |||
| SDSFactory: func(options *security.Options, workloadSecretCache security.SecretManager, pkpConf *meshconfig.PrivateKeyProvider) istioagent.SDSService { | |||
There was a problem hiding this comment.
How does this help reduce imports? We still import sds and would always do so, even if the underlying sds.newServer is later changed to import less
There was a problem hiding this comment.
It's temporary so we can lock down all of pkg/istio-agent. I don't know when we can convert SDS to be slim. Once that is done, we can simplify. CC @keithmattix
There was a problem hiding this comment.
Can we get this in while we sort out SDS?
|
/retest |
| @@ -71,6 +73,9 @@ func NewAgentOptions(proxy *model.Proxy, cfg *meshconfig.ProxyConfig) *istioagen | |||
| DualStack: features.EnableDualStack, | |||
| UseExternalWorkloadSDS: useExternalWorkloadSDSEnv, | |||
| MetadataDiscovery: enableWDSEnv, | |||
| SDSFactory: func(options *security.Options, workloadSecretCache security.SecretManager, pkpConf *meshconfig.PrivateKeyProvider) istioagent.SDSService { | |||
| return sds.NewServer(options, workloadSecretCache, pkpConf) | |||
| }, | |||
There was a problem hiding this comment.
I was wondering - can we refactor the code so that components like SDS are in their own package, and the agent uses conditional compilation to load them ? Envoy using SDS to get certs from Istiod is great, but the agent use of SDS has always been over-complex and not necessary.
If we move to a model where Sidecar is also sandwiched - ztunnel taking care of security - we can greatly simplify and reduce the code ( and risks ), while keeping most of the sidecar functionality and benefits.
There was a problem hiding this comment.
Go plugins are terrible, so dynamic module loading is unlikely to work. Wasm was suggested, but then these plugins are gRPC heavy which also don't work well in Wasm. My proposal was to just use files as SDS data, and then run the providers in a separate, optional process, which eventually would be replaced by kubernetes/enhancements#4318
|
@costinm That's not true, Envoy gateway has an agent https://github.com/envoyproxy/gateway/blob/main/internal/cmd/server.go. You have to have an agent on k8s because Envoy is not k8s-native. We can leverage any improvement here in Envoy, but that will take time. |
|
Kuat - I'm running it in a cluster, and the yaml looks like:
command:
- envoy
args:
- args:
- --service-cluster default/eg
- --service-node $(ENVOY_POD_NAME)
- |
--config-yaml
admin:
... (normal bootstrap )
And the image is envoyproxy/envoy:distroless-dev
…On Wed, Apr 3, 2024 at 10:24 AM Kuat ***@***.***> wrote:
@costinm <https://github.com/costinm> That's not true, Envoy gateway has
an agent
https://github.com/envoyproxy/gateway/blob/main/internal/cmd/server.go.
You have to have an agent on k8s because Envoy is not k8s-native. We can
leverage any improvement here in Envoy, but that will take time.
—
Reply to this email directly, view it on GitHub
<#50212 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAUR2WSWWEAHI6IUC7VU7LY3Q3OPAVCNFSM6AAAAABFUEDA76VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMZVGE3TKNRVGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@costinm The challenge is the build. Yes, I think it would be nice to run with files, without SDS, but then we need a separate build with or without SDS. We should certainly pursue that for Google build, but the OSS needs to through a long migration process to shed the code. |
Continuation of #50134 to apply to
pkg/istio-agent. This leaves just the SDS server to finish.