Merge pull request #24844 from yongtang/24712-create-service-env-file

Add `--env-file` flag to `docker create service`

cli/command/service/create.go

@@ -32,6 +32,7 @@ func newCreateCommand(dockerCli *command.DockerCli) *cobra.Command {
 	flags.VarP(&opts.labels, flagLabel, "l", "Service labels")
 	flags.Var(&opts.containerLabels, flagContainerLabel, "Container labels")
 	flags.VarP(&opts.env, flagEnv, "e", "Set environment variables")
+	flags.Var(&opts.envFile, flagEnvFile, "Read in a file of environment variables")
 	flags.Var(&opts.mounts, flagMount, "Attach a mount to the service")
 	flags.StringSliceVar(&opts.constraints, flagConstraint, []string{}, "Placement constraints")
 	flags.StringSliceVar(&opts.networks, flagNetwork, []string{}, "Network attachments")

cli/command/service/opts.go

@@ -395,6 +395,7 @@ type serviceOptions struct {
 	image           string
 	args            []string
 	env             opts.ListOpts
+	envFile         opts.ListOpts
 	workdir         string
 	user            string
 	groups          []string
@@ -422,6 +423,7 @@ func newServiceOptions() *serviceOptions {
 		labels:          opts.NewListOpts(runconfigopts.ValidateEnv),
 		containerLabels: opts.NewListOpts(runconfigopts.ValidateEnv),
 		env:             opts.NewListOpts(runconfigopts.ValidateEnv),
+		envFile:         opts.NewListOpts(nil),
 		endpoint: endpointOptions{
 			ports: opts.NewListOpts(ValidatePort),
 		},
@@ -432,6 +434,25 @@ func newServiceOptions() *serviceOptions {
 func (opts *serviceOptions) ToService() (swarm.ServiceSpec, error) {
 	var service swarm.ServiceSpec
 
+	envVariables, err := runconfigopts.ReadKVStrings(opts.envFile.GetAll(), opts.env.GetAll())
+	if err != nil {
+		return service, err
+	}
+
+	currentEnv := make([]string, 0, len(envVariables))
+	for _, env := range envVariables { // need to process each var, in order
+		k := strings.SplitN(env, "=", 2)[0]
+		for i, current := range currentEnv { // remove duplicates
+			if current == env {
+				continue // no update required, may hide this behind flag to preserve order of envVariables
+			}
+			if strings.HasPrefix(current, k+"=") {
+				currentEnv = append(currentEnv[:i], currentEnv[i+1:]...)
+			}
+		}
+		currentEnv = append(currentEnv, env)
+	}
+
 	service = swarm.ServiceSpec{
 		Annotations: swarm.Annotations{
 			Name:   opts.name,
@@ -441,7 +462,7 @@ func (opts *serviceOptions) ToService() (swarm.ServiceSpec, error) {
 			ContainerSpec: swarm.ContainerSpec{
 				Image:           opts.image,
 				Args:            opts.args,
-				Env:             opts.env.GetAll(),
+				Env:             currentEnv,
 				Labels:          runconfigopts.ConvertKVStringsToMap(opts.containerLabels.GetAll()),
 				Dir:             opts.workdir,
 				User:            opts.user,
@@ -532,6 +553,7 @@ const (
 	flagContainerLabelAdd     = "container-label-add"
 	flagEndpointMode          = "endpoint-mode"
 	flagEnv                   = "env"
+	flagEnvFile               = "env-file"
 	flagEnvRemove             = "env-rm"
 	flagEnvAdd                = "env-add"
 	flagGroupAdd              = "group-add"

docs/reference/commandline/service_create.md

@@ -25,6 +25,7 @@ Options:
       --container-label value            Service container labels (default [])
       --endpoint-mode string             Endpoint mode (vip or dnsrr)
   -e, --env value                        Set environment variables (default [])
+      --env-file value                   Read in a file of environment variables (default [])
       --group-add value                  Add additional user groups to the container (default [])
       --help                             Print usage
   -l, --label value                      Service labels (default [])

integration-cli/docker_cli_swarm_test.go

@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"path/filepath"
 	"strings"
 	"time"
 
@@ -568,3 +569,22 @@ func (s *DockerSwarmSuite) TestSwarmNetworkPlugin(c *check.C) {
 	c.Assert(err, checker.IsNil)
 	c.Assert(strings.TrimSpace(out), checker.Equals, "foo")
 }
+
+// Test case for #24712
+func (s *DockerSwarmSuite) TestSwarmServiceEnvFile(c *check.C) {
+	d := s.AddDaemon(c, true, true)
+
+	path := filepath.Join(d.folder, "env.txt")
+	err := ioutil.WriteFile(path, []byte("VAR1=A\nVAR2=A\n"), 0644)
+	c.Assert(err, checker.IsNil)
+
+	name := "worker"
+	out, err := d.Cmd("service", "create", "--env-file", path, "--env", "VAR1=B", "--env", "VAR1=C", "--env", "VAR2=", "--env", "VAR2", "--name", name, "busybox", "top")
+	c.Assert(err, checker.IsNil)
+	c.Assert(strings.TrimSpace(out), checker.Not(checker.Equals), "")
+
+	// The complete env is [VAR1=A VAR2=A VAR1=B VAR1=C VAR2= VAR2] and duplicates will be removed => [VAR1=C VAR2]
+	out, err = d.Cmd("inspect", "--format", "{{ .Spec.TaskTemplate.ContainerSpec.Env }}", name)
+	c.Assert(err, checker.IsNil)
+	c.Assert(out, checker.Contains, "[VAR1=C VAR2]")
+}

runconfig/opts/parse.go

@@ -427,13 +427,13 @@ func Parse(flags *pflag.FlagSet, copts *ContainerOptions) (*container.Config, *c
 	}
 
 	// collect all the environment variables for the container
-	envVariables, err := readKVStrings(copts.envFile.GetAll(), copts.env.GetAll())
+	envVariables, err := ReadKVStrings(copts.envFile.GetAll(), copts.env.GetAll())
 	if err != nil {
 		return nil, nil, nil, err
 	}
 
 	// collect all the labels for the container
-	labels, err := readKVStrings(copts.labelsFile.GetAll(), copts.labels.GetAll())
+	labels, err := ReadKVStrings(copts.labelsFile.GetAll(), copts.labels.GetAll())
 	if err != nil {
 		return nil, nil, nil, err
 	}
@@ -663,9 +663,9 @@ func Parse(flags *pflag.FlagSet, copts *ContainerOptions) (*container.Config, *c
 	return config, hostConfig, networkingConfig, nil
 }
 
-// reads a file of line terminated key=value pairs, and overrides any keys
+// ReadKVStrings reads a file of line terminated key=value pairs, and overrides any keys
 // present in the file with additional pairs specified in the override parameter
-func readKVStrings(files []string, override []string) ([]string, error) {
+func ReadKVStrings(files []string, override []string) ([]string, error) {
 	envVariables := []string{}
 	for _, ef := range files {
 		parsedVars, err := ParseEnvFile(ef)