From 39fe903498aa43d9058ebc92a7231804b40f67a9 Mon Sep 17 00:00:00 2001 From: Prachi Pendse Date: Tue, 31 Mar 2020 01:08:54 -0700 Subject: [PATCH 1/5] Support mounting volumes to containers --- cmd/config/internal/commands/run-fns.go | 5 + cmd/config/internal/commands/run_test.go | 33 ++++ kyaml/kio/filters/container.go | 8 + kyaml/kio/filters/container_test.go | 215 +++++++++++++++++++++++ kyaml/kio/filters/functiontypes.go | 7 + kyaml/runfn/runfn.go | 29 +++ kyaml/runfn/runfn_test.go | 10 ++ 7 files changed, 307 insertions(+) diff --git a/cmd/config/internal/commands/run-fns.go b/cmd/config/internal/commands/run-fns.go index fe24a97fd..f31d2fe7f 100644 --- a/cmd/config/internal/commands/run-fns.go +++ b/cmd/config/internal/commands/run-fns.go @@ -54,6 +54,9 @@ func GetRunFnRunner(name string) *RunFnRunner { &r.Network, "network", false, "enable network access for functions that declare it") r.Command.Flags().StringVar( &r.NetworkName, "network-name", "bridge", "the docker network to run the container in") + r.Command.Flags().StringSliceVar( + &r.Volumes, "volume", []string{}, + "the volumes to bind mount to the container.") return r } @@ -75,6 +78,7 @@ type RunFnRunner struct { RunFns runfn.RunFns Network bool NetworkName string + Volumes []string } func (r *RunFnRunner) runE(c *cobra.Command, args []string) error { @@ -250,6 +254,7 @@ func (r *RunFnRunner) preRunE(c *cobra.Command, args []string) error { Network: r.Network, NetworkName: r.NetworkName, EnableStarlark: r.EnableStar, + Volumes: r.Volumes, } // don't consider args for the function diff --git a/cmd/config/internal/commands/run_test.go b/cmd/config/internal/commands/run_test.go index 4a2062072..887451035 100644 --- a/cmd/config/internal/commands/run_test.go +++ b/cmd/config/internal/commands/run_test.go @@ -27,6 +27,7 @@ func TestRunFnCommand_preRunE(t *testing.T) { functionPaths []string network bool networkName string + volumes []string }{ { name: "config map", @@ -213,6 +214,29 @@ metadata: data: {g: h, i: j=k} kind: Foo apiVersion: v1 +`, + }, + { + name: "volumes", + args: []string{"run", "dir", "--volume", "vol1", "--volume", "vol2"}, + path: "dir", + volumes: []string{"vol1", "vol2"}, + }, + { + name: "custom kind with volumes", + args: []string{ + "run", "dir", "--volume", "vol", "--image", "foo:bar", "--", "Foo", "g=h", "i=j=k"}, + path: "dir", + volumes: []string{"vol"}, + expected: ` +metadata: + name: function-input + annotations: + config.kubernetes.io/function: | + container: {image: 'foo:bar'} +data: {g: h, i: j=k} +kind: Foo +apiVersion: v1 `, }, { @@ -303,6 +327,15 @@ apiVersion: v1 t.FailNow() } + // check if Volumes were set + if tt.volumes == nil { + // make Equal work against flag default + tt.volumes = []string{} + } + if !assert.Equal(t, tt.volumes, r.RunFns.Volumes) { + t.FailNow() + } + // check if Functions were set if tt.expected != "" { if !assert.Len(t, r.RunFns.Functions, 1) { diff --git a/kyaml/kio/filters/container.go b/kyaml/kio/filters/container.go index 6e3dee11f..663774356 100644 --- a/kyaml/kio/filters/container.go +++ b/kyaml/kio/filters/container.go @@ -134,6 +134,9 @@ type ContainerFilter struct { // Network is the container network to use. Network string `yaml:"network,omitempty"` + // Volumes are the directories to mount as container volumes. + Volumes []string `yaml:"volumes,omitempty"` + // StorageMounts is a list of storage options that the container will have mounted. StorageMounts []StorageMount @@ -335,6 +338,11 @@ func (c *ContainerFilter) getArgs() []string { args = append(args, "--mount", storageMount.String()) } + // export volumes to the container + for _, volume := range c.Volumes { + args = append(args, "--volume", volume) + } + // export the local environment vars to the container for _, pair := range os.Environ() { tokens := strings.Split(pair, "=") diff --git a/kyaml/kio/filters/container_test.go b/kyaml/kio/filters/container_test.go index 3ae8d901a..5d37ee55e 100644 --- a/kyaml/kio/filters/container_test.go +++ b/kyaml/kio/filters/container_test.go @@ -141,6 +141,87 @@ metadata: assert.Equal(t, expected, cmd.Args) } +func TestFilter_command_volume(t *testing.T) { + cfg, err := yaml.Parse(`apiVersion: apps/v1 +kind: Deployment +metadata: + name: foo +`) + if !assert.NoError(t, err) { + return + } + instance := &ContainerFilter{ + Image: "example.com:version", + Volumes: []string{"/host-src:/container-dest:ro"}, + Config: cfg, + } + cmd, err := instance.getCommand() + if !assert.NoError(t, err) { + return + } + + expected := []string{ + "docker", "run", + "--rm", + "-i", "-a", "STDIN", "-a", "STDOUT", "-a", "STDERR", + "--network", "none", + "--user", "nobody", + "--security-opt=no-new-privileges", + "--volume", "/host-src:/container-dest:ro", + } + for _, e := range os.Environ() { + // the process env + tokens := strings.Split(e, "=") + if tokens[0] == "" { + continue + } + expected = append(expected, "-e", tokens[0]) + } + expected = append(expected, "example.com:version") + assert.Equal(t, expected, cmd.Args) +} + +func TestFilter_command_volumes(t *testing.T) { + cfg, err := yaml.Parse(`apiVersion: apps/v1 +kind: Deployment +metadata: + name: foo +`) + if !assert.NoError(t, err) { + return + } + instance := &ContainerFilter{ + Image: "example.com:version", + Volumes: []string{"/host-src1:/container-dest1:ro", "/host-src2:/container-dest2:rw"}, + Config: cfg, + } + cmd, err := instance.getCommand() + if !assert.NoError(t, err) { + return + } + + expected := []string{ + "docker", "run", + "--rm", + "-i", "-a", "STDIN", "-a", "STDOUT", "-a", "STDERR", + "--network", "none", + "--user", "nobody", + "--security-opt=no-new-privileges", + "--volume", "/host-src1:/container-dest1:ro", + "--volume", "/host-src2:/container-dest2:rw", + } + for _, e := range os.Environ() { + // the process env + tokens := strings.Split(e, "=") + if tokens[0] == "" { + continue + } + expected = append(expected, "-e", tokens[0]) + } + expected = append(expected, "example.com:version") + assert.Equal(t, expected, cmd.Args) +} + func TestFilter_Filter(t *testing.T) { cfg, err := yaml.Parse(`apiVersion: apps/v1 kind: Deployment @@ -355,6 +436,70 @@ container: `, }, + { + name: "volume", + resource: ` +apiVersion: v1beta1 +kind: Example +metadata: + annotations: + config.kubernetes.io/function: |- + container: + image: foo:v1.0.0 + volumes: ["/host-src:/container-dest:ro"] +`, + expectedFn: ` +container: + image: foo:v1.0.0 + volumes: + - /host-src:/container-dest:ro +`, + }, + + { + name: "volumes as array", + resource: ` +apiVersion: v1beta1 +kind: Example +metadata: + annotations: + config.kubernetes.io/function: |- + container: + image: foo:v1.0.0 + volumes: ["/host-src1:/container-dest1:ro", "/host-src2:/container-dest2:rw"] +`, + expectedFn: ` +container: + image: foo:v1.0.0 + volumes: + - /host-src1:/container-dest1:ro + - /host-src2:/container-dest2:rw +`, + }, + + { + name: "volumes as list", + resource: ` +apiVersion: v1beta1 +kind: Example +metadata: + annotations: + config.kubernetes.io/function: |- + container: + image: foo:v1.0.0 + volumes: + - "/host-src1:/container-dest1:ro" + - "/host-src2:/container-dest2:rw" +`, + expectedFn: ` +container: + image: foo:v1.0.0 + volumes: + - /host-src1:/container-dest1:ro + - /host-src2:/container-dest2:rw +`, + }, + { name: "path", resource: ` @@ -516,6 +661,76 @@ metadata: } } +func Test_GetContainerVolumeRequired(t *testing.T) { + tests := []struct { + input string + volumes []string + }{ + { + input: `apiVersion: v1 +kind: Foo +metadata: + name: foo + configFn: + container: + image: gcr.io/kustomize-functions/example-tshirt:v0.1.0 + volumes: [ /host-src:/container-dest:ro ] +`, + volumes: []string{"/host-src:/container-dest:ro"}, + }, + { + + input: `apiVersion: v1 +kind: Foo +metadata: + name: foo + configFn: + container: + image: gcr.io/kustomize-functions/example-tshirt:v0.1.0 +`, + volumes: []string(nil), + }, + { + input: `apiVersion: v1 +kind: Foo +metadata: + name: foo + annotations: + config.kubernetes.io/function: | + container: + image: gcr.io/kustomize-functions/example-tshirt:v0.1.0 + volumes: [ "/host-src1:/container-dest1:ro", "/host-src2:/container-dest2:rw" ] +`, + volumes: []string{"/host-src1:/container-dest1:ro", "/host-src2:/container-dest2:rw"}, + }, + { + input: `apiVersion: v1 +kind: Foo +metadata: + name: foo + annotations: + config.kubernetes.io/function: | + container: + image: gcr.io/kustomize-functions/example-tshirt:v0.1.0 + volumes: + - /host-src1:/container-dest1:ro + - /host-src2:/container-dest2:rw +`, + volumes: []string{"/host-src1:/container-dest1:ro", "/host-src2:/container-dest2:rw"}, + }, + } + + for _, tc := range tests { + cfg, err := yaml.Parse(tc.input) + if !assert.NoError(t, err) { + return + } + + fn := GetFunctionSpec(cfg) + assert.Equal(t, tc.volumes, fn.Container.Volumes) + } +} + func TestFilter_Filter_defaultNaming(t *testing.T) { cfg, err := yaml.Parse(`apiVersion: apps/v1 kind: Deployment diff --git a/kyaml/kio/filters/functiontypes.go b/kyaml/kio/filters/functiontypes.go index 83f6f647c..4096cd952 100644 --- a/kyaml/kio/filters/functiontypes.go +++ b/kyaml/kio/filters/functiontypes.go @@ -28,6 +28,9 @@ type FunctionSpec struct { // Starlark is the spec for running a function as a starlark script Starlark StarlarkSpec `json:"starlark,omitempty" yaml:"starlark,omitempty"` + + // Volumes are the directories to mount as container volumes + Volumes []string `json:"volumes,omitempty" yaml:"volumes,omitempty"` } // ContainerSpec defines a spec for running a function as a container @@ -37,6 +40,9 @@ type ContainerSpec struct { // Network defines network specific configuration Network ContainerNetwork `json:"network,omitempty" yaml:"network,omitempty"` + + // Volumes are the directories to mount as container volumes + Volumes []string `json:"volumes,omitempty" yaml:"volumes,omitempty"` } // ContainerNetwork @@ -68,6 +74,7 @@ func GetFunctionSpec(n *yaml.RNode) *FunctionSpec { path := meta.Annotations[kioutil.PathAnnotation] if fn := getFunctionSpecFromAnnotation(n, meta); fn != nil { fn.Network = "" + fn.Volumes = []string{} fn.Path = path return fn } diff --git a/kyaml/runfn/runfn.go b/kyaml/runfn/runfn.go index 19d348486..23ccd00f6 100644 --- a/kyaml/runfn/runfn.go +++ b/kyaml/runfn/runfn.go @@ -51,6 +51,10 @@ type RunFns struct { // NetworkName is the name of the docker network to use for the container NetworkName string + // Volumes Volumes allows directories to be specified outside the configuration + // directory. + Volumes []string + // Output can be set to write the result to Output rather than back to the directory Output io.Writer @@ -133,6 +137,13 @@ func (r RunFns) getFilters(nodes []*yaml.RNode) ([]kio.Filter, error) { } fltrs = append(fltrs, f...) + // directories from volumes specified on the struct + f, err = r.getDirectoriesFromVolumes() + if err != nil { + return nil, err + } + fltrs = append(fltrs, f...) + // explicit fns specified on the struct f, err = r.getFunctionsFromFunctions() if err != nil { @@ -196,6 +207,24 @@ func (r RunFns) getFunctionsFromFunctionPaths() ([]kio.Filter, error) { return r.getFunctionFilters(true, buff.Nodes...) } +// getDirectoriesFromVolumes returns the set of directories read from r.Volumes +// as a slice of Filters +func (r RunFns) getDirectoriesFromVolumes() ([]kio.Filter, error) { + buff := &kio.PackageBuffer{} + for i := range r.Volumes { + err := kio.Pipeline{ + Inputs: []kio.Reader{ + kio.LocalPackageReader{PackagePath: r.Volumes[i]}, + }, + Outputs: []kio.Writer{buff}, + }.Execute() + if err != nil { + return nil, err + } + } + return r.getFunctionFilters(true, buff.Nodes...) +} + // getFunctionsFromFunctions returns the set of explicitly provided functions as // Filters func (r RunFns) getFunctionsFromFunctions() ([]kio.Filter, error) { diff --git a/kyaml/runfn/runfn_test.go b/kyaml/runfn/runfn_test.go index f88349194..b3018129f 100644 --- a/kyaml/runfn/runfn_test.go +++ b/kyaml/runfn/runfn_test.go @@ -140,6 +140,16 @@ func TestRunFns_Execute__initDefault(t *testing.T) { FunctionPaths: []string{"foo"}, }, }, + { + name: "explicit directories in volumes", + instance: RunFns{Volumes: []string{"vol"}}, + expected: RunFns{ + Output: os.Stdout, + Input: os.Stdin, + NoFunctionsFromInput: getFalse(), + Volumes: []string{"vol"}, + }, + }, } for i := range tests { tt := tests[i] From a4ee1c2e72c9d2d77daa78b24f025af2971ab38c Mon Sep 17 00:00:00 2001 From: Prachi Pendse Date: Tue, 31 Mar 2020 10:19:17 -0700 Subject: [PATCH 2/5] Remove TODO for network and volume issues --- .../validator-kubeval/local-resource/example-use.yaml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/functions/examples/validator-kubeval/local-resource/example-use.yaml b/functions/examples/validator-kubeval/local-resource/example-use.yaml index d18722b62..0c040422e 100644 --- a/functions/examples/validator-kubeval/local-resource/example-use.yaml +++ b/functions/examples/validator-kubeval/local-resource/example-use.yaml @@ -12,13 +12,6 @@ spec: strict: true ignoreMissingSchemas: true - # TODO: Remove these once function container network/volumes features are - # stabilized. - # Relevant issues: - # - https://github.com/kubernetes-sigs/kustomize/issues/1901 - # - https://github.com/kubernetes-sigs/kustomize/issues/1902 - kubernetesVersion: "1.16.0" - schemaLocation: "file:///schemas" --- apiVersion: v1 kind: Service From 38973a80c343b1980c62f95405fdf78c3260a88f Mon Sep 17 00:00:00 2001 From: Prachi Pendse Date: Wed, 1 Apr 2020 15:03:57 -0700 Subject: [PATCH 3/5] Use mount flag to pass storage mounts to functions --- cmd/config/internal/commands/run-fns.go | 20 +- cmd/config/internal/commands/run_test.go | 26 +-- kyaml/kio/filters/container.go | 46 ++-- kyaml/kio/filters/container_test.go | 277 +++++------------------ kyaml/kio/filters/functiontypes.go | 25 +- kyaml/runfn/runfn.go | 29 --- kyaml/runfn/runfn_test.go | 6 +- 7 files changed, 132 insertions(+), 297 deletions(-) diff --git a/cmd/config/internal/commands/run-fns.go b/cmd/config/internal/commands/run-fns.go index f31d2fe7f..523cf9015 100644 --- a/cmd/config/internal/commands/run-fns.go +++ b/cmd/config/internal/commands/run-fns.go @@ -11,6 +11,7 @@ import ( "github.com/spf13/cobra" "sigs.k8s.io/kustomize/cmd/config/internal/generateddocs/commands" "sigs.k8s.io/kustomize/kyaml/errors" + "sigs.k8s.io/kustomize/kyaml/kio/filters" "sigs.k8s.io/kustomize/kyaml/runfn" "sigs.k8s.io/kustomize/kyaml/yaml" ) @@ -55,8 +56,8 @@ func GetRunFnRunner(name string) *RunFnRunner { r.Command.Flags().StringVar( &r.NetworkName, "network-name", "bridge", "the docker network to run the container in") r.Command.Flags().StringSliceVar( - &r.Volumes, "volume", []string{}, - "the volumes to bind mount to the container.") + &r.Mounts, "mount", []string{}, + "a list of storage options read from the filesystem") return r } @@ -78,7 +79,7 @@ type RunFnRunner struct { RunFns runfn.RunFns Network bool NetworkName string - Volumes []string + Mounts []string } func (r *RunFnRunner) runE(c *cobra.Command, args []string) error { @@ -203,6 +204,14 @@ data: {} return []*yaml.RNode{rc}, nil } +func toStorageMounts(mounts []string) []filters.StorageMount { + var sms []filters.StorageMount + for _, mount := range mounts { + sms = append(sms, filters.StringToStorageMount(mount)) + } + return sms +} + func (r *RunFnRunner) preRunE(c *cobra.Command, args []string) error { if r.EnableStar != (r.StarPath != "") { return errors.Errorf("must specify --star-path with --enable-star") @@ -244,6 +253,9 @@ func (r *RunFnRunner) preRunE(c *cobra.Command, args []string) error { path = args[0] } + // parse mounts to set storageMounts + storageMounts := toStorageMounts(r.Mounts) + r.RunFns = runfn.RunFns{ FunctionPaths: r.FnPaths, GlobalScope: r.GlobalScope, @@ -254,7 +266,7 @@ func (r *RunFnRunner) preRunE(c *cobra.Command, args []string) error { Network: r.Network, NetworkName: r.NetworkName, EnableStarlark: r.EnableStar, - Volumes: r.Volumes, + StorageMounts: storageMounts, } // don't consider args for the function diff --git a/cmd/config/internal/commands/run_test.go b/cmd/config/internal/commands/run_test.go index 887451035..a3e8f492d 100644 --- a/cmd/config/internal/commands/run_test.go +++ b/cmd/config/internal/commands/run_test.go @@ -27,7 +27,7 @@ func TestRunFnCommand_preRunE(t *testing.T) { functionPaths []string network bool networkName string - volumes []string + mount []string }{ { name: "config map", @@ -217,17 +217,14 @@ apiVersion: v1 `, }, { - name: "volumes", - args: []string{"run", "dir", "--volume", "vol1", "--volume", "vol2"}, - path: "dir", - volumes: []string{"vol1", "vol2"}, - }, - { - name: "custom kind with volumes", + name: "custom kind with storage mounts", args: []string{ - "run", "dir", "--volume", "vol", "--image", "foo:bar", "--", "Foo", "g=h", "i=j=k"}, - path: "dir", - volumes: []string{"vol"}, + "run", "dir", "--mount", "type=bind;src=/mount/path;dst=/local/", + "--mount", "type=volume;src=myvol;dst=/local/", + "--mount", "type=tmpfs;dst=/local/", + "--image", "foo:bar", "--", "Foo", "g=h", "i=j=k"}, + path: "dir", + mount: []string{"type=bind;src=/mount/path;dst=/local/", "type=volume;src=myvol;dst=/local/", "type=tmpfs;dst=/local/"}, expected: ` metadata: name: function-input @@ -327,12 +324,7 @@ apiVersion: v1 t.FailNow() } - // check if Volumes were set - if tt.volumes == nil { - // make Equal work against flag default - tt.volumes = []string{} - } - if !assert.Equal(t, tt.volumes, r.RunFns.Volumes) { + if !assert.Equal(t, toStorageMounts(tt.mount), r.RunFns.StorageMounts) { t.FailNow() } diff --git a/kyaml/kio/filters/container.go b/kyaml/kio/filters/container.go index 663774356..54dfdec57 100644 --- a/kyaml/kio/filters/container.go +++ b/kyaml/kio/filters/container.go @@ -134,11 +134,8 @@ type ContainerFilter struct { // Network is the container network to use. Network string `yaml:"network,omitempty"` - // Volumes are the directories to mount as container volumes. - Volumes []string `yaml:"volumes,omitempty"` - // StorageMounts is a list of storage options that the container will have mounted. - StorageMounts []StorageMount + StorageMounts []StorageMount `yaml:"mounts,omitempty"` // Config is the API configuration for the container and passed through the // API_CONFIG env var to the container. @@ -159,25 +156,31 @@ func (c ContainerFilter) String() string { return c.Image } -// StorageMount represents a container's mounted storage option(s) -type StorageMount struct { - // Type of mount e.g. bind mount, local volume, etc. - MountType string - - // Source for the storage to be mounted. - // For named volumes, this is the name of the volume. - // For anonymous volumes, this field is omitted (empty string). - // For bind mounts, this is the path to the file or directory on the host. - Src string - - // The path where the file or directory is mounted in the container. - DstPath string -} - func (s *StorageMount) String() string { return fmt.Sprintf("type=%s,src=%s,dst=%s:ro", s.MountType, s.Src, s.DstPath) } +func StringToStorageMount(s string) StorageMount { + m := make(map[string]string) + options := strings.Split(s, ";") + for _, option := range options { + keyVal := strings.Split(option, "=") + m[keyVal[0]] = keyVal[1] + } + var sm StorageMount + for key, value := range m { + switch { + case key == "type": + sm.MountType = value + case key == "src": + sm.Src = value + case key == "dst": + sm.DstPath = value + } + } + return sm +} + // functionsDirectoryName is keyword directory name for functions scoped 1 directory higher const functionsDirectoryName = "functions" @@ -338,11 +341,6 @@ func (c *ContainerFilter) getArgs() []string { args = append(args, "--mount", storageMount.String()) } - // export volumes to the container - for _, volume := range c.Volumes { - args = append(args, "--volume", volume) - } - // export the local environment vars to the container for _, pair := range os.Environ() { tokens := strings.Split(pair, "=") diff --git a/kyaml/kio/filters/container_test.go b/kyaml/kio/filters/container_test.go index 5d37ee55e..319a1a535 100644 --- a/kyaml/kio/filters/container_test.go +++ b/kyaml/kio/filters/container_test.go @@ -141,87 +141,6 @@ metadata: assert.Equal(t, expected, cmd.Args) } -func TestFilter_command_volume(t *testing.T) { - cfg, err := yaml.Parse(`apiVersion: apps/v1 -kind: Deployment -metadata: - name: foo -`) - if !assert.NoError(t, err) { - return - } - instance := &ContainerFilter{ - Image: "example.com:version", - Volumes: []string{"/host-src:/container-dest:ro"}, - Config: cfg, - } - cmd, err := instance.getCommand() - if !assert.NoError(t, err) { - return - } - - expected := []string{ - "docker", "run", - "--rm", - "-i", "-a", "STDIN", "-a", "STDOUT", "-a", "STDERR", - "--network", "none", - "--user", "nobody", - "--security-opt=no-new-privileges", - "--volume", "/host-src:/container-dest:ro", - } - for _, e := range os.Environ() { - // the process env - tokens := strings.Split(e, "=") - if tokens[0] == "" { - continue - } - expected = append(expected, "-e", tokens[0]) - } - expected = append(expected, "example.com:version") - assert.Equal(t, expected, cmd.Args) -} - -func TestFilter_command_volumes(t *testing.T) { - cfg, err := yaml.Parse(`apiVersion: apps/v1 -kind: Deployment -metadata: - name: foo -`) - if !assert.NoError(t, err) { - return - } - instance := &ContainerFilter{ - Image: "example.com:version", - Volumes: []string{"/host-src1:/container-dest1:ro", "/host-src2:/container-dest2:rw"}, - Config: cfg, - } - cmd, err := instance.getCommand() - if !assert.NoError(t, err) { - return - } - - expected := []string{ - "docker", "run", - "--rm", - "-i", "-a", "STDIN", "-a", "STDOUT", "-a", "STDERR", - "--network", "none", - "--user", "nobody", - "--security-opt=no-new-privileges", - "--volume", "/host-src1:/container-dest1:ro", - "--volume", "/host-src2:/container-dest2:rw", - } - for _, e := range os.Environ() { - // the process env - tokens := strings.Split(e, "=") - if tokens[0] == "" { - continue - } - expected = append(expected, "-e", tokens[0]) - } - expected = append(expected, "example.com:version") - assert.Equal(t, expected, cmd.Args) -} - func TestFilter_Filter(t *testing.T) { cfg, err := yaml.Parse(`apiVersion: apps/v1 kind: Deployment @@ -415,6 +334,68 @@ container: image: foo:v1.0.0`, }, + { + name: "storage mounts json style", + resource: ` +apiVersion: v1beta1 +kind: Example +metadata: + annotations: + config.kubernetes.io/function: |- + container: + image: foo:v1.0.0 + mounts: [ {type: bind, src: /mount/path, dst: /local/}, {src: myvol, dst: /local/, type: volume}, {dst: /local/, type: tmpfs} ] +`, + expectedFn: ` +container: + image: foo:v1.0.0 + mounts: + - type: bind + src: /mount/path + dst: /local/ + - type: volume + src: myvol + dst: /local/ + - type: tmpfs + dst: /local/ +`, + }, + + { + name: "storage mounts yaml style", + resource: ` +apiVersion: v1beta1 +kind: Example +metadata: + annotations: + config.kubernetes.io/function: |- + container: + image: foo:v1.0.0 + mounts: + - src: /mount/path + type: bind + dst: /local/ + - dst: /local/ + src: myvol + type: volume + - type: tmpfs + dst: /local/ +`, + expectedFn: ` +container: + image: foo:v1.0.0 + mounts: + - type: bind + src: /mount/path + dst: /local/ + - type: volume + src: myvol + dst: /local/ + - type: tmpfs + dst: /local/ +`, + }, + { name: "network", resource: ` @@ -436,70 +417,6 @@ container: `, }, - { - name: "volume", - resource: ` -apiVersion: v1beta1 -kind: Example -metadata: - annotations: - config.kubernetes.io/function: |- - container: - image: foo:v1.0.0 - volumes: ["/host-src:/container-dest:ro"] -`, - expectedFn: ` -container: - image: foo:v1.0.0 - volumes: - - /host-src:/container-dest:ro -`, - }, - - { - name: "volumes as array", - resource: ` -apiVersion: v1beta1 -kind: Example -metadata: - annotations: - config.kubernetes.io/function: |- - container: - image: foo:v1.0.0 - volumes: ["/host-src1:/container-dest1:ro", "/host-src2:/container-dest2:rw"] -`, - expectedFn: ` -container: - image: foo:v1.0.0 - volumes: - - /host-src1:/container-dest1:ro - - /host-src2:/container-dest2:rw -`, - }, - - { - name: "volumes as list", - resource: ` -apiVersion: v1beta1 -kind: Example -metadata: - annotations: - config.kubernetes.io/function: |- - container: - image: foo:v1.0.0 - volumes: - - "/host-src1:/container-dest1:ro" - - "/host-src2:/container-dest2:rw" -`, - expectedFn: ` -container: - image: foo:v1.0.0 - volumes: - - /host-src1:/container-dest1:ro - - /host-src2:/container-dest2:rw -`, - }, - { name: "path", resource: ` @@ -661,76 +578,6 @@ metadata: } } -func Test_GetContainerVolumeRequired(t *testing.T) { - tests := []struct { - input string - volumes []string - }{ - { - input: `apiVersion: v1 -kind: Foo -metadata: - name: foo - configFn: - container: - image: gcr.io/kustomize-functions/example-tshirt:v0.1.0 - volumes: [ /host-src:/container-dest:ro ] -`, - volumes: []string{"/host-src:/container-dest:ro"}, - }, - { - - input: `apiVersion: v1 -kind: Foo -metadata: - name: foo - configFn: - container: - image: gcr.io/kustomize-functions/example-tshirt:v0.1.0 -`, - volumes: []string(nil), - }, - { - input: `apiVersion: v1 -kind: Foo -metadata: - name: foo - annotations: - config.kubernetes.io/function: | - container: - image: gcr.io/kustomize-functions/example-tshirt:v0.1.0 - volumes: [ "/host-src1:/container-dest1:ro", "/host-src2:/container-dest2:rw" ] -`, - volumes: []string{"/host-src1:/container-dest1:ro", "/host-src2:/container-dest2:rw"}, - }, - { - input: `apiVersion: v1 -kind: Foo -metadata: - name: foo - annotations: - config.kubernetes.io/function: | - container: - image: gcr.io/kustomize-functions/example-tshirt:v0.1.0 - volumes: - - /host-src1:/container-dest1:ro - - /host-src2:/container-dest2:rw -`, - volumes: []string{"/host-src1:/container-dest1:ro", "/host-src2:/container-dest2:rw"}, - }, - } - - for _, tc := range tests { - cfg, err := yaml.Parse(tc.input) - if !assert.NoError(t, err) { - return - } - - fn := GetFunctionSpec(cfg) - assert.Equal(t, tc.volumes, fn.Container.Volumes) - } -} - func TestFilter_Filter_defaultNaming(t *testing.T) { cfg, err := yaml.Parse(`apiVersion: apps/v1 kind: Deployment diff --git a/kyaml/kio/filters/functiontypes.go b/kyaml/kio/filters/functiontypes.go index 4096cd952..db5a8adb3 100644 --- a/kyaml/kio/filters/functiontypes.go +++ b/kyaml/kio/filters/functiontypes.go @@ -29,8 +29,8 @@ type FunctionSpec struct { // Starlark is the spec for running a function as a starlark script Starlark StarlarkSpec `json:"starlark,omitempty" yaml:"starlark,omitempty"` - // Volumes are the directories to mount as container volumes - Volumes []string `json:"volumes,omitempty" yaml:"volumes,omitempty"` + // Mounts are the storage or directories to mount into the container + StorageMounts []StorageMount `json:"mounts,omitempty" yaml:"mounts,omitempty"` } // ContainerSpec defines a spec for running a function as a container @@ -41,8 +41,8 @@ type ContainerSpec struct { // Network defines network specific configuration Network ContainerNetwork `json:"network,omitempty" yaml:"network,omitempty"` - // Volumes are the directories to mount as container volumes - Volumes []string `json:"volumes,omitempty" yaml:"volumes,omitempty"` + // Mounts are the storage or directories to mount into the container + StorageMounts []StorageMount `json:"mounts,omitempty" yaml:"mounts,omitempty"` } // ContainerNetwork @@ -59,6 +59,21 @@ type StarlarkSpec struct { Path string `json:"path,omitempty" yaml:"path,omitempty"` } +// StorageMount represents a container's mounted storage option(s) +type StorageMount struct { + // Type of mount e.g. bind mount, local volume, etc. + MountType string `json:"type,omitempty" yaml:"type,omitempty"` + + // Source for the storage to be mounted. + // For named volumes, this is the name of the volume. + // For anonymous volumes, this field is omitted (empty string). + // For bind mounts, this is the path to the file or directory on the host. + Src string `json:"src,omitempty" yaml:"src,omitempty"` + + // The path where the file or directory is mounted in the container. + DstPath string `json:"dst,omitempty" yaml:"dst,omitempty"` +} + // GetFunctionSpec returns the FunctionSpec for a resource. Returns // nil if the resource does not have a FunctionSpec. // @@ -74,7 +89,7 @@ func GetFunctionSpec(n *yaml.RNode) *FunctionSpec { path := meta.Annotations[kioutil.PathAnnotation] if fn := getFunctionSpecFromAnnotation(n, meta); fn != nil { fn.Network = "" - fn.Volumes = []string{} + fn.StorageMounts = []StorageMount{} fn.Path = path return fn } diff --git a/kyaml/runfn/runfn.go b/kyaml/runfn/runfn.go index 23ccd00f6..19d348486 100644 --- a/kyaml/runfn/runfn.go +++ b/kyaml/runfn/runfn.go @@ -51,10 +51,6 @@ type RunFns struct { // NetworkName is the name of the docker network to use for the container NetworkName string - // Volumes Volumes allows directories to be specified outside the configuration - // directory. - Volumes []string - // Output can be set to write the result to Output rather than back to the directory Output io.Writer @@ -137,13 +133,6 @@ func (r RunFns) getFilters(nodes []*yaml.RNode) ([]kio.Filter, error) { } fltrs = append(fltrs, f...) - // directories from volumes specified on the struct - f, err = r.getDirectoriesFromVolumes() - if err != nil { - return nil, err - } - fltrs = append(fltrs, f...) - // explicit fns specified on the struct f, err = r.getFunctionsFromFunctions() if err != nil { @@ -207,24 +196,6 @@ func (r RunFns) getFunctionsFromFunctionPaths() ([]kio.Filter, error) { return r.getFunctionFilters(true, buff.Nodes...) } -// getDirectoriesFromVolumes returns the set of directories read from r.Volumes -// as a slice of Filters -func (r RunFns) getDirectoriesFromVolumes() ([]kio.Filter, error) { - buff := &kio.PackageBuffer{} - for i := range r.Volumes { - err := kio.Pipeline{ - Inputs: []kio.Reader{ - kio.LocalPackageReader{PackagePath: r.Volumes[i]}, - }, - Outputs: []kio.Writer{buff}, - }.Execute() - if err != nil { - return nil, err - } - } - return r.getFunctionFilters(true, buff.Nodes...) -} - // getFunctionsFromFunctions returns the set of explicitly provided functions as // Filters func (r RunFns) getFunctionsFromFunctions() ([]kio.Filter, error) { diff --git a/kyaml/runfn/runfn_test.go b/kyaml/runfn/runfn_test.go index b3018129f..236f5610c 100644 --- a/kyaml/runfn/runfn_test.go +++ b/kyaml/runfn/runfn_test.go @@ -141,13 +141,13 @@ func TestRunFns_Execute__initDefault(t *testing.T) { }, }, { - name: "explicit directories in volumes", - instance: RunFns{Volumes: []string{"vol"}}, + name: "explicit directories in mounts", + instance: RunFns{StorageMounts: []filters.StorageMount{{MountType: "volume", Src: "myvol", DstPath: "/local/"}}}, expected: RunFns{ Output: os.Stdout, Input: os.Stdin, NoFunctionsFromInput: getFalse(), - Volumes: []string{"vol"}, + StorageMounts: []filters.StorageMount{{MountType: "volume", Src: "myvol", DstPath: "/local/"}}, }, }, } From b17ea88bf79d9311d2f214328e4e498e37ea2ef0 Mon Sep 17 00:00:00 2001 From: Prachi Pendse Date: Thu, 2 Apr 2020 10:31:15 -0700 Subject: [PATCH 4/5] Update mount flag to match docker docs - Also modify TODO in validator-kubeval example --- cmd/config/internal/commands/run-fns.go | 2 +- cmd/config/internal/commands/run_test.go | 8 ++++---- .../validator-kubeval/local-resource/example-use.yaml | 6 ++++++ kyaml/kio/filters/container.go | 2 +- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/cmd/config/internal/commands/run-fns.go b/cmd/config/internal/commands/run-fns.go index 523cf9015..12558f044 100644 --- a/cmd/config/internal/commands/run-fns.go +++ b/cmd/config/internal/commands/run-fns.go @@ -55,7 +55,7 @@ func GetRunFnRunner(name string) *RunFnRunner { &r.Network, "network", false, "enable network access for functions that declare it") r.Command.Flags().StringVar( &r.NetworkName, "network-name", "bridge", "the docker network to run the container in") - r.Command.Flags().StringSliceVar( + r.Command.Flags().StringArrayVar( &r.Mounts, "mount", []string{}, "a list of storage options read from the filesystem") return r diff --git a/cmd/config/internal/commands/run_test.go b/cmd/config/internal/commands/run_test.go index a3e8f492d..33a400df6 100644 --- a/cmd/config/internal/commands/run_test.go +++ b/cmd/config/internal/commands/run_test.go @@ -219,12 +219,12 @@ apiVersion: v1 { name: "custom kind with storage mounts", args: []string{ - "run", "dir", "--mount", "type=bind;src=/mount/path;dst=/local/", - "--mount", "type=volume;src=myvol;dst=/local/", - "--mount", "type=tmpfs;dst=/local/", + "run", "dir", "--mount", "type=bind,src=/mount/path,dst=/local/", + "--mount", "type=volume,src=myvol,dst=/local/", + "--mount", "type=tmpfs,dst=/local/", "--image", "foo:bar", "--", "Foo", "g=h", "i=j=k"}, path: "dir", - mount: []string{"type=bind;src=/mount/path;dst=/local/", "type=volume;src=myvol;dst=/local/", "type=tmpfs;dst=/local/"}, + mount: []string{"type=bind,src=/mount/path,dst=/local/", "type=volume,src=myvol,dst=/local/", "type=tmpfs,dst=/local/"}, expected: ` metadata: name: function-input diff --git a/functions/examples/validator-kubeval/local-resource/example-use.yaml b/functions/examples/validator-kubeval/local-resource/example-use.yaml index 0c040422e..e1067b543 100644 --- a/functions/examples/validator-kubeval/local-resource/example-use.yaml +++ b/functions/examples/validator-kubeval/local-resource/example-use.yaml @@ -12,6 +12,12 @@ spec: strict: true ignoreMissingSchemas: true + # TODO: Update this to use network/volumes features. + # Relevant issues: + # - https://github.com/kubernetes-sigs/kustomize/issues/1901 + # - https://github.com/kubernetes-sigs/kustomize/issues/1902 + kubernetesVersion: "1.16.0" + schemaLocation: "file:///schemas" --- apiVersion: v1 kind: Service diff --git a/kyaml/kio/filters/container.go b/kyaml/kio/filters/container.go index 54dfdec57..9e6a681fb 100644 --- a/kyaml/kio/filters/container.go +++ b/kyaml/kio/filters/container.go @@ -162,7 +162,7 @@ func (s *StorageMount) String() string { func StringToStorageMount(s string) StorageMount { m := make(map[string]string) - options := strings.Split(s, ";") + options := strings.Split(s, ",") for _, option := range options { keyVal := strings.Split(option, "=") m[keyVal[0]] = keyVal[1] From 109ffdaec5d0b2266bd62ce817fc2e52a3c256da Mon Sep 17 00:00:00 2001 From: Prachi Pendse Date: Thu, 2 Apr 2020 12:12:27 -0700 Subject: [PATCH 5/5] Update parsing of StringToStorageMount to use SplitN --- kyaml/kio/filters/container.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kyaml/kio/filters/container.go b/kyaml/kio/filters/container.go index 9e6a681fb..f4294d3b8 100644 --- a/kyaml/kio/filters/container.go +++ b/kyaml/kio/filters/container.go @@ -164,7 +164,7 @@ func StringToStorageMount(s string) StorageMount { m := make(map[string]string) options := strings.Split(s, ",") for _, option := range options { - keyVal := strings.Split(option, "=") + keyVal := strings.SplitN(option, "=", 2) m[keyVal[0]] = keyVal[1] } var sm StorageMount