Skip to content

Commit

Permalink
Sidecar container names prefixed with sidecar
Browse files Browse the repository at this point in the history
Prefix all sidecar container names with 'sidecar-'
Counting, stopping and status collection of sidecar status will
all look at the container name to determine if the operation applies.
  • Loading branch information
fraenkel committed Nov 5, 2019
1 parent bb7d0b7 commit 55debdb
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 18 deletions.
16 changes: 14 additions & 2 deletions pkg/reconciler/taskrun/resources/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ const (
workingDirInit = "working-dir-initializer"
ReadyAnnotation = "tekton.dev/ready"
readyAnnotationValue = "READY"
sidecarPrefix = "sidecar-"
)

func makeCredentialInitializer(credsImage, serviceAccountName, namespace string, kubeclient kubernetes.Interface) (*v1alpha1.Step, []corev1.Volume, error) {
Expand Down Expand Up @@ -381,8 +382,9 @@ cat > ${tmpfile} << '%s'
for _, s := range mergedPodSteps {
mergedPodContainers = append(mergedPodContainers, s.Container)
}
if len(taskSpec.Sidecars) > 0 {
mergedPodContainers = append(mergedPodContainers, taskSpec.Sidecars...)
for _, sc := range taskSpec.Sidecars {
sc.Name = names.SimpleNameGenerator.RestrictLength(fmt.Sprintf("%v%v", sidecarPrefix, sc.Name))
mergedPodContainers = append(mergedPodContainers, sc)
}

return &corev1.Pod{
Expand Down Expand Up @@ -439,6 +441,10 @@ func IsContainerStep(name string) bool {
return strings.HasPrefix(name, containerPrefix)
}

func IsContainerSidecar(name string) bool {
return strings.HasPrefix(name, sidecarPrefix)
}

// makeLabels constructs the labels we will propagate from TaskRuns to Pods.
func makeLabels(s *v1alpha1.TaskRun) map[string]string {
labels := make(map[string]string, len(s.ObjectMeta.Labels)+1)
Expand Down Expand Up @@ -512,3 +518,9 @@ func findMaxResourceRequest(steps []v1alpha1.Step, resourceNames ...corev1.Resou
func TrimContainerNamePrefix(containerName string) string {
return strings.TrimPrefix(containerName, containerPrefix)
}

// TrimSidecarNamePrefix trim the sidecar name prefix to get the corresponding
// sidecar name
func TrimSidecarNamePrefix(containerName string) string {
return strings.TrimPrefix(containerName, sidecarPrefix)
}
4 changes: 2 additions & 2 deletions pkg/reconciler/taskrun/resources/pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ func TestMakePod(t *testing.T) {
Image: "primary-image",
}}},
Sidecars: []corev1.Container{{
Name: "sidecar-name",
Name: "sc-name",
Image: "sidecar-image",
}},
},
Expand Down Expand Up @@ -463,7 +463,7 @@ func TestMakePod(t *testing.T) {
},
},
}, {
Name: "sidecar-name",
Name: "sidecar-sc-name",
Image: "sidecar-image",
Resources: corev1.ResourceRequirements{
Requests: nil,
Expand Down
3 changes: 2 additions & 1 deletion pkg/reconciler/taskrun/sidecars/stop.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package sidecars

import (
"github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
Expand All @@ -37,7 +38,7 @@ func Stop(pod *corev1.Pod, nopImage string, updatePod UpdatePod) error {
updated := false
if pod.Status.Phase == corev1.PodRunning {
for _, s := range pod.Status.ContainerStatuses {
if s.State.Running != nil {
if resources.IsContainerSidecar(s.Name) && s.State.Running != nil {
for j, c := range pod.Spec.Containers {
if c.Name == s.Name && c.Image != nopImage {
updated = true
Expand Down
6 changes: 3 additions & 3 deletions pkg/reconciler/taskrun/sidecars/stop_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestStop(t *testing.T) {
description: "a running sidecar container should be stopped",
podPhase: corev1.PodRunning,
sidecarContainer: corev1.Container{
Name: "echo-hello",
Name: "sidecar-echo-hello",
Image: "echo-hello:latest",
Command: []string{"echo"},
Args: []string{"hello"},
Expand All @@ -56,7 +56,7 @@ func TestStop(t *testing.T) {
description: "a pending pod should not have its sidecars stopped",
podPhase: corev1.PodPending,
sidecarContainer: corev1.Container{
Name: "echo-hello",
Name: "sidecar-echo-hello",
Image: "echo-hello:latest",
Command: []string{"echo"},
Args: []string{"hello"},
Expand All @@ -69,7 +69,7 @@ func TestStop(t *testing.T) {
description: "a sidecar container that is not in a running state should not be stopped",
podPhase: corev1.PodRunning,
sidecarContainer: corev1.Container{
Name: "echo-hello",
Name: "sidecar-echo-hello",
Image: "echo-hello:latest",
Command: []string{"echo"},
Args: []string{"hello"},
Expand Down
6 changes: 3 additions & 3 deletions pkg/status/taskrunpod.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ func UpdateStatusFromPod(taskRun *v1alpha1.TaskRun, pod *corev1.Pod, resourceLis
ContainerName: s.Name,
ImageID: s.ImageID,
})
} else {
} else if resources.IsContainerSidecar(s.Name) {
taskRun.Status.Sidecars = append(taskRun.Status.Sidecars, v1alpha1.SidecarState{
Name: s.Name,
Name: resources.TrimSidecarNamePrefix(s.Name),
ImageID: s.ImageID,
})
}
Expand Down Expand Up @@ -151,7 +151,7 @@ func areStepsComplete(pod *corev1.Pod) bool {

func countSidecars(pod *corev1.Pod) (total int, readyOrTerminated int) {
for _, s := range pod.Status.ContainerStatuses {
if !resources.IsContainerStep(s.Name) {
if resources.IsContainerSidecar(s.Name) {
if s.State.Running != nil && s.Ready {
readyOrTerminated++
} else if s.State.Terminated != nil {
Expand Down
12 changes: 6 additions & 6 deletions pkg/status/taskrunpod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ func TestUpdateStatusFromPod(t *testing.T) {
Sidecars: []v1alpha1.SidecarState{},
},
}, {
desc: "with-running-sidecar",
desc: "with-sidecar-running",
podStatus: corev1.PodStatus{
Phase: corev1.PodRunning,
ContainerStatuses: []corev1.ContainerStatus{
Expand All @@ -387,7 +387,7 @@ func TestUpdateStatusFromPod(t *testing.T) {
},
},
{
Name: "running-sidecar",
Name: "sidecar-running",
ImageID: "image-id",
State: corev1.ContainerState{
Running: &corev1.ContainerStateRunning{},
Expand All @@ -408,7 +408,7 @@ func TestUpdateStatusFromPod(t *testing.T) {
ContainerName: "step-running-step",
}},
Sidecars: []v1alpha1.SidecarState{{
Name: "running-sidecar",
Name: "running",
ImageID: "image-id",
}},
},
Expand Down Expand Up @@ -503,7 +503,7 @@ func TestCountSidecars(t *testing.T) {
},
},
}, {
Name: "stopped-sidecar-baz",
Name: "sidecar-stopped-baz",
State: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 99,
Expand All @@ -518,7 +518,7 @@ func TestCountSidecars(t *testing.T) {
statuses: []corev1.ContainerStatus{
{Name: "step-ignore-me"},
{
Name: "ready-sidecar",
Name: "sidecar-ready",
Ready: true,
State: corev1.ContainerState{
Running: &corev1.ContainerStateRunning{
Expand All @@ -527,7 +527,7 @@ func TestCountSidecars(t *testing.T) {
},
},
{
Name: "unready-sidecar",
Name: "sidecar-unready",
State: corev1.ContainerState{
Running: &corev1.ContainerStateRunning{
StartedAt: metav1.NewTime(time.Now()),
Expand Down
2 changes: 1 addition & 1 deletion test/sidecar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func TestSidecarTaskSupport(t *testing.T) {
primaryTerminated = true
}
}
if c.Name == sidecarContainerName {
if c.Name == fmt.Sprintf("sidecar-%s", sidecarContainerName) {
if c.State.Terminated == nil {
t.Errorf("Sidecar container has a nil Terminated status but non-nil is expected.")
} else {
Expand Down

0 comments on commit 55debdb

Please sign in to comment.