Merge pull request kubernetes-sigs#9737 from sbueringer/pr-improve-ap…

…ply-output

🌱 Improve output of exec.KubectlApply

cmd/clusterctl/cmd/topology_plan.go

@@ -18,6 +18,7 @@ package cmd
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -28,7 +29,7 @@ import (
 	"strings"
 
 	"github.com/olekukonko/tablewriter"
-	"github.com/pkg/errors"
+	pkgerrors "github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/utils/exec"
@@ -121,11 +122,11 @@ func runTopologyPlan() error {
 	for _, f := range tp.files {
 		raw, err := os.ReadFile(f) //nolint:gosec
 		if err != nil {
-			return errors.Wrapf(err, "failed to read input file %q", f)
+			return pkgerrors.Wrapf(err, "failed to read input file %q", f)
 		}
 		objects, err := utilyaml.ToUnstructured(raw)
 		if err != nil {
-			return errors.Wrapf(err, "failed to convert file %q to list of objects", f)
+			return pkgerrors.Wrapf(err, "failed to convert file %q to list of objects", f)
 		}
 		objs = append(objs, objects...)
 	}
@@ -154,7 +155,7 @@ func printTopologyPlanOutput(out *cluster.TopologyPlanOutput, outdir string) err
 	} else {
 		printChangeSummary(out)
 		if err := writeOutputFiles(out, outdir); err != nil {
-			return errors.Wrap(err, "failed to write output files of target cluster changes")
+			return pkgerrors.Wrap(err, "failed to write output files of target cluster changes")
 		}
 	}
 	fmt.Printf("\n")
@@ -233,17 +234,17 @@ func writeOutputFiles(out *cluster.TopologyPlanOutput, outDir string) error {
 	// Write created files
 	createdDir := path.Join(outDir, "created")
 	if err := os.MkdirAll(createdDir, 0750); err != nil {
-		return errors.Wrapf(err, "failed to create %q directory", createdDir)
+		return pkgerrors.Wrapf(err, "failed to create %q directory", createdDir)
 	}
 	for _, c := range out.Created {
 		yaml, err := utilyaml.FromUnstructured([]unstructured.Unstructured{*c})
 		if err != nil {
-			return errors.Wrap(err, "failed to convert object to yaml")
+			return pkgerrors.Wrap(err, "failed to convert object to yaml")
 		}
 		fileName := fmt.Sprintf("%s_%s_%s.yaml", c.GetKind(), c.GetNamespace(), c.GetName())
 		filePath := path.Join(createdDir, fileName)
 		if err := os.WriteFile(filePath, yaml, 0600); err != nil {
-			return errors.Wrapf(err, "failed to write yaml to file %q", filePath)
+			return pkgerrors.Wrapf(err, "failed to write yaml to file %q", filePath)
 		}
 	}
 	if len(out.Created) != 0 {
@@ -253,44 +254,44 @@ func writeOutputFiles(out *cluster.TopologyPlanOutput, outDir string) error {
 	// Write modified files
 	modifiedDir := path.Join(outDir, "modified")
 	if err := os.MkdirAll(modifiedDir, 0750); err != nil {
-		return errors.Wrapf(err, "failed to create %q directory", modifiedDir)
+		return pkgerrors.Wrapf(err, "failed to create %q directory", modifiedDir)
 	}
 	for _, m := range out.Modified {
 		// Write the modified object to file.
 		fileNameModified := fmt.Sprintf("%s_%s_%s.modified.yaml", m.After.GetKind(), m.After.GetNamespace(), m.After.GetName())
 		filePathModified := path.Join(modifiedDir, fileNameModified)
 		if err := writeObjectToFile(filePathModified, m.After); err != nil {
-			return errors.Wrap(err, "failed to write modified object to file")
+			return pkgerrors.Wrap(err, "failed to write modified object to file")
 		}
 
 		// Write the original object to file.
 		fileNameOriginal := fmt.Sprintf("%s_%s_%s.original.yaml", m.Before.GetKind(), m.Before.GetNamespace(), m.Before.GetName())
 		filePathOriginal := path.Join(modifiedDir, fileNameOriginal)
 		if err := writeObjectToFile(filePathOriginal, m.Before); err != nil {
-			return errors.Wrap(err, "failed to write original object to file")
+			return pkgerrors.Wrap(err, "failed to write original object to file")
 		}
 
 		// Calculate the jsonpatch and write to a file.
 		patch := crclient.MergeFrom(m.Before)
 		jsonPatch, err := patch.Data(m.After)
 		if err != nil {
-			return errors.Wrapf(err, "failed to calculate jsonpatch of modified object %s/%s", m.After.GetNamespace(), m.After.GetName())
+			return pkgerrors.Wrapf(err, "failed to calculate jsonpatch of modified object %s/%s", m.After.GetNamespace(), m.After.GetName())
 		}
 		patchFileName := fmt.Sprintf("%s_%s_%s.jsonpatch", m.After.GetKind(), m.After.GetNamespace(), m.After.GetName())
 		patchFilePath := path.Join(modifiedDir, patchFileName)
 		if err := os.WriteFile(patchFilePath, jsonPatch, 0600); err != nil {
-			return errors.Wrapf(err, "failed to write jsonpatch to file %q", patchFilePath)
+			return pkgerrors.Wrapf(err, "failed to write jsonpatch to file %q", patchFilePath)
 		}
 
 		// Calculate the diff and write to a file.
 		diffFileName := fmt.Sprintf("%s_%s_%s.diff", m.After.GetKind(), m.After.GetNamespace(), m.After.GetName())
 		diffFilePath := path.Join(modifiedDir, diffFileName)
 		diffFile, err := os.OpenFile(filepath.Clean(diffFilePath), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600)
 		if err != nil {
-			return errors.Wrapf(err, "unable to open file %q", diffFilePath)
+			return pkgerrors.Wrapf(err, "unable to open file %q", diffFilePath)
 		}
 		if err := writeDiffToFile(filePathOriginal, filePathModified, diffFile); err != nil {
-			return errors.Wrapf(err, "failed to write diff to file %q", diffFilePath)
+			return pkgerrors.Wrapf(err, "failed to write diff to file %q", diffFilePath)
 		}
 	}
 	if len(out.Modified) != 0 {
@@ -303,10 +304,10 @@ func writeOutputFiles(out *cluster.TopologyPlanOutput, outDir string) error {
 func writeObjectToFile(filePath string, obj *unstructured.Unstructured) error {
 	yaml, err := utilyaml.FromUnstructured([]unstructured.Unstructured{*obj})
 	if err != nil {
-		return errors.Wrap(err, "failed to convert object to yaml")
+		return pkgerrors.Wrap(err, "failed to convert object to yaml")
 	}
 	if err := os.WriteFile(filePath, yaml, 0600); err != nil {
-		return errors.Wrapf(err, "failed to write yaml to file %q", filePath)
+		return pkgerrors.Wrapf(err, "failed to write yaml to file %q", filePath)
 	}
 	return nil
 }
@@ -348,7 +349,7 @@ func writeDiffToFile(from, to string, out io.Writer) error {
 	cmd.SetStdout(out)
 
 	if err := cmd.Run(); err != nil && !isDiffError(err) {
-		return errors.Wrapf(err, "failed to run %q", diff)
+		return pkgerrors.Wrapf(err, "failed to run %q", diff)
 	}
 	return nil
 }
@@ -382,7 +383,8 @@ func getDiffCommand(args ...string) (string, exec.Cmd) {
 // This makes use of the exit code of diff programs which is 0 for no diff, 1 for
 // modified and 2 for other errors.
 func isDiffError(err error) bool {
-	if err, ok := err.(exec.ExitError); ok && err.ExitStatus() <= 1 {
+	var exitErr exec.ExitError
+	if errors.As(err, &exitErr) && exitErr.ExitStatus() <= 1 {
 		return true
 	}
 	return false

test/framework/cluster_proxy.go

@@ -22,13 +22,15 @@ import (
 	"fmt"
 	"net/url"
 	"os"
+	"os/exec"
 	"path"
 	goruntime "runtime"
 	"sync"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	pkgerrors "github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -42,7 +44,7 @@ import (
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
-	"sigs.k8s.io/cluster-api/test/framework/exec"
+	testexec "sigs.k8s.io/cluster-api/test/framework/exec"
 	"sigs.k8s.io/cluster-api/test/framework/internal/log"
 	"sigs.k8s.io/cluster-api/test/infrastructure/container"
 )
@@ -250,7 +252,15 @@ func (p *clusterProxy) Apply(ctx context.Context, resources []byte, args ...stri
 	Expect(ctx).NotTo(BeNil(), "ctx is required for Apply")
 	Expect(resources).NotTo(BeNil(), "resources is required for Apply")
 
-	return exec.KubectlApply(ctx, p.kubeconfigPath, resources, args...)
+	if err := testexec.KubectlApply(ctx, p.kubeconfigPath, resources, args...); err != nil {
+		var exitErr *exec.ExitError
+		if errors.As(err, &exitErr) {
+			return pkgerrors.New(fmt.Sprintf("%s: stderr: %s", err.Error(), exitErr.Stderr))
+		}
+		return err
+	}
+
+	return nil
 }
 
 func (p *clusterProxy) GetRESTConfig() *rest.Config {

test/framework/clusterctl/client.go

@@ -18,6 +18,7 @@ package clusterctl
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -99,7 +100,8 @@ func InitWithBinary(_ context.Context, binary string, input InitInput) {
 	_ = os.WriteFile(filepath.Join(input.LogFolder, "clusterctl-init.log"), out, 0644) //nolint:gosec // this is a log file to be shared via prow artifacts
 	var stdErr string
 	if err != nil {
-		if exitErr, ok := err.(*exec.ExitError); ok {
+		var exitErr *exec.ExitError
+		if errors.As(err, &exitErr) {
 			stdErr = string(exitErr.Stderr)
 		}
 	}
@@ -333,7 +335,8 @@ func ConfigClusterWithBinary(_ context.Context, clusterctlBinaryPath string, inp
 	_ = os.WriteFile(filepath.Join(input.LogFolder, fmt.Sprintf("%s-cluster-template.yaml", input.ClusterName)), out, 0644) //nolint:gosec // this is a log file to be shared via prow artifacts
 	var stdErr string
 	if err != nil {
-		if exitErr, ok := err.(*exec.ExitError); ok {
+		var exitErr *exec.ExitError
+		if errors.As(err, &exitErr) {
 			stdErr = string(exitErr.Stderr)
 		}
 	}

test/framework/exec/kubectl.go

@@ -21,6 +21,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"strings"
 )
 
 // KubectlApply shells out to kubectl apply.
@@ -34,13 +35,12 @@ func KubectlApply(ctx context.Context, kubeconfigPath string, resources []byte,
 		WithArgs(aargs...),
 		WithStdin(rbytes),
 	)
+
+	fmt.Printf("Running kubectl %s\n", strings.Join(aargs, " "))
 	stdout, stderr, err := applyCmd.Run(ctx)
-	if err != nil {
-		fmt.Println(string(stderr))
-		return err
-	}
-	fmt.Println(string(stdout))
-	return nil
+	fmt.Printf("stderr:\n%s\n", string(stderr))
+	fmt.Printf("stdout:\n%s\n", string(stdout))
+	return err
 }
 
 // KubectlWait shells out to kubectl wait.