making metrics port configurable and constructor for metrics

Signed-off-by: sethiyash <[email protected]>

Author: sethiyash

cmd/controller/run.go

@@ -95,13 +95,6 @@ func Run(opts Options, runLog logr.Logger) error {
 		return fmt.Errorf("Building packaging client: %s", err)
 	}
 
-	runLog.Info("setting up metrics")
-	countMetrics := metrics.NewCountMetrics()
-	countMetrics.RegisterAllMetrics()
-
-	reconcileTimeMetrics := metrics.NewReconcileTimeMetrics()
-	reconcileTimeMetrics.RegisterAllMetrics()
-
 	var server *apiserver.APIServer
 	if opts.StartAPIServer {
 		// assign bindPort to env var KAPPCTRL_API_PORT if available
@@ -189,7 +182,11 @@ func Run(opts Options, runLog logr.Logger) error {
 	// initialize cluster access once - it contains a service account token cache which should be only setup once.
 	kubeconf := kubeconfig.NewKubeconfig(coreClient, runLog)
 	compInfo := componentinfo.NewComponentInfo(coreClient, kubeconf, Version)
-	appMetrics := &metrics.Metrics{ReconcileCountMetrics: countMetrics, ReconcileTimeMetrics: reconcileTimeMetrics}
+
+	runLog.Info("setting up metrics")
+	appMetrics := metrics.NewMetrics()
+	appMetrics.ReconcileTimeMetrics.RegisterAllMetrics()
+	appMetrics.ReconcileCountMetrics.RegisterAllMetrics()
 
 	cacheFolderApps := memdir.NewTmpDir("cache-appcr")
 	err = cacheFolderApps.Create()

config/config/deployment.yml

@@ -52,7 +52,7 @@ spec:
         - containerPort: #@ data.values.apiPort
           name: api
           protocol: TCP
-        - containerPort: 8080
+        - containerPort: #@ data.values.metricsPort
           name: metrics
           protocol: TCP
         securityContext:

config/values-schema.yml

@@ -17,6 +17,8 @@ dangerousEnablePprof: false
 tlsCipherSuites: ""
 #@schema/desc "API port"
 apiPort: 8443
+#@schema/desc "Metrics port"
+metricsPort: 8080
 #@schema/desc "The coreDNSIP will be injected into /etc/resolv.conf of kapp-controller pod"
 coreDNSIP: ""
 #@schema/desc "HostNetwork of kapp-controller deployment."

pkg/app/app_reconcile_test.go

@@ -29,10 +29,6 @@ import (
 
 func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) {
 	log := logf.Log.WithName("kc")
-	var (
-		appMetrics  = metrics.NewCountMetrics()
-		timeMetrics = metrics.NewReconcileTimeMetrics()
-	)
 
 	// The url under fetch is invalid, which will cause this
 	// app to fail before deploy.
@@ -55,7 +51,7 @@ func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) {
 	tmpFac := template.NewFactory(k8scs, fetchFac, false, exec.NewPlainCmdRunner())
 	deployFac := deploy.NewFactory(k8scs, kubeconfig.NewKubeconfig(k8scs, log), nil, exec.NewPlainCmdRunner(), log)
 
-	crdApp := NewCRDApp(&app, log, &metrics.Metrics{ReconcileCountMetrics: appMetrics, ReconcileTimeMetrics: timeMetrics}, kappcs, fetchFac, tmpFac, deployFac, FakeComponentInfo{}, Opts{MinimumSyncPeriod: 30 * time.Second})
+	crdApp := NewCRDApp(&app, log, metrics.NewMetrics(), kappcs, fetchFac, tmpFac, deployFac, FakeComponentInfo{}, Opts{MinimumSyncPeriod: 30 * time.Second})
 	_, err := crdApp.Reconcile(false)
 	assert.Nil(t, err, "unexpected error with reconciling", err)
 
@@ -89,10 +85,6 @@ func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) {
 
 func Test_NoInspectReconcile_IfInspectNotEnabled(t *testing.T) {
 	log := logf.Log.WithName("kc")
-	var (
-		appMetrics  = metrics.NewCountMetrics()
-		timeMetrics = metrics.NewReconcileTimeMetrics()
-	)
 
 	app := v1alpha1.App{
 		ObjectMeta: metav1.ObjectMeta{
@@ -125,7 +117,7 @@ func Test_NoInspectReconcile_IfInspectNotEnabled(t *testing.T) {
 	tmpFac := template.NewFactory(k8scs, fetchFac, false, exec.NewPlainCmdRunner())
 	deployFac := deploy.NewFactory(k8scs, kubeconfig.NewKubeconfig(k8scs, log), nil, exec.NewPlainCmdRunner(), log)
 
-	crdApp := NewCRDApp(&app, log, &metrics.Metrics{ReconcileCountMetrics: appMetrics, ReconcileTimeMetrics: timeMetrics}, kappcs, fetchFac, tmpFac, deployFac, FakeComponentInfo{}, Opts{MinimumSyncPeriod: 30 * time.Second})
+	crdApp := NewCRDApp(&app, log, metrics.NewMetrics(), kappcs, fetchFac, tmpFac, deployFac, FakeComponentInfo{}, Opts{MinimumSyncPeriod: 30 * time.Second})
 	_, err := crdApp.Reconcile(false)
 	assert.Nil(t, err, "unexpected error with reconciling", err)
 
@@ -170,10 +162,6 @@ func Test_NoInspectReconcile_IfInspectNotEnabled(t *testing.T) {
 
 func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing.T) {
 	log := logf.Log.WithName("kc")
-	var (
-		appMetrics  = metrics.NewCountMetrics()
-		timeMetrics = metrics.NewReconcileTimeMetrics()
-	)
 
 	fetchInline := map[string]string{
 		"file.yml": `foo: #@ data.values.nothere`,
@@ -200,7 +188,7 @@ func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing.
 	tmpFac := template.NewFactory(k8scs, fetchFac, false, exec.NewPlainCmdRunner())
 	deployFac := deploy.NewFactory(k8scs, kubeconfig.NewKubeconfig(k8scs, log), nil, exec.NewPlainCmdRunner(), log)
 
-	crdApp := NewCRDApp(&app, log, &metrics.Metrics{ReconcileCountMetrics: appMetrics, ReconcileTimeMetrics: timeMetrics}, kappcs, fetchFac, tmpFac, deployFac, FakeComponentInfo{}, Opts{MinimumSyncPeriod: 30 * time.Second})
+	crdApp := NewCRDApp(&app, log, metrics.NewMetrics(), kappcs, fetchFac, tmpFac, deployFac, FakeComponentInfo{}, Opts{MinimumSyncPeriod: 30 * time.Second})
 	_, err := crdApp.Reconcile(false)
 	assert.Nil(t, err, "Unexpected error with reconciling", err)
 

pkg/app/app_template_test.go

@@ -61,7 +61,7 @@ func Test_BuildAdditionalDownwardAPIValues_MemoizedCallCount(t *testing.T) {
 		K8sAPIsCount:    &k8sAPIsCallCount,
 		KCVersionCount:  &kcVersionCallCount,
 	}
-	app := NewApp(appEmpty, Hooks{}, fetchFac, tmpFac, deployFac, log, Opts{}, &metrics.Metrics{}, fakeInfo)
+	app := NewApp(appEmpty, Hooks{}, fetchFac, tmpFac, deployFac, log, Opts{}, metrics.NewMetrics(), fakeInfo)
 
 	dir, err := os.MkdirTemp("", "temp")
 	assert.NoError(t, err)

pkg/app/app_test.go

@@ -65,7 +65,7 @@ func Test_SecretRefs_RetrievesAllSecretRefs(t *testing.T) {
 	tmpFac := template.NewFactory(k8scs, fetchFac, false, exec.NewPlainCmdRunner())
 	deployFac := deploy.NewFactory(k8scs, kubeconfig.NewKubeconfig(k8scs, log), nil, exec.NewPlainCmdRunner(), log)
 
-	app := apppkg.NewApp(appWithRefs, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, apppkg.Opts{}, &metrics.Metrics{}, FakeComponentInfo{})
+	app := apppkg.NewApp(appWithRefs, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, apppkg.Opts{}, metrics.NewMetrics(), FakeComponentInfo{})
 
 	out := app.SecretRefs()
 	assert.Truef(t, reflect.DeepEqual(out, expected), "Expected: %s\nGot: %s\n", expected, out)
@@ -89,7 +89,7 @@ func Test_SecretRefs_RetrievesNoSecretRefs_WhenNonePresent(t *testing.T) {
 	tmpFac := template.NewFactory(k8scs, fetchFac, false, exec.NewPlainCmdRunner())
 	deployFac := deploy.NewFactory(k8scs, kubeconfig.NewKubeconfig(k8scs, log), nil, exec.NewPlainCmdRunner(), log)
 
-	app := apppkg.NewApp(appEmpty, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, apppkg.Opts{}, &metrics.Metrics{}, FakeComponentInfo{})
+	app := apppkg.NewApp(appEmpty, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, apppkg.Opts{}, metrics.NewMetrics(), FakeComponentInfo{})
 
 	out := app.SecretRefs()
 	assert.Equal(t, 0, len(out), "No SecretRefs to be returned")
@@ -127,7 +127,7 @@ func Test_ConfigMapRefs_RetrievesAllConfigMapRefs(t *testing.T) {
 	tmpFac := template.NewFactory(k8scs, fetchFac, false, exec.NewPlainCmdRunner())
 	deployFac := deploy.NewFactory(k8scs, kubeconfig.NewKubeconfig(k8scs, log), nil, exec.NewPlainCmdRunner(), log)
 
-	app := apppkg.NewApp(appWithRefs, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, apppkg.Opts{}, &metrics.Metrics{}, FakeComponentInfo{})
+	app := apppkg.NewApp(appWithRefs, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, apppkg.Opts{}, metrics.NewMetrics(), FakeComponentInfo{})
 
 	out := app.ConfigMapRefs()
 	assert.Truef(t, reflect.DeepEqual(out, expected), "Expected: %s\nGot: %s\n", expected, out)
@@ -151,7 +151,7 @@ func Test_ConfigMapRefs_RetrievesNoConfigMapRefs_WhenNonePresent(t *testing.T) {
 	tmpFac := template.NewFactory(k8scs, fetchFac, false, exec.NewPlainCmdRunner())
 	deployFac := deploy.NewFactory(k8scs, kubeconfig.NewKubeconfig(k8scs, log), nil, exec.NewPlainCmdRunner(), log)
 
-	app := apppkg.NewApp(appEmpty, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, apppkg.Opts{}, &metrics.Metrics{}, FakeComponentInfo{})
+	app := apppkg.NewApp(appEmpty, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, apppkg.Opts{}, metrics.NewMetrics(), FakeComponentInfo{})
 
 	out := app.ConfigMapRefs()
 	assert.Lenf(t, out, 0, "Expected: %s\nGot: %s\n", "No ConfigMapRefs to be returned", out)

pkg/metrics/metrics.go

@@ -9,3 +9,12 @@ type Metrics struct {
 	*ReconcileTimeMetrics
 	IsFirstReconcile bool
 }
+
+// NewMetrics is a factory function that returns a new instance of Metrics.
+func NewMetrics() *Metrics {
+	return &Metrics{
+		ReconcileCountMetrics: NewCountMetrics(),
+		ReconcileTimeMetrics:  NewReconcileTimeMetrics(),
+		IsFirstReconcile:      false,
+	}
+}

pkg/metrics/reconcile_time_metrics.go

@@ -5,11 +5,12 @@
 package metrics
 
 import (
-	"github.com/prometheus/client_golang/prometheus"
-	"sigs.k8s.io/controller-runtime/pkg/metrics"
 	"strconv"
 	"sync"
 	"time"
+
+	"github.com/prometheus/client_golang/prometheus"
+	"sigs.k8s.io/controller-runtime/pkg/metrics"
 )
 
 // ReconcileTimeMetrics holds reconcile time metrics

pkg/packageinstall/packageinstall_deletion_test.go

@@ -68,7 +68,7 @@ func Test_PackageInstallDeletion(t *testing.T) {
 
 		ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, coreClient,
 			FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{},
-			&metrics.Metrics{ReconcileCountMetrics: metrics.NewCountMetrics(), ReconcileTimeMetrics: metrics.NewReconcileTimeMetrics()})
+			metrics.NewMetrics())
 		_, err := ip.Reconcile()
 		assert.Nil(t, err)
 

pkg/packageinstall/packageinstall_downgrade_test.go

@@ -26,8 +26,6 @@ import (
 
 func Test_PackageInstallVersionDowngrades(t *testing.T) {
 	log := logf.Log.WithName("kc")
-	var reconcileTimeMetrics = metrics.NewReconcileTimeMetrics()
-	var reconcileCountMetrics = metrics.NewCountMetrics()
 
 	pkg1 := datapkgingv1alpha1.Package{
 		ObjectMeta: metav1.ObjectMeta{
@@ -103,7 +101,7 @@ func Test_PackageInstallVersionDowngrades(t *testing.T) {
 
 		ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s,
 			FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{},
-			&metrics.Metrics{ReconcileCountMetrics: reconcileCountMetrics, ReconcileTimeMetrics: reconcileTimeMetrics})
+			metrics.NewMetrics())
 		_, err := ip.Reconcile()
 		assert.Nil(t, err)
 
@@ -154,7 +152,7 @@ func Test_PackageInstallVersionDowngrades(t *testing.T) {
 
 		ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s,
 			FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{},
-			&metrics.Metrics{ReconcileCountMetrics: reconcileCountMetrics, ReconcileTimeMetrics: reconcileTimeMetrics})
+			metrics.NewMetrics())
 		_, err := ip.Reconcile()
 		assert.Nil(t, err)
 
@@ -205,7 +203,7 @@ func Test_PackageInstallVersionDowngrades(t *testing.T) {
 
 		ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s,
 			FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{},
-			&metrics.Metrics{ReconcileCountMetrics: reconcileCountMetrics, ReconcileTimeMetrics: reconcileTimeMetrics})
+			metrics.NewMetrics())
 		_, err := ip.Reconcile()
 		assert.Nil(t, err)
 
@@ -262,7 +260,7 @@ func Test_PackageInstallVersionDowngrades(t *testing.T) {
 
 		ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s,
 			FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{},
-			&metrics.Metrics{ReconcileCountMetrics: reconcileCountMetrics, ReconcileTimeMetrics: reconcileTimeMetrics})
+			metrics.NewMetrics())
 		_, err := ip.Reconcile()
 		assert.Nil(t, err)