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

Is Tekton compatible with injected sidecars? (E.g. istio) #701

Closed
dicarlo2 opened this issue Mar 31, 2019 · 3 comments
Closed

Is Tekton compatible with injected sidecars? (E.g. istio) #701

dicarlo2 opened this issue Mar 31, 2019 · 3 comments
Labels
design This task is about creating and discussing a design kind/feature Categorizes issue or PR as related to a new feature.

Comments

@dicarlo2
Copy link
Contributor

dicarlo2 commented Mar 31, 2019

How does Tekton handle injected sidecars? Off the top of my head, potential issues:

  1. Are containers run by Tekton (git-init, creds-init, etc.) resilient to an unavailable network on pod startup. Use case: Service mesh (istio, linkerd) sidecars can take some time on startup to configure the proxy. During that time, all network connections will fail.
  2. Will Tekton kill injected sidecars (or delete the Pod) once the main container exits?
@dicarlo2
Copy link
Contributor Author

dicarlo2 commented Apr 2, 2019

Looking through the codebase, here are the answers:

  1. No. Containers will not wait for injected sidecars nor are they resilient to network failure. E.g. git-init is a very simple container which just runs a few git commands without retries. Containers defined in steps, either added automatically or by users will wait for previous steps to complete by watching for a wait_file indicating the completion of the previous step.
  2. No. The TaskRun controller watches the pod status and waits for it to exit.

Another thing to note is that there are two init containers remaining, creds-init and the entrypoint copy. I believe the creds-init can be relatively trivially added as the first step to every task. It also does not block execution, at least in the istio case, since neither dockercreds nor gitcreds 1 2 have a network requirement. We should probably convert this to a step in order to support credential use-cases that require network access (e.g. vault credentials), but it's not blocking for istio. I doubt the entrypoint setup will ever require network access, so that's probably fine to stay as an init container (and I'm not sure there's any other way to implement it).

Since our pods require istio in order interface with the network correctly, both internal and external, we're going to try to do the following as a temporary workaround:

  1. Override the entrypoint binary with one that waits for network connectivity.
  2. Override the nop container with one that additionally kills all sidecars via the kube api.

Some thoughts on followups:

  1. Implement first-class support for sidecars. Task configuration would probably come in two flavors, one for specifying sidecars that are expected to be injected by a webhook and one for specifying sidecars that Tekton should start. The latter would cover the istio case for user's willing to manually specify the istio sidecar.
  2. Input/output resource steps need retries, either in process or externally. feat(retries) Design and implement retries #658 is adding support for retries so perhaps we can add configuration to input/output resources that allows specifying retries. Alternatively, they could just have a default number of retries to avoid increasing the API surface area of tasks, potentially with a global configuration option.
  3. On usability/discoverability, it's probably worth documenting somewhere how to share arbitrary directories with downstream tasks in a pipeline. It appears that one way to do this is to chain git resources from input to output. Example use-case: build repo in one step and pass cached build outputs/resources to a fan-out of downstream tasks.
  4. Convert creds-init to a step for future proofing.[low-pri]

cc @bobcatfish @vdemeester

@vdemeester vdemeester added kind/feature Categorizes issue or PR as related to a new feature. design This task is about creating and discussing a design labels Apr 2, 2019
@bobcatfish
Copy link
Collaborator

Thanks for doing this investigation @dicarlo2 !!

If you feel like creating issues in the backlog for any/all of these that would be super cool (and apologies if they already exist and I haven't caught up to them yet haha), in the meantime I've added these to my secret queue of issues to write XD

Implement first-class support for sidecars.

We'd need:

  • To add sidecars to the Task spec
  • Anything else?

one for specifying sidecars that are expected to be injected by a webhook

I'm not familiar with this use case - in this case would the webhook manipulate the pod spec to add the sidecar before the pod actually starts executing? (What would stop that from working currently?)

Input/output resource steps need retries, either in process or externally.

This makes all kinds of sense (and someone else requested this earlier today!!)

it's probably worth documenting somewhere how to share arbitrary directories with downstream tasks
in a pipeline. It appears that one way to do this is to chain git resources from input to output.
I think we should add better support for this, maybe with a new type of PipelineResource?

Convert creds-init to a step for future proofing.

👍

@bobcatfish
Copy link
Collaborator

and apologies if they already exist and I haven't caught up to them yet haha

Aaaand of course looks like ~50% of this is covered in the proposal in #727 😎

No issues yet (I don't think 🤞 ) for adding sidecars to the Task spec, adding retries to PipelineResources and moving creds-init to a step

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design This task is about creating and discussing a design kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

3 participants