From 672edfe807c597a1c245bce996a150dfdf273a3c Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Fri, 14 Mar 2014 16:53:43 -0700 Subject: [PATCH 1/3] Remove the concept of a root dir out of engine This makes the engine more general purpose so that we can use it and the job routing functionality for reexec'ing our binary Docker-DCO-1.1-Signed-off-by: Michael Crosby (github: crosbymichael) Conflicts: integration/runtime_test.go --- docker/docker.go | 30 +++++++++++++++++++++++++- engine/engine.go | 42 +++---------------------------------- engine/engine_test.go | 42 ------------------------------------- engine/helpers_test.go | 7 +------ engine/job_test.go | 6 ------ integration/runtime_test.go | 7 +++---- integration/server_test.go | 2 +- integration/utils_test.go | 2 +- 8 files changed, 38 insertions(+), 100 deletions(-) diff --git a/docker/docker.go b/docker/docker.go index a4cbe8d865..aed3b0778d 100644 --- a/docker/docker.go +++ b/docker/docker.go @@ -7,6 +7,7 @@ import ( "io/ioutil" "log" "os" + "runtime" "strings" "github.com/dotcloud/docker/api" @@ -121,7 +122,10 @@ func main() { } } - eng, err := engine.New(realRoot) + if err := checkKernelAndArch(); err != nil { + log.Fatal(err) + } + eng, err := engine.New() if err != nil { log.Fatal(err) } @@ -239,3 +243,27 @@ func main() { func showVersion() { fmt.Printf("Docker version %s, build %s\n", dockerversion.VERSION, dockerversion.GITCOMMIT) } + +func checkKernelAndArch() error { + // Check for unsupported architectures + if runtime.GOARCH != "amd64" { + return fmt.Errorf("The docker runtime currently only supports amd64 (not %s). This will change in the future. Aborting.", runtime.GOARCH) + } + // Check for unsupported kernel versions + // FIXME: it would be cleaner to not test for specific versions, but rather + // test for specific functionalities. + // Unfortunately we can't test for the feature "does not cause a kernel panic" + // without actually causing a kernel panic, so we need this workaround until + // the circumstances of pre-3.8 crashes are clearer. + // For details see http://github.com/dotcloud/docker/issues/407 + if k, err := utils.GetKernelVersion(); err != nil { + log.Printf("WARNING: %s\n", err) + } else { + if utils.CompareKernelVersion(k, &utils.KernelVersionInfo{Kernel: 3, Major: 8, Minor: 0}) < 0 { + if os.Getenv("DOCKER_NOWARN_KERNEL_VERSION") == "" { + log.Printf("WARNING: You are running linux kernel version %s, which might be unstable running docker. Please upgrade your kernel to 3.8.0.", k.String()) + } + } + } + return nil +} diff --git a/engine/engine.go b/engine/engine.go index 6a54b3591e..9cc1bd5004 100644 --- a/engine/engine.go +++ b/engine/engine.go @@ -5,9 +5,7 @@ import ( "fmt" "github.com/dotcloud/docker/utils" "io" - "log" "os" - "runtime" "sort" "strings" ) @@ -37,7 +35,6 @@ func unregister(name string) { // It acts as a store for *containers*, and allows manipulation of these // containers by executing *jobs*. type Engine struct { - root string handlers map[string]Handler hack Hack // data for temporary hackery (see hack.go) id string @@ -47,10 +44,6 @@ type Engine struct { Logging bool } -func (eng *Engine) Root() string { - return eng.root -} - func (eng *Engine) Register(name string, handler Handler) error { _, exists := eng.handlers[name] if exists { @@ -60,38 +53,9 @@ func (eng *Engine) Register(name string, handler Handler) error { return nil } -// New initializes a new engine managing the directory specified at `root`. -// `root` is used to store containers and any other state private to the engine. -// Changing the contents of the root without executing a job will cause unspecified -// behavior. -func New(root string) (*Engine, error) { - // Check for unsupported architectures - if runtime.GOARCH != "amd64" { - return nil, fmt.Errorf("The docker runtime currently only supports amd64 (not %s). This will change in the future. Aborting.", runtime.GOARCH) - } - // Check for unsupported kernel versions - // FIXME: it would be cleaner to not test for specific versions, but rather - // test for specific functionalities. - // Unfortunately we can't test for the feature "does not cause a kernel panic" - // without actually causing a kernel panic, so we need this workaround until - // the circumstances of pre-3.8 crashes are clearer. - // For details see http://github.com/dotcloud/docker/issues/407 - if k, err := utils.GetKernelVersion(); err != nil { - log.Printf("WARNING: %s\n", err) - } else { - if utils.CompareKernelVersion(k, &utils.KernelVersionInfo{Kernel: 3, Major: 8, Minor: 0}) < 0 { - if os.Getenv("DOCKER_NOWARN_KERNEL_VERSION") == "" { - log.Printf("WARNING: You are running linux kernel version %s, which might be unstable running docker. Please upgrade your kernel to 3.8.0.", k.String()) - } - } - } - - if err := os.MkdirAll(root, 0700); err != nil && !os.IsExist(err) { - return nil, err - } - +// New initializes a new engine. +func New() (*Engine, error) { eng := &Engine{ - root: root, handlers: make(map[string]Handler), id: utils.RandomString(), Stdout: os.Stdout, @@ -113,7 +77,7 @@ func New(root string) (*Engine, error) { } func (eng *Engine) String() string { - return fmt.Sprintf("%s|%s", eng.Root(), eng.id[:8]) + return fmt.Sprintf("%s", eng.id[:8]) } // Commands returns a list of all currently registered commands, diff --git a/engine/engine_test.go b/engine/engine_test.go index a16c352678..63c4660eb1 100644 --- a/engine/engine_test.go +++ b/engine/engine_test.go @@ -2,10 +2,6 @@ package engine import ( "bytes" - "io/ioutil" - "os" - "path" - "path/filepath" "strings" "testing" ) @@ -67,7 +63,6 @@ func TestJob(t *testing.T) { func TestEngineCommands(t *testing.T) { eng := newTestEngine(t) - defer os.RemoveAll(eng.Root()) handler := func(job *Job) Status { return StatusOK } eng.Register("foo", handler) eng.Register("bar", handler) @@ -83,44 +78,9 @@ func TestEngineCommands(t *testing.T) { } } -func TestEngineRoot(t *testing.T) { - tmp, err := ioutil.TempDir("", "docker-test-TestEngineCreateDir") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tmp) - // We expect Root to resolve to an absolute path. - // FIXME: this should not be necessary. - // Until the above FIXME is implemented, let's check for the - // current behavior. - tmp, err = filepath.EvalSymlinks(tmp) - if err != nil { - t.Fatal(err) - } - tmp, err = filepath.Abs(tmp) - if err != nil { - t.Fatal(err) - } - dir := path.Join(tmp, "dir") - eng, err := New(dir) - if err != nil { - t.Fatal(err) - } - if st, err := os.Stat(dir); err != nil { - t.Fatal(err) - } else if !st.IsDir() { - t.Fatalf("engine.New() created something other than a directory at %s", dir) - } - if r := eng.Root(); r != dir { - t.Fatalf("Expected: %v\nReceived: %v", dir, r) - } -} - func TestEngineString(t *testing.T) { eng1 := newTestEngine(t) - defer os.RemoveAll(eng1.Root()) eng2 := newTestEngine(t) - defer os.RemoveAll(eng2.Root()) s1 := eng1.String() s2 := eng2.String() if eng1 == eng2 { @@ -130,7 +90,6 @@ func TestEngineString(t *testing.T) { func TestEngineLogf(t *testing.T) { eng := newTestEngine(t) - defer os.RemoveAll(eng.Root()) input := "Test log line" if n, err := eng.Logf("%s\n", input); err != nil { t.Fatal(err) @@ -141,7 +100,6 @@ func TestEngineLogf(t *testing.T) { func TestParseJob(t *testing.T) { eng := newTestEngine(t) - defer os.RemoveAll(eng.Root()) // Verify that the resulting job calls to the right place var called bool eng.Register("echo", func(job *Job) Status { diff --git a/engine/helpers_test.go b/engine/helpers_test.go index 488529fc7f..a8d3dfc4d4 100644 --- a/engine/helpers_test.go +++ b/engine/helpers_test.go @@ -1,18 +1,13 @@ package engine import ( - "github.com/dotcloud/docker/utils" "testing" ) var globalTestID string func newTestEngine(t *testing.T) *Engine { - tmp, err := utils.TestDirectory("") - if err != nil { - t.Fatal(err) - } - eng, err := New(tmp) + eng, err := New() if err != nil { t.Fatal(err) } diff --git a/engine/job_test.go b/engine/job_test.go index 50d882c44b..ace398f934 100644 --- a/engine/job_test.go +++ b/engine/job_test.go @@ -1,13 +1,11 @@ package engine import ( - "os" "testing" ) func TestJobStatusOK(t *testing.T) { eng := newTestEngine(t) - defer os.RemoveAll(eng.Root()) eng.Register("return_ok", func(job *Job) Status { return StatusOK }) err := eng.Job("return_ok").Run() if err != nil { @@ -17,7 +15,6 @@ func TestJobStatusOK(t *testing.T) { func TestJobStatusErr(t *testing.T) { eng := newTestEngine(t) - defer os.RemoveAll(eng.Root()) eng.Register("return_err", func(job *Job) Status { return StatusErr }) err := eng.Job("return_err").Run() if err == nil { @@ -27,7 +24,6 @@ func TestJobStatusErr(t *testing.T) { func TestJobStatusNotFound(t *testing.T) { eng := newTestEngine(t) - defer os.RemoveAll(eng.Root()) eng.Register("return_not_found", func(job *Job) Status { return StatusNotFound }) err := eng.Job("return_not_found").Run() if err == nil { @@ -37,7 +33,6 @@ func TestJobStatusNotFound(t *testing.T) { func TestJobStdoutString(t *testing.T) { eng := newTestEngine(t) - defer os.RemoveAll(eng.Root()) // FIXME: test multiple combinations of output and status eng.Register("say_something_in_stdout", func(job *Job) Status { job.Printf("Hello world\n") @@ -59,7 +54,6 @@ func TestJobStdoutString(t *testing.T) { func TestJobStderrString(t *testing.T) { eng := newTestEngine(t) - defer os.RemoveAll(eng.Root()) // FIXME: test multiple combinations of output and status eng.Register("say_something_in_stderr", func(job *Job) Status { job.Errorf("Warning, something might happen\nHere it comes!\nOh no...\nSomething happened\n") diff --git a/integration/runtime_test.go b/integration/runtime_test.go index 38b3277afd..497d1c51c8 100644 --- a/integration/runtime_test.go +++ b/integration/runtime_test.go @@ -627,10 +627,9 @@ func TestRestore(t *testing.T) { // Here are are simulating a docker restart - that is, reloading all containers // from scratch - eng = newTestEngine(t, false, eng.Root()) - daemon2 := mkDaemonFromEngine(eng, t) - if len(daemon2.List()) != 2 { - t.Errorf("Expected 2 container, %v found", len(daemon2.List())) + eng = newTestEngine(t, false, runtime.Config().Root) + if len(runtime2.List()) != 2 { + t.Errorf("Expected 2 container, %v found", len(runtime2.List())) } runningCount := 0 for _, c := range daemon2.List() { diff --git a/integration/server_test.go b/integration/server_test.go index cb3063ded7..226247556d 100644 --- a/integration/server_test.go +++ b/integration/server_test.go @@ -149,7 +149,7 @@ func TestRestartKillWait(t *testing.T) { t.Fatal(err) } - eng = newTestEngine(t, false, eng.Root()) + eng = newTestEngine(t, false, runtime.Config().Root) srv = mkServerFromEngine(eng, t) job = srv.Eng.Job("containers") diff --git a/integration/utils_test.go b/integration/utils_test.go index ab9ca5b72d..6e2b8abc9e 100644 --- a/integration/utils_test.go +++ b/integration/utils_test.go @@ -181,7 +181,7 @@ func newTestEngine(t utils.Fataler, autorestart bool, root string) *engine.Engin root = dir } } - eng, err := engine.New(root) + eng, err := engine.New() if err != nil { t.Fatal(err) } From 87e8d7754e218d93b480b94537d07793f6680515 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Tue, 22 Apr 2014 19:24:47 -0700 Subject: [PATCH 2/3] Update tests with engine root removal Docker-DCO-1.1-Signed-off-by: Michael Crosby (github: crosbymichael) --- api/server/server_unit_test.go | 14 +------------- integration/runtime_test.go | 7 ++++--- integration/utils_test.go | 1 + 3 files changed, 6 insertions(+), 16 deletions(-) diff --git a/api/server/server_unit_test.go b/api/server/server_unit_test.go index 561f47d343..2dcd0df790 100644 --- a/api/server/server_unit_test.go +++ b/api/server/server_unit_test.go @@ -6,11 +6,9 @@ import ( "fmt" "github.com/dotcloud/docker/api" "github.com/dotcloud/docker/engine" - "github.com/dotcloud/docker/utils" "io" "net/http" "net/http/httptest" - "os" "testing" ) @@ -60,7 +58,6 @@ func TesthttpError(t *testing.T) { func TestGetVersion(t *testing.T) { eng := tmpEngine(t) - defer rmEngine(eng) var called bool eng.Register("version", func(job *engine.Job) engine.Status { called = true @@ -90,7 +87,6 @@ func TestGetVersion(t *testing.T) { func TestGetInfo(t *testing.T) { eng := tmpEngine(t) - defer rmEngine(eng) var called bool eng.Register("info", func(job *engine.Job) engine.Status { called = true @@ -131,21 +127,13 @@ func serveRequest(method, target string, body io.Reader, eng *engine.Engine, t * } func tmpEngine(t *testing.T) *engine.Engine { - tmp, err := utils.TestDirectory("") - if err != nil { - t.Fatal(err) - } - eng, err := engine.New(tmp) + eng, err := engine.New() if err != nil { t.Fatal(err) } return eng } -func rmEngine(eng *engine.Engine) { - os.RemoveAll(eng.Root()) -} - func readEnv(src io.Reader, t *testing.T) *engine.Env { out := engine.NewOutput() v, err := out.AddEnv() diff --git a/integration/runtime_test.go b/integration/runtime_test.go index 497d1c51c8..bf00437547 100644 --- a/integration/runtime_test.go +++ b/integration/runtime_test.go @@ -627,9 +627,10 @@ func TestRestore(t *testing.T) { // Here are are simulating a docker restart - that is, reloading all containers // from scratch - eng = newTestEngine(t, false, runtime.Config().Root) - if len(runtime2.List()) != 2 { - t.Errorf("Expected 2 container, %v found", len(runtime2.List())) + eng = newTestEngine(t, false, daemon1.Config().Root) + daemon2 := mkDaemonFromEngine(eng, t) + if len(daemon2.List()) != 2 { + t.Errorf("Expected 2 container, %v found", len(daemon2.List())) } runningCount := 0 for _, c := range daemon2.List() { diff --git a/integration/utils_test.go b/integration/utils_test.go index 6e2b8abc9e..f455657705 100644 --- a/integration/utils_test.go +++ b/integration/utils_test.go @@ -181,6 +181,7 @@ func newTestEngine(t utils.Fataler, autorestart bool, root string) *engine.Engin root = dir } } + os.MkdirAll(root, 0700) eng, err := engine.New() if err != nil { t.Fatal(err) From 7100ace42bda2660d1eaecb2ec096ba6753688ea Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Wed, 23 Apr 2014 11:54:35 -0700 Subject: [PATCH 3/3] Remove error from engine.New() Without creating a root there is no way for the engine to return an error from the new function. Docker-DCO-1.1-Signed-off-by: Michael Crosby (github: crosbymichael) --- api/server/server_unit_test.go | 12 ++---------- docker/docker.go | 7 ++----- engine/engine.go | 4 ++-- engine/engine_test.go | 14 +++++++------- engine/helpers_test.go | 10 +--------- engine/job_test.go | 10 +++++----- integration/utils_test.go | 6 ++---- 7 files changed, 21 insertions(+), 42 deletions(-) diff --git a/api/server/server_unit_test.go b/api/server/server_unit_test.go index 2dcd0df790..8ab34127ac 100644 --- a/api/server/server_unit_test.go +++ b/api/server/server_unit_test.go @@ -57,7 +57,7 @@ func TesthttpError(t *testing.T) { } func TestGetVersion(t *testing.T) { - eng := tmpEngine(t) + eng := engine.New() var called bool eng.Register("version", func(job *engine.Job) engine.Status { called = true @@ -86,7 +86,7 @@ func TestGetVersion(t *testing.T) { } func TestGetInfo(t *testing.T) { - eng := tmpEngine(t) + eng := engine.New() var called bool eng.Register("info", func(job *engine.Job) engine.Status { called = true @@ -126,14 +126,6 @@ func serveRequest(method, target string, body io.Reader, eng *engine.Engine, t * return r } -func tmpEngine(t *testing.T) *engine.Engine { - eng, err := engine.New() - if err != nil { - t.Fatal(err) - } - return eng -} - func readEnv(src io.Reader, t *testing.T) *engine.Env { out := engine.NewOutput() v, err := out.AddEnv() diff --git a/docker/docker.go b/docker/docker.go index aed3b0778d..7a4ddc72a5 100644 --- a/docker/docker.go +++ b/docker/docker.go @@ -121,14 +121,11 @@ func main() { log.Fatalf("Unable to get the full path to root (%s): %s", root, err) } } - if err := checkKernelAndArch(); err != nil { log.Fatal(err) } - eng, err := engine.New() - if err != nil { - log.Fatal(err) - } + + eng := engine.New() // Load builtins builtins.Register(eng) // load the daemon in the background so we can immediately start diff --git a/engine/engine.go b/engine/engine.go index 9cc1bd5004..58c37ab933 100644 --- a/engine/engine.go +++ b/engine/engine.go @@ -54,7 +54,7 @@ func (eng *Engine) Register(name string, handler Handler) error { } // New initializes a new engine. -func New() (*Engine, error) { +func New() *Engine { eng := &Engine{ handlers: make(map[string]Handler), id: utils.RandomString(), @@ -73,7 +73,7 @@ func New() (*Engine, error) { for k, v := range globalHandlers { eng.handlers[k] = v } - return eng, nil + return eng } func (eng *Engine) String() string { diff --git a/engine/engine_test.go b/engine/engine_test.go index 63c4660eb1..8023bd58f3 100644 --- a/engine/engine_test.go +++ b/engine/engine_test.go @@ -17,7 +17,7 @@ func TestRegister(t *testing.T) { // Register is global so let's cleanup to avoid conflicts defer unregister("dummy1") - eng := newTestEngine(t) + eng := New() //Should fail because global handlers are copied //at the engine creation @@ -36,7 +36,7 @@ func TestRegister(t *testing.T) { } func TestJob(t *testing.T) { - eng := newTestEngine(t) + eng := New() job1 := eng.Job("dummy1", "--level=awesome") if job1.handler != nil { @@ -62,7 +62,7 @@ func TestJob(t *testing.T) { } func TestEngineCommands(t *testing.T) { - eng := newTestEngine(t) + eng := New() handler := func(job *Job) Status { return StatusOK } eng.Register("foo", handler) eng.Register("bar", handler) @@ -79,8 +79,8 @@ func TestEngineCommands(t *testing.T) { } func TestEngineString(t *testing.T) { - eng1 := newTestEngine(t) - eng2 := newTestEngine(t) + eng1 := New() + eng2 := New() s1 := eng1.String() s2 := eng2.String() if eng1 == eng2 { @@ -89,7 +89,7 @@ func TestEngineString(t *testing.T) { } func TestEngineLogf(t *testing.T) { - eng := newTestEngine(t) + eng := New() input := "Test log line" if n, err := eng.Logf("%s\n", input); err != nil { t.Fatal(err) @@ -99,7 +99,7 @@ func TestEngineLogf(t *testing.T) { } func TestParseJob(t *testing.T) { - eng := newTestEngine(t) + eng := New() // Verify that the resulting job calls to the right place var called bool eng.Register("echo", func(job *Job) Status { diff --git a/engine/helpers_test.go b/engine/helpers_test.go index a8d3dfc4d4..cfa11da7cd 100644 --- a/engine/helpers_test.go +++ b/engine/helpers_test.go @@ -6,14 +6,6 @@ import ( var globalTestID string -func newTestEngine(t *testing.T) *Engine { - eng, err := New() - if err != nil { - t.Fatal(err) - } - return eng -} - func mkJob(t *testing.T, name string, args ...string) *Job { - return newTestEngine(t).Job(name, args...) + return New().Job(name, args...) } diff --git a/engine/job_test.go b/engine/job_test.go index ace398f934..1f927cbafc 100644 --- a/engine/job_test.go +++ b/engine/job_test.go @@ -5,7 +5,7 @@ import ( ) func TestJobStatusOK(t *testing.T) { - eng := newTestEngine(t) + eng := New() eng.Register("return_ok", func(job *Job) Status { return StatusOK }) err := eng.Job("return_ok").Run() if err != nil { @@ -14,7 +14,7 @@ func TestJobStatusOK(t *testing.T) { } func TestJobStatusErr(t *testing.T) { - eng := newTestEngine(t) + eng := New() eng.Register("return_err", func(job *Job) Status { return StatusErr }) err := eng.Job("return_err").Run() if err == nil { @@ -23,7 +23,7 @@ func TestJobStatusErr(t *testing.T) { } func TestJobStatusNotFound(t *testing.T) { - eng := newTestEngine(t) + eng := New() eng.Register("return_not_found", func(job *Job) Status { return StatusNotFound }) err := eng.Job("return_not_found").Run() if err == nil { @@ -32,7 +32,7 @@ func TestJobStatusNotFound(t *testing.T) { } func TestJobStdoutString(t *testing.T) { - eng := newTestEngine(t) + eng := New() // FIXME: test multiple combinations of output and status eng.Register("say_something_in_stdout", func(job *Job) Status { job.Printf("Hello world\n") @@ -53,7 +53,7 @@ func TestJobStdoutString(t *testing.T) { } func TestJobStderrString(t *testing.T) { - eng := newTestEngine(t) + eng := New() // FIXME: test multiple combinations of output and status eng.Register("say_something_in_stderr", func(job *Job) Status { job.Errorf("Warning, something might happen\nHere it comes!\nOh no...\nSomething happened\n") diff --git a/integration/utils_test.go b/integration/utils_test.go index f455657705..6901662ce6 100644 --- a/integration/utils_test.go +++ b/integration/utils_test.go @@ -182,10 +182,8 @@ func newTestEngine(t utils.Fataler, autorestart bool, root string) *engine.Engin } } os.MkdirAll(root, 0700) - eng, err := engine.New() - if err != nil { - t.Fatal(err) - } + + eng := engine.New() // Load default plugins builtins.Register(eng) // (This is manually copied and modified from main() until we have a more generic plugin system)