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.

Sign up for GitHub

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

Jump to bottom

Set Ready annotation during pod creation for Taskruns with no Sidecars #2080

Closed
dibyom opened this issue Feb 20, 2020 · 5 comments · Fixed by #2158
Closed

Set Ready annotation during pod creation for Taskruns with no Sidecars #2080

dibyom opened this issue Feb 20, 2020 · 5 comments · Fixed by #2158
Assignees
@dibyom
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@dibyom
Copy link
Member

dibyom commented Feb 20, 2020

Context

Jason has a really nice description of this in the Problem section of #1569. But in short, today a step in a Taskrun waits to start until it sees a "READY" value that is populated by the Taskrun controller using the Downward API. The controller waits for all sidecars to be up an running setting the annotation.

Suggestion

For taskruns without sidecars, the controller should be able to set the ready value when creating the pod.

This is a performance optimization and should help the taskrun start running faster and the taskrun controller have less overall load.
@dibyom
Copy link
Member Author

dibyom commented Feb 20, 2020

I was chatting about this with @sbwsg who mentioned that one of the reason the sidecar logic is the way it is today is to support injected sidecars such as istio. Since the TaskRun controller does not know about any injected sidecars until the pod has been created, implementing the suggestion will mean we break support for injected sidecars.

An alternative:
We can make this an option that the operator can control via a configMap. So the clusters that do not use injected sidecars can benefit from the performance gains

@nishpa214
Copy link

Ah, interesting. I hadn't thought of that use case.

We can make this an option that the operator can control via a configMap. So the clusters that do not use injected sidecars can benefit from the performance gains

Yeah, we don't have an istio setup, so controlling this via a configMap sounds like a good path forward.

@dibyom
Copy link
Member Author

dibyom commented Feb 20, 2020

Yeah, the config can default to the current behavior and those needing better performance and not using injected sidecars can use the config option for this optimization. What do you think @sbwsg

@bobcatfish bobcatfish added the kind/feature Categorizes issue or PR as related to a new feature. label Feb 21, 2020
@ghost
Copy link

ghost commented Feb 26, 2020

Yeah, the config can default to the current behavior and those needing better performance and not using injected sidecars can use the config option for this optimization.

That sounds to me like a solid approach to take!

@dibyom
Copy link
Member Author

dibyom commented Mar 4, 2020

/assign

@dibyom dibyom mentioned this issue Mar 4, 2020
Merged
3 tasks
dibyom added a commit to dibyom/pipeline that referenced this issue Mar 9, 2020
@dibyom
Allow setting sidecar ready annotation early
19d304e 
If no sidercars are present, we can set the `tekton.dev/ready`
annotation during pod creation itself instead waiting for the
controller to set it after pod creation.

This commit adds an option to the `feature-flags` ConfigMap to
control this behavior (`enable-ready-annotation-on-pod-create`).
By default, the option is set to "false".Setting this flag to "true"
will set the annotation at pod creation time for Taskruns without Sidercars.

Enabling this option should decrease the time it takes for a TaskRun to
start running. However, for clusters that use injected sidecars e.g. istio
enabling this option can lead to unexpected behavior.

Fixes tektoncd#2080

Signed-off-by: Dibyo Mukherjee <[email protected]>
dibyom added a commit to dibyom/pipeline that referenced this issue Mar 9, 2020
@dibyom
Allow setting sidecar ready annotation early
2d85c85 
If no sidercars are present, we can set the `tekton.dev/ready`
annotation during pod creation itself instead waiting for the
controller to set it after pod creation.

This commit adds an option to the `feature-flags` ConfigMap to
control this behavior (`enable-ready-annotation-on-pod-create`).
By default, the option is set to "false".Setting this flag to "true"
will set the annotation at pod creation time for Taskruns without Sidercars.

Enabling this option should decrease the time it takes for a TaskRun to
start running. However, for clusters that use injected sidecars e.g. istio
enabling this option can lead to unexpected behavior.

Fixes tektoncd#2080

Signed-off-by: Dibyo Mukherjee <[email protected]>
dibyom added a commit to dibyom/pipeline that referenced this issue Mar 9, 2020
@dibyom
Allow setting sidecar ready annotation early
3268eaf 
If no sidercars are present, we can set the `tekton.dev/ready`
annotation during pod creation itself instead waiting for the
controller to set it after pod creation.

This commit adds an option to the `feature-flags` ConfigMap to
control this behavior (`enable-ready-annotation-on-pod-create`).
By default, the option is set to "false".Setting this flag to "true"
will set the annotation at pod creation time for Taskruns without Sidercars.

Enabling this option should decrease the time it takes for a TaskRun to
start running. However, for clusters that use injected sidecars e.g. istio
enabling this option can lead to unexpected behavior.

Fixes tektoncd#2080

Signed-off-by: Dibyo Mukherjee <[email protected]>
dibyom added a commit to dibyom/pipeline that referenced this issue Apr 10, 2020
@dibyom
Allow setting sidecar ready annotation early
f64e567 
If no sidercars are present, we can set the `tekton.dev/ready`
annotation during pod creation itself instead waiting for the
controller to set it after pod creation.

This commit adds an option to the `feature-flags` ConfigMap to
control this behavior (`enable-ready-annotation-on-pod-create`).
By default, the option is set to "false".Setting this flag to "true"
will set the annotation at pod creation time for Taskruns without Sidercars.

Enabling this option should decrease the time it takes for a TaskRun to
start running. However, for clusters that use injected sidecars e.g. istio
enabling this option can lead to unexpected behavior.

Fixes tektoncd#2080

Signed-off-by: Dibyo Mukherjee <[email protected]>
dibyom added a commit to dibyom/pipeline that referenced this issue May 26, 2020
@dibyom
Allow setting sidecar ready annotation early
b66c776 
If no sidercars are present, we can set the `tekton.dev/ready`
annotation during pod creation itself instead waiting for the
controller to set it after pod creation.

This commit adds an option to the `feature-flags` ConfigMap to
control this behavior (`enable-ready-annotation-on-pod-create`).
By default, the option is set to "false".Setting this flag to "true"
will set the annotation at pod creation time for Taskruns without Sidercars.

Enabling this option should decrease the time it takes for a TaskRun to
start running. However, for clusters that use injected sidecars e.g. istio
enabling this option can lead to unexpected behavior.

Fixes tektoncd#2080

Signed-off-by: Dibyo Mukherjee <[email protected]>
dibyom added a commit to dibyom/pipeline that referenced this issue May 26, 2020
@dibyom
Allow setting sidecar ready annotation early
fb20f5e 
This commit adds an option to a new `config-features` ConfigMap called
`running-in-environment-with-injected-sidecars`.

Enabling this option will decrease the time it takes for a TaskRun to start
running(when no sidecars are present). However, for clusters that use injected
sidecars e.g. istio enabling this option can lead to unexpected behavior.By
default, the option is set to "true" for backwards compatibility.

When no sidercars are present and the cluster does not use injected sidecars,
the pipeline controller can optimize the TaskRun Pod creation process by
setting the `tekton.dev/ready` annotation during pod creation itself instead of
setting it after pod creation.

Fixes tektoncd#2080

Signed-off-by: Dibyo Mukherjee <[email protected]>
dibyom added a commit to dibyom/pipeline that referenced this issue May 28, 2020
@dibyom
Optimize start time for TaskRuns with no sidecars
9e1827b 
When no sidercars are present **and** the cluster does not use
injected sidecars, the pipeline controller can optimize the
TaskRun Pod creation process by setting the `tekton.dev/ready`
annotation before pod creation itself instead of setting it after
the pod has been created.

This commit adds an option to a new `config-features` ConfigMap
called `running-in-environment-with-injected-sidecars`. Enabling
this option will decrease the time it takes for a TaskRun to start
running(when no sidecars are present). However, for clusters that
use injected sidecars e.g. istio enabling this option can lead to
unexpected behavior.By default, the option is set to "true" for
backwards compatibility.

Fixes tektoncd#2080

Signed-off-by: Dibyo Mukherjee <[email protected]>
dibyom added a commit to dibyom/pipeline that referenced this issue May 29, 2020
@dibyom
Optimize start time for TaskRuns with no sidecars
c29ab1d 
When no sidercars are present **and** the cluster does not use
injected sidecars, the pipeline controller can optimize the
TaskRun Pod creation process by setting the `tekton.dev/ready`
annotation before pod creation itself instead of setting it after
the pod has been created.

This commit adds an option to a new `config-features` ConfigMap
called `running-in-environment-with-injected-sidecars`. Enabling
this option will decrease the time it takes for a TaskRun to start
running(when no sidecars are present). However, for clusters that
use injected sidecars e.g. istio enabling this option can lead to
unexpected behavior.By default, the option is set to "true" for
backwards compatibility.

Fixes tektoncd#2080

Signed-off-by: Dibyo Mukherjee <[email protected]>
tekton-robot pushed a commit that referenced this issue Jun 3, 2020
@dibyom @tekton-robot
Optimize start time for TaskRuns with no sidecars
5175c4a 
When no sidercars are present **and** the cluster does not use
injected sidecars, the pipeline controller can optimize the
TaskRun Pod creation process by setting the `tekton.dev/ready`
annotation before pod creation itself instead of setting it after
the pod has been created.

This commit adds an option to a new `config-features` ConfigMap
called `running-in-environment-with-injected-sidecars`. Enabling
this option will decrease the time it takes for a TaskRun to start
running(when no sidecars are present). However, for clusters that
use injected sidecars e.g. istio enabling this option can lead to
unexpected behavior.By default, the option is set to "true" for
backwards compatibility.

Fixes #2080

Signed-off-by: Dibyo Mukherjee <[email protected]>
@dibyom dibyom mentioned this issue Sep 18, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dibyom dibyom

Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Optimize start time for TaskRuns with no sidecars dibyom/pipeline
3 participants
@bobcatfish @dibyom @nishpa214