Merge pull request #3833 from vieux/fix_env_conversion_issues

Fix env conversion issues in env

config.go

@@ -23,29 +23,31 @@ type DaemonConfig struct {
 
 // ConfigFromJob creates and returns a new DaemonConfig object
 // by parsing the contents of a job's environment.
-func ConfigFromJob(job *engine.Job) *DaemonConfig {
-	var config DaemonConfig
-	config.Pidfile = job.Getenv("Pidfile")
-	config.Root = job.Getenv("Root")
-	config.AutoRestart = job.GetenvBool("AutoRestart")
+func DaemonConfigFromJob(job *engine.Job) *DaemonConfig {
+	config := &DaemonConfig{
+		Pidfile:                     job.Getenv("Pidfile"),
+		Root:                        job.Getenv("Root"),
+		AutoRestart:                 job.GetenvBool("AutoRestart"),
+		EnableIptables:              job.GetenvBool("EnableIptables"),
+		EnableIpForward:             job.GetenvBool("EnableIpForward"),
+		BridgeIp:                    job.Getenv("BridgeIp"),
+		DefaultIp:                   net.ParseIP(job.Getenv("DefaultIp")),
+		InterContainerCommunication: job.GetenvBool("InterContainerCommunication"),
+		GraphDriver:                 job.Getenv("GraphDriver"),
+	}
 	if dns := job.GetenvList("Dns"); dns != nil {
 		config.Dns = dns
 	}
-	config.EnableIptables = job.GetenvBool("EnableIptables")
-	config.EnableIpForward = job.GetenvBool("EnableIpForward")
 	if br := job.Getenv("BridgeIface"); br != "" {
 		config.BridgeIface = br
 	} else {
 		config.BridgeIface = DefaultNetworkBridge
 	}
-	config.BridgeIp = job.Getenv("BridgeIp")
-	config.DefaultIp = net.ParseIP(job.Getenv("DefaultIp"))
-	config.InterContainerCommunication = job.GetenvBool("InterContainerCommunication")
-	config.GraphDriver = job.Getenv("GraphDriver")
 	if mtu := job.GetenvInt("Mtu"); mtu != -1 {
 		config.Mtu = mtu
 	} else {
 		config.Mtu = DefaultNetworkMtu
 	}
-	return &config
+
+	return config
 }

container.go

@@ -104,6 +104,46 @@ type Config struct {
 	NetworkDisabled bool
 }
 
+func ContainerConfigFromJob(job *engine.Job) *Config {
+	config := &Config{
+		Hostname:        job.Getenv("Hostname"),
+		Domainname:      job.Getenv("Domainname"),
+		User:            job.Getenv("User"),
+		Memory:          job.GetenvInt64("Memory"),
+		MemorySwap:      job.GetenvInt64("MemorySwap"),
+		CpuShares:       job.GetenvInt64("CpuShares"),
+		AttachStdin:     job.GetenvBool("AttachStdin"),
+		AttachStdout:    job.GetenvBool("AttachStdout"),
+		AttachStderr:    job.GetenvBool("AttachStderr"),
+		Tty:             job.GetenvBool("Tty"),
+		OpenStdin:       job.GetenvBool("OpenStdin"),
+		StdinOnce:       job.GetenvBool("StdinOnce"),
+		Image:           job.Getenv("Image"),
+		VolumesFrom:     job.Getenv("VolumesFrom"),
+		WorkingDir:      job.Getenv("WorkingDir"),
+		NetworkDisabled: job.GetenvBool("NetworkDisabled"),
+	}
+	job.GetenvJson("ExposedPorts", &config.ExposedPorts)
+	job.GetenvJson("Volumes", &config.Volumes)
+	if PortSpecs := job.GetenvList("PortSpecs"); PortSpecs != nil {
+		config.PortSpecs = PortSpecs
+	}
+	if Env := job.GetenvList("Env"); Env != nil {
+		config.Env = Env
+	}
+	if Cmd := job.GetenvList("Cmd"); Cmd != nil {
+		config.Cmd = Cmd
+	}
+	if Dns := job.GetenvList("Dns"); Dns != nil {
+		config.Dns = Dns
+	}
+	if Entrypoint := job.GetenvList("Entrypoint"); Entrypoint != nil {
+		config.Entrypoint = Entrypoint
+	}
+
+	return config
+}
+
 type HostConfig struct {
 	Binds           []string
 	ContainerIDFile string
@@ -114,6 +154,24 @@ type HostConfig struct {
 	PublishAllPorts bool
 }
 
+func ContainerHostConfigFromJob(job *engine.Job) *HostConfig {
+	hostConfig := &HostConfig{
+		ContainerIDFile: job.Getenv("ContainerIDFile"),
+		Privileged:      job.GetenvBool("Privileged"),
+		PublishAllPorts: job.GetenvBool("PublishAllPorts"),
+	}
+	job.GetenvJson("LxcConf", &hostConfig.LxcConf)
+	job.GetenvJson("PortBindings", &hostConfig.PortBindings)
+	if Binds := job.GetenvList("Binds"); Binds != nil {
+		hostConfig.Binds = Binds
+	}
+	if Links := job.GetenvList("Links"); Links != nil {
+		hostConfig.Links = Links
+	}
+
+	return hostConfig
+}
+
 type BindMap struct {
 	SrcPath string
 	DstPath string

engine/env.go

@@ -191,24 +191,6 @@ func (env *Env) WriteTo(dst io.Writer) (n int64, err error) {
 	return 0, env.Encode(dst)
 }
 
-func (env *Env) Export(dst interface{}) (err error) {
-	defer func() {
-		if err != nil {
-			err = fmt.Errorf("ExportEnv %s", err)
-		}
-	}()
-	var buf bytes.Buffer
-	// step 1: encode/marshal the env to an intermediary json representation
-	if err := env.Encode(&buf); err != nil {
-		return err
-	}
-	// step 2: decode/unmarshal the intermediary json into the destination object
-	if err := json.NewDecoder(&buf).Decode(dst); err != nil {
-		return err
-	}
-	return nil
-}
-
 func (env *Env) Import(src interface{}) (err error) {
 	defer func() {
 		if err != nil {

engine/env_test.go

@@ -84,32 +84,6 @@ func TestSetenvList(t *testing.T) {
 	}
 }
 
-func TestImportEnv(t *testing.T) {
-	type dummy struct {
-		DummyInt         int
-		DummyStringArray []string
-	}
-
-	job := mkJob(t, "dummy")
-	if err := job.ImportEnv(&dummy{42, []string{"foo", "bar"}}); err != nil {
-		t.Fatal(err)
-	}
-
-	dmy := dummy{}
-	if err := job.ExportEnv(&dmy); err != nil {
-		t.Fatal(err)
-	}
-
-	if dmy.DummyInt != 42 {
-		t.Fatalf("Expected 42, got %d", dmy.DummyInt)
-	}
-
-	if len(dmy.DummyStringArray) != 2 || dmy.DummyStringArray[0] != "foo" || dmy.DummyStringArray[1] != "bar" {
-		t.Fatalf("Expected {foo, bar}, got %v", dmy.DummyStringArray)
-	}
-
-}
-
 func TestEnviron(t *testing.T) {
 	job := mkJob(t, "dummy")
 	job.Setenv("foo", "bar")

engine/job.go

@@ -164,10 +164,6 @@ func (job *Job) EncodeEnv(dst io.Writer) error {
 	return job.env.Encode(dst)
 }
 
-func (job *Job) ExportEnv(dst interface{}) (err error) {
-	return job.env.Export(dst)
-}
-
 func (job *Job) ImportEnv(src interface{}) (err error) {
 	return job.env.Import(src)
 }

integration/server_test.go

@@ -114,6 +114,30 @@ func TestCreateRm(t *testing.T) {
 
 }
 
+func TestCreateNumberHostname(t *testing.T) {
+	eng := NewTestEngine(t)
+	defer mkRuntimeFromEngine(eng, t).Nuke()
+
+	config, _, _, err := docker.ParseRun([]string{"-h", "web.0", unitTestImageID, "echo test"}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	createTestContainer(eng, config, t)
+}
+
+func TestCreateNumberUsername(t *testing.T) {
+	eng := NewTestEngine(t)
+	defer mkRuntimeFromEngine(eng, t).Nuke()
+
+	config, _, _, err := docker.ParseRun([]string{"-u", "1002", unitTestImageID, "echo test"}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	createTestContainer(eng, config, t)
+}
+
 func TestCreateRmVolumes(t *testing.T) {
 	eng := NewTestEngine(t)
 	defer mkRuntimeFromEngine(eng, t).Nuke()

server.go

@@ -42,8 +42,7 @@ func init() {
 // The signals SIGINT, SIGQUIT and SIGTERM are intercepted for cleanup.
 func jobInitApi(job *engine.Job) engine.Status {
 	job.Logf("Creating server")
-	// FIXME: ImportEnv deprecates ConfigFromJob
-	srv, err := NewServer(job.Eng, ConfigFromJob(job))
+	srv, err := NewServer(job.Eng, DaemonConfigFromJob(job))
 	if err != nil {
 		job.Error(err)
 		return engine.StatusErr
@@ -1742,11 +1741,7 @@ func (srv *Server) ContainerCreate(job *engine.Job) engine.Status {
 		job.Printf("Usage: %s", job.Name)
 		return engine.StatusErr
 	}
-	var config Config
-	if err := job.ExportEnv(&config); err != nil {
-		job.Error(err)
-		return engine.StatusErr
-	}
+	config := ContainerConfigFromJob(job)
 	if config.Memory != 0 && config.Memory < 524288 {
 		job.Errorf("Minimum memory limit allowed is 512k")
 		return engine.StatusErr
@@ -1769,7 +1764,7 @@ func (srv *Server) ContainerCreate(job *engine.Job) engine.Status {
 		config.Dns = defaultDns
 	}
 
-	container, buildWarnings, err := srv.runtime.Create(&config, name)
+	container, buildWarnings, err := srv.runtime.Create(config, name)
 	if err != nil {
 		if srv.runtime.graph.IsNotExist(err) {
 			_, tag := utils.ParseRepositoryTag(config.Image)
@@ -2196,11 +2191,7 @@ func (srv *Server) ContainerStart(job *engine.Job) engine.Status {
 	}
 	// If no environment was set, then no hostconfig was passed.
 	if len(job.Environ()) > 0 {
-		var hostConfig HostConfig
-		if err := job.ExportEnv(&hostConfig); err != nil {
-			job.Error(err)
-			return engine.StatusErr
-		}
+		hostConfig := ContainerHostConfigFromJob(job)
 		// Validate the HostConfig binds. Make sure that:
 		// 1) the source of a bind mount isn't /
 		//         The bind mount "/:/foo" isn't allowed.
@@ -2227,11 +2218,11 @@ func (srv *Server) ContainerStart(job *engine.Job) engine.Status {
 			}
 		}
 		// Register any links from the host config before starting the container
-		if err := srv.RegisterLinks(container, &hostConfig); err != nil {
+		if err := srv.RegisterLinks(container, hostConfig); err != nil {
 			job.Error(err)
 			return engine.StatusErr
 		}
-		container.hostConfig = &hostConfig
+		container.hostConfig = hostConfig
 		container.ToDisk()
 	}
 	if err := container.Start(); err != nil {