Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
jnicholls opened this issue Apr 7, 2020 · 3 comments
Labels
enhancement New feature or request

Comments

@jnicholls
Copy link

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.

  • The Kubernetes workload would be statically configured with the appropriate scuttle ENV vars (such as ENVOY_ADMIN_API and ISTIO_QUIT_API).
  • scuttle would conditionally enforce waiting for Envoy to start based on heuristics where scuttle would inspect its own Pod to determine if istio-proxy is present.
    • If the istio-proxy container is present in the Pod, scuttle will block and wait for ENVOY_ADMIN_API to respond, per its current behavior.
    • If the istio-proxy container is not present in the Pod, scuttle will effectively disable itself, as if the ENV vars were never provided, per its current behavior.

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
Copy link

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:

  • Add a timeout to Envoy waiting. Pros: Easy, Cons: Envoy/Istio could be broken rather than not installed as a sidecar, will slow down the container startup. (e.g. if running locally to debug, waiting 30s to run your code is not ideal)
  • Add a kubernetes client library to scuttle and have it check it's own pod. Pros: Can definitively say if Istio is injected or not, Cons: I think scuttle pod's would need to run under a service account with read permissions to kube api. Not 100% on that.

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.

@linjmeyer linjmeyer added the enhancement New feature or request label Apr 8, 2020
@jnicholls
Copy link
Author

jnicholls commented Apr 8, 2020

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
Copy link
Author

jnicholls commented Apr 9, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants