-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
TL;DR Tekton Pipelines now gracefully start and stop with sidecars running alongside TaskRun pods. Long Version :- TaskRun pods can have sidecars injected by Admission Controllers. This is common practice with Istio where a proxy sidecar is injected into pods so that they can be included as part of a service mesh. Unfortunately there is no built-in Kubernetes mechanism to set the lifetime of a sidecar container to match that of the pod's "primary" container. In practice this means that pods injected with sidecars will not stop until both the primary and all sidecar containers are stopped. Prior to this commit injected sidecars would cause a TaskRun pod to run forever, even when all Step containers of a Task had completed. This commit introduces mechanisms to start and stop sidecars gracefully meaning that a) Step containers wait until all sidecars are in a ready state before starting and b) Sidecar containers are stopped when a TaskRun's Step containers are done. No end-to-end tests have been added as part of this PR because doing so would require Istio to be enabled on the CI cluster (or a dummy Admission Controller / Mutating Webhook thing to be written just for injecting containers into test Pods). The Istio requirement would put an undue burden on those users running tests in non-GKE environments. It's currently planned for a follow-up PR to introduce a) user-defined sidecars and b) a Tekton-injected sidecar that performs logging. Once that work is complete it will be much simpler to e2e test this sidecar code. Further to the above, a number of smaller refactors have taken place in this PR: - a sidecars package has been created to encapsulate the logic for stopping sidecars with the nop container image - the updateStatusFromPod func has been moved into a taskrun/status package and broken down into constituent helpers - a small amount of dead code has been eliminated Co-authored-by: Jose Blas Camacho Taboada <[email protected]> Co-authored-by: Scott Seaward <[email protected]>
- Loading branch information
1 parent
3dfea62
commit ace3ab6
Showing
20 changed files
with
1,301 additions
and
722 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| package main | ||
|
|
||
| import ( | ||
| "io/ioutil" | ||
| "os" | ||
| "testing" | ||
| "time" | ||
| ) | ||
|
|
||
| func TestRealWaiterWaitMissingFile(t *testing.T) { | ||
| // Create a temp file and then immediately delete it to get | ||
| // a legitimate tmp path and ensure the file doesnt exist | ||
| // prior to testing Wait(). | ||
| tmp, err := ioutil.TempFile("", "real_waiter_test_file") | ||
| if err != nil { | ||
| t.Errorf("error creating temp file: %v", err) | ||
| } | ||
| os.Remove(tmp.Name()) | ||
| rw := RealWaiter{} | ||
| doneCh := make(chan struct{}) | ||
| go func() { | ||
| err := rw.Wait(tmp.Name(), false) | ||
| if err != nil { | ||
| t.Errorf("error waiting on tmp file %q", tmp.Name()) | ||
| } | ||
| close(doneCh) | ||
| }() | ||
| select { | ||
| case <-doneCh: | ||
| t.Errorf("did not expect Wait() to have detected a file at path %q", tmp.Name()) | ||
| case <-time.After(2 * waitPollingInterval): | ||
| // Success | ||
| } | ||
| } | ||
|
|
||
| func TestRealWaiterWaitWithFile(t *testing.T) { | ||
| tmp, err := ioutil.TempFile("", "real_waiter_test_file") | ||
| if err != nil { | ||
| t.Errorf("error creating temp file: %v", err) | ||
| } | ||
| defer os.Remove(tmp.Name()) | ||
| rw := RealWaiter{} | ||
| doneCh := make(chan struct{}) | ||
| go func() { | ||
| err := rw.Wait(tmp.Name(), false) | ||
| if err != nil { | ||
| t.Errorf("error waiting on tmp file %q", tmp.Name()) | ||
| } | ||
| close(doneCh) | ||
| }() | ||
| select { | ||
| case <-doneCh: | ||
| // Success | ||
| case <-time.After(2 * waitPollingInterval): | ||
| t.Errorf("expected Wait() to have detected the file's existence by now") | ||
| } | ||
| } | ||
|
|
||
| func TestRealWaiterWaitMissingContent(t *testing.T) { | ||
| tmp, err := ioutil.TempFile("", "real_waiter_test_file") | ||
| if err != nil { | ||
| t.Errorf("error creating temp file: %v", err) | ||
| } | ||
| defer os.Remove(tmp.Name()) | ||
| rw := RealWaiter{} | ||
| doneCh := make(chan struct{}) | ||
| go func() { | ||
| err := rw.Wait(tmp.Name(), true) | ||
| if err != nil { | ||
| t.Errorf("error waiting on tmp file %q", tmp.Name()) | ||
| } | ||
| close(doneCh) | ||
| }() | ||
| select { | ||
| case <-doneCh: | ||
| t.Errorf("no data was written to tmp file, did not expect Wait() to have detected a non-zero file size and returned") | ||
| case <-time.After(2 * waitPollingInterval): | ||
| // Success | ||
| } | ||
| } | ||
|
|
||
| func TestRealWaiterWaitWithContent(t *testing.T) { | ||
| tmp, err := ioutil.TempFile("", "real_waiter_test_file") | ||
| if err != nil { | ||
| t.Errorf("error creating temp file: %v", err) | ||
| } | ||
| defer os.Remove(tmp.Name()) | ||
| rw := RealWaiter{} | ||
| doneCh := make(chan struct{}) | ||
| go func() { | ||
| err := rw.Wait(tmp.Name(), true) | ||
| if err != nil { | ||
| t.Errorf("error waiting on tmp file %q", tmp.Name()) | ||
| } | ||
| close(doneCh) | ||
| }() | ||
| if err := ioutil.WriteFile(tmp.Name(), []byte("😺"), 0700); err != nil { | ||
| t.Errorf("error writing content to temp file: %v", err) | ||
| } | ||
| select { | ||
| case <-doneCh: | ||
| // Success | ||
| case <-time.After(2 * waitPollingInterval): | ||
| t.Errorf("expected Wait() to have detected a non-zero file size by now") | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.