sealer icon indicating copy to clipboard operation
sealer copied to clipboard

fix: remove exit status 1 and display stderr info

Open allencloud opened this issue 2 years ago • 1 comments

Signed-off-by: Allen Sun [email protected]

Describe what this PR does / why we need it

When sealer binary runs into an error, it is quite important for sealer to throw out the detailed and explicit error message. While I found that in lots of cases, sealer binary always throws an error message containing exit status 1, which seems to meaningless and helpless for any end-user. Here is an issue list which reflect the situation above:

  • https://github.com/sealerio/sealer/issues/1618
  • https://github.com/sealerio/sealer/issues/1580
  • https://github.com/sealerio/sealer/issues/1519
  • https://github.com/sealerio/sealer/issues/1437
  • and so on

This pull request changes such situation with outputting the stderr information. If we call some shell scripts, generally the error message would be always thrown into the os.Stderr. And if end-user could see the os.Stderr details, there is much more possibility for end-user to be noticed what detailed error happened there.

Since sealer replies on https://github.com/sealerio/basefs mostly, the shell scripts in basefs repo should be organized much more carefully, especially to throw out error message into stderr rather than throwing error message into stdout casually. @Stevent-fei @kakaZhou719

Does this pull request fix one issue?

Describe how you did it

none

Describe how to verify it

none

Special notes for reviews

I just modified func (s *SSH) Cmd(host net.IP, cmd string) and unc (s *SSH) CmdToString(host net.IP, cmd, split string), and have not modified func (s *SSH) CmdAsync(host net.IP, cmds ...string).

I wish to see the initial effect of this pull request, and move it on CmdAsync by the result.

allencloud avatar Sep 14 '22 06:09 allencloud

Codecov Report

Base: 18.42% // Head: 18.40% // Decreases project coverage by -0.02% :warning:

Coverage data is based on head (b413a20) compared to base (e971bc8). Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1705      +/-   ##
==========================================
- Coverage   18.42%   18.40%   -0.03%     
==========================================
  Files          66       66              
  Lines        5350     5356       +6     
==========================================
  Hits          986      986              
- Misses       4238     4244       +6     
  Partials      126      126              
Impacted Files Coverage Δ
utils/ssh/sshcmd.go 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov-commenter avatar Sep 14 '22 06:09 codecov-commenter