Skip to content

Commit

Permalink
fix: send diagnostic output to stderr consistently
Browse files Browse the repository at this point in the history
Fixes #6676

There was a mix of stdout/stderr, move more consistently to stderr.

Signed-off-by: Andrey Smirnov <andrey.smirnov@talos-systems.com>
  • Loading branch information
smira committed Dec 23, 2022
1 parent 9a5f4c0 commit a0c0352
Show file tree
Hide file tree
Showing 23 changed files with 111 additions and 60 deletions.
2 changes: 1 addition & 1 deletion cmd/installer/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ var rootCmd = &cobra.Command{
// This is called by main.main(). It only needs to happen once to the rootCmd.
func Execute() {
if err := rootCmd.Execute(); err != nil {
fmt.Println(err)
fmt.Fprintln(os.Stderr, err)
os.Exit(1)
}
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/talosctl/cmd/mgmt/cluster/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func create(ctx context.Context, flags *pflag.FlagSet) (err error) {
workerMemory := int64(workersMemory) * 1024 * 1024

// Validate CIDR range and allocate IPs
fmt.Println("validating CIDR and reserving IPs")
fmt.Fprintln(os.Stderr, "validating CIDR and reserving IPs")

cidr4, err := netip.ParsePrefix(networkCIDR)
if err != nil {
Expand Down Expand Up @@ -714,7 +714,7 @@ func saveConfig(talosConfigObj *clientconfig.Config) (err error) {

renames := c.Merge(talosConfigObj)
for _, rename := range renames {
fmt.Printf("renamed talosconfig context %s\n", rename.String())
fmt.Fprintf(os.Stderr, "renamed talosconfig context %s\n", rename.String())
}

return c.Save(talosconfig)
Expand All @@ -726,7 +726,7 @@ func mergeKubeconfig(ctx context.Context, clusterAccess *access.Adapter) error {
return err
}

fmt.Printf("\nmerging kubeconfig into %q\n", kubeconfigPath)
fmt.Fprintf(os.Stderr, "\nmerging kubeconfig into %q\n", kubeconfigPath)

k8sconfig, err := clusterAccess.Kubeconfig(ctx)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion cmd/talosctl/cmd/mgmt/gen/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ func writeToDestination(data []byte, destination string, permissions os.FileMode

err := os.WriteFile(destination, data, permissions)

fmt.Printf("Created %s\n", destination)
fmt.Fprintf(os.Stderr, "Created %s\n", destination)

return err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/talosctl/cmd/talos/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ var configMergeCmd = &cobra.Command{

renames := c.Merge(secondConfig)
for _, rename := range renames {
fmt.Printf("renamed talosconfig context %s\n", rename.String())
fmt.Fprintf(os.Stderr, "renamed talosconfig context %s\n", rename.String())
}

if err := c.Save(GlobalArgs.Talosconfig); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions cmd/talosctl/cmd/talos/edit.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,13 @@ func editFn(c *client.Client) func(context.Context, string, resource.Resource, e
}

if len(bytes.TrimSpace(bytes.TrimSpace(cmdutil.StripComments(edited)))) == 0 {
fmt.Println("Apply was skipped: empty file.")
fmt.Fprintln(os.Stderr, "Apply was skipped: empty file.")

break
}

if bytes.Equal(edited, body) {
fmt.Println("Apply was skipped: no changes detected.")
fmt.Fprintln(os.Stderr, "Apply was skipped: no changes detected.")

break
}
Expand Down
5 changes: 3 additions & 2 deletions cmd/talosctl/cmd/talos/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"bytes"
"context"
"fmt"
"os"
"time"

"github.com/cosi-project/runtime/pkg/resource"
Expand Down Expand Up @@ -71,12 +72,12 @@ func patchFn(c *client.Client, patches []configpatcher.Patch) func(context.Conte
bytes.TrimSpace(cmdutil.StripComments(patched)),
bytes.TrimSpace(cmdutil.StripComments(body)),
) {
fmt.Println("Apply was skipped: no changes detected.")
fmt.Fprintln(os.Stderr, "Apply was skipped: no changes detected.")

return nil
}

fmt.Printf("patched %s/%s at the node %s\n",
fmt.Fprintf(os.Stderr, "patched %s/%s at the node %s\n",
mc.Metadata().Type(),
mc.Metadata().ID(),
node,
Expand Down
6 changes: 3 additions & 3 deletions cmd/talosctl/cmd/talos/support.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ var supportCmd = &cobra.Command{
}
}

fmt.Printf("Support bundle is written to %s\n", supportCmdFlags.output)
fmt.Fprintf(os.Stderr, "Support bundle is written to %s\n", supportCmdFlags.output)

if err = archive.Archive.Close(); err != nil {
return err
Expand Down Expand Up @@ -234,7 +234,7 @@ func printErrors(err error) error {
var errs *multierror.Error

if !errors.As(err, &errs) {
fmt.Printf("Processed with errors:\n%s\n", color.RedString(err.Error()))
fmt.Fprintf(os.Stderr, "Processed with errors:\n%s\n", color.RedString(err.Error()))

return nil
}
Expand All @@ -243,7 +243,7 @@ func printErrors(err error) error {
if !wroteHeader {
wroteHeader = true

fmt.Println("Processed with errors:")
fmt.Fprintln(os.Stderr, "Processed with errors:")
fmt.Fprintln(w, "\tSOURCE\tERROR")
}

Expand Down
8 changes: 4 additions & 4 deletions cmd/talosctl/pkg/talos/action/tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,14 +210,14 @@ func (a *Tracker) Run() error {
if len(failedNodes) > 0 {
sort.Strings(failedNodes)

fmt.Printf("console logs for nodes %q:\n", failedNodes)
fmt.Fprintf(os.Stderr, "console logs for nodes %q:\n", failedNodes)

for _, node := range failedNodes {
dmesgReader, _ := failedNodesToDmesgs.Get(node)

_, copyErr := io.Copy(os.Stdout, dmesgReader)
_, copyErr := io.Copy(os.Stderr, dmesgReader)
if copyErr != nil {
fmt.Printf("%q: failed to print debug logs: %v\n", node, copyErr)
fmt.Fprintf(os.Stderr, "%q: failed to print debug logs: %v\n", node, copyErr)
}
}
}
Expand Down Expand Up @@ -251,7 +251,7 @@ func (a *Tracker) runReporter(ctx context.Context) error {

case update = <-a.reportCh:
if !a.isTerminal {
fmt.Printf("%q: %v\n", update.node, update.update.Message)
fmt.Fprintf(os.Stderr, "%q: %v\n", update.node, update.update.Message)

continue
}
Expand Down
3 changes: 2 additions & 1 deletion cmd/talosctl/pkg/talos/helpers/mode.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package helpers

import (
"fmt"
"os"
"sort"
"strings"

Expand Down Expand Up @@ -126,7 +127,7 @@ func PrintApplyResults(resp *machine.ApplyConfigurationResponse) {
}

if m.ModeDetails != "" {
fmt.Println(m.ModeDetails)
fmt.Fprintln(os.Stderr, m.ModeDetails)
}
}
}
6 changes: 5 additions & 1 deletion internal/integration/cli/apply-config.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ machine:

suite.Require().NoError(os.WriteFile(configPath, []byte(data), 0o777))

suite.RunCLI([]string{"apply-config", "--nodes", node, "--config-patch", fmt.Sprintf("@%s", patchPath), "-f", configPath})
suite.RunCLI([]string{"apply-config", "--nodes", node, "--config-patch", fmt.Sprintf("@%s", patchPath), "-f", configPath},
base.StdoutEmpty(),
base.StderrNotEmpty(),
base.StderrShouldMatch(regexp.MustCompile("Applied configuration without a reboot")),
)

suite.RunCLI([]string{"get", "--nodes", node, "links"},
base.StdoutShouldMatch(regexp.MustCompile("dummy-ap-patch")),
Expand Down
10 changes: 8 additions & 2 deletions internal/integration/cli/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ func (suite *TalosconfigSuite) TestList() {
func (suite *TalosconfigSuite) TestMerge() {
tempDir := suite.T().TempDir()

suite.RunCLI([]string{"gen", "config", "-o", tempDir, "foo", "https://192.168.0.1:6443"})
suite.RunCLI([]string{"gen", "config", "-o", tempDir, "foo", "https://192.168.0.1:6443"},
base.StdoutEmpty(),
base.StderrNotEmpty(),
)

talosconfigPath := filepath.Join(tempDir, "talosconfig")

Expand All @@ -58,7 +61,10 @@ func (suite *TalosconfigSuite) TestMerge() {
suite.Require().NotNil(c.Contexts["foo"])

suite.RunCLI([]string{"config", "merge", "--talosconfig", path, talosconfigPath},
base.StdoutShouldMatch(regexp.MustCompile(`renamed`)))
base.StdoutEmpty(),
base.StderrNotEmpty(),
base.StderrShouldMatch(regexp.MustCompile(`renamed`)),
)

c, err = clientconfig.Open(path)
suite.Require().NoError(err)
Expand Down
36 changes: 27 additions & 9 deletions internal/integration/cli/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,10 @@ func (suite *GenSuite) testGenConfigPatch(patch []byte) {
tt := tt

suite.Run(tt.flag, func() {
suite.RunCLI([]string{"gen", "config", "foo", "https://192.168.0.1:6443", "--" + tt.flag, string(patch)})
suite.RunCLI([]string{"gen", "config", "foo", "https://192.168.0.1:6443", "--" + tt.flag, string(patch)},
base.StdoutEmpty(),
base.StderrNotEmpty(),
base.StderrShouldMatch(regexp.MustCompile("generating PKI and tokens")))

for _, configName := range []string{"controlplane.yaml", "worker.yaml"} {
cfg, err := configloader.NewFromFile(configName)
Expand Down Expand Up @@ -270,7 +273,10 @@ func (suite *GenSuite) TestConfigWithSecrets() {
secretsYaml, err := os.ReadFile("secrets.yaml")
suite.Assert().NoError(err)

suite.RunCLI([]string{"gen", "config", "foo", "https://192.168.0.1:6443", "--with-secrets", "secrets.yaml"})
suite.RunCLI([]string{"gen", "config", "foo", "https://192.168.0.1:6443", "--with-secrets", "secrets.yaml"},
base.StdoutEmpty(),
base.StderrNotEmpty(),
)

config, err := configloader.NewFromFile("controlplane.yaml")
suite.Assert().NoError(err)
Expand All @@ -290,7 +296,10 @@ func (suite *GenSuite) TestGenConfigWithDeprecatedOutputDirFlag() {
"gen", "config",
"foo", "https://192.168.0.1:6443",
"--output-dir", tempDir,
})
},
base.StdoutEmpty(),
base.StderrNotEmpty(),
)

suite.Assert().FileExists(filepath.Join(tempDir, "controlplane.yaml"))
suite.Assert().FileExists(filepath.Join(tempDir, "worker.yaml"))
Expand All @@ -304,7 +313,7 @@ func (suite *GenSuite) TestGenConfigToStdoutControlPlane() {
"foo", "https://192.168.0.1:6443",
"--output-types", "controlplane",
"--output", "-",
}, base.StdoutMatchFunc(func(output string) error {
}, base.StderrNotEmpty(), base.StdoutMatchFunc(func(output string) error {
expected := "type: controlplane"
if !strings.Contains(output, expected) {
return fmt.Errorf("stdout does not contain %q: %q", expected, output)
Expand All @@ -321,7 +330,7 @@ func (suite *GenSuite) TestGenConfigToStdoutWorker() {
"foo", "https://192.168.0.1:6443",
"--output-types", "worker",
"--output", "-",
}, base.StdoutMatchFunc(func(output string) error {
}, base.StderrNotEmpty(), base.StdoutMatchFunc(func(output string) error {
expected := "type: worker"
if !strings.Contains(output, expected) {
return fmt.Errorf("stdout does not contain %q: %q", expected, output)
Expand All @@ -338,7 +347,7 @@ func (suite *GenSuite) TestGenConfigToStdoutTalosconfig() {
"foo", "https://192.168.0.1:6443",
"--output-types", "talosconfig",
"--output", "-",
}, base.StdoutMatchFunc(func(output string) error {
}, base.StderrNotEmpty(), base.StdoutMatchFunc(func(output string) error {
expected := "context: foo"
if !strings.Contains(output, expected) {
return fmt.Errorf("stdout does not contain %q: %q", expected, output)
Expand Down Expand Up @@ -376,7 +385,10 @@ func (suite *GenSuite) TestGenConfigMultipleTypesToDirectory() {
"foo", "https://192.168.0.1:6443",
"--output-types", "controlplane,worker",
"--output", tempDir,
})
},
base.StdoutEmpty(),
base.StderrNotEmpty(),
)

suite.Assert().FileExists(filepath.Join(tempDir, "controlplane.yaml"))
suite.Assert().FileExists(filepath.Join(tempDir, "worker.yaml"))
Expand All @@ -393,7 +405,10 @@ func (suite *GenSuite) TestGenConfigSingleTypeToFile() {
"foo", "https://192.168.0.1:6443",
"--output-types", "worker",
"--output", tempFile,
})
},
base.StdoutEmpty(),
base.StderrNotEmpty(),
)

suite.Assert().FileExists(tempFile)
}
Expand All @@ -408,7 +423,10 @@ func (suite *GenSuite) TestGenConfigSingleTypeWithDeprecatedOutputDirFlagToDirec
"foo", "https://192.168.0.1:6443",
"--output-types", "worker",
"--output-dir", tempDir,
})
},
base.StdoutEmpty(),
base.StderrNotEmpty(),
)

suite.Assert().FileExists(filepath.Join(tempDir, "worker.yaml"))
}
Expand Down
12 changes: 9 additions & 3 deletions internal/integration/cli/machineconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (suite *MachineConfigSuite) TestGen() {
"foo", "https://192.168.0.1:6443",
"--output-types", "controlplane",
"--output", "-",
}, base.StdoutMatchFunc(func(output string) error {
}, base.StderrNotEmpty(), base.StdoutMatchFunc(func(output string) error {
expected := "type: controlplane"
if !strings.Contains(output, expected) {
return fmt.Errorf("stdout does not contain %q: %q", expected, output)
Expand Down Expand Up @@ -96,7 +96,10 @@ func (suite *MachineConfigSuite) TestPatchPrintStdout() {
"foo", "https://192.168.0.1:6443",
"--output-types", "controlplane",
"--output", mc,
})
},
base.StderrNotEmpty(),
base.StdoutEmpty(),
)

suite.RunCLI([]string{
"machineconfig", "patch", mc, "--patch", string(patch1), "-p", "@" + patch2Path,
Expand Down Expand Up @@ -133,7 +136,10 @@ func (suite *MachineConfigSuite) TestPatchWriteToFile() {
"foo", "https://192.168.0.1:6443",
"--output-types", "controlplane",
"--output", mc,
})
},
base.StderrNotEmpty(),
base.StdoutEmpty(),
)

outputFile := filepath.Join(suite.T().TempDir(), "inner", "output.yaml")

Expand Down
10 changes: 8 additions & 2 deletions internal/integration/cli/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,14 @@ func (suite *PatchSuite) TestSuccess() {
data, err := json.Marshal(patch)
suite.Require().NoError(err)

suite.RunCLI([]string{"patch", "--nodes", node, "--patch", string(data), "machineconfig", "--mode=no-reboot"})
suite.RunCLI([]string{"patch", "--nodes", node, "--patch", string(data), "machineconfig", "--mode=no-reboot", "--dry-run"})
suite.RunCLI([]string{"patch", "--nodes", node, "--patch", string(data), "machineconfig", "--mode=no-reboot"},
base.StdoutEmpty(),
base.StderrNotEmpty(),
)
suite.RunCLI([]string{"patch", "--nodes", node, "--patch", string(data), "machineconfig", "--mode=no-reboot", "--dry-run"},
base.StdoutEmpty(),
base.StderrNotEmpty(),
)
}

// TestError runs comand with error.
Expand Down
31 changes: 17 additions & 14 deletions internal/integration/cli/reboot.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,25 +44,28 @@ func (suite *RebootSuite) TestReboot() {

suite.T().Logf("rebooting nodes %s via the CLI", nodes)

suite.RunCLI([]string{"reboot", "-n", nodes, "--debug"}, base.StdoutMatchFunc(func(stdout string) error {
if strings.Contains(stdout, "method is not supported") {
suite.T().Skip("reboot is not supported")
}
suite.RunCLI([]string{"reboot", "-n", nodes, "--debug"},
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")
}

var err error
var err error

for _, node := range []string{controlPlaneNode, workerNode} {
if !strings.Contains(stdout, fmt.Sprintf("%q: events check condition met", node)) {
err = multierror.Append(err, fmt.Errorf("events check condition not met on %v", node))
}
for _, node := range []string{controlPlaneNode, workerNode} {
if !strings.Contains(stdout, fmt.Sprintf("%q: events check condition met", node)) {
err = multierror.Append(err, fmt.Errorf("events check condition not met on %v", node))
}

if !strings.Contains(stdout, fmt.Sprintf("%q: post check passed", node)) {
err = multierror.Append(err, fmt.Errorf("post check not passed on %v", node))
if !strings.Contains(stdout, fmt.Sprintf("%q: post check passed", node)) {
err = multierror.Append(err, fmt.Errorf("post check not passed on %v", node))
}
}
}

return err
}))
return err
}))

suite.T().Logf("running the cluster health check")

Expand Down

0 comments on commit a0c0352

Please sign in to comment.