redboxllc / scuttle

A wrapper for applications to help with running Istio Sidecars
MIT License
116 stars 25 forks source link

Dynamically determine whether istio-proxy sidecar is present before blocking to wait for it to start #21

Open jnicholls opened 4 years ago

jnicholls commented 4 years ago

I have a use case that may be pertinent for others interested in using scuttle for their project. My use case revolves around software that wants to use scuttle conditionally, depending on whether Istio sidecar injection is installed or not, without the software (Kubernetes resources, Helm chart, etc.) having any knowledge about Istio being present or not.

This I feel makes scuttle more robust in environments that cannot dynamically configure their ENV vars or command arguments based on the pre-determined knowledge that Istio is present or not, and yet need a solution to properly wait for the istio-proxy sidecar to be up and ready to go before they begin network activity.

I hope this makes sense. This could be a good first contribution by someone (like myself), and this heuristic check can itself be conditional for starters. It will involve importing the client-go Kubernetes client and doing intra-cluster inspection of its own Pod. Technically the shareProcessNamespace would allow for a solution without a kube-apiserver client, but I think it's more elegant to inspect the workload metadata versus the OS-level namespace.

BONUS: This same heuristic could be used to intelligently enable/disable a default value for ENVOY_ADMIN_API and ISTIO_QUIT_API. As noted in those two issues (#9 and #12), it is a concern that having those variables default to a value will make it unpleasant to run the container in a testing or development fashion. Intraspecting the Kubernetes workload (if Kubernetes even exists, and the istio-proxy sidecar exists) to flip on the default value might be a good approach to the problem.

linjmeyer commented 4 years ago

Hi! We don't have this issue at Redbox, we can say when Istio is used/not used and configure Scuttle via env vars ahead of time. I get the use case though, so I think we could support both. Some options I can think of:

For option 2, if the additional permissions are needed I would see it as an opt in feature. I think adding a service account and RBAC permissions to the kube API for all cronjobs and jobs would be a concern for some. Maybe a pod can get information about itself already though? I'm really not sure on that. It's a cool feature though, lets people use Scuttle in a more automated way, or if they don't like that they can set the env vars themselves and Scuttle can use those.

jnicholls commented 4 years ago

Good points. I had also considered a timeout as another option, and concluded with the same drawbacks as you.

I suppose a timeout is a simple and viable addition to the project, even if it is non-deterministic for this particular use case. I'd say that regardless of the timeout result for proceeding forward with executing the underlying application, that scuttle still attempt to terminate the sidecar as if everything was enabled.

jnicholls commented 4 years ago

I submitted PR #22 to address the feature of an optional timeout. A more deterministic option (with the caveat to users that RBAC permissions need apply) can be done later to address this issue #21.

Maybe a pod can get information about itself already though?

Up to at least kube 1.13 (maybe 1.14) the default service account could get readonly info pods including its own pod. But in 1.15+ the default service account is more locked down. And in general, anyone can modify their default service account to be whatever role(s) they want, so no assumptions can be made in general. It would have to be a feature that documents the explicit need for v1/pods GET support.