Skip to content

Commit

Permalink
Adds message context to fleet apply errors
Browse files Browse the repository at this point in the history
Adds context to message in fleet apply. Specially to those running inside loops.

For example, when loading directories, remote charts, etc...

Contexts are wrapped in top of the previous one.

It also fixes the `gitjob` init container to use `log.sh` and redirect the errors to `/dev/termination-log`
Related to: #3234

Signed-off-by: Xavi Garcia <[email protected]>
  • Loading branch information
0xavi0 committed Feb 21, 2025
1 parent c497d38 commit 4dc0fd7
Show file tree
Hide file tree
Showing 8 changed files with 89 additions and 39 deletions.
20 changes: 13 additions & 7 deletions integrationtests/cli/apply/helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,13 +184,19 @@ func testHelmRepo(path, port string) {
})
Expect(err).To(HaveOccurred())
return err.Error()
}).Should(Equal(
fmt.Sprintf(
"repo=%s chart=%s version=%s: error parsing regexp: missing closing ): `a(b`",
fy.Helm.Repo,
fy.Helm.Chart,
fy.Helm.Version,
)))
}).Should(
And(
ContainSubstring("processing bundle"),
ContainSubstring(
fmt.Sprintf(
"repo=%s chart=%s version=%s: error parsing regexp: missing closing ): `a(b`",
fy.Helm.Repo,
fy.Helm.Chart,
fy.Helm.Version,
),
),
),
)
})
})

Expand Down
17 changes: 12 additions & 5 deletions internal/bundlereader/read.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/rancher/fleet/internal/fleetyaml"
"github.com/rancher/fleet/internal/names"
fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
"github.com/rancher/fleet/pkg/contexterror"

"github.com/sirupsen/logrus"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -42,12 +43,17 @@ type Options struct {
// Then it reads/downloads all referenced resources. It returns the populated
// bundle and any existing imagescans.
func Open(ctx context.Context, name, baseDir, file string, opts *Options) (*fleet.Bundle, []*fleet.ImageScan, error) {
errorContext := fmt.Sprintf("processing bundle %s", name)

if baseDir == "" {
baseDir = "."
}

if file == "-" {
return mayCompress(ctx, name, baseDir, os.Stdin, opts)
b, s, err := mayCompress(ctx, name, baseDir, os.Stdin, opts)
if err != nil {
return b, s, contexterror.Error(errorContext, err)
}
}

var (
Expand All @@ -56,7 +62,7 @@ func Open(ctx context.Context, name, baseDir, file string, opts *Options) (*flee

if file == "" {
if file, err := setupIOReader(baseDir); err != nil {
return nil, nil, err
return nil, nil, contexterror.Error(errorContext, err)
} else if file != nil {
in = file
defer file.Close()
Expand All @@ -67,13 +73,14 @@ func Open(ctx context.Context, name, baseDir, file string, opts *Options) (*flee
} else {
f, err := os.Open(filepath.Join(baseDir, file))
if err != nil {
return nil, nil, err
return nil, nil, contexterror.Error(errorContext, err)
}
defer f.Close()
in = f
}

return mayCompress(ctx, name, baseDir, in, opts)
b, s, err := mayCompress(ctx, name, baseDir, in, opts)
return b, s, contexterror.Error(errorContext, err)
}

// Try accessing the documented, primary fleet.yaml extension first. If that returns an "IsNotExist" error, then we
Expand Down Expand Up @@ -148,7 +155,7 @@ func read(ctx context.Context, name, baseDir string, bundleSpecReader io.Reader,

fy := &fleet.FleetYAML{}
if err := yaml.Unmarshal(bytes, fy); err != nil {
return nil, nil, err
return nil, nil, contexterror.Error("reading fleet.yaml", err)
}

var scans []*fleet.ImageScan
Expand Down
18 changes: 9 additions & 9 deletions internal/bundlereader/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/rancher/fleet/internal/bundlereader/progress"
fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
"github.com/rancher/fleet/pkg/contexterror"
"golang.org/x/sync/errgroup"
"golang.org/x/sync/semaphore"

Expand Down Expand Up @@ -128,12 +129,12 @@ func generateValues(base string, chart *fleet.HelmOptions) (valuesMap *fleet.Gen
for _, value := range chart.ValuesFiles {
valuesByte, err := os.ReadFile(base + "/" + value)
if err != nil {
return nil, err
return nil, contexterror.Error(fmt.Sprintf("reading values file: %s/%s", base, value), err)
}
tmpDataOpt := &fleet.GenericMap{}
err = yaml.Unmarshal(valuesByte, tmpDataOpt)
if err != nil {
return nil, err
return nil, contexterror.Error(fmt.Sprintf("reading values file: %s/%s", base, value), err)
}
valuesMap = mergeGenericMap(valuesMap, tmpDataOpt)
}
Expand All @@ -154,15 +155,15 @@ func addRemoteCharts(directories []directory, base string, charts []*fleet.HelmO
if _, err := os.Stat(filepath.Join(base, chart.Chart)); os.IsNotExist(err) || chart.Repo != "" {
shouldAddAuthToRequest, err := shouldAddAuthToRequest(helmRepoURLRegex, chart.Repo, chart.Chart)
if err != nil {
return nil, downloadChartError(*chart, err)
return nil, contexterror.Error(downloadChartError(*chart), err)
}
if !shouldAddAuthToRequest {
auth = Auth{}
}

chartURL, err := chartURL(*chart, auth)
if err != nil {
return nil, downloadChartError(*chart, err)
return nil, contexterror.Error(downloadChartError(*chart), err)
}

directories = append(directories, directory{
Expand All @@ -178,13 +179,12 @@ func addRemoteCharts(directories []directory, base string, charts []*fleet.HelmO
return directories, nil
}

func downloadChartError(c fleet.HelmOptions, e error) error {
return fmt.Errorf(
"repo=%s chart=%s version=%s: %w",
func downloadChartError(c fleet.HelmOptions) string {
return fmt.Sprintf(
"repo=%s chart=%s version=%s",
c.Repo,
c.Chart,
c.Version,
e,
)
}

Expand Down Expand Up @@ -226,7 +226,7 @@ func loadDirectories(ctx context.Context, compress bool, disableDepsUpdate bool,
defer sem.Release(1)
resources, err := loadDirectory(ctx, compress, disableDepsUpdate, dir.prefix, dir.base, dir.source, dir.version, dir.auth)
if err != nil {
return err
return contexterror.Error(fmt.Sprintf("loading directory %s, %s", dir.prefix, dir.base), err)
}

key := dir.key
Expand Down
6 changes: 4 additions & 2 deletions internal/cmd/cli/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/rancher/fleet/internal/cmd/cli/apply"
"github.com/rancher/fleet/internal/cmd/cli/writer"
fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
"github.com/rancher/fleet/pkg/contexterror"

"k8s.io/apimachinery/pkg/util/yaml"
)
Expand Down Expand Up @@ -98,11 +99,12 @@ func (a *Apply) Run(cmd *cobra.Command, args []string) error {
CorrectDriftKeepFailHistory: a.CorrectDriftKeepFailHistory,
}
if err := a.addAuthToOpts(&opts, os.ReadFile); err != nil {
return err
return contexterror.Error("adding auth to opts", err)
}
if err := a.addOCISpecToOpts(&opts, os.ReadFile); err != nil {
return err
return contexterror.Error("adding oci spec to opts", err)
}

if a.File == "-" {
opts.BundleReader = os.Stdin
if len(args) != 1 {
Expand Down
7 changes: 4 additions & 3 deletions internal/cmd/cli/apply/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/rancher/fleet/internal/names"
"github.com/rancher/fleet/internal/ociwrapper"
fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
"github.com/rancher/fleet/pkg/contexterror"

"github.com/rancher/wrangler/v3/pkg/yaml"

Expand Down Expand Up @@ -105,14 +106,14 @@ func CreateBundles(ctx context.Context, client Getter, repoName string, baseDirs
for _, baseDir := range matches {
if i > 0 && opts.Output != nil {
if _, err := opts.Output.Write([]byte("\n---\n")); err != nil {
return err
return contexterror.Error("writing to bundle output", err)
}
}
err := filepath.Walk(baseDir, func(path string, info os.FileInfo, err error) error {
opts := opts
createBundle, e := shouldCreateBundleForThisPath(baseDir, path, info)
if e != nil {
return e
return contexterror.Error(fmt.Sprintf("checking for bundle in path %s", path), e)
}
if !createBundle {
return nil
Expand Down Expand Up @@ -180,7 +181,7 @@ func readBundle(ctx context.Context, name, baseDir string, opts *Options) (*flee
if opts.BundleReader != nil {
var bundle *fleet.Bundle
if err := json.NewDecoder(opts.BundleReader).Decode(bundle); err != nil {
return nil, nil, err
return nil, nil, contexterror.Error(fmt.Sprintf("decoding bundle %s", name), err)
}
return bundle, nil, nil
}
Expand Down
34 changes: 25 additions & 9 deletions internal/cmd/cli/gitcloner/cloner.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
httpgit "github.com/go-git/go-git/v5/plumbing/transport/http"
gossh "github.com/go-git/go-git/v5/plumbing/transport/ssh"

"github.com/rancher/fleet/pkg/contexterror"
giturls "github.com/rancher/fleet/pkg/git-urls"
"github.com/sirupsen/logrus"
"golang.org/x/crypto/ssh"
Expand Down Expand Up @@ -49,11 +50,11 @@ func (c *Cloner) CloneRepo(opts *GitCloner) error {
}
auth, err := createAuthFromOpts(opts)
if err != nil {
return err
return contexterror.Error(gitClonerErrorContext(opts), err)
}
caBundle, err := getCABundleFromFile(opts.CABundleFile)
if err != nil {
return err
return contexterror.Error(gitClonerErrorContext(opts), err)
}

if opts.Branch == "" && opts.Revision == "" {
Expand Down Expand Up @@ -82,7 +83,10 @@ func cloneBranch(opts *GitCloner, auth transport.AuthMethod, caBundle []byte) er
RecurseSubmodules: git.DefaultSubmoduleRecursionDepth,
})

return err
if err != nil {
return contexterror.Error(gitClonerErrorContext(opts), err)
}
return nil
}

func cloneRevision(opts *GitCloner, auth transport.AuthMethod, caBundle []byte) error {
Expand All @@ -94,20 +98,22 @@ func cloneRevision(opts *GitCloner, auth transport.AuthMethod, caBundle []byte)
RecurseSubmodules: git.DefaultSubmoduleRecursionDepth,
})
if err != nil {
return err
return contexterror.Error(gitClonerErrorContext(opts), err)
}
h, err := r.ResolveRevision(plumbing.Revision(opts.Revision))
if err != nil {
return err
return contexterror.Error(gitClonerErrorContext(opts), err)
}
w, err := r.Worktree()
if err != nil {
return err
return contexterror.Error(gitClonerErrorContext(opts), err)
}

return w.Checkout(&git.CheckoutOptions{
Hash: *h,
})
if err := w.Checkout(&git.CheckoutOptions{Hash: *h}); err != nil {
return contexterror.Error(gitClonerErrorContext(opts), err)
}

return nil
}

func getCABundleFromFile(path string) ([]byte, error) {
Expand Down Expand Up @@ -182,3 +188,13 @@ func createKnownHostsCallBack(knownHosts []byte) (ssh.HostKeyCallback, error) {

return gossh.NewKnownHostsCallback(f.Name())
}

func gitClonerErrorContext(opts *GitCloner) string {
return fmt.Sprintf(
"gitcloner: repo=[%s] branch=[%s] revision=[%s] path=[%s]",
opts.Repo,
opts.Branch,
opts.Revision,
opts.Path,
)
}
15 changes: 11 additions & 4 deletions internal/cmd/controller/gitops/reconciler/gitjob_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1121,7 +1121,7 @@ func volumesFromSecret(
}

func (r *GitJobReconciler) newGitCloner(ctx context.Context, obj *v1alpha1.GitRepo) (corev1.Container, error) {
args := []string{"gitcloner", obj.Spec.Repo, "/workspace"}
args := []string{"fleet", "gitcloner", obj.Spec.Repo, "/workspace"}
volumeMounts := []corev1.VolumeMount{
{
Name: gitClonerVolumeName,
Expand Down Expand Up @@ -1194,9 +1194,7 @@ func (r *GitJobReconciler) newGitCloner(ctx context.Context, obj *v1alpha1.GitRe
}

return corev1.Container{
Command: []string{
"fleet",
},
Command: []string{"log.sh"},
Args: args,
Image: r.Image,
Name: "gitcloner-initializer",
Expand Down Expand Up @@ -1372,6 +1370,15 @@ func setStatusFromGitjob(ctx context.Context, c client.Client, gitRepo *v1alpha1
terminationMessage += podStatus.State.Terminated.Message
}
}

// set also the message from init containers (if they failed)
for _, podStatus := range podList.Items[len(podList.Items)-1].Status.InitContainerStatuses {
if podStatus.Name != "step-git-source" &&
podStatus.State.Terminated != nil &&
podStatus.State.Terminated.ExitCode != 0 {
terminationMessage += podStatus.State.Terminated.Message
}
}
}
}

Expand Down
11 changes: 11 additions & 0 deletions pkg/contexterror/contexterror.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package contexterror

import "fmt"

// Error wraps the given error adding additional context if the error is not nil
func Error(context string, e error) error {
if e == nil {
return e
}
return fmt.Errorf("%s: %w", context, e)
}

0 comments on commit 4dc0fd7

Please sign in to comment.