chore: report result when action fails

[kizuna-lek]

• close: [Improvement] Report the execution result of kbagent when the action fails #10062

[apecloud-bot]

Auto Cherry-pick Instructions

Usage:
  - /nopick: Not auto cherry-pick when PR merged.
  - /pick: release-x.x [release-x.x]: Auto cherry-pick to the specified branch when PR merged.

Example:
  - /nopick
  - /pick release-1.1

[codecov]

Codecov Report

❌ Patch coverage is 55.55556% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.31%. Comparing base (ea54720) to head (ee670af).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/kbagent/service/action_utils.go	55.55%	6 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10063      +/-   ##
==========================================
- Coverage   51.38%   51.31%   -0.07%     
==========================================
  Files         540      540              
  Lines       58973    58985      +12     
==========================================
- Hits        30306    30271      -35     
- Misses      25688    25747      +59     
+ Partials     2979     2967      -12     

Flag	Coverage Δ
unittests	51.31% <55.55%> (-0.07%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[cjc7373]

kubeblocks/pkg/kbagent/service/action_utils.go

Lines 104 to 126 in 65dfc72

	func blockingCallAction(ctx context.Context, action *kbaproto.Action, parameters map[string]string, timeout *int32) ([]byte, error) {
	resultChan, err := nonBlockingCallAction(ctx, action, parameters, timeout)
	if err != nil {
	return nil, err
	}
	result := <-resultChan
	err = result.err
	if err != nil {
	var exitErr *exec.ExitError
	if errors.As(err, &exitErr) {
	errMsg := fmt.Sprintf("exit code: %d", exitErr.ExitCode())
	if stderrMsg := result.stderr.String(); len(stderrMsg) > 0 {
	errMsg += fmt.Sprintf(", stderr: %s", stderrMsg)
	}
	return nil, errors.Wrapf(kbaproto.ErrFailed, "%s", errMsg)
	}
	if errMsg := result.stderr.String(); len(errMsg) > 0 {
	return nil, errors.Wrapf(err, "%s", errMsg)
	}
	return nil, err
	}
	return result.stdout.Bytes(), nil
	}

result would be nil if an error occurred? (line 123)

[kizuna-lek]

yes, result maybe nil. but string(resp) works because resp has a known type ([]byte)

[cjc7373]

I mean, you don't get any useful information because result is nil.

[cjc7373]

Check my previous PR #9454. Do you want the similar thing?

[kizuna-lek]

Check my previous PR #9454. Do you want the similar thing?

yes, report result would be helpful for debug, especially in scenarios where action execution times out

[cjc7373]

For now, if an action exits normally:

• stdout is logged
• stderr is not logged

If we want to align with API definitions, we should always log stderr and never log stdout? @leon-ape what's your point?