From 79ed0886c62fa96014a25af92733259b49ea6019 Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Tue, 5 Feb 2019 11:54:57 +0100 Subject: [PATCH 1/8] virtcontainers: Reduce hyperstart mock test noise We don't need that many logs, especially for the positive path. Fixes: #1211 Signed-off-by: Samuel Ortiz --- .../pkg/hyperstart/mock/hyperstart.go | 48 +------------------ 1 file changed, 2 insertions(+), 46 deletions(-) diff --git a/virtcontainers/pkg/hyperstart/mock/hyperstart.go b/virtcontainers/pkg/hyperstart/mock/hyperstart.go index 8714849a1c..086d394ecc 100644 --- a/virtcontainers/pkg/hyperstart/mock/hyperstart.go +++ b/virtcontainers/pkg/hyperstart/mock/hyperstart.go @@ -42,27 +42,6 @@ const ( SetupRoute = "setuproute" ) -var codeList = map[int]string{ - hyper.VersionCode: Version, - hyper.StartSandboxCode: StartSandbox, - hyper.DestroySandboxCode: DestroySandbox, - hyper.ExecCmdCode: ExecCmd, - hyper.ReadyCode: Ready, - hyper.AckCode: Ack, - hyper.ErrorCode: Error, - hyper.WinsizeCode: WinSize, - hyper.PingCode: Ping, - hyper.NextCode: Next, - hyper.WriteFileCode: WriteFile, - hyper.ReadFileCode: ReadFile, - hyper.NewContainerCode: NewContainer, - hyper.KillContainerCode: KillContainer, - hyper.OnlineCPUMemCode: OnlineCPUMem, - hyper.SetupInterfaceCode: SetupInterface, - hyper.SetupRouteCode: SetupRoute, - hyper.RemoveContainerCode: RemoveContainer, -} - // Hyperstart is an object mocking the hyperstart agent. type Hyperstart struct { t *testing.T @@ -119,10 +98,6 @@ func (h *Hyperstart) GetLastMessages() []hyper.DecodedMessage { return msgs } -func (h *Hyperstart) log(s string) { - h.logf("%s\n", s) -} - func (h *Hyperstart) logf(format string, args ...interface{}) { h.t.Logf("[hyperstart] "+format, args...) } @@ -214,24 +189,13 @@ func (h *Hyperstart) readMessage() (int, []byte, error) { return cmd, data, nil } -func cmdToString(cmd int) (string, error) { - _, ok := codeList[cmd] - if ok == false { - return "", fmt.Errorf("unknown command '%d'", cmd) - } - - return codeList[cmd], nil -} - func (h *Hyperstart) handleCtl() { for { cmd, data, err := h.readMessage() if err != nil { break } - cmdName, err := cmdToString(cmd) - assert.Nil(h.t, err) - h.logf("ctl: --> command %s, payload_len=%d\n", cmdName, len(data)) + if len(data) != 0 { h.logData(data) } @@ -244,8 +208,6 @@ func (h *Hyperstart) handleCtl() { // answer back with the message exit status // XXX: may be interesting to be able to configure the mock // hyperstart to fail and test the reaction of proxy/clients - h.logf("ctl: <-- command %s executed successfully\n", cmdName) - h.SendMessage(hyper.AckCode, nil) } @@ -274,8 +236,6 @@ func (h *Hyperstart) SendIo(seq uint64, data []byte) { length := ioHeaderSize + len(data) header := make([]byte, ioHeaderSize) - h.logf("io: <-- writing %d bytes for seq %d\n", len(data), seq) - binary.BigEndian.PutUint64(header[:], seq) binary.BigEndian.PutUint32(header[8:], uint32(length)) h.writeIo(header) @@ -329,15 +289,14 @@ func (h *Hyperstart) startListening(path string, cb acceptCb) *net.UnixListener assert.Nil(h.t, err) go func() { - h.logf("%s: waiting for connection\n", path) c, err := l.Accept() if err != nil { + h.logf("%s: Connection failed %s\n", path, err) cb(nil) return } cb(c) - h.logf("%s: accepted connection\n", path) }() return l @@ -346,7 +305,6 @@ func (h *Hyperstart) startListening(path string, cb acceptCb) *net.UnixListener // Start will // Once finished with the Hyperstart object, Close must be called. func (h *Hyperstart) Start() { - h.log("start") h.wgConnected.Add(1) h.wgConnected.Add(1) h.ctlListener = h.startListening(h.ctlSocketPath, func(s net.Conn) { @@ -394,6 +352,4 @@ func (h *Hyperstart) Stop() { os.Remove(h.ctlSocketPath) os.Remove(h.ioSocketPath) - - h.log("stopped") } From 2093fe6bfdc775760069736a3b991ba423fabea4 Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Tue, 5 Feb 2019 11:55:37 +0100 Subject: [PATCH 2/8] virtcontainers: Reduce cc_proxy mock test noise We don't need that many logs, especially for the positive path. Fixes: #1211 Signed-off-by: Samuel Ortiz --- virtcontainers/pkg/mock/cc_proxy_mock.go | 33 +++++------------------- 1 file changed, 7 insertions(+), 26 deletions(-) diff --git a/virtcontainers/pkg/mock/cc_proxy_mock.go b/virtcontainers/pkg/mock/cc_proxy_mock.go index 23dd7f980d..3490bc0fd1 100644 --- a/virtcontainers/pkg/mock/cc_proxy_mock.go +++ b/virtcontainers/pkg/mock/cc_proxy_mock.go @@ -9,6 +9,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "net" "os" "sync" @@ -109,10 +110,9 @@ func connectShimHandler(data []byte, userData interface{}, response *handlerResp if payload.Token != proxy.token { response.SetErrorMsg("Invalid Token") + proxy.logF("Invalid Token (token=%s)", payload.Token) } - proxy.logF("ConnectShim(token=%s)", payload.Token) - response.AddResult("version", api.Version) proxy.ShimConnected <- true } @@ -125,8 +125,6 @@ func signalShimHandler(data []byte, userData interface{}, response *handlerRespo err := json.Unmarshal(data, &signalPayload) assert.Nil(proxy.t, err) - proxy.logF("CCProxyMock received signal: %v", signalPayload) - proxy.Signal <- signalPayload } @@ -134,7 +132,6 @@ func disconnectShimHandler(data []byte, userData interface{}, response *handlerR client := userData.(*client) proxy := client.proxy - proxy.log("Client sent DisconnectShim Command") proxy.ShimDisconnected <- true } @@ -150,10 +147,9 @@ func registerVMHandler(data []byte, userData interface{}, response *handlerRespo client := userData.(*client) proxy := client.proxy - proxy.log("Register VM") - payload := api.RegisterVM{} if err := json.Unmarshal(data, &payload); err != nil { + proxy.logF("Register VM failed (%s)", err) response.SetError(err) return } @@ -172,20 +168,15 @@ func registerVMHandler(data []byte, userData interface{}, response *handlerRespo } func unregisterVMHandler(data []byte, userData interface{}, response *handlerResponse) { - client := userData.(*client) - proxy := client.proxy - - proxy.log("Unregister VM") } func attachVMHandler(data []byte, userData interface{}, response *handlerResponse) { client := userData.(*client) proxy := client.proxy - proxy.log("Attach VM") - payload := api.AttachVM{} if err := json.Unmarshal(data, &payload); err != nil { + proxy.logF("Attach VM failed (%s)", err) response.SetError(err) return } @@ -204,11 +195,6 @@ func attachVMHandler(data []byte, userData interface{}, response *handlerRespons } func hyperCmdHandler(data []byte, userData interface{}, response *handlerResponse) { - client := userData.(*client) - proxy := client.proxy - - proxy.log("Hyper command") - response.SetData([]byte{}) } @@ -235,8 +221,6 @@ func (proxy *CCProxyMock) startListening() { l, err := net.ListenUnix("unix", &net.UnixAddr{Name: proxy.connectionPath, Net: "unix"}) assert.Nil(proxy.t, err) - proxy.logF("listening on %s", proxy.connectionPath) - proxy.listener = l } @@ -245,11 +229,11 @@ func (proxy *CCProxyMock) serveClient(proto *ccProxyProtocol, newConn net.Conn) proxy: proxy, conn: newConn, } - err := proto.Serve(newConn, newClient) - proxy.logF("Error serving client : %v\n", err) + if err := proto.Serve(newConn, newClient); err != nil && err != io.EOF { + proxy.logF("Error serving client : %v\n", err) + } newConn.Close() - proxy.log("Client closed connection") proxy.wg.Done() } @@ -281,7 +265,6 @@ func (proxy *CCProxyMock) serve() { } assert.NotNil(proxy.t, conn) - proxy.log("Client connected") proxy.wg.Add(1) @@ -319,7 +302,6 @@ func (proxy *CCProxyMock) Stop() { proxy.listener.Close() if proxy.cl != nil { - proxy.log("Closing client connection") proxy.cl.Close() proxy.cl = nil } else { @@ -332,7 +314,6 @@ func (proxy *CCProxyMock) Stop() { close(proxy.ShimDisconnected) close(proxy.StdinReceived) os.Remove(proxy.connectionPath) - proxy.log("Stopped") } // XXX: could do with its own package to remove that ugly namespacing From 560902c8f119a9ae79669acac606b8a333bfe900 Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Tue, 5 Feb 2019 11:59:50 +0100 Subject: [PATCH 3/8] virtcontainers: Reduce kata_agent test noise We only need to set the agent context before calling into its API. Fixes: #1211 Signed-off-by: Samuel Ortiz --- virtcontainers/kata_agent_test.go | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/virtcontainers/kata_agent_test.go b/virtcontainers/kata_agent_test.go index 5ea995a3a3..85c8493612 100644 --- a/virtcontainers/kata_agent_test.go +++ b/virtcontainers/kata_agent_test.go @@ -73,6 +73,7 @@ func TestKataAgentConnect(t *testing.T) { defer proxy.Stop() k := &kataAgent{ + ctx: context.Background(), state: KataAgentState{ URL: testKataProxyURL, }, @@ -105,6 +106,7 @@ func TestKataAgentDisconnect(t *testing.T) { defer proxy.Stop() k := &kataAgent{ + ctx: context.Background(), state: KataAgentState{ URL: testKataProxyURL, }, @@ -294,6 +296,7 @@ func TestKataAgentSendReq(t *testing.T) { defer proxy.Stop() k := &kataAgent{ + ctx: context.Background(), state: KataAgentState{ URL: testKataProxyURL, }, @@ -722,7 +725,8 @@ func TestAgentCreateContainer(t *testing.T) { assert := assert.New(t) sandbox := &Sandbox{ - id: "foobar", + ctx: context.Background(), + id: "foobar", config: &SandboxConfig{ ID: "foobar", HypervisorType: MockHypervisor, @@ -736,6 +740,7 @@ func TestAgentCreateContainer(t *testing.T) { } container := &Container{ + ctx: sandbox.ctx, id: "barfoo", sandboxID: "foobar", sandbox: sandbox, @@ -768,6 +773,7 @@ func TestAgentCreateContainer(t *testing.T) { defer proxy.Stop() k := &kataAgent{ + ctx: context.Background(), state: KataAgentState{ URL: testKataProxyURL, }, @@ -807,6 +813,7 @@ func TestAgentNetworkOperation(t *testing.T) { defer proxy.Stop() k := &kataAgent{ + ctx: context.Background(), state: KataAgentState{ URL: testKataProxyURL, }, @@ -828,9 +835,14 @@ func TestAgentNetworkOperation(t *testing.T) { func TestKataAgentSetProxy(t *testing.T) { assert := assert.New(t) - k := &kataAgent{} + k := &kataAgent{ctx: context.Background()} p := &kataBuiltInProxy{} - s := &Sandbox{storage: &filesystem{}} + s := &Sandbox{ + ctx: context.Background(), + storage: &filesystem{ + ctx: context.Background(), + }, + } err := k.setProxy(s, p, 0, "") assert.Error(err) @@ -877,6 +889,7 @@ func TestKataCopyFile(t *testing.T) { defer proxy.Stop() k := &kataAgent{ + ctx: context.Background(), state: KataAgentState{ URL: testKataProxyURL, }, From 799ac6edf6dd2e48ee98fc5ed7c6a973690e4258 Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Tue, 5 Feb 2019 12:02:19 +0100 Subject: [PATCH 4/8] virtcontainers: Reduce qemu test noise We only need to set the context before calling into qemu's API. Fixes: #1211 Signed-off-by: Samuel Ortiz --- virtcontainers/qemu_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/virtcontainers/qemu_test.go b/virtcontainers/qemu_test.go index 1a01868167..7b53371502 100644 --- a/virtcontainers/qemu_test.go +++ b/virtcontainers/qemu_test.go @@ -200,6 +200,7 @@ func TestQemuMemoryTopology(t *testing.T) { func testQemuAddDevice(t *testing.T, devInfo interface{}, devType deviceType, expected []govmmQemu.Device) { q := &qemu{ + ctx: context.Background(), arch: &qemuArchBase{}, } @@ -286,7 +287,9 @@ func TestQemuAddDeviceKataVSOCK(t *testing.T) { } func TestQemuGetSandboxConsole(t *testing.T) { - q := &qemu{} + q := &qemu{ + ctx: context.Background(), + } sandboxID := "testSandboxID" expected := filepath.Join(RunVMStoragePath, sandboxID, consoleSocket) @@ -302,6 +305,7 @@ func TestQemuGetSandboxConsole(t *testing.T) { func TestQemuCapabilities(t *testing.T) { q := &qemu{ + ctx: context.Background(), arch: &qemuArchBase{}, } @@ -365,6 +369,7 @@ func TestHotplugUnsupportedDeviceType(t *testing.T) { qemuConfig := newQemuConfig() fs := &filesystem{} q := &qemu{ + ctx: context.Background(), config: qemuConfig, storage: fs, } @@ -394,6 +399,7 @@ func TestQemuCleanup(t *testing.T) { assert := assert.New(t) q := &qemu{ + ctx: context.Background(), config: newQemuConfig(), } From e402601cf832bf605a2365ef02e7fc767952453d Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Tue, 5 Feb 2019 12:17:51 +0100 Subject: [PATCH 5/8] virtcontainers: Reduce sandbox test noise We need to set the sandbox context before calling into its API. Fixes: #1211 Signed-off-by: Samuel Ortiz --- virtcontainers/sandbox_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go index 39701332d3..26c2979533 100644 --- a/virtcontainers/sandbox_test.go +++ b/virtcontainers/sandbox_test.go @@ -1759,7 +1759,10 @@ func TestStartNetworkMonitor(t *testing.T) { } func TestSandboxStopStopped(t *testing.T) { - s := &Sandbox{state: types.State{State: types.StateStopped}} + s := &Sandbox{ + ctx: context.Background(), + state: types.State{State: types.StateStopped}, + } err := s.Stop() assert.Nil(t, err) From f0312f607b4d10b6f6ac257ee0ed375f3f1b5b24 Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Tue, 5 Feb 2019 14:07:40 +0100 Subject: [PATCH 6/8] virtcontainers: Reduce filesystem test noise We need to set the context before calling into the API. Fixes: #1211 Signed-off-by: Samuel Ortiz --- virtcontainers/filesystem_resource_storage_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/virtcontainers/filesystem_resource_storage_test.go b/virtcontainers/filesystem_resource_storage_test.go index 3e51a845be..0090f00d8b 100644 --- a/virtcontainers/filesystem_resource_storage_test.go +++ b/virtcontainers/filesystem_resource_storage_test.go @@ -32,6 +32,7 @@ func TestFilesystemCreateAllResourcesSuccessful(t *testing.T) { } sandbox := &Sandbox{ + ctx: context.Background(), id: testSandboxID, storage: fs, config: sandboxConfig, From a3eff87e805ee9cab65494fa8465a1b3233fc7e2 Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Tue, 5 Feb 2019 12:23:07 +0100 Subject: [PATCH 7/8] virtcontainers: Make proxy startup sequence less noisy We only want to know which proxy started when debugging. Fixes: #1211 Signed-off-by: Samuel Ortiz --- virtcontainers/cc_proxy.go | 2 +- virtcontainers/kata_builtin_proxy.go | 2 +- virtcontainers/kata_proxy.go | 2 +- virtcontainers/no_proxy.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/virtcontainers/cc_proxy.go b/virtcontainers/cc_proxy.go index fbfa733f28..0bfde40363 100644 --- a/virtcontainers/cc_proxy.go +++ b/virtcontainers/cc_proxy.go @@ -16,7 +16,7 @@ func (p *ccProxy) start(params proxyParams) (int, string, error) { return -1, "", err } - params.logger.Info("Starting cc proxy") + params.logger.Debug("Starting cc proxy") // construct the socket path the proxy instance will use proxyURL, err := defaultProxyURL(params.id, SocketTypeUNIX) diff --git a/virtcontainers/kata_builtin_proxy.go b/virtcontainers/kata_builtin_proxy.go index ef18d0765b..0185403717 100644 --- a/virtcontainers/kata_builtin_proxy.go +++ b/virtcontainers/kata_builtin_proxy.go @@ -52,7 +52,7 @@ func (p *kataBuiltInProxy) start(params proxyParams) (int, string, error) { return -1, "", fmt.Errorf("kata builtin proxy running for sandbox %s", params.id) } - params.logger.Info("Starting builtin kata proxy") + params.logger.Debug("Starting builtin kata proxy") p.sandboxID = params.id err := p.watchConsole(buildinProxyConsoleProto, params.consoleURL, params.logger) diff --git a/virtcontainers/kata_proxy.go b/virtcontainers/kata_proxy.go index 1ff9d01f14..0b622f27c5 100644 --- a/virtcontainers/kata_proxy.go +++ b/virtcontainers/kata_proxy.go @@ -27,7 +27,7 @@ func (p *kataProxy) start(params proxyParams) (int, string, error) { return -1, "", err } - params.logger.Info("Starting regular Kata proxy rather than built-in") + params.logger.Debug("Starting regular Kata proxy rather than built-in") // construct the socket path the proxy instance will use proxyURL, err := defaultProxyURL(params.id, SocketTypeUNIX) diff --git a/virtcontainers/no_proxy.go b/virtcontainers/no_proxy.go index ef97e4a30d..a97270c3d2 100644 --- a/virtcontainers/no_proxy.go +++ b/virtcontainers/no_proxy.go @@ -28,7 +28,7 @@ func (p *noProxy) start(params proxyParams) (int, string, error) { return -1, "", fmt.Errorf("proxy logger is not set") } - params.logger.Info("No proxy started because of no-proxy implementation") + params.logger.Debug("No proxy started because of no-proxy implementation") if params.agentURL == "" { return -1, "", fmt.Errorf("AgentURL cannot be empty") From 2affa1fe26da550a61af9f8331f3c354c4dadc64 Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Tue, 5 Feb 2019 14:11:53 +0100 Subject: [PATCH 8/8] virtcontainers: Reduce hyperstart agent test noise We need to pass a context to the filesystem handle. Fixes: #1211 Signed-off-by: Samuel Ortiz --- virtcontainers/hyperstart_agent_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/virtcontainers/hyperstart_agent_test.go b/virtcontainers/hyperstart_agent_test.go index 6821601bed..06c7a782f9 100644 --- a/virtcontainers/hyperstart_agent_test.go +++ b/virtcontainers/hyperstart_agent_test.go @@ -6,6 +6,7 @@ package virtcontainers import ( + "context" "fmt" "io/ioutil" "net" @@ -245,7 +246,9 @@ func TestHyperSetProxy(t *testing.T) { h := &hyper{} p := &ccProxy{} - s := &Sandbox{storage: &filesystem{}} + s := &Sandbox{ + storage: &filesystem{ctx: context.Background()}, + } err := h.setProxy(s, p, 0, "") assert.Error(err)