From adce02023c22d8681eb4ff5e0ae8df9eee5b8420 Mon Sep 17 00:00:00 2001 From: Stefan Bueringer Date: Mon, 20 Nov 2023 17:09:58 +0100 Subject: [PATCH] Improve output of exec.KubectlApply MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stefan Büringer buringerst@vmware.com --- cmd/clusterctl/cmd/topology_plan.go | 38 +++++++++++++++-------------- test/framework/cluster_proxy.go | 14 +++++++++-- test/framework/clusterctl/client.go | 7 ++++-- test/framework/exec/kubectl.go | 12 ++++----- 4 files changed, 43 insertions(+), 28 deletions(-) diff --git a/cmd/clusterctl/cmd/topology_plan.go b/cmd/clusterctl/cmd/topology_plan.go index cec05594628d..0fe62a1ab3dc 100644 --- a/cmd/clusterctl/cmd/topology_plan.go +++ b/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,33 +254,33 @@ 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. @@ -287,10 +288,10 @@ func writeOutputFiles(out *cluster.TopologyPlanOutput, outDir string) error { 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 diff --git a/test/framework/cluster_proxy.go b/test/framework/cluster_proxy.go index 6de289af4ca0..b52943f790c7 100644 --- a/test/framework/cluster_proxy.go +++ b/test/framework/cluster_proxy.go @@ -22,6 +22,7 @@ import ( "fmt" "net/url" "os" + "os/exec" "path" goruntime "runtime" "sync" @@ -29,6 +30,7 @@ import ( . "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 { diff --git a/test/framework/clusterctl/client.go b/test/framework/clusterctl/client.go index 92dc77ae5e29..8419689de8ed 100644 --- a/test/framework/clusterctl/client.go +++ b/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) } } diff --git a/test/framework/exec/kubectl.go b/test/framework/exec/kubectl.go index 0b3069348cd4..98c8313797bc 100644 --- a/test/framework/exec/kubectl.go +++ b/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.