Skip to content

Commit

Permalink
Change the type of TaskSpec.Steps to a Step type that embeds Container
Browse files Browse the repository at this point in the history 
- move pkg/merge into v1alpha1 to avoid circular dependency
- move pkg/substitution into v1alpha1 to avoid circular dependency
- change resource interface from Get*ContainerSpec -> *Steps
- change artifact storage interface from GetCopy*Storage*ContainerSpec -> *Steps
- removed some unnecessarily table-driven tests
- squashed whitespace and brackets where possible
  • Loading branch information
@imjasonh
imjasonh committed Aug 13, 2019
1 parent 799457d commit 7092bf8
Show file tree
Hide file tree
Showing 76 changed files with 1,406 additions and 10,523 deletions.
22 changes: 10 additions & 12 deletions pkg/apis/pipeline/v1alpha1/artifact_bucket.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,7 @@ const (
ArtifactStoragePVCType = "pvc"
)

var (
secretVolumeMountPath = "/var/bucketsecret"
)
var secretVolumeMountPath = "/var/bucketsecret"

// ArtifactBucket contains the Storage bucket configuration defined in the
// Bucket config map.
Expand All @@ -75,43 +73,43 @@ func (b *ArtifactBucket) StorageBasePath(pr *PipelineRun) string {
return fmt.Sprintf("%s-%s-bucket", pr.Name, pr.Namespace)
}

// GetCopyFromStorageToContainerSpec returns a container used to download artifacts from temporary storage
func (b *ArtifactBucket) GetCopyFromStorageToContainerSpec(name, sourcePath, destinationPath string) []corev1.Container {
// GetCopyFromStorageToSteps returns a container used to download artifacts from temporary storage
func (b *ArtifactBucket) GetCopyFromStorageToSteps(name, sourcePath, destinationPath string) []Step {
args := []string{"-args", fmt.Sprintf("cp -P -r %s %s", fmt.Sprintf("%s/%s/*", b.Location, sourcePath), destinationPath)}

envVars, secretVolumeMount := getSecretEnvVarsAndVolumeMounts("bucket", secretVolumeMountPath, b.Secrets)

return []corev1.Container{{
return []Step{{Container: corev1.Container{
Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("artifact-dest-mkdir-%s", name)),
Image: *BashNoopImage,
Command: []string{"/ko-app/bash"},
Args: []string{
"-args", strings.Join([]string{"mkdir", "-p", destinationPath}, " "),
},
}, {
}}, {Container: corev1.Container{
Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("artifact-copy-from-%s", name)),
Image: *gsutilImage,
Command: []string{"/ko-app/gsutil"},
Args: args,
Env: envVars,
VolumeMounts: secretVolumeMount,
}}
}}}
}

// GetCopyToStorageFromContainerSpec returns a container used to upload artifacts for temporary storage
func (b *ArtifactBucket) GetCopyToStorageFromContainerSpec(name, sourcePath, destinationPath string) []corev1.Container {
// GetCopyToStorageFromSteps returns a container used to upload artifacts for temporary storage
func (b *ArtifactBucket) GetCopyToStorageFromSteps(name, sourcePath, destinationPath string) []Step {
args := []string{"-args", fmt.Sprintf("cp -P -r %s %s", sourcePath, fmt.Sprintf("%s/%s", b.Location, destinationPath))}

envVars, secretVolumeMount := getSecretEnvVarsAndVolumeMounts("bucket", secretVolumeMountPath, b.Secrets)

return []corev1.Container{{
return []Step{{Container: corev1.Container{
Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("artifact-copy-to-%s", name)),
Image: *gsutilImage,
Command: []string{"/ko-app/gsutil"},
Args: args,
Env: envVars,
VolumeMounts: secretVolumeMount,
}}
}}}
}

// GetSecretsVolumes returns the list of volumes for secrets to be mounted
Expand Down
14 changes: 7 additions & 7 deletions pkg/apis/pipeline/v1alpha1/artifact_bucket_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,38 +46,38 @@ var (
func TestBucketGetCopyFromContainerSpec(t *testing.T) {
names.TestingSeed()

want := []corev1.Container{{
want := []v1alpha1.Step{{Container: corev1.Container{
Name: "artifact-dest-mkdir-workspace-9l9zj",
Image: "override-with-bash-noop:latest",
Command: []string{"/ko-app/bash"},
Args: []string{"-args", "mkdir -p /workspace/destination"},
}, {
}}, {Container: corev1.Container{
Name: "artifact-copy-from-workspace-mz4c7",
Image: "override-with-gsutil-image:latest",
Command: []string{"/ko-app/gsutil"},
Args: []string{"-args", "cp -P -r gs://fake-bucket/src-path/* /workspace/destination"},
Env: []corev1.EnvVar{{Name: "GOOGLE_APPLICATION_CREDENTIALS", Value: fmt.Sprintf("/var/bucketsecret/%s/serviceaccount", secretName)}},
VolumeMounts: []corev1.VolumeMount{{Name: expectedVolumeName, MountPath: fmt.Sprintf("/var/bucketsecret/%s", secretName)}},
}}
}}}

got := bucket.GetCopyFromStorageToContainerSpec("workspace", "src-path", "/workspace/destination")
got := bucket.GetCopyFromStorageToSteps("workspace", "src-path", "/workspace/destination")
if d := cmp.Diff(got, want); d != "" {
t.Errorf("Diff:\n%s", d)
}
}

func TestBucketGetCopyToContainerSpec(t *testing.T) {
names.TestingSeed()
want := []corev1.Container{{
want := []v1alpha1.Step{{Container: corev1.Container{
Name: "artifact-copy-to-workspace-9l9zj",
Image: "override-with-gsutil-image:latest",
Command: []string{"/ko-app/gsutil"},
Args: []string{"-args", "cp -P -r src-path gs://fake-bucket/workspace/destination"},
Env: []corev1.EnvVar{{Name: "GOOGLE_APPLICATION_CREDENTIALS", Value: fmt.Sprintf("/var/bucketsecret/%s/serviceaccount", secretName)}},
VolumeMounts: []corev1.VolumeMount{{Name: expectedVolumeName, MountPath: fmt.Sprintf("/var/bucketsecret/%s", secretName)}},
}}
}}}

got := bucket.GetCopyToStorageFromContainerSpec("workspace", "src-path", "workspace/destination")
got := bucket.GetCopyToStorageFromSteps("workspace", "src-path", "workspace/destination")
if d := cmp.Diff(got, want); d != "" {
t.Errorf("Diff:\n%s", d)
}
Expand Down
35 changes: 16 additions & 19 deletions pkg/apis/pipeline/v1alpha1/artifact_pvc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,36 +47,35 @@ func (p *ArtifactPVC) StorageBasePath(pr *PipelineRun) string {
return pvcDir
}

// GetCopyFromStorageToContainerSpec returns a container used to download artifacts from temporary storage
func (p *ArtifactPVC) GetCopyFromStorageToContainerSpec(name, sourcePath, destinationPath string) []corev1.Container {
return []corev1.Container{{
// GetCopyFromStorageToSteps returns a container used to download artifacts from temporary storage
func (p *ArtifactPVC) GetCopyFromStorageToSteps(name, sourcePath, destinationPath string) []Step {
return []Step{{Container: corev1.Container{
Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("source-copy-%s", name)),
Image: *BashNoopImage,
Command: []string{"/ko-app/bash"},
Args: []string{"-args", strings.Join([]string{"cp", "-r", fmt.Sprintf("%s/.", sourcePath), destinationPath}, " ")},
}}
}}}
}

// GetCopyToStorageFromContainerSpec returns a container used to upload artifacts for temporary storage
func (p *ArtifactPVC) GetCopyToStorageFromContainerSpec(name, sourcePath, destinationPath string) []corev1.Container {
return []corev1.Container{{
// GetCopyToStorageFromSteps returns a container used to upload artifacts for temporary storage
func (p *ArtifactPVC) GetCopyToStorageFromSteps(name, sourcePath, destinationPath string) []Step {
return []Step{{Container: corev1.Container{
Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("source-mkdir-%s", name)),
Image: *BashNoopImage,
Command: []string{"/ko-app/bash"},
Args: []string{

"-args", strings.Join([]string{"mkdir", "-p", destinationPath}, " "),
},
VolumeMounts: []corev1.VolumeMount{GetPvcMount(p.Name)},
}, {
}}, {Container: corev1.Container{
Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("source-copy-%s", name)),
Image: *BashNoopImage,
Command: []string{"/ko-app/bash"},
Args: []string{
"-args", strings.Join([]string{"cp", "-r", fmt.Sprintf("%s/.", sourcePath), destinationPath}, " "),
},
VolumeMounts: []corev1.VolumeMount{GetPvcMount(p.Name)},
}}
}}}
}

// GetPvcMount returns a mounting of the volume with the mount path /pvc
Expand All @@ -87,18 +86,16 @@ func GetPvcMount(name string) corev1.VolumeMount {
}
}

// CreateDirContainer returns a container step to create a dir
func CreateDirContainer(name, destinationPath string) corev1.Container {
return corev1.Container{
// CreateDirStep returns a container step to create a dir
func CreateDirStep(name, destinationPath string) Step {
return Step{Container: corev1.Container{
Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("create-dir-%s", strings.ToLower(name))),
Image: *BashNoopImage,
Command: []string{"/ko-app/bash"},
Args: []string{"-args", strings.Join([]string{"mkdir", "-p", destinationPath}, " ")},
}
}}
}

// GetSecretsVolumes returns the list of volumes for secrets to be mounted
// on pod
func (p *ArtifactPVC) GetSecretsVolumes() []corev1.Volume {
return nil
}
// GetSecretsVolumes returns the list of volumes for secrets to be mounted on
// pod.
func (p *ArtifactPVC) GetSecretsVolumes() []corev1.Volume { return nil }
22 changes: 11 additions & 11 deletions pkg/apis/pipeline/v1alpha1/artifact_pvc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,14 @@ func TestPVCGetCopyFromContainerSpec(t *testing.T) {
pvc := v1alpha1.ArtifactPVC{
Name: "pipelinerun-pvc",
}
want := []corev1.Container{{
want := []v1alpha1.Step{{Container: corev1.Container{
Name: "source-copy-workspace-9l9zj",
Image: "override-with-bash-noop:latest",
Command: []string{"/ko-app/bash"},
Args: []string{"-args", "cp -r src-path/. /workspace/destination"},
}}
}}}

got := pvc.GetCopyFromStorageToContainerSpec("workspace", "src-path", "/workspace/destination")
got := pvc.GetCopyFromStorageToSteps("workspace", "src-path", "/workspace/destination")
if d := cmp.Diff(got, want); d != "" {
t.Errorf("Diff:\n%s", d)
}
Expand All @@ -51,21 +51,21 @@ func TestPVCGetCopyToContainerSpec(t *testing.T) {
pvc := v1alpha1.ArtifactPVC{
Name: "pipelinerun-pvc",
}
want := []corev1.Container{{
want := []v1alpha1.Step{{Container: corev1.Container{
Name: "source-mkdir-workspace-9l9zj",
Image: "override-with-bash-noop:latest",
Command: []string{"/ko-app/bash"},
Args: []string{"-args", "mkdir -p /workspace/destination"},
VolumeMounts: []corev1.VolumeMount{{MountPath: "/pvc", Name: "pipelinerun-pvc"}},
}, {
}}, {Container: corev1.Container{
Name: "source-copy-workspace-mz4c7",
Image: "override-with-bash-noop:latest",
Command: []string{"/ko-app/bash"},
Args: []string{"-args", "cp -r src-path/. /workspace/destination"},
VolumeMounts: []corev1.VolumeMount{{MountPath: "/pvc", Name: "pipelinerun-pvc"}},
}}
}}}

got := pvc.GetCopyToStorageFromContainerSpec("workspace", "src-path", "/workspace/destination")
got := pvc.GetCopyToStorageFromSteps("workspace", "src-path", "/workspace/destination")
if d := cmp.Diff(got, want); d != "" {
t.Errorf("Diff:\n%s", d)
}
Expand All @@ -86,16 +86,16 @@ func TestPVCGetPvcMount(t *testing.T) {
}
}

func TestPVCGetMakeDirContainerSpec(t *testing.T) {
func TestPVCGetMakeStep(t *testing.T) {
names.TestingSeed()

want := corev1.Container{
want := v1alpha1.Step{Container: corev1.Container{
Name: "create-dir-workspace-9l9zj",
Image: "override-with-bash-noop:latest",
Command: []string{"/ko-app/bash"},
Args: []string{"-args", "mkdir -p /workspace/destination"},
}
got := v1alpha1.CreateDirContainer("workspace", "/workspace/destination")
}}
got := v1alpha1.CreateDirStep("workspace", "/workspace/destination")
if d := cmp.Diff(got, want); d != "" {
t.Errorf("Diff:\n%s", d)
}
Expand Down
19 changes: 10 additions & 9 deletions pkg/apis/pipeline/v1alpha1/build_gcs_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,25 +118,26 @@ func (s *BuildGCSResource) Replacements() map[string]string {
}
}

// GetDownloadContainerSpec returns an array of container specs to download gcs storage object
func (s *BuildGCSResource) GetDownloadContainerSpec(sourcePath string) ([]corev1.Container, error) {
// GetDownloadSteps returns an array of container specs to download gcs storage object
func (s *BuildGCSResource) GetDownloadSteps(sourcePath string) ([]Step, error) {
args := []string{"--type", string(s.ArtifactType), "--location", s.Location}
// dest_dir is the destination directory for GCS files to be copies"
if sourcePath != "" {
args = append(args, "--dest_dir", sourcePath)
}

return []corev1.Container{
CreateDirContainer(s.Name, sourcePath), {
return []Step{
CreateDirStep(s.Name, sourcePath),
{Container: corev1.Container{
Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("storage-fetch-%s", s.Name)),
Image: *buildGCSFetcherImage,
Args: args,
}}, nil
}}}, nil
}

// GetUploadContainerSpec gets container spec for gcs resource to be uploaded like
// GetUploadSteps gets container spec for gcs resource to be uploaded like
// set environment variable from secret params and set volume mounts for those secrets
func (s *BuildGCSResource) GetUploadContainerSpec(sourcePath string) ([]corev1.Container, error) {
func (s *BuildGCSResource) GetUploadSteps(sourcePath string) ([]Step, error) {
if s.ArtifactType != GCSManifest {
return nil, xerrors.Errorf("BuildGCSResource: Can only upload Artifacts of type Manifest: %s", s.Name)
}
Expand All @@ -145,11 +146,11 @@ func (s *BuildGCSResource) GetUploadContainerSpec(sourcePath string) ([]corev1.C
}
args := []string{"--location", s.Location, "--dir", sourcePath}

return []corev1.Container{{
return []Step{{Container: corev1.Container{
Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("storage-upload-%s", s.Name)),
Image: *buildGCSUploaderImage,
Args: args,
}}, nil
}}}, nil
}

func getArtifactType(val string) (GCSArtifactType, error) {
Expand Down
Loading

0 comments on commit 7092bf8

Please sign in to comment.