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

Add ContainerState and ContainerName for Sidecars #2075

Merged
merged 1 commit into from
Feb 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
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
10 changes: 6 additions & 4 deletions pkg/apis/pipeline/v1alpha2/taskrun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,18 +173,20 @@ func (tr *TaskRunStatus) SetCondition(newCond *apis.Condition) {
}
}

// StepState reports the results of running a step in the Task.
// StepState reports the results of running a step in a Task.
type StepState struct {
corev1.ContainerState
Name string `json:"name,omitempty"`
ContainerName string `json:"container,omitempty"`
ImageID string `json:"imageID,omitempty"`
}

// SidecarState reports the results of sidecar in the Task.
// SidecarState reports the results of running a sidecar in a Task.
type SidecarState struct {
Name string `json:"name,omitempty"`
ImageID string `json:"imageID,omitempty"`
corev1.ContainerState
Name string `json:"name,omitempty"`
ContainerName string `json:"container,omitempty"`
ImageID string `json:"imageID,omitempty"`
}

// CloudEventDelivery is the target of a cloud event along with the state of
Expand Down
5 changes: 4 additions & 1 deletion pkg/apis/pipeline/v1alpha2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 15 additions & 3 deletions pkg/pod/entrypoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,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 !IsContainerStep(s.Name) && s.State.Running != nil {
for j, c := range newPod.Spec.Containers {
if c.Name == s.Name && c.Image != nopImage {
updated = true
Expand All @@ -212,9 +212,21 @@ func StopSidecars(nopImage string, kubeclient kubernetes.Interface, pod corev1.P
return nil
}

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

return false
}

// isContainerStep returns true if the container name indicates that it
// represents a step.
func isContainerStep(name string) bool { return strings.HasPrefix(name, stepPrefix) }
func IsContainerStep(name string) bool { return strings.HasPrefix(name, stepPrefix) }

// isContainerSidecar returns true if the container name indicates that it
// represents a sidecar.
Expand All @@ -225,4 +237,4 @@ func trimStepPrefix(name string) string { return strings.TrimPrefix(name, stepPr

// trimSidecarPrefix returns the container name, stripped of its sidecar
// prefix.
func trimSidecarPrefix(name string) string { return strings.TrimPrefix(name, sidecarPrefix) }
func TrimSidecarPrefix(name string) string { return strings.TrimPrefix(name, sidecarPrefix) }
18 changes: 10 additions & 8 deletions pkg/pod/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func SidecarsReady(podStatus corev1.PodStatus) bool {
// An injected sidecar might not have the "sidecar-" prefix, so
// we can't just look for that prefix, we need to look at any
// non-step container.
if isContainerStep(s.Name) {
if IsContainerStep(s.Name) {
continue
}
if s.State.Running != nil && s.Ready {
Expand Down Expand Up @@ -112,7 +112,6 @@ func MakeTaskRunStatus(tr v1alpha1.TaskRun, pod *corev1.Pod, taskSpec v1alpha1.T
}

trs.PodName = pod.Name

trs.Steps = []v1alpha1.StepState{}
trs.Sidecars = []v1alpha1.SidecarState{}
logger, _ := logging.NewLogger("", "status")
Expand All @@ -121,7 +120,7 @@ func MakeTaskRunStatus(tr v1alpha1.TaskRun, pod *corev1.Pod, taskSpec v1alpha1.T
}()

for _, s := range pod.Status.ContainerStatuses {
if isContainerStep(s.Name) {
if IsContainerStep(s.Name) {
if s.State.Terminated != nil && len(s.State.Terminated.Message) != 0 {
msg := s.State.Terminated.Message
r, err := termination.ParseMessage(msg)
Expand Down Expand Up @@ -158,8 +157,10 @@ func MakeTaskRunStatus(tr v1alpha1.TaskRun, pod *corev1.Pod, taskSpec v1alpha1.T
})
} else if isContainerSidecar(s.Name) {
trs.Sidecars = append(trs.Sidecars, v1alpha1.SidecarState{
Name: trimSidecarPrefix(s.Name),
ImageID: s.ImageID,
ContainerState: *s.State.DeepCopy(),
Name: TrimSidecarPrefix(s.Name),
ContainerName: s.Name,
ImageID: s.ImageID,
})
}
}
Expand Down Expand Up @@ -196,6 +197,7 @@ func updateCompletedTaskRun(trs *v1alpha1.TaskRunStatus, pod *corev1.Pod) {
Message: "All Steps have completed executing",
})
}

// update tr completed time
trs.CompletionTime = &metav1.Time{Time: time.Now()}
}
Expand Down Expand Up @@ -235,7 +237,7 @@ func updateIncompleteTaskRun(trs *v1alpha1.TaskRunStatus, pod *corev1.Pod) {
func DidTaskRunFail(pod *corev1.Pod) bool {
f := pod.Status.Phase == corev1.PodFailed
for _, s := range pod.Status.ContainerStatuses {
if isContainerStep(s.Name) {
if IsContainerStep(s.Name) {
if s.State.Terminated != nil {
f = f || s.State.Terminated.ExitCode != 0 || isOOMKilled(s)
}
Expand All @@ -247,7 +249,7 @@ func DidTaskRunFail(pod *corev1.Pod) bool {
func areStepsComplete(pod *corev1.Pod) bool {
stepsComplete := len(pod.Status.ContainerStatuses) > 0 && pod.Status.Phase == corev1.PodRunning
for _, s := range pod.Status.ContainerStatuses {
if isContainerStep(s.Name) {
if IsContainerStep(s.Name) {
if s.State.Terminated == nil {
stepsComplete = false
}
Expand Down Expand Up @@ -287,7 +289,7 @@ func getFailureMessage(pod *corev1.Pod) string {
}

for _, s := range pod.Status.ContainerStatuses {
if isContainerStep(s.Name) {
if IsContainerStep(s.Name) {
if s.State.Terminated != nil {
if isOOMKilled(s) {
return oomKilled
Expand Down
108 changes: 106 additions & 2 deletions pkg/pod/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -491,8 +491,112 @@ func TestMakeTaskRunStatus(t *testing.T) {
ContainerName: "step-running-step",
}},
Sidecars: []v1alpha1.SidecarState{{
Name: "running",
ImageID: "image-id",
ContainerState: corev1.ContainerState{
Running: &corev1.ContainerStateRunning{},
},
Name: "running",
ImageID: "image-id",
ContainerName: "sidecar-running",
}},
},
},
}, {
desc: "with-sidecar-waiting",
podStatus: corev1.PodStatus{
Phase: corev1.PodRunning,
ContainerStatuses: []corev1.ContainerStatus{{
Name: "step-waiting-step",
State: corev1.ContainerState{
Waiting: &corev1.ContainerStateWaiting{
Reason: "PodInitializing",
Message: "PodInitializing",
},
},
}, {
Name: "sidecar-waiting",
ImageID: "image-id",
State: corev1.ContainerState{
Waiting: &corev1.ContainerStateWaiting{
Reason: "PodInitializing",
Message: "PodInitializing",
},
},
Ready: true,
}},
},
want: v1alpha1.TaskRunStatus{
Status: duckv1beta1.Status{
Conditions: []apis.Condition{conditionRunning},
},
TaskRunStatusFields: v1alpha1.TaskRunStatusFields{
Steps: []v1alpha1.StepState{{
ContainerState: corev1.ContainerState{
Waiting: &corev1.ContainerStateWaiting{
Reason: "PodInitializing",
Message: "PodInitializing",
},
},
Name: "waiting-step",
ContainerName: "step-waiting-step",
}},
Sidecars: []v1alpha1.SidecarState{{
ContainerState: corev1.ContainerState{
Waiting: &corev1.ContainerStateWaiting{
Reason: "PodInitializing",
Message: "PodInitializing",
},
},
Name: "waiting",
ImageID: "image-id",
ContainerName: "sidecar-waiting",
}},
},
},
}, {
desc: "with-sidecar-terminated",
podStatus: corev1.PodStatus{
Phase: corev1.PodRunning,
ContainerStatuses: []corev1.ContainerStatus{{
Name: "step-running-step",
State: corev1.ContainerState{
Running: &corev1.ContainerStateRunning{},
},
}, {
Name: "sidecar-error",
ImageID: "image-id",
State: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 1,
Reason: "Error",
Message: "Error",
},
},
Ready: true,
}},
},
want: v1alpha1.TaskRunStatus{
Status: duckv1beta1.Status{
Conditions: []apis.Condition{conditionRunning},
},
TaskRunStatusFields: v1alpha1.TaskRunStatusFields{
Steps: []v1alpha1.StepState{{
ContainerState: corev1.ContainerState{
Running: &corev1.ContainerStateRunning{},
},
Name: "running-step",
ContainerName: "step-running-step",
}},
Sidecars: []v1alpha1.SidecarState{{
ContainerState: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 1,
Reason: "Error",
Message: "Error",
},
},
Name: "error",
ImageID: "image-id",
ContainerName: "sidecar-error",
}},
},
},
Expand Down
44 changes: 44 additions & 0 deletions pkg/reconciler/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,13 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
pod, err := c.KubeClientSet.CoreV1().Pods(tr.Namespace).Get(tr.Status.PodName, metav1.GetOptions{})
if err == nil {
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(pod, tr, c)
}
}
} else if errors.IsNotFound(err) {
return merr.ErrorOrNil()
}
Expand Down Expand Up @@ -598,3 +605,40 @@ func getLabelSelector(tr *v1alpha1.TaskRun) string {
}
return strings.Join(labels, ",")
}

// updateStoppedSidecarStatus updates SidecarStatus for sidecars that were
// terminated by nop image
func updateStoppedSidecarStatus(pod *corev1.Pod, tr *v1alpha1.TaskRun, c *Reconciler) error {
tr.Status.Sidecars = []v1alpha1.SidecarState{}
for _, s := range pod.Status.ContainerStatuses {
if !podconvert.IsContainerStep(s.Name) {
var sidecarState corev1.ContainerState
if s.LastTerminationState.Terminated != nil {
// Sidecar has successfully by terminated by nop image
lastTerminatedState := s.LastTerminationState.Terminated
sidecarState = corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
ExitCode: lastTerminatedState.ExitCode,
Reason: "Completed",
Message: "Sidecar container successfully stopped by nop image",
StartedAt: lastTerminatedState.StartedAt,
FinishedAt: lastTerminatedState.FinishedAt,
ContainerID: lastTerminatedState.ContainerID,
},
}
} else {
// Sidecar has not been terminated
sidecarState = s.State
}

tr.Status.Sidecars = append(tr.Status.Sidecars, v1alpha1.SidecarState{
ContainerState: *sidecarState.DeepCopy(),
Name: podconvert.TrimSidecarPrefix(s.Name),
ContainerName: s.Name,
ImageID: s.ImageID,
})
}
}
_, err := c.updateStatus(tr)
return err
}
Loading