Merge pull request #12406 from runcom/remove-job-container-inspect

Remove job from container_inspect

api/client/attach.go

@@ -1,12 +1,13 @@
 package client
 
 import (
+	"encoding/json"
 	"fmt"
 	"io"
 	"net/url"
 
 	"github.com/Sirupsen/logrus"
-	"github.com/docker/docker/engine"
+	"github.com/docker/docker/api/types"
 	flag "github.com/docker/docker/pkg/mflag"
 	"github.com/docker/docker/pkg/signal"
 )
@@ -30,25 +31,20 @@ func (cli *DockerCli) CmdAttach(args ...string) error {
 		return err
 	}
 
-	env := engine.Env{}
+	var c types.ContainerJSON
-	if err := env.Decode(stream); err != nil {
+	if err := json.NewDecoder(stream).Decode(&c); err != nil {
 		return err
 	}
 
-	if !env.GetSubEnv("State").GetBool("Running") {
+	if !c.State.Running {
 		return fmt.Errorf("You cannot attach to a stopped container, start it first")
 	}
 
-	var (
+	if err := cli.CheckTtyInput(!*noStdin, c.Config.Tty); err != nil {
-		config = env.GetSubEnv("Config")
-		tty    = config.GetBool("Tty")
-	)
-
-	if err := cli.CheckTtyInput(!*noStdin, tty); err != nil {
 		return err
 	}
 
-	if tty && cli.isTerminalOut {
+	if c.Config.Tty && cli.isTerminalOut {
 		if err := cli.monitorTtySize(cmd.Arg(0), false); err != nil {
 			logrus.Debugf("Error monitoring TTY size: %s", err)
 		}
@@ -58,7 +54,7 @@ func (cli *DockerCli) CmdAttach(args ...string) error {
 
 	v := url.Values{}
 	v.Set("stream", "1")
-	if !*noStdin && config.GetBool("OpenStdin") {
+	if !*noStdin && c.Config.OpenStdin {
 		v.Set("stdin", "1")
 		in = cli.in
 	}
@@ -66,12 +62,12 @@ func (cli *DockerCli) CmdAttach(args ...string) error {
 	v.Set("stdout", "1")
 	v.Set("stderr", "1")
 
-	if *proxy && !tty {
+	if *proxy && !c.Config.Tty {
 		sigc := cli.forwardAllSignals(cmd.Arg(0))
 		defer signal.StopCatch(sigc)
 	}
 
-	if err := cli.hijack("POST", "/containers/"+cmd.Arg(0)+"/attach?"+v.Encode(), tty, in, cli.out, cli.err, nil, nil); err != nil {
+	if err := cli.hijack("POST", "/containers/"+cmd.Arg(0)+"/attach?"+v.Encode(), c.Config.Tty, in, cli.out, cli.err, nil, nil); err != nil {
 		return err
 	}
 

api/client/logs.go

@@ -1,10 +1,11 @@
 package client
 
 import (
+	"encoding/json"
 	"fmt"
 	"net/url"
 
-	"github.com/docker/docker/engine"
+	"github.com/docker/docker/api/types"
 	flag "github.com/docker/docker/pkg/mflag"
 )
 
@@ -29,12 +30,12 @@ func (cli *DockerCli) CmdLogs(args ...string) error {
 		return err
 	}
 
-	env := engine.Env{}
+	var c types.ContainerJSON
-	if err := env.Decode(stream); err != nil {
+	if err := json.NewDecoder(stream).Decode(&c); err != nil {
 		return err
 	}
 
-	if env.GetSubEnv("HostConfig").GetSubEnv("LogConfig").Get("Type") != "json-file" {
+	if c.HostConfig.LogConfig.Type != "json-file" {
 		return fmt.Errorf("\"logs\" command is supported only for \"json-file\" logging driver")
 	}
 
@@ -51,5 +52,5 @@ func (cli *DockerCli) CmdLogs(args ...string) error {
 	}
 	v.Set("tail", *tail)
 
-	return cli.streamHelper("GET", "/containers/"+name+"/logs?"+v.Encode(), env.GetSubEnv("Config").GetBool("Tty"), nil, cli.out, cli.err, nil)
+	return cli.streamHelper("GET", "/containers/"+name+"/logs?"+v.Encode(), c.Config.Tty, nil, cli.out, cli.err, nil)
 }

api/client/utils.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/api"
+	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/autogen/dockerversion"
 	"github.com/docker/docker/engine"
 	"github.com/docker/docker/pkg/jsonmessage"
@@ -238,11 +239,12 @@ func waitForExit(cli *DockerCli, containerID string) (int, error) {
 		return -1, err
 	}
 
-	var out engine.Env
+	var res types.ContainerWaitResponse
-	if err := out.Decode(stream); err != nil {
+	if err := json.NewDecoder(stream).Decode(&res); err != nil {
 		return -1, err
 	}
-	return out.GetInt("StatusCode"), nil
+
+	return res.StatusCode, nil
 }
 
 // getExitCode perform an inspect on the container. It returns
@@ -257,13 +259,12 @@ func getExitCode(cli *DockerCli, containerID string) (bool, int, error) {
 		return false, -1, nil
 	}
 
-	var result engine.Env
+	var c types.ContainerJSON
-	if err := result.Decode(stream); err != nil {
+	if err := json.NewDecoder(stream).Decode(&c); err != nil {
 		return false, -1, err
 	}
 
-	state := result.GetSubEnv("State")
+	return c.State.Running, c.State.ExitCode, nil
-	return state.GetBool("Running"), state.GetInt("ExitCode"), nil
 }
 
 // getExecExitCode perform an inspect on the exec command. It returns
@@ -278,12 +279,18 @@ func getExecExitCode(cli *DockerCli, execID string) (bool, int, error) {
 		return false, -1, nil
 	}
 
-	var result engine.Env
+	//TODO: Should we reconsider having a type in api/types?
-	if err := result.Decode(stream); err != nil {
+	//this is a response to exex/id/json not container
+	var c struct {
+		Running  bool
+		ExitCode int
+	}
+
+	if err := json.NewDecoder(stream).Decode(&c); err != nil {
 		return false, -1, err
 	}
 
-	return result.GetBool("Running"), result.GetInt("ExitCode"), nil
+	return c.Running, c.ExitCode, nil
 }
 
 func (cli *DockerCli) monitorTtySize(id string, isExec bool) error {

api/common.go

@@ -10,7 +10,6 @@ import (
 
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/api/types"
-	"github.com/docker/docker/pkg/parsers"
 	"github.com/docker/docker/pkg/version"
 	"github.com/docker/libtrust"
 )
@@ -18,19 +17,9 @@ import (
 // Common constants for daemon and client.
 const (
 	APIVERSION            version.Version = "1.19"       // Current REST API version
-	DEFAULTHTTPHOST                       = "127.0.0.1"            // Default HTTP Host used if only port is provided to -H flag e.g. docker -d -H tcp://:8080
-	DEFAULTUNIXSOCKET                     = "/var/run/docker.sock" // Docker daemon by default always listens on the default unix socket
 	DefaultDockerfileName string          = "Dockerfile" // Default filename with Docker commands, read by docker build
 )
 
-func ValidateHost(val string) (string, error) {
-	host, err := parsers.ParseHost(DEFAULTHTTPHOST, DEFAULTUNIXSOCKET, val)
-	if err != nil {
-		return val, err
-	}
-	return host, nil
-}
-
 type ByPrivatePort []types.Port
 
 func (r ByPrivatePort) Len() int           { return len(r) }

api/server/server.go

@@ -1210,12 +1210,21 @@ func (s *Server) getContainersByName(eng *engine.Engine, version version.Version
 	if vars == nil {
 		return fmt.Errorf("Missing parameter")
 	}
-	var job = eng.Job("container_inspect", vars["name"])
+
+	name := vars["name"]
+
 	if version.LessThan("1.12") {
-		job.SetenvBool("raw", true)
+		containerJSONRaw, err := s.daemon.ContainerInspectRaw(name)
+		if err != nil {
+			return err
 		}
-	streamJSON(job.Stdout, w, false)
+		return writeJSON(w, http.StatusOK, containerJSONRaw)
-	return job.Run()
+	}
+	containerJSON, err := s.daemon.ContainerInspect(name)
+	if err != nil {
+		return err
+	}
+	return writeJSON(w, http.StatusOK, containerJSON)
 }
 
 func (s *Server) getExecByID(eng *engine.Engine, version version.Version, w http.ResponseWriter, r *http.Request, vars map[string]string) error {

api/server/server_unit_test.go

@@ -32,41 +32,6 @@ func TesthttpError(t *testing.T) {
 	}
 }
 
-func TestGetContainersByName(t *testing.T) {
-	eng := engine.New()
-	name := "container_name"
-	var called bool
-	eng.Register("container_inspect", func(job *engine.Job) error {
-		called = true
-		if job.Args[0] != name {
-			t.Errorf("name != '%s': %#v", name, job.Args[0])
-		}
-		if api.APIVERSION.LessThan("1.12") && !job.GetenvBool("dirty") {
-			t.Errorf("dirty env variable not set")
-		} else if api.APIVERSION.GreaterThanOrEqualTo("1.12") && job.GetenvBool("dirty") {
-			t.Errorf("dirty env variable set when it shouldn't")
-		}
-		v := &engine.Env{}
-		v.SetBool("dirty", true)
-		if _, err := v.WriteTo(job.Stdout); err != nil {
-			return err
-		}
-		return nil
-	})
-	r := serveRequest("GET", "/containers/"+name+"/json", nil, eng, t)
-	if !called {
-		t.Fatal("handler was not called")
-	}
-	assertContentType(r, "application/json", t)
-	var stdoutJson interface{}
-	if err := json.Unmarshal(r.Body.Bytes(), &stdoutJson); err != nil {
-		t.Fatalf("%#v", err)
-	}
-	if stdoutJson.(map[string]interface{})["dirty"].(float64) != 1 {
-		t.Fatalf("%#v", stdoutJson)
-	}
-}
-
 func TestGetImagesByName(t *testing.T) {
 	eng := engine.New()
 	name := "image_name"

api/types/types.go

@@ -1,6 +1,12 @@
 package types
 
-import "github.com/docker/docker/pkg/version"
+import (
+	"time"
+
+	"github.com/docker/docker/daemon/network"
+	"github.com/docker/docker/pkg/version"
+	"github.com/docker/docker/runconfig"
+)
 
 // ContainerCreateResponse contains the information returned to a client on the
 // creation of a new container.
@@ -162,3 +168,43 @@ type ExecStartCheck struct {
 	// Check if there's a tty
 	Tty bool
 }
+
+type ContainerState struct {
+	Running    bool
+	Paused     bool
+	Restarting bool
+	OOMKilled  bool
+	Dead       bool
+	Pid        int
+	ExitCode   int
+	Error      string
+	StartedAt  time.Time
+	FinishedAt time.Time
+}
+
+// GET "/containers/{name:.*}/json"
+type ContainerJSON struct {
+	Id              string
+	Created         time.Time
+	Path            string
+	Args            []string
+	Config          *runconfig.Config
+	State           *ContainerState
+	Image           string
+	NetworkSettings *network.Settings
+	ResolvConfPath  string
+	HostnamePath    string
+	HostsPath       string
+	LogPath         string
+	Name            string
+	RestartCount    int
+	Driver          string
+	ExecDriver      string
+	MountLabel      string
+	ProcessLabel    string
+	Volumes         map[string]string
+	VolumesRW       map[string]bool
+	AppArmorProfile string
+	ExecIDs         []string
+	HostConfig      *runconfig.HostConfig
+}

daemon/daemon.go

@@ -116,13 +116,6 @@ type Daemon struct {
 
 // Install installs daemon capabilities to eng.
 func (daemon *Daemon) Install(eng *engine.Engine) error {
-	for name, method := range map[string]engine.Handler{
-		"container_inspect": daemon.ContainerInspect,
-	} {
-		if err := eng.Register(name, method); err != nil {
-			return err
-		}
-	}
 	if err := daemon.Repositories().Install(eng); err != nil {
 		return err
 	}

daemon/inspect.go

@@ -1,83 +1,92 @@
 package daemon
 
 import (
-	"encoding/json"
 	"fmt"
 
-	"github.com/docker/docker/engine"
+	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/runconfig"
 )
 
-func (daemon *Daemon) ContainerInspect(job *engine.Job) error {
+type ContainerJSONRaw struct {
-	if len(job.Args) != 1 {
+	*Container
-		return fmt.Errorf("usage: %s NAME", job.Name)
+	HostConfig *runconfig.HostConfig
 }
-	name := job.Args[0]
+
+func (daemon *Daemon) ContainerInspectRaw(name string) (*ContainerJSONRaw, error) {
 	container, err := daemon.Get(name)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	container.Lock()
 	defer container.Unlock()
-	if job.GetenvBool("raw") {
+
-		b, err := json.Marshal(&struct {
+	return &ContainerJSONRaw{container, container.hostConfig}, nil
-			*Container
+}
-			HostConfig *runconfig.HostConfig
+
-		}{container, container.hostConfig})
+func (daemon *Daemon) ContainerInspect(name string) (*types.ContainerJSON, error) {
+	container, err := daemon.Get(name)
 	if err != nil {
-			return err
+		return nil, err
-		}
-		job.Stdout.Write(b)
-		return nil
 	}
 
-	out := &engine.Env{}
+	container.Lock()
-	out.SetJson("Id", container.ID)
+	defer container.Unlock()
-	out.SetAuto("Created", container.Created)
-	out.SetJson("Path", container.Path)
-	out.SetList("Args", container.Args)
-	out.SetJson("Config", container.Config)
-	out.SetJson("State", container.State)
-	out.Set("Image", container.ImageID)
-	out.SetJson("NetworkSettings", container.NetworkSettings)
-	out.Set("ResolvConfPath", container.ResolvConfPath)
-	out.Set("HostnamePath", container.HostnamePath)
-	out.Set("HostsPath", container.HostsPath)
-	out.Set("LogPath", container.LogPath)
-	out.SetJson("Name", container.Name)
-	out.SetInt("RestartCount", container.RestartCount)
-	out.Set("Driver", container.Driver)
-	out.Set("ExecDriver", container.ExecDriver)
-	out.Set("MountLabel", container.MountLabel)
-	out.Set("ProcessLabel", container.ProcessLabel)
-	out.SetJson("Volumes", container.Volumes)
-	out.SetJson("VolumesRW", container.VolumesRW)
-	out.SetJson("AppArmorProfile", container.AppArmorProfile)
 
-	out.SetList("ExecIDs", container.GetExecIDs())
+	// make a copy to play with
+	hostConfig := *container.hostConfig
 
 	if children, err := daemon.Children(container.Name); err == nil {
 		for linkAlias, child := range children {
-			container.hostConfig.Links = append(container.hostConfig.Links, fmt.Sprintf("%s:%s", child.Name, linkAlias))
+			hostConfig.Links = append(hostConfig.Links, fmt.Sprintf("%s:%s", child.Name, linkAlias))
 		}
 	}
 	// we need this trick to preserve empty log driver, so
 	// container will use daemon defaults even if daemon change them
-	if container.hostConfig.LogConfig.Type == "" {
+	if hostConfig.LogConfig.Type == "" {
-		container.hostConfig.LogConfig = daemon.defaultLogConfig
+		hostConfig.LogConfig = daemon.defaultLogConfig
-		defer func() {
-			container.hostConfig.LogConfig = runconfig.LogConfig{}
-		}()
 	}
 
-	out.SetJson("HostConfig", container.hostConfig)
+	containerState := &types.ContainerState{
-
+		Running:    container.State.Running,
-	container.hostConfig.Links = nil
+		Paused:     container.State.Paused,
-	if _, err := out.WriteTo(job.Stdout); err != nil {
+		Restarting: container.State.Restarting,
-		return err
+		OOMKilled:  container.State.OOMKilled,
+		Dead:       container.State.Dead,
+		Pid:        container.State.Pid,
+		ExitCode:   container.State.ExitCode,
+		Error:      container.State.Error,
+		StartedAt:  container.State.StartedAt,
+		FinishedAt: container.State.FinishedAt,
 	}
-	return nil
+
+	contJSON := &types.ContainerJSON{
+		Id:              container.ID,
+		Created:         container.Created,
+		Path:            container.Path,
+		Args:            container.Args,
+		Config:          container.Config,
+		State:           containerState,
+		Image:           container.ImageID,
+		NetworkSettings: container.NetworkSettings,
+		ResolvConfPath:  container.ResolvConfPath,
+		HostnamePath:    container.HostnamePath,
+		HostsPath:       container.HostsPath,
+		LogPath:         container.LogPath,
+		Name:            container.Name,
+		RestartCount:    container.RestartCount,
+		Driver:          container.Driver,
+		ExecDriver:      container.ExecDriver,
+		MountLabel:      container.MountLabel,
+		ProcessLabel:    container.ProcessLabel,
+		Volumes:         container.Volumes,
+		VolumesRW:       container.VolumesRW,
+		AppArmorProfile: container.AppArmorProfile,
+		ExecIDs:         container.GetExecIDs(),
+		HostConfig:      &hostConfig,
+	}
+
+	return contJSON, nil
 }
 
 func (daemon *Daemon) ContainerExecInspect(id string) (*execConfig, error) {

docker/docker.go

@@ -9,9 +9,9 @@ import (
 	"strings"
 
 	"github.com/Sirupsen/logrus"
-	"github.com/docker/docker/api"
 	"github.com/docker/docker/api/client"
 	"github.com/docker/docker/autogen/dockerversion"
+	"github.com/docker/docker/opts"
 	flag "github.com/docker/docker/pkg/mflag"
 	"github.com/docker/docker/pkg/reexec"
 	"github.com/docker/docker/pkg/term"
@@ -63,9 +63,9 @@ func main() {
 		defaultHost := os.Getenv("DOCKER_HOST")
 		if defaultHost == "" || *flDaemon {
 			// If we do not have a host, default to unix socket
-			defaultHost = fmt.Sprintf("unix://%s", api.DEFAULTUNIXSOCKET)
+			defaultHost = fmt.Sprintf("unix://%s", opts.DefaultUnixSocket)
 		}
-		defaultHost, err := api.ValidateHost(defaultHost)
+		defaultHost, err := opts.ValidateHost(defaultHost)
 		if err != nil {
 			logrus.Fatal(err)
 		}

integration-cli/docker_utils.go

@@ -20,7 +20,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/docker/docker/api"
+	"github.com/docker/docker/opts"
 	"github.com/docker/docker/pkg/ioutils"
 	"github.com/docker/docker/pkg/stringutils"
 	"github.com/go-check/check"
@@ -274,7 +274,7 @@ func (d *Daemon) LogfileName() string {
 }
 
 func daemonHost() string {
-	daemonUrlStr := "unix://" + api.DEFAULTUNIXSOCKET
+	daemonUrlStr := "unix://" + opts.DefaultUnixSocket
 	if daemonHostVar := os.Getenv("DOCKER_HOST"); daemonHostVar != "" {
 		daemonUrlStr = daemonHostVar
 	}

opts/opts.go

@@ -8,16 +8,16 @@ import (
 	"regexp"
 	"strings"
 
-	"github.com/docker/docker/api"
 	flag "github.com/docker/docker/pkg/mflag"
 	"github.com/docker/docker/pkg/parsers"
 	"github.com/docker/docker/pkg/ulimit"
-	"github.com/docker/docker/utils"
 )
 
 var (
 	alphaRegexp       = regexp.MustCompile(`[a-zA-Z]`)
 	domainRegexp      = regexp.MustCompile(`^(:?(:?[a-zA-Z0-9]|(:?[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9]))(:?\.(:?[a-zA-Z0-9]|(:?[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])))*)\.?\s*$`)
+	DefaultHTTPHost   = "127.0.0.1"            // Default HTTP Host used if only port is provided to -H flag e.g. docker -d -H tcp://:8080
+	DefaultUnixSocket = "/var/run/docker.sock" // Docker daemon by default always listens on the default unix socket
 )
 
 func ListVar(values *[]string, names []string, usage string) {
@@ -25,7 +25,7 @@ func ListVar(values *[]string, names []string, usage string) {
 }
 
 func HostListVar(values *[]string, names []string, usage string) {
-	flag.Var(newListOptsRef(values, api.ValidateHost), names, usage)
+	flag.Var(newListOptsRef(values, ValidateHost), names, usage)
 }
 
 func IPListVar(values *[]string, names []string, usage string) {
@@ -174,7 +174,7 @@ func ValidateEnv(val string) (string, error) {
 	if len(arr) > 1 {
 		return val, nil
 	}
-	if !utils.DoesEnvExist(val) {
+	if !doesEnvExist(val) {
 		return val, nil
 	}
 	return fmt.Sprintf("%s=%s", val, os.Getenv(val)), nil
@@ -234,3 +234,21 @@ func ValidateLabel(val string) (string, error) {
 	}
 	return val, nil
 }
+
+func ValidateHost(val string) (string, error) {
+	host, err := parsers.ParseHost(DefaultHTTPHost, DefaultUnixSocket, val)
+	if err != nil {
+		return val, err
+	}
+	return host, nil
+}
+
+func doesEnvExist(name string) bool {
+	for _, entry := range os.Environ() {
+		parts := strings.SplitN(entry, "=", 2)
+		if parts[0] == name {
+			return true
+		}
+	}
+	return false
+}

utils/utils.go

@@ -239,16 +239,6 @@ func ReplaceOrAppendEnvValues(defaults, overrides []string) []string {
 	return defaults
 }
 
-func DoesEnvExist(name string) bool {
-	for _, entry := range os.Environ() {
-		parts := strings.SplitN(entry, "=", 2)
-		if parts[0] == name {
-			return true
-		}
-	}
-	return false
-}
-
 // ValidateContextDirectory checks if all the contents of the directory
 // can be read and returns an error if some files can't be read
 // symlinks which point to non-existing files don't trigger an error