Merge branch 'master' of github.com:deliveroo/paddle into PRIC-1537_A…

…llow_pipeline_command_to_accept_list_of_files

README.md

@@ -44,22 +44,68 @@ region=eu-west-1
 $ go build
 ```
 
-## Testing
+## Running Unit Tests
 
 ```
 $ go test ./...
 ```
 
+## Testing on Staging
+There is no dedicated test environment in staging for Paddle, but it is possible to test in staging using another Canoe pipeline that uses Paddle.
+
+1. Download and setup a pipeline repo that can be tested in staging, for example [`rider-planning-pipeline`](https://github.com/deliveroo/rider-planning-pipeline). Make sure you can build at least one of the pipeline steps listed in the definition `yml` file. The step should also contain inputs from a previous step if the paddle `get` command needs to be tested.
+2. Modify the paddle template.go `podTemplate` to use a temporary paddlecontainer tag. e.g `localtest001`. The `latest` tag cannot be used as it is pulled into production images.
+    ```
+    ...
+    image: "219541440308.dkr.ecr.eu-west-1.amazonaws.com/paddlecontainer:localtest001"
+    ...
+    ```
+3. Copy the paddle build that needs to be tested to root of pipeline repo. Build using:
+    ```
+    go build
+    ```
+4. Copy a linux+amd64 paddle build that needs to be tested to the root of a new directory. Build for linux+amd64 using:
+    ```
+    GOOS=linux GOARCH=amd64 go build
+    ```
+5. Create a new Dockerfile in the same directory with the following content:
+    ```
+    FROM ubuntu
+
+    RUN apt-get update
+    RUN apt-get install -y ca-certificates curl wget
+
+    COPY ./paddle /usr/bin/paddle
+    ```
+6. From a shell, build and push a Docker image using the Dockerfile. Ensure local AWS credentials are setup to enable login.
+
+    **The image _must_ be built using a tag other than `latest` otherwise the image used in production will be overwritten!** In the example below, `localtest001` has been used to match the change made above.
+    ```
+    $(aws ecr get-login --profile k8s_production --no-include-email --region eu-west-1)
+
+    docker build -t 219541440308.dkr.ecr.eu-west-1.amazonaws.com/paddlecontainer:localtest001 .
+
+    docker push 219541440308.dkr.ecr.eu-west-1.amazonaws.com/paddlecontainer:localtest001
+    ```
+7. Modify the testing step(s) in the pipeline `yml` file with the test paddle container tag e.g. `localtest001` and make any other amendements as necessary.
+8. Re-run the pipeline to test the new version of paddle.
+9. If successful, follow the process below to release a new version of paddle.
+
 ## Release
 
 In order to release a new version, set up github export GITHUB_TOKEN=[YOUR_TOKEN]. Make sure that you have goreleaser installed from [goreleaser.com](http://goreleaser.com).
 
-Ensure your git repo is clean. Then update VERSION (no need to commit it, it will be committed automatically), and run:
+Ensure your git repo is clean, new PRs have already merged and your branch is set to master. Then update VERSION (no need to commit it, it will be committed automatically), and run:
 
 ```
 $ ./release.sh
 ```
 
+If the final `goreleaser` part of this process fails (e.g. due to a missing GITHUB_TOKEN or `goreleaser` install), the version will have likely been incremented and pushed to origin already. In this case, run the final `goreleaser` step manually:
+```
+goreleaser --rm-dist
+```
+
 ## Usage
 
 ```

VERSION

@@ -1 +1 @@
-0.14.0
+0.17.0

cli/data/get.go

@@ -36,6 +36,7 @@ var (
 	getCommitPath string
 	getBucket     string
 	getKeys       []string
+	getSubdir     string
 )
 
 const (
@@ -45,14 +46,14 @@ const (
 )
 
 var getCmd = &cobra.Command{
-	Use:   "get [version] [destination path]",
+	Use:   "get [step/version] [destination path]",
 	Short: "Fetch data from S3",
 	Args:  cobra.ExactArgs(2),
 	Long: `Fetch data from a S3 versioned path.
 
 Example:
 
-$ paddle data get -b experimental --bucket roo-pipeline trained-model/version1 dest/path
+$ paddle data get -b experimental --bucket roo-pipeline --subdir version1 trained-model/version1 dest/path
 $ paddle data get -b experimental --bucket roo-pipeline --keys file1.csv,file2.csv trained-model/version1 dest/path
 `,
 	Run: func(cmd *cobra.Command, args []string) {
@@ -68,7 +69,7 @@ $ paddle data get -b experimental --bucket roo-pipeline --keys file1.csv,file2.c
 			path:   fmt.Sprintf("%s/%s/%s", args[0], getBranch, getCommitPath),
 		}
 
-		copyPathToDestination(source, args[1], getKeys)
+		copyPathToDestination(source, args[1], getKeys, getSubdir)
 	},
 }
 
@@ -77,9 +78,10 @@ func init() {
 	getCmd.Flags().StringVar(&getBucket, "bucket", "", "Bucket to use")
 	getCmd.Flags().StringVarP(&getCommitPath, "path", "p", "HEAD", "Path to fetch (instead of HEAD)")
 	getCmd.Flags().StringSliceVarP(&getKeys, "keys", "k", []string{}, "A list of keys to download separated by comma")
+	getCmd.Flags().StringVarP(&getSubdir, "subdir", "d", "", "Custom subfolder name for export path")
 }
 
-func copyPathToDestination(source S3Path, destination string, keys []string) {
+func copyPathToDestination(source S3Path, destination string, keys []string, subdir string) {
 	session := session.Must(session.NewSessionWithOptions(session.Options{
 		SharedConfigState: session.SharedConfigEnable,
 	}))
@@ -94,6 +96,9 @@ func copyPathToDestination(source S3Path, destination string, keys []string) {
 	if !strings.HasSuffix(source.path, "/") {
 		source.path += "/"
 	}
+	if subdir != "" {
+		destination = parseDestination(destination, subdir)
+	}
 
 	fmt.Println("Copying " + source.path + " to " + destination)
 	copy(session, source, destination, keys)
@@ -113,6 +118,15 @@ func readHEAD(session *session.Session, source S3Path) string {
 	return buf.String()
 }
 
+func parseDestination(destination string, subdir string) string {
+	if !strings.HasSuffix(destination, "/") {
+		destination += "/" + subdir
+	} else {
+		destination += subdir
+	}
+	return destination
+}
+
 func copy(session *session.Session, source S3Path, destination string, keys []string) {
 	query := &s3.ListObjectsV2Input{
 		Bucket: aws.String(source.bucket),

cli/pipeline/pipeline_definition.go

@@ -14,12 +14,13 @@ type PipelineDefinitionStep struct {
 	Branch  string `yaml:"branch" json:"branch"`
 	Image   string `yaml:"image" json:"image"`
 	Inputs  []struct {
-		Step    string   `yaml:"step" json:"step"`
-		Version string   `yaml:"version" json:"version"`
-		Branch  string   `yaml:"branch" json:"branch"`
-		Path    string   `yaml:"path" json:"path"`
-		Bucket  string   `yaml:"bucket" json:"bucket"`
+		Step    string `yaml:"step" json:"step"`
+		Version string `yaml:"version" json:"version"`
+		Branch  string `yaml:"branch" json:"branch"`
+		Path    string `yaml:"path" json:"path"`
+		Bucket  string `yaml:"bucket" json:"bucket"`
 		Keys    []string `yaml:"keys" json:"keys"`
+		Subdir  string `yaml:"subdir" json:"subdir"`
 	} `yaml:"inputs" json:"inputs"`
 	Commands  []string `yaml:"commands" json:"commands"`
 	Resources struct {

cli/pipeline/pipeline_definition_test.go

@@ -13,12 +13,17 @@ func TestParsePipeline(t *testing.T) {
 	pipeline := ParsePipeline(data)
 
 	if len(pipeline.Steps) != 2 {
-		t.Errorf("excepted two steps, got %d", len(pipeline.Steps))
+		t.Errorf("expected two steps, got %d", len(pipeline.Steps))
 	}
 
 	if pipeline.Bucket != "canoe-sample-pipeline" {
 		t.Errorf("Expected bucket to be canoe-sample-pipeline, got %s", pipeline.Bucket)
 	}
+
+	subdir := pipeline.Steps[1].Inputs[0].Subdir
+	if subdir != "step1-version1" {
+		t.Errorf("expected second step input subdir to be 'step1-version1' but got %s", subdir)
+	}
 }
 
 func TestOverrideTag(t *testing.T) {

cli/pipeline/run_test.go

@@ -2,15 +2,16 @@ package pipeline
 
 import (
 	"fmt"
-	"k8s.io/api/core/v1"
+	"testing"
+	"time"
+
+	v1 "k8s.io/api/core/v1"
 	k8errors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/watch"
 	"k8s.io/client-go/kubernetes/fake"
 	ktesting "k8s.io/client-go/testing"
-	"testing"
-	"time"
 )
 
 func parseTimeOrDie(ts string) metav1.Time {
@@ -112,11 +113,11 @@ func TestRunPipelineSuccess(t *testing.T) {
 	expectPods := [2]string{"sample-steps-passing-version1-step1-master", "sample-steps-passing-version1a-step2-master"}
 
 	for _, p := range expectPods {
-		if deleted[p] != 2 {
-			t.Errorf("excepted delete of "+p+" to be called twice, got %i", deleted[p])
+		if deleted[p] != 3 {
+			t.Errorf("expected delete of "+p+" to be called three times, got %i", deleted[p])
 		}
 		if created[p] != 1 {
-			t.Errorf("excepted create of "+p+" to be called once, got %i", created[p])
+			t.Errorf("expected create of "+p+" to be called once, got %i", created[p])
 		}
 	}
 }
@@ -162,7 +163,7 @@ func TestRunPipelineFailure(t *testing.T) {
 	runPipeline("test/sample_steps_passing.yml", testRunFlags)
 
 	if len(errors) != 2 {
-		t.Errorf("excepted two errors, actual %v", len(errors))
+		t.Errorf("expected two errors, actual %v", len(errors))
 	}
 }
 
@@ -205,7 +206,7 @@ func TestRunPipelineStartTimeout(t *testing.T) {
 	runPipeline("test/sample_steps_passing.yml", &flags)
 
 	if len(errors) != 2 {
-		t.Errorf("excepted two errors, actual %v", len(errors))
+		t.Errorf("expected two errors, actual %v", len(errors))
 	}
 	msg := "[paddle] [Timed out waiting for pod to start. Cluster might not have sufficient resources.]"
 	for _, err := range errors {

cli/pipeline/template.go

@@ -132,7 +132,7 @@ spec:
         - "-c"
         - "mkdir -p $INPUT_PATH $OUTPUT_PATH &&
           {{ range $index, $input := .Step.Inputs }}
-          paddle data get {{ $input.Step }}/{{ $input.Version }} $INPUT_PATH -b {{ $input.Branch | sanitizeName }} -p {{ $input.Path }} {{ $input.Bucket | bucketParam }} {{$input.Keys | keysParam}} &&
+          paddle data get {{ $input.Step }}/{{ $input.Version }} $INPUT_PATH -b {{ $input.Branch | sanitizeName }} -p {{ $input.Path }} {{ $input.Bucket | bucketParam }} {{$input.Keys | keysParam}} {{ $input.Subdir | subdirParam }} &&
           {{ end }}
           touch /data/first-step.txt &&
           echo first step finished &&
@@ -210,6 +210,7 @@ func (p PodDefinition) compile() *bytes.Buffer {
 		"sanitizeName": sanitizeName,
 		"bucketParam":  p.bucketParam,
 		"keysParam":    p.keysParam,
+		"subdirParam":  p.subdirParam,
 	}
 	tmpl := template.Must(template.New("podTemplate").Funcs(fmap).Parse(podTemplate))
 	buffer := new(bytes.Buffer)
@@ -277,6 +278,13 @@ func (p *PodDefinition) keysParam(keys []string) string {
 	return ""
 }
 
+func (p *PodDefinition) subdirParam(subdir string) string {
+	if subdir != "" {
+		return "-d " + subdir
+	}
+	return ""
+}
+
 func sanitizeName(name string) string {
 	str := strings.ToLower(name)
 	str = strings.Replace(str, "_", "-", -1)

cli/pipeline/template_test.go

@@ -5,7 +5,7 @@ import (
 	"strings"
 	"testing"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/yaml"
 )
 
@@ -16,20 +16,28 @@ func TestCompileTemplate(t *testing.T) {
 	}
 	pipeline := ParsePipeline(data)
 
-	podDefinition := NewPodDefinition(pipeline, &pipeline.Steps[0])
+	podDefinition := NewPodDefinition(pipeline, &pipeline.Steps[1])
 
 	stepPodBuffer := podDefinition.compile()
 
 	pod := &v1.Pod{}
 	yaml.NewYAMLOrJSONDecoder(stepPodBuffer, 4096).Decode(pod)
 
-	if pod.Name != "sample-steps-passing-version1-step1-master" {
+	if pod.Name != "sample-steps-passing-version1a-step2-master" {
 		t.Errorf("Pod name is %s", pod.Name)
 	}
 
 	if pod.Spec.Containers[0].Image != pipeline.Steps[0].Image {
 		t.Errorf("First image is %s", pod.Spec.Containers[0].Image)
 	}
+
+	if !strings.Contains(pod.Spec.Containers[1].Command[2], "-d step1-version1") {
+		t.Errorf("Expected paddle get command to contain -d step1-version1, but it did not")
+	}
+
+	if strings.Count(pod.Spec.Containers[1].Command[2], "-d") != 1 {
+		t.Errorf("Only one paddle get command should contain the -d parameter")
+	}
 }
 
 func TestSecrets(t *testing.T) {

cli/pipeline/test/sample_steps_passing.yml

@@ -20,6 +20,12 @@ steps:
     step: step2
     version: version1a
     inputs:
+      -
+        step: step1
+        version: version1
+        branch: master
+        path: HEAD
+        subdir: 'step1-version1'
       -
         step: step1
         version: version1

cli/version.go

@@ -1,3 +1,3 @@
 package cli
 //go:generate bash ./generate_version.sh
-var PaddleVersion = "0.14.0"
+var PaddleVersion = "0.17.0"