From ddc78f1244c38670d534ebc117a5fb7646ff7ef2 Mon Sep 17 00:00:00 2001 From: Ilya Maximets Date: Wed, 28 Feb 2024 23:52:48 +0100 Subject: [PATCH] server: More concise error messages. On the CNI request failure, multus-cni prints out cmdArgs. In all cases, except for debug printing, this is done with %s and a special printing function. However, the handleCNIRequest is an exception for some reason. That leads to unintelligible error messages in case of CNI request failures (severely abridged): CmdAdd (shim): CNI request failed with status 400: '&{ContainerID: Netns:/var/run/netns/ IfName:eth0 Args: Path: StdinData:[125 121 111 117 114 32 97 100 118 101 114 116 105 115 101 109 101 110 116 32 99 111 117 108 100 32 98 101 32 104 101 114 101 125 ... another 650 numbers ]} ContainerID:"" Netns:"/var/run/netns/" IfName:"eth0" Args:"" Path:"" ERRORED: error configuring pod ... printCmdArgs() should be used for this case as well to avoid huge hardly readable logs. At the same time, the content of cniCmdArgs is always appended to the error twice as seen in the example above. The first time by the HandleCNIRequest and another time by the handleCNIRequest. Same for the HandleDelegateRequest path. Just removing the prefixing from the lower level handlers while keeping higher level ones. The 'ERRORED' part migrated to the higher level handler functions to preserve the overall look of the error. Signed-off-by: Ilya Maximets --- pkg/server/server.go | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/pkg/server/server.go b/pkg/server/server.go index e534e91bb..d469af23a 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -99,11 +99,7 @@ func (s *Server) HandleCNIRequest(cmd string, k8sArgs *types.K8sArgs, cniCmdArgs return []byte(""), fmt.Errorf("unknown cmd type: %s", cmd) } logging.Verbosef("%s finished CNI request %s, result: %q, err: %v", cmd, printCmdArgs(cniCmdArgs), string(result), err) - if err != nil { - // Prefix errors with request info for easier failure debugging - return nil, fmt.Errorf("%s ERRORED: %v", printCmdArgs(cniCmdArgs), err) - } - return result, nil + return result, err } // HandleDelegateRequest is the CNI server handler function; it is invoked whenever @@ -129,11 +125,7 @@ func (s *Server) HandleDelegateRequest(cmd string, k8sArgs *types.K8sArgs, cniCm return []byte(""), fmt.Errorf("unknown cmd type: %s", cmd) } logging.Verbosef("%s finished Delegate request %s, result: %q, err: %v", cmd, printCmdArgs(cniCmdArgs), string(result), err) - if err != nil { - // Prefix errors with request info for easier failure debugging - return nil, fmt.Errorf("%s ERRORED: %v", printCmdArgs(cniCmdArgs), err) - } - return result, nil + return result, err } // GetListener creates a listener to a unix socket located in `socketPath` @@ -415,7 +407,7 @@ func (s *Server) handleCNIRequest(r *http.Request) ([]byte, error) { result, err := s.HandleCNIRequest(cmdType, k8sArgs, cniCmdArgs) if err != nil { // Prefix error with request information for easier debugging - return nil, fmt.Errorf("%+v %v", cniCmdArgs, err) + return nil, fmt.Errorf("%s ERRORED: %v", printCmdArgs(cniCmdArgs), err) } return result, nil } @@ -442,7 +434,7 @@ func (s *Server) handleDelegateRequest(r *http.Request) ([]byte, error) { result, err := s.HandleDelegateRequest(cmdType, k8sArgs, cniCmdArgs, cr.InterfaceAttributes) if err != nil { // Prefix error with request information for easier debugging - return nil, fmt.Errorf("%s %v", printCmdArgs(cniCmdArgs), err) + return nil, fmt.Errorf("%s ERRORED: %v", printCmdArgs(cniCmdArgs), err) } return result, nil }