From c76caffcd59d1623827437165c3fafd51b352a5e Mon Sep 17 00:00:00 2001
From: Roy Paulin Nguetsop <roy.paulin@microfocus.com>
Date: Thu, 20 Jan 2022 16:16:31 +0100
Subject: [PATCH 1/4] Make logging more consumable

---
 cmd/operator/main.go                          | 114 ++++++++++++++++--
 config/default/manager_auth_proxy_patch.yaml  |   7 ++
 go.mod                                        |   4 +-
 go.sum                                        |  15 +--
 helm-charts/verticadb-operator/values.yaml    |  21 ++++
 scripts/create-helm-charts.sh                 |   9 ++
 scripts/create-patch.sh                       |  42 +++++++
 .../10-deploy-operator.yaml                   |   2 +-
 .../e2e/custom-cert-webhook/11-add-patch.yaml |  22 ++++
 tests/e2e/custom-cert-webhook/11-assert.yaml  |  19 +++
 .../12-check-log-file.yaml                    |  20 +++
 11 files changed, 252 insertions(+), 23 deletions(-)
 create mode 100755 scripts/create-patch.sh
 create mode 100644 tests/e2e/custom-cert-webhook/11-add-patch.yaml
 create mode 100644 tests/e2e/custom-cert-webhook/11-assert.yaml
 create mode 100644 tests/e2e/custom-cert-webhook/12-check-log-file.yaml

diff --git a/cmd/operator/main.go b/cmd/operator/main.go
index 14a0d2009..2022c54f6 100644
--- a/cmd/operator/main.go
+++ b/cmd/operator/main.go
@@ -18,6 +18,7 @@ package main
 import (
 	"flag"
 	"fmt"
+	"log"
 	"os"
 	"strconv"
 
@@ -26,25 +27,45 @@ import (
 	"net/http"
 	_ "net/http/pprof" // nolint:gosec
 
-	_ "k8s.io/client-go/plugin/pkg/client/auth"
-
+	"github.com/go-logr/zapr"
+	"go.uber.org/zap"
+	"go.uber.org/zap/zapcore"
+	lumberjack "gopkg.in/natefinch/lumberjack.v2"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+	_ "k8s.io/client-go/plugin/pkg/client/auth"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
-	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 
 	verticacomv1beta1 "github.com/vertica/vertica-kubernetes/api/v1beta1"
 	"github.com/vertica/vertica-kubernetes/pkg/controllers"
 	//+kubebuilder:scaffold:imports
 )
 
+const (
+	MaxFileSize     = 500
+	MaxFileAge      = 7
+	MaxFileRotation = 3
+	Level           = "info"
+	DevMode         = true
+	Stdout          = false
+)
+
 var (
 	scheme   = runtime.NewScheme()
 	setupLog = ctrl.Log.WithName("setup")
 )
 
+type Logging struct {
+	FilePath        string
+	Level           string
+	MaxFileSize     int
+	MaxFileAge      int
+	MaxFileRotation int
+	Stdout          bool
+}
+
 func init() {
 	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
 
@@ -81,11 +102,75 @@ func getIsWebhookEnabled() bool {
 	return enabled
 }
 
+// getEncoderConfig returns a concrete encoders configuration
+func getEncoderConfig(devMode bool) zapcore.EncoderConfig {
+	encoderConfig := zap.NewProductionEncoderConfig()
+	if devMode {
+		encoderConfig = zap.NewDevelopmentEncoderConfig()
+	}
+	return encoderConfig
+}
+
+// getLogWriter returns an io.writer (setting up rolling files) converted
+// into a zapcore.WriteSyncer
+func getLogWriter(path string, age, size, rotation int) zapcore.WriteSyncer {
+	lumberJackLogger := &lumberjack.Logger{
+		Filename:   path,
+		MaxSize:    size, // megabytes
+		MaxBackups: rotation,
+		MaxAge:     age, // days
+	}
+	return zapcore.AddSync(lumberJackLogger)
+}
+
+// getZapcoreLevel takes the level as string and returns the corresponding
+// zapcore.Level. If the string level is invalid, it returns the default
+// level
+func getZapcoreLevel(lvl string) zapcore.Level {
+	lvls := map[string]zapcore.Level{
+		"debug": zapcore.DebugLevel,
+		"info":  zapcore.InfoLevel,
+		"warn":  zapcore.WarnLevel,
+		"error": zapcore.ErrorLevel,
+	}
+	if _, found := lvls[lvl]; !found {
+		// returns default level if the input level is invalid
+		log.Println(fmt.Sprintf("Invalid level, %s level will be used instead", Level))
+		return lvls[Level]
+	}
+	return lvls[lvl]
+}
+
+// getLogger is a wrapper that calls other functions
+// to build a logger.
+func getLogger(logArgs Logging, devMode bool) *zap.Logger {
+	encoderConfig := getEncoderConfig(devMode)
+	writes := []zapcore.WriteSyncer{}
+	lvl := zap.NewAtomicLevelAt(zap.DebugLevel)
+	if logArgs.FilePath != "" {
+		w := getLogWriter(logArgs.FilePath, logArgs.MaxFileAge, logArgs.MaxFileSize, logArgs.MaxFileRotation)
+		lvl = zap.NewAtomicLevelAt(getZapcoreLevel(logArgs.Level))
+		writes = append(writes, w)
+	}
+	if logArgs.FilePath == "" || logArgs.Stdout {
+		writes = append(writes, zapcore.AddSync(os.Stdout))
+	}
+	core := zapcore.NewCore(
+		zapcore.NewConsoleEncoder(encoderConfig),
+		zapcore.NewMultiWriteSyncer(writes...),
+		lvl,
+	)
+	return zap.New(core)
+}
+
+// nolint:funlen
 func main() {
 	var metricsAddr string
 	var enableLeaderElection bool
 	var probeAddr string
 	var enableProfiler bool
+	var devMode bool
+	logArgs := Logging{}
 	flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
 	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
 	flag.BoolVar(&enableLeaderElection, "leader-elect", false,
@@ -94,13 +179,26 @@ func main() {
 	flag.BoolVar(&enableProfiler, "enable-profiler", false,
 		"Enables runtime profiling collection.  The profiling data can be inspected by connecting to port 6060 "+
 			"with the path /debug/pprof.  See https://golang.org/pkg/net/http/pprof/ for more info.")
-	opts := zap.Options{
-		Development: true,
-	}
-	opts.BindFlags(flag.CommandLine)
+	flag.StringVar(&logArgs.FilePath, "filepath", "", "The file logging will write to.")
+	flag.IntVar(&logArgs.MaxFileSize, "maxfilesize", MaxFileSize,
+		"The maximum size in megabytes of the log file "+
+			"before it gets rotated.")
+	flag.IntVar(&logArgs.MaxFileAge, "maxfileage", MaxFileAge,
+		"This is the maximum age, in days, of the logging "+
+			"before log rotation gets rid of it.")
+	flag.IntVar(&logArgs.MaxFileRotation, "maxfilerotation", MaxFileRotation,
+		"this is the maximum number of files that are kept in rotation before the old ones are removed.")
+	flag.StringVar(&logArgs.Level, "level", Level, "The minimum logging level.  Valid values are: debug, info, warn, and error.")
+	flag.BoolVar(&devMode, "dev", DevMode, "Enables development mode if true and production mode otherwise.")
+	flag.BoolVar(&logArgs.Stdout, "stdout", Stdout, "Enables logging to stdout.")
 	flag.Parse()
 
-	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))
+	logger := getLogger(logArgs, devMode)
+	if logArgs.FilePath != "" {
+		log.Println(fmt.Sprintf("Now logging in file %s", logArgs.FilePath))
+	}
+
+	ctrl.SetLogger(zapr.NewLogger(logger))
 
 	if enableProfiler {
 		go func() {
diff --git a/config/default/manager_auth_proxy_patch.yaml b/config/default/manager_auth_proxy_patch.yaml
index a224be19e..8946897ea 100644
--- a/config/default/manager_auth_proxy_patch.yaml
+++ b/config/default/manager_auth_proxy_patch.yaml
@@ -24,3 +24,10 @@ spec:
         - "--health-probe-bind-address=:8081"
         - "--metrics-bind-address=127.0.0.1:8080"
         - "--leader-elect"
+        - "--filepath="
+        - "--maxfilesize="
+        - "--maxfileage="
+        - "--maxfilerotation="
+        - "--level="
+        - "--dev="
+        - "--stdout="
\ No newline at end of file
diff --git a/go.mod b/go.mod
index 0288f3f44..57dc03fc3 100644
--- a/go.mod
+++ b/go.mod
@@ -7,11 +7,13 @@ require (
 	github.com/bigkevmcd/go-configparser v0.0.0-20210106142102-909504547ead
 	github.com/ghodss/yaml v1.0.0
 	github.com/go-logr/logr v0.3.0
+	github.com/go-logr/zapr v0.2.0 // indirect
 	github.com/lithammer/dedent v1.1.0
-	github.com/mogensen/kubernetes-split-yaml v0.3.0 // indirect
 	github.com/onsi/ginkgo v1.14.1
 	github.com/onsi/gomega v1.10.2
 	github.com/vertica/vertica-sql-go v1.1.1
+	go.uber.org/zap v1.15.0
+	gopkg.in/natefinch/lumberjack.v2 v2.0.0
 	k8s.io/api v0.19.2
 	k8s.io/apimachinery v0.19.2
 	k8s.io/client-go v0.19.2
diff --git a/go.sum b/go.sum
index b1d9cbb98..9921c6410 100644
--- a/go.sum
+++ b/go.sum
@@ -78,8 +78,6 @@ github.com/coreos/go-systemd v0.0.0-20180511133405-39ca1b05acc7/go.mod h1:F5haX7
 github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4=
 github.com/coreos/pkg v0.0.0-20160727233714-3ac0863d7acf/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA=
 github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA=
-github.com/cpuguy83/go-md2man/v2 v2.0.0-20190314233015-f79a8a8ca69d/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU=
-github.com/cpuguy83/go-md2man/v2 v2.0.0 h1:EoUDS0afbrsXAZ9YQ9jdu/mZ2sXgT1/2yyNng4PGlyM=
 github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU=
 github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
@@ -95,6 +93,7 @@ github.com/docker/spdystream v0.0.0-20160310174837-449fdfce4d96/go.mod h1:Qh8CwZ
 github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3ebgob9U8Nd0kOddGdZWjyMGR8Wziv+TBNwSE=
 github.com/dustin/go-humanize v0.0.0-20171111073723-bb3d318650d4/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
 github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
+github.com/elazarl/goproxy v0.0.0-20180725130230-947c36da3153 h1:yUdfgN0XgIJw7foRItutHYUIhlcKzcSf5vDpdhQAKTc=
 github.com/elazarl/goproxy v0.0.0-20180725130230-947c36da3153/go.mod h1:/Zj4wYkgs4iZTTu3o/KG3Itv/qCCa8VVMlb3i9OVuzc=
 github.com/emicklei/go-restful v0.0.0-20170410110728-ff4f55a20633/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs=
 github.com/emicklei/go-restful v2.9.5+incompatible/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs=
@@ -247,8 +246,6 @@ github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvW
 github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQLJ+jE2L00=
 github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
 github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
-github.com/konsorten/go-windows-terminal-sequences v1.0.2/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
-github.com/konsorten/go-windows-terminal-sequences v1.0.3 h1:CE8S1cTafDpPvMhIxNJKvHsGVBgn1xWYf1NbHQhywc8=
 github.com/konsorten/go-windows-terminal-sequences v1.0.3/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
 github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc=
 github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
@@ -282,8 +279,6 @@ github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJ
 github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
 github.com/modern-go/reflect2 v1.0.1 h1:9f412s+6RmYXLWZSEzVVgPGK7C2PphHj5RJrvfx9AWI=
 github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
-github.com/mogensen/kubernetes-split-yaml v0.3.0 h1:Phj6jF9bcLpXlTUepRvopCtwl6oKPtnISn1LyQBGTfE=
-github.com/mogensen/kubernetes-split-yaml v0.3.0/go.mod h1:oRr9IK7d6ID2o/w7CnVlJSz1EZNZyqiJbxnZreCZNXI=
 github.com/munnerz/goautoneg v0.0.0-20120707110453-a547fc61f48d/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
 github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
@@ -337,15 +332,11 @@ github.com/prometheus/procfs v0.1.3/go.mod h1:lV6e/gmhEcM9IjHGsFOCxxuZ+z1YqCvr4O
 github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU=
 github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg=
 github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
-github.com/russross/blackfriday/v2 v2.0.1 h1:lPqVAte+HuHNfhJ/0LC98ESWRz8afy9tM/0RK8m9o+Q=
 github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
 github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
-github.com/shurcooL/sanitized_anchor_name v1.0.0 h1:PdmoCO6wvbs+7yrJyMORt4/BmY5IYyJwS/kOiWx8mHo=
 github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc=
 github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo=
 github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE=
-github.com/sirupsen/logrus v1.5.0/go.mod h1:+F7Ogzej0PZc/94MaYx/nvG9jOFMD2osvC3s+Squfpo=
-github.com/sirupsen/logrus v1.6.0 h1:UBcNElsrwanuuMsnGSlYmtmgbb23qDR5dG+6X6Oo89I=
 github.com/sirupsen/logrus v1.6.0/go.mod h1:7uNnSEd1DgxDLC74fIahvMZmmYsHGZGEOFrfsX/uA88=
 github.com/soheilhy/cmux v0.1.4/go.mod h1:IM3LyeVVIOuxMH7sFAkER9+bJ4dT7Ms6E4xg4kGIyLM=
 github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA=
@@ -374,10 +365,7 @@ github.com/tidwall/pretty v1.0.0/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhV
 github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
 github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc=
-github.com/urfave/cli v1.20.0 h1:fDqGv3UG/4jbVl/QkFwEdddtEDjh/5Ov6X+0B/3bPaw=
 github.com/urfave/cli v1.20.0/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijbERA=
-github.com/urfave/cli/v2 v2.2.0 h1:JTTnM6wKzdA0Jqodd966MVj4vWbbquZykeX1sKbe2C4=
-github.com/urfave/cli/v2 v2.2.0/go.mod h1:SE9GqnLQmjVa0iPEY0f1w3ygNIYcIJ0OKPMoW2caLfQ=
 github.com/vektah/gqlparser v1.1.2/go.mod h1:1ycwN7Ij5njmMkPPAOaRFY4rET2Enx7IkVv3vaXspKw=
 github.com/vertica/vertica-sql-go v1.1.1 h1:sZYijzBbvdAbJcl4cYlKjR+Eh/X1hGKzukWuhh8PjvI=
 github.com/vertica/vertica-sql-go v1.1.1/go.mod h1:fGr44VWdEvL+f+Qt5LkKLOT7GoxaWdoUCnPBU9h6t04=
@@ -605,6 +593,7 @@ gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI=
 gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys=
 gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc=
 gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw=
+gopkg.in/natefinch/lumberjack.v2 v2.0.0 h1:1Lc07Kr7qY4U2YPouBjpCLxpiyxIVoxqXgkXLknAOE8=
 gopkg.in/natefinch/lumberjack.v2 v2.0.0/go.mod h1:l0ndWWf7gzL7RNwBG7wST/UCcT4T24xpD6X8LsfU/+k=
 gopkg.in/resty.v1 v1.12.0/go.mod h1:mDo4pnntr5jdWRML875a/NmxYqAlA73dVijT2AXvQQo=
 gopkg.in/square/go-jose.v2 v2.2.2/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI=
diff --git a/helm-charts/verticadb-operator/values.yaml b/helm-charts/verticadb-operator/values.yaml
index 4c657e830..5813ba48f 100644
--- a/helm-charts/verticadb-operator/values.yaml
+++ b/helm-charts/verticadb-operator/values.yaml
@@ -37,6 +37,27 @@ webhook:
   # apiserver are used.
   caBundle: ""
 
+logging:
+  # filePath, when specified, is the file logging will write to.
+  # When empty, logging will write to standard out.
+  filePath: ""
+  # maxFileSize, when logging to a file, is the maximum size, 
+  # in MB, of the logging file before log rotation occurs.
+  maxFileSize: 500
+  # maxFileAge, when logging to a file, is the maximum age, in days,
+  # of the logging before log rotation gets rid of it.
+  maxFileAge: 7
+  # maxFileRotation, when logging to a file, is the maximum number of files 
+  # that are kept in rotation before the old ones are removed.
+  maxFileRotation: 3
+  # level is the minimum logging level. Valid values are: debug, info, warn, and error
+  level: info
+  # stdOut is used to enforce logging to stdout.
+  stdOut: false
+
+# devEnables development mode if true and production mode otherwise
+dev: true
+
 # The resource requirements for the operator pod.  See this for more info:
 # https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
 # These defaults must be kept in sync with config/manifests/kustomization.yaml
diff --git a/scripts/create-helm-charts.sh b/scripts/create-helm-charts.sh
index c74567b0e..02a8ee601 100755
--- a/scripts/create-helm-charts.sh
+++ b/scripts/create-helm-charts.sh
@@ -49,6 +49,15 @@ done
 # 5. Template the resource limits and requests
 sed -i 's/resources: template-placeholder/resources:\n          {{- toYaml .Values.resources | nindent 10 }}/' $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
 
+# 6.  Template the logging
+sed -i "s/--filepath=/--filepath={{ .Values.logging.filePath }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
+sed -i "s/--maxfilesize=/--maxfilesize={{ .Values.logging.maxFileSize }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
+sed -i "s/--maxfileage=/--maxfileage={{ .Values.logging.maxFileAge }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
+sed -i "s/--maxfilerotation=/--maxfilerotation={{ .Values.logging.maxFileRotation }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
+sed -i "s/--level=/--level={{ .Values.logging.level }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
+sed -i "s/--dev=/--dev={{ .Values.dev }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
+sed -i "s/--stdout=/--stdout={{ .Values.logging.stdOut }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
+
 # Delete openshift clusterRole and clusterRoleBinding files
 rm $TEMPLATE_DIR/verticadb-operator-openshift-cluster-role-cr.yaml 
 rm $TEMPLATE_DIR/verticadb-operator-openshift-cluster-rolebinding-crb.yaml
\ No newline at end of file
diff --git a/scripts/create-patch.sh b/scripts/create-patch.sh
new file mode 100755
index 000000000..0da4ade9d
--- /dev/null
+++ b/scripts/create-patch.sh
@@ -0,0 +1,42 @@
+#!/bin/bash
+
+# (c) Copyright [2021] Micro Focus or one of its affiliates.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# You may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# A script that will be used to test logging to file.
+
+cat <<EOF > test-logging-patch.yaml
+spec:
+  template:
+    spec:
+      containers:
+      - name: manager
+        volumeMounts:
+        - mountPath: /logs
+          name: shared-data
+      - args:
+        - -c
+        - while true; do tail -n 1 /var/logs/try.log; sleep 5;done
+        command:
+        - /bin/sh
+        image: busybox
+        name: sidecar-container
+        securityContext:
+          runAsUser: 65532
+        volumeMounts:
+        - mountPath: /var/logs
+          name: shared-data
+      volumes:
+      - emptyDir: {}
+        name: shared-data
+EOF
\ No newline at end of file
diff --git a/tests/e2e/custom-cert-webhook/10-deploy-operator.yaml b/tests/e2e/custom-cert-webhook/10-deploy-operator.yaml
index c886fe9d2..b4350f5fa 100644
--- a/tests/e2e/custom-cert-webhook/10-deploy-operator.yaml
+++ b/tests/e2e/custom-cert-webhook/10-deploy-operator.yaml
@@ -18,5 +18,5 @@ commands:
       cd ../../.. && TF=$(mktemp /tmp/cabundle.pem.XXXXXX) \
        && echo $TF \
        && kubectl get secrets -n $NAMESPACE custom-cert -o json | jq -r '.data."ca.crt"' | tee $TF \
-       && make deploy-operator DEPLOY_WITH=helm NAMESPACE=$NAMESPACE HELM_OVERRIDES="--set webhook.tlsSecret=custom-cert --set webhook.caBundle=$(cat $TF)" \
+       && make deploy-operator DEPLOY_WITH=helm NAMESPACE=$NAMESPACE HELM_OVERRIDES="--set webhook.tlsSecret=custom-cert --set webhook.caBundle=$(cat $TF) --set logging.filePath=/logs/try.log" \
        && rm $TF
diff --git a/tests/e2e/custom-cert-webhook/11-add-patch.yaml b/tests/e2e/custom-cert-webhook/11-add-patch.yaml
new file mode 100644
index 000000000..a3601d5f5
--- /dev/null
+++ b/tests/e2e/custom-cert-webhook/11-add-patch.yaml
@@ -0,0 +1,22 @@
+# (c) Copyright [2021] Micro Focus or one of its affiliates.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# You may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+  - script: |
+      PN=$(kubectl get pods -n $NAMESPACE --no-headers  -o custom-columns=":metadata.name" | grep verticadb ) \
+      && ../../../scripts/create-patch.sh \
+      && kubectl patch deployment verticadb-operator-controller-manager -n $NAMESPACE --patch "$(cat test-logging-patch.yaml)" \
+      && kubectl wait --for=delete pod/$PN -n $NAMESPACE --timeout=180s 
+      rm test-logging-patch.yaml
diff --git a/tests/e2e/custom-cert-webhook/11-assert.yaml b/tests/e2e/custom-cert-webhook/11-assert.yaml
new file mode 100644
index 000000000..85312102c
--- /dev/null
+++ b/tests/e2e/custom-cert-webhook/11-assert.yaml
@@ -0,0 +1,19 @@
+# (c) Copyright [2021] Micro Focus or one of its affiliates.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# You may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: verticadb-operator-controller-manager
+status:
+  readyReplicas: 1
\ No newline at end of file
diff --git a/tests/e2e/custom-cert-webhook/12-check-log-file.yaml b/tests/e2e/custom-cert-webhook/12-check-log-file.yaml
new file mode 100644
index 000000000..bd1c595f9
--- /dev/null
+++ b/tests/e2e/custom-cert-webhook/12-check-log-file.yaml
@@ -0,0 +1,20 @@
+# (c) Copyright [2021] Micro Focus or one of its affiliates.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# You may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+  - script: |
+      PN=$(kubectl get pods -n $NAMESPACE --no-headers -o custom-columns=":metadata.name" | grep verticadb ) \
+      && kubectl wait --for=condition=Ready=true pod/$PN -n $NAMESPACE \
+      && kubectl exec $PN -n $NAMESPACE -c sidecar-container -- tail -n 1  var/logs/try.log

From 177c08a0d4bc9c5a5b28d61d4f9a3ce256d69a33 Mon Sep 17 00:00:00 2001
From: Roy Paulin Nguetsop <roy.paulin@microfocus.com>
Date: Fri, 21 Jan 2022 21:15:58 +0100
Subject: [PATCH 2/4] Apply 1st review comments

---
 Makefile                                      |   5 +-
 cmd/operator/main.go                          | 126 ++++++++++--------
 config/default/manager_auth_proxy_patch.yaml  |  16 ++-
 helm-charts/verticadb-operator/README.md      |   6 +
 helm-charts/verticadb-operator/values.yaml    |  12 +-
 scripts/create-helm-charts.sh                 |  11 +-
 .../e2e/custom-cert-webhook/11-add-patch.yaml |   4 +-
 .../12-check-log-file.yaml                    |   2 +-
 .../test-logging-patch.yaml                   |  16 +--
 9 files changed, 111 insertions(+), 87 deletions(-)
 rename scripts/create-patch.sh => tests/e2e/custom-cert-webhook/test-logging-patch.yaml (76%)
 mode change 100755 => 100644

diff --git a/Makefile b/Makefile
index 71c77a34a..322d0de62 100644
--- a/Makefile
+++ b/Makefile
@@ -115,6 +115,9 @@ HELM_RELEASE_NAME?=vdb-op
 # For example to specify a custom webhook tls cert when deploying use this command:
 #   HELM_OVERRIDES="--set webhook.tlsSecret=custom-cert" make deploy-operator
 HELM_OVERRIDES?=
+# Enables development mode by default. Is used only when the operator is deployed
+# through the Makefile 
+DEV_MODE?=true
 # Maximum number of tests to run at once. (default 2)
 # Set it to any value not greater than 8 to override the default one
 E2E_PARALLELISM?=2
@@ -354,7 +357,7 @@ uninstall: manifests kustomize ## Uninstall CRDs from the K8s cluster specified
 
 deploy-operator: manifests kustomize ## Using helm or olm, deploy the operator in the K8s cluster
 ifeq ($(DEPLOY_WITH), helm)
-	helm install --wait -n $(NAMESPACE) $(HELM_RELEASE_NAME) $(OPERATOR_CHART) --set image.name=${OPERATOR_IMG} $(HELM_OVERRIDES)
+	helm install --wait -n $(NAMESPACE) $(HELM_RELEASE_NAME) $(OPERATOR_CHART) --set image.name=${OPERATOR_IMG} --set logging.dev=${DEV_MODE} $(HELM_OVERRIDES)
 	scripts/wait-for-webhook.sh -n $(NAMESPACE) -t 60
 else ifeq ($(DEPLOY_WITH), olm)
 	scripts/deploy-olm.sh -n $(NAMESPACE) $(OLM_TEST_CATALOG_SOURCE)
diff --git a/cmd/operator/main.go b/cmd/operator/main.go
index 2022c54f6..fc7a623f5 100644
--- a/cmd/operator/main.go
+++ b/cmd/operator/main.go
@@ -44,12 +44,12 @@ import (
 )
 
 const (
-	MaxFileSize     = 500
-	MaxFileAge      = 7
-	MaxFileRotation = 3
-	Level           = "info"
-	DevMode         = true
-	Stdout          = false
+	DefaultMaxFileSize     = 500
+	DefaultMaxFileAge      = 7
+	DefaultMaxFileRotation = 3
+	DefaultLevel           = "info"
+	DefaultDevMode         = true
+	DefaultZapcoreLevel    = zapcore.InfoLevel
 )
 
 var (
@@ -57,13 +57,21 @@ var (
 	setupLog = ctrl.Log.WithName("setup")
 )
 
+type FlagConfig struct {
+	MetricsAddr          string
+	EnableLeaderElection bool
+	ProbeAddr            string
+	EnableProfiler       bool
+	LogArgs              *Logging
+}
+
 type Logging struct {
 	FilePath        string
 	Level           string
 	MaxFileSize     int
 	MaxFileAge      int
 	MaxFileRotation int
-	Stdout          bool
+	DevMode         bool
 }
 
 func init() {
@@ -73,6 +81,35 @@ func init() {
 	//+kubebuilder:scaffold:scheme
 }
 
+// setLoggingFlagArgs define logging flags with specified names and default values
+func (l *Logging) setLoggingFlagArgs() {
+	flag.StringVar(&l.FilePath, "filepath", "", "The file logging will write to.")
+	flag.IntVar(&l.MaxFileSize, "maxfilesize", DefaultMaxFileSize,
+		"The maximum size in megabytes of the log file "+
+			"before it gets rotated.")
+	flag.IntVar(&l.MaxFileAge, "maxfileage", DefaultMaxFileAge,
+		"This is the maximum age, in days, of the logging "+
+			"before log rotation gets rid of it.")
+	flag.IntVar(&l.MaxFileRotation, "maxfilerotation", DefaultMaxFileRotation,
+		"this is the maximum number of files that are kept in rotation before the old ones are removed.")
+	flag.StringVar(&l.Level, "level", DefaultLevel, "The minimum logging level.  Valid values are: debug, info, warn, and error.")
+	flag.BoolVar(&l.DevMode, "dev", DefaultDevMode, "Enables development mode if true and production mode otherwise.")
+}
+
+// setFlagArgs define flags with specified names and default values
+func (fc *FlagConfig) setFlagArgs() {
+	flag.StringVar(&fc.MetricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
+	flag.StringVar(&fc.ProbeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
+	flag.BoolVar(&fc.EnableLeaderElection, "leader-elect", false,
+		"Enable leader election for controller manager. "+
+			"Enabling this will ensure there is only one active controller manager.")
+	flag.BoolVar(&fc.EnableProfiler, "enable-profiler", false,
+		"Enables runtime profiling collection.  The profiling data can be inspected by connecting to port 6060 "+
+			"with the path /debug/pprof.  See https://golang.org/pkg/net/http/pprof/ for more info.")
+	fc.LogArgs = &Logging{}
+	fc.LogArgs.setLoggingFlagArgs()
+}
+
 // getWatchNamespace returns the Namespace the operator should be watching for changes
 func getWatchNamespace() (string, error) {
 	// WatchNamespaceEnvVar is the constant for env variable WATCH_NAMESPACE
@@ -113,12 +150,12 @@ func getEncoderConfig(devMode bool) zapcore.EncoderConfig {
 
 // getLogWriter returns an io.writer (setting up rolling files) converted
 // into a zapcore.WriteSyncer
-func getLogWriter(path string, age, size, rotation int) zapcore.WriteSyncer {
+func getLogWriter(logArgs Logging) zapcore.WriteSyncer {
 	lumberJackLogger := &lumberjack.Logger{
-		Filename:   path,
-		MaxSize:    size, // megabytes
-		MaxBackups: rotation,
-		MaxAge:     age, // days
+		Filename:   logArgs.FilePath,
+		MaxSize:    logArgs.MaxFileSize, // megabytes
+		MaxBackups: logArgs.MaxFileRotation,
+		MaxAge:     logArgs.MaxFileAge, // days
 	}
 	return zapcore.AddSync(lumberJackLogger)
 }
@@ -127,32 +164,26 @@ func getLogWriter(path string, age, size, rotation int) zapcore.WriteSyncer {
 // zapcore.Level. If the string level is invalid, it returns the default
 // level
 func getZapcoreLevel(lvl string) zapcore.Level {
-	lvls := map[string]zapcore.Level{
-		"debug": zapcore.DebugLevel,
-		"info":  zapcore.InfoLevel,
-		"warn":  zapcore.WarnLevel,
-		"error": zapcore.ErrorLevel,
-	}
-	if _, found := lvls[lvl]; !found {
-		// returns default level if the input level is invalid
-		log.Println(fmt.Sprintf("Invalid level, %s level will be used instead", Level))
-		return lvls[Level]
+	var level = new(zapcore.Level)
+	err := level.UnmarshalText([]byte(lvl))
+	if err != nil {
+		log.Println(fmt.Sprintf("unrecognized level, %s level will be used instead", DefaultLevel))
+		return DefaultZapcoreLevel
 	}
-	return lvls[lvl]
+	return *level
 }
 
 // getLogger is a wrapper that calls other functions
 // to build a logger.
-func getLogger(logArgs Logging, devMode bool) *zap.Logger {
-	encoderConfig := getEncoderConfig(devMode)
+func getLogger(logArgs Logging) *zap.Logger {
+	encoderConfig := getEncoderConfig(logArgs.DevMode)
 	writes := []zapcore.WriteSyncer{}
-	lvl := zap.NewAtomicLevelAt(zap.DebugLevel)
+	lvl := zap.NewAtomicLevelAt(getZapcoreLevel(logArgs.Level))
 	if logArgs.FilePath != "" {
-		w := getLogWriter(logArgs.FilePath, logArgs.MaxFileAge, logArgs.MaxFileSize, logArgs.MaxFileRotation)
-		lvl = zap.NewAtomicLevelAt(getZapcoreLevel(logArgs.Level))
+		w := getLogWriter(logArgs)
 		writes = append(writes, w)
 	}
-	if logArgs.FilePath == "" || logArgs.Stdout {
+	if logArgs.FilePath == "" || logArgs.DevMode {
 		writes = append(writes, zapcore.AddSync(os.Stdout))
 	}
 	core := zapcore.NewCore(
@@ -163,37 +194,18 @@ func getLogger(logArgs Logging, devMode bool) *zap.Logger {
 	return zap.New(core)
 }
 
-// nolint:funlen
 func main() {
-	var metricsAddr string
-	var enableLeaderElection bool
-	var probeAddr string
-	var enableProfiler bool
-	var devMode bool
-	logArgs := Logging{}
-	flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
-	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
-	flag.BoolVar(&enableLeaderElection, "leader-elect", false,
-		"Enable leader election for controller manager. "+
-			"Enabling this will ensure there is only one active controller manager.")
-	flag.BoolVar(&enableProfiler, "enable-profiler", false,
-		"Enables runtime profiling collection.  The profiling data can be inspected by connecting to port 6060 "+
-			"with the path /debug/pprof.  See https://golang.org/pkg/net/http/pprof/ for more info.")
-	flag.StringVar(&logArgs.FilePath, "filepath", "", "The file logging will write to.")
-	flag.IntVar(&logArgs.MaxFileSize, "maxfilesize", MaxFileSize,
-		"The maximum size in megabytes of the log file "+
-			"before it gets rotated.")
-	flag.IntVar(&logArgs.MaxFileAge, "maxfileage", MaxFileAge,
-		"This is the maximum age, in days, of the logging "+
-			"before log rotation gets rid of it.")
-	flag.IntVar(&logArgs.MaxFileRotation, "maxfilerotation", MaxFileRotation,
-		"this is the maximum number of files that are kept in rotation before the old ones are removed.")
-	flag.StringVar(&logArgs.Level, "level", Level, "The minimum logging level.  Valid values are: debug, info, warn, and error.")
-	flag.BoolVar(&devMode, "dev", DevMode, "Enables development mode if true and production mode otherwise.")
-	flag.BoolVar(&logArgs.Stdout, "stdout", Stdout, "Enables logging to stdout.")
+	flagArgs := &FlagConfig{}
+	flagArgs.setFlagArgs()
 	flag.Parse()
 
-	logger := getLogger(logArgs, devMode)
+	metricsAddr := flagArgs.MetricsAddr
+	enableLeaderElection := flagArgs.EnableLeaderElection
+	probeAddr := flagArgs.ProbeAddr
+	enableProfiler := flagArgs.EnableProfiler
+	logArgs := flagArgs.LogArgs
+
+	logger := getLogger(*logArgs)
 	if logArgs.FilePath != "" {
 		log.Println(fmt.Sprintf("Now logging in file %s", logArgs.FilePath))
 	}
diff --git a/config/default/manager_auth_proxy_patch.yaml b/config/default/manager_auth_proxy_patch.yaml
index 8946897ea..619ff83de 100644
--- a/config/default/manager_auth_proxy_patch.yaml
+++ b/config/default/manager_auth_proxy_patch.yaml
@@ -24,10 +24,14 @@ spec:
         - "--health-probe-bind-address=:8081"
         - "--metrics-bind-address=127.0.0.1:8080"
         - "--leader-elect"
+        # These are the placeholders that we patch in when applying the helm chart.
+        # The default values here will be replaced in helm templates by the values 
+        # of parameters. When creating the bundle instead for olm these flags with
+        # these default values will be put in the CSV and as filepath is not set, there
+        # will be no logging to file with olm as expected.
         - "--filepath="
-        - "--maxfilesize="
-        - "--maxfileage="
-        - "--maxfilerotation="
-        - "--level="
-        - "--dev="
-        - "--stdout="
\ No newline at end of file
+        - "--maxfilesize=500"
+        - "--maxfileage=7"
+        - "--maxfilerotation=3"
+        - "--level=info"
+        - "--dev=false"
diff --git a/helm-charts/verticadb-operator/README.md b/helm-charts/verticadb-operator/README.md
index 4ba64b62c..05ff0fca3 100644
--- a/helm-charts/verticadb-operator/README.md
+++ b/helm-charts/verticadb-operator/README.md
@@ -5,5 +5,11 @@ This helm chart will install the operator and an admission controller webhook.
 | image.name | The name of image that runs the operator. | vertica/verticadb-operator:1.0.0 |
 | webhook.caBundle | A PEM encoded CA bundle that will be used to validate the webhook's server certificate.  If unspecified, system trust roots on the apiserver are used. | |
 | webhook.tlsSecret | The webhook requires a TLS certficate to work.  By default we rely on cert-manager to be installed as we use it generate the cert.  If you don't want to use cert-manager, you need to specify your own cert, which you can do with this parameter.  When set, it is a name of a secret in the same namespace the chart is being installed in.  The secret must have the keys: tls.key, ca.crt, and tls.crt. | |
+| logging.filePath | When specified, it is the file logging will write to. When empty, logging will write to standard out.  | |
+| logging.maxFileSize | When logging to a file, it is the maximum size, in MB, of the logging file before log rotation occurs. | 500 |
+| logging.maxFileAge | When logging to a file, it is the maximum age, in days, of the logging before log rotation gets rid of it. | 7 |
+| logging.maxFileRotation | When logging to a file, it is the maximum number of files that are kept in rotation before the old ones are removed. | 3 |
+| logging.level | It is the minimum logging level. Valid values are: debug, info, warn, and error | info |
+| logging.dev | Enables development mode if true and production mode otherwise. | false |
 | resources.\* | The resource requirements for the operator pod. | <pre>limits:<br>  cpu: 100m<br>  memory: 750Mi<br>requests:<br>  cpu: 100m<br>  memory: 20Mi</pre> |
 
diff --git a/helm-charts/verticadb-operator/values.yaml b/helm-charts/verticadb-operator/values.yaml
index 5813ba48f..3805fcca7 100644
--- a/helm-charts/verticadb-operator/values.yaml
+++ b/helm-charts/verticadb-operator/values.yaml
@@ -52,11 +52,15 @@ logging:
   maxFileRotation: 3
   # level is the minimum logging level. Valid values are: debug, info, warn, and error
   level: info
-  # stdOut is used to enforce logging to stdout.
-  stdOut: false
+  # dev Enables development mode if true and production mode otherwise and also affects
+  # logs format. Afew differences on logging will be: in dev mode the the level is displayed
+  # in capital letters, the time is in a ISO8601-formatted string, in prod, the level is in
+  # lower case, the time is in epoch time format.
+  # example:
+  # dev ->  2022-01-21T12:50:10.078Z        INFO    controller-runtime.certwatcher  Starting certificate watcher
+  # prod -> 1.6427683530752437e+09  info    controller-runtime.certwatcher  Starting certificate watcher
+  dev: false
 
-# devEnables development mode if true and production mode otherwise
-dev: true
 
 # The resource requirements for the operator pod.  See this for more info:
 # https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
diff --git a/scripts/create-helm-charts.sh b/scripts/create-helm-charts.sh
index 02a8ee601..50e0eac01 100755
--- a/scripts/create-helm-charts.sh
+++ b/scripts/create-helm-charts.sh
@@ -51,12 +51,11 @@ sed -i 's/resources: template-placeholder/resources:\n          {{- toYaml .Valu
 
 # 6.  Template the logging
 sed -i "s/--filepath=/--filepath={{ .Values.logging.filePath }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
-sed -i "s/--maxfilesize=/--maxfilesize={{ .Values.logging.maxFileSize }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
-sed -i "s/--maxfileage=/--maxfileage={{ .Values.logging.maxFileAge }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
-sed -i "s/--maxfilerotation=/--maxfilerotation={{ .Values.logging.maxFileRotation }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
-sed -i "s/--level=/--level={{ .Values.logging.level }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
-sed -i "s/--dev=/--dev={{ .Values.dev }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
-sed -i "s/--stdout=/--stdout={{ .Values.logging.stdOut }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
+sed -i "s/--maxfilesize=500/--maxfilesize={{ .Values.logging.maxFileSize }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
+sed -i "s/--maxfileage=7/--maxfileage={{ .Values.logging.maxFileAge }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
+sed -i "s/--maxfilerotation=3/--maxfilerotation={{ .Values.logging.maxFileRotation }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
+sed -i "s/--level=info/--level={{ .Values.logging.level }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
+sed -i "s/--dev=false/--dev={{ .Values.logging.dev }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
 
 # Delete openshift clusterRole and clusterRoleBinding files
 rm $TEMPLATE_DIR/verticadb-operator-openshift-cluster-role-cr.yaml 
diff --git a/tests/e2e/custom-cert-webhook/11-add-patch.yaml b/tests/e2e/custom-cert-webhook/11-add-patch.yaml
index a3601d5f5..684752685 100644
--- a/tests/e2e/custom-cert-webhook/11-add-patch.yaml
+++ b/tests/e2e/custom-cert-webhook/11-add-patch.yaml
@@ -16,7 +16,5 @@ kind: TestStep
 commands:
   - script: |
       PN=$(kubectl get pods -n $NAMESPACE --no-headers  -o custom-columns=":metadata.name" | grep verticadb ) \
-      && ../../../scripts/create-patch.sh \
       && kubectl patch deployment verticadb-operator-controller-manager -n $NAMESPACE --patch "$(cat test-logging-patch.yaml)" \
-      && kubectl wait --for=delete pod/$PN -n $NAMESPACE --timeout=180s 
-      rm test-logging-patch.yaml
+      && kubectl wait --for=delete pod/$PN -n $NAMESPACE --timeout=180s
diff --git a/tests/e2e/custom-cert-webhook/12-check-log-file.yaml b/tests/e2e/custom-cert-webhook/12-check-log-file.yaml
index bd1c595f9..7b8a1e0f2 100644
--- a/tests/e2e/custom-cert-webhook/12-check-log-file.yaml
+++ b/tests/e2e/custom-cert-webhook/12-check-log-file.yaml
@@ -17,4 +17,4 @@ commands:
   - script: |
       PN=$(kubectl get pods -n $NAMESPACE --no-headers -o custom-columns=":metadata.name" | grep verticadb ) \
       && kubectl wait --for=condition=Ready=true pod/$PN -n $NAMESPACE \
-      && kubectl exec $PN -n $NAMESPACE -c sidecar-container -- tail -n 1  var/logs/try.log
+      && kubectl exec $PN -n $NAMESPACE -c sidecar-container -- tail -n 1  /logs/try.log
diff --git a/scripts/create-patch.sh b/tests/e2e/custom-cert-webhook/test-logging-patch.yaml
old mode 100755
new mode 100644
similarity index 76%
rename from scripts/create-patch.sh
rename to tests/e2e/custom-cert-webhook/test-logging-patch.yaml
index 0da4ade9d..cc01874f2
--- a/scripts/create-patch.sh
+++ b/tests/e2e/custom-cert-webhook/test-logging-patch.yaml
@@ -1,5 +1,3 @@
-#!/bin/bash
-
 # (c) Copyright [2021] Micro Focus or one of its affiliates.
 # Licensed under the Apache License, Version 2.0 (the "License");
 # You may not use this file except in compliance with the License.
@@ -13,9 +11,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# A script that will be used to test logging to file.
+# This is used to test logging to file. manager will log
+# to the shared volume and the sidecar will be used to
+# verify that.
 
-cat <<EOF > test-logging-patch.yaml
 spec:
   template:
     spec:
@@ -26,17 +25,16 @@ spec:
           name: shared-data
       - args:
         - -c
-        - while true; do tail -n 1 /var/logs/try.log; sleep 5;done
+        - while true; do tail -n 1 /logs/try.log; sleep 5;done
         command:
         - /bin/sh
-        image: busybox
+        image: quay.io/helmpack/chart-testing:v3.3.1
         name: sidecar-container
         securityContext:
           runAsUser: 65532
         volumeMounts:
-        - mountPath: /var/logs
+        - mountPath: /logs
           name: shared-data
       volumes:
       - emptyDir: {}
-        name: shared-data
-EOF
\ No newline at end of file
+        name: shared-data
\ No newline at end of file

From 477d53ee650ee0201510e13b4cfe8ddaeb0716a9 Mon Sep 17 00:00:00 2001
From: Roy Paulin Nguetsop <roy.paulin@microfocus.com>
Date: Mon, 24 Jan 2022 20:39:48 +0100
Subject: [PATCH 3/4] Apply 2nd review comments

---
 cmd/operator/main.go                       | 43 +++++++++++++++++-----
 helm-charts/verticadb-operator/README.md   | 10 ++---
 helm-charts/verticadb-operator/values.yaml | 24 +++++-------
 scripts/create-helm-charts.sh              | 12 +++---
 4 files changed, 54 insertions(+), 35 deletions(-)

diff --git a/cmd/operator/main.go b/cmd/operator/main.go
index fc7a623f5..67c02ac3d 100644
--- a/cmd/operator/main.go
+++ b/cmd/operator/main.go
@@ -21,6 +21,7 @@ import (
 	"log"
 	"os"
 	"strconv"
+	"time"
 
 	// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
 	// to ensure that exec-entrypoint and run can make use of them.
@@ -50,6 +51,8 @@ const (
 	DefaultLevel           = "info"
 	DefaultDevMode         = true
 	DefaultZapcoreLevel    = zapcore.InfoLevel
+	First                  = 100
+	ThereAfter             = 100
 )
 
 var (
@@ -83,17 +86,19 @@ func init() {
 
 // setLoggingFlagArgs define logging flags with specified names and default values
 func (l *Logging) setLoggingFlagArgs() {
-	flag.StringVar(&l.FilePath, "filepath", "", "The file logging will write to.")
+	flag.StringVar(&l.FilePath, "filepath", "",
+		"The path to the log file. If omitted, all logging will be written to stdout.")
 	flag.IntVar(&l.MaxFileSize, "maxfilesize", DefaultMaxFileSize,
 		"The maximum size in megabytes of the log file "+
 			"before it gets rotated.")
 	flag.IntVar(&l.MaxFileAge, "maxfileage", DefaultMaxFileAge,
-		"This is the maximum age, in days, of the logging "+
-			"before log rotation gets rid of it.")
+		"The maximum number of days to retain old log files based on the timestamp encoded in the file.")
 	flag.IntVar(&l.MaxFileRotation, "maxfilerotation", DefaultMaxFileRotation,
-		"this is the maximum number of files that are kept in rotation before the old ones are removed.")
-	flag.StringVar(&l.Level, "level", DefaultLevel, "The minimum logging level.  Valid values are: debug, info, warn, and error.")
-	flag.BoolVar(&l.DevMode, "dev", DefaultDevMode, "Enables development mode if true and production mode otherwise.")
+		"The maximum number of files that are kept in rotation before the old ones are removed.")
+	flag.StringVar(&l.Level, "level", DefaultLevel,
+		"The minimum logging level.  Valid values are: debug, info, warn, and error.")
+	flag.BoolVar(&l.DevMode, "dev", DefaultDevMode,
+		"Enables development mode if true and production mode otherwise.")
 }
 
 // setFlagArgs define flags with specified names and default values
@@ -141,9 +146,11 @@ func getIsWebhookEnabled() bool {
 
 // getEncoderConfig returns a concrete encoders configuration
 func getEncoderConfig(devMode bool) zapcore.EncoderConfig {
-	encoderConfig := zap.NewProductionEncoderConfig()
-	if devMode {
-		encoderConfig = zap.NewDevelopmentEncoderConfig()
+	encoderConfig := zap.NewDevelopmentEncoderConfig()
+	if !devMode {
+		encoderConfig = zap.NewProductionEncoderConfig()
+		encoderConfig.EncodeLevel = zapcore.CapitalLevelEncoder
+		encoderConfig.EncodeTime = zapcore.ISO8601TimeEncoder
 	}
 	return encoderConfig
 }
@@ -173,11 +180,22 @@ func getZapcoreLevel(lvl string) zapcore.Level {
 	return *level
 }
 
+// getStackTrace returns an option that configures
+// the logger to record a stack strace.
+func getStackTrace(devMode bool) zap.Option {
+	lvl := zapcore.ErrorLevel
+	if devMode {
+		lvl = zapcore.WarnLevel
+	}
+	return zap.AddStacktrace(zapcore.LevelEnabler(lvl))
+}
+
 // getLogger is a wrapper that calls other functions
 // to build a logger.
 func getLogger(logArgs Logging) *zap.Logger {
 	encoderConfig := getEncoderConfig(logArgs.DevMode)
 	writes := []zapcore.WriteSyncer{}
+	opts := []zap.Option{}
 	lvl := zap.NewAtomicLevelAt(getZapcoreLevel(logArgs.Level))
 	if logArgs.FilePath != "" {
 		w := getLogWriter(logArgs)
@@ -191,7 +209,12 @@ func getLogger(logArgs Logging) *zap.Logger {
 		zapcore.NewMultiWriteSyncer(writes...),
 		lvl,
 	)
-	return zap.New(core)
+	opts = append(opts, getStackTrace(logArgs.DevMode))
+	if !logArgs.DevMode {
+		// This enables sampling only in prod
+		core = zapcore.NewSamplerWithOptions(core, time.Second, First, ThereAfter)
+	}
+	return zap.New(core, opts...)
 }
 
 func main() {
diff --git a/helm-charts/verticadb-operator/README.md b/helm-charts/verticadb-operator/README.md
index 05ff0fca3..06a23d520 100644
--- a/helm-charts/verticadb-operator/README.md
+++ b/helm-charts/verticadb-operator/README.md
@@ -5,11 +5,11 @@ This helm chart will install the operator and an admission controller webhook.
 | image.name | The name of image that runs the operator. | vertica/verticadb-operator:1.0.0 |
 | webhook.caBundle | A PEM encoded CA bundle that will be used to validate the webhook's server certificate.  If unspecified, system trust roots on the apiserver are used. | |
 | webhook.tlsSecret | The webhook requires a TLS certficate to work.  By default we rely on cert-manager to be installed as we use it generate the cert.  If you don't want to use cert-manager, you need to specify your own cert, which you can do with this parameter.  When set, it is a name of a secret in the same namespace the chart is being installed in.  The secret must have the keys: tls.key, ca.crt, and tls.crt. | |
-| logging.filePath | When specified, it is the file logging will write to. When empty, logging will write to standard out.  | |
-| logging.maxFileSize | When logging to a file, it is the maximum size, in MB, of the logging file before log rotation occurs. | 500 |
-| logging.maxFileAge | When logging to a file, it is the maximum age, in days, of the logging before log rotation gets rid of it. | 7 |
-| logging.maxFileRotation | When logging to a file, it is the maximum number of files that are kept in rotation before the old ones are removed. | 3 |
-| logging.level | It is the minimum logging level. Valid values are: debug, info, warn, and error | info |
+| logging.filePath | The path to the log file. If omitted, all logging will be written to stdout.  | |
+| logging.maxFileSize | The maximum size, in MB, of the logging file before log rotation occurs. This is only applicable if logging to a file. | 500 |
+| logging.maxFileAge | The maximum number of days to retain old log files based on the timestamp encoded in the file. This is only applicable if logging to a file. |
+| logging.maxFileRotation | The maximum number of files that are kept in rotation before the old ones are removed. This is only applicable if logging to a file. | 3 |
+| logging.level | The minimum logging level. Valid values are: debug, info, warn, and error | info |
 | logging.dev | Enables development mode if true and production mode otherwise. | false |
 | resources.\* | The resource requirements for the operator pod. | <pre>limits:<br>  cpu: 100m<br>  memory: 750Mi<br>requests:<br>  cpu: 100m<br>  memory: 20Mi</pre> |
 
diff --git a/helm-charts/verticadb-operator/values.yaml b/helm-charts/verticadb-operator/values.yaml
index 3805fcca7..4c5a442d6 100644
--- a/helm-charts/verticadb-operator/values.yaml
+++ b/helm-charts/verticadb-operator/values.yaml
@@ -38,27 +38,23 @@ webhook:
   caBundle: ""
 
 logging:
-  # filePath, when specified, is the file logging will write to.
-  # When empty, logging will write to standard out.
+  # filePath is the path to the log file. If omitted, all logging will be written to stdout.
   filePath: ""
-  # maxFileSize, when logging to a file, is the maximum size, 
-  # in MB, of the logging file before log rotation occurs.
+  # maxFileSize is the maximum size, in MB, of the logging file before log rotation occurs. 
+  # This is only applicable if logging to a file.
   maxFileSize: 500
-  # maxFileAge, when logging to a file, is the maximum age, in days,
-  # of the logging before log rotation gets rid of it.
+  # maxFileAge is the maximum number of days to retain old log files based on the timestamp
+  # encoded in the file. This is only applicable if logging to a file.
   maxFileAge: 7
-  # maxFileRotation, when logging to a file, is the maximum number of files 
-  # that are kept in rotation before the old ones are removed.
+  # maxFileRotation is the maximum number of files that are kept in rotation before the old ones are removed. 
+  # This is only applicable if logging to a file.
   maxFileRotation: 3
   # level is the minimum logging level. Valid values are: debug, info, warn, and error
   level: info
   # dev Enables development mode if true and production mode otherwise and also affects
-  # logs format. Afew differences on logging will be: in dev mode the the level is displayed
-  # in capital letters, the time is in a ISO8601-formatted string, in prod, the level is in
-  # lower case, the time is in epoch time format.
-  # example:
-  # dev ->  2022-01-21T12:50:10.078Z        INFO    controller-runtime.certwatcher  Starting certificate watcher
-  # prod -> 1.6427683530752437e+09  info    controller-runtime.certwatcher  Starting certificate watcher
+  # logs format. A few differences on logging will be: in dev mode stack traces are produced more liberally,
+  # on logs of WarnLevel and above while in production, they are included on logs of ErrorLevel and above.
+  # Moreover dev mode disables sampling. 
   dev: false
 
 
diff --git a/scripts/create-helm-charts.sh b/scripts/create-helm-charts.sh
index 50e0eac01..323c72e1d 100755
--- a/scripts/create-helm-charts.sh
+++ b/scripts/create-helm-charts.sh
@@ -50,12 +50,12 @@ done
 sed -i 's/resources: template-placeholder/resources:\n          {{- toYaml .Values.resources | nindent 10 }}/' $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
 
 # 6.  Template the logging
-sed -i "s/--filepath=/--filepath={{ .Values.logging.filePath }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
-sed -i "s/--maxfilesize=500/--maxfilesize={{ .Values.logging.maxFileSize }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
-sed -i "s/--maxfileage=7/--maxfileage={{ .Values.logging.maxFileAge }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
-sed -i "s/--maxfilerotation=3/--maxfilerotation={{ .Values.logging.maxFileRotation }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
-sed -i "s/--level=info/--level={{ .Values.logging.level }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
-sed -i "s/--dev=false/--dev={{ .Values.logging.dev }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
+sed -i "s/--filepath=.*/--filepath={{ .Values.logging.filePath }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
+sed -i "s/--maxfilesize=.*/--maxfilesize={{ .Values.logging.maxFileSize }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
+sed -i "s/--maxfileage=.*/--maxfileage={{ .Values.logging.maxFileAge }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
+sed -i "s/--maxfilerotation=.*/--maxfilerotation={{ .Values.logging.maxFileRotation }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
+sed -i "s/--level=.*/--level={{ .Values.logging.level }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
+sed -i "s/--dev=.*/--dev={{ .Values.logging.dev }}/" $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
 
 # Delete openshift clusterRole and clusterRoleBinding files
 rm $TEMPLATE_DIR/verticadb-operator-openshift-cluster-role-cr.yaml 

From 31102ad5f7da10f807f42d18af4d5901ba0fdef9 Mon Sep 17 00:00:00 2001
From: Roy Paulin Nguetsop <roy.paulin@microfocus.com>
Date: Tue, 25 Jan 2022 15:48:30 +0100
Subject: [PATCH 4/4] Add Changie entry for logging changes

---
 changes/unreleased/Added-20220125-154801.yaml | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100755 changes/unreleased/Added-20220125-154801.yaml

diff --git a/changes/unreleased/Added-20220125-154801.yaml b/changes/unreleased/Added-20220125-154801.yaml
new file mode 100755
index 000000000..c9363fea0
--- /dev/null
+++ b/changes/unreleased/Added-20220125-154801.yaml
@@ -0,0 +1,6 @@
+kind: Added
+body: New helm parameters to control the logging level and log path location for the
+  operator pod
+time: 2022-01-25T15:48:01.6005315+01:00
+custom:
+  Issue: "143"