From f3b8815c967da5efe17b247b6dac4a0800995055 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Mon, 9 Jul 2018 12:19:02 -0700 Subject: [PATCH 1/5] rkt: fix failing TestRktDriver_UserGroup test Started failing due to the docker redis image switching from Debian jessie to stretch: https://github.com/docker-library/redis/commit/53f86805506b103b503fd392e029929290fe5346#diff-acff46b161a3b7d6ed01ba79a032acc9 Switched from Debian based image to Alpine to get a working `ps` command again (albeit busybox's stripped down implementation) --- client/driver/rkt_test.go | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/client/driver/rkt_test.go b/client/driver/rkt_test.go index 3aae9ff57..d2c75cb74 100644 --- a/client/driver/rkt_test.go +++ b/client/driver/rkt_test.go @@ -335,11 +335,11 @@ func TestRktDriver_UserGroup(t *testing.T) { require := assert.New(t) task := &structs.Task{ - Name: "etcd", + Name: "redis", Driver: "rkt", User: "nobody", Config: map[string]interface{}{ - "image": "docker://redis:3.2", + "image": "docker://redis:4-alpine", "group": "nogroup", }, LogConfig: &structs.LogConfig{ @@ -357,9 +357,9 @@ func TestRktDriver_UserGroup(t *testing.T) { d := NewRktDriver(tctx.DriverCtx) _, err := d.Prestart(tctx.ExecCtx, task) - require.Nil(err) + require.NoError(err) resp, err := d.Start(tctx.ExecCtx, task) - require.Nil(err) + require.NoError(err) defer resp.Handle.Kill() timeout := time.Duration(testutil.TestMultiplier()*15) * time.Second @@ -368,10 +368,11 @@ func TestRktDriver_UserGroup(t *testing.T) { defer cancel() // WaitUntil we can determine the user/group redis is running as - expected := []byte("redis-server *:6379 nobody nogroup\n") + expected := []byte("nobody nogroup redis-server\n") testutil.WaitForResult(func() (bool, error) { - raw, code, err := resp.Handle.Exec(ctx, "/bin/bash", []string{"-c", "ps -eo args,user,group | grep ^redis"}) + raw, code, err := resp.Handle.Exec(ctx, "/bin/sh", []string{"-c", "ps -o user,group,args | grep 'redis-server$'"}) if err != nil { + err = fmt.Errorf("original error: %v; code: %d; raw output: %s", err, code, string(raw)) return false, err } if code != 0 { @@ -382,7 +383,7 @@ func TestRktDriver_UserGroup(t *testing.T) { t.Fatalf("err: %v", err) }) - require.Nil(resp.Handle.Kill()) + require.NoError(resp.Handle.Kill()) } func TestRktTrustPrefix(t *testing.T) { @@ -460,10 +461,10 @@ func TestRktDriver_PortMapping(t *testing.T) { } task := &structs.Task{ - Name: "etcd", + Name: "redis", Driver: "rkt", Config: map[string]interface{}{ - "image": "docker://redis:3.2", + "image": "docker://redis:4-alpine", "port_map": []map[string]string{ { "main": "6379-tcp", @@ -529,10 +530,10 @@ func TestRktDriver_PortsMapping_Host(t *testing.T) { } task := &structs.Task{ - Name: "etcd", + Name: "redis", Driver: "rkt", Config: map[string]interface{}{ - "image": "docker://redis:latest", + "image": "docker://redis:4-alpine", "net": []string{"host"}, }, LogConfig: &structs.LogConfig{ From 0fbc84b81de0166c00fb1d7a81e1baeb2d2c926f Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Mon, 9 Jul 2018 13:37:35 -0700 Subject: [PATCH 2/5] tests: make alloc id consistent in helper It worked, but the old code used a different alloc id for the path than the actual alloc! Use the same alloc id everywhere to prevent confusing test output. --- client/driver/driver_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/client/driver/driver_test.go b/client/driver/driver_test.go index d17ffefcc..104fc6b2c 100644 --- a/client/driver/driver_test.go +++ b/client/driver/driver_test.go @@ -15,7 +15,6 @@ import ( "github.com/hashicorp/nomad/client/driver/env" "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/helper/testtask" - "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" ) @@ -112,11 +111,13 @@ type testContext struct { func testDriverContexts(t *testing.T, task *structs.Task) *testContext { cfg := testConfig(t) cfg.Node = mock.Node() - allocDir := allocdir.NewAllocDir(testlog.Logger(t), filepath.Join(cfg.AllocDir, uuid.Generate())) + alloc := mock.Alloc() + alloc.NodeID = cfg.Node.ID + + allocDir := allocdir.NewAllocDir(testlog.Logger(t), filepath.Join(cfg.AllocDir, alloc.ID)) if err := allocDir.Build(); err != nil { t.Fatalf("AllocDir.Build() failed: %v", err) } - alloc := mock.Alloc() // Build a temp driver so we can call FSIsolation and build the task dir tmpdrv, err := NewDriver(task.Driver, NewEmptyDriverContext()) From a1d4f77ce072c5fa9a9632fbd693851f032e9329 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Mon, 9 Jul 2018 13:39:05 -0700 Subject: [PATCH 3/5] rkt: skip retrieving network information when net=none Even when net=none we would attempt to retrieve network information from rkt which would spew useless log lines such as: ``` testlog.go:30: 20:37:31.409209 [DEBUG] driver.rkt: failed getting network info for pod UUID 8303cfe6-0c10-4288-84f5-cb79ad6dbf1c attempt 2: no networks found. Sleeping for 970ms ``` It would also delay tests for ~60s during the network information retry period. So skip this when net=none. It's unlikely anyone actually uses net=none outside of tests, so I doubt anyone will notice this change. Official docs: https://coreos.com/rkt/docs/latest/networking/overview.html#no-loopback-only-networking --- client/driver/rkt.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/client/driver/rkt.go b/client/driver/rkt.go index 1ca82ebb3..f417fdcfc 100644 --- a/client/driver/rkt.go +++ b/client/driver/rkt.go @@ -699,9 +699,12 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse, } go h.run() - // Only return a driver network if *not* using host networking + // Do not attempt to retrieve driver network if one won't exist: + // - "host" means the container itself has no networking metadata + // - "none" means no network is configured + // https://coreos.com/rkt/docs/latest/networking/overview.html#no-loopback-only-networking var driverNetwork *cstructs.DriverNetwork - if network != "host" { + if network != "host" && network != "none" { d.logger.Printf("[DEBUG] driver.rkt: retrieving network information for pod %q (UUID: %s) for task %q", img, uuid, d.taskName) driverNetwork, err = rktGetDriverNetwork(uuid, driverConfig.PortMap, d.logger) if err != nil && !pluginClient.Exited() { From c56f899ee9b078bb91482ba0d6055022a56cc6b4 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Mon, 9 Jul 2018 13:45:18 -0700 Subject: [PATCH 4/5] rkt: speed up tests Disable networking when it's not needed and improve failure message for UserGroup test by including the full ps output on failure. --- client/driver/rkt_test.go | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/client/driver/rkt_test.go b/client/driver/rkt_test.go index d2c75cb74..d9900ffae 100644 --- a/client/driver/rkt_test.go +++ b/client/driver/rkt_test.go @@ -144,6 +144,8 @@ func TestRktDriver_Start_Wait(t *testing.T) { "image": "coreos.com/etcd:v2.0.4", "command": "/etcd", "args": []string{"--version"}, + // Disable networking to speed up test as it's not needed + "net": []string{"none"}, }, LogConfig: &structs.LogConfig{ MaxFiles: 10, @@ -214,6 +216,8 @@ func TestRktDriver_Start_Wait_Skip_Trust(t *testing.T) { "image": "coreos.com/etcd:v2.0.4", "command": "/etcd", "args": []string{"--version"}, + // Disable networking to speed up test as it's not needed + "net": []string{"none"}, }, LogConfig: &structs.LogConfig{ MaxFiles: 10, @@ -273,11 +277,11 @@ func TestRktDriver_Start_Wait_AllocDir(t *testing.T) { Name: "rkttest_alpine", Driver: "rkt", Config: map[string]interface{}{ - "image": "docker://alpine", + "image": "docker://redis:4-alpine", "command": "/bin/sh", "args": []string{ "-c", - fmt.Sprintf(`echo -n %s > foo/%s`, string(exp), file), + fmt.Sprintf("echo -n %s > /foo/%s", string(exp), file), }, "net": []string{"none"}, "volumes": []string{fmt.Sprintf("%s:/foo", tmpvol)}, @@ -335,12 +339,14 @@ func TestRktDriver_UserGroup(t *testing.T) { require := assert.New(t) task := &structs.Task{ - Name: "redis", + Name: "sleepy", Driver: "rkt", User: "nobody", Config: map[string]interface{}{ - "image": "docker://redis:4-alpine", - "group": "nogroup", + "image": "docker://redis:4-alpine", + "group": "nogroup", + "command": "sleep", + "args": []string{"9000"}, }, LogConfig: &structs.LogConfig{ MaxFiles: 10, @@ -362,15 +368,12 @@ func TestRktDriver_UserGroup(t *testing.T) { require.NoError(err) defer resp.Handle.Kill() - timeout := time.Duration(testutil.TestMultiplier()*15) * time.Second - - ctx, cancel := context.WithTimeout(context.Background(), timeout) - defer cancel() + ctx := context.Background() // WaitUntil we can determine the user/group redis is running as - expected := []byte("nobody nogroup redis-server\n") + expected := []byte("\nnobody nogroup /bin/sleep 9000\n") testutil.WaitForResult(func() (bool, error) { - raw, code, err := resp.Handle.Exec(ctx, "/bin/sh", []string{"-c", "ps -o user,group,args | grep 'redis-server$'"}) + raw, code, err := resp.Handle.Exec(ctx, "ps", []string{"-o", "user,group,args"}) if err != nil { err = fmt.Errorf("original error: %v; code: %d; raw output: %s", err, code, string(raw)) return false, err @@ -378,7 +381,7 @@ func TestRktDriver_UserGroup(t *testing.T) { if code != 0 { return false, fmt.Errorf("unexpected exit code: %d", code) } - return bytes.Equal(expected, raw), fmt.Errorf("expected %q but found %q", expected, raw) + return bytes.Contains(raw, expected), fmt.Errorf("expected %q but found:\n%s", expected, raw) }, func(err error) { t.Fatalf("err: %v", err) }) From 91588cb861b053b7cc4c6f4b43d415bc43a948dd Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Mon, 9 Jul 2018 16:15:32 -0700 Subject: [PATCH 5/5] rkt: revert to redis 3.2 to favor stability --- client/driver/rkt_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/client/driver/rkt_test.go b/client/driver/rkt_test.go index d9900ffae..5e2f72d72 100644 --- a/client/driver/rkt_test.go +++ b/client/driver/rkt_test.go @@ -277,7 +277,7 @@ func TestRktDriver_Start_Wait_AllocDir(t *testing.T) { Name: "rkttest_alpine", Driver: "rkt", Config: map[string]interface{}{ - "image": "docker://redis:4-alpine", + "image": "docker://redis:3.2-alpine", "command": "/bin/sh", "args": []string{ "-c", @@ -343,7 +343,7 @@ func TestRktDriver_UserGroup(t *testing.T) { Driver: "rkt", User: "nobody", Config: map[string]interface{}{ - "image": "docker://redis:4-alpine", + "image": "docker://redis:3.2-alpine", "group": "nogroup", "command": "sleep", "args": []string{"9000"}, @@ -467,7 +467,7 @@ func TestRktDriver_PortMapping(t *testing.T) { Name: "redis", Driver: "rkt", Config: map[string]interface{}{ - "image": "docker://redis:4-alpine", + "image": "docker://redis:3.2-alpine", "port_map": []map[string]string{ { "main": "6379-tcp", @@ -536,7 +536,7 @@ func TestRktDriver_PortsMapping_Host(t *testing.T) { Name: "redis", Driver: "rkt", Config: map[string]interface{}{ - "image": "docker://redis:4-alpine", + "image": "docker://redis:3.2-alpine", "net": []string{"host"}, }, LogConfig: &structs.LogConfig{