fix: fix error output of cli action tracker

Before we started a reboot/shutdown/reset/upgrade action with the action tracker (`--wait`), we were setting a flag to prevent cobra from printing the returned error from the command.

This was to prevent the error from being printed twice, as the reporter of the action tracker already prints any errors occurred during the action execution.

But if the error happens too early - i.e. before we even started the status printer goroutine, then that error wouldn't be printed at all, as we have suppressed the errors.

This PR moves the suppression flag to be set after the status printer is started - so we still do not double-print the errors, but neither do we suppress any early-stage error from being printed.

Closes siderolabs/talos#7900.

Signed-off-by: Utku Ozdemir <utku.ozdemir@siderolabs.com>
This commit is contained in:
Utku Ozdemir 2023-10-27 18:42:12 +02:00
parent ef5056122b
commit 5dff164f1c
No known key found for this signature in database
GPG Key ID: 65933E76F0549B0D
6 changed files with 32 additions and 12 deletions

View File

@ -10,7 +10,6 @@ import (
"github.com/spf13/cobra"
"github.com/siderolabs/talos/cmd/talosctl/cmd/common"
"github.com/siderolabs/talos/cmd/talosctl/pkg/talos/action"
"github.com/siderolabs/talos/cmd/talosctl/pkg/talos/helpers"
"github.com/siderolabs/talos/pkg/machinery/client"
@ -57,8 +56,6 @@ var rebootCmd = &cobra.Command{
})
}
common.SuppressErrors = true
return action.NewTracker(
&GlobalArgs,
action.MachineReadyEventFn,

View File

@ -13,7 +13,6 @@ import (
"github.com/siderolabs/gen/maps"
"github.com/spf13/cobra"
"github.com/siderolabs/talos/cmd/talosctl/cmd/common"
"github.com/siderolabs/talos/cmd/talosctl/pkg/talos/action"
"github.com/siderolabs/talos/cmd/talosctl/pkg/talos/helpers"
machineapi "github.com/siderolabs/talos/pkg/machinery/api/machine"
@ -140,8 +139,6 @@ var resetCmd = &cobra.Command{
}
}
common.SuppressErrors = true
return action.NewTracker(
&GlobalArgs,
action.StopAllServicesEventFn,

View File

@ -10,7 +10,6 @@ import (
"github.com/spf13/cobra"
"github.com/siderolabs/talos/cmd/talosctl/cmd/common"
"github.com/siderolabs/talos/cmd/talosctl/pkg/talos/action"
"github.com/siderolabs/talos/cmd/talosctl/pkg/talos/helpers"
"github.com/siderolabs/talos/pkg/machinery/client"
@ -50,8 +49,6 @@ var shutdownCmd = &cobra.Command{
})
}
common.SuppressErrors = true
return action.NewTracker(
&GlobalArgs,
action.StopAllServicesEventFn,

View File

@ -19,7 +19,6 @@ import (
"google.golang.org/grpc"
"google.golang.org/grpc/peer"
"github.com/siderolabs/talos/cmd/talosctl/cmd/common"
"github.com/siderolabs/talos/cmd/talosctl/pkg/talos/action"
"github.com/siderolabs/talos/cmd/talosctl/pkg/talos/helpers"
"github.com/siderolabs/talos/pkg/cli"
@ -73,8 +72,6 @@ var upgradeCmd = &cobra.Command{
return runUpgradeNoWait(opts)
}
common.SuppressErrors = true
return action.NewTracker(
&GlobalArgs,
action.MachineReadyEventFn,

View File

@ -23,6 +23,7 @@ import (
"google.golang.org/grpc"
"google.golang.org/grpc/backoff"
"github.com/siderolabs/talos/cmd/talosctl/cmd/common"
"github.com/siderolabs/talos/cmd/talosctl/pkg/talos/global"
"github.com/siderolabs/talos/cmd/talosctl/pkg/talos/helpers"
machineapi "github.com/siderolabs/talos/pkg/machinery/api/machine"
@ -155,6 +156,10 @@ func (a *Tracker) Run() error {
return a.runReporter(ctx)
})
// Reporter is started, it will print the errors if there is any.
// So from here on we can suppress the command error to be printed to avoid it being printed twice.
common.SuppressErrors = true
var trackEg errgroup.Group
for _, node := range a.cliContext.Nodes {

View File

@ -9,6 +9,7 @@ package cli
import (
"fmt"
"path/filepath"
"strings"
"testing"
@ -82,6 +83,32 @@ func (suite *RebootSuite) TestReboot() {
)
}
// TestRebootEarlyFailPrintsOutput tests the action tracker used by reboot command to track reboot status
// does not suppress the stderr output if there is an error occurring at an early stage, i.e. before the
// action status reporting starts.
func (suite *RebootSuite) TestRebootEarlyFailPrintsOutput() {
controlPlaneNode := suite.RandomDiscoveredNodeInternalIP(machine.TypeControlPlane)
invalidTalosconfig := filepath.Join(suite.T().TempDir(), "talosconfig.yaml")
suite.T().Logf("attempting to reboot node %q using talosconfig %q", controlPlaneNode, invalidTalosconfig)
suite.RunCLI([]string{"--talosconfig", invalidTalosconfig, "reboot", "-n", controlPlaneNode},
base.ShouldFail(),
base.StdoutEmpty(),
base.StderrNotEmpty(),
base.StderrMatchFunc(func(stdout string) error {
if strings.Contains(stdout, "method is not supported") {
suite.T().Skip("reboot is not supported")
}
if !strings.Contains(stdout, "failed to determine endpoints") {
return fmt.Errorf("expected to find 'failed to determine endpoints' in stderr")
}
return nil
}))
}
func init() {
allSuites = append(allSuites, new(RebootSuite))
}