From 9e1249fdb366476c58ab3ef7584cb049c886a682 Mon Sep 17 00:00:00 2001 From: Tom Wieczorek Date: Mon, 9 Dec 2024 10:22:24 +0100 Subject: [PATCH 1/2] Use strings.Builder instead of bytes.Buffer The out and err streams are always treated as strings for better readability, so it's more idiomatic to use strings.Builder for them. Also, replace intoLines with strings.Split, as strings.Builder doesn't implement io.Reader, which eliminates another helper function on the fly. Signed-off-by: Tom Wieczorek --- cmd/airgap/listimages_test.go | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/cmd/airgap/listimages_test.go b/cmd/airgap/listimages_test.go index 38c42aaf0959..171f62b50518 100644 --- a/cmd/airgap/listimages_test.go +++ b/cmd/airgap/listimages_test.go @@ -17,14 +17,12 @@ limitations under the License. package airgap import ( - "bufio" - "bytes" "errors" "fmt" - "io" "os" "path/filepath" "runtime" + "strings" "testing" "testing/iotest" @@ -48,7 +46,7 @@ func TestAirgapListImages(t *testing.T) { underTest, out, err := newAirgapListImagesCmdWithConfig(t, "{}", "--all") require.NoError(t, underTest.Execute()) - lines := intoLines(out) + lines := strings.Split(out.String(), "\n") if runtime.GOARCH == "arm" { assert.NotContains(t, lines, defaultImage) } else { @@ -88,7 +86,7 @@ spec: require.NoError(t, underTest.Execute()) - lines := intoLines(out) + lines := strings.Split(out.String(), "\n") for _, contained := range test.contained { if runtime.GOARCH == "arm" { assert.NotContains(t, lines, contained) @@ -105,11 +103,11 @@ spec: }) } -func newAirgapListImagesCmdWithConfig(t *testing.T, config string, args ...string) (_ *cobra.Command, out, err *bytes.Buffer) { +func newAirgapListImagesCmdWithConfig(t *testing.T, config string, args ...string) (_ *cobra.Command, out, err *strings.Builder) { configFile := filepath.Join(t.TempDir(), "k0s.yaml") require.NoError(t, os.WriteFile(configFile, []byte(config), 0644)) - out, err = new(bytes.Buffer), new(bytes.Buffer) + out, err = new(strings.Builder), new(strings.Builder) cmd := NewAirgapListImagesCmd() cmd.SetArgs(append([]string{"--config=" + configFile}, args...)) cmd.SetIn(iotest.ErrReader(errors.New("unexpected read from standard input"))) @@ -117,12 +115,3 @@ func newAirgapListImagesCmdWithConfig(t *testing.T, config string, args ...strin cmd.SetErr(err) return cmd, out, err } - -func intoLines(in io.Reader) (lines []string) { - scanner := bufio.NewScanner(in) - scanner.Split(bufio.ScanLines) - for scanner.Scan() { - lines = append(lines, scanner.Text()) - } - return -} From 868a10982a823b4491f1971fd0b423a0e7c42ed8 Mon Sep 17 00:00:00 2001 From: Tom Wieczorek Date: Mon, 9 Dec 2024 11:12:25 +0100 Subject: [PATCH 2/2] Honor IO errors in list-images sub-command Previously, the sub-command did not interrupt the loop and returned successfully, even when it encountered IO errors while writing to standard out. Signed-off-by: Tom Wieczorek --- cmd/airgap/listimages.go | 5 ++++- cmd/airgap/listimages_test.go | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/cmd/airgap/listimages.go b/cmd/airgap/listimages.go index 1291de8a22e7..34cf42a7adc0 100644 --- a/cmd/airgap/listimages.go +++ b/cmd/airgap/listimages.go @@ -48,8 +48,11 @@ func NewAirgapListImagesCmd() *cobra.Command { return fmt.Errorf("failed to get config: %w", err) } + out := cmd.OutOrStdout() for _, uri := range airgap.GetImageURIs(clusterConfig.Spec, all) { - fmt.Fprintln(cmd.OutOrStdout(), uri) + if _, err := fmt.Fprintln(out, uri); err != nil { + return err + } } return nil }, diff --git a/cmd/airgap/listimages_test.go b/cmd/airgap/listimages_test.go index 171f62b50518..be20ffc85a87 100644 --- a/cmd/airgap/listimages_test.go +++ b/cmd/airgap/listimages_test.go @@ -26,7 +26,9 @@ import ( "testing" "testing/iotest" + internalio "github.com/k0sproject/k0s/internal/io" "github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1" + "github.com/spf13/cobra" "github.com/stretchr/testify/assert" @@ -42,6 +44,23 @@ func TestAirgapListImages(t *testing.T) { defaultImage := v1beta1.DefaultEnvoyProxyImage().URI() + t.Run("HonorsIOErrors", func(t *testing.T) { + var writes uint + underTest := NewAirgapListImagesCmd() + underTest.SetIn(iotest.ErrReader(errors.New("unexpected read from standard input"))) + underTest.SilenceUsage = true // Cobra writes usage to stdout on errors 🤔 + underTest.SetOut(internalio.WriterFunc(func(p []byte) (int, error) { + writes++ + return 0, assert.AnError + })) + var stderr strings.Builder + underTest.SetErr(&stderr) + + assert.Same(t, assert.AnError, underTest.Execute()) + assert.Equal(t, uint(1), writes, "Expected a single write to stdout") + assert.Equal(t, fmt.Sprintf("Error: %v\n", assert.AnError), stderr.String()) + }) + t.Run("All", func(t *testing.T) { underTest, out, err := newAirgapListImagesCmdWithConfig(t, "{}", "--all")