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

Fixes: #1131, Enable graceful sidecar kill #3013

Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
make it a config per sidecar
  • Loading branch information
Shahnewaz Rifat committed Jul 16, 2020
commit a44ac03f9a5cd035ff9640c454e68c23383d92aa
8 changes: 4 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@ require (
golang.org/x/sys v0.0.0-20200625212154-ddb9806d33ae // indirect
golang.org/x/text v0.3.3 // indirect
golang.org/x/time v0.0.0-20200630173020-3af7569d3a1e // indirect
golang.org/x/tools v0.0.0-20200714190737-9048b464a08d // indirect
golang.org/x/tools v0.0.0-20200715235423-130c9f19d3fe // indirect
gomodules.xyz/jsonpatch/v2 v2.1.0
google.golang.org/protobuf v1.25.0 // indirect
gopkg.in/ini.v1 v1.57.0 // indirect
gopkg.in/yaml.v2 v2.3.0 // indirect
k8s.io/api v0.18.5
k8s.io/apimachinery v0.18.5
k8s.io/cli-runtime v0.18.5 // indirect
k8s.io/api v0.18.6
k8s.io/apimachinery v0.18.6
k8s.io/cli-runtime v0.18.6 // indirect
k8s.io/client-go v11.0.1-0.20190805182717-6502b5e7b1b5+incompatible
k8s.io/code-generator v0.18.0
k8s.io/kube-openapi v0.0.0-20200615155156-dffdd1682719
Expand Down
4 changes: 4 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1523,6 +1523,8 @@ golang.org/x/tools v0.0.0-20200512131952-2bc93b1c0c88/go.mod h1:EkVYQZoAsY45+roY
golang.org/x/tools v0.0.0-20200527183253-8e7acdbce89d/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
golang.org/x/tools v0.0.0-20200714190737-9048b464a08d h1:hYhnolbefSSt3WZp66sgmgnEOFv5PD6a5PIcnKJ8jdU=
golang.org/x/tools v0.0.0-20200714190737-9048b464a08d/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA=
golang.org/x/tools v0.0.0-20200715235423-130c9f19d3fe h1:q+DqTBqwpg3C8p5kTZ0WSLOmLxH57dKhWAieWmuFv7M=
golang.org/x/tools v0.0.0-20200715235423-130c9f19d3fe/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
Expand Down Expand Up @@ -1717,6 +1719,8 @@ k8s.io/cli-runtime v0.17.3 h1:0ZlDdJgJBKsu77trRUynNiWsRuAvAVPBNaQfnt/1qtc=
k8s.io/cli-runtime v0.17.3/go.mod h1:X7idckYphH4SZflgNpOOViSxetiMj6xI0viMAjM81TA=
k8s.io/cli-runtime v0.18.5 h1:Wr2hTNFlrb9smNqTGCfJHd21F9j0rh3jzB8Pkn0h2RE=
k8s.io/cli-runtime v0.18.5/go.mod h1:uS210tk6ngtwwIJctPLs4ul1r7XlrEtwh9dA1oB700A=
k8s.io/cli-runtime v0.18.6 h1:I8BkH5NyqMQ4zqUBmpXJ1LxIqpCH88H/1edPkPVWzjQ=
k8s.io/cli-runtime v0.18.6/go.mod h1:+G/WTNqHgUv636e5y7rhOQ7epUbRXnwmPnhOhD6t9uM=
k8s.io/client-go v0.17.6 h1:W/JkbAcIZUPb9vENRTC75ymjQQO3qEJAZyYhOIEOifM=
k8s.io/client-go v0.17.6/go.mod h1:tX5eAbQR/Kbqv+5R93rzHQoyRnPjjW2mm9i0lXnW218=
k8s.io/cloud-provider v0.17.0/go.mod h1:Ze4c3w2C0bRsjkBUoHpFi+qWe3ob1wI2/7cUn+YQIDE=
Expand Down
12 changes: 11 additions & 1 deletion pkg/apis/pipeline/v1beta1/task_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,17 @@ type Step struct {
}

// A sidecar has the same data structure as a Step, consisting of a Container, and optional Script.
type Sidecar = Step
type Sidecar struct {
corev1.Container `json:",inline"`

// Script is the contents of an executable file to execute.
//
// If Script is not empty, the Step cannot have an Command or Args.
Script string `json:"script,omitempty"`
// ForceSidecarTermination is a bool flag to toggle sidecar termination when all the steps in the task are completed.
// +optional
ForceTermination bool `json:"forceTermination,omitempty"`
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
// TaskList contains a list of Task
Expand Down
3 changes: 0 additions & 3 deletions pkg/apis/pipeline/v1beta1/taskrun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,6 @@ type TaskRunSpec struct {
// Workspaces is a list of WorkspaceBindings from volumes to workspaces.
// +optional
Workspaces []WorkspaceBinding `json:"workspaces,omitempty"`
// ForceSidecarTermination is a bool flag to toggle sidecar termination when all the steps in the task are completed.
// +optional
ForceSidecarTermination bool `json:"forceSidecarTermination,omitempty"`
}

// TaskRunSpecStatus defines the taskrun spec status the user can provide
Expand Down
13 changes: 6 additions & 7 deletions pkg/pod/entrypoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,9 @@ func UpdateReady(kubeclient kubernetes.Interface, pod corev1.Pod) error {
return nil
}

// StopSidecars updates sidecar containers in the Pod to a nop image, which
// StopSidecar updates sidecar container in the Pod to a nop image, which
// exits successfully immediately.
func StopSidecars(nopImage string, kubeclient kubernetes.Interface, pod corev1.Pod) error {
func StopSidecar(nopImage string, kubeclient kubernetes.Interface, pod corev1.Pod, sideCarContainerName string) error {
newPod, err := kubeclient.CoreV1().Pods(pod.Namespace).Get(pod.Name, metav1.GetOptions{})
if err != nil {
return fmt.Errorf("error getting Pod %q when stopping sidecars: %w", pod.Name, err)
Expand All @@ -195,7 +195,7 @@ func StopSidecars(nopImage string, kubeclient kubernetes.Interface, pod corev1.P
// An injected sidecar container might not have the
// "sidecar-" prefix, so we can't just look for that
// prefix.
if !IsContainerStep(s.Name) && s.State.Running != nil {
if s.Name == sideCarContainerName && s.State.Running != nil {
for j, c := range newPod.Spec.Containers {
if c.Name == s.Name && c.Image != nopImage {
updated = true
Expand All @@ -213,15 +213,14 @@ func StopSidecars(nopImage string, kubeclient kubernetes.Interface, pod corev1.P
return nil
}

// IsSidecarStatusRunning determines if any SidecarStatus on a TaskRun
// IsSidecarStatusRunning determines if SidecarStatus for a particular sidecar on a TaskRun
// is still running.
func IsSidecarStatusRunning(tr *v1beta1.TaskRun) bool {
func IsSidecarStatusRunning(tr *v1beta1.TaskRun, sidecarName string) bool {
for _, sidecar := range tr.Status.Sidecars {
if sidecar.Terminated == nil {
if sidecar.Name == sidecarName && sidecar.Terminated == nil {
return true
}
}

return false
}

Expand Down
28 changes: 16 additions & 12 deletions pkg/reconciler/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1beta1.TaskRun) pkg
afterCondition := tr.Status.GetCondition(apis.ConditionSucceeded)
events.Emit(ctx, nil, afterCondition, tr)
}

// If the TaskRun is complete, run some post run fixtures when applicable
if tr.IsDone() {
logger.Infof("taskrun done : %s \n", tr.Name)
Expand All @@ -125,15 +124,20 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1beta1.TaskRun) pkg
}
c.timeoutHandler.Release(tr)
pod, err := c.KubeClientSet.CoreV1().Pods(tr.Namespace).Get(tr.Status.PodName, metav1.GetOptions{})
if err == nil && tr.Spec.ForceSidecarTermination == false {
err = podconvert.StopSidecars(c.Images.NopImage, c.KubeClientSet, *pod)
if err == nil {
// Check if any SidecarStatuses are still shown as Running after stopping
// Sidecars. If any Running, update SidecarStatuses based on Pod ContainerStatuses.
if podconvert.IsSidecarStatusRunning(tr) {
err = updateStoppedSidecarStatus(ctx, pod, tr, c)

if err == nil {
for _, sidecar := range tr.Spec.TaskSpec.Sidecars {
if sidecar.ForceTermination == false {
continue
}
err = podconvert.StopSidecar(c.Images.NopImage, c.KubeClientSet, *pod, sidecar.Name)
if err == nil {
if podconvert.IsSidecarStatusRunning(tr, sidecar.Name) {
err = updateStoppedSidecarStatus(sidecar.Name, pod, tr)
}
}
}

} else if k8serrors.IsNotFound(err) {
return merr.ErrorOrNil()
}
Expand Down Expand Up @@ -636,15 +640,15 @@ func getLabelSelector(tr *v1beta1.TaskRun) string {
return strings.Join(labels, ",")
}

// updateStoppedSidecarStatus updates SidecarStatus for sidecars that were
// updateStoppedSidecarStatus updates SidecarStatus for a particular sidecar that were
// terminated by nop image
func updateStoppedSidecarStatus(ctx context.Context, pod *corev1.Pod, tr *v1beta1.TaskRun, c *Reconciler) error {
func updateStoppedSidecarStatus(sidecarName string, pod *corev1.Pod, tr *v1beta1.TaskRun) error {
tr.Status.Sidecars = []v1beta1.SidecarState{}
for _, s := range pod.Status.ContainerStatuses {
if !podconvert.IsContainerStep(s.Name) {
if s.Name == sidecarName {
var sidecarState corev1.ContainerState
if s.LastTerminationState.Terminated != nil {
// Sidecar has successfully by terminated by nop image
// Sidecar has been successfully terminated by nop image
lastTerminatedState := s.LastTerminationState.Terminated
sidecarState = corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
Expand Down