Repository: rancher/fleet
Pull request #3385 (2025-02-21 12:38:10)

Adds message context to fleet apply errors
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: https://github.com/rancher/fleet/issues/3234




Patch:
diff --git a/internal/bundlereader/read.go b/internal/bundlereader/read.go
index 1b669473..b7fef8fa 100644
--- a/internal/bundlereader/read.go
+++ b/internal/bundlereader/read.go
@@ -47,7 +47,10 @@ func Open(ctx context.Context, name, baseDir, file string, opts *Options) (*flee
 	}
 
 	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, fmt.Errorf("failed to process bundle from STDIN: %w", err)
+		}
 	}
 
 	var (
@@ -56,7 +59,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, fmt.Errorf("failed to open existing fleet.yaml in %q: %w", baseDir, err)
 		} else if file != nil {
 			in = file
 			defer file.Close()
@@ -67,13 +70,18 @@ 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, fmt.Errorf("failed to open file %q: %w", file, err)
 		}
 		defer f.Close()
 		in = f
 	}
 
-	return mayCompress(ctx, name, baseDir, in, opts)
+	b, s, err := mayCompress(ctx, name, baseDir, in, opts)
+	if err != nil {
+		return b, s, fmt.Errorf("failed to process bundle: %w", err)
+	}
+
+	return b, s, nil
 }
 
 // Try accessing the documented, primary fleet.yaml extension first. If that returns an "IsNotExist" error, then we
@@ -148,7 +156,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, fmt.Errorf("reading fleet.yaml: %w", err)
 	}
 
 	var scans []*fleet.ImageScan
diff --git a/internal/bundlereader/resources.go b/internal/bundlereader/resources.go
index a513e532..cd56d596 100644
--- a/internal/bundlereader/resources.go
+++ b/internal/bundlereader/resources.go
@@ -128,12 +128,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, fmt.Errorf("reading values file: %s/%s: %w", base, value, err)
 		}
 		tmpDataOpt := &fleet.GenericMap{}
 		err = yaml.Unmarshal(valuesByte, tmpDataOpt)
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("reading values file: %s/%s: %w", base, value, err)
 		}
 		valuesMap = mergeGenericMap(valuesMap, tmpDataOpt)
 	}
@@ -154,7 +154,7 @@ 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, fmt.Errorf("failed to add auth to request for %s: %w", downloadChartError(*chart), err)
 			}
 			if !shouldAddAuthToRequest {
 				auth = Auth{}
@@ -162,7 +162,7 @@ func addRemoteCharts(directories []directory, base string, charts []*fleet.HelmO
 
 			chartURL, err := chartURL(*chart, auth)
 			if err != nil {
-				return nil, downloadChartError(*chart, err)
+				return nil, fmt.Errorf("failed to resolve URL of %s: %w", downloadChartError(*chart), err)
 			}
 
 			directories = append(directories, directory{
@@ -178,13 +178,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,
 	)
 }
 
@@ -226,7 +225,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 fmt.Errorf("loading directory %s, %s: %w", dir.prefix, dir.base, err)
 			}
 
 			key := dir.key
diff --git a/internal/cmd/cli/apply.go b/internal/cmd/cli/apply.go
index 59166932..14b9416d 100644
--- a/internal/cmd/cli/apply.go
+++ b/internal/cmd/cli/apply.go
@@ -98,11 +98,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 fmt.Errorf("adding auth to opts: %w", err)
 	}
 	if err := a.addOCISpecToOpts(&opts, os.ReadFile); err != nil {
-		return err
+		return fmt.Errorf("adding oci spec to opts: %w", err)
 	}
+
 	if a.File == "-" {
 		opts.BundleReader = os.Stdin
 		if len(args) != 1 {
diff --git a/internal/cmd/cli/apply/apply.go b/internal/cmd/cli/apply/apply.go
index 5c55c600..6dc7d378 100644
--- a/internal/cmd/cli/apply/apply.go
+++ b/internal/cmd/cli/apply/apply.go
@@ -105,14 +105,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 fmt.Errorf("writing to bundle output: %w", 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 fmt.Errorf("checking for bundle in path %q: %w", path, err)
 				}
 				if !createBundle {
 					return nil
@@ -180,7 +180,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, fmt.Errorf("decoding bundle %s: %w", name, err)
 		}
 		return bundle, nil, nil
 	}
diff --git a/internal/cmd/cli/gitcloner/cloner.go b/internal/cmd/cli/gitcloner/cloner.go
index d0e63f17..78286b8f 100644
--- a/internal/cmd/cli/gitcloner/cloner.go
+++ b/internal/cmd/cli/gitcloner/cloner.go
@@ -49,11 +49,11 @@ func (c *Cloner) CloneRepo(opts *GitCloner) error {
 	}
 	auth, err := createAuthFromOpts(opts)
 	if err != nil {
-		return err
+		return fmt.Errorf("%s: %w", gitClonerErrorContext(opts), err)
 	}
 	caBundle, err := getCABundleFromFile(opts.CABundleFile)
 	if err != nil {
-		return err
+		return fmt.Errorf("%s: %w", gitClonerErrorContext(opts), err)
 	}
 
 	if opts.Branch == "" && opts.Revision == "" {
@@ -82,7 +82,10 @@ func cloneBranch(opts *GitCloner, auth transport.AuthMethod, caBundle []byte) er
 		RecurseSubmodules: git.DefaultSubmoduleRecursionDepth,
 	})
 
-	return err
+	if err != nil {
+		return fmt.Errorf("%s: %w", gitClonerErrorContext(opts), err)
+	}
+	return nil
 }
 
 func cloneRevision(opts *GitCloner, auth transport.AuthMethod, caBundle []byte) error {
@@ -94,20 +97,22 @@ func cloneRevision(opts *GitCloner, auth transport.AuthMethod, caBundle []byte)
 		RecurseSubmodules: git.DefaultSubmoduleRecursionDepth,
 	})
 	if err != nil {
-		return err
+		return fmt.Errorf("%s: %w", gitClonerErrorContext(opts), err)
 	}
 	h, err := r.ResolveRevision(plumbing.Revision(opts.Revision))
 	if err != nil {
-		return err
+		return fmt.Errorf("%s: %w", gitClonerErrorContext(opts), err)
 	}
 	w, err := r.Worktree()
 	if err != nil {
-		return err
+		return fmt.Errorf("%s: %w", gitClonerErrorContext(opts), err)
 	}
 
-	return w.Checkout(&git.CheckoutOptions{
-		Hash: *h,
-	})
+	if err := w.Checkout(&git.CheckoutOptions{Hash: *h}); err != nil {
+		return fmt.Errorf("%s: %w", gitClonerErrorContext(opts), err)
+	}
+
+	return nil
 }
 
 func getCABundleFromFile(path string) ([]byte, error) {
@@ -182,3 +187,13 @@ func createKnownHostsCallBack(knownHosts []byte) (ssh.HostKeyCallback, error) {
 
 	return gossh.NewKnownHostsCallback(f.Name())
 }
+
+func gitClonerErrorContext(opts *GitCloner) string {
+	return fmt.Sprintf(
+		"failed checkout of: repo=[%s] branch=[%s] revision=[%s] path=[%s]",
+		opts.Repo,
+		opts.Branch,
+		opts.Revision,
+		opts.Path,
+	)
+}
diff --git a/internal/cmd/controller/gitops/reconciler/gitjob_controller.go b/internal/cmd/controller/gitops/reconciler/gitjob_controller.go
index efbfb6e5..33364d45 100644
--- a/internal/cmd/controller/gitops/reconciler/gitjob_controller.go
+++ b/internal/cmd/controller/gitops/reconciler/gitjob_controller.go
@@ -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,
@@ -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",
@@ -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
+				}
+			}
 		}
 	}
 


Test patch:
diff --git a/integrationtests/cli/apply/helm_test.go b/integrationtests/cli/apply/helm_test.go
index 9e9bfc3e..30ad347c 100644
--- a/integrationtests/cli/apply/helm_test.go
+++ b/integrationtests/cli/apply/helm_test.go
@@ -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("failed to process bundle: failed to add auth to request for"),
+					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,
+						),
+					),
+				),
+			)
 		})
 	})
 
diff --git a/internal/cmd/cli/gitcloner/cloner_test.go b/internal/cmd/cli/gitcloner/cloner_test.go
index a50505fa..c268832e 100644
--- a/internal/cmd/cli/gitcloner/cloner_test.go
+++ b/internal/cmd/cli/gitcloner/cloner_test.go
@@ -47,9 +47,6 @@ udiSlDctMM/X3ZM2JN5M1rtAJ2WR3ZQtmWbOjZAbG2Eq
 			x.Signer.PublicKey().Type() == y.Signer.PublicKey().Type() &&
 			cmp.Equal(x.Signer.PublicKey().Marshal(), y.Signer.PublicKey().Marshal())
 	})
-	errorComparer := cmp.Comparer(func(x, y error) bool {
-		return x.Error() == y.Error()
-	})
 	plainClone = func(path string, isBare bool, o *git.CloneOptions) (*git.Repository, error) {
 		pathCalled = path
 		isBareCalled = isBare
@@ -131,7 +128,7 @@ udiSlDctMM/X3ZM2JN5M1rtAJ2WR3ZQtmWbOjZAbG2Eq
 				Username:     "user",
 			},
 			expectedCloneOpts: nil,
-			expectedErr:       errors.New("file not found"),
+			expectedErr:       errors.New("failed checkout of: repo=[repo] branch=[master] revision=[] path=[]: file not found"),
 		},
 		"ca file does not exist": {
 			opts: &GitCloner{
@@ -140,7 +137,7 @@ udiSlDctMM/X3ZM2JN5M1rtAJ2WR3ZQtmWbOjZAbG2Eq
 				CABundleFile: "doesntexist",
 			},
 			expectedCloneOpts: nil,
-			expectedErr:       errors.New("file not found"),
+			expectedErr:       errors.New("failed checkout of: repo=[repo] branch=[master] revision=[] path=[]: file not found"),
 		},
 		"ssh private key file does not exist": {
 			opts: &GitCloner{
@@ -149,7 +146,7 @@ udiSlDctMM/X3ZM2JN5M1rtAJ2WR3ZQtmWbOjZAbG2Eq
 				SSHPrivateKeyFile: "doesntexist",
 			},
 			expectedCloneOpts: nil,
-			expectedErr:       errors.New("file not found"),
+			expectedErr:       errors.New("failed checkout of: repo=[repo] branch=[master] revision=[] path=[]: file not found"),
 		},
 	}
 
@@ -161,8 +158,14 @@ udiSlDctMM/X3ZM2JN5M1rtAJ2WR3ZQtmWbOjZAbG2Eq
 		t.Run(name, func(t *testing.T) {
 			c := Cloner{}
 			err := c.CloneRepo(test.opts)
-			if !cmp.Equal(err, test.expectedErr, errorComparer) {
-				t.Fatalf("err expected to be %v, got %v", test.expectedErr, err)
+			if err == nil && test.expectedErr != nil {
+				t.Fatalf("err expected to be [%v], got [%v]", test.expectedErr, err)
+			} else if test.expectedErr != nil && err == nil {
+				t.Fatalf("err expected to be [%v], got [%v]", test.expectedErr, err)
+			} else if test.expectedErr != nil && err != nil {
+				if !cmp.Equal(test.expectedErr.Error(), err.Error()) {
+					t.Fatalf("err expected to be [%s], got [%s]", test.expectedErr.Error(), err.Error())
+				}
 			}
 
 			if pathCalled != test.opts.Path {
diff --git a/internal/cmd/controller/gitops/reconciler/gitjob_test.go b/internal/cmd/controller/gitops/reconciler/gitjob_test.go
index 71d745c9..e4841590 100644
--- a/internal/cmd/controller/gitops/reconciler/gitjob_test.go
+++ b/internal/cmd/controller/gitops/reconciler/gitjob_test.go
@@ -711,9 +711,9 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 			expectedInitContainers: []corev1.Container{
 				{
 					Command: []string{
-						"fleet",
+						"log.sh",
 					},
-					Args:  []string{"gitcloner", "repo", "/workspace", "--branch", "master"},
+					Args:  []string{"fleet", "gitcloner", "repo", "/workspace", "--branch", "master"},
 					Image: "test",
 					Name:  "gitcloner-initializer",
 					VolumeMounts: []corev1.VolumeMount{
@@ -754,9 +754,9 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 			expectedInitContainers: []corev1.Container{
 				{
 					Command: []string{
-						"fleet",
+						"log.sh",
 					},
-					Args:  []string{"gitcloner", "repo", "/workspace", "--branch", "foo"},
+					Args:  []string{"fleet", "gitcloner", "repo", "/workspace", "--branch", "foo"},
 					Image: "test",
 					Name:  "gitcloner-initializer",
 					VolumeMounts: []corev1.VolumeMount{
@@ -797,9 +797,9 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 			expectedInitContainers: []corev1.Container{
 				{
 					Command: []string{
-						"fleet",
+						"log.sh",
 					},
-					Args:  []string{"gitcloner", "repo", "/workspace", "--revision", "foo"},
+					Args:  []string{"fleet", "gitcloner", "repo", "/workspace", "--revision", "foo"},
 					Image: "test",
 					Name:  "gitcloner-initializer",
 					VolumeMounts: []corev1.VolumeMount{
@@ -840,9 +840,10 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 			expectedInitContainers: []corev1.Container{
 				{
 					Command: []string{
-						"fleet",
+						"log.sh",
 					},
 					Args: []string{
+						"fleet",
 						"gitcloner",
 						"repo",
 						"/workspace",
@@ -915,9 +916,10 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 			expectedInitContainers: []corev1.Container{
 				{
 					Command: []string{
-						"fleet",
+						"log.sh",
 					},
 					Args: []string{
+						"fleet",
 						"gitcloner",
 						"repo",
 						"/workspace",
@@ -991,9 +993,10 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 			expectedInitContainers: []corev1.Container{
 				{
 					Command: []string{
-						"fleet",
+						"log.sh",
 					},
 					Args: []string{
+						"fleet",
 						"gitcloner",
 						"repo",
 						"/workspace",
@@ -1069,9 +1072,10 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 			expectedInitContainers: []corev1.Container{
 				{
 					Command: []string{
-						"fleet",
+						"log.sh",
 					},
 					Args: []string{
+						"fleet",
 						"gitcloner",
 						"repo",
 						"/workspace",
@@ -1181,9 +1185,10 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 			expectedInitContainers: []corev1.Container{
 				{
 					Command: []string{
-						"fleet",
+						"log.sh",
 					},
 					Args: []string{
+						"fleet",
 						"gitcloner",
 						"repo",
 						"/workspace",
@@ -1228,9 +1233,9 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 			expectedInitContainers: []corev1.Container{
 				{
 					Command: []string{
-						"fleet",
+						"log.sh",
 					},
-					Args:  []string{"gitcloner", "repo", "/workspace", "--branch", "master"},
+					Args:  []string{"fleet", "gitcloner", "repo", "/workspace", "--branch", "master"},
 					Image: "test",
 					Name:  "gitcloner-initializer",
 					VolumeMounts: []corev1.VolumeMount{