Repository: rancher/fleet
Pull request #3559 (2025-04-16 13:08:40)

fleet cli json output
Uses a JSONFormatter in fleet cli.
It also filters out the output received from fleet cli so the error set in the conditions is only the one sent by the fleet cli code and not from any used libraries.

In order to use the json formatter we need to set the `FLEET_JSON_OUTPUT` env var to `true`. This way the command line user won't get the json output unless it's explicitly specified.

Difference in the UI

Before the change:
![Screenshot 2025-04-15 at 16 10 54](https://github.com/user-attachments/assets/ae3512f2-8a30-4716-8772-0029b1390177)

After the change:
![Screenshot 2025-04-15 at 15 37 49](https://github.com/user-attachments/assets/9e1b9d17-72e3-4c87-9abc-c62ff30f9ec1)

 
Refers to: https://github.com/rancher/fleet/issues/3234



Patch:
diff --git a/cmd/fleetcli/main.go b/cmd/fleetcli/main.go
index 2200f872..3b54cfe6 100644
--- a/cmd/fleetcli/main.go
+++ b/cmd/fleetcli/main.go
@@ -2,6 +2,9 @@
 package main
 
 import (
+	"os"
+	"strings"
+
 	// Ensure GVKs are registered
 	_ "github.com/rancher/fleet/pkg/generated/controllers/fleet.cattle.io"
 	_ "github.com/rancher/wrangler/v3/pkg/generated/controllers/apiextensions.k8s.io"
@@ -22,7 +25,16 @@ func main() {
 	ctx := signals.SetupSignalContext()
 	cmd := cmds.App()
 	if err := cmd.ExecuteContext(ctx); err != nil {
-		logrus.Fatal(err)
+		if strings.ToLower(os.Getenv(cmds.JSONOutputEnvVar)) == "true" {
+			log := logrus.New()
+			log.SetFormatter(&logrus.JSONFormatter{})
+			// use a fleet specific field name so we are sure logs from other libraries
+			// are not considered.
+			log.WithFields(logrus.Fields{
+				"fleetErrorMessage": err.Error(),
+			}).Fatal("Fleet cli failed")
+		} else {
+			logrus.Fatal(err)
+		}
 	}
-
 }
diff --git a/internal/cmd/cli/root.go b/internal/cmd/cli/root.go
index 3c9bb4ea..050e94b5 100644
--- a/internal/cmd/cli/root.go
+++ b/internal/cmd/cli/root.go
@@ -19,6 +19,8 @@ var (
 	Client Getter
 )
 
+const JSONOutputEnvVar = "FLEET_JSON_OUTPUT"
+
 func App() *cobra.Command {
 	root := command.Command(&Fleet{}, cobra.Command{
 		Version:       version.FriendlyVersion(),
diff --git a/internal/cmd/controller/gitops/reconciler/gitjob.go b/internal/cmd/controller/gitops/reconciler/gitjob.go
index 1b868b6f..f40fb206 100644
--- a/internal/cmd/controller/gitops/reconciler/gitjob.go
+++ b/internal/cmd/controller/gitops/reconciler/gitjob.go
@@ -552,7 +552,13 @@ func (r *GitJobReconciler) newGitCloner(
 		args = append(args, "--ca-bundle-file", "/gitjob/cabundle/"+bundleCAFile)
 	}
 
-	env := proxyEnvVars()
+	env := []corev1.EnvVar{
+		{
+			Name:  fleetcli.JSONOutputEnvVar,
+			Value: "true",
+		},
+	}
+	env = append(env, proxyEnvVars()...)
 
 	// If strict host key checks are enabled but no entries are available, another error will be shown by the known
 	// hosts getter, as that means that the Fleet deployment is incomplete.
@@ -639,6 +645,10 @@ func argsAndEnvs(
 			Name:  "HOME",
 			Value: fleetHomeDir,
 		},
+		{
+			Name:  fleetcli.JSONOutputEnvVar,
+			Value: "true",
+		},
 		{
 			Name:  fleetcli.FleetApplyConflictRetriesEnv,
 			Value: strconv.Itoa(fleetApplyRetries),
diff --git a/internal/cmd/controller/gitops/reconciler/gitjob_controller.go b/internal/cmd/controller/gitops/reconciler/gitjob_controller.go
index 7a65331f..a538b715 100644
--- a/internal/cmd/controller/gitops/reconciler/gitjob_controller.go
+++ b/internal/cmd/controller/gitops/reconciler/gitjob_controller.go
@@ -2,6 +2,7 @@ package reconciler
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"math/rand/v2"
 	"sort"
@@ -562,7 +563,7 @@ func setStatusFromGitjob(ctx context.Context, c client.Client, gitRepo *v1alpha1
 	//   - Terminating
 	switch result.Status {
 	case status.FailedStatus:
-		kstatus.SetError(gitRepo, terminationMessage)
+		kstatus.SetError(gitRepo, filterFleetCLIJobOutput(terminationMessage))
 	case status.CurrentStatus:
 		if strings.Contains(result.Message, "Job Completed") {
 			gitRepo.Status.Commit = job.Annotations["commit"]
@@ -654,3 +655,59 @@ func updateStatus(ctx context.Context, c client.Client, req types.NamespacedName
 		return nil
 	})
 }
+
+func filterFleetCLIJobOutput(output string) string {
+	// first split the output in lines
+	lines := strings.Split(output, "\n")
+	s := ""
+	for _, l := range lines {
+		s = s + getFleetCLIErrorsFromLine(l)
+	}
+
+	s = strings.Trim(s, "\n")
+	// in the case that all the messages from fleet apply are from libraries
+	// we just report an unknown error
+	if s == "" {
+		s = "Unknown error"
+	}
+
+	return s
+}
+
+func getFleetCLIErrorsFromLine(l string) string {
+	type LogEntry struct {
+		Level         string `json:"level"`
+		FleetErrorMsg string `json:"fleetErrorMessage"`
+		Time          string `json:"time"`
+		Msg           string `json:"msg"`
+	}
+	s := ""
+	open := strings.IndexByte(l, '{')
+	if open == -1 {
+		// line does not contain a valid json string
+		return ""
+	}
+	close := strings.IndexByte(l, '}')
+	if close != -1 {
+		if close < open {
+			// looks like there is some garbage before a possible json string
+			// ignore everything up to that closing bracked and try again
+			return getFleetCLIErrorsFromLine(l[close+1:])
+		}
+	} else if close == -1 {
+		// line does not contain a valid json string
+		return ""
+	}
+	var entry LogEntry
+	if err := json.Unmarshal([]byte(l[open:close+1]), &entry); err == nil {
+		if entry.FleetErrorMsg != "" {
+			s = s + entry.FleetErrorMsg + "\n"
+		}
+	}
+	// check if there's more to parse
+	if close+1 < len(l) {
+		s = s + getFleetCLIErrorsFromLine(l[close+1:])
+	}
+
+	return s
+}


Test patch:
diff --git a/integrationtests/gitjob/controller/controller_test.go b/integrationtests/gitjob/controller/controller_test.go
index d18ac926..1816eabd 100644
--- a/integrationtests/gitjob/controller/controller_test.go
+++ b/integrationtests/gitjob/controller/controller_test.go
@@ -542,10 +542,15 @@ var _ = Describe("GitJob controller", func() {
 					job.Status.StartTime = &metav1.Time{Time: time.Now().Add(-1 * time.Hour)}
 					job.Status.Conditions = []batchv1.JobCondition{
 						{
-							Type:    "Failed",
+							// using Stalled because the Compute function uses Stalled
+							// for returning the condition message and it's simpler.
+							// For testing it in a different way we would need to setup a more complex
+							// scenario defining the job pods
+							// We are simulating job failures.
+							Type:    "Stalled",
 							Status:  "True",
 							Reason:  "BackoffLimitExceeded",
-							Message: "Job has reached the specified backoff limit",
+							Message: `{"fleetErrorMessage":"fleet error message","level":"fatal","msg":"Fleet cli failed","time":"2025-04-15T14:53:15+02:00"}`,
 						},
 						{
 							Type:   batchv1.JobFailureTarget,
@@ -566,7 +571,7 @@ var _ = Describe("GitJob controller", func() {
 					// check the conditions related to the job
 					// Failed.... Stalled=true and Reconcilling=false
 					g.Expect(checkCondition(&gitRepo, "Reconciling", corev1.ConditionFalse, "")).To(BeTrue())
-					g.Expect(checkCondition(&gitRepo, "Stalled", corev1.ConditionTrue, "Job Failed. failed: 1/1")).To(BeTrue())
+					g.Expect(checkCondition(&gitRepo, "Stalled", corev1.ConditionTrue, "fleet error message")).To(BeTrue())
 
 					// check the rest
 					g.Expect(checkCondition(&gitRepo, "GitPolling", corev1.ConditionTrue, "")).To(BeTrue())
diff --git a/internal/cmd/controller/gitops/reconciler/gitjob_test.go b/internal/cmd/controller/gitops/reconciler/gitjob_test.go
index 40c5b2a2..ccb46556 100644
--- a/internal/cmd/controller/gitops/reconciler/gitjob_test.go
+++ b/internal/cmd/controller/gitops/reconciler/gitjob_test.go
@@ -12,14 +12,14 @@ import (
 	"time"
 
 	"github.com/google/go-cmp/cmp"
-	"go.uber.org/mock/gomock"
-
+	fleetcli "github.com/rancher/fleet/internal/cmd/cli"
 	"github.com/rancher/fleet/internal/cmd/controller/finalize"
 	"github.com/rancher/fleet/internal/config"
 	"github.com/rancher/fleet/internal/mocks"
 	"github.com/rancher/fleet/internal/ssh"
 	fleetv1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
 	"github.com/rancher/wrangler/v3/pkg/genericcondition"
+	"go.uber.org/mock/gomock"
 
 	fleetevent "github.com/rancher/fleet/pkg/event"
 	gitmocks "github.com/rancher/fleet/pkg/git/mocks"
@@ -741,6 +741,12 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 						},
 					},
 					SecurityContext: securityContext,
+					Env: []corev1.EnvVar{
+						{
+							Name:  fleetcli.JSONOutputEnvVar,
+							Value: "true",
+						},
+					},
 				},
 			},
 			expectedVolumes: []corev1.Volume{
@@ -808,6 +814,12 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 						},
 					},
 					SecurityContext: securityContext,
+					Env: []corev1.EnvVar{
+						{
+							Name:  fleetcli.JSONOutputEnvVar,
+							Value: "true",
+						},
+					},
 				},
 			},
 			expectedVolumes: []corev1.Volume{
@@ -875,6 +887,12 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 						},
 					},
 					SecurityContext: securityContext,
+					Env: []corev1.EnvVar{
+						{
+							Name:  fleetcli.JSONOutputEnvVar,
+							Value: "true",
+						},
+					},
 				},
 			},
 			expectedVolumes: []corev1.Volume{
@@ -950,6 +968,12 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 						},
 					},
 					SecurityContext: securityContext,
+					Env: []corev1.EnvVar{
+						{
+							Name:  fleetcli.JSONOutputEnvVar,
+							Value: "true",
+						},
+					},
 				},
 			},
 			expectedVolumes: []corev1.Volume{
@@ -1043,6 +1067,12 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 						},
 					},
 					SecurityContext: securityContext,
+					Env: []corev1.EnvVar{
+						{
+							Name:  fleetcli.JSONOutputEnvVar,
+							Value: "true",
+						},
+					},
 				},
 			},
 			expectedVolumes: []corev1.Volume{
@@ -1105,6 +1135,10 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 						"/gitjob/ssh/" + corev1.SSHAuthPrivateKey,
 					},
 					Env: []corev1.EnvVar{
+						{
+							Name:  fleetcli.JSONOutputEnvVar,
+							Value: "true",
+						},
 						{
 							Name:  "FLEET_KNOWN_HOSTS",
 							Value: "some known hosts",
@@ -1193,6 +1227,10 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 						"/gitjob/ssh/" + corev1.SSHAuthPrivateKey,
 					},
 					Env: []corev1.EnvVar{
+						{
+							Name:  fleetcli.JSONOutputEnvVar,
+							Value: "true",
+						},
 						{
 							Name:  "FLEET_KNOWN_HOSTS",
 							Value: "foo",
@@ -1286,6 +1324,10 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 						"master",
 					},
 					Env: []corev1.EnvVar{
+						{
+							Name:  fleetcli.JSONOutputEnvVar,
+							Value: "true",
+						},
 						{
 							Name:  "FLEET_KNOWN_HOSTS",
 							Value: "foo",
@@ -1375,6 +1417,12 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 						},
 					},
 					SecurityContext: securityContext,
+					Env: []corev1.EnvVar{
+						{
+							Name:  fleetcli.JSONOutputEnvVar,
+							Value: "true",
+						},
+					},
 				},
 			},
 			expectedVolumes: []corev1.Volume{
@@ -1465,6 +1513,12 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 						},
 					},
 					SecurityContext: securityContext,
+					Env: []corev1.EnvVar{
+						{
+							Name:  fleetcli.JSONOutputEnvVar,
+							Value: "true",
+						},
+					},
 				},
 			},
 			expectedVolumes: []corev1.Volume{
@@ -1588,6 +1642,12 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 						},
 					},
 					SecurityContext: securityContext,
+					Env: []corev1.EnvVar{
+						{
+							Name:  fleetcli.JSONOutputEnvVar,
+							Value: "true",
+						},
+					},
 				},
 			},
 			expectedVolumes: []corev1.Volume{
@@ -1652,6 +1712,12 @@ func TestNewJob(t *testing.T) { // nolint:funlen
 						},
 					},
 					SecurityContext: securityContext,
+					Env: []corev1.EnvVar{
+						{
+							Name:  fleetcli.JSONOutputEnvVar,
+							Value: "true",
+						},
+					},
 				},
 			},
 			expectedVolumes: []corev1.Volume{
@@ -1835,6 +1901,10 @@ func TestGenerateJob_EnvVars(t *testing.T) {
 					Name:  "HOME",
 					Value: "/fleet-home",
 				},
+				{
+					Name:  fleetcli.JSONOutputEnvVar,
+					Value: "true",
+				},
 				{
 					Name:  "FLEET_APPLY_CONFLICT_RETRIES",
 					Value: "1",
@@ -1860,6 +1930,12 @@ func TestGenerateJob_EnvVars(t *testing.T) {
 					Value: "commit",
 				},
 			},
+			expectedInitContainerEnvVars: []corev1.EnvVar{
+				{
+					Name:  fleetcli.JSONOutputEnvVar,
+					Value: "true",
+				},
+			},
 		},
 		"Helm secret name with strict host key checks": {
 			gitrepo: &fleetv1.GitRepo{
@@ -1872,6 +1948,10 @@ func TestGenerateJob_EnvVars(t *testing.T) {
 			},
 			strictSSHHostKeyChecks: true,
 			expectedInitContainerEnvVars: []corev1.EnvVar{
+				{
+					Name:  fleetcli.JSONOutputEnvVar,
+					Value: "true",
+				},
 				{
 					Name: "FLEET_KNOWN_HOSTS",
 				},
@@ -1881,6 +1961,10 @@ func TestGenerateJob_EnvVars(t *testing.T) {
 					Name:  "HOME",
 					Value: "/fleet-home",
 				},
+				{
+					Name:  fleetcli.JSONOutputEnvVar,
+					Value: "true",
+				},
 				{
 					Name:  "FLEET_APPLY_CONFLICT_RETRIES",
 					Value: "1",
@@ -1921,6 +2005,10 @@ func TestGenerateJob_EnvVars(t *testing.T) {
 					Name:  "HOME",
 					Value: "/fleet-home",
 				},
+				{
+					Name:  fleetcli.JSONOutputEnvVar,
+					Value: "true",
+				},
 				{
 					Name:  "FLEET_APPLY_CONFLICT_RETRIES",
 					Value: "1",
@@ -1934,6 +2022,12 @@ func TestGenerateJob_EnvVars(t *testing.T) {
 					Value: "commit",
 				},
 			},
+			expectedInitContainerEnvVars: []corev1.EnvVar{
+				{
+					Name:  fleetcli.JSONOutputEnvVar,
+					Value: "true",
+				},
+			},
 		},
 		"Helm secret name for paths with strict host key checks": {
 			gitrepo: &fleetv1.GitRepo{
@@ -1946,6 +2040,10 @@ func TestGenerateJob_EnvVars(t *testing.T) {
 			},
 			strictSSHHostKeyChecks: true,
 			expectedInitContainerEnvVars: []corev1.EnvVar{
+				{
+					Name:  fleetcli.JSONOutputEnvVar,
+					Value: "true",
+				},
 				{
 					Name: "FLEET_KNOWN_HOSTS",
 				},
@@ -1955,6 +2053,10 @@ func TestGenerateJob_EnvVars(t *testing.T) {
 					Name:  "HOME",
 					Value: "/fleet-home",
 				},
+				{
+					Name:  fleetcli.JSONOutputEnvVar,
+					Value: "true",
+				},
 				{
 					Name:  "FLEET_APPLY_CONFLICT_RETRIES",
 					Value: "1",
@@ -1981,6 +2083,10 @@ func TestGenerateJob_EnvVars(t *testing.T) {
 					Name:  "HOME",
 					Value: "/fleet-home",
 				},
+				{
+					Name:  fleetcli.JSONOutputEnvVar,
+					Value: "true",
+				},
 				{
 					Name:  "FLEET_APPLY_CONFLICT_RETRIES",
 					Value: "1",
@@ -1999,6 +2105,10 @@ func TestGenerateJob_EnvVars(t *testing.T) {
 				},
 			},
 			expectedInitContainerEnvVars: []corev1.EnvVar{
+				{
+					Name:  fleetcli.JSONOutputEnvVar,
+					Value: "true",
+				},
 				{
 					Name:  "HTTP_PROXY",
 					Value: "httpProxy",
@@ -2022,6 +2132,10 @@ func TestGenerateJob_EnvVars(t *testing.T) {
 					Name:  "HOME",
 					Value: "/fleet-home",
 				},
+				{
+					Name:  fleetcli.JSONOutputEnvVar,
+					Value: "true",
+				},
 				{
 					Name:  "FLEET_APPLY_CONFLICT_RETRIES",
 					Value: "3",
@@ -2032,6 +2146,12 @@ func TestGenerateJob_EnvVars(t *testing.T) {
 				},
 			},
 			osEnv: map[string]string{"FLEET_APPLY_CONFLICT_RETRIES": "3"},
+			expectedInitContainerEnvVars: []corev1.EnvVar{
+				{
+					Name:  fleetcli.JSONOutputEnvVar,
+					Value: "true",
+				},
+			},
 		},
 		"retries_not_valid": {
 			gitrepo: &fleetv1.GitRepo{
@@ -2045,6 +2165,10 @@ func TestGenerateJob_EnvVars(t *testing.T) {
 					Name:  "HOME",
 					Value: "/fleet-home",
 				},
+				{
+					Name:  fleetcli.JSONOutputEnvVar,
+					Value: "true",
+				},
 				{
 					Name:  "FLEET_APPLY_CONFLICT_RETRIES",
 					Value: "1",
@@ -2055,6 +2179,12 @@ func TestGenerateJob_EnvVars(t *testing.T) {
 				},
 			},
 			osEnv: map[string]string{"FLEET_APPLY_CONFLICT_RETRIES": "this_is_not_an_int"},
+			expectedInitContainerEnvVars: []corev1.EnvVar{
+				{
+					Name:  fleetcli.JSONOutputEnvVar,
+					Value: "true",
+				},
+			},
 		},
 	}
 
@@ -2444,6 +2574,86 @@ func TestDrivenScanSeparator(t *testing.T) {
 	}
 }
 
+func TestFilterFleetApplyJobOutput(t *testing.T) {
+	tests := map[string]struct {
+		input          string
+		expectedOutput string
+	}{
+		"Filter a few lines and return a couple from Fleet": {
+			input: `this line should be ignored
+		this line should be ignored, too
+		{"level":"fatal","msg":"This line is not from fleet cli","time":"2025-04-15T14:53:15+02:00"}
+		{"fleetErrorMessage":"fleet line 1","level":"fatal","msg":"Fleet cli failed","time":"2025-04-15T14:53:15+02:00"}
+		ignore this line as well
+		{"fleetErrorMessage":"fleet line 2","level":"fatal","msg":"Fleet cli failed","time":"2025-04-15T14:53:15+02:00"}`,
+			expectedOutput: "fleet line 1\nfleet line 2",
+		},
+		"There are no lines from fleet apply": {
+			input: `this line should be ignored
+		this line should be ignored, too
+		{"level":"fatal","msg":"This line is not from fleet cli","time":"2025-04-15T14:53:15+02:00"}
+		ignore this line as well`,
+			expectedOutput: "Unknown error",
+		},
+		"The output is from fleet apply, but it's not in json format": {
+			input:          "FATA[0000] no resource found at the following paths to deploy: [tt]",
+			expectedOutput: "Unknown error",
+		},
+		"Valid message with some extra text before and after": {
+			input: `this line should be ignored
+		this line should be ignored, too
+		This line is OK{"fleetErrorMessage":"fleet line error 1","level":"fatal","msg":"Fleet cli failed","time":"2025-04-15T14:53:15+02:00"}This part should be ignored
+		ignore this line as well`,
+			expectedOutput: "fleet line error 1",
+		},
+		"Valid message with some extra text before": {
+			input: `this line should be ignored
+		this line should be ignored, too
+		This line is OK{"fleetErrorMessage":"fleet line error 1","level":"fatal","msg":"Fleet cli failed","time":"2025-04-15T14:53:15+02:00"}
+		ignore this line as well`,
+			expectedOutput: "fleet line error 1",
+		},
+		"Valid message with some extra text after": {
+			input: `this line should be ignored
+		this line should be ignored, too
+		This line is OK{"fleetErrorMessage":"fleet line error 1","level":"fatal","msg":"Fleet cli failed","time":"2025-04-15T14:53:15+02:00"}
+		ignore this line as well`,
+			expectedOutput: "fleet line error 1",
+		},
+		"Not valid json message": {
+			input: `this line should be ignored
+		this line should be ignored, too
+		This lin}e is OK "{fleetErrorMessage":"fleet line error 1","level":"fatal","msg":"Fleet cli failed","time":"2025-04-15T14:53:15+02:00"}
+		ignore this line as well`,
+			expectedOutput: "Unknown error",
+		},
+		"More than one error in the same line": {
+			input: `this line should be ignored
+this line should be ignored, too
+T}his{ lin}e is OK "{"fleetErrorMessage":"fleet line error 1","level":"fatal","msg":"Fleet cli failed","time":"2025-04-15T14:53:15+02:00"}more garbage{"fleetErrorMessage":"fleet line error 2","level":"fatal","msg":"Fleet cli failed","time":"2025-04-15T14:53:15+02:00"}some more noise
+ignore this line as well`,
+			expectedOutput: "fleet line error 1\nfleet line error 2",
+		},
+		"Empty input": {
+			input:          "",
+			expectedOutput: "Unknown error",
+		},
+
+		"The fleetErrorMessage field is empty": {
+			input:          `{"fleetErrorMessage":"","level":"fatal","msg":"Fleet cli failed","time":"2025-04-15T14:53:15+02:00"}`,
+			expectedOutput: "Unknown error",
+		},
+	}
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			filteredMsg := filterFleetCLIJobOutput(test.input)
+			if filteredMsg != test.expectedOutput {
+				t.Errorf("expecting output %q, got %q", test.expectedOutput, filteredMsg)
+			}
+		})
+	}
+}
+
 func getFakeClient(tolerations []corev1.Toleration, objs ...runtime.Object) client.Client {
 	scheme := runtime.NewScheme()
 	utilruntime.Must(corev1.AddToScheme(scheme))