Removes transitive dependency flags for operator. (#971)

* internal/manager: removes transitive dependency flags
explicitly bind only needed flags to the manager.
removes VictoriaMetrics httpserver for debug pprof. Use built-in pprof manager httpserver
#963

* fixes linter warnings

* Makefile: adds /internal packages to linter args

Makefile

@@ -234,7 +234,7 @@ e2e-local: fmt vet manifests fix118 fix_crd_nulls
 	$(GOCMD) tool cover -func coverage.txt  | grep total
 
 lint:
-	golangci-lint run --exclude '(SA1019):' -E typecheck -E gosimple -E gocritic   --timeout 5m ./controllers/...
+	golangci-lint run --exclude '(SA1019):' -E typecheck -E gosimple -E gocritic   --timeout 5m ./controllers/... ./internal/...
 	golint ./controllers/...
 
 .PHONY:clean

controllers/controllers.go

@@ -19,15 +19,22 @@ import (
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/client-go/util/workqueue"
+	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/metrics"
 )
 
+// BindFlags binds package flags to the given flagSet
+func BindFlags(f *flag.FlagSet) {
+	cacheSyncTimeout = f.Duration("controller.cacheSyncTimeout", *cacheSyncTimeout, "controls timeout for caches to be synced.")
+	maxConcurrency = f.Int("controller.maxConcurrentReconciles", *maxConcurrency, "Configures number of concurrent reconciles. It should improve performance for clusters with many objects.")
+}
+
 var (
-	cacheSyncTimeout = flag.Duration("controller.cacheSyncTimeout", 3*time.Minute, "controls timeout for caches to be synced.")
-	maxConcurrency   = flag.Int("controller.maxConcurrentReconciles", 1, "Configures number of concurrent reconciles. It should improve performance for clusters with many objects.")
+	cacheSyncTimeout = ptr.To(3 * time.Minute)
+	maxConcurrency   = ptr.To(1)
 )
 
 var (

docs/CHANGELOG.md

@@ -16,6 +16,7 @@ aliases:
 
 ## Next release
 
+- [operator](#./README.md): expose only command-line flags related to the operator. Remove all transitive dependency flags. See this [issue](https://github.com/VictoriaMetrics/operator/issues/963) for details.
 - [vmalertmanager](./api.md#vmalertmanager): ignores content of `cr.spec.configSecret` if it's name clashes with secret used by operator for storing alertmanager config. See this [issue](https://github.com/VictoriaMetrics/operator/issues/954) for details.
 - [operator](./README.md): remove finalizer for child objects with non-empty `DeletetionTimestamp`.  See this [issue](https://github.com/VictoriaMetrics/operator/issues/953) for details.
 - [operator](./README.md): skip storageClass check if there is no PVC size change. See this [issue](https://github.com/VictoriaMetrics/operator/issues/957) for details.

internal/config-reloader/k8s_watch.go

@@ -70,7 +70,7 @@ func newKubernetesWatcher(ctx context.Context, secretName, namespace string) (*k
 	}, &v1.Secret{}, 0, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
 
 	syncChan := make(chan syncEvent, 10)
-	inf.AddEventHandler(cache.ResourceEventHandlerFuncs{
+	if _, err := inf.AddEventHandler(cache.ResourceEventHandlerFuncs{
 		AddFunc: func(obj interface{}) {
 			s := obj.(*v1.Secret)
 			syncChan <- syncEvent{op: "create", obj: s}
@@ -83,7 +83,9 @@ func newKubernetesWatcher(ctx context.Context, secretName, namespace string) (*k
 			s := obj.(*v1.Secret)
 			syncChan <- syncEvent{op: "delete", obj: s}
 		},
-	})
+	}); err != nil {
+		return nil, fmt.Errorf("cannot build eventHandler: %w", err)
+	}
 
 	return &k8sWatcher{inf: inf, c: c, events: syncChan, namespace: namespace, secretName: secretName}, nil
 }

internal/config-reloader/main.go

@@ -296,7 +296,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool {
 		metrics.WritePrometheus(w, true)
 	case "/health":
 		w.WriteHeader(http.StatusOK)
-		w.Write([]byte(`OK`))
+		_, _ = w.Write([]byte(`OK`))
 	}
 	return false
 }

internal/config/config.go

@@ -357,7 +357,7 @@ func (boc BaseOperatorConf) Validate() error {
 func (boc BaseOperatorConf) PrintDefaults(format string) error {
 	tabs := tabwriter.NewWriter(os.Stdout, 1, 0, 4, ' ', 0)
 
-	formatter := "unknown"
+	var formatter string
 	switch format {
 	case "table":
 		formatter = envconfig.DefaultTableFormat

internal/manager/manager.go

@@ -11,7 +11,6 @@ import (
 	"time"
 
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/buildinfo"
-	"github.com/VictoriaMetrics/VictoriaMetrics/lib/httpserver"
 	victoriametricsv1beta1 "github.com/VictoriaMetrics/operator/api/v1beta1"
 	"github.com/VictoriaMetrics/operator/controllers"
 	"github.com/VictoriaMetrics/operator/controllers/factory/k8stools"
@@ -41,8 +40,9 @@ import (
 )
 
 var (
-	startTime  = time.Now()
-	appVersion = prometheus.NewGaugeFunc(prometheus.GaugeOpts{Name: "vm_app_version", Help: "version of application", ConstLabels: map[string]string{"version": buildinfo.Version}}, func() float64 {
+	managerFlags = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
+	startTime    = time.Now()
+	appVersion   = prometheus.NewGaugeFunc(prometheus.GaugeOpts{Name: "vm_app_version", Help: "version of application", ConstLabels: map[string]string{"version": buildinfo.Version}}, func() float64 {
 		return 1.0
 	})
 	uptime = prometheus.NewGaugeFunc(prometheus.GaugeOpts{Name: "vm_app_uptime_seconds", Help: "uptime"}, func() float64 {
@@ -53,21 +53,21 @@ var (
 	})
 	scheme               = runtime.NewScheme()
 	setupLog             = ctrl.Log.WithName("setup")
-	enableLeaderElection = flag.Bool("enable-leader-election", false, "Enable leader election for controller manager. "+
+	enableLeaderElection = managerFlags.Bool("enable-leader-election", false, "Enable leader election for controller manager. "+
 		"Enabling this will ensure there is only one active controller manager.")
-	enableWebhooks                = flag.Bool("webhook.enable", false, "adds webhook server, you must mount cert and key or use cert-manager")
-	disableCRDOwnership           = flag.Bool("controller.disableCRDOwnership", false, "disables CRD ownership add to cluster wide objects, must be disabled for clusters, lower than v1.16.0")
-	webhooksDir                   = flag.String("webhook.certDir", "/tmp/k8s-webhook-server/serving-certs/", "root directory for webhook cert and key")
-	webhookCertName               = flag.String("webhook.certName", "tls.crt", "name of webhook server Tls certificate inside tls.certDir")
-	webhookKeyName                = flag.String("webhook.keyName", "tls.key", "name of webhook server Tls key inside tls.certDir")
-	metricsAddr                   = flag.String("metrics-addr", ":8080", "The address the metric endpoint binds to.")
-	probeAddr                     = flag.String("http.readyListenAddr", "localhost:8081", "The address the probes (health, ready) binds to.")
-	listenAddr                    = flag.String("http.listenAddr", ":8435", "http server listen addr - serves victoria-metrics http server + metrics.")
-	defaultKubernetesMinorVersion = flag.Uint64("default.kubernetesVersion.minor", 21, "Minor version of kubernetes server, if operator cannot parse actual kubernetes response")
-	defaultKubernetesMajorVersion = flag.Uint64("default.kubernetesVersion.major", 1, "Major version of kubernetes server, if operator cannot parse actual kubernetes response")
-	printDefaults                 = flag.Bool("printDefaults", false, "print all variables with their default values and exit")
-	printFormat                   = flag.String("printFormat", "table", "output format for --printDefaults. Can be table, json, yaml or list")
-	promCRDResyncPeriod           = flag.Duration("controller.prometheusCRD.resyncPeriod", 0, "Configures resync period for prometheus CRD converter. Disabled by default")
+	enableWebhooks                = managerFlags.Bool("webhook.enable", false, "adds webhook server, you must mount cert and key or use cert-manager")
+	disableCRDOwnership           = managerFlags.Bool("controller.disableCRDOwnership", false, "disables CRD ownership add to cluster wide objects, must be disabled for clusters, lower than v1.16.0")
+	webhooksDir                   = managerFlags.String("webhook.certDir", "/tmp/k8s-webhook-server/serving-certs/", "root directory for webhook cert and key")
+	webhookCertName               = managerFlags.String("webhook.certName", "tls.crt", "name of webhook server Tls certificate inside tls.certDir")
+	webhookKeyName                = managerFlags.String("webhook.keyName", "tls.key", "name of webhook server Tls key inside tls.certDir")
+	metricsAddr                   = managerFlags.String("metrics-addr", ":8080", "The address the metric endpoint binds to.")
+	pprofAddr                     = managerFlags.String("pprof-addr", "localhost:8435", "The address for pprof/debug API. Empty value disables server")
+	probeAddr                     = managerFlags.String("http.readyListenAddr", "localhost:8081", "The address the probes (health, ready) binds to.")
+	defaultKubernetesMinorVersion = managerFlags.Uint64("default.kubernetesVersion.minor", 21, "Minor version of kubernetes server, if operator cannot parse actual kubernetes response")
+	defaultKubernetesMajorVersion = managerFlags.Uint64("default.kubernetesVersion.major", 1, "Major version of kubernetes server, if operator cannot parse actual kubernetes response")
+	printDefaults                 = managerFlags.Bool("printDefaults", false, "print all variables with their default values and exit")
+	printFormat                   = managerFlags.String("printFormat", "table", "output format for --printDefaults. Can be table, json, yaml or list")
+	promCRDResyncPeriod           = managerFlags.Duration("controller.prometheusCRD.resyncPeriod", 0, "Configures resync period for prometheus CRD converter. Disabled by default")
 	wasCacheSynced                = uint32(0)
 )
 
@@ -86,9 +86,10 @@ func RunManager(ctx context.Context) error {
 	// Add flags registered by imported packages (e.g. glog and
 	// controller-runtime)
 	opts := zap.Options{}
-	opts.BindFlags(flag.CommandLine)
+	opts.BindFlags(managerFlags)
+	controllers.BindFlags(managerFlags)
 
-	pflag.CommandLine.AddGoFlagSet(flag.CommandLine)
+	pflag.CommandLine.AddGoFlagSet(managerFlags)
 
 	pflag.Parse()
 
@@ -134,13 +135,13 @@ func RunManager(ctx context.Context) error {
 	}
 
 	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
-		Logger: ctrl.Log.WithName("manager"),
-		Scheme: scheme,
-		//	MetricsBindAddress:     *metricsAddr,
+		Logger:                 ctrl.Log.WithName("manager"),
+		Scheme:                 scheme,
 		Metrics:                metricsserver.Options{BindAddress: *metricsAddr},
 		HealthProbeBindAddress: *probeAddr,
 		ReadinessEndpointName:  "/ready",
 		LivenessEndpointName:   "/health",
+		PprofBindAddress:       *pprofAddr,
 		// port for webhook
 		WebhookServer: webhook.NewServer(webhook.Options{
 			Port:     9443,
@@ -379,20 +380,12 @@ func RunManager(ctx context.Context) error {
 		setupLog.Error(err, "cannot add runnable")
 		return err
 	}
-	if len(*listenAddr) > 0 {
-		go httpserver.Serve(*listenAddr, false, requestHandler)
-	}
 
 	setupLog.Info("starting manager")
 	if err := mgr.Start(ctx); err != nil {
 		setupLog.Error(err, "problem running manager")
 		return err
 	}
-	if len(*listenAddr) > 0 {
-		if err := httpserver.Stop(*listenAddr); err != nil {
-			setupLog.Error(err, "failed to gracefully stop HTTP server")
-		}
-	}
 
 	setupLog.Info("gracefully stopped")
 	return nil
@@ -420,8 +413,3 @@ func addWebhooks(mgr ctrl.Manager) error {
 		&victoriametricsv1beta1.VMRule{},
 	})
 }
-
-// no-op
-func requestHandler(w http.ResponseWriter, r *http.Request) bool {
-	return false
-}