Skip to content

Commit

Permalink
separate Step from Sidecar
Browse files Browse the repository at this point in the history
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 10, 2020
1 parent 3a7a693 commit f0954c7
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 9 deletions.
5 changes: 4 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@
# Emacs garbage
*~

# Vim garbage
*.swp

# VSCode config
.vscode

Expand All @@ -49,4 +52,4 @@ cmd/*/kodata/source.tar.gz
# binaries
test/pullrequest/pullrequest-init
/.bin/
/bin/
/bin/
12 changes: 10 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,16 @@ 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 embeds the Container type, which allows it to include fields not
// provided by Container.
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")
// convertListOfSteps operates on overlapping fields across Step and Sidecar, hence a conversion
// from Sidecar into Step
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 f0954c7

Please sign in to comment.