Skip to content

Commit

Permalink
separate Step from Sidecar
Browse files Browse the repository at this point in the history
separate Step from Sidecar

Originally the Sidecar type is an alias of the Step type.
Both #3013 and #1690 will require the two types to divert from each other.
This commit separates the two types and updates code to accomodate convertListOfSteps() which originally depended on theses types to be the same.
Partially based on: https://github.com/tektoncd/pipeline/pull/3013/files
  • Loading branch information
Peaorl committed Aug 7, 2020
1 parent d9d3621 commit f0a8201
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 8 deletions.
11 changes: 9 additions & 2 deletions pkg/apis/pipeline/v1beta1/task_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,15 @@ type Step struct {
Script string `json:"script,omitempty"`
}

// A sidecar has the same data structure as a Step, consisting of a Container, and optional Script.
type Sidecar = Step
// Sidecar has nearly the same data structure as Step, consisting of a Container and an optional Script, but does not have the ability to timeout.
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"`
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
// TaskList contains a list of Task
Expand Down
19 changes: 18 additions & 1 deletion pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go

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

12 changes: 11 additions & 1 deletion pkg/pod/script.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,17 @@ func convertScripts(shellImage string, steps []v1beta1.Step, sidecars []v1beta1.
}

convertedStepContainers := convertListOfSteps(steps, &placeScriptsInit, &placeScripts, "script")
sidecarContainers := convertListOfSteps(sidecars, &placeScriptsInit, &placeScripts, "sidecar-script")
// As the Sidecar type will divert slightly from the Step type in https://github.com/tektoncd/pipeline/issues/1690 and https://github.com/tektoncd/pipeline/pull/3013,
// while convertListOfSteps operates on overlapping fields, Sidecar is converted into Step by copying the overlapping fields between the types to avoid code duplication
sideCarSteps := []v1beta1.Step{}
for _, step := range sidecars {
sidecarStep := v1beta1.Step{
step.Container,
step.Script,
}
sideCarSteps = append(sideCarSteps, sidecarStep)
}
sidecarContainers := convertListOfSteps(sideCarSteps, &placeScriptsInit, &placeScripts, "sidecar-script")

if placeScripts {
return &placeScriptsInit, convertedStepContainers, sidecarContainers
Expand Down
8 changes: 4 additions & 4 deletions pkg/pod/script_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func TestConvertScripts_NothingToConvert_EmptySidecars(t *testing.T) {
Container: corev1.Container{
Image: "step-2",
},
}}, []v1alpha1.Step{})
}}, []v1alpha1.Sidecar{})
want := []corev1.Container{{
Image: "step-1",
}, {
Expand Down Expand Up @@ -89,7 +89,7 @@ func TestConvertScripts_NothingToConvert_WithSidecar(t *testing.T) {
Container: corev1.Container{
Image: "step-2",
},
}}, []v1alpha1.Step{{
}}, []v1alpha1.Sidecar{}{{
Container: corev1.Container{
Image: "sidecar-1",
},
Expand Down Expand Up @@ -153,7 +153,7 @@ script-3`,
VolumeMounts: preExistingVolumeMounts,
Args: []string{"my", "args"},
},
}}, []v1alpha1.Step{})
}}, []v1alpha1.Sidecar{})
wantInit := &corev1.Container{
Name: "place-scripts",
Image: images.ShellImage,
Expand Down Expand Up @@ -241,7 +241,7 @@ script-3`,
VolumeMounts: preExistingVolumeMounts,
Args: []string{"my", "args"},
},
}}, []v1alpha1.Step{{
}}, []v1alpha1.Sidecar{}{{
Script: `#!/bin/sh
sidecar-1`,
Container: corev1.Container{Image: "sidecar-1"},
Expand Down

0 comments on commit f0a8201

Please sign in to comment.