Improved pipeline config for artifact.sync

Currently, the pipeline config is a map of local glob pattern to destination directory. This scheme has been extended with some magic sequences, making it difficult to understand. This commit converts the sync map into a list of sync rules. All sync rules are consulted to determine the destination paths. This prepares for further changes to the sync logic. Also see GoogleContainerTools#1844 Signed-off-by: Cornelius Weig <[email protected]>
corneliusweig · Apr 8, 2019 · 87a6708 · 87a6708
1 parent 533378c
commit 87a6708
Show file tree

Hide file tree

Showing 12 changed files with 249 additions and 245 deletions.
diff --git a/docs/content/en/schemas/v1beta8.json b/docs/content/en/schemas/v1beta8.json
@@ -70,16 +70,12 @@
               "x-intellij-html-description": "plugin used to build this artifact."
             },
             "sync": {
-              "additionalProperties": {
-                "type": "string"
+              "items": {
+                "$ref": "#/definitions/SyncRule"
               },
-              "type": "object",
-              "description": "*alpha* local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "default": "{}",
-              "examples": [
-                "{\"*.py\": \".\", \"css/**/*.css\": \"app/css\"}"
-              ]
+              "type": "array",
+              "description": "*alpha* local files synced to pods instead of triggering an image build when modified. This is a list of sync rules indicating source and destination.",
+              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified. This is a list of sync rules indicating source and destination."
             }
           },
           "preferredOrder": [
@@ -117,16 +113,12 @@
               "x-intellij-html-description": "plugin used to build this artifact."
             },
             "sync": {
-              "additionalProperties": {
-                "type": "string"
+              "items": {
+                "$ref": "#/definitions/SyncRule"
               },
-              "type": "object",
-              "description": "*alpha* local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "default": "{}",
-              "examples": [
-                "{\"*.py\": \".\", \"css/**/*.css\": \"app/css\"}"
-              ]
+              "type": "array",
+              "description": "*alpha* local files synced to pods instead of triggering an image build when modified. This is a list of sync rules indicating source and destination.",
+              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified. This is a list of sync rules indicating source and destination."
             }
           },
           "preferredOrder": [
@@ -165,16 +157,12 @@
               "x-intellij-html-description": "plugin used to build this artifact."
             },
             "sync": {
-              "additionalProperties": {
-                "type": "string"
+              "items": {
+                "$ref": "#/definitions/SyncRule"
               },
-              "type": "object",
-              "description": "*alpha* local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "default": "{}",
-              "examples": [
-                "{\"*.py\": \".\", \"css/**/*.css\": \"app/css\"}"
-              ]
+              "type": "array",
+              "description": "*alpha* local files synced to pods instead of triggering an image build when modified. This is a list of sync rules indicating source and destination.",
+              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified. This is a list of sync rules indicating source and destination."
             }
           },
           "preferredOrder": [
@@ -213,16 +201,12 @@
               "x-intellij-html-description": "plugin used to build this artifact."
             },
             "sync": {
-              "additionalProperties": {
-                "type": "string"
+              "items": {
+                "$ref": "#/definitions/SyncRule"
               },
-              "type": "object",
-              "description": "*alpha* local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "default": "{}",
-              "examples": [
-                "{\"*.py\": \".\", \"css/**/*.css\": \"app/css\"}"
-              ]
+              "type": "array",
+              "description": "*alpha* local files synced to pods instead of triggering an image build when modified. This is a list of sync rules indicating source and destination.",
+              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified. This is a list of sync rules indicating source and destination."
             }
           },
           "preferredOrder": [
@@ -261,16 +245,12 @@
               "x-intellij-html-description": "plugin used to build this artifact."
             },
             "sync": {
-              "additionalProperties": {
-                "type": "string"
+              "items": {
+                "$ref": "#/definitions/SyncRule"
               },
-              "type": "object",
-              "description": "*alpha* local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "default": "{}",
-              "examples": [
-                "{\"*.py\": \".\", \"css/**/*.css\": \"app/css\"}"
-              ]
+              "type": "array",
+              "description": "*alpha* local files synced to pods instead of triggering an image build when modified. This is a list of sync rules indicating source and destination.",
+              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified. This is a list of sync rules indicating source and destination."
             }
           },
           "preferredOrder": [
@@ -309,16 +289,12 @@
               "x-intellij-html-description": "plugin used to build this artifact."
             },
             "sync": {
-              "additionalProperties": {
-                "type": "string"
+              "items": {
+                "$ref": "#/definitions/SyncRule"
               },
-              "type": "object",
-              "description": "*alpha* local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "default": "{}",
-              "examples": [
-                "{\"*.py\": \".\", \"css/**/*.css\": \"app/css\"}"
-              ]
+              "type": "array",
+              "description": "*alpha* local files synced to pods instead of triggering an image build when modified. This is a list of sync rules indicating source and destination.",
+              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified. This is a list of sync rules indicating source and destination."
             }
           },
           "preferredOrder": [
@@ -1587,6 +1563,47 @@
       "description": "holds the fields parsed from the Skaffold configuration file (skaffold.yaml).",
       "x-intellij-html-description": "holds the fields parsed from the Skaffold configuration file (skaffold.yaml)."
     },
+    "SyncRule": {
+      "required": [
+        "from",
+        "to"
+      ],
+      "properties": {
+        "flatten": {
+          "type": "boolean",
+          "description": "controls if all files matched by the from rule should be put flat into the destination directory.",
+          "x-intellij-html-description": "controls if all files matched by the from rule should be put flat into the destination directory.",
+          "default": "false"
+        },
+        "from": {
+          "type": "string",
+          "description": "a glob pattern to match local paths against.",
+          "x-intellij-html-description": "a glob pattern to match local paths against.",
+          "examples": [
+            "\"css/**/*.css\""
+          ]
+        },
+        "strip": {
+          "type": "string",
+          "description": "specifies the path prefix to remove from the source path when transplanting the files into the destination folder.",
+          "x-intellij-html-description": "specifies the path prefix to remove from the source path when transplanting the files into the destination folder."
+        },
+        "to": {
+          "type": "string",
+          "description": "destination path in the container where the files should be synced to. For Docker artifacts the destination may be inferred by omitting it.",
+          "x-intellij-html-description": "destination path in the container where the files should be synced to. For Docker artifacts the destination may be inferred by omitting it."
+        }
+      },
+      "preferredOrder": [
+        "from",
+        "to",
+        "flatten",
+        "strip"
+      ],
+      "additionalProperties": false,
+      "description": "specifies which local files to sync to remote folders.",
+      "x-intellij-html-description": "specifies which local files to sync to remote folders."
+    },
     "TagPolicy": {
       "properties": {
         "dateTime": {

diff --git a/examples/hot-reload/skaffold.yaml b/examples/hot-reload/skaffold.yaml
@@ -5,11 +5,15 @@ build:
   - image: gcr.io/k8s-skaffold/node-example
     context: node
     sync:
-      '**/*.js': .
+    - from: '**/*.js'
+      to: .
+      flatten: true
   - image: gcr.io/k8s-skaffold/python-reload
     context: python
     sync:
-      '**/*.py': .
+    - from: '**/*.py'
+      to: .
+      flatten: true
 deploy:
   kubectl:
     manifests:

diff --git a/examples/nodejs/skaffold.yaml b/examples/nodejs/skaffold.yaml
@@ -5,7 +5,9 @@ build:
   - image: gcr.io/k8s-skaffold/node-example
     context: backend
     sync:
-      '*.js': .
+    - from: '*.js'
+      to: .
+      flatten: true
 deploy:
   kubectl:
     manifests:

diff --git a/integration/examples/hot-reload/skaffold.yaml b/integration/examples/hot-reload/skaffold.yaml
@@ -5,11 +5,15 @@ build:
   - image: gcr.io/k8s-skaffold/node-example
     context: node
     sync:
-      '**/*.js': .
+    - from: '**/*.js'
+      to: .
+      flatten: true
   - image: gcr.io/k8s-skaffold/python-reload
     context: python
     sync:
-      '**/*.py': .
+    - from: '**/*.py'
+      to: .
+      flatten: true
 deploy:
   kubectl:
     manifests:

diff --git a/integration/examples/nodejs/skaffold.yaml b/integration/examples/nodejs/skaffold.yaml
@@ -5,7 +5,9 @@ build:
   - image: gcr.io/k8s-skaffold/node-example
     context: backend
     sync:
-      '*.js': .
+    - from: '*.js'
+      to: .
+      flatten: true
 deploy:
   kubectl:
     manifests:

diff --git a/integration/testdata/file-sync/skaffold.yaml b/integration/testdata/file-sync/skaffold.yaml
@@ -7,7 +7,9 @@ build:
   - image: gcr.io/k8s-skaffold/test-file-sync
     context: .
     sync:
-      '**/foo*' : /test
+    - from: '**/foo*'
+      to: /test
+      flatten: true
 deploy:
  kubectl:
    manifests:

diff --git a/pkg/skaffold/runner/dev_test.go b/pkg/skaffold/runner/dev_test.go
@@ -354,9 +354,7 @@ func TestDevSync(t *testing.T) {
 			err := runner.Dev(context.Background(), discardOutput(), []*latest.Artifact{
 				{
 					ImageName: "img1",
-					Sync: map[string]string{
-						"file1": "file1",
-					},
+					Sync:      []*latest.SyncRule{{From: "file1", To: "file1"}},
 				},
 				{
 					ImageName: "img2",

diff --git a/pkg/skaffold/schema/latest/config.go b/pkg/skaffold/schema/latest/config.go
@@ -488,9 +488,8 @@ type Artifact struct {
 
 	// Sync *alpha* lists local files synced to pods instead
 	// of triggering an image build when modified.
-	// This is a mapping of local files to sync to remote folders.
-	// For example: `{"*.py": ".", "css/**/*.css": "app/css"}`.
-	Sync map[string]string `yaml:"sync,omitempty"`
+	// This is a list of sync rules indicating source and destination.
+	Sync []*SyncRule `yaml:"sync,omitempty"`
 
 	// ArtifactType describes how to build an artifact.
 	ArtifactType `yaml:",inline"`
@@ -501,6 +500,25 @@ type Artifact struct {
 	BuilderPlugin *BuilderPlugin `yaml:"plugin,omitempty"`
 }
 
+// SyncRule specifies which local files to sync to remote folders.
+type SyncRule struct {
+	// From is a glob pattern to match local paths against.
+	// For example: `"css/**/*.css"`.
+	From string `yaml:"from,omitempty" yamltags:"required"`
+
+	// To is the destination path in the container where the files should be synced to.
+	// For Docker artifacts the destination may be inferred by omitting it.
+	To string `yaml:"to,omitempty" yamltags:"required"`
+
+	// Flatten controls if all files matched by the from rule should be put
+	// flat into the destination directory.
+	Flatten bool `yaml:"flatten,omitempty"`
+
+	// Strip specifies the path prefix to remove from the source path when
+	// transplanting the files into the destination folder.
+	Strip string `yaml:"strip,omitempty"`
+}
+
 // Profile *beta* profiles are used to override any `build`, `test` or `deploy` configuration.
 type Profile struct {
 	// Name is a unique profile name.

diff --git a/pkg/skaffold/sync/kubectl/kubectl.go b/pkg/skaffold/sync/kubectl/kubectl.go
@@ -58,19 +58,19 @@ func (k *Syncer) Sync(ctx context.Context, s *sync.Item) error {
 	return nil
 }
 
-func deleteFileFn(ctx context.Context, pod v1.Pod, container v1.Container, files map[string]string) []*exec.Cmd {
+func deleteFileFn(ctx context.Context, pod v1.Pod, container v1.Container, files map[string][]string) []*exec.Cmd {
 	// "kubectl" is below...
-	deleteCmd := []string{"exec", pod.Name, "--namespace", pod.Namespace, "-c", container.Name, "--", "rm", "-rf"}
+	deleteCmd := []string{"exec", pod.Name, "--namespace", pod.Namespace, "-c", container.Name, "--", "rm", "-rf", "--"}
 	args := make([]string, 0, len(deleteCmd)+len(files))
 	args = append(args, deleteCmd...)
-	for _, dst := range files {
-		args = append(args, dst)
+	for _, dsts := range files {
+		args = append(args, dsts...)
 	}
 	delete := exec.CommandContext(ctx, "kubectl", args...)
 	return []*exec.Cmd{delete}
 }
 
-func copyFileFn(ctx context.Context, pod v1.Pod, container v1.Container, files map[string]string) []*exec.Cmd {
+func copyFileFn(ctx context.Context, pod v1.Pod, container v1.Container, files map[string][]string) []*exec.Cmd {
 	// Use "m" flag to touch the files as they are copied.
 	reader, writer := io.Pipe()
 	copy := exec.CommandContext(ctx, "kubectl", "exec", pod.Name, "--namespace", pod.Namespace, "-c", container.Name, "-i",