Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log stdout when exec errors #1495

Merged
merged 4 commits into from
Aug 4, 2023
Merged

Log stdout when exec errors #1495

merged 4 commits into from
Aug 4, 2023

Conversation

hellt
Copy link
Member

@hellt hellt commented Aug 4, 2023

When docker exec fails to find a command to run it seems it puts it in the stdout, rather than stderr.

This new log print prints stderr and stdout when exec errors or contains non 0 rc.

DEBU[0009] error during mgmt_server status check: %!s(<nil>), exit code: 126, stderr: , stdout: OCI runtime exec failed: exec failed: unable to  tart container process: exec: "sr_cli": executable file not found in $PATH: unknown

Comment on lines 104 to 115
var s strings.Builder

s.WriteString(fmt.Sprintf("Cmd: %s\nReturnCode: %d", e.GetCmdString(), e.ReturnCode))

if e.Stdout != "" {
s.WriteString(fmt.Sprintf("\nStdout: %s", e.Stdout))
}
if e.Stderr != "" {
s.WriteString(fmt.Sprintf("\nStderr: %s", e.Stderr))
}

return s.String()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@steiler I think I am going mad lol
For some reason the logger eats up the first character when printing it the Stdout: here.
I am curious if you can crack this mystery:

DEBU[0002] mgmt_server status check failed, output: 
Cmd: sr_cli -d info from state system app-management application mgmt_server state | grep running
ReturnCode: 126
 tdout: OCI runtime exec failed: exec failed: unable to start container process: exec: "sr_cli": executable file not found in $PATH: unknown

Note, the tdout string.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for this log message to appear, change the command here from sr_cli to some garbage

mgmtServerRdyCmd = `sr_cli -d "info from state system app-management application mgmt_server state | grep running"`

@hellt hellt marked this pull request as ready for review August 4, 2023 13:23
@codecov
Copy link

codecov bot commented Aug 4, 2023

Codecov Report

Merging #1495 (f77d8b6) into main (4348879) will decrease coverage by 0.03%.
Report is 5 commits behind head on main.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #1495      +/-   ##
==========================================
- Coverage   21.08%   21.05%   -0.03%     
==========================================
  Files          65       65              
  Lines        7063     7073      +10     
==========================================
  Hits         1489     1489              
- Misses       5443     5453      +10     
  Partials      131      131              
Files Changed Coverage Δ
clab/exec/exec.go 6.29% <0.00%> (-0.54%) ⬇️

@hellt hellt merged commit 4cade7f into main Aug 4, 2023
18 of 19 checks passed
@hellt hellt deleted the exec-stdout branch August 4, 2023 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant