Add WithEnv and WithMount oci options

Signed-off-by: Paul Knopf <pauldotknopf@gmail.com>

cmd/ctr/commands/run/run.go

@@ -19,26 +19,17 @@ import (
 	"github.com/urfave/cli"
 )
 
-func withEnv(context *cli.Context) oci.SpecOpts {
-	return func(_ gocontext.Context, _ oci.Client, _ *containers.Container, s *specs.Spec) error {
-		env := context.StringSlice("env")
-		if len(env) > 0 {
-			s.Process.Env = replaceOrAppendEnvValues(s.Process.Env, env)
-		}
-		return nil
-	}
-}
-
 func withMounts(context *cli.Context) oci.SpecOpts {
-	return func(_ gocontext.Context, _ oci.Client, _ *containers.Container, s *specs.Spec) error {
+	return func(ctx gocontext.Context, client oci.Client, container *containers.Container, s *specs.Spec) error {
+		mounts := make([]specs.Mount, 0)
 		for _, mount := range context.StringSlice("mount") {
 			m, err := parseMountFlag(mount)
 			if err != nil {
 				return err
 			}
-			s.Mounts = append(s.Mounts, m)
+			mounts = append(mounts, m)
 		}
-		return nil
+		return oci.WithMounts(mounts)(ctx, client, container, s)
 	}
 }
 
@@ -77,44 +68,6 @@ func parseMountFlag(m string) (specs.Mount, error) {
 	return mount, nil
 }
 
-// replaceOrAppendEnvValues returns the defaults with the overrides either
-// replaced by env key or appended to the list
-func replaceOrAppendEnvValues(defaults, overrides []string) []string {
-	cache := make(map[string]int, len(defaults))
-	for i, e := range defaults {
-		parts := strings.SplitN(e, "=", 2)
-		cache[parts[0]] = i
-	}
-
-	for _, value := range overrides {
-		// Values w/o = means they want this env to be removed/unset.
-		if !strings.Contains(value, "=") {
-			if i, exists := cache[value]; exists {
-				defaults[i] = "" // Used to indicate it should be removed
-			}
-			continue
-		}
-
-		// Just do a normal set/update
-		parts := strings.SplitN(value, "=", 2)
-		if i, exists := cache[parts[0]]; exists {
-			defaults[i] = value
-		} else {
-			defaults = append(defaults, value)
-		}
-	}
-
-	// Now remove all entries that we want to "unset"
-	for i := 0; i < len(defaults); i++ {
-		if defaults[i] == "" {
-			defaults = append(defaults[:i], defaults[i+1:]...)
-			i--
-		}
-	}
-
-	return defaults
-}
-
 // Command runs a container
 var Command = cli.Command{
 	Name:      "run",

cmd/ctr/commands/run/run_unix.go

@@ -61,8 +61,8 @@ func newContainer(ctx gocontext.Context, client *containerd.Client, context *cli
 		opts = append(opts, oci.WithRootFSReadonly())
 	}
 	cOpts = append(cOpts, containerd.WithRuntime(context.String("runtime"), nil))
-
+	opts = append(opts, oci.WithEnv(context.StringSlice("env")))
-	opts = append(opts, withEnv(context), withMounts(context))
+	opts = append(opts, withMounts(context))
 	if len(args) > 0 {
 		opts = append(opts, oci.WithProcessArgs(args...))
 	}

cmd/ctr/commands/run/run_windows.go

@@ -65,7 +65,7 @@ func newContainer(ctx gocontext.Context, client *containerd.Client, context *cli
 	opts := []oci.SpecOpts{
 		// TODO(mlaventure): use oci.WithImageConfig once we have a snapshotter
 		withLayers(context),
-		withEnv(context),
+		oci.WithEnv(context.StringSlice("env")),
 		withMounts(context),
 		withTTY(tty),
 	}

oci/spec_opts.go

@@ -2,6 +2,7 @@ package oci
 
 import (
 	"context"
+	"strings"
 
 	"github.com/containerd/containerd/containers"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
@@ -33,3 +34,59 @@ func WithHostname(name string) SpecOpts {
 		return nil
 	}
 }
+
+// WithEnv appends environment variables
+func WithEnv(environmnetVariables []string) SpecOpts {
+	return func(_ context.Context, _ Client, _ *containers.Container, s *specs.Spec) error {
+		if len(environmnetVariables) > 0 {
+			s.Process.Env = replaceOrAppendEnvValues(s.Process.Env, environmnetVariables)
+		}
+		return nil
+	}
+}
+
+// WithMounts appends mounts
+func WithMounts(mounts []specs.Mount) SpecOpts {
+	return func(_ context.Context, _ Client, _ *containers.Container, s *specs.Spec) error {
+		s.Mounts = append(s.Mounts, mounts...)
+		return nil
+	}
+}
+
+// replaceOrAppendEnvValues returns the defaults with the overrides either
+// replaced by env key or appended to the list
+func replaceOrAppendEnvValues(defaults, overrides []string) []string {
+	cache := make(map[string]int, len(defaults))
+	for i, e := range defaults {
+		parts := strings.SplitN(e, "=", 2)
+		cache[parts[0]] = i
+	}
+
+	for _, value := range overrides {
+		// Values w/o = means they want this env to be removed/unset.
+		if !strings.Contains(value, "=") {
+			if i, exists := cache[value]; exists {
+				defaults[i] = "" // Used to indicate it should be removed
+			}
+			continue
+		}
+
+		// Just do a normal set/update
+		parts := strings.SplitN(value, "=", 2)
+		if i, exists := cache[parts[0]]; exists {
+			defaults[i] = value
+		} else {
+			defaults = append(defaults, value)
+		}
+	}
+
+	// Now remove all entries that we want to "unset"
+	for i := 0; i < len(defaults); i++ {
+		if defaults[i] == "" {
+			defaults = append(defaults[:i], defaults[i+1:]...)
+			i--
+		}
+	}
+
+	return defaults
+}

oci/spec_opts_test.go

@@ -0,0 +1,73 @@
+package oci
+
+import (
+	"testing"
+
+	specs "github.com/opencontainers/runtime-spec/specs-go"
+)
+
+func TestWithEnv(t *testing.T) {
+	t.Parallel()
+
+	s := specs.Spec{}
+	s.Process = &specs.Process{
+		Env: []string{"DEFAULT=test"},
+	}
+
+	WithEnv([]string{"env=1"})(nil, nil, nil, &s)
+
+	if len(s.Process.Env) != 2 {
+		t.Fatal("didn't append")
+	}
+
+	WithEnv([]string{"env2=1"})(nil, nil, nil, &s)
+
+	if len(s.Process.Env) != 3 {
+		t.Fatal("didn't append")
+	}
+
+	WithEnv([]string{"env2=2"})(nil, nil, nil, &s)
+
+	if s.Process.Env[2] != "env2=2" {
+		t.Fatal("could't update")
+	}
+
+	WithEnv([]string{"env2"})(nil, nil, nil, &s)
+
+	if len(s.Process.Env) != 2 {
+		t.Fatal("coudn't unset")
+	}
+}
+
+func TestWithMounts(t *testing.T) {
+
+	t.Parallel()
+
+	s := specs.Spec{
+		Mounts: []specs.Mount{
+			{
+				Source:      "default-source",
+				Destination: "default-dest",
+			},
+		},
+	}
+
+	WithMounts([]specs.Mount{
+		{
+			Source:      "new-source",
+			Destination: "new-dest",
+		},
+	})(nil, nil, nil, &s)
+
+	if len(s.Mounts) != 2 {
+		t.Fatal("didn't append")
+	}
+
+	if s.Mounts[1].Source != "new-source" {
+		t.Fatal("invaid mount")
+	}
+
+	if s.Mounts[1].Destination != "new-dest" {
+		t.Fatal("invaid mount")
+	}
+}