fix: applicationset controller should respect logging flags (#10513)

* Align logging setup with other controllers Signed-off-by: toVersus <toversus2357@gmail.com> Signed-off-by: toVersus <toversus2357@gmail.com>
argoproj · Oct 7, 2022 · f47a5f9 · f47a5f9
1 parent faa01bb
commit f47a5f9
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 32 deletions.
diff --git a/applicationset/controllers/applicationset_controller.go b/applicationset/controllers/applicationset_controller.go
@@ -19,7 +19,6 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/go-logr/logr"
 	log "github.com/sirupsen/logrus"
 	corev1 "k8s.io/api/core/v1"
 	apierr "k8s.io/apimachinery/pkg/api/errors"
@@ -62,7 +61,6 @@ var (
 // ApplicationSetReconciler reconciles a ApplicationSet object
 type ApplicationSetReconciler struct {
 	client.Client
-	Log              logr.Logger
 	Scheme           *runtime.Scheme
 	Recorder         record.EventRecorder
 	Generators       map[string]generators.Generator
@@ -77,15 +75,14 @@ type ApplicationSetReconciler struct {
 // +kubebuilder:rbac:groups=argoproj.io,resources=applicationsets/status,verbs=get;update;patch
 
 func (r *ApplicationSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-	_ = r.Log.WithValues("applicationset", req.NamespacedName)
-	_ = log.WithField("applicationset", req.NamespacedName)
+	logCtx := log.WithField("applicationset", req.NamespacedName)
 
 	var applicationSetInfo argov1alpha1.ApplicationSet
 	parametersGenerated := false
 
 	if err := r.Get(ctx, req.NamespacedName, &applicationSetInfo); err != nil {
 		if client.IgnoreNotFound(err) != nil {
-			log.WithError(err).Infof("unable to get ApplicationSet: '%v' ", err)
+			logCtx.WithError(err).Infof("unable to get ApplicationSet: '%v' ", err)
 		}
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
@@ -123,7 +120,7 @@ func (r *ApplicationSetReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		//
 		// Changes to watched resources will cause this to be reconciled sooner than
 		// the RequeueAfter time.
-		log.Errorf("error occurred during application validation: %s", err.Error())
+		logCtx.Errorf("error occurred during application validation: %s", err.Error())
 
 		_ = r.setApplicationSetStatusCondition(ctx,
 			&applicationSetInfo,
@@ -148,7 +145,7 @@ func (r *ApplicationSetReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		var message string
 		for _, v := range validateErrors {
 			message = v.Error()
-			log.Errorf("validation error found during application validation: %s", message)
+			logCtx.Errorf("validation error found during application validation: %s", message)
 		}
 		if len(validateErrors) > 1 {
 			// Only the last message gets added to the appset status, to keep the size reasonable.
@@ -215,7 +212,7 @@ func (r *ApplicationSetReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		delete(applicationSetInfo.Annotations, common.AnnotationApplicationSetRefresh)
 		err := r.Client.Update(ctx, &applicationSetInfo)
 		if err != nil {
-			log.Warnf("error occurred while updating ApplicationSet: %v", err)
+			logCtx.Warnf("error occurred while updating ApplicationSet: %v", err)
 			_ = r.setApplicationSetStatusCondition(ctx,
 				&applicationSetInfo,
 				argov1alpha1.ApplicationSetCondition{
@@ -230,7 +227,7 @@ func (r *ApplicationSetReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	}
 
 	requeueAfter := r.getMinRequeueAfter(&applicationSetInfo)
-	log.WithField("requeueAfter", requeueAfter).Info("end reconcile")
+	logCtx.WithField("requeueAfter", requeueAfter).Info("end reconcile")
 
 	if len(validateErrors) == 0 {
 		if err := r.setApplicationSetStatusCondition(ctx,

diff --git a/applicationset/controllers/applicationset_controller_test.go b/applicationset/controllers/applicationset_controller_test.go
@@ -1832,7 +1832,6 @@ func TestReconcilerValidationErrorBehaviour(t *testing.T) {
 	}}, nil)
 
 	r := ApplicationSetReconciler{
-		Log:      ctrl.Log.WithName("controllers").WithName("ApplicationSet"),
 		Client:   client,
 		Scheme:   scheme,
 		Renderer: &utils.Render{},
@@ -1908,7 +1907,6 @@ func TestSetApplicationSetStatusCondition(t *testing.T) {
 	client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&appSet).Build()
 
 	r := ApplicationSetReconciler{
-		Log:      ctrl.Log.WithName("controllers").WithName("ApplicationSet"),
 		Client:   client,
 		Scheme:   scheme,
 		Renderer: &utils.Render{},

diff --git a/cmd/argocd-applicationset-controller/commands/applicationset_controller.go b/cmd/argocd-applicationset-controller/commands/applicationset_controller.go
@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"net/http"
 	"os"
-	"strings"
 	"time"
 
 	"github.com/argoproj/pkg/stats"
@@ -16,6 +15,7 @@ import (
 	"github.com/argoproj/argo-cd/v2/applicationset/generators"
 	"github.com/argoproj/argo-cd/v2/applicationset/utils"
 	"github.com/argoproj/argo-cd/v2/applicationset/webhook"
+	cmdutil "github.com/argoproj/argo-cd/v2/cmd/util"
 	"github.com/argoproj/argo-cd/v2/common"
 	"github.com/argoproj/argo-cd/v2/reposerver/askpass"
 	"github.com/argoproj/argo-cd/v2/util/env"
@@ -56,8 +56,6 @@ func NewCommand() *cobra.Command {
 		policy               string
 		debugLog             bool
 		dryRun               bool
-		logFormat            string
-		logLevel             string
 	)
 	scheme := runtime.NewScheme()
 	_ = clientgoscheme.AddToScheme(scheme)
@@ -79,28 +77,16 @@ func NewCommand() *cobra.Command {
 				},
 			)
 
+			cli.SetLogFormat(cmdutil.LogFormat)
+			cli.SetLogLevel(cmdutil.LogLevel)
+
 			restConfig, err := clientConfig.ClientConfig()
 			if err != nil {
 				return err
 			}
 
 			restConfig.UserAgent = fmt.Sprintf("argocd-applicationset-controller/%s (%s)", vers.Version, vers.Platform)
 
-			level, err := log.ParseLevel(logLevel)
-			if err != nil {
-				return err
-			}
-			log.SetLevel(level)
-			switch strings.ToLower(logFormat) {
-			case "json":
-				log.SetFormatter(&log.JSONFormatter{})
-			case "text":
-				if os.Getenv("FORCE_LOG_COLORS") == "1" {
-					log.SetFormatter(&log.TextFormatter{ForceColors: true})
-				}
-			default:
-				return fmt.Errorf("Unknown log format '%s'", logFormat)
-			}
 			policyObj, exists := utils.Policies[policy]
 			if !exists {
 				log.Info("Policy value can be: sync, create-only, create-update")
@@ -184,7 +170,6 @@ func NewCommand() *cobra.Command {
 			if err = (&controllers.ApplicationSetReconciler{
 				Generators:       topLevelGenerators,
 				Client:           mgr.GetClient(),
-				Log:              ctrl.Log.WithName("controllers").WithName("ApplicationSet"),
 				Scheme:           mgr.GetScheme(),
 				Recorder:         mgr.GetEventRecorderFor("applicationset-controller"),
 				Renderer:         &utils.Render{},
@@ -217,9 +202,9 @@ func NewCommand() *cobra.Command {
 	command.Flags().StringVar(&argocdRepoServer, "argocd-repo-server", "argocd-repo-server:8081", "Argo CD repo server address")
 	command.Flags().StringVar(&policy, "policy", "sync", "Modify how application is synced between the generator and the cluster. Default is 'sync' (create & update & delete), options: 'create-only', 'create-update' (no deletion)")
 	command.Flags().BoolVar(&debugLog, "debug", false, "Print debug logs. Takes precedence over loglevel")
-	command.Flags().StringVar(&logLevel, "loglevel", "info", "Set the logging level. One of: debug|info|warn|error")
+	command.Flags().StringVar(&cmdutil.LogFormat, "logformat", "text", "Set the logging format. One of: text|json")
+	command.Flags().StringVar(&cmdutil.LogLevel, "loglevel", "info", "Set the logging level. One of: debug|info|warn|error")
 	command.Flags().BoolVar(&dryRun, "dry-run", false, "Enable dry run mode")
-	command.Flags().StringVar(&logFormat, "logformat", "text", "Set the logging format. One of: text|json")
 	return &command
 }