From cbf417978e65f7028c1b70b6b9439ac0d16e8ae2 Mon Sep 17 00:00:00 2001 From: mereta Date: Thu, 16 Jan 2025 12:07:36 +0000 Subject: [PATCH 1/6] Adding telemetry shutdown for Retina Operator --- operator/cmd/legacy/deployment.go | 48 +++++++++++++++++-------------- operator/cmd/root.go | 9 ++++-- 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/operator/cmd/legacy/deployment.go b/operator/cmd/legacy/deployment.go index 41a060b485..f24cf81d6f 100644 --- a/operator/cmd/legacy/deployment.go +++ b/operator/cmd/legacy/deployment.go @@ -8,7 +8,6 @@ import ( "fmt" "net/http" "net/http/pprof" - "os" "time" "go.uber.org/zap/zapcore" @@ -81,7 +80,7 @@ func NewOperator(metricsAddr, probeAddr, configFile string, enableLeaderElection } } -func (o *Operator) Start() { +func (o *Operator) Start() error { mainLogger = log.Logger().Named("main") mainLogger.Sugar().Infof("Starting legacy operator") @@ -94,7 +93,7 @@ func (o *Operator) Start() { oconfig, err = config.GetConfig(o.configFile) if err != nil { fmt.Printf("failed to load config with err %s", err.Error()) - os.Exit(1) + return err } mainLogger.Sugar().Infof("Operator configuration", zap.Any("configuration", oconfig)) @@ -105,13 +104,13 @@ func (o *Operator) Start() { if err != nil { fmt.Printf("failed to load config with err %s", err.Error()) - os.Exit(1) + return err } err = initLogging(oconfig, buildinfo.ApplicationInsightsID) if err != nil { fmt.Printf("failed to initialize logging with err %s", err.Error()) - os.Exit(1) + return err } ctrl.SetLogger(crzap.New(crzap.UseFlagOptions(opts), crzap.Encoder(zapcore.NewConsoleEncoder(log.EncoderConfig())))) @@ -139,14 +138,14 @@ func (o *Operator) Start() { }) if err != nil { mainLogger.Error("Unable to start manager", zap.Error(err)) - os.Exit(1) + return err } ctx := context.Background() clientset, err := apiextv1.NewForConfig(mgr.GetConfig()) if err != nil { mainLogger.Error("Failed to get apiextension clientset", zap.Error(err)) - os.Exit(1) + return err } if oconfig.InstallCRDs { @@ -156,7 +155,7 @@ func (o *Operator) Start() { crds, err = deploy.InstallOrUpdateCRDs(ctx, oconfig.EnableRetinaEndpoint, clientset) if err != nil { mainLogger.Error("unable to register CRDs", zap.Error(err)) - os.Exit(1) + return err } for name := range crds { mainLogger.Info("CRD registered", zap.String("name", name)) @@ -166,7 +165,7 @@ func (o *Operator) Start() { apiserverURL, err := telemetry.GetK8SApiserverURLFromKubeConfig() if err != nil { mainLogger.Error("Apiserver URL is cannot be found", zap.Error(err)) - os.Exit(1) + return err } var tel telemetry.Telemetry @@ -176,10 +175,15 @@ func (o *Operator) Start() { "version": buildinfo.Version, telemetry.PropertyApiserver: apiserverURL, } + + telemetry.InitAppInsights(buildinfo.ApplicationInsightsID, buildinfo.Version) + defer telemetry.ShutdownAppInsights() + defer telemetry.TrackPanic() + tel, err = telemetry.NewAppInsightsTelemetryClient("retina-operator", properties) if err != nil { mainLogger.Error("failed to create telemetry client", zap.Error(err)) - os.Exit(1) + return err } } else { mainLogger.Info("telemetry disabled", zap.String("apiserver", apiserverURL)) @@ -189,7 +193,7 @@ func (o *Operator) Start() { kubeClient, err := kubernetes.NewForConfig(mgr.GetConfig()) if err != nil { mainLogger.Error("Failed to get clientset", zap.Error(err)) - os.Exit(1) + return err } captureReconciler, err := captureController.NewCaptureReconciler( @@ -197,11 +201,11 @@ func (o *Operator) Start() { ) if err != nil { mainLogger.Error("Unable to create capture reconciler", zap.Error(err)) - os.Exit(1) + return err } if err = captureReconciler.SetupWithManager(mgr); err != nil { mainLogger.Error("Unable to setup retina capture controller with manager", zap.Error(err)) - os.Exit(1) + return err } ctrlCtx := ctrl.SetupSignalHandler() @@ -226,7 +230,7 @@ func (o *Operator) Start() { pc := podcontroller.New(mgr.GetClient(), mgr.GetScheme(), retinaendpointchannel) if err = (pc).SetupWithManager(mgr); err != nil { mainLogger.Error("Unable to create controller", zap.String("controller", "podcontroller"), zap.Error(err)) - os.Exit(1) + return err } } } @@ -234,28 +238,30 @@ func (o *Operator) Start() { mc := metricsconfiguration.New(mgr.GetClient(), mgr.GetScheme()) if err = (mc).SetupWithManager(mgr); err != nil { mainLogger.Error("Unable to create controller", zap.String("controller", "metricsconfiguration"), zap.Error(err)) - os.Exit(1) + return err } //+kubebuilder:scaffold:builder - if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { + if err = mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { mainLogger.Error("Unable to set up health check", zap.Error(err)) - os.Exit(1) + return err } - if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil { + if err = mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil { mainLogger.Error("Unable to set up ready check", zap.Error(err)) - os.Exit(1) + return err } mainLogger.Info("Starting manager") - if err := mgr.Start(ctrlCtx); err != nil { + if err = mgr.Start(ctrlCtx); err != nil { mainLogger.Error("Problem running manager", zap.Error(err)) - os.Exit(1) + return err } // start heartbeat goroutine for application insights go tel.Heartbeat(ctx, HeartbeatFrequency) + + return nil } func EnablePProf() { diff --git a/operator/cmd/root.go b/operator/cmd/root.go index f689f12909..d3a2d3e0b6 100644 --- a/operator/cmd/root.go +++ b/operator/cmd/root.go @@ -25,10 +25,15 @@ var ( Use: "retina-operator", Short: "Retina Operator", Long: "Start Retina Operator", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { fmt.Println("Starting Retina Operator") d := legacy.NewOperator(metricsAddr, probeAddr, cfgFile, enableLeaderElection) - d.Start() + + if err := d.Start(); err != nil { + fmt.Println(err) + return err + } + return nil }, } ) From e2dcffbf699b498109751c5b5626ef87970ff51d Mon Sep 17 00:00:00 2001 From: mereta Date: Thu, 16 Jan 2025 12:14:43 +0000 Subject: [PATCH 2/6] Adding telemetry shutdown for Retina Operator --- operator/cmd/legacy/deployment.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/operator/cmd/legacy/deployment.go b/operator/cmd/legacy/deployment.go index f24cf81d6f..192112a64a 100644 --- a/operator/cmd/legacy/deployment.go +++ b/operator/cmd/legacy/deployment.go @@ -243,17 +243,17 @@ func (o *Operator) Start() error { //+kubebuilder:scaffold:builder - if err = mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { + if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { mainLogger.Error("Unable to set up health check", zap.Error(err)) return err } - if err = mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil { + if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil { mainLogger.Error("Unable to set up ready check", zap.Error(err)) return err } mainLogger.Info("Starting manager") - if err = mgr.Start(ctrlCtx); err != nil { + if err := mgr.Start(ctrlCtx); err != nil { mainLogger.Error("Problem running manager", zap.Error(err)) return err } From ae09d717b89b24860101bad2898dd2b60951d57c Mon Sep 17 00:00:00 2001 From: mereta Date: Thu, 16 Jan 2025 13:00:56 +0000 Subject: [PATCH 3/6] Wrap errors --- operator/cmd/legacy/deployment.go | 4 +++- operator/cmd/root.go | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/operator/cmd/legacy/deployment.go b/operator/cmd/legacy/deployment.go index 192112a64a..a5bfe8630b 100644 --- a/operator/cmd/legacy/deployment.go +++ b/operator/cmd/legacy/deployment.go @@ -5,6 +5,7 @@ package legacy import ( "context" + "errors" "fmt" "net/http" "net/http/pprof" @@ -93,7 +94,8 @@ func (o *Operator) Start() error { oconfig, err = config.GetConfig(o.configFile) if err != nil { fmt.Printf("failed to load config with err %s", err.Error()) - return err + + return errors.Wrap(err, "") } mainLogger.Sugar().Infof("Operator configuration", zap.Any("configuration", oconfig)) diff --git a/operator/cmd/root.go b/operator/cmd/root.go index d3a2d3e0b6..1ec47d290f 100644 --- a/operator/cmd/root.go +++ b/operator/cmd/root.go @@ -25,7 +25,7 @@ var ( Use: "retina-operator", Short: "Retina Operator", Long: "Start Retina Operator", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(_ *cobra.Command, args []string) error { fmt.Println("Starting Retina Operator") d := legacy.NewOperator(metricsAddr, probeAddr, cfgFile, enableLeaderElection) From e91b49649a768023c39d1809d0bb8b3b9063ed03 Mon Sep 17 00:00:00 2001 From: mereta Date: Thu, 16 Jan 2025 13:04:18 +0000 Subject: [PATCH 4/6] Wrap errors --- operator/cmd/legacy/deployment.go | 53 ++++++++++--------------------- 1 file changed, 16 insertions(+), 37 deletions(-) diff --git a/operator/cmd/legacy/deployment.go b/operator/cmd/legacy/deployment.go index a5bfe8630b..3f1bb23b20 100644 --- a/operator/cmd/legacy/deployment.go +++ b/operator/cmd/legacy/deployment.go @@ -5,7 +5,6 @@ package legacy import ( "context" - "errors" "fmt" "net/http" "net/http/pprof" @@ -40,6 +39,7 @@ import ( retinaendpointcontroller "github.com/microsoft/retina/pkg/controllers/operator/retinaendpoint" "github.com/microsoft/retina/pkg/log" "github.com/microsoft/retina/pkg/telemetry" + "github.com/pkg/errors" ) var ( @@ -93,9 +93,7 @@ func (o *Operator) Start() error { var err error oconfig, err = config.GetConfig(o.configFile) if err != nil { - fmt.Printf("failed to load config with err %s", err.Error()) - - return errors.Wrap(err, "") + return errors.Wrap(err, "failed to load config") } mainLogger.Sugar().Infof("Operator configuration", zap.Any("configuration", oconfig)) @@ -104,15 +102,9 @@ func (o *Operator) Start() error { oconfig.CaptureConfig.CaptureImageVersion = buildinfo.Version oconfig.CaptureConfig.CaptureImageVersionSource = captureUtils.VersionSourceOperatorImageVersion - if err != nil { - fmt.Printf("failed to load config with err %s", err.Error()) - return err - } - err = initLogging(oconfig, buildinfo.ApplicationInsightsID) if err != nil { - fmt.Printf("failed to initialize logging with err %s", err.Error()) - return err + return errors.Wrap(err, "failed to initialize logging") } ctrl.SetLogger(crzap.New(crzap.UseFlagOptions(opts), crzap.Encoder(zapcore.NewConsoleEncoder(log.EncoderConfig())))) @@ -139,15 +131,13 @@ func (o *Operator) Start() error { // LeaderElectionReleaseOnCancel: true, }) if err != nil { - mainLogger.Error("Unable to start manager", zap.Error(err)) - return err + return errors.Wrap(err, "unable to start manager") } ctx := context.Background() clientset, err := apiextv1.NewForConfig(mgr.GetConfig()) if err != nil { - mainLogger.Error("Failed to get apiextension clientset", zap.Error(err)) - return err + return errors.Wrap(err, "failed to get apiextension clientset") } if oconfig.InstallCRDs { @@ -156,8 +146,7 @@ func (o *Operator) Start() error { var crds map[string]*v1.CustomResourceDefinition crds, err = deploy.InstallOrUpdateCRDs(ctx, oconfig.EnableRetinaEndpoint, clientset) if err != nil { - mainLogger.Error("unable to register CRDs", zap.Error(err)) - return err + return errors.Wrap(err, "unable to register CRDs") } for name := range crds { mainLogger.Info("CRD registered", zap.String("name", name)) @@ -166,8 +155,7 @@ func (o *Operator) Start() error { apiserverURL, err := telemetry.GetK8SApiserverURLFromKubeConfig() if err != nil { - mainLogger.Error("Apiserver URL is cannot be found", zap.Error(err)) - return err + return errors.Wrap(err, "apiserver URL is cannot be found") } var tel telemetry.Telemetry @@ -184,8 +172,7 @@ func (o *Operator) Start() error { tel, err = telemetry.NewAppInsightsTelemetryClient("retina-operator", properties) if err != nil { - mainLogger.Error("failed to create telemetry client", zap.Error(err)) - return err + return errors.Wrap(err, "failed to create telemetry client") } } else { mainLogger.Info("telemetry disabled", zap.String("apiserver", apiserverURL)) @@ -194,20 +181,17 @@ func (o *Operator) Start() error { kubeClient, err := kubernetes.NewForConfig(mgr.GetConfig()) if err != nil { - mainLogger.Error("Failed to get clientset", zap.Error(err)) - return err + return errors.Wrap(err, "failed to get clientset") } captureReconciler, err := captureController.NewCaptureReconciler( mgr.GetClient(), mgr.GetScheme(), kubeClient, oconfig.CaptureConfig, ) if err != nil { - mainLogger.Error("Unable to create capture reconciler", zap.Error(err)) - return err + return errors.Wrap(err, "unable to create capture reconciler") } if err = captureReconciler.SetupWithManager(mgr); err != nil { - mainLogger.Error("Unable to setup retina capture controller with manager", zap.Error(err)) - return err + return errors.Wrap(err, "unable to setup retina capture controller with manager") } ctrlCtx := ctrl.SetupSignalHandler() @@ -231,33 +215,28 @@ func (o *Operator) Start() error { pc := podcontroller.New(mgr.GetClient(), mgr.GetScheme(), retinaendpointchannel) if err = (pc).SetupWithManager(mgr); err != nil { - mainLogger.Error("Unable to create controller", zap.String("controller", "podcontroller"), zap.Error(err)) - return err + return errors.Wrap(err, "unable to create controller - podcontroller") } } } mc := metricsconfiguration.New(mgr.GetClient(), mgr.GetScheme()) if err = (mc).SetupWithManager(mgr); err != nil { - mainLogger.Error("Unable to create controller", zap.String("controller", "metricsconfiguration"), zap.Error(err)) - return err + return errors.Wrap(err, "unable to create controller - metricsconfiguration") } //+kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { - mainLogger.Error("Unable to set up health check", zap.Error(err)) - return err + return errors.Wrap(err, "unable to set up health check") } if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil { - mainLogger.Error("Unable to set up ready check", zap.Error(err)) - return err + return errors.Wrap(err, "unable to set up ready check") } mainLogger.Info("Starting manager") if err := mgr.Start(ctrlCtx); err != nil { - mainLogger.Error("Problem running manager", zap.Error(err)) - return err + return errors.Wrap(err, "problem running manager") } // start heartbeat goroutine for application insights From 75f2ee70b0f3f2a3a1b375adf547f42d61ce0439 Mon Sep 17 00:00:00 2001 From: mereta Date: Thu, 16 Jan 2025 13:42:44 +0000 Subject: [PATCH 5/6] Wrap errors --- operator/cmd/root.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/operator/cmd/root.go b/operator/cmd/root.go index 1ec47d290f..05f06ecf47 100644 --- a/operator/cmd/root.go +++ b/operator/cmd/root.go @@ -8,6 +8,7 @@ import ( "os" "github.com/microsoft/retina/operator/cmd/legacy" + "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -30,8 +31,7 @@ var ( d := legacy.NewOperator(metricsAddr, probeAddr, cfgFile, enableLeaderElection) if err := d.Start(); err != nil { - fmt.Println(err) - return err + return errors.Wrap(err, "failed to start retina-operator") } return nil }, From 094e24ce5d50ff63477abff4bd0ead8a079c7c22 Mon Sep 17 00:00:00 2001 From: mereta Date: Thu, 16 Jan 2025 13:51:52 +0000 Subject: [PATCH 6/6] Fix linter issue - unused var --- operator/cmd/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator/cmd/root.go b/operator/cmd/root.go index 05f06ecf47..9cbcfb3811 100644 --- a/operator/cmd/root.go +++ b/operator/cmd/root.go @@ -26,7 +26,7 @@ var ( Use: "retina-operator", Short: "Retina Operator", Long: "Start Retina Operator", - RunE: func(_ *cobra.Command, args []string) error { + RunE: func(_ *cobra.Command, _ []string) error { fmt.Println("Starting Retina Operator") d := legacy.NewOperator(metricsAddr, probeAddr, cfgFile, enableLeaderElection)