From f0e42c0a976e05ff9f652b7018e22387e3142b2e Mon Sep 17 00:00:00 2001 From: sethiyash Date: Thu, 23 Nov 2023 12:25:54 +0530 Subject: [PATCH 1/9] Added gauge metrics to record reconcile time for app and pkg repo Signed-off-by: sethiyash --- cmd/controller/run.go | 5 ++ pkg/app/app.go | 11 ++-- pkg/app/app_deploy.go | 6 ++ pkg/app/app_factory.go | 3 +- pkg/app/app_fetch.go | 7 ++ pkg/app/app_reconcile.go | 5 ++ pkg/app/app_reconcile_test.go | 21 ++++-- pkg/app/app_template.go | 6 ++ pkg/app/app_template_test.go | 2 +- pkg/app/app_test.go | 8 +-- pkg/app/crd_app.go | 4 +- pkg/metrics/reconcile_time_metrics.go | 85 +++++++++++++++++++++++++ pkg/pkgrepository/app.go | 8 ++- pkg/pkgrepository/app_deploy.go | 6 ++ pkg/pkgrepository/app_factory.go | 4 +- pkg/pkgrepository/app_fetch.go | 4 ++ pkg/pkgrepository/app_reconcile.go | 5 ++ pkg/pkgrepository/app_reconcile_test.go | 10 ++- pkg/pkgrepository/app_template.go | 6 ++ pkg/pkgrepository/crd_app.go | 16 +++-- 20 files changed, 190 insertions(+), 32 deletions(-) create mode 100644 pkg/metrics/reconcile_time_metrics.go diff --git a/cmd/controller/run.go b/cmd/controller/run.go index 7d7826925..f16955f1e 100644 --- a/cmd/controller/run.go +++ b/cmd/controller/run.go @@ -99,6 +99,9 @@ func Run(opts Options, runLog logr.Logger) error { appMetrics := metrics.NewAppMetrics() appMetrics.RegisterAllMetrics() + reconcileTimeMetrics := metrics.NewReconcileTimeMetrics() + reconcileTimeMetrics.RegisterAllMetrics() + var server *apiserver.APIServer if opts.StartAPIServer { // assign bindPort to env var KAPPCTRL_API_PORT if available @@ -198,6 +201,7 @@ func Run(opts Options, runLog logr.Logger) error { AppClient: kcClient, KcConfig: kcConfig, AppMetrics: appMetrics, + TimeMetrics: reconcileTimeMetrics, CmdRunner: sidecarCmdExec, Kubeconf: kubeconf, CompInfo: compInfo, @@ -254,6 +258,7 @@ func Run(opts Options, runLog logr.Logger) error { CoreClient: coreClient, AppClient: kcClient, KcConfig: kcConfig, + TimeMetrics: reconcileTimeMetrics, CmdRunner: sidecarCmdExec, Kubeconf: kubeconf, CacheFolder: cacheFolderPkgRepoApps, diff --git a/pkg/app/app.go b/pkg/app/app.go index 42027dcf9..678c7cca6 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -55,9 +55,10 @@ type App struct { memoizedKubernetesVersion string memoizedKubernetesAPIs []string - log logr.Logger - opts Opts - appMetrics *metrics.AppMetrics + log logr.Logger + opts Opts + appMetrics *metrics.AppMetrics + timeMetrics *metrics.ReconcileTimeMetrics pendingStatusUpdate bool flushAllStatusUpdates bool @@ -66,11 +67,11 @@ type App struct { func NewApp(app v1alpha1.App, hooks Hooks, fetchFactory fetch.Factory, templateFactory template.Factory, - deployFactory deploy.Factory, log logr.Logger, opts Opts, appMetrics *metrics.AppMetrics, compInfo ComponentInfo) *App { + deployFactory deploy.Factory, log logr.Logger, opts Opts, appMetrics *metrics.AppMetrics, timeMetrics *metrics.ReconcileTimeMetrics, compInfo ComponentInfo) *App { return &App{app: app, appPrev: *(app.DeepCopy()), hooks: hooks, fetchFactory: fetchFactory, templateFactory: templateFactory, - deployFactory: deployFactory, log: log, opts: opts, appMetrics: appMetrics, compInfo: compInfo} + deployFactory: deployFactory, log: log, opts: opts, appMetrics: appMetrics, timeMetrics: timeMetrics, compInfo: compInfo} } func (a *App) Name() string { return a.app.Name } diff --git a/pkg/app/app_deploy.go b/pkg/app/app_deploy.go index 9bc6afcc7..c9c0df564 100644 --- a/pkg/app/app_deploy.go +++ b/pkg/app/app_deploy.go @@ -5,6 +5,7 @@ package app import ( "fmt" + "time" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" ctldep "github.com/vmware-tanzu/carvel-kapp-controller/pkg/deploy" @@ -13,6 +14,11 @@ import ( ) func (a *App) deploy(tplOutput string, changedFunc func(exec.CmdRunResult)) exec.CmdRunResult { + reconcileStartTS := time.Now() + defer func() { + a.timeMetrics.RegisterDeployTime(a.app.Kind, a.app.Name, a.app.Namespace, "", time.Since(reconcileStartTS)) + }() + err := a.blockDeletion() if err != nil { return exec.NewCmdRunResultWithErr(fmt.Errorf("Blocking for deploy: %s", err)) diff --git a/pkg/app/app_factory.go b/pkg/app/app_factory.go index 4b42fb86d..a37ec40b8 100644 --- a/pkg/app/app_factory.go +++ b/pkg/app/app_factory.go @@ -27,6 +27,7 @@ type CRDAppFactory struct { AppClient kcclient.Interface KcConfig *config.Config AppMetrics *metrics.AppMetrics + TimeMetrics *metrics.ReconcileTimeMetrics VendirConfigHook func(vendirconf.Config) vendirconf.Config KbldAllowBuild bool CmdRunner exec.CmdRunner @@ -48,7 +49,7 @@ func (f *CRDAppFactory) NewCRDApp(app *kcv1alpha1.App, log logr.Logger) *CRDApp templateFactory := template.NewFactory(f.CoreClient, fetchFactory, f.KbldAllowBuild, f.CmdRunner) deployFactory := deploy.NewFactory(f.CoreClient, f.Kubeconf, f.KcConfig, f.CmdRunner, log) - return NewCRDApp(app, log, f.AppMetrics, f.AppClient, fetchFactory, templateFactory, deployFactory, f.CompInfo, Opts{ + return NewCRDApp(app, log, f.AppMetrics, f.TimeMetrics, f.AppClient, fetchFactory, templateFactory, deployFactory, f.CompInfo, Opts{ DefaultSyncPeriod: f.KcConfig.AppDefaultSyncPeriod(), MinimumSyncPeriod: f.KcConfig.AppMinimumSyncPeriod(), }) diff --git a/pkg/app/app_fetch.go b/pkg/app/app_fetch.go index fd0ca8eab..67da673d3 100644 --- a/pkg/app/app_fetch.go +++ b/pkg/app/app_fetch.go @@ -20,6 +20,13 @@ const ( ) func (a *App) fetch(dstPath string) (string, exec.CmdRunResult) { + // update metrics with the total fetch time + reconcileStartTS := time.Now() + defer func() { + a.timeMetrics.RegisterFetchTime(a.app.Kind, a.app.Name, a.app.Namespace, "", time.Since(reconcileStartTS)) + }() + + // fetch init stage if len(a.app.Spec.Fetch) == 0 { return "", exec.NewCmdRunResultWithErr(fmt.Errorf("Expected at least one fetch option")) } diff --git a/pkg/app/app_reconcile.go b/pkg/app/app_reconcile.go index fc36ab365..03997bed2 100644 --- a/pkg/app/app_reconcile.go +++ b/pkg/app/app_reconcile.go @@ -103,6 +103,11 @@ func (a *App) reconcileDeploy() error { } func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { + reconcileStartTS := time.Now() + defer func() { + a.timeMetrics.RegisterOverallTime(a.app.Kind, a.app.Name, a.app.Namespace, "", time.Since(reconcileStartTS)) + }() + tmpDir := memdir.NewTmpDir("fetch-template-deploy") err := tmpDir.Create() diff --git a/pkg/app/app_reconcile_test.go b/pkg/app/app_reconcile_test.go index 47755dd70..25380b15b 100644 --- a/pkg/app/app_reconcile_test.go +++ b/pkg/app/app_reconcile_test.go @@ -29,7 +29,10 @@ import ( func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) { log := logf.Log.WithName("kc") - var appMetrics = metrics.NewAppMetrics() + var ( + appMetrics = metrics.NewAppMetrics() + timeMetrics = metrics.NewReconcileTimeMetrics() + ) // The url under fetch is invalid, which will cause this // app to fail before deploy. @@ -52,7 +55,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, appMetrics, kappcs, fetchFac, tmpFac, deployFac, FakeComponentInfo{}, Opts{MinimumSyncPeriod: 30 * time.Second}) + crdApp := NewCRDApp(&app, log, appMetrics, timeMetrics, kappcs, fetchFac, tmpFac, deployFac, FakeComponentInfo{}, Opts{MinimumSyncPeriod: 30 * time.Second}) _, err := crdApp.Reconcile(false) assert.Nil(t, err, "unexpected error with reconciling", err) @@ -86,7 +89,10 @@ func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) { func Test_NoInspectReconcile_IfInspectNotEnabled(t *testing.T) { log := logf.Log.WithName("kc") - var appMetrics = metrics.NewAppMetrics() + var ( + appMetrics = metrics.NewAppMetrics() + timeMetrics = metrics.NewReconcileTimeMetrics() + ) app := v1alpha1.App{ ObjectMeta: metav1.ObjectMeta{ @@ -119,7 +125,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, appMetrics, kappcs, fetchFac, tmpFac, deployFac, FakeComponentInfo{}, Opts{MinimumSyncPeriod: 30 * time.Second}) + crdApp := NewCRDApp(&app, log, appMetrics, timeMetrics, kappcs, fetchFac, tmpFac, deployFac, FakeComponentInfo{}, Opts{MinimumSyncPeriod: 30 * time.Second}) _, err := crdApp.Reconcile(false) assert.Nil(t, err, "unexpected error with reconciling", err) @@ -164,7 +170,10 @@ func Test_NoInspectReconcile_IfInspectNotEnabled(t *testing.T) { func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing.T) { log := logf.Log.WithName("kc") - var appMetrics = metrics.NewAppMetrics() + var ( + appMetrics = metrics.NewAppMetrics() + timeMetrics = metrics.NewReconcileTimeMetrics() + ) fetchInline := map[string]string{ "file.yml": `foo: #@ data.values.nothere`, @@ -191,7 +200,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, appMetrics, kappcs, fetchFac, tmpFac, deployFac, FakeComponentInfo{}, Opts{MinimumSyncPeriod: 30 * time.Second}) + crdApp := NewCRDApp(&app, log, appMetrics, timeMetrics, kappcs, fetchFac, tmpFac, deployFac, FakeComponentInfo{}, Opts{MinimumSyncPeriod: 30 * time.Second}) _, err := crdApp.Reconcile(false) assert.Nil(t, err, "Unexpected error with reconciling", err) diff --git a/pkg/app/app_template.go b/pkg/app/app_template.go index 91b335aa5..ec8b0fd0d 100644 --- a/pkg/app/app_template.go +++ b/pkg/app/app_template.go @@ -6,6 +6,7 @@ package app import ( "fmt" "strings" + "time" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/exec" @@ -13,6 +14,11 @@ import ( ) func (a *App) template(dirPath string) exec.CmdRunResult { + reconcileStartTS := time.Now() + defer func() { + a.timeMetrics.RegisterTemplateTime(a.app.Kind, a.app.Name, a.app.Namespace, "", time.Since(reconcileStartTS)) + }() + if len(a.app.Spec.Template) == 0 { return exec.NewCmdRunResultWithErr(fmt.Errorf("Expected at least one template option")) } diff --git a/pkg/app/app_template_test.go b/pkg/app/app_template_test.go index f66d89c21..465649547 100644 --- a/pkg/app/app_template_test.go +++ b/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.NewAppMetrics(), fakeInfo) + app := NewApp(appEmpty, Hooks{}, fetchFac, tmpFac, deployFac, log, Opts{}, metrics.NewAppMetrics(), metrics.NewReconcileTimeMetrics(), fakeInfo) dir, err := os.MkdirTemp("", "temp") assert.NoError(t, err) diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index fc612db86..87e5f87b9 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -64,7 +64,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{}, nil, FakeComponentInfo{}) + app := apppkg.NewApp(appWithRefs, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, apppkg.Opts{}, nil, nil, FakeComponentInfo{}) out := app.SecretRefs() assert.Truef(t, reflect.DeepEqual(out, expected), "Expected: %s\nGot: %s\n", expected, out) @@ -88,7 +88,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{}, nil, FakeComponentInfo{}) + app := apppkg.NewApp(appEmpty, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, apppkg.Opts{}, nil, nil, FakeComponentInfo{}) out := app.SecretRefs() assert.Equal(t, 0, len(out), "No SecretRefs to be returned") @@ -126,7 +126,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{}, nil, FakeComponentInfo{}) + app := apppkg.NewApp(appWithRefs, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, apppkg.Opts{}, nil, nil, FakeComponentInfo{}) out := app.ConfigMapRefs() assert.Truef(t, reflect.DeepEqual(out, expected), "Expected: %s\nGot: %s\n", expected, out) @@ -150,7 +150,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{}, nil, FakeComponentInfo{}) + app := apppkg.NewApp(appEmpty, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, apppkg.Opts{}, nil, nil, FakeComponentInfo{}) out := app.ConfigMapRefs() assert.Lenf(t, out, 0, "Expected: %s\nGot: %s\n", "No ConfigMapRefs to be returned", out) diff --git a/pkg/app/crd_app.go b/pkg/app/crd_app.go index 97132754b..b5658690f 100644 --- a/pkg/app/crd_app.go +++ b/pkg/app/crd_app.go @@ -30,7 +30,7 @@ type CRDApp struct { // NewCRDApp creates new CRD app func NewCRDApp(appModel *kcv1alpha1.App, log logr.Logger, appMetrics *metrics.AppMetrics, - appClient kcclient.Interface, fetchFactory fetch.Factory, + timeMetrics *metrics.ReconcileTimeMetrics, appClient kcclient.Interface, fetchFactory fetch.Factory, templateFactory template.Factory, deployFactory deploy.Factory, compInfo ComponentInfo, opts Opts) *CRDApp { @@ -41,7 +41,7 @@ func NewCRDApp(appModel *kcv1alpha1.App, log logr.Logger, appMetrics *metrics.Ap UnblockDeletion: crdApp.unblockDeletion, UpdateStatus: crdApp.updateStatus, WatchChanges: crdApp.watchChanges, - }, fetchFactory, templateFactory, deployFactory, log, opts, appMetrics, compInfo) + }, fetchFactory, templateFactory, deployFactory, log, opts, appMetrics, timeMetrics, compInfo) return crdApp } diff --git a/pkg/metrics/reconcile_time_metrics.go b/pkg/metrics/reconcile_time_metrics.go new file mode 100644 index 000000000..7d7a10419 --- /dev/null +++ b/pkg/metrics/reconcile_time_metrics.go @@ -0,0 +1,85 @@ +package metrics + +import ( + "github.com/prometheus/client_golang/prometheus" + "sigs.k8s.io/controller-runtime/pkg/metrics" + "time" +) + +type ReconcileTimeMetrics struct { + reconcileTimeSeconds *prometheus.GaugeVec + reconcileDeployTimeSeconds *prometheus.GaugeVec + reconcileFetchTimeSeconds *prometheus.GaugeVec + reconcileTemplateTimeSeconds *prometheus.GaugeVec +} + +func NewReconcileTimeMetrics() *ReconcileTimeMetrics { + const ( + metricNamespace = "kappctrl_reconcile_time_seconds" + resourceTypeLabel = "controller" + resourceNameLabel = "name" + firstReconcileLabel = "firstReconcile" + namespace = "namespace" + ) + return &ReconcileTimeMetrics{ + reconcileTimeSeconds: prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Namespace: metricNamespace, + Name: "reconcile_time_seconds", + Help: "Overall time taken to reconcile a CR", + }, + []string{resourceTypeLabel, resourceNameLabel, namespace, firstReconcileLabel}, + ), + reconcileFetchTimeSeconds: prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Namespace: metricNamespace, + Name: "reconcile_fetch_time_seconds", + Help: "Time taken to perform a fetch for a CR", + }, + []string{resourceTypeLabel, resourceNameLabel, namespace, firstReconcileLabel}, + ), + reconcileTemplateTimeSeconds: prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Namespace: metricNamespace, + Name: "reconcile_template_time_seconds", + Help: "Time taken to perform a templating for a CR", + }, + []string{resourceTypeLabel, resourceNameLabel, namespace, firstReconcileLabel}, + ), + reconcileDeployTimeSeconds: prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Namespace: metricNamespace, + Name: "reconcile_deploy_time_seconds", + Help: "Time taken to perform a deploy for a CR", + }, + []string{resourceTypeLabel, resourceNameLabel, namespace, firstReconcileLabel}, + ), + } +} + +func (tm *ReconcileTimeMetrics) RegisterAllMetrics() { + once.Do(func() { + metrics.Registry.MustRegister( + tm.reconcileTimeSeconds, + tm.reconcileFetchTimeSeconds, + tm.reconcileTemplateTimeSeconds, + tm.reconcileDeployTimeSeconds, + ) + }) +} + +func (tm *ReconcileTimeMetrics) RegisterOverallTime(resourceType, name, namespace, firstReconcile string, time time.Duration) { + tm.reconcileTimeSeconds.WithLabelValues(resourceType, name, namespace, firstReconcile).Set(time.Seconds()) +} + +func (tm *ReconcileTimeMetrics) RegisterFetchTime(resourceType, name, namespace, firstReconcile string, time time.Duration) { + tm.reconcileFetchTimeSeconds.WithLabelValues(resourceType, name, namespace, firstReconcile).Set(time.Seconds()) +} + +func (tm *ReconcileTimeMetrics) RegisterTemplateTime(resourceType, name, namespace, firstReconcile string, time time.Duration) { + tm.reconcileTemplateTimeSeconds.WithLabelValues(resourceType, name, namespace, firstReconcile).Set(time.Seconds()) +} + +func (tm *ReconcileTimeMetrics) RegisterDeployTime(resourceType, name, namespace, firstReconcile string, time time.Duration) { + tm.reconcileDeployTimeSeconds.WithLabelValues(resourceType, name, namespace, firstReconcile).Set(time.Seconds()) +} diff --git a/pkg/pkgrepository/app.go b/pkg/pkgrepository/app.go index a6951fade..4786e5e84 100644 --- a/pkg/pkgrepository/app.go +++ b/pkg/pkgrepository/app.go @@ -8,6 +8,7 @@ import ( "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/deploy" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/fetch" + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/reftracker" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/template" types "k8s.io/apimachinery/pkg/types" @@ -29,6 +30,8 @@ type App struct { templateFactory template.Factory deployFactory deploy.Factory + timeMetrics *metrics.ReconcileTimeMetrics + log logr.Logger pendingStatusUpdate bool @@ -36,10 +39,11 @@ type App struct { } // NewApp creates a new instance of an App based on v1alpha1.App -func NewApp(app v1alpha1.App, hooks Hooks, fetchFactory fetch.Factory, templateFactory template.Factory, deployFactory deploy.Factory, log logr.Logger, pkgRepoUID types.UID) *App { +func NewApp(app v1alpha1.App, hooks Hooks, fetchFactory fetch.Factory, templateFactory template.Factory, deployFactory deploy.Factory, + log logr.Logger, timeMetrics *metrics.ReconcileTimeMetrics, pkgRepoUID types.UID) *App { return &App{app: app, appPrev: *(app.DeepCopy()), hooks: hooks, fetchFactory: fetchFactory, templateFactory: templateFactory, - deployFactory: deployFactory, log: log, pkgRepoUID: pkgRepoUID} + deployFactory: deployFactory, log: log, timeMetrics: timeMetrics, pkgRepoUID: pkgRepoUID} } func (a *App) Name() string { return a.app.Name } diff --git a/pkg/pkgrepository/app_deploy.go b/pkg/pkgrepository/app_deploy.go index 1f5bc6131..4e2b46ebf 100644 --- a/pkg/pkgrepository/app_deploy.go +++ b/pkg/pkgrepository/app_deploy.go @@ -5,6 +5,7 @@ package pkgrepository import ( "fmt" + "time" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" ctldep "github.com/vmware-tanzu/carvel-kapp-controller/pkg/deploy" @@ -13,6 +14,11 @@ import ( ) func (a *App) deploy(tplOutput string) exec.CmdRunResult { + reconcileStartTS := time.Now() + defer func() { + a.timeMetrics.RegisterDeployTime(a.app.Kind, a.app.Name, a.app.Namespace, "", time.Since(reconcileStartTS)) + }() + err := a.blockDeletion() if err != nil { return exec.NewCmdRunResultWithErr(fmt.Errorf("Blocking for deploy: %s", err)) diff --git a/pkg/pkgrepository/app_factory.go b/pkg/pkgrepository/app_factory.go index f3695baf9..faeb71b60 100644 --- a/pkg/pkgrepository/app_factory.go +++ b/pkg/pkgrepository/app_factory.go @@ -4,6 +4,7 @@ package pkgrepository import ( + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "path/filepath" "github.com/go-logr/logr" @@ -24,6 +25,7 @@ import ( type AppFactory struct { CoreClient kubernetes.Interface AppClient kcclient.Interface + TimeMetrics *metrics.ReconcileTimeMetrics KcConfig *config.Config CmdRunner exec.CmdRunner Kubeconf *kubeconfig.Kubeconfig @@ -39,5 +41,5 @@ func (f *AppFactory) NewCRDPackageRepo(app *kcv1alpha1.App, pkgr *pkgv1alpha1.Pa fetchFactory := fetch.NewFactory(f.CoreClient, vendirOpts, f.CmdRunner) templateFactory := template.NewFactory(f.CoreClient, fetchFactory, false, f.CmdRunner) deployFactory := deploy.NewFactory(f.CoreClient, f.Kubeconf, nil, f.CmdRunner, log) - return NewCRDApp(app, pkgr, log, f.AppClient, fetchFactory, templateFactory, deployFactory) + return NewCRDApp(app, pkgr, log, f.AppClient, f.TimeMetrics, fetchFactory, templateFactory, deployFactory) } diff --git a/pkg/pkgrepository/app_fetch.go b/pkg/pkgrepository/app_fetch.go index 86839d787..495f3b1b5 100644 --- a/pkg/pkgrepository/app_fetch.go +++ b/pkg/pkgrepository/app_fetch.go @@ -14,6 +14,10 @@ import ( ) func (a *App) fetch(dstPath string) (string, exec.CmdRunResult) { + reconcileStartTS := time.Now() + defer func() { + a.timeMetrics.RegisterFetchTime(a.app.Kind, a.app.Name, a.app.Namespace, "", time.Since(reconcileStartTS)) + }() if len(a.app.Spec.Fetch) == 0 { return "", exec.NewCmdRunResultWithErr(fmt.Errorf("Expected at least one fetch option")) } diff --git a/pkg/pkgrepository/app_reconcile.go b/pkg/pkgrepository/app_reconcile.go index 288f45d05..bee2bb034 100644 --- a/pkg/pkgrepository/app_reconcile.go +++ b/pkg/pkgrepository/app_reconcile.go @@ -90,6 +90,11 @@ func (a *App) reconcileDeploy() error { } func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { + reconcileStartTS := time.Now() + defer func() { + a.timeMetrics.RegisterOverallTime(a.app.Kind, a.app.Name, a.app.Namespace, "", time.Since(reconcileStartTS)) + }() + tmpDir := memdir.NewTmpDir("fetch-template-deploy") err := tmpDir.Create() diff --git a/pkg/pkgrepository/app_reconcile_test.go b/pkg/pkgrepository/app_reconcile_test.go index 9962d1820..e9268c8b1 100644 --- a/pkg/pkgrepository/app_reconcile_test.go +++ b/pkg/pkgrepository/app_reconcile_test.go @@ -4,6 +4,7 @@ package pkgrepository import ( + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "testing" "github.com/stretchr/testify/assert" @@ -24,6 +25,7 @@ import ( func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) { log := logf.Log.WithName("kc") + var timeMetrics = metrics.NewReconcileTimeMetrics() // The url under fetch is invalid, which will cause this // app to fail before deploy. @@ -46,7 +48,7 @@ func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) { deployFac := deploy.NewFactory(k8scs, kubeconfig.NewKubeconfig(k8scs, log), nil, exec.NewPlainCmdRunner(), log) pkgr := v1alpha12.PackageRepository{} - crdApp := NewCRDApp(&app, &pkgr, log, kappcs, fetchFac, tmpFac, deployFac) + crdApp := NewCRDApp(&app, &pkgr, log, kappcs, timeMetrics, fetchFac, tmpFac, deployFac) _, err := crdApp.Reconcile(false) if err != nil { t.Fatalf("Unexpected error with reconciling: %v", err) @@ -78,6 +80,7 @@ func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) { func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing.T) { log := logf.Log.WithName("kc") + var timeMetrics = metrics.NewReconcileTimeMetrics() fetchInline := map[string]string{ "packages/file.yml": `foo: #@ data.values.nothere`, @@ -102,7 +105,7 @@ func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing. deployFac := deploy.NewFactory(k8scs, kubeconfig.NewKubeconfig(k8scs, log), nil, exec.NewPlainCmdRunner(), log) pkgr := v1alpha12.PackageRepository{} - crdApp := NewCRDApp(&app, &pkgr, log, kappcs, fetchFac, tmpFac, deployFac) + crdApp := NewCRDApp(&app, &pkgr, log, kappcs, timeMetrics, fetchFac, tmpFac, deployFac) _, err := crdApp.Reconcile(false) if err != nil { t.Fatalf("Unexpected error with reconciling: %v", err) @@ -135,6 +138,7 @@ func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing. func TestInvalidPackageRepositoryFormat(t *testing.T) { log := logf.Log.WithName("kc") + var timeMetrics = metrics.NewReconcileTimeMetrics() fetchInline := map[string]string{ "file.yml": `foo: hi`, @@ -158,7 +162,7 @@ func TestInvalidPackageRepositoryFormat(t *testing.T) { deployFac := deploy.NewFactory(k8scs, kubeconfig.NewKubeconfig(k8scs, log), nil, exec.NewPlainCmdRunner(), log) pkgr := v1alpha12.PackageRepository{} - crdApp := NewCRDApp(&app, &pkgr, log, kappcs, fetchFac, tmpFac, deployFac) + crdApp := NewCRDApp(&app, &pkgr, log, kappcs, timeMetrics, fetchFac, tmpFac, deployFac) _, err := crdApp.Reconcile(false) if err != nil { t.Fatalf("Unexpected error with reconciling: %v", err) diff --git a/pkg/pkgrepository/app_template.go b/pkg/pkgrepository/app_template.go index 241cee7ee..f98761764 100644 --- a/pkg/pkgrepository/app_template.go +++ b/pkg/pkgrepository/app_template.go @@ -11,6 +11,7 @@ import ( "os" "path/filepath" "strings" + "time" kcv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" datapackagingv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/apis/datapackaging/v1alpha1" @@ -25,6 +26,11 @@ import ( var errInvalidPackageRepo = exec.NewCmdRunResultWithErr(fmt.Errorf("Invalid package repository content: must contain 'packages/' directory but did not")) func (a *App) template(dirPath string) exec.CmdRunResult { + reconcileStartTS := time.Now() + defer func() { + a.timeMetrics.RegisterTemplateTime(a.app.Kind, a.app.Name, a.app.Namespace, "", time.Since(reconcileStartTS)) + }() + fileInfo, err := os.Lstat(filepath.Join(dirPath, "packages")) if err != nil { if os.IsNotExist(err) { diff --git a/pkg/pkgrepository/crd_app.go b/pkg/pkgrepository/crd_app.go index c6bdbabc8..5f02ae5d7 100644 --- a/pkg/pkgrepository/crd_app.go +++ b/pkg/pkgrepository/crd_app.go @@ -6,6 +6,7 @@ package pkgrepository import ( "context" "fmt" + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "github.com/go-logr/logr" kcv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" @@ -20,15 +21,16 @@ import ( ) type CRDApp struct { - app *App - appModel *kcv1alpha1.App - pkgrModel *pkgingv1alpha1.PackageRepository - log logr.Logger - appClient kcclient.Interface + app *App + appModel *kcv1alpha1.App + pkgrModel *pkgingv1alpha1.PackageRepository + timeMetrics *metrics.ReconcileTimeMetrics + log logr.Logger + appClient kcclient.Interface } func NewCRDApp(appModel *kcv1alpha1.App, packageRepo *pkgingv1alpha1.PackageRepository, log logr.Logger, - appClient kcclient.Interface, fetchFactory fetch.Factory, + appClient kcclient.Interface, timeMetrics *metrics.ReconcileTimeMetrics, fetchFactory fetch.Factory, templateFactory template.Factory, deployFactory deploy.Factory) *CRDApp { crdApp := &CRDApp{appModel: appModel, pkgrModel: packageRepo, log: log, appClient: appClient} @@ -37,7 +39,7 @@ func NewCRDApp(appModel *kcv1alpha1.App, packageRepo *pkgingv1alpha1.PackageRepo BlockDeletion: crdApp.blockDeletion, UnblockDeletion: crdApp.unblockDeletion, UpdateStatus: crdApp.updateStatus, - }, fetchFactory, templateFactory, deployFactory, log, packageRepo.UID) + }, fetchFactory, templateFactory, deployFactory, log, timeMetrics, packageRepo.UID) return crdApp } From 09d6cd0567b96c8dc5aa9076d391792d806dadd0 Mon Sep 17 00:00:00 2001 From: sethiyash Date: Wed, 6 Dec 2023 00:50:59 +0530 Subject: [PATCH 2/9] gaugevec metrics for pkg repo, app, pkg install Signed-off-by: sethiyash --- cmd/controller/run.go | 4 +++- config/config/agg-api.yml | 5 +++++ config/config/deployment.yml | 3 +++ go.mod | 2 +- pkg/app/app.go | 1 + pkg/app/app_deploy.go | 7 ------- pkg/app/app_reconcile.go | 19 ++++++++++++++++- pkg/app/app_template.go | 9 +------- pkg/metrics/app_metrics.go | 9 ++++++++ pkg/metrics/reconcile_time_metrics.go | 7 ++++++- pkg/packageinstall/app.go | 5 ++--- pkg/packageinstall/packageinstall.go | 13 ++++++++++-- .../packageinstall_deletion_test.go | 3 ++- .../packageinstall_downgrade_test.go | 10 +++++---- pkg/packageinstall/packageinstall_test.go | 13 ++++++++---- pkg/packageinstall/reconciler.go | 8 ++++--- pkg/pkgrepository/app.go | 6 ++++-- pkg/pkgrepository/app_factory.go | 3 ++- pkg/pkgrepository/app_fetch.go | 4 ---- pkg/pkgrepository/app_reconcile.go | 21 ++++++++++++++++++- pkg/pkgrepository/app_reconcile_test.go | 9 +++++--- pkg/pkgrepository/app_template.go | 6 ------ pkg/pkgrepository/crd_app.go | 4 ++-- 23 files changed, 116 insertions(+), 55 deletions(-) diff --git a/cmd/controller/run.go b/cmd/controller/run.go index f16955f1e..433f7ea8b 100644 --- a/cmd/controller/run.go +++ b/cmd/controller/run.go @@ -231,7 +231,8 @@ func Run(opts Options, runLog logr.Logger) error { pkgToPkgInstallHandler := pkginstall.NewPackageInstallVersionHandler( kcClient, opts.PackagingGlobalNS, runLog.WithName("handler")) - reconciler := pkginstall.NewReconciler(kcClient, pkgClient, coreClient, pkgToPkgInstallHandler, runLog.WithName("pkgi"), compInfo, kcConfig) + reconciler := pkginstall.NewReconciler(kcClient, pkgClient, coreClient, pkgToPkgInstallHandler, + runLog.WithName("pkgi"), compInfo, kcConfig, reconcileTimeMetrics) ctrl, err := controller.New("pkgi", mgr, controller.Options{ Reconciler: reconciler, @@ -258,6 +259,7 @@ func Run(opts Options, runLog logr.Logger) error { CoreClient: coreClient, AppClient: kcClient, KcConfig: kcConfig, + AppMetrics: appMetrics, TimeMetrics: reconcileTimeMetrics, CmdRunner: sidecarCmdExec, Kubeconf: kubeconf, diff --git a/config/config/agg-api.yml b/config/config/agg-api.yml index 5a0912822..181b9708c 100644 --- a/config/config/agg-api.yml +++ b/config/config/agg-api.yml @@ -23,5 +23,10 @@ spec: - port: 443 protocol: TCP targetPort: api + name: main + - port: 8080 + protocol: TCP + targetPort: metrics + name: metrics selector: app: kapp-controller diff --git a/config/config/deployment.yml b/config/config/deployment.yml index 11b138032..6c56e3c7f 100644 --- a/config/config/deployment.yml +++ b/config/config/deployment.yml @@ -52,6 +52,9 @@ spec: - containerPort: #@ data.values.apiPort name: api protocol: TCP + - containerPort: 8080 + name: metrics + protocol: TCP securityContext: allowPrivilegeEscalation: false readOnlyRootFilesystem: true diff --git a/go.mod b/go.mod index 677508826..00f3d49eb 100644 --- a/go.mod +++ b/go.mod @@ -30,6 +30,7 @@ require ( github.com/cppforlife/go-cli-ui v0.0.0-20220425131040-94f26b16bc14 github.com/go-logr/logr v1.2.4 github.com/k14s/semver/v4 v4.0.1-0.20210701191048-266d47ac6115 + github.com/prometheus/client_model v0.4.0 github.com/spf13/cobra v1.6.1 golang.org/x/sync v0.2.0 gopkg.in/yaml.v2 v2.4.0 @@ -86,7 +87,6 @@ require ( github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/prometheus/client_model v0.4.0 // indirect github.com/prometheus/common v0.42.0 // indirect github.com/prometheus/procfs v0.9.0 // indirect github.com/spf13/pflag v1.0.5 // indirect diff --git a/pkg/app/app.go b/pkg/app/app.go index 678c7cca6..8e8fb586a 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -60,6 +60,7 @@ type App struct { appMetrics *metrics.AppMetrics timeMetrics *metrics.ReconcileTimeMetrics + isFirstReconcile string pendingStatusUpdate bool flushAllStatusUpdates bool metadata *deploy.Meta diff --git a/pkg/app/app_deploy.go b/pkg/app/app_deploy.go index c9c0df564..33e0dea42 100644 --- a/pkg/app/app_deploy.go +++ b/pkg/app/app_deploy.go @@ -5,8 +5,6 @@ package app import ( "fmt" - "time" - "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" ctldep "github.com/vmware-tanzu/carvel-kapp-controller/pkg/deploy" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/exec" @@ -14,11 +12,6 @@ import ( ) func (a *App) deploy(tplOutput string, changedFunc func(exec.CmdRunResult)) exec.CmdRunResult { - reconcileStartTS := time.Now() - defer func() { - a.timeMetrics.RegisterDeployTime(a.app.Kind, a.app.Name, a.app.Namespace, "", time.Since(reconcileStartTS)) - }() - err := a.blockDeletion() if err != nil { return exec.NewCmdRunResultWithErr(fmt.Errorf("Blocking for deploy: %s", err)) diff --git a/pkg/app/app_reconcile.go b/pkg/app/app_reconcile.go index 03997bed2..030690ba8 100644 --- a/pkg/app/app_reconcile.go +++ b/pkg/app/app_reconcile.go @@ -104,8 +104,14 @@ func (a *App) reconcileDeploy() error { func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { reconcileStartTS := time.Now() + a.isFirstReconcile = "false" + if a.appMetrics.GetReconcileAttemptCounterValue(a.app.Name, a.app.Namespace) == 1 { + a.isFirstReconcile = "true" + } + defer func() { - a.timeMetrics.RegisterOverallTime(a.app.Kind, a.app.Name, a.app.Namespace, "", time.Since(reconcileStartTS)) + a.timeMetrics.RegisterOverallTime(a.app.Kind, a.app.Name, a.app.Namespace, a.isFirstReconcile, + time.Since(reconcileStartTS)) }() tmpDir := memdir.NewTmpDir("fetch-template-deploy") @@ -134,6 +140,9 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } + a.timeMetrics.RegisterFetchTime(a.app.Kind, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.app.Status.Fetch.UpdatedAt.Sub(a.app.Status.Fetch.StartedAt.Time)) + err := a.updateStatus("marking fetch completed") if err != nil { return exec.NewCmdRunResultWithErr(err) @@ -144,6 +153,8 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { } } + templateStartTime := time.Now() + tplResult := a.template(assetsPath) a.app.Status.Template = &v1alpha1.AppStatusTemplate{ @@ -153,6 +164,9 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } + a.timeMetrics.RegisterTemplateTime(a.app.Kind, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.app.Status.Template.UpdatedAt.Sub(templateStartTime)) + err = a.updateStatus("marking template completed") if err != nil { return exec.NewCmdRunResultWithErr(err) @@ -201,6 +215,9 @@ func (a *App) updateLastDeploy(result exec.CmdRunResult) exec.CmdRunResult { }, } + a.timeMetrics.RegisterDeployTime(a.app.Kind, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.Status().Deploy.UpdatedAt.Sub(a.Status().Deploy.StartedAt.Time)) + return result } diff --git a/pkg/app/app_template.go b/pkg/app/app_template.go index ec8b0fd0d..6c296a694 100644 --- a/pkg/app/app_template.go +++ b/pkg/app/app_template.go @@ -5,20 +5,13 @@ package app import ( "fmt" - "strings" - "time" - "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/exec" ctltpl "github.com/vmware-tanzu/carvel-kapp-controller/pkg/template" + "strings" ) func (a *App) template(dirPath string) exec.CmdRunResult { - reconcileStartTS := time.Now() - defer func() { - a.timeMetrics.RegisterTemplateTime(a.app.Kind, a.app.Name, a.app.Namespace, "", time.Since(reconcileStartTS)) - }() - if len(a.app.Spec.Template) == 0 { return exec.NewCmdRunResultWithErr(fmt.Errorf("Expected at least one template option")) } diff --git a/pkg/metrics/app_metrics.go b/pkg/metrics/app_metrics.go index 434bae711..771e8b7ef 100644 --- a/pkg/metrics/app_metrics.go +++ b/pkg/metrics/app_metrics.go @@ -7,6 +7,7 @@ import ( "sync" "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" "sigs.k8s.io/controller-runtime/pkg/metrics" ) @@ -131,3 +132,11 @@ func (am *AppMetrics) RegisterReconcileDeleteAttempt(appName string, namespace s func (am *AppMetrics) RegisterReconcileDeleteFailed(appName string, namespace string) { am.reconcileDeleteFailedTotal.WithLabelValues(appName, namespace).Inc() } + +func (am *AppMetrics) GetReconcileAttemptCounterValue(appName, namespace string) int64 { + var m = &dto.Metric{} + if err := am.reconcileAttemptTotal.WithLabelValues(appName, namespace).Write(m); err != nil { + return 0 + } + return int64(m.Counter.GetValue()) +} diff --git a/pkg/metrics/reconcile_time_metrics.go b/pkg/metrics/reconcile_time_metrics.go index 7d7a10419..5f5762030 100644 --- a/pkg/metrics/reconcile_time_metrics.go +++ b/pkg/metrics/reconcile_time_metrics.go @@ -3,6 +3,7 @@ package metrics import ( "github.com/prometheus/client_golang/prometheus" "sigs.k8s.io/controller-runtime/pkg/metrics" + "sync" "time" ) @@ -13,6 +14,10 @@ type ReconcileTimeMetrics struct { reconcileTemplateTimeSeconds *prometheus.GaugeVec } +var ( + timeMetricsOnce sync.Once +) + func NewReconcileTimeMetrics() *ReconcileTimeMetrics { const ( metricNamespace = "kappctrl_reconcile_time_seconds" @@ -58,7 +63,7 @@ func NewReconcileTimeMetrics() *ReconcileTimeMetrics { } func (tm *ReconcileTimeMetrics) RegisterAllMetrics() { - once.Do(func() { + timeMetricsOnce.Do(func() { metrics.Registry.MustRegister( tm.reconcileTimeSeconds, tm.reconcileFetchTimeSeconds, diff --git a/pkg/packageinstall/app.go b/pkg/packageinstall/app.go index 23ce547d7..23ce2c658 100644 --- a/pkg/packageinstall/app.go +++ b/pkg/packageinstall/app.go @@ -5,9 +5,6 @@ package packageinstall import ( "fmt" - "sort" - "strings" - "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" kcv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" pkgingv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/packaging/v1alpha1" @@ -15,6 +12,8 @@ import ( "github.com/vmware-tanzu/carvel-kapp-controller/pkg/client/clientset/versioned/scheme" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sort" + "strings" ) const ( diff --git a/pkg/packageinstall/packageinstall.go b/pkg/packageinstall/packageinstall.go index 9745f8498..5f0d89443 100644 --- a/pkg/packageinstall/packageinstall.go +++ b/pkg/packageinstall/packageinstall.go @@ -6,6 +6,7 @@ package packageinstall import ( "context" "fmt" + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "time" "github.com/go-logr/logr" @@ -54,6 +55,8 @@ type PackageInstallCR struct { coreClient kubernetes.Interface compInfo ComponentInfo opts Opts + + timeMetrics *metrics.ReconcileTimeMetrics } // nolint: revive @@ -62,10 +65,11 @@ type Opts struct { } func NewPackageInstallCR(model *pkgingv1alpha1.PackageInstall, log logr.Logger, - kcclient kcclient.Interface, pkgclient pkgclient.Interface, coreClient kubernetes.Interface, compInfo ComponentInfo, opts Opts) *PackageInstallCR { + kcclient kcclient.Interface, pkgclient pkgclient.Interface, coreClient kubernetes.Interface, + compInfo ComponentInfo, opts Opts, timeMetrics *metrics.ReconcileTimeMetrics) *PackageInstallCR { return &PackageInstallCR{model: model, unmodifiedModel: model.DeepCopy(), log: log, - kcclient: kcclient, pkgclient: pkgclient, coreClient: coreClient, compInfo: compInfo, opts: opts} + kcclient: kcclient, pkgclient: pkgclient, coreClient: coreClient, compInfo: compInfo, opts: opts, timeMetrics: timeMetrics} } func (pi *PackageInstallCR) Reconcile() (reconcile.Result, error) { @@ -101,6 +105,11 @@ func (pi *PackageInstallCR) Reconcile() (reconcile.Result, error) { func (pi *PackageInstallCR) reconcile(modelStatus *reconciler.Status) (reconcile.Result, error) { pi.log.Info("Reconciling") + reconcileStartTS := time.Now() + defer func() { + pi.timeMetrics.RegisterOverallTime("pkgi", pi.model.Name, pi.model.Namespace, "", time.Since(reconcileStartTS)) + }() + err := pi.blockDeletion() if err != nil { return reconcile.Result{Requeue: true}, err diff --git a/pkg/packageinstall/packageinstall_deletion_test.go b/pkg/packageinstall/packageinstall_deletion_test.go index 0ef39fc80..0cdfd9b0c 100644 --- a/pkg/packageinstall/packageinstall_deletion_test.go +++ b/pkg/packageinstall/packageinstall_deletion_test.go @@ -4,6 +4,7 @@ package packageinstall import ( + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "testing" "github.com/k14s/semver/v4" @@ -65,7 +66,7 @@ func Test_PackageInstallDeletion(t *testing.T) { appClient := fakekappctrl.NewSimpleClientset(pkgInstall, existingApp) coreClient := fake.NewSimpleClientset() - ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, coreClient, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}) + ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, coreClient, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, &metrics.ReconcileTimeMetrics{}) _, err := ip.Reconcile() assert.Nil(t, err) diff --git a/pkg/packageinstall/packageinstall_downgrade_test.go b/pkg/packageinstall/packageinstall_downgrade_test.go index 7810d6f32..776f106e2 100644 --- a/pkg/packageinstall/packageinstall_downgrade_test.go +++ b/pkg/packageinstall/packageinstall_downgrade_test.go @@ -4,6 +4,7 @@ package packageinstall import ( + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "testing" "github.com/k14s/semver/v4" @@ -25,6 +26,7 @@ import ( func Test_PackageInstallVersionDowngrades(t *testing.T) { log := logf.Log.WithName("kc") + var reconcileTimeMetrics = metrics.NewReconcileTimeMetrics() pkg1 := datapkgingv1alpha1.Package{ ObjectMeta: metav1.ObjectMeta{ @@ -98,7 +100,7 @@ func Test_PackageInstallVersionDowngrades(t *testing.T) { GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}) + ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileTimeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) @@ -147,7 +149,7 @@ func Test_PackageInstallVersionDowngrades(t *testing.T) { GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}) + ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileTimeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) @@ -196,7 +198,7 @@ func Test_PackageInstallVersionDowngrades(t *testing.T) { GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}) + ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileTimeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) @@ -251,7 +253,7 @@ func Test_PackageInstallVersionDowngrades(t *testing.T) { GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}) + ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileTimeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) diff --git a/pkg/packageinstall/packageinstall_test.go b/pkg/packageinstall/packageinstall_test.go index 174eb302b..0fc71cb75 100644 --- a/pkg/packageinstall/packageinstall_test.go +++ b/pkg/packageinstall/packageinstall_test.go @@ -5,6 +5,7 @@ package packageinstall import ( "fmt" + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "reflect" "testing" @@ -353,6 +354,7 @@ func Test_PackageRefUsesName(t *testing.T) { } func Test_PlaceHolderSecretCreated_WhenPackageHasNoSecretRef(t *testing.T) { + var timeMetrics = metrics.NewReconcileTimeMetrics() pkg := datapkgingv1alpha1.Package{ ObjectMeta: metav1.ObjectMeta{ Name: "expected-pkg", @@ -402,7 +404,7 @@ func Test_PlaceHolderSecretCreated_WhenPackageHasNoSecretRef(t *testing.T) { GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}) + ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, timeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) @@ -428,6 +430,7 @@ func Test_PlaceHolderSecretCreated_WhenPackageHasNoSecretRef(t *testing.T) { } func Test_PlaceHolderSecretsCreated_WhenPackageHasMultipleFetchStages(t *testing.T) { + var timeMetrics = metrics.NewReconcileTimeMetrics() pkg := datapkgingv1alpha1.Package{ ObjectMeta: metav1.ObjectMeta{ Name: "expected-pkg", @@ -481,7 +484,7 @@ func Test_PlaceHolderSecretsCreated_WhenPackageHasMultipleFetchStages(t *testing GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}) + ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, timeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) @@ -517,6 +520,7 @@ func Test_PlaceHolderSecretsCreated_WhenPackageHasMultipleFetchStages(t *testing } func Test_PlaceHolderSecretsNotCreated_WhenFetchStagesHaveSecrets(t *testing.T) { + var timeMetrics = metrics.NewReconcileTimeMetrics() pkg := datapkgingv1alpha1.Package{ ObjectMeta: metav1.ObjectMeta{ Name: "expected-pkg", @@ -571,7 +575,7 @@ func Test_PlaceHolderSecretsNotCreated_WhenFetchStagesHaveSecrets(t *testing.T) GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}) + ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, timeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) @@ -591,6 +595,7 @@ func Test_PlaceHolderSecretsNotCreated_WhenFetchStagesHaveSecrets(t *testing.T) } func Test_PlaceHolderSecretCreated_WhenPackageInstallUpdated(t *testing.T) { + var timeMetrics = metrics.NewReconcileTimeMetrics() appSpec := v1alpha1.AppSpec{ Fetch: []v1alpha1.AppFetch{ { @@ -648,7 +653,7 @@ func Test_PlaceHolderSecretCreated_WhenPackageInstallUpdated(t *testing.T) { fakekctrl := fakekappctrl.NewSimpleClientset(model, existingApp) fakek8s := fake.NewSimpleClientset() - ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}) + ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, timeMetrics) // mock the kubernetes server version fakeDiscovery, _ := fakek8s.Discovery().(*fakediscovery.FakeDiscovery) diff --git a/pkg/packageinstall/reconciler.go b/pkg/packageinstall/reconciler.go index 451106abd..f1ba01e3e 100644 --- a/pkg/packageinstall/reconciler.go +++ b/pkg/packageinstall/reconciler.go @@ -6,7 +6,6 @@ package packageinstall import ( "context" "fmt" - "github.com/go-logr/logr" kappctrlv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" pkgingv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/packaging/v1alpha1" @@ -14,6 +13,7 @@ import ( pkgclient "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/client/clientset/versioned" kcclient "github.com/vmware-tanzu/carvel-kapp-controller/pkg/client/clientset/versioned" kcconfig "github.com/vmware-tanzu/carvel-kapp-controller/pkg/config" + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" @@ -33,12 +33,13 @@ type Reconciler struct { compInfo ComponentInfo log logr.Logger kcConfig *kcconfig.Config + timeMetrics *metrics.ReconcileTimeMetrics } // NewReconciler is the constructor for the Reconciler struct func NewReconciler(kcClient kcclient.Interface, pkgClient pkgclient.Interface, coreClient kubernetes.Interface, pkgToPkgInstallHandler *PackageInstallVersionHandler, - log logr.Logger, compInfo ComponentInfo, kcConfig *kcconfig.Config) *Reconciler { + log logr.Logger, compInfo ComponentInfo, kcConfig *kcconfig.Config, timeMetrics *metrics.ReconcileTimeMetrics) *Reconciler { return &Reconciler{kcClient: kcClient, pkgClient: pkgClient, @@ -47,6 +48,7 @@ func NewReconciler(kcClient kcclient.Interface, pkgClient pkgclient.Interface, compInfo: compInfo, log: log, kcConfig: kcConfig, + timeMetrics: timeMetrics, } } @@ -88,5 +90,5 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) ( return reconcile.Result{}, err } - return NewPackageInstallCR(existingPkgInstall, log, r.kcClient, r.pkgClient, r.coreClient, r.compInfo, Opts{DefaultSyncPeriod: r.kcConfig.PackageInstallDefaultSyncPeriod()}).Reconcile() + return NewPackageInstallCR(existingPkgInstall, log, r.kcClient, r.pkgClient, r.coreClient, r.compInfo, Opts{DefaultSyncPeriod: r.kcConfig.PackageInstallDefaultSyncPeriod()}, r.timeMetrics).Reconcile() } diff --git a/pkg/pkgrepository/app.go b/pkg/pkgrepository/app.go index 4786e5e84..3a03920be 100644 --- a/pkg/pkgrepository/app.go +++ b/pkg/pkgrepository/app.go @@ -31,19 +31,21 @@ type App struct { deployFactory deploy.Factory timeMetrics *metrics.ReconcileTimeMetrics + appMetrics *metrics.AppMetrics log logr.Logger + isFirstReconcile string pendingStatusUpdate bool flushAllStatusUpdates bool } // NewApp creates a new instance of an App based on v1alpha1.App func NewApp(app v1alpha1.App, hooks Hooks, fetchFactory fetch.Factory, templateFactory template.Factory, deployFactory deploy.Factory, - log logr.Logger, timeMetrics *metrics.ReconcileTimeMetrics, pkgRepoUID types.UID) *App { + log logr.Logger, appMetrics *metrics.AppMetrics, timeMetrics *metrics.ReconcileTimeMetrics, pkgRepoUID types.UID) *App { return &App{app: app, appPrev: *(app.DeepCopy()), hooks: hooks, fetchFactory: fetchFactory, templateFactory: templateFactory, - deployFactory: deployFactory, log: log, timeMetrics: timeMetrics, pkgRepoUID: pkgRepoUID} + deployFactory: deployFactory, log: log, appMetrics: appMetrics, timeMetrics: timeMetrics, pkgRepoUID: pkgRepoUID} } func (a *App) Name() string { return a.app.Name } diff --git a/pkg/pkgrepository/app_factory.go b/pkg/pkgrepository/app_factory.go index faeb71b60..379d1c849 100644 --- a/pkg/pkgrepository/app_factory.go +++ b/pkg/pkgrepository/app_factory.go @@ -26,6 +26,7 @@ type AppFactory struct { CoreClient kubernetes.Interface AppClient kcclient.Interface TimeMetrics *metrics.ReconcileTimeMetrics + AppMetrics *metrics.AppMetrics KcConfig *config.Config CmdRunner exec.CmdRunner Kubeconf *kubeconfig.Kubeconfig @@ -41,5 +42,5 @@ func (f *AppFactory) NewCRDPackageRepo(app *kcv1alpha1.App, pkgr *pkgv1alpha1.Pa fetchFactory := fetch.NewFactory(f.CoreClient, vendirOpts, f.CmdRunner) templateFactory := template.NewFactory(f.CoreClient, fetchFactory, false, f.CmdRunner) deployFactory := deploy.NewFactory(f.CoreClient, f.Kubeconf, nil, f.CmdRunner, log) - return NewCRDApp(app, pkgr, log, f.AppClient, f.TimeMetrics, fetchFactory, templateFactory, deployFactory) + return NewCRDApp(app, pkgr, log, f.AppClient, f.TimeMetrics, f.AppMetrics, fetchFactory, templateFactory, deployFactory) } diff --git a/pkg/pkgrepository/app_fetch.go b/pkg/pkgrepository/app_fetch.go index 495f3b1b5..86839d787 100644 --- a/pkg/pkgrepository/app_fetch.go +++ b/pkg/pkgrepository/app_fetch.go @@ -14,10 +14,6 @@ import ( ) func (a *App) fetch(dstPath string) (string, exec.CmdRunResult) { - reconcileStartTS := time.Now() - defer func() { - a.timeMetrics.RegisterFetchTime(a.app.Kind, a.app.Name, a.app.Namespace, "", time.Since(reconcileStartTS)) - }() if len(a.app.Spec.Fetch) == 0 { return "", exec.NewCmdRunResultWithErr(fmt.Errorf("Expected at least one fetch option")) } diff --git a/pkg/pkgrepository/app_reconcile.go b/pkg/pkgrepository/app_reconcile.go index bee2bb034..66461214e 100644 --- a/pkg/pkgrepository/app_reconcile.go +++ b/pkg/pkgrepository/app_reconcile.go @@ -21,6 +21,8 @@ func (a *App) Reconcile(force bool) (reconcile.Result, error) { var err error + a.appMetrics.InitMetrics(a.Name(), a.Namespace()) + switch { case a.app.Spec.Paused: a.log.Info("PackageRepository is paused, not reconciling") @@ -91,8 +93,12 @@ func (a *App) reconcileDeploy() error { func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { reconcileStartTS := time.Now() + a.isFirstReconcile = "false" + if a.appMetrics.GetReconcileAttemptCounterValue(a.app.Name, a.app.Namespace) == 1 { + a.isFirstReconcile = "true" + } defer func() { - a.timeMetrics.RegisterOverallTime(a.app.Kind, a.app.Name, a.app.Namespace, "", time.Since(reconcileStartTS)) + a.timeMetrics.RegisterOverallTime(a.app.Kind, a.app.Name, a.app.Namespace, a.isFirstReconcile, time.Since(reconcileStartTS)) }() tmpDir := memdir.NewTmpDir("fetch-template-deploy") @@ -121,6 +127,9 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } + a.timeMetrics.RegisterFetchTime("pkgr", a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.app.Status.Fetch.UpdatedAt.Sub(a.app.Status.Fetch.StartedAt.Time)) + err := a.updateStatus("marking fetch completed") if err != nil { return exec.NewCmdRunResultWithErr(err) @@ -131,6 +140,8 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { } } + templateStartTime := time.Now() + tplResult := a.template(assetsPath) a.app.Status.Template = &v1alpha1.AppStatusTemplate{ @@ -140,6 +151,9 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } + a.timeMetrics.RegisterTemplateTime("pkgr", a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.app.Status.Template.UpdatedAt.Sub(templateStartTime)) + err = a.updateStatus("marking template completed") if err != nil { return exec.NewCmdRunResultWithErr(err) @@ -167,6 +181,9 @@ func (a *App) updateLastDeploy(result exec.CmdRunResult) exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } + a.timeMetrics.RegisterDeployTime("pkgr", a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.Status().Deploy.UpdatedAt.Sub(a.Status().Deploy.StartedAt.Time)) + a.updateStatus("marking last deploy") return result @@ -198,6 +215,8 @@ func (a *App) setReconciling() { Status: corev1.ConditionTrue, }) + a.appMetrics.RegisterReconcileAttempt(a.app.Name, a.app.Namespace) + a.app.Status.FriendlyDescription = "Reconciling" } diff --git a/pkg/pkgrepository/app_reconcile_test.go b/pkg/pkgrepository/app_reconcile_test.go index e9268c8b1..78ebf7726 100644 --- a/pkg/pkgrepository/app_reconcile_test.go +++ b/pkg/pkgrepository/app_reconcile_test.go @@ -26,6 +26,7 @@ import ( func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) { log := logf.Log.WithName("kc") var timeMetrics = metrics.NewReconcileTimeMetrics() + var appMetrics = metrics.NewAppMetrics() // The url under fetch is invalid, which will cause this // app to fail before deploy. @@ -48,7 +49,7 @@ func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) { deployFac := deploy.NewFactory(k8scs, kubeconfig.NewKubeconfig(k8scs, log), nil, exec.NewPlainCmdRunner(), log) pkgr := v1alpha12.PackageRepository{} - crdApp := NewCRDApp(&app, &pkgr, log, kappcs, timeMetrics, fetchFac, tmpFac, deployFac) + crdApp := NewCRDApp(&app, &pkgr, log, kappcs, timeMetrics, appMetrics, fetchFac, tmpFac, deployFac) _, err := crdApp.Reconcile(false) if err != nil { t.Fatalf("Unexpected error with reconciling: %v", err) @@ -81,6 +82,7 @@ func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) { func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing.T) { log := logf.Log.WithName("kc") var timeMetrics = metrics.NewReconcileTimeMetrics() + var appMetrics = metrics.NewAppMetrics() fetchInline := map[string]string{ "packages/file.yml": `foo: #@ data.values.nothere`, @@ -105,7 +107,7 @@ func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing. deployFac := deploy.NewFactory(k8scs, kubeconfig.NewKubeconfig(k8scs, log), nil, exec.NewPlainCmdRunner(), log) pkgr := v1alpha12.PackageRepository{} - crdApp := NewCRDApp(&app, &pkgr, log, kappcs, timeMetrics, fetchFac, tmpFac, deployFac) + crdApp := NewCRDApp(&app, &pkgr, log, kappcs, timeMetrics, appMetrics, fetchFac, tmpFac, deployFac) _, err := crdApp.Reconcile(false) if err != nil { t.Fatalf("Unexpected error with reconciling: %v", err) @@ -139,6 +141,7 @@ func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing. func TestInvalidPackageRepositoryFormat(t *testing.T) { log := logf.Log.WithName("kc") var timeMetrics = metrics.NewReconcileTimeMetrics() + var appMetrics = metrics.NewAppMetrics() fetchInline := map[string]string{ "file.yml": `foo: hi`, @@ -162,7 +165,7 @@ func TestInvalidPackageRepositoryFormat(t *testing.T) { deployFac := deploy.NewFactory(k8scs, kubeconfig.NewKubeconfig(k8scs, log), nil, exec.NewPlainCmdRunner(), log) pkgr := v1alpha12.PackageRepository{} - crdApp := NewCRDApp(&app, &pkgr, log, kappcs, timeMetrics, fetchFac, tmpFac, deployFac) + crdApp := NewCRDApp(&app, &pkgr, log, kappcs, timeMetrics, appMetrics, fetchFac, tmpFac, deployFac) _, err := crdApp.Reconcile(false) if err != nil { t.Fatalf("Unexpected error with reconciling: %v", err) diff --git a/pkg/pkgrepository/app_template.go b/pkg/pkgrepository/app_template.go index f98761764..241cee7ee 100644 --- a/pkg/pkgrepository/app_template.go +++ b/pkg/pkgrepository/app_template.go @@ -11,7 +11,6 @@ import ( "os" "path/filepath" "strings" - "time" kcv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" datapackagingv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/apis/datapackaging/v1alpha1" @@ -26,11 +25,6 @@ import ( var errInvalidPackageRepo = exec.NewCmdRunResultWithErr(fmt.Errorf("Invalid package repository content: must contain 'packages/' directory but did not")) func (a *App) template(dirPath string) exec.CmdRunResult { - reconcileStartTS := time.Now() - defer func() { - a.timeMetrics.RegisterTemplateTime(a.app.Kind, a.app.Name, a.app.Namespace, "", time.Since(reconcileStartTS)) - }() - fileInfo, err := os.Lstat(filepath.Join(dirPath, "packages")) if err != nil { if os.IsNotExist(err) { diff --git a/pkg/pkgrepository/crd_app.go b/pkg/pkgrepository/crd_app.go index 5f02ae5d7..a99c99d1d 100644 --- a/pkg/pkgrepository/crd_app.go +++ b/pkg/pkgrepository/crd_app.go @@ -30,7 +30,7 @@ type CRDApp struct { } func NewCRDApp(appModel *kcv1alpha1.App, packageRepo *pkgingv1alpha1.PackageRepository, log logr.Logger, - appClient kcclient.Interface, timeMetrics *metrics.ReconcileTimeMetrics, fetchFactory fetch.Factory, + appClient kcclient.Interface, timeMetrics *metrics.ReconcileTimeMetrics, appMetrics *metrics.AppMetrics, fetchFactory fetch.Factory, templateFactory template.Factory, deployFactory deploy.Factory) *CRDApp { crdApp := &CRDApp{appModel: appModel, pkgrModel: packageRepo, log: log, appClient: appClient} @@ -39,7 +39,7 @@ func NewCRDApp(appModel *kcv1alpha1.App, packageRepo *pkgingv1alpha1.PackageRepo BlockDeletion: crdApp.blockDeletion, UnblockDeletion: crdApp.unblockDeletion, UpdateStatus: crdApp.updateStatus, - }, fetchFactory, templateFactory, deployFactory, log, timeMetrics, packageRepo.UID) + }, fetchFactory, templateFactory, deployFactory, log, appMetrics, timeMetrics, packageRepo.UID) return crdApp } From 16316081dcdedcd7c5aa7ba19314ddbae87328b7 Mon Sep 17 00:00:00 2001 From: sethiyash Date: Thu, 7 Dec 2023 19:27:00 +0530 Subject: [PATCH 3/9] final refactoring Signed-off-by: sethiyash --- cmd/controller/run.go | 40 ++--- pkg/app/app.go | 14 +- pkg/app/app_factory.go | 4 +- pkg/app/app_fetch.go | 6 - pkg/app/app_reconcile.go | 30 ++-- pkg/app/app_reconcile_test.go | 6 +- pkg/app/app_template_test.go | 2 +- pkg/app/crd_app.go | 14 +- pkg/metrics/app_metrics.go | 142 ----------------- pkg/metrics/reconcile_count_metrics.go | 144 ++++++++++++++++++ pkg/metrics/reconcile_time_metrics.go | 40 +++-- pkg/packageinstall/packageinstall.go | 18 ++- .../packageinstall_deletion_test.go | 2 +- .../packageinstall_downgrade_test.go | 9 +- pkg/packageinstall/packageinstall_test.go | 12 +- pkg/packageinstall/reconciler.go | 8 +- pkg/pkgrepository/app.go | 10 +- pkg/pkgrepository/app_deploy.go | 7 - pkg/pkgrepository/app_factory.go | 18 +-- pkg/pkgrepository/app_reconcile.go | 20 +-- pkg/pkgrepository/app_reconcile_test.go | 6 +- pkg/pkgrepository/crd_app.go | 15 +- 22 files changed, 292 insertions(+), 275 deletions(-) delete mode 100644 pkg/metrics/app_metrics.go create mode 100644 pkg/metrics/reconcile_count_metrics.go diff --git a/cmd/controller/run.go b/cmd/controller/run.go index 433f7ea8b..f8f3350e6 100644 --- a/cmd/controller/run.go +++ b/cmd/controller/run.go @@ -96,8 +96,8 @@ func Run(opts Options, runLog logr.Logger) error { } runLog.Info("setting up metrics") - appMetrics := metrics.NewAppMetrics() - appMetrics.RegisterAllMetrics() + countMetrics := metrics.NewCountMetrics() + countMetrics.RegisterAllMetrics() reconcileTimeMetrics := metrics.NewReconcileTimeMetrics() reconcileTimeMetrics.RegisterAllMetrics() @@ -197,15 +197,15 @@ func Run(opts Options, runLog logr.Logger) error { } { // add controller for apps appFactory := app.CRDAppFactory{ - CoreClient: coreClient, - AppClient: kcClient, - KcConfig: kcConfig, - AppMetrics: appMetrics, - TimeMetrics: reconcileTimeMetrics, - CmdRunner: sidecarCmdExec, - Kubeconf: kubeconf, - CompInfo: compInfo, - CacheFolder: cacheFolderApps, + CoreClient: coreClient, + AppClient: kcClient, + KcConfig: kcConfig, + CountMetrics: countMetrics, + TimeMetrics: reconcileTimeMetrics, + CmdRunner: sidecarCmdExec, + Kubeconf: kubeconf, + CompInfo: compInfo, + CacheFolder: cacheFolderApps, } reconciler := app.NewReconciler(kcClient, runLog.WithName("app"), appFactory, refTracker, updateStatusTracker, compInfo) @@ -232,7 +232,7 @@ func Run(opts Options, runLog logr.Logger) error { kcClient, opts.PackagingGlobalNS, runLog.WithName("handler")) reconciler := pkginstall.NewReconciler(kcClient, pkgClient, coreClient, pkgToPkgInstallHandler, - runLog.WithName("pkgi"), compInfo, kcConfig, reconcileTimeMetrics) + runLog.WithName("pkgi"), compInfo, kcConfig, countMetrics, reconcileTimeMetrics) ctrl, err := controller.New("pkgi", mgr, controller.Options{ Reconciler: reconciler, @@ -256,14 +256,14 @@ func Run(opts Options, runLog logr.Logger) error { { // add controller for pkgrepositories appFactory := pkgrepository.AppFactory{ - CoreClient: coreClient, - AppClient: kcClient, - KcConfig: kcConfig, - AppMetrics: appMetrics, - TimeMetrics: reconcileTimeMetrics, - CmdRunner: sidecarCmdExec, - Kubeconf: kubeconf, - CacheFolder: cacheFolderPkgRepoApps, + CoreClient: coreClient, + AppClient: kcClient, + KcConfig: kcConfig, + CountMetrics: countMetrics, + TimeMetrics: reconcileTimeMetrics, + CmdRunner: sidecarCmdExec, + Kubeconf: kubeconf, + CacheFolder: cacheFolderPkgRepoApps, } reconciler := pkgrepository.NewReconciler(kcClient, coreClient, diff --git a/pkg/app/app.go b/pkg/app/app.go index 8e8fb586a..a13da4d63 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -55,12 +55,12 @@ type App struct { memoizedKubernetesVersion string memoizedKubernetesAPIs []string - log logr.Logger - opts Opts - appMetrics *metrics.AppMetrics - timeMetrics *metrics.ReconcileTimeMetrics + log logr.Logger + opts Opts + countMetrics *metrics.ReconcileCountMetrics + timeMetrics *metrics.ReconcileTimeMetrics - isFirstReconcile string + isFirstReconcile bool pendingStatusUpdate bool flushAllStatusUpdates bool metadata *deploy.Meta @@ -68,11 +68,11 @@ type App struct { func NewApp(app v1alpha1.App, hooks Hooks, fetchFactory fetch.Factory, templateFactory template.Factory, - deployFactory deploy.Factory, log logr.Logger, opts Opts, appMetrics *metrics.AppMetrics, timeMetrics *metrics.ReconcileTimeMetrics, compInfo ComponentInfo) *App { + deployFactory deploy.Factory, log logr.Logger, opts Opts, appMetrics *metrics.ReconcileCountMetrics, timeMetrics *metrics.ReconcileTimeMetrics, compInfo ComponentInfo) *App { return &App{app: app, appPrev: *(app.DeepCopy()), hooks: hooks, fetchFactory: fetchFactory, templateFactory: templateFactory, - deployFactory: deployFactory, log: log, opts: opts, appMetrics: appMetrics, timeMetrics: timeMetrics, compInfo: compInfo} + deployFactory: deployFactory, log: log, opts: opts, countMetrics: appMetrics, timeMetrics: timeMetrics, compInfo: compInfo} } func (a *App) Name() string { return a.app.Name } diff --git a/pkg/app/app_factory.go b/pkg/app/app_factory.go index a37ec40b8..504384a5b 100644 --- a/pkg/app/app_factory.go +++ b/pkg/app/app_factory.go @@ -26,7 +26,7 @@ type CRDAppFactory struct { CoreClient kubernetes.Interface AppClient kcclient.Interface KcConfig *config.Config - AppMetrics *metrics.AppMetrics + CountMetrics *metrics.ReconcileCountMetrics TimeMetrics *metrics.ReconcileTimeMetrics VendirConfigHook func(vendirconf.Config) vendirconf.Config KbldAllowBuild bool @@ -49,7 +49,7 @@ func (f *CRDAppFactory) NewCRDApp(app *kcv1alpha1.App, log logr.Logger) *CRDApp templateFactory := template.NewFactory(f.CoreClient, fetchFactory, f.KbldAllowBuild, f.CmdRunner) deployFactory := deploy.NewFactory(f.CoreClient, f.Kubeconf, f.KcConfig, f.CmdRunner, log) - return NewCRDApp(app, log, f.AppMetrics, f.TimeMetrics, f.AppClient, fetchFactory, templateFactory, deployFactory, f.CompInfo, Opts{ + return NewCRDApp(app, log, f.CountMetrics, f.TimeMetrics, f.AppClient, fetchFactory, templateFactory, deployFactory, f.CompInfo, Opts{ DefaultSyncPeriod: f.KcConfig.AppDefaultSyncPeriod(), MinimumSyncPeriod: f.KcConfig.AppMinimumSyncPeriod(), }) diff --git a/pkg/app/app_fetch.go b/pkg/app/app_fetch.go index 67da673d3..07a12f0e1 100644 --- a/pkg/app/app_fetch.go +++ b/pkg/app/app_fetch.go @@ -20,12 +20,6 @@ const ( ) func (a *App) fetch(dstPath string) (string, exec.CmdRunResult) { - // update metrics with the total fetch time - reconcileStartTS := time.Now() - defer func() { - a.timeMetrics.RegisterFetchTime(a.app.Kind, a.app.Name, a.app.Namespace, "", time.Since(reconcileStartTS)) - }() - // fetch init stage if len(a.app.Spec.Fetch) == 0 { return "", exec.NewCmdRunResultWithErr(fmt.Errorf("Expected at least one fetch option")) diff --git a/pkg/app/app_reconcile.go b/pkg/app/app_reconcile.go index 030690ba8..5b00d45d6 100644 --- a/pkg/app/app_reconcile.go +++ b/pkg/app/app_reconcile.go @@ -15,13 +15,15 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" ) +const appResourceType = "app" + // Reconcile is not expected to be called concurrently func (a *App) Reconcile(force bool) (reconcile.Result, error) { defer a.flushUpdateStatus("app reconciled") var err error - a.appMetrics.InitMetrics(a.Name(), a.Namespace()) + a.countMetrics.InitMetrics(appResourceType, a.Name(), a.Namespace()) timerOpts := ReconcileTimerOpts{ DefaultSyncPeriod: a.opts.DefaultSyncPeriod, @@ -104,13 +106,9 @@ func (a *App) reconcileDeploy() error { func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { reconcileStartTS := time.Now() - a.isFirstReconcile = "false" - if a.appMetrics.GetReconcileAttemptCounterValue(a.app.Name, a.app.Namespace) == 1 { - a.isFirstReconcile = "true" - } - + a.isFirstReconcile = a.countMetrics.GetReconcileAttemptCounterValue("app", a.app.Name, a.app.Namespace) == 1 defer func() { - a.timeMetrics.RegisterOverallTime(a.app.Kind, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.timeMetrics.RegisterOverallTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, time.Since(reconcileStartTS)) }() @@ -140,7 +138,7 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.timeMetrics.RegisterFetchTime(a.app.Kind, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.timeMetrics.RegisterFetchTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, a.app.Status.Fetch.UpdatedAt.Sub(a.app.Status.Fetch.StartedAt.Time)) err := a.updateStatus("marking fetch completed") @@ -164,7 +162,7 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.timeMetrics.RegisterTemplateTime(a.app.Kind, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.timeMetrics.RegisterTemplateTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, a.app.Status.Template.UpdatedAt.Sub(templateStartTime)) err = a.updateStatus("marking template completed") @@ -215,7 +213,7 @@ func (a *App) updateLastDeploy(result exec.CmdRunResult) exec.CmdRunResult { }, } - a.timeMetrics.RegisterDeployTime(a.app.Kind, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.timeMetrics.RegisterDeployTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, a.Status().Deploy.UpdatedAt.Sub(a.Status().Deploy.StartedAt.Time)) return result @@ -269,7 +267,7 @@ func (a *App) setReconciling() { Status: corev1.ConditionTrue, }) - a.appMetrics.RegisterReconcileAttempt(a.app.Name, a.app.Namespace) + a.countMetrics.RegisterReconcileAttempt(appResourceType, a.app.Name, a.app.Namespace) a.app.Status.FriendlyDescription = "Reconciling" } @@ -285,7 +283,7 @@ func (a *App) setReconcileCompleted(result exec.CmdRunResult) { a.app.Status.ConsecutiveReconcileFailures++ a.app.Status.ConsecutiveReconcileSuccesses = 0 a.app.Status.FriendlyDescription = fmt.Sprintf("Reconcile failed: %s", result.ErrorStr()) - a.appMetrics.RegisterReconcileFailure(a.app.Name, a.app.Namespace) + a.countMetrics.RegisterReconcileFailure(appResourceType, a.app.Name, a.app.Namespace) a.setUsefulErrorMessage(result) } else { a.app.Status.Conditions = append(a.app.Status.Conditions, v1alpha1.Condition{ @@ -296,7 +294,7 @@ func (a *App) setReconcileCompleted(result exec.CmdRunResult) { a.app.Status.ConsecutiveReconcileSuccesses++ a.app.Status.ConsecutiveReconcileFailures = 0 a.app.Status.FriendlyDescription = "Reconcile succeeded" - a.appMetrics.RegisterReconcileSuccess(a.app.Name, a.app.Namespace) + a.countMetrics.RegisterReconcileSuccess(appResourceType, a.app.Name, a.app.Namespace) a.app.Status.UsefulErrorMessage = "" } } @@ -309,7 +307,7 @@ func (a *App) setDeleting() { Status: corev1.ConditionTrue, }) - a.appMetrics.RegisterReconcileDeleteAttempt(a.app.Name, a.app.Namespace) + a.countMetrics.RegisterReconcileDeleteAttempt(appResourceType, a.app.Name, a.app.Namespace) a.app.Status.FriendlyDescription = "Deleting" } @@ -325,10 +323,10 @@ func (a *App) setDeleteCompleted(result exec.CmdRunResult) { a.app.Status.ConsecutiveReconcileFailures++ a.app.Status.ConsecutiveReconcileSuccesses = 0 a.app.Status.FriendlyDescription = fmt.Sprintf("Delete failed: %s", result.ErrorStr()) - a.appMetrics.RegisterReconcileDeleteFailed(a.app.Name, a.app.Namespace) + a.countMetrics.RegisterReconcileDeleteFailed(appResourceType, a.app.Name, a.app.Namespace) a.setUsefulErrorMessage(result) } else { - a.appMetrics.DeleteMetrics(a.app.Name, a.app.Namespace) + a.countMetrics.DeleteMetrics(appResourceType, a.app.Name, a.app.Namespace) } } diff --git a/pkg/app/app_reconcile_test.go b/pkg/app/app_reconcile_test.go index 25380b15b..a9becd7c7 100644 --- a/pkg/app/app_reconcile_test.go +++ b/pkg/app/app_reconcile_test.go @@ -30,7 +30,7 @@ import ( func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) { log := logf.Log.WithName("kc") var ( - appMetrics = metrics.NewAppMetrics() + appMetrics = metrics.NewCountMetrics() timeMetrics = metrics.NewReconcileTimeMetrics() ) @@ -90,7 +90,7 @@ func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) { func Test_NoInspectReconcile_IfInspectNotEnabled(t *testing.T) { log := logf.Log.WithName("kc") var ( - appMetrics = metrics.NewAppMetrics() + appMetrics = metrics.NewCountMetrics() timeMetrics = metrics.NewReconcileTimeMetrics() ) @@ -171,7 +171,7 @@ func Test_NoInspectReconcile_IfInspectNotEnabled(t *testing.T) { func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing.T) { log := logf.Log.WithName("kc") var ( - appMetrics = metrics.NewAppMetrics() + appMetrics = metrics.NewCountMetrics() timeMetrics = metrics.NewReconcileTimeMetrics() ) diff --git a/pkg/app/app_template_test.go b/pkg/app/app_template_test.go index 465649547..09bd16187 100644 --- a/pkg/app/app_template_test.go +++ b/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.NewAppMetrics(), metrics.NewReconcileTimeMetrics(), fakeInfo) + app := NewApp(appEmpty, Hooks{}, fetchFac, tmpFac, deployFac, log, Opts{}, metrics.NewCountMetrics(), metrics.NewReconcileTimeMetrics(), fakeInfo) dir, err := os.MkdirTemp("", "temp") assert.NoError(t, err) diff --git a/pkg/app/crd_app.go b/pkg/app/crd_app.go index b5658690f..2f33b8cf3 100644 --- a/pkg/app/crd_app.go +++ b/pkg/app/crd_app.go @@ -21,20 +21,20 @@ import ( ) type CRDApp struct { - app *App - appModel *kcv1alpha1.App - log logr.Logger - appMetrics *metrics.AppMetrics - appClient kcclient.Interface + app *App + appModel *kcv1alpha1.App + log logr.Logger + countMetrics *metrics.ReconcileCountMetrics + appClient kcclient.Interface } // NewCRDApp creates new CRD app -func NewCRDApp(appModel *kcv1alpha1.App, log logr.Logger, appMetrics *metrics.AppMetrics, +func NewCRDApp(appModel *kcv1alpha1.App, log logr.Logger, appMetrics *metrics.ReconcileCountMetrics, timeMetrics *metrics.ReconcileTimeMetrics, appClient kcclient.Interface, fetchFactory fetch.Factory, templateFactory template.Factory, deployFactory deploy.Factory, compInfo ComponentInfo, opts Opts) *CRDApp { - crdApp := &CRDApp{appModel: appModel, log: log, appMetrics: appMetrics, appClient: appClient} + crdApp := &CRDApp{appModel: appModel, log: log, countMetrics: appMetrics, appClient: appClient} crdApp.app = NewApp(*appModel, Hooks{ BlockDeletion: crdApp.blockDeletion, diff --git a/pkg/metrics/app_metrics.go b/pkg/metrics/app_metrics.go deleted file mode 100644 index 771e8b7ef..000000000 --- a/pkg/metrics/app_metrics.go +++ /dev/null @@ -1,142 +0,0 @@ -// Copyright 2021 VMware, Inc. -// SPDX-License-Identifier: Apache-2.0 - -package metrics - -import ( - "sync" - - "github.com/prometheus/client_golang/prometheus" - dto "github.com/prometheus/client_model/go" - "sigs.k8s.io/controller-runtime/pkg/metrics" -) - -// AppMetrics holds server metrics -type AppMetrics struct { - reconcileAttemptTotal *prometheus.CounterVec - reconcileSuccessTotal *prometheus.CounterVec - reconcileFailureTotal *prometheus.CounterVec - reconcileDeleteAttemptTotal *prometheus.CounterVec - reconcileDeleteFailedTotal *prometheus.CounterVec -} - -var ( - once sync.Once -) - -// NewAppMetrics creates AppMetrics object -func NewAppMetrics() *AppMetrics { - const ( - metricNamespace = "kappctrl" - kappNameLabel = "app_name" - kappNamespaceLabel = "namespace" - ) - return &AppMetrics{ - reconcileAttemptTotal: prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: metricNamespace, - Name: "app_reconcile_attempt_total", - Help: "Total number of attempted reconciles", - }, - []string{kappNameLabel, kappNamespaceLabel}, - ), - reconcileSuccessTotal: prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: metricNamespace, - Name: "app_reconcile_success_total", - Help: "Total number of succeeded reconciles", - }, - []string{kappNameLabel, kappNamespaceLabel}, - ), - reconcileFailureTotal: prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: metricNamespace, - Name: "app_reconcile_failure_total", - Help: "Total number of failed reconciles", - }, - []string{kappNameLabel, kappNamespaceLabel}, - ), - reconcileDeleteAttemptTotal: prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: metricNamespace, - Name: "app_reconcile_delete_attempt_total", - Help: "Total number of attempted reconcile deletion", - }, - []string{kappNameLabel, kappNamespaceLabel}, - ), - reconcileDeleteFailedTotal: prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: metricNamespace, - Name: "app_reconcile_delete_failed_total", - Help: "Total number of failed reconcile deletion", - }, - []string{kappNameLabel, kappNamespaceLabel}, - ), - } -} - -// RegisterAllMetrics registers all prometheus metrics. -func (am *AppMetrics) RegisterAllMetrics() { - once.Do(func() { - metrics.Registry.MustRegister( - am.reconcileAttemptTotal, - am.reconcileSuccessTotal, - am.reconcileFailureTotal, - am.reconcileDeleteAttemptTotal, - am.reconcileDeleteFailedTotal, - ) - }) -} - -// InitMetrics initializes metrics -func (am *AppMetrics) InitMetrics(appName string, namespace string) { - // Initializes counter metrics - am.reconcileAttemptTotal.WithLabelValues(appName, namespace).Add(0) - am.reconcileSuccessTotal.WithLabelValues(appName, namespace).Add(0) - am.reconcileFailureTotal.WithLabelValues(appName, namespace).Add(0) - am.reconcileDeleteAttemptTotal.WithLabelValues(appName, namespace).Add(0) - am.reconcileDeleteFailedTotal.WithLabelValues(appName, namespace).Add(0) -} - -// DeleteMetrics deletes metrics -func (am *AppMetrics) DeleteMetrics(appName string, namespace string) { - // Delete counter metrics - am.reconcileAttemptTotal.DeleteLabelValues(appName, namespace) - am.reconcileSuccessTotal.DeleteLabelValues(appName, namespace) - am.reconcileFailureTotal.DeleteLabelValues(appName, namespace) - am.reconcileDeleteAttemptTotal.DeleteLabelValues(appName, namespace) - am.reconcileDeleteFailedTotal.DeleteLabelValues(appName, namespace) -} - -// RegisterReconcileAttempt increments reconcileAttemptTotal -func (am *AppMetrics) RegisterReconcileAttempt(appName string, namespace string) { - am.reconcileAttemptTotal.WithLabelValues(appName, namespace).Inc() -} - -// RegisterReconcileSuccess increments reconcileSuccessTotal -func (am *AppMetrics) RegisterReconcileSuccess(appName string, namespace string) { - am.reconcileSuccessTotal.WithLabelValues(appName, namespace).Inc() -} - -// RegisterReconcileFailure increments reconcileFailureTotal -func (am *AppMetrics) RegisterReconcileFailure(appName string, namespace string) { - am.reconcileFailureTotal.WithLabelValues(appName, namespace).Inc() -} - -// RegisterReconcileDeleteAttempt increments reconcileDeleteAttemptTotal -func (am *AppMetrics) RegisterReconcileDeleteAttempt(appName string, namespace string) { - am.reconcileDeleteAttemptTotal.WithLabelValues(appName, namespace).Inc() -} - -// RegisterReconcileDeleteFailed increments reconcileDeleteFailedTotal -func (am *AppMetrics) RegisterReconcileDeleteFailed(appName string, namespace string) { - am.reconcileDeleteFailedTotal.WithLabelValues(appName, namespace).Inc() -} - -func (am *AppMetrics) GetReconcileAttemptCounterValue(appName, namespace string) int64 { - var m = &dto.Metric{} - if err := am.reconcileAttemptTotal.WithLabelValues(appName, namespace).Write(m); err != nil { - return 0 - } - return int64(m.Counter.GetValue()) -} diff --git a/pkg/metrics/reconcile_count_metrics.go b/pkg/metrics/reconcile_count_metrics.go new file mode 100644 index 000000000..c0e6c4635 --- /dev/null +++ b/pkg/metrics/reconcile_count_metrics.go @@ -0,0 +1,144 @@ +// Copyright 2021 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package metrics + +import ( + "sync" + + "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" + "sigs.k8s.io/controller-runtime/pkg/metrics" +) + +// ReconcileCountMetrics holds server metrics +type ReconcileCountMetrics struct { + reconcileAttemptTotal *prometheus.CounterVec + reconcileSuccessTotal *prometheus.CounterVec + reconcileFailureTotal *prometheus.CounterVec + reconcileDeleteAttemptTotal *prometheus.CounterVec + reconcileDeleteFailedTotal *prometheus.CounterVec +} + +var ( + once sync.Once +) + +// NewCountMetrics creates ReconcileCountMetrics object +func NewCountMetrics() *ReconcileCountMetrics { + const ( + metricNamespace = "kappctrl" + kappNameLabel = "name" + kappNamespaceLabel = "namespace" + resourceTypeLabel = "controller" + ) + return &ReconcileCountMetrics{ + reconcileAttemptTotal: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: metricNamespace, + Name: "app_reconcile_attempt_total", + Help: "Total number of attempted reconciles", + }, + []string{resourceTypeLabel, kappNameLabel, kappNamespaceLabel}, + ), + reconcileSuccessTotal: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: metricNamespace, + Name: "app_reconcile_success_total", + Help: "Total number of succeeded reconciles", + }, + []string{resourceTypeLabel, kappNameLabel, kappNamespaceLabel}, + ), + reconcileFailureTotal: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: metricNamespace, + Name: "app_reconcile_failure_total", + Help: "Total number of failed reconciles", + }, + []string{resourceTypeLabel, kappNameLabel, kappNamespaceLabel}, + ), + reconcileDeleteAttemptTotal: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: metricNamespace, + Name: "app_reconcile_delete_attempt_total", + Help: "Total number of attempted reconcile deletion", + }, + []string{resourceTypeLabel, kappNameLabel, kappNamespaceLabel}, + ), + reconcileDeleteFailedTotal: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: metricNamespace, + Name: "app_reconcile_delete_failed_total", + Help: "Total number of failed reconcile deletion", + }, + []string{resourceTypeLabel, kappNameLabel, kappNamespaceLabel}, + ), + } +} + +// RegisterAllMetrics registers all prometheus metrics. +func (am *ReconcileCountMetrics) RegisterAllMetrics() { + once.Do(func() { + metrics.Registry.MustRegister( + am.reconcileAttemptTotal, + am.reconcileSuccessTotal, + am.reconcileFailureTotal, + am.reconcileDeleteAttemptTotal, + am.reconcileDeleteFailedTotal, + ) + }) +} + +// InitMetrics initializes metrics +func (am *ReconcileCountMetrics) InitMetrics(resourceType, name, namespace string) { + // Initializes counter metrics + am.reconcileAttemptTotal.WithLabelValues(resourceType, name, namespace).Add(0) + am.reconcileSuccessTotal.WithLabelValues(resourceType, name, namespace).Add(0) + am.reconcileFailureTotal.WithLabelValues(resourceType, name, namespace).Add(0) + am.reconcileDeleteAttemptTotal.WithLabelValues(resourceType, name, namespace).Add(0) + am.reconcileDeleteFailedTotal.WithLabelValues(resourceType, name, namespace).Add(0) +} + +// DeleteMetrics deletes metrics +func (am *ReconcileCountMetrics) DeleteMetrics(resourceType, name, namespace string) { + // Delete counter metrics + am.reconcileAttemptTotal.DeleteLabelValues(resourceType, name, namespace) + am.reconcileSuccessTotal.DeleteLabelValues(resourceType, name, namespace) + am.reconcileFailureTotal.DeleteLabelValues(resourceType, name, namespace) + am.reconcileDeleteAttemptTotal.DeleteLabelValues(resourceType, name, namespace) + am.reconcileDeleteFailedTotal.DeleteLabelValues(resourceType, name, namespace) +} + +// RegisterReconcileAttempt increments reconcileAttemptTotal +func (am *ReconcileCountMetrics) RegisterReconcileAttempt(resourceType, appName, namespace string) { + am.reconcileAttemptTotal.WithLabelValues(resourceType, appName, namespace).Inc() +} + +// RegisterReconcileSuccess increments reconcileSuccessTotal +func (am *ReconcileCountMetrics) RegisterReconcileSuccess(resourceType, appName, namespace string) { + am.reconcileSuccessTotal.WithLabelValues(resourceType, appName, namespace).Inc() +} + +// RegisterReconcileFailure increments reconcileFailureTotal +func (am *ReconcileCountMetrics) RegisterReconcileFailure(resourceType, appName, namespace string) { + am.reconcileFailureTotal.WithLabelValues(resourceType, appName, namespace).Inc() +} + +// RegisterReconcileDeleteAttempt increments reconcileDeleteAttemptTotal +func (am *ReconcileCountMetrics) RegisterReconcileDeleteAttempt(resourceType, appName, namespace string) { + am.reconcileDeleteAttemptTotal.WithLabelValues(resourceType, appName, namespace).Inc() +} + +// RegisterReconcileDeleteFailed increments reconcileDeleteFailedTotal +func (am *ReconcileCountMetrics) RegisterReconcileDeleteFailed(resourceType, appName, namespace string) { + am.reconcileDeleteFailedTotal.WithLabelValues(resourceType, appName, namespace).Inc() +} + +// GetReconcileAttemptCounterValue return reconcile count +func (am *ReconcileCountMetrics) GetReconcileAttemptCounterValue(resourceType, appName, namespace string) int64 { + var m = &dto.Metric{} + if err := am.reconcileAttemptTotal.WithLabelValues(resourceType, appName, namespace).Write(m); err != nil { + return 0 + } + return int64(m.Counter.GetValue()) +} diff --git a/pkg/metrics/reconcile_time_metrics.go b/pkg/metrics/reconcile_time_metrics.go index 5f5762030..eaae34230 100644 --- a/pkg/metrics/reconcile_time_metrics.go +++ b/pkg/metrics/reconcile_time_metrics.go @@ -1,12 +1,18 @@ +// Copyright 2021 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +// Package metrics to define all prometheus metric methods package metrics import ( "github.com/prometheus/client_golang/prometheus" "sigs.k8s.io/controller-runtime/pkg/metrics" + "strconv" "sync" "time" ) +// ReconcileTimeMetrics holds reconcile time metrics type ReconcileTimeMetrics struct { reconcileTimeSeconds *prometheus.GaugeVec reconcileDeployTimeSeconds *prometheus.GaugeVec @@ -18,13 +24,14 @@ var ( timeMetricsOnce sync.Once ) +// NewReconcileTimeMetrics creates ReconcileTimeMetrics object func NewReconcileTimeMetrics() *ReconcileTimeMetrics { const ( - metricNamespace = "kappctrl_reconcile_time_seconds" + metricNamespace = "kappctrl" resourceTypeLabel = "controller" resourceNameLabel = "name" firstReconcileLabel = "firstReconcile" - namespace = "namespace" + namespaceLabel = "namespace" ) return &ReconcileTimeMetrics{ reconcileTimeSeconds: prometheus.NewGaugeVec( @@ -33,7 +40,7 @@ func NewReconcileTimeMetrics() *ReconcileTimeMetrics { Name: "reconcile_time_seconds", Help: "Overall time taken to reconcile a CR", }, - []string{resourceTypeLabel, resourceNameLabel, namespace, firstReconcileLabel}, + []string{resourceTypeLabel, resourceNameLabel, namespaceLabel, firstReconcileLabel}, ), reconcileFetchTimeSeconds: prometheus.NewGaugeVec( prometheus.GaugeOpts{ @@ -41,7 +48,7 @@ func NewReconcileTimeMetrics() *ReconcileTimeMetrics { Name: "reconcile_fetch_time_seconds", Help: "Time taken to perform a fetch for a CR", }, - []string{resourceTypeLabel, resourceNameLabel, namespace, firstReconcileLabel}, + []string{resourceTypeLabel, resourceNameLabel, namespaceLabel, firstReconcileLabel}, ), reconcileTemplateTimeSeconds: prometheus.NewGaugeVec( prometheus.GaugeOpts{ @@ -49,7 +56,7 @@ func NewReconcileTimeMetrics() *ReconcileTimeMetrics { Name: "reconcile_template_time_seconds", Help: "Time taken to perform a templating for a CR", }, - []string{resourceTypeLabel, resourceNameLabel, namespace, firstReconcileLabel}, + []string{resourceTypeLabel, resourceNameLabel, namespaceLabel, firstReconcileLabel}, ), reconcileDeployTimeSeconds: prometheus.NewGaugeVec( prometheus.GaugeOpts{ @@ -57,11 +64,12 @@ func NewReconcileTimeMetrics() *ReconcileTimeMetrics { Name: "reconcile_deploy_time_seconds", Help: "Time taken to perform a deploy for a CR", }, - []string{resourceTypeLabel, resourceNameLabel, namespace, firstReconcileLabel}, + []string{resourceTypeLabel, resourceNameLabel, namespaceLabel, firstReconcileLabel}, ), } } +// RegisterAllMetrics registers reconcile time prometheus metrics. func (tm *ReconcileTimeMetrics) RegisterAllMetrics() { timeMetricsOnce.Do(func() { metrics.Registry.MustRegister( @@ -73,18 +81,22 @@ func (tm *ReconcileTimeMetrics) RegisterAllMetrics() { }) } -func (tm *ReconcileTimeMetrics) RegisterOverallTime(resourceType, name, namespace, firstReconcile string, time time.Duration) { - tm.reconcileTimeSeconds.WithLabelValues(resourceType, name, namespace, firstReconcile).Set(time.Seconds()) +// RegisterOverallTime sets overall time +func (tm *ReconcileTimeMetrics) RegisterOverallTime(resourceType, name, namespace string, firstReconcile bool, time time.Duration) { + tm.reconcileTimeSeconds.WithLabelValues(resourceType, name, namespace, strconv.FormatBool(firstReconcile)).Set(time.Seconds()) } -func (tm *ReconcileTimeMetrics) RegisterFetchTime(resourceType, name, namespace, firstReconcile string, time time.Duration) { - tm.reconcileFetchTimeSeconds.WithLabelValues(resourceType, name, namespace, firstReconcile).Set(time.Seconds()) +// RegisterFetchTime sets fetch time +func (tm *ReconcileTimeMetrics) RegisterFetchTime(resourceType, name, namespace string, firstReconcile bool, time time.Duration) { + tm.reconcileFetchTimeSeconds.WithLabelValues(resourceType, name, namespace, strconv.FormatBool(firstReconcile)).Set(time.Seconds()) } -func (tm *ReconcileTimeMetrics) RegisterTemplateTime(resourceType, name, namespace, firstReconcile string, time time.Duration) { - tm.reconcileTemplateTimeSeconds.WithLabelValues(resourceType, name, namespace, firstReconcile).Set(time.Seconds()) +// RegisterTemplateTime sets template time +func (tm *ReconcileTimeMetrics) RegisterTemplateTime(resourceType, name, namespace string, firstReconcile bool, time time.Duration) { + tm.reconcileTemplateTimeSeconds.WithLabelValues(resourceType, name, namespace, strconv.FormatBool(firstReconcile)).Set(time.Seconds()) } -func (tm *ReconcileTimeMetrics) RegisterDeployTime(resourceType, name, namespace, firstReconcile string, time time.Duration) { - tm.reconcileDeployTimeSeconds.WithLabelValues(resourceType, name, namespace, firstReconcile).Set(time.Seconds()) +// RegisterDeployTime sets deploy time +func (tm *ReconcileTimeMetrics) RegisterDeployTime(resourceType, name, namespace string, firstReconcile bool, time time.Duration) { + tm.reconcileDeployTimeSeconds.WithLabelValues(resourceType, name, namespace, strconv.FormatBool(firstReconcile)).Set(time.Seconds()) } diff --git a/pkg/packageinstall/packageinstall.go b/pkg/packageinstall/packageinstall.go index 5f0d89443..ee3331cd6 100644 --- a/pkg/packageinstall/packageinstall.go +++ b/pkg/packageinstall/packageinstall.go @@ -42,6 +42,7 @@ const ( // PackageInstall to indicate that lower version of the package // can be selected vs whats currently installed. DowngradableAnnKey = "packaging.carvel.dev/downgradable" + packageInstallType = "pkgi" ) // nolint: revive @@ -56,7 +57,10 @@ type PackageInstallCR struct { compInfo ComponentInfo opts Opts - timeMetrics *metrics.ReconcileTimeMetrics + countMetrics *metrics.ReconcileCountMetrics + timeMetrics *metrics.ReconcileTimeMetrics + + firstReconcile bool } // nolint: revive @@ -66,10 +70,11 @@ type Opts struct { func NewPackageInstallCR(model *pkgingv1alpha1.PackageInstall, log logr.Logger, kcclient kcclient.Interface, pkgclient pkgclient.Interface, coreClient kubernetes.Interface, - compInfo ComponentInfo, opts Opts, timeMetrics *metrics.ReconcileTimeMetrics) *PackageInstallCR { + compInfo ComponentInfo, opts Opts, countMetrics *metrics.ReconcileCountMetrics, timeMetrics *metrics.ReconcileTimeMetrics) *PackageInstallCR { return &PackageInstallCR{model: model, unmodifiedModel: model.DeepCopy(), log: log, - kcclient: kcclient, pkgclient: pkgclient, coreClient: coreClient, compInfo: compInfo, opts: opts, timeMetrics: timeMetrics} + kcclient: kcclient, pkgclient: pkgclient, coreClient: coreClient, compInfo: compInfo, opts: opts, countMetrics: countMetrics, + timeMetrics: timeMetrics} } func (pi *PackageInstallCR) Reconcile() (reconcile.Result, error) { @@ -78,6 +83,8 @@ func (pi *PackageInstallCR) Reconcile() (reconcile.Result, error) { func(st kcv1alpha1.GenericStatus) { pi.model.Status.GenericStatus = st }, } + pi.countMetrics.InitMetrics(packageInstallType, pi.model.Name, pi.model.Namespace) + var result reconcile.Result var err error @@ -104,10 +111,13 @@ func (pi *PackageInstallCR) Reconcile() (reconcile.Result, error) { func (pi *PackageInstallCR) reconcile(modelStatus *reconciler.Status) (reconcile.Result, error) { pi.log.Info("Reconciling") + pi.countMetrics.RegisterReconcileAttempt(packageInstallType, pi.model.Name, pi.model.Namespace) reconcileStartTS := time.Now() + pi.firstReconcile = pi.countMetrics.GetReconcileAttemptCounterValue("pkgi", pi.model.Name, pi.model.Namespace) == 1 defer func() { - pi.timeMetrics.RegisterOverallTime("pkgi", pi.model.Name, pi.model.Namespace, "", time.Since(reconcileStartTS)) + pi.timeMetrics.RegisterOverallTime(packageInstallType, pi.model.Name, pi.model.Namespace, + pi.firstReconcile, time.Since(reconcileStartTS)) }() err := pi.blockDeletion() diff --git a/pkg/packageinstall/packageinstall_deletion_test.go b/pkg/packageinstall/packageinstall_deletion_test.go index 0cdfd9b0c..1117e596d 100644 --- a/pkg/packageinstall/packageinstall_deletion_test.go +++ b/pkg/packageinstall/packageinstall_deletion_test.go @@ -66,7 +66,7 @@ func Test_PackageInstallDeletion(t *testing.T) { appClient := fakekappctrl.NewSimpleClientset(pkgInstall, existingApp) coreClient := fake.NewSimpleClientset() - ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, coreClient, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, &metrics.ReconcileTimeMetrics{}) + ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, coreClient, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, metrics.NewCountMetrics(), metrics.NewReconcileTimeMetrics()) _, err := ip.Reconcile() assert.Nil(t, err) diff --git a/pkg/packageinstall/packageinstall_downgrade_test.go b/pkg/packageinstall/packageinstall_downgrade_test.go index 776f106e2..6c79161ab 100644 --- a/pkg/packageinstall/packageinstall_downgrade_test.go +++ b/pkg/packageinstall/packageinstall_downgrade_test.go @@ -27,6 +27,7 @@ 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{ @@ -100,7 +101,7 @@ func Test_PackageInstallVersionDowngrades(t *testing.T) { GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileTimeMetrics) + ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileCountMetrics, reconcileTimeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) @@ -149,7 +150,7 @@ func Test_PackageInstallVersionDowngrades(t *testing.T) { GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileTimeMetrics) + ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileCountMetrics, reconcileTimeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) @@ -198,7 +199,7 @@ func Test_PackageInstallVersionDowngrades(t *testing.T) { GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileTimeMetrics) + ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileCountMetrics, reconcileTimeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) @@ -253,7 +254,7 @@ func Test_PackageInstallVersionDowngrades(t *testing.T) { GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileTimeMetrics) + ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileCountMetrics, reconcileTimeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) diff --git a/pkg/packageinstall/packageinstall_test.go b/pkg/packageinstall/packageinstall_test.go index 0fc71cb75..08e95a2d2 100644 --- a/pkg/packageinstall/packageinstall_test.go +++ b/pkg/packageinstall/packageinstall_test.go @@ -355,6 +355,7 @@ func Test_PackageRefUsesName(t *testing.T) { func Test_PlaceHolderSecretCreated_WhenPackageHasNoSecretRef(t *testing.T) { var timeMetrics = metrics.NewReconcileTimeMetrics() + var countMetrics = metrics.NewCountMetrics() pkg := datapkgingv1alpha1.Package{ ObjectMeta: metav1.ObjectMeta{ Name: "expected-pkg", @@ -404,7 +405,7 @@ func Test_PlaceHolderSecretCreated_WhenPackageHasNoSecretRef(t *testing.T) { GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, timeMetrics) + ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, countMetrics, timeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) @@ -431,6 +432,7 @@ func Test_PlaceHolderSecretCreated_WhenPackageHasNoSecretRef(t *testing.T) { func Test_PlaceHolderSecretsCreated_WhenPackageHasMultipleFetchStages(t *testing.T) { var timeMetrics = metrics.NewReconcileTimeMetrics() + var countMetrics = metrics.NewCountMetrics() pkg := datapkgingv1alpha1.Package{ ObjectMeta: metav1.ObjectMeta{ Name: "expected-pkg", @@ -484,7 +486,7 @@ func Test_PlaceHolderSecretsCreated_WhenPackageHasMultipleFetchStages(t *testing GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, timeMetrics) + ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, countMetrics, timeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) @@ -521,6 +523,7 @@ func Test_PlaceHolderSecretsCreated_WhenPackageHasMultipleFetchStages(t *testing func Test_PlaceHolderSecretsNotCreated_WhenFetchStagesHaveSecrets(t *testing.T) { var timeMetrics = metrics.NewReconcileTimeMetrics() + var countMetrics = metrics.NewCountMetrics() pkg := datapkgingv1alpha1.Package{ ObjectMeta: metav1.ObjectMeta{ Name: "expected-pkg", @@ -575,7 +578,7 @@ func Test_PlaceHolderSecretsNotCreated_WhenFetchStagesHaveSecrets(t *testing.T) GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, timeMetrics) + ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, countMetrics, timeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) @@ -596,6 +599,7 @@ func Test_PlaceHolderSecretsNotCreated_WhenFetchStagesHaveSecrets(t *testing.T) func Test_PlaceHolderSecretCreated_WhenPackageInstallUpdated(t *testing.T) { var timeMetrics = metrics.NewReconcileTimeMetrics() + var countMetrics = metrics.NewCountMetrics() appSpec := v1alpha1.AppSpec{ Fetch: []v1alpha1.AppFetch{ { @@ -653,7 +657,7 @@ func Test_PlaceHolderSecretCreated_WhenPackageInstallUpdated(t *testing.T) { fakekctrl := fakekappctrl.NewSimpleClientset(model, existingApp) fakek8s := fake.NewSimpleClientset() - ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, timeMetrics) + ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, countMetrics, timeMetrics) // mock the kubernetes server version fakeDiscovery, _ := fakek8s.Discovery().(*fakediscovery.FakeDiscovery) diff --git a/pkg/packageinstall/reconciler.go b/pkg/packageinstall/reconciler.go index f1ba01e3e..bb00af117 100644 --- a/pkg/packageinstall/reconciler.go +++ b/pkg/packageinstall/reconciler.go @@ -34,12 +34,14 @@ type Reconciler struct { log logr.Logger kcConfig *kcconfig.Config timeMetrics *metrics.ReconcileTimeMetrics + countMetrics *metrics.ReconcileCountMetrics } // NewReconciler is the constructor for the Reconciler struct func NewReconciler(kcClient kcclient.Interface, pkgClient pkgclient.Interface, coreClient kubernetes.Interface, pkgToPkgInstallHandler *PackageInstallVersionHandler, - log logr.Logger, compInfo ComponentInfo, kcConfig *kcconfig.Config, timeMetrics *metrics.ReconcileTimeMetrics) *Reconciler { + log logr.Logger, compInfo ComponentInfo, kcConfig *kcconfig.Config, countMetrics *metrics.ReconcileCountMetrics, + timeMetrics *metrics.ReconcileTimeMetrics) *Reconciler { return &Reconciler{kcClient: kcClient, pkgClient: pkgClient, @@ -48,6 +50,7 @@ func NewReconciler(kcClient kcclient.Interface, pkgClient pkgclient.Interface, compInfo: compInfo, log: log, kcConfig: kcConfig, + countMetrics: countMetrics, timeMetrics: timeMetrics, } } @@ -90,5 +93,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) ( return reconcile.Result{}, err } - return NewPackageInstallCR(existingPkgInstall, log, r.kcClient, r.pkgClient, r.coreClient, r.compInfo, Opts{DefaultSyncPeriod: r.kcConfig.PackageInstallDefaultSyncPeriod()}, r.timeMetrics).Reconcile() + return NewPackageInstallCR(existingPkgInstall, log, r.kcClient, r.pkgClient, r.coreClient, r.compInfo, + Opts{DefaultSyncPeriod: r.kcConfig.PackageInstallDefaultSyncPeriod()}, r.countMetrics, r.timeMetrics).Reconcile() } diff --git a/pkg/pkgrepository/app.go b/pkg/pkgrepository/app.go index 3a03920be..69b2545cf 100644 --- a/pkg/pkgrepository/app.go +++ b/pkg/pkgrepository/app.go @@ -30,22 +30,22 @@ type App struct { templateFactory template.Factory deployFactory deploy.Factory - timeMetrics *metrics.ReconcileTimeMetrics - appMetrics *metrics.AppMetrics + timeMetrics *metrics.ReconcileTimeMetrics + countMetrics *metrics.ReconcileCountMetrics log logr.Logger - isFirstReconcile string + isFirstReconcile bool pendingStatusUpdate bool flushAllStatusUpdates bool } // NewApp creates a new instance of an App based on v1alpha1.App func NewApp(app v1alpha1.App, hooks Hooks, fetchFactory fetch.Factory, templateFactory template.Factory, deployFactory deploy.Factory, - log logr.Logger, appMetrics *metrics.AppMetrics, timeMetrics *metrics.ReconcileTimeMetrics, pkgRepoUID types.UID) *App { + log logr.Logger, countMetrics *metrics.ReconcileCountMetrics, timeMetrics *metrics.ReconcileTimeMetrics, pkgRepoUID types.UID) *App { return &App{app: app, appPrev: *(app.DeepCopy()), hooks: hooks, fetchFactory: fetchFactory, templateFactory: templateFactory, - deployFactory: deployFactory, log: log, appMetrics: appMetrics, timeMetrics: timeMetrics, pkgRepoUID: pkgRepoUID} + deployFactory: deployFactory, log: log, countMetrics: countMetrics, timeMetrics: timeMetrics, pkgRepoUID: pkgRepoUID} } func (a *App) Name() string { return a.app.Name } diff --git a/pkg/pkgrepository/app_deploy.go b/pkg/pkgrepository/app_deploy.go index 4e2b46ebf..7b8b236e4 100644 --- a/pkg/pkgrepository/app_deploy.go +++ b/pkg/pkgrepository/app_deploy.go @@ -5,8 +5,6 @@ package pkgrepository import ( "fmt" - "time" - "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" ctldep "github.com/vmware-tanzu/carvel-kapp-controller/pkg/deploy" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/exec" @@ -14,11 +12,6 @@ import ( ) func (a *App) deploy(tplOutput string) exec.CmdRunResult { - reconcileStartTS := time.Now() - defer func() { - a.timeMetrics.RegisterDeployTime(a.app.Kind, a.app.Name, a.app.Namespace, "", time.Since(reconcileStartTS)) - }() - err := a.blockDeletion() if err != nil { return exec.NewCmdRunResultWithErr(fmt.Errorf("Blocking for deploy: %s", err)) diff --git a/pkg/pkgrepository/app_factory.go b/pkg/pkgrepository/app_factory.go index 379d1c849..f2f194b21 100644 --- a/pkg/pkgrepository/app_factory.go +++ b/pkg/pkgrepository/app_factory.go @@ -23,14 +23,14 @@ import ( // AppFactory allows to create "hidden" Apps for reconciling PackageRepositories. type AppFactory struct { - CoreClient kubernetes.Interface - AppClient kcclient.Interface - TimeMetrics *metrics.ReconcileTimeMetrics - AppMetrics *metrics.AppMetrics - KcConfig *config.Config - CmdRunner exec.CmdRunner - Kubeconf *kubeconfig.Kubeconfig - CacheFolder *memdir.TmpDir + CoreClient kubernetes.Interface + AppClient kcclient.Interface + TimeMetrics *metrics.ReconcileTimeMetrics + CountMetrics *metrics.ReconcileCountMetrics + KcConfig *config.Config + CmdRunner exec.CmdRunner + Kubeconf *kubeconfig.Kubeconfig + CacheFolder *memdir.TmpDir } // NewCRDPackageRepo constructs "hidden" App to reconcile PackageRepository. @@ -42,5 +42,5 @@ func (f *AppFactory) NewCRDPackageRepo(app *kcv1alpha1.App, pkgr *pkgv1alpha1.Pa fetchFactory := fetch.NewFactory(f.CoreClient, vendirOpts, f.CmdRunner) templateFactory := template.NewFactory(f.CoreClient, fetchFactory, false, f.CmdRunner) deployFactory := deploy.NewFactory(f.CoreClient, f.Kubeconf, nil, f.CmdRunner, log) - return NewCRDApp(app, pkgr, log, f.AppClient, f.TimeMetrics, f.AppMetrics, fetchFactory, templateFactory, deployFactory) + return NewCRDApp(app, pkgr, log, f.AppClient, f.TimeMetrics, f.CountMetrics, fetchFactory, templateFactory, deployFactory) } diff --git a/pkg/pkgrepository/app_reconcile.go b/pkg/pkgrepository/app_reconcile.go index 66461214e..194c52d80 100644 --- a/pkg/pkgrepository/app_reconcile.go +++ b/pkg/pkgrepository/app_reconcile.go @@ -15,13 +15,15 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" ) +const packageRepoResourceType = "pkgr" + // Reconcile is not expected to be called concurrently func (a *App) Reconcile(force bool) (reconcile.Result, error) { defer a.flushUpdateStatus("packagerepository reconciled") var err error - a.appMetrics.InitMetrics(a.Name(), a.Namespace()) + a.countMetrics.InitMetrics("pkgr", a.Name(), a.Namespace()) switch { case a.app.Spec.Paused: @@ -93,12 +95,10 @@ func (a *App) reconcileDeploy() error { func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { reconcileStartTS := time.Now() - a.isFirstReconcile = "false" - if a.appMetrics.GetReconcileAttemptCounterValue(a.app.Name, a.app.Namespace) == 1 { - a.isFirstReconcile = "true" - } + a.isFirstReconcile = a.countMetrics.GetReconcileAttemptCounterValue(packageRepoResourceType, a.app.Name, a.app.Namespace) == 1 defer func() { - a.timeMetrics.RegisterOverallTime(a.app.Kind, a.app.Name, a.app.Namespace, a.isFirstReconcile, time.Since(reconcileStartTS)) + a.timeMetrics.RegisterOverallTime(packageRepoResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, + time.Since(reconcileStartTS)) }() tmpDir := memdir.NewTmpDir("fetch-template-deploy") @@ -127,7 +127,7 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.timeMetrics.RegisterFetchTime("pkgr", a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.timeMetrics.RegisterFetchTime(packageRepoResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, a.app.Status.Fetch.UpdatedAt.Sub(a.app.Status.Fetch.StartedAt.Time)) err := a.updateStatus("marking fetch completed") @@ -151,7 +151,7 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.timeMetrics.RegisterTemplateTime("pkgr", a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.timeMetrics.RegisterTemplateTime(packageRepoResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, a.app.Status.Template.UpdatedAt.Sub(templateStartTime)) err = a.updateStatus("marking template completed") @@ -181,7 +181,7 @@ func (a *App) updateLastDeploy(result exec.CmdRunResult) exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.timeMetrics.RegisterDeployTime("pkgr", a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.timeMetrics.RegisterDeployTime(packageRepoResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, a.Status().Deploy.UpdatedAt.Sub(a.Status().Deploy.StartedAt.Time)) a.updateStatus("marking last deploy") @@ -215,7 +215,7 @@ func (a *App) setReconciling() { Status: corev1.ConditionTrue, }) - a.appMetrics.RegisterReconcileAttempt(a.app.Name, a.app.Namespace) + a.countMetrics.RegisterReconcileAttempt("pkgr", a.app.Name, a.app.Namespace) a.app.Status.FriendlyDescription = "Reconciling" } diff --git a/pkg/pkgrepository/app_reconcile_test.go b/pkg/pkgrepository/app_reconcile_test.go index 78ebf7726..7ae8ac5c3 100644 --- a/pkg/pkgrepository/app_reconcile_test.go +++ b/pkg/pkgrepository/app_reconcile_test.go @@ -26,7 +26,7 @@ import ( func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) { log := logf.Log.WithName("kc") var timeMetrics = metrics.NewReconcileTimeMetrics() - var appMetrics = metrics.NewAppMetrics() + var appMetrics = metrics.NewCountMetrics() // The url under fetch is invalid, which will cause this // app to fail before deploy. @@ -82,7 +82,7 @@ func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) { func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing.T) { log := logf.Log.WithName("kc") var timeMetrics = metrics.NewReconcileTimeMetrics() - var appMetrics = metrics.NewAppMetrics() + var appMetrics = metrics.NewCountMetrics() fetchInline := map[string]string{ "packages/file.yml": `foo: #@ data.values.nothere`, @@ -141,7 +141,7 @@ func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing. func TestInvalidPackageRepositoryFormat(t *testing.T) { log := logf.Log.WithName("kc") var timeMetrics = metrics.NewReconcileTimeMetrics() - var appMetrics = metrics.NewAppMetrics() + var appMetrics = metrics.NewCountMetrics() fetchInline := map[string]string{ "file.yml": `foo: hi`, diff --git a/pkg/pkgrepository/crd_app.go b/pkg/pkgrepository/crd_app.go index a99c99d1d..bf32098f1 100644 --- a/pkg/pkgrepository/crd_app.go +++ b/pkg/pkgrepository/crd_app.go @@ -21,16 +21,15 @@ import ( ) type CRDApp struct { - app *App - appModel *kcv1alpha1.App - pkgrModel *pkgingv1alpha1.PackageRepository - timeMetrics *metrics.ReconcileTimeMetrics - log logr.Logger - appClient kcclient.Interface + app *App + appModel *kcv1alpha1.App + pkgrModel *pkgingv1alpha1.PackageRepository + log logr.Logger + appClient kcclient.Interface } func NewCRDApp(appModel *kcv1alpha1.App, packageRepo *pkgingv1alpha1.PackageRepository, log logr.Logger, - appClient kcclient.Interface, timeMetrics *metrics.ReconcileTimeMetrics, appMetrics *metrics.AppMetrics, fetchFactory fetch.Factory, + appClient kcclient.Interface, timeMetrics *metrics.ReconcileTimeMetrics, countMetrics *metrics.ReconcileCountMetrics, fetchFactory fetch.Factory, templateFactory template.Factory, deployFactory deploy.Factory) *CRDApp { crdApp := &CRDApp{appModel: appModel, pkgrModel: packageRepo, log: log, appClient: appClient} @@ -39,7 +38,7 @@ func NewCRDApp(appModel *kcv1alpha1.App, packageRepo *pkgingv1alpha1.PackageRepo BlockDeletion: crdApp.blockDeletion, UnblockDeletion: crdApp.unblockDeletion, UpdateStatus: crdApp.updateStatus, - }, fetchFactory, templateFactory, deployFactory, log, appMetrics, timeMetrics, packageRepo.UID) + }, fetchFactory, templateFactory, deployFactory, log, countMetrics, timeMetrics, packageRepo.UID) return crdApp } From c8713036ce04b68f4dd1acac1d2719d302a5cbe6 Mon Sep 17 00:00:00 2001 From: sethiyash Date: Tue, 12 Dec 2023 11:51:05 +0530 Subject: [PATCH 4/9] resolved the nits Signed-off-by: sethiyash --- cmd/controller/run.go | 18 ++++++++--------- pkg/app/app_factory.go | 26 ++++++++++++------------- pkg/app/app_reconcile.go | 4 ++-- pkg/packageinstall/packageinstall.go | 4 ++-- pkg/pkgrepository/app_reconcile.go | 4 ++-- pkg/pkgrepository/app_reconcile_test.go | 2 +- pkg/pkgrepository/crd_app.go | 2 +- 7 files changed, 30 insertions(+), 30 deletions(-) diff --git a/cmd/controller/run.go b/cmd/controller/run.go index f8f3350e6..4b40be043 100644 --- a/cmd/controller/run.go +++ b/cmd/controller/run.go @@ -197,15 +197,15 @@ func Run(opts Options, runLog logr.Logger) error { } { // add controller for apps appFactory := app.CRDAppFactory{ - CoreClient: coreClient, - AppClient: kcClient, - KcConfig: kcConfig, - CountMetrics: countMetrics, - TimeMetrics: reconcileTimeMetrics, - CmdRunner: sidecarCmdExec, - Kubeconf: kubeconf, - CompInfo: compInfo, - CacheFolder: cacheFolderApps, + CoreClient: coreClient, + AppClient: kcClient, + KcConfig: kcConfig, + CountMetrics: countMetrics, + ReconcileTimeMetrics: reconcileTimeMetrics, + CmdRunner: sidecarCmdExec, + Kubeconf: kubeconf, + CompInfo: compInfo, + CacheFolder: cacheFolderApps, } reconciler := app.NewReconciler(kcClient, runLog.WithName("app"), appFactory, refTracker, updateStatusTracker, compInfo) diff --git a/pkg/app/app_factory.go b/pkg/app/app_factory.go index 504384a5b..ea4287313 100644 --- a/pkg/app/app_factory.go +++ b/pkg/app/app_factory.go @@ -23,18 +23,18 @@ import ( // CRDAppFactory allows to create CRDApps. type CRDAppFactory struct { - CoreClient kubernetes.Interface - AppClient kcclient.Interface - KcConfig *config.Config - CountMetrics *metrics.ReconcileCountMetrics - TimeMetrics *metrics.ReconcileTimeMetrics - VendirConfigHook func(vendirconf.Config) vendirconf.Config - KbldAllowBuild bool - CmdRunner exec.CmdRunner - Kubeconf *kubeconfig.Kubeconfig - CompInfo ComponentInfo - DeployFactory deploy.Factory - CacheFolder *memdir.TmpDir + CoreClient kubernetes.Interface + AppClient kcclient.Interface + KcConfig *config.Config + CountMetrics *metrics.ReconcileCountMetrics + ReconcileTimeMetrics *metrics.ReconcileTimeMetrics + VendirConfigHook func(vendirconf.Config) vendirconf.Config + KbldAllowBuild bool + CmdRunner exec.CmdRunner + Kubeconf *kubeconfig.Kubeconfig + CompInfo ComponentInfo + DeployFactory deploy.Factory + CacheFolder *memdir.TmpDir } // NewCRDApp creates a CRDApp injecting necessary dependencies. @@ -49,7 +49,7 @@ func (f *CRDAppFactory) NewCRDApp(app *kcv1alpha1.App, log logr.Logger) *CRDApp templateFactory := template.NewFactory(f.CoreClient, fetchFactory, f.KbldAllowBuild, f.CmdRunner) deployFactory := deploy.NewFactory(f.CoreClient, f.Kubeconf, f.KcConfig, f.CmdRunner, log) - return NewCRDApp(app, log, f.CountMetrics, f.TimeMetrics, f.AppClient, fetchFactory, templateFactory, deployFactory, f.CompInfo, Opts{ + return NewCRDApp(app, log, f.CountMetrics, f.ReconcileTimeMetrics, f.AppClient, fetchFactory, templateFactory, deployFactory, f.CompInfo, Opts{ DefaultSyncPeriod: f.KcConfig.AppDefaultSyncPeriod(), MinimumSyncPeriod: f.KcConfig.AppMinimumSyncPeriod(), }) diff --git a/pkg/app/app_reconcile.go b/pkg/app/app_reconcile.go index 5b00d45d6..9f1d721a3 100644 --- a/pkg/app/app_reconcile.go +++ b/pkg/app/app_reconcile.go @@ -105,11 +105,11 @@ func (a *App) reconcileDeploy() error { } func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { - reconcileStartTS := time.Now() + reconcileStartTime := time.Now() a.isFirstReconcile = a.countMetrics.GetReconcileAttemptCounterValue("app", a.app.Name, a.app.Namespace) == 1 defer func() { a.timeMetrics.RegisterOverallTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, - time.Since(reconcileStartTS)) + time.Since(reconcileStartTime)) }() tmpDir := memdir.NewTmpDir("fetch-template-deploy") diff --git a/pkg/packageinstall/packageinstall.go b/pkg/packageinstall/packageinstall.go index ee3331cd6..724f1297a 100644 --- a/pkg/packageinstall/packageinstall.go +++ b/pkg/packageinstall/packageinstall.go @@ -113,11 +113,11 @@ func (pi *PackageInstallCR) reconcile(modelStatus *reconciler.Status) (reconcile pi.log.Info("Reconciling") pi.countMetrics.RegisterReconcileAttempt(packageInstallType, pi.model.Name, pi.model.Namespace) - reconcileStartTS := time.Now() + reconcileStartTime := time.Now() pi.firstReconcile = pi.countMetrics.GetReconcileAttemptCounterValue("pkgi", pi.model.Name, pi.model.Namespace) == 1 defer func() { pi.timeMetrics.RegisterOverallTime(packageInstallType, pi.model.Name, pi.model.Namespace, - pi.firstReconcile, time.Since(reconcileStartTS)) + pi.firstReconcile, time.Since(reconcileStartTime)) }() err := pi.blockDeletion() diff --git a/pkg/pkgrepository/app_reconcile.go b/pkg/pkgrepository/app_reconcile.go index 194c52d80..2bec61024 100644 --- a/pkg/pkgrepository/app_reconcile.go +++ b/pkg/pkgrepository/app_reconcile.go @@ -94,11 +94,11 @@ func (a *App) reconcileDeploy() error { } func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { - reconcileStartTS := time.Now() + reconcileStartTime := time.Now() a.isFirstReconcile = a.countMetrics.GetReconcileAttemptCounterValue(packageRepoResourceType, a.app.Name, a.app.Namespace) == 1 defer func() { a.timeMetrics.RegisterOverallTime(packageRepoResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, - time.Since(reconcileStartTS)) + time.Since(reconcileStartTime)) }() tmpDir := memdir.NewTmpDir("fetch-template-deploy") diff --git a/pkg/pkgrepository/app_reconcile_test.go b/pkg/pkgrepository/app_reconcile_test.go index 7ae8ac5c3..6b22252d4 100644 --- a/pkg/pkgrepository/app_reconcile_test.go +++ b/pkg/pkgrepository/app_reconcile_test.go @@ -4,7 +4,6 @@ package pkgrepository import ( - "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "testing" "github.com/stretchr/testify/assert" @@ -15,6 +14,7 @@ import ( "github.com/vmware-tanzu/carvel-kapp-controller/pkg/exec" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/fetch" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/kubeconfig" + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/template" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" diff --git a/pkg/pkgrepository/crd_app.go b/pkg/pkgrepository/crd_app.go index bf32098f1..ca7be6a32 100644 --- a/pkg/pkgrepository/crd_app.go +++ b/pkg/pkgrepository/crd_app.go @@ -6,7 +6,6 @@ package pkgrepository import ( "context" "fmt" - "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "github.com/go-logr/logr" kcv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" @@ -14,6 +13,7 @@ import ( kcclient "github.com/vmware-tanzu/carvel-kapp-controller/pkg/client/clientset/versioned" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/deploy" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/fetch" + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/reftracker" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/template" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" From 017446906bbbe6154d0331bafdb0e9e197ebf982 Mon Sep 17 00:00:00 2001 From: sethiyash Date: Wed, 20 Dec 2023 21:19:20 +0530 Subject: [PATCH 5/9] having single struct for both metrics Signed-off-by: sethiyash --- cmd/controller/run.go | 35 +++++++++---------- pkg/app/app.go | 11 +++--- pkg/app/app_factory.go | 25 +++++++------ pkg/app/app_reconcile.go | 24 ++++++------- pkg/app/app_reconcile_test.go | 6 ++-- pkg/app/app_template_test.go | 2 +- pkg/app/app_test.go | 9 ++--- pkg/app/crd_app.go | 7 ++-- pkg/metrics/metrics.go | 10 ++++++ pkg/packageinstall/packageinstall.go | 16 ++++----- .../packageinstall_deletion_test.go | 4 ++- .../packageinstall_downgrade_test.go | 16 ++++++--- pkg/packageinstall/packageinstall_test.go | 16 ++++++--- pkg/packageinstall/reconciler.go | 11 +++--- pkg/pkgrepository/app.go | 7 ++-- pkg/pkgrepository/app_factory.go | 17 +++++---- pkg/pkgrepository/app_reconcile.go | 14 ++++---- pkg/pkgrepository/app_reconcile_test.go | 9 +++-- pkg/pkgrepository/crd_app.go | 4 +-- 19 files changed, 132 insertions(+), 111 deletions(-) create mode 100644 pkg/metrics/metrics.go diff --git a/cmd/controller/run.go b/cmd/controller/run.go index 4b40be043..243552c53 100644 --- a/cmd/controller/run.go +++ b/cmd/controller/run.go @@ -197,15 +197,14 @@ func Run(opts Options, runLog logr.Logger) error { } { // add controller for apps appFactory := app.CRDAppFactory{ - CoreClient: coreClient, - AppClient: kcClient, - KcConfig: kcConfig, - CountMetrics: countMetrics, - ReconcileTimeMetrics: reconcileTimeMetrics, - CmdRunner: sidecarCmdExec, - Kubeconf: kubeconf, - CompInfo: compInfo, - CacheFolder: cacheFolderApps, + CoreClient: coreClient, + AppClient: kcClient, + KcConfig: kcConfig, + AppMetrics: &metrics.Metrics{ReconcileCountMetrics: countMetrics, ReconcileTimeMetrics: reconcileTimeMetrics}, + CmdRunner: sidecarCmdExec, + Kubeconf: kubeconf, + CompInfo: compInfo, + CacheFolder: cacheFolderApps, } reconciler := app.NewReconciler(kcClient, runLog.WithName("app"), appFactory, refTracker, updateStatusTracker, compInfo) @@ -232,7 +231,8 @@ func Run(opts Options, runLog logr.Logger) error { kcClient, opts.PackagingGlobalNS, runLog.WithName("handler")) reconciler := pkginstall.NewReconciler(kcClient, pkgClient, coreClient, pkgToPkgInstallHandler, - runLog.WithName("pkgi"), compInfo, kcConfig, countMetrics, reconcileTimeMetrics) + runLog.WithName("pkgi"), compInfo, kcConfig, &metrics.Metrics{ReconcileCountMetrics: countMetrics, + ReconcileTimeMetrics: reconcileTimeMetrics}) ctrl, err := controller.New("pkgi", mgr, controller.Options{ Reconciler: reconciler, @@ -256,14 +256,13 @@ func Run(opts Options, runLog logr.Logger) error { { // add controller for pkgrepositories appFactory := pkgrepository.AppFactory{ - CoreClient: coreClient, - AppClient: kcClient, - KcConfig: kcConfig, - CountMetrics: countMetrics, - TimeMetrics: reconcileTimeMetrics, - CmdRunner: sidecarCmdExec, - Kubeconf: kubeconf, - CacheFolder: cacheFolderPkgRepoApps, + CoreClient: coreClient, + AppClient: kcClient, + KcConfig: kcConfig, + AppMetrics: &metrics.Metrics{ReconcileCountMetrics: countMetrics, ReconcileTimeMetrics: reconcileTimeMetrics}, + CmdRunner: sidecarCmdExec, + Kubeconf: kubeconf, + CacheFolder: cacheFolderPkgRepoApps, } reconciler := pkgrepository.NewReconciler(kcClient, coreClient, diff --git a/pkg/app/app.go b/pkg/app/app.go index a13da4d63..f68f8d7a3 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -55,10 +55,9 @@ type App struct { memoizedKubernetesVersion string memoizedKubernetesAPIs []string - log logr.Logger - opts Opts - countMetrics *metrics.ReconcileCountMetrics - timeMetrics *metrics.ReconcileTimeMetrics + log logr.Logger + opts Opts + appMetrics *metrics.Metrics isFirstReconcile bool pendingStatusUpdate bool @@ -68,11 +67,11 @@ type App struct { func NewApp(app v1alpha1.App, hooks Hooks, fetchFactory fetch.Factory, templateFactory template.Factory, - deployFactory deploy.Factory, log logr.Logger, opts Opts, appMetrics *metrics.ReconcileCountMetrics, timeMetrics *metrics.ReconcileTimeMetrics, compInfo ComponentInfo) *App { + deployFactory deploy.Factory, log logr.Logger, opts Opts, appMetrics *metrics.Metrics, compInfo ComponentInfo) *App { return &App{app: app, appPrev: *(app.DeepCopy()), hooks: hooks, fetchFactory: fetchFactory, templateFactory: templateFactory, - deployFactory: deployFactory, log: log, opts: opts, countMetrics: appMetrics, timeMetrics: timeMetrics, compInfo: compInfo} + deployFactory: deployFactory, log: log, opts: opts, appMetrics: appMetrics, compInfo: compInfo} } func (a *App) Name() string { return a.app.Name } diff --git a/pkg/app/app_factory.go b/pkg/app/app_factory.go index ea4287313..79c290a27 100644 --- a/pkg/app/app_factory.go +++ b/pkg/app/app_factory.go @@ -23,18 +23,17 @@ import ( // CRDAppFactory allows to create CRDApps. type CRDAppFactory struct { - CoreClient kubernetes.Interface - AppClient kcclient.Interface - KcConfig *config.Config - CountMetrics *metrics.ReconcileCountMetrics - ReconcileTimeMetrics *metrics.ReconcileTimeMetrics - VendirConfigHook func(vendirconf.Config) vendirconf.Config - KbldAllowBuild bool - CmdRunner exec.CmdRunner - Kubeconf *kubeconfig.Kubeconfig - CompInfo ComponentInfo - DeployFactory deploy.Factory - CacheFolder *memdir.TmpDir + CoreClient kubernetes.Interface + AppClient kcclient.Interface + KcConfig *config.Config + AppMetrics *metrics.Metrics + VendirConfigHook func(vendirconf.Config) vendirconf.Config + KbldAllowBuild bool + CmdRunner exec.CmdRunner + Kubeconf *kubeconfig.Kubeconfig + CompInfo ComponentInfo + DeployFactory deploy.Factory + CacheFolder *memdir.TmpDir } // NewCRDApp creates a CRDApp injecting necessary dependencies. @@ -49,7 +48,7 @@ func (f *CRDAppFactory) NewCRDApp(app *kcv1alpha1.App, log logr.Logger) *CRDApp templateFactory := template.NewFactory(f.CoreClient, fetchFactory, f.KbldAllowBuild, f.CmdRunner) deployFactory := deploy.NewFactory(f.CoreClient, f.Kubeconf, f.KcConfig, f.CmdRunner, log) - return NewCRDApp(app, log, f.CountMetrics, f.ReconcileTimeMetrics, f.AppClient, fetchFactory, templateFactory, deployFactory, f.CompInfo, Opts{ + return NewCRDApp(app, log, f.AppMetrics, f.AppClient, fetchFactory, templateFactory, deployFactory, f.CompInfo, Opts{ DefaultSyncPeriod: f.KcConfig.AppDefaultSyncPeriod(), MinimumSyncPeriod: f.KcConfig.AppMinimumSyncPeriod(), }) diff --git a/pkg/app/app_reconcile.go b/pkg/app/app_reconcile.go index 9f1d721a3..260acf856 100644 --- a/pkg/app/app_reconcile.go +++ b/pkg/app/app_reconcile.go @@ -23,7 +23,7 @@ func (a *App) Reconcile(force bool) (reconcile.Result, error) { var err error - a.countMetrics.InitMetrics(appResourceType, a.Name(), a.Namespace()) + a.appMetrics.ReconcileCountMetrics.InitMetrics(appResourceType, a.Name(), a.Namespace()) timerOpts := ReconcileTimerOpts{ DefaultSyncPeriod: a.opts.DefaultSyncPeriod, @@ -106,9 +106,9 @@ func (a *App) reconcileDeploy() error { func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { reconcileStartTime := time.Now() - a.isFirstReconcile = a.countMetrics.GetReconcileAttemptCounterValue("app", a.app.Name, a.app.Namespace) == 1 + a.isFirstReconcile = a.appMetrics.ReconcileCountMetrics.GetReconcileAttemptCounterValue("app", a.app.Name, a.app.Namespace) == 1 defer func() { - a.timeMetrics.RegisterOverallTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterOverallTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, time.Since(reconcileStartTime)) }() @@ -138,7 +138,7 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.timeMetrics.RegisterFetchTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterFetchTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, a.app.Status.Fetch.UpdatedAt.Sub(a.app.Status.Fetch.StartedAt.Time)) err := a.updateStatus("marking fetch completed") @@ -162,7 +162,7 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.timeMetrics.RegisterTemplateTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterTemplateTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, a.app.Status.Template.UpdatedAt.Sub(templateStartTime)) err = a.updateStatus("marking template completed") @@ -213,7 +213,7 @@ func (a *App) updateLastDeploy(result exec.CmdRunResult) exec.CmdRunResult { }, } - a.timeMetrics.RegisterDeployTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterDeployTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, a.Status().Deploy.UpdatedAt.Sub(a.Status().Deploy.StartedAt.Time)) return result @@ -267,7 +267,7 @@ func (a *App) setReconciling() { Status: corev1.ConditionTrue, }) - a.countMetrics.RegisterReconcileAttempt(appResourceType, a.app.Name, a.app.Namespace) + a.appMetrics.ReconcileCountMetrics.RegisterReconcileAttempt(appResourceType, a.app.Name, a.app.Namespace) a.app.Status.FriendlyDescription = "Reconciling" } @@ -283,7 +283,7 @@ func (a *App) setReconcileCompleted(result exec.CmdRunResult) { a.app.Status.ConsecutiveReconcileFailures++ a.app.Status.ConsecutiveReconcileSuccesses = 0 a.app.Status.FriendlyDescription = fmt.Sprintf("Reconcile failed: %s", result.ErrorStr()) - a.countMetrics.RegisterReconcileFailure(appResourceType, a.app.Name, a.app.Namespace) + a.appMetrics.ReconcileCountMetrics.RegisterReconcileFailure(appResourceType, a.app.Name, a.app.Namespace) a.setUsefulErrorMessage(result) } else { a.app.Status.Conditions = append(a.app.Status.Conditions, v1alpha1.Condition{ @@ -294,7 +294,7 @@ func (a *App) setReconcileCompleted(result exec.CmdRunResult) { a.app.Status.ConsecutiveReconcileSuccesses++ a.app.Status.ConsecutiveReconcileFailures = 0 a.app.Status.FriendlyDescription = "Reconcile succeeded" - a.countMetrics.RegisterReconcileSuccess(appResourceType, a.app.Name, a.app.Namespace) + a.appMetrics.ReconcileCountMetrics.RegisterReconcileSuccess(appResourceType, a.app.Name, a.app.Namespace) a.app.Status.UsefulErrorMessage = "" } } @@ -307,7 +307,7 @@ func (a *App) setDeleting() { Status: corev1.ConditionTrue, }) - a.countMetrics.RegisterReconcileDeleteAttempt(appResourceType, a.app.Name, a.app.Namespace) + a.appMetrics.ReconcileCountMetrics.RegisterReconcileDeleteAttempt(appResourceType, a.app.Name, a.app.Namespace) a.app.Status.FriendlyDescription = "Deleting" } @@ -323,10 +323,10 @@ func (a *App) setDeleteCompleted(result exec.CmdRunResult) { a.app.Status.ConsecutiveReconcileFailures++ a.app.Status.ConsecutiveReconcileSuccesses = 0 a.app.Status.FriendlyDescription = fmt.Sprintf("Delete failed: %s", result.ErrorStr()) - a.countMetrics.RegisterReconcileDeleteFailed(appResourceType, a.app.Name, a.app.Namespace) + a.appMetrics.ReconcileCountMetrics.RegisterReconcileDeleteFailed(appResourceType, a.app.Name, a.app.Namespace) a.setUsefulErrorMessage(result) } else { - a.countMetrics.DeleteMetrics(appResourceType, a.app.Name, a.app.Namespace) + a.appMetrics.ReconcileCountMetrics.DeleteMetrics(appResourceType, a.app.Name, a.app.Namespace) } } diff --git a/pkg/app/app_reconcile_test.go b/pkg/app/app_reconcile_test.go index a9becd7c7..e7d4808de 100644 --- a/pkg/app/app_reconcile_test.go +++ b/pkg/app/app_reconcile_test.go @@ -55,7 +55,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, appMetrics, timeMetrics, kappcs, fetchFac, tmpFac, deployFac, FakeComponentInfo{}, Opts{MinimumSyncPeriod: 30 * time.Second}) + crdApp := NewCRDApp(&app, log, &metrics.Metrics{ReconcileCountMetrics: appMetrics, ReconcileTimeMetrics: timeMetrics}, kappcs, fetchFac, tmpFac, deployFac, FakeComponentInfo{}, Opts{MinimumSyncPeriod: 30 * time.Second}) _, err := crdApp.Reconcile(false) assert.Nil(t, err, "unexpected error with reconciling", err) @@ -125,7 +125,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, appMetrics, timeMetrics, kappcs, fetchFac, tmpFac, deployFac, FakeComponentInfo{}, Opts{MinimumSyncPeriod: 30 * time.Second}) + crdApp := NewCRDApp(&app, log, &metrics.Metrics{ReconcileCountMetrics: appMetrics, ReconcileTimeMetrics: timeMetrics}, kappcs, fetchFac, tmpFac, deployFac, FakeComponentInfo{}, Opts{MinimumSyncPeriod: 30 * time.Second}) _, err := crdApp.Reconcile(false) assert.Nil(t, err, "unexpected error with reconciling", err) @@ -200,7 +200,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, appMetrics, timeMetrics, kappcs, fetchFac, tmpFac, deployFac, FakeComponentInfo{}, Opts{MinimumSyncPeriod: 30 * time.Second}) + crdApp := NewCRDApp(&app, log, &metrics.Metrics{ReconcileCountMetrics: appMetrics, ReconcileTimeMetrics: timeMetrics}, kappcs, fetchFac, tmpFac, deployFac, FakeComponentInfo{}, Opts{MinimumSyncPeriod: 30 * time.Second}) _, err := crdApp.Reconcile(false) assert.Nil(t, err, "Unexpected error with reconciling", err) diff --git a/pkg/app/app_template_test.go b/pkg/app/app_template_test.go index 09bd16187..77a081958 100644 --- a/pkg/app/app_template_test.go +++ b/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.NewCountMetrics(), metrics.NewReconcileTimeMetrics(), fakeInfo) + app := NewApp(appEmpty, Hooks{}, fetchFac, tmpFac, deployFac, log, Opts{}, &metrics.Metrics{}, fakeInfo) dir, err := os.MkdirTemp("", "temp") assert.NoError(t, err) diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index 87e5f87b9..91fa25a4c 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -15,6 +15,7 @@ import ( "github.com/vmware-tanzu/carvel-kapp-controller/pkg/exec" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/fetch" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/kubeconfig" + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/reftracker" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/template" v1 "k8s.io/api/core/v1" @@ -64,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{}, nil, nil, FakeComponentInfo{}) + app := apppkg.NewApp(appWithRefs, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, apppkg.Opts{}, &metrics.Metrics{}, FakeComponentInfo{}) out := app.SecretRefs() assert.Truef(t, reflect.DeepEqual(out, expected), "Expected: %s\nGot: %s\n", expected, out) @@ -88,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{}, nil, nil, FakeComponentInfo{}) + app := apppkg.NewApp(appEmpty, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, apppkg.Opts{}, &metrics.Metrics{}, FakeComponentInfo{}) out := app.SecretRefs() assert.Equal(t, 0, len(out), "No SecretRefs to be returned") @@ -126,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{}, nil, nil, FakeComponentInfo{}) + app := apppkg.NewApp(appWithRefs, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, apppkg.Opts{}, &metrics.Metrics{}, FakeComponentInfo{}) out := app.ConfigMapRefs() assert.Truef(t, reflect.DeepEqual(out, expected), "Expected: %s\nGot: %s\n", expected, out) @@ -150,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{}, nil, nil, FakeComponentInfo{}) + app := apppkg.NewApp(appEmpty, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, apppkg.Opts{}, &metrics.Metrics{}, FakeComponentInfo{}) out := app.ConfigMapRefs() assert.Lenf(t, out, 0, "Expected: %s\nGot: %s\n", "No ConfigMapRefs to be returned", out) diff --git a/pkg/app/crd_app.go b/pkg/app/crd_app.go index 2f33b8cf3..20d1cf91e 100644 --- a/pkg/app/crd_app.go +++ b/pkg/app/crd_app.go @@ -29,19 +29,18 @@ type CRDApp struct { } // NewCRDApp creates new CRD app -func NewCRDApp(appModel *kcv1alpha1.App, log logr.Logger, appMetrics *metrics.ReconcileCountMetrics, - timeMetrics *metrics.ReconcileTimeMetrics, appClient kcclient.Interface, fetchFactory fetch.Factory, +func NewCRDApp(appModel *kcv1alpha1.App, log logr.Logger, appMetrics *metrics.Metrics, appClient kcclient.Interface, fetchFactory fetch.Factory, templateFactory template.Factory, deployFactory deploy.Factory, compInfo ComponentInfo, opts Opts) *CRDApp { - crdApp := &CRDApp{appModel: appModel, log: log, countMetrics: appMetrics, appClient: appClient} + crdApp := &CRDApp{appModel: appModel, log: log, countMetrics: appMetrics.ReconcileCountMetrics, appClient: appClient} crdApp.app = NewApp(*appModel, Hooks{ BlockDeletion: crdApp.blockDeletion, UnblockDeletion: crdApp.unblockDeletion, UpdateStatus: crdApp.updateStatus, WatchChanges: crdApp.watchChanges, - }, fetchFactory, templateFactory, deployFactory, log, opts, appMetrics, timeMetrics, compInfo) + }, fetchFactory, templateFactory, deployFactory, log, opts, appMetrics, compInfo) return crdApp } diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go new file mode 100644 index 000000000..7fbf6c75a --- /dev/null +++ b/pkg/metrics/metrics.go @@ -0,0 +1,10 @@ +// Copyright 2021 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package metrics + +// Metrics holds all metrics +type Metrics struct { + *ReconcileCountMetrics + *ReconcileTimeMetrics +} diff --git a/pkg/packageinstall/packageinstall.go b/pkg/packageinstall/packageinstall.go index 724f1297a..0b7b6cd5c 100644 --- a/pkg/packageinstall/packageinstall.go +++ b/pkg/packageinstall/packageinstall.go @@ -57,8 +57,7 @@ type PackageInstallCR struct { compInfo ComponentInfo opts Opts - countMetrics *metrics.ReconcileCountMetrics - timeMetrics *metrics.ReconcileTimeMetrics + pkgMetrics *metrics.Metrics firstReconcile bool } @@ -70,11 +69,10 @@ type Opts struct { func NewPackageInstallCR(model *pkgingv1alpha1.PackageInstall, log logr.Logger, kcclient kcclient.Interface, pkgclient pkgclient.Interface, coreClient kubernetes.Interface, - compInfo ComponentInfo, opts Opts, countMetrics *metrics.ReconcileCountMetrics, timeMetrics *metrics.ReconcileTimeMetrics) *PackageInstallCR { + compInfo ComponentInfo, opts Opts, pkgMetrics *metrics.Metrics) *PackageInstallCR { return &PackageInstallCR{model: model, unmodifiedModel: model.DeepCopy(), log: log, - kcclient: kcclient, pkgclient: pkgclient, coreClient: coreClient, compInfo: compInfo, opts: opts, countMetrics: countMetrics, - timeMetrics: timeMetrics} + kcclient: kcclient, pkgclient: pkgclient, coreClient: coreClient, compInfo: compInfo, opts: opts, pkgMetrics: pkgMetrics} } func (pi *PackageInstallCR) Reconcile() (reconcile.Result, error) { @@ -83,7 +81,7 @@ func (pi *PackageInstallCR) Reconcile() (reconcile.Result, error) { func(st kcv1alpha1.GenericStatus) { pi.model.Status.GenericStatus = st }, } - pi.countMetrics.InitMetrics(packageInstallType, pi.model.Name, pi.model.Namespace) + pi.pkgMetrics.ReconcileCountMetrics.InitMetrics(packageInstallType, pi.model.Name, pi.model.Namespace) var result reconcile.Result var err error @@ -111,12 +109,12 @@ func (pi *PackageInstallCR) Reconcile() (reconcile.Result, error) { func (pi *PackageInstallCR) reconcile(modelStatus *reconciler.Status) (reconcile.Result, error) { pi.log.Info("Reconciling") - pi.countMetrics.RegisterReconcileAttempt(packageInstallType, pi.model.Name, pi.model.Namespace) + pi.pkgMetrics.ReconcileCountMetrics.RegisterReconcileAttempt(packageInstallType, pi.model.Name, pi.model.Namespace) reconcileStartTime := time.Now() - pi.firstReconcile = pi.countMetrics.GetReconcileAttemptCounterValue("pkgi", pi.model.Name, pi.model.Namespace) == 1 + pi.firstReconcile = pi.pkgMetrics.ReconcileCountMetrics.GetReconcileAttemptCounterValue("pkgi", pi.model.Name, pi.model.Namespace) == 1 defer func() { - pi.timeMetrics.RegisterOverallTime(packageInstallType, pi.model.Name, pi.model.Namespace, + pi.pkgMetrics.ReconcileTimeMetrics.RegisterOverallTime(packageInstallType, pi.model.Name, pi.model.Namespace, pi.firstReconcile, time.Since(reconcileStartTime)) }() diff --git a/pkg/packageinstall/packageinstall_deletion_test.go b/pkg/packageinstall/packageinstall_deletion_test.go index 1117e596d..1e574adcc 100644 --- a/pkg/packageinstall/packageinstall_deletion_test.go +++ b/pkg/packageinstall/packageinstall_deletion_test.go @@ -66,7 +66,9 @@ func Test_PackageInstallDeletion(t *testing.T) { appClient := fakekappctrl.NewSimpleClientset(pkgInstall, existingApp) coreClient := fake.NewSimpleClientset() - ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, coreClient, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, metrics.NewCountMetrics(), metrics.NewReconcileTimeMetrics()) + ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, coreClient, + FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, + &metrics.Metrics{ReconcileCountMetrics: metrics.NewCountMetrics(), ReconcileTimeMetrics: metrics.NewReconcileTimeMetrics()}) _, err := ip.Reconcile() assert.Nil(t, err) diff --git a/pkg/packageinstall/packageinstall_downgrade_test.go b/pkg/packageinstall/packageinstall_downgrade_test.go index 6c79161ab..c3a76ce24 100644 --- a/pkg/packageinstall/packageinstall_downgrade_test.go +++ b/pkg/packageinstall/packageinstall_downgrade_test.go @@ -101,7 +101,9 @@ func Test_PackageInstallVersionDowngrades(t *testing.T) { GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileCountMetrics, reconcileTimeMetrics) + ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, + FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, + &metrics.Metrics{ReconcileCountMetrics: reconcileCountMetrics, ReconcileTimeMetrics: reconcileTimeMetrics}) _, err := ip.Reconcile() assert.Nil(t, err) @@ -150,7 +152,9 @@ func Test_PackageInstallVersionDowngrades(t *testing.T) { GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileCountMetrics, reconcileTimeMetrics) + ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, + FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, + &metrics.Metrics{ReconcileCountMetrics: reconcileCountMetrics, ReconcileTimeMetrics: reconcileTimeMetrics}) _, err := ip.Reconcile() assert.Nil(t, err) @@ -199,7 +203,9 @@ func Test_PackageInstallVersionDowngrades(t *testing.T) { GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileCountMetrics, reconcileTimeMetrics) + ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, + FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, + &metrics.Metrics{ReconcileCountMetrics: reconcileCountMetrics, ReconcileTimeMetrics: reconcileTimeMetrics}) _, err := ip.Reconcile() assert.Nil(t, err) @@ -254,7 +260,9 @@ func Test_PackageInstallVersionDowngrades(t *testing.T) { GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileCountMetrics, reconcileTimeMetrics) + ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, + FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, + &metrics.Metrics{ReconcileCountMetrics: reconcileCountMetrics, ReconcileTimeMetrics: reconcileTimeMetrics}) _, err := ip.Reconcile() assert.Nil(t, err) diff --git a/pkg/packageinstall/packageinstall_test.go b/pkg/packageinstall/packageinstall_test.go index 08e95a2d2..c217c5c99 100644 --- a/pkg/packageinstall/packageinstall_test.go +++ b/pkg/packageinstall/packageinstall_test.go @@ -405,7 +405,9 @@ func Test_PlaceHolderSecretCreated_WhenPackageHasNoSecretRef(t *testing.T) { GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, countMetrics, timeMetrics) + ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, + FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, + &metrics.Metrics{ReconcileCountMetrics: countMetrics, ReconcileTimeMetrics: timeMetrics}) _, err := ip.Reconcile() assert.Nil(t, err) @@ -486,7 +488,9 @@ func Test_PlaceHolderSecretsCreated_WhenPackageHasMultipleFetchStages(t *testing GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, countMetrics, timeMetrics) + ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, + FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, + &metrics.Metrics{ReconcileCountMetrics: countMetrics, ReconcileTimeMetrics: timeMetrics}) _, err := ip.Reconcile() assert.Nil(t, err) @@ -578,7 +582,9 @@ func Test_PlaceHolderSecretsNotCreated_WhenFetchStagesHaveSecrets(t *testing.T) GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, countMetrics, timeMetrics) + ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, + FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, + &metrics.Metrics{ReconcileCountMetrics: countMetrics, ReconcileTimeMetrics: timeMetrics}) _, err := ip.Reconcile() assert.Nil(t, err) @@ -657,7 +663,9 @@ func Test_PlaceHolderSecretCreated_WhenPackageInstallUpdated(t *testing.T) { fakekctrl := fakekappctrl.NewSimpleClientset(model, existingApp) fakek8s := fake.NewSimpleClientset() - ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, countMetrics, timeMetrics) + ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, + FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, + &metrics.Metrics{ReconcileCountMetrics: countMetrics, ReconcileTimeMetrics: timeMetrics}) // mock the kubernetes server version fakeDiscovery, _ := fakek8s.Discovery().(*fakediscovery.FakeDiscovery) diff --git a/pkg/packageinstall/reconciler.go b/pkg/packageinstall/reconciler.go index bb00af117..99637679b 100644 --- a/pkg/packageinstall/reconciler.go +++ b/pkg/packageinstall/reconciler.go @@ -33,15 +33,13 @@ type Reconciler struct { compInfo ComponentInfo log logr.Logger kcConfig *kcconfig.Config - timeMetrics *metrics.ReconcileTimeMetrics - countMetrics *metrics.ReconcileCountMetrics + pkgMetrics *metrics.Metrics } // NewReconciler is the constructor for the Reconciler struct func NewReconciler(kcClient kcclient.Interface, pkgClient pkgclient.Interface, coreClient kubernetes.Interface, pkgToPkgInstallHandler *PackageInstallVersionHandler, - log logr.Logger, compInfo ComponentInfo, kcConfig *kcconfig.Config, countMetrics *metrics.ReconcileCountMetrics, - timeMetrics *metrics.ReconcileTimeMetrics) *Reconciler { + log logr.Logger, compInfo ComponentInfo, kcConfig *kcconfig.Config, pkgMetrics *metrics.Metrics) *Reconciler { return &Reconciler{kcClient: kcClient, pkgClient: pkgClient, @@ -50,8 +48,7 @@ func NewReconciler(kcClient kcclient.Interface, pkgClient pkgclient.Interface, compInfo: compInfo, log: log, kcConfig: kcConfig, - countMetrics: countMetrics, - timeMetrics: timeMetrics, + pkgMetrics: pkgMetrics, } } @@ -94,5 +91,5 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) ( } return NewPackageInstallCR(existingPkgInstall, log, r.kcClient, r.pkgClient, r.coreClient, r.compInfo, - Opts{DefaultSyncPeriod: r.kcConfig.PackageInstallDefaultSyncPeriod()}, r.countMetrics, r.timeMetrics).Reconcile() + Opts{DefaultSyncPeriod: r.kcConfig.PackageInstallDefaultSyncPeriod()}, r.pkgMetrics).Reconcile() } diff --git a/pkg/pkgrepository/app.go b/pkg/pkgrepository/app.go index 69b2545cf..3f0bc1c8c 100644 --- a/pkg/pkgrepository/app.go +++ b/pkg/pkgrepository/app.go @@ -30,8 +30,7 @@ type App struct { templateFactory template.Factory deployFactory deploy.Factory - timeMetrics *metrics.ReconcileTimeMetrics - countMetrics *metrics.ReconcileCountMetrics + appMetrics *metrics.Metrics log logr.Logger @@ -42,10 +41,10 @@ type App struct { // NewApp creates a new instance of an App based on v1alpha1.App func NewApp(app v1alpha1.App, hooks Hooks, fetchFactory fetch.Factory, templateFactory template.Factory, deployFactory deploy.Factory, - log logr.Logger, countMetrics *metrics.ReconcileCountMetrics, timeMetrics *metrics.ReconcileTimeMetrics, pkgRepoUID types.UID) *App { + log logr.Logger, appMetrics *metrics.Metrics, pkgRepoUID types.UID) *App { return &App{app: app, appPrev: *(app.DeepCopy()), hooks: hooks, fetchFactory: fetchFactory, templateFactory: templateFactory, - deployFactory: deployFactory, log: log, countMetrics: countMetrics, timeMetrics: timeMetrics, pkgRepoUID: pkgRepoUID} + deployFactory: deployFactory, appMetrics: appMetrics, log: log, pkgRepoUID: pkgRepoUID} } func (a *App) Name() string { return a.app.Name } diff --git a/pkg/pkgrepository/app_factory.go b/pkg/pkgrepository/app_factory.go index f2f194b21..198ce4fed 100644 --- a/pkg/pkgrepository/app_factory.go +++ b/pkg/pkgrepository/app_factory.go @@ -23,14 +23,13 @@ import ( // AppFactory allows to create "hidden" Apps for reconciling PackageRepositories. type AppFactory struct { - CoreClient kubernetes.Interface - AppClient kcclient.Interface - TimeMetrics *metrics.ReconcileTimeMetrics - CountMetrics *metrics.ReconcileCountMetrics - KcConfig *config.Config - CmdRunner exec.CmdRunner - Kubeconf *kubeconfig.Kubeconfig - CacheFolder *memdir.TmpDir + CoreClient kubernetes.Interface + AppClient kcclient.Interface + AppMetrics *metrics.Metrics + KcConfig *config.Config + CmdRunner exec.CmdRunner + Kubeconf *kubeconfig.Kubeconfig + CacheFolder *memdir.TmpDir } // NewCRDPackageRepo constructs "hidden" App to reconcile PackageRepository. @@ -42,5 +41,5 @@ func (f *AppFactory) NewCRDPackageRepo(app *kcv1alpha1.App, pkgr *pkgv1alpha1.Pa fetchFactory := fetch.NewFactory(f.CoreClient, vendirOpts, f.CmdRunner) templateFactory := template.NewFactory(f.CoreClient, fetchFactory, false, f.CmdRunner) deployFactory := deploy.NewFactory(f.CoreClient, f.Kubeconf, nil, f.CmdRunner, log) - return NewCRDApp(app, pkgr, log, f.AppClient, f.TimeMetrics, f.CountMetrics, fetchFactory, templateFactory, deployFactory) + return NewCRDApp(app, pkgr, log, f.AppClient, f.AppMetrics, fetchFactory, templateFactory, deployFactory) } diff --git a/pkg/pkgrepository/app_reconcile.go b/pkg/pkgrepository/app_reconcile.go index 2bec61024..5386fd428 100644 --- a/pkg/pkgrepository/app_reconcile.go +++ b/pkg/pkgrepository/app_reconcile.go @@ -23,7 +23,7 @@ func (a *App) Reconcile(force bool) (reconcile.Result, error) { var err error - a.countMetrics.InitMetrics("pkgr", a.Name(), a.Namespace()) + a.appMetrics.ReconcileCountMetrics.InitMetrics("pkgr", a.Name(), a.Namespace()) switch { case a.app.Spec.Paused: @@ -95,9 +95,9 @@ func (a *App) reconcileDeploy() error { func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { reconcileStartTime := time.Now() - a.isFirstReconcile = a.countMetrics.GetReconcileAttemptCounterValue(packageRepoResourceType, a.app.Name, a.app.Namespace) == 1 + a.isFirstReconcile = a.appMetrics.ReconcileCountMetrics.GetReconcileAttemptCounterValue(packageRepoResourceType, a.app.Name, a.app.Namespace) == 1 defer func() { - a.timeMetrics.RegisterOverallTime(packageRepoResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterOverallTime(packageRepoResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, time.Since(reconcileStartTime)) }() @@ -127,7 +127,7 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.timeMetrics.RegisterFetchTime(packageRepoResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterFetchTime(packageRepoResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, a.app.Status.Fetch.UpdatedAt.Sub(a.app.Status.Fetch.StartedAt.Time)) err := a.updateStatus("marking fetch completed") @@ -151,7 +151,7 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.timeMetrics.RegisterTemplateTime(packageRepoResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterTemplateTime(packageRepoResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, a.app.Status.Template.UpdatedAt.Sub(templateStartTime)) err = a.updateStatus("marking template completed") @@ -181,7 +181,7 @@ func (a *App) updateLastDeploy(result exec.CmdRunResult) exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.timeMetrics.RegisterDeployTime(packageRepoResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterDeployTime(packageRepoResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, a.Status().Deploy.UpdatedAt.Sub(a.Status().Deploy.StartedAt.Time)) a.updateStatus("marking last deploy") @@ -215,7 +215,7 @@ func (a *App) setReconciling() { Status: corev1.ConditionTrue, }) - a.countMetrics.RegisterReconcileAttempt("pkgr", a.app.Name, a.app.Namespace) + a.appMetrics.ReconcileCountMetrics.RegisterReconcileAttempt("pkgr", a.app.Name, a.app.Namespace) a.app.Status.FriendlyDescription = "Reconciling" } diff --git a/pkg/pkgrepository/app_reconcile_test.go b/pkg/pkgrepository/app_reconcile_test.go index 6b22252d4..d93938e93 100644 --- a/pkg/pkgrepository/app_reconcile_test.go +++ b/pkg/pkgrepository/app_reconcile_test.go @@ -49,7 +49,8 @@ func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) { deployFac := deploy.NewFactory(k8scs, kubeconfig.NewKubeconfig(k8scs, log), nil, exec.NewPlainCmdRunner(), log) pkgr := v1alpha12.PackageRepository{} - crdApp := NewCRDApp(&app, &pkgr, log, kappcs, timeMetrics, appMetrics, fetchFac, tmpFac, deployFac) + crdApp := NewCRDApp(&app, &pkgr, log, kappcs, &metrics.Metrics{ReconcileCountMetrics: appMetrics, ReconcileTimeMetrics: timeMetrics}, + fetchFac, tmpFac, deployFac) _, err := crdApp.Reconcile(false) if err != nil { t.Fatalf("Unexpected error with reconciling: %v", err) @@ -107,7 +108,8 @@ func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing. deployFac := deploy.NewFactory(k8scs, kubeconfig.NewKubeconfig(k8scs, log), nil, exec.NewPlainCmdRunner(), log) pkgr := v1alpha12.PackageRepository{} - crdApp := NewCRDApp(&app, &pkgr, log, kappcs, timeMetrics, appMetrics, fetchFac, tmpFac, deployFac) + crdApp := NewCRDApp(&app, &pkgr, log, kappcs, &metrics.Metrics{ReconcileCountMetrics: appMetrics, ReconcileTimeMetrics: timeMetrics}, + fetchFac, tmpFac, deployFac) _, err := crdApp.Reconcile(false) if err != nil { t.Fatalf("Unexpected error with reconciling: %v", err) @@ -165,7 +167,8 @@ func TestInvalidPackageRepositoryFormat(t *testing.T) { deployFac := deploy.NewFactory(k8scs, kubeconfig.NewKubeconfig(k8scs, log), nil, exec.NewPlainCmdRunner(), log) pkgr := v1alpha12.PackageRepository{} - crdApp := NewCRDApp(&app, &pkgr, log, kappcs, timeMetrics, appMetrics, fetchFac, tmpFac, deployFac) + crdApp := NewCRDApp(&app, &pkgr, log, kappcs, &metrics.Metrics{ReconcileCountMetrics: appMetrics, ReconcileTimeMetrics: timeMetrics}, + fetchFac, tmpFac, deployFac) _, err := crdApp.Reconcile(false) if err != nil { t.Fatalf("Unexpected error with reconciling: %v", err) diff --git a/pkg/pkgrepository/crd_app.go b/pkg/pkgrepository/crd_app.go index ca7be6a32..a85c56fd3 100644 --- a/pkg/pkgrepository/crd_app.go +++ b/pkg/pkgrepository/crd_app.go @@ -29,7 +29,7 @@ type CRDApp struct { } func NewCRDApp(appModel *kcv1alpha1.App, packageRepo *pkgingv1alpha1.PackageRepository, log logr.Logger, - appClient kcclient.Interface, timeMetrics *metrics.ReconcileTimeMetrics, countMetrics *metrics.ReconcileCountMetrics, fetchFactory fetch.Factory, + appClient kcclient.Interface, appMetrics *metrics.Metrics, fetchFactory fetch.Factory, templateFactory template.Factory, deployFactory deploy.Factory) *CRDApp { crdApp := &CRDApp{appModel: appModel, pkgrModel: packageRepo, log: log, appClient: appClient} @@ -38,7 +38,7 @@ func NewCRDApp(appModel *kcv1alpha1.App, packageRepo *pkgingv1alpha1.PackageRepo BlockDeletion: crdApp.blockDeletion, UnblockDeletion: crdApp.unblockDeletion, UpdateStatus: crdApp.updateStatus, - }, fetchFactory, templateFactory, deployFactory, log, countMetrics, timeMetrics, packageRepo.UID) + }, fetchFactory, templateFactory, deployFactory, log, appMetrics, packageRepo.UID) return crdApp } From 621f9470cca84a5e5e73d407447e70e3005b4b02 Mon Sep 17 00:00:00 2001 From: sethiyash Date: Wed, 3 Jan 2024 13:46:51 +0530 Subject: [PATCH 6/9] Using name namespace methods and refactor imports Signed-off-by: sethiyash --- cmd/controller/run.go | 8 +++---- pkg/app/app.go | 1 - pkg/app/app_deploy.go | 1 + pkg/app/app_fetch.go | 1 - pkg/app/app_reconcile.go | 22 +++++++++---------- pkg/app/app_template.go | 3 ++- pkg/app/crd_app.go | 11 +++++----- pkg/metrics/metrics.go | 1 + pkg/packageinstall/app.go | 5 +++-- pkg/packageinstall/packageinstall.go | 8 +++---- .../packageinstall_deletion_test.go | 2 +- .../packageinstall_downgrade_test.go | 2 +- pkg/packageinstall/packageinstall_test.go | 2 +- pkg/packageinstall/reconciler.go | 1 + pkg/pkgrepository/app.go | 1 - pkg/pkgrepository/app_deploy.go | 1 + pkg/pkgrepository/app_factory.go | 2 +- pkg/pkgrepository/app_reconcile.go | 14 ++++++------ 18 files changed, 43 insertions(+), 43 deletions(-) diff --git a/cmd/controller/run.go b/cmd/controller/run.go index 243552c53..4ed897a27 100644 --- a/cmd/controller/run.go +++ b/cmd/controller/run.go @@ -189,6 +189,7 @@ 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} cacheFolderApps := memdir.NewTmpDir("cache-appcr") err = cacheFolderApps.Create() @@ -200,7 +201,7 @@ func Run(opts Options, runLog logr.Logger) error { CoreClient: coreClient, AppClient: kcClient, KcConfig: kcConfig, - AppMetrics: &metrics.Metrics{ReconcileCountMetrics: countMetrics, ReconcileTimeMetrics: reconcileTimeMetrics}, + AppMetrics: appMetrics, CmdRunner: sidecarCmdExec, Kubeconf: kubeconf, CompInfo: compInfo, @@ -231,8 +232,7 @@ func Run(opts Options, runLog logr.Logger) error { kcClient, opts.PackagingGlobalNS, runLog.WithName("handler")) reconciler := pkginstall.NewReconciler(kcClient, pkgClient, coreClient, pkgToPkgInstallHandler, - runLog.WithName("pkgi"), compInfo, kcConfig, &metrics.Metrics{ReconcileCountMetrics: countMetrics, - ReconcileTimeMetrics: reconcileTimeMetrics}) + runLog.WithName("pkgi"), compInfo, kcConfig, appMetrics) ctrl, err := controller.New("pkgi", mgr, controller.Options{ Reconciler: reconciler, @@ -259,7 +259,7 @@ func Run(opts Options, runLog logr.Logger) error { CoreClient: coreClient, AppClient: kcClient, KcConfig: kcConfig, - AppMetrics: &metrics.Metrics{ReconcileCountMetrics: countMetrics, ReconcileTimeMetrics: reconcileTimeMetrics}, + AppMetrics: appMetrics, CmdRunner: sidecarCmdExec, Kubeconf: kubeconf, CacheFolder: cacheFolderPkgRepoApps, diff --git a/pkg/app/app.go b/pkg/app/app.go index f68f8d7a3..23d626fd5 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -59,7 +59,6 @@ type App struct { opts Opts appMetrics *metrics.Metrics - isFirstReconcile bool pendingStatusUpdate bool flushAllStatusUpdates bool metadata *deploy.Meta diff --git a/pkg/app/app_deploy.go b/pkg/app/app_deploy.go index 33e0dea42..9bc6afcc7 100644 --- a/pkg/app/app_deploy.go +++ b/pkg/app/app_deploy.go @@ -5,6 +5,7 @@ package app import ( "fmt" + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" ctldep "github.com/vmware-tanzu/carvel-kapp-controller/pkg/deploy" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/exec" diff --git a/pkg/app/app_fetch.go b/pkg/app/app_fetch.go index 07a12f0e1..fd0ca8eab 100644 --- a/pkg/app/app_fetch.go +++ b/pkg/app/app_fetch.go @@ -20,7 +20,6 @@ const ( ) func (a *App) fetch(dstPath string) (string, exec.CmdRunResult) { - // fetch init stage if len(a.app.Spec.Fetch) == 0 { return "", exec.NewCmdRunResultWithErr(fmt.Errorf("Expected at least one fetch option")) } diff --git a/pkg/app/app_reconcile.go b/pkg/app/app_reconcile.go index 260acf856..d80809f48 100644 --- a/pkg/app/app_reconcile.go +++ b/pkg/app/app_reconcile.go @@ -106,9 +106,9 @@ func (a *App) reconcileDeploy() error { func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { reconcileStartTime := time.Now() - a.isFirstReconcile = a.appMetrics.ReconcileCountMetrics.GetReconcileAttemptCounterValue("app", a.app.Name, a.app.Namespace) == 1 + a.appMetrics.IsFirstReconcile = a.appMetrics.ReconcileCountMetrics.GetReconcileAttemptCounterValue(appResourceType, a.Name(), a.Namespace()) == 1 defer func() { - a.appMetrics.ReconcileTimeMetrics.RegisterOverallTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterOverallTime(appResourceType, a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, time.Since(reconcileStartTime)) }() @@ -138,7 +138,7 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.appMetrics.ReconcileTimeMetrics.RegisterFetchTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterFetchTime(appResourceType, a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, a.app.Status.Fetch.UpdatedAt.Sub(a.app.Status.Fetch.StartedAt.Time)) err := a.updateStatus("marking fetch completed") @@ -162,7 +162,7 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.appMetrics.ReconcileTimeMetrics.RegisterTemplateTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterTemplateTime(appResourceType, a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, a.app.Status.Template.UpdatedAt.Sub(templateStartTime)) err = a.updateStatus("marking template completed") @@ -213,7 +213,7 @@ func (a *App) updateLastDeploy(result exec.CmdRunResult) exec.CmdRunResult { }, } - a.appMetrics.ReconcileTimeMetrics.RegisterDeployTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterDeployTime(appResourceType, a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, a.Status().Deploy.UpdatedAt.Sub(a.Status().Deploy.StartedAt.Time)) return result @@ -267,7 +267,7 @@ func (a *App) setReconciling() { Status: corev1.ConditionTrue, }) - a.appMetrics.ReconcileCountMetrics.RegisterReconcileAttempt(appResourceType, a.app.Name, a.app.Namespace) + a.appMetrics.ReconcileCountMetrics.RegisterReconcileAttempt(appResourceType, a.Name(), a.Namespace()) a.app.Status.FriendlyDescription = "Reconciling" } @@ -283,7 +283,7 @@ func (a *App) setReconcileCompleted(result exec.CmdRunResult) { a.app.Status.ConsecutiveReconcileFailures++ a.app.Status.ConsecutiveReconcileSuccesses = 0 a.app.Status.FriendlyDescription = fmt.Sprintf("Reconcile failed: %s", result.ErrorStr()) - a.appMetrics.ReconcileCountMetrics.RegisterReconcileFailure(appResourceType, a.app.Name, a.app.Namespace) + a.appMetrics.ReconcileCountMetrics.RegisterReconcileFailure(appResourceType, a.Name(), a.Namespace()) a.setUsefulErrorMessage(result) } else { a.app.Status.Conditions = append(a.app.Status.Conditions, v1alpha1.Condition{ @@ -294,7 +294,7 @@ func (a *App) setReconcileCompleted(result exec.CmdRunResult) { a.app.Status.ConsecutiveReconcileSuccesses++ a.app.Status.ConsecutiveReconcileFailures = 0 a.app.Status.FriendlyDescription = "Reconcile succeeded" - a.appMetrics.ReconcileCountMetrics.RegisterReconcileSuccess(appResourceType, a.app.Name, a.app.Namespace) + a.appMetrics.ReconcileCountMetrics.RegisterReconcileSuccess(appResourceType, a.Name(), a.Namespace()) a.app.Status.UsefulErrorMessage = "" } } @@ -307,7 +307,7 @@ func (a *App) setDeleting() { Status: corev1.ConditionTrue, }) - a.appMetrics.ReconcileCountMetrics.RegisterReconcileDeleteAttempt(appResourceType, a.app.Name, a.app.Namespace) + a.appMetrics.ReconcileCountMetrics.RegisterReconcileDeleteAttempt(appResourceType, a.Name(), a.Namespace()) a.app.Status.FriendlyDescription = "Deleting" } @@ -323,10 +323,10 @@ func (a *App) setDeleteCompleted(result exec.CmdRunResult) { a.app.Status.ConsecutiveReconcileFailures++ a.app.Status.ConsecutiveReconcileSuccesses = 0 a.app.Status.FriendlyDescription = fmt.Sprintf("Delete failed: %s", result.ErrorStr()) - a.appMetrics.ReconcileCountMetrics.RegisterReconcileDeleteFailed(appResourceType, a.app.Name, a.app.Namespace) + a.appMetrics.ReconcileCountMetrics.RegisterReconcileDeleteFailed(appResourceType, a.Name(), a.Namespace()) a.setUsefulErrorMessage(result) } else { - a.appMetrics.ReconcileCountMetrics.DeleteMetrics(appResourceType, a.app.Name, a.app.Namespace) + a.appMetrics.ReconcileCountMetrics.DeleteMetrics(appResourceType, a.Name(), a.Namespace()) } } diff --git a/pkg/app/app_template.go b/pkg/app/app_template.go index 6c296a694..91b335aa5 100644 --- a/pkg/app/app_template.go +++ b/pkg/app/app_template.go @@ -5,10 +5,11 @@ package app import ( "fmt" + "strings" + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/exec" ctltpl "github.com/vmware-tanzu/carvel-kapp-controller/pkg/template" - "strings" ) func (a *App) template(dirPath string) exec.CmdRunResult { diff --git a/pkg/app/crd_app.go b/pkg/app/crd_app.go index 20d1cf91e..0b3fcbe7d 100644 --- a/pkg/app/crd_app.go +++ b/pkg/app/crd_app.go @@ -21,11 +21,10 @@ import ( ) type CRDApp struct { - app *App - appModel *kcv1alpha1.App - log logr.Logger - countMetrics *metrics.ReconcileCountMetrics - appClient kcclient.Interface + app *App + appModel *kcv1alpha1.App + log logr.Logger + appClient kcclient.Interface } // NewCRDApp creates new CRD app @@ -33,7 +32,7 @@ func NewCRDApp(appModel *kcv1alpha1.App, log logr.Logger, appMetrics *metrics.Me templateFactory template.Factory, deployFactory deploy.Factory, compInfo ComponentInfo, opts Opts) *CRDApp { - crdApp := &CRDApp{appModel: appModel, log: log, countMetrics: appMetrics.ReconcileCountMetrics, appClient: appClient} + crdApp := &CRDApp{appModel: appModel, log: log, appClient: appClient} crdApp.app = NewApp(*appModel, Hooks{ BlockDeletion: crdApp.blockDeletion, diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index 7fbf6c75a..4434534e9 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -7,4 +7,5 @@ package metrics type Metrics struct { *ReconcileCountMetrics *ReconcileTimeMetrics + IsFirstReconcile bool } diff --git a/pkg/packageinstall/app.go b/pkg/packageinstall/app.go index 23ce2c658..23ce547d7 100644 --- a/pkg/packageinstall/app.go +++ b/pkg/packageinstall/app.go @@ -5,6 +5,9 @@ package packageinstall import ( "fmt" + "sort" + "strings" + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" kcv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" pkgingv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/packaging/v1alpha1" @@ -12,8 +15,6 @@ import ( "github.com/vmware-tanzu/carvel-kapp-controller/pkg/client/clientset/versioned/scheme" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sort" - "strings" ) const ( diff --git a/pkg/packageinstall/packageinstall.go b/pkg/packageinstall/packageinstall.go index 0b7b6cd5c..2ae9614fb 100644 --- a/pkg/packageinstall/packageinstall.go +++ b/pkg/packageinstall/packageinstall.go @@ -6,7 +6,6 @@ package packageinstall import ( "context" "fmt" - "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "time" "github.com/go-logr/logr" @@ -18,6 +17,7 @@ import ( pkgclient "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/client/clientset/versioned" kcclient "github.com/vmware-tanzu/carvel-kapp-controller/pkg/client/clientset/versioned" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/client/clientset/versioned/scheme" + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/reconciler" "github.com/vmware-tanzu/carvel-vendir/pkg/vendir/versions" verv1alpha1 "github.com/vmware-tanzu/carvel-vendir/pkg/vendir/versions/v1alpha1" @@ -58,8 +58,6 @@ type PackageInstallCR struct { opts Opts pkgMetrics *metrics.Metrics - - firstReconcile bool } // nolint: revive @@ -112,10 +110,10 @@ func (pi *PackageInstallCR) reconcile(modelStatus *reconciler.Status) (reconcile pi.pkgMetrics.ReconcileCountMetrics.RegisterReconcileAttempt(packageInstallType, pi.model.Name, pi.model.Namespace) reconcileStartTime := time.Now() - pi.firstReconcile = pi.pkgMetrics.ReconcileCountMetrics.GetReconcileAttemptCounterValue("pkgi", pi.model.Name, pi.model.Namespace) == 1 + pi.pkgMetrics.IsFirstReconcile = pi.pkgMetrics.ReconcileCountMetrics.GetReconcileAttemptCounterValue(packageInstallType, pi.model.Name, pi.model.Namespace) == 1 defer func() { pi.pkgMetrics.ReconcileTimeMetrics.RegisterOverallTime(packageInstallType, pi.model.Name, pi.model.Namespace, - pi.firstReconcile, time.Since(reconcileStartTime)) + pi.pkgMetrics.IsFirstReconcile, time.Since(reconcileStartTime)) }() err := pi.blockDeletion() diff --git a/pkg/packageinstall/packageinstall_deletion_test.go b/pkg/packageinstall/packageinstall_deletion_test.go index 1e574adcc..3545808fd 100644 --- a/pkg/packageinstall/packageinstall_deletion_test.go +++ b/pkg/packageinstall/packageinstall_deletion_test.go @@ -4,7 +4,6 @@ package packageinstall import ( - "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "testing" "github.com/k14s/semver/v4" @@ -13,6 +12,7 @@ import ( pkgingv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/packaging/v1alpha1" fakeapiserver "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/client/clientset/versioned/fake" fakekappctrl "github.com/vmware-tanzu/carvel-kapp-controller/pkg/client/clientset/versioned/fake" + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" versions "github.com/vmware-tanzu/carvel-vendir/pkg/vendir/versions/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" diff --git a/pkg/packageinstall/packageinstall_downgrade_test.go b/pkg/packageinstall/packageinstall_downgrade_test.go index c3a76ce24..fa6a089ef 100644 --- a/pkg/packageinstall/packageinstall_downgrade_test.go +++ b/pkg/packageinstall/packageinstall_downgrade_test.go @@ -4,7 +4,6 @@ package packageinstall import ( - "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "testing" "github.com/k14s/semver/v4" @@ -15,6 +14,7 @@ import ( datapkgingv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/apis/datapackaging/v1alpha1" fakeapiserver "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/client/clientset/versioned/fake" fakekappctrl "github.com/vmware-tanzu/carvel-kapp-controller/pkg/client/clientset/versioned/fake" + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" versions "github.com/vmware-tanzu/carvel-vendir/pkg/vendir/versions/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" diff --git a/pkg/packageinstall/packageinstall_test.go b/pkg/packageinstall/packageinstall_test.go index c217c5c99..69677d6e7 100644 --- a/pkg/packageinstall/packageinstall_test.go +++ b/pkg/packageinstall/packageinstall_test.go @@ -5,7 +5,6 @@ package packageinstall import ( "fmt" - "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "reflect" "testing" @@ -17,6 +16,7 @@ import ( datapkgingv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/apis/datapackaging/v1alpha1" fakeapiserver "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/client/clientset/versioned/fake" fakekappctrl "github.com/vmware-tanzu/carvel-kapp-controller/pkg/client/clientset/versioned/fake" + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" versions "github.com/vmware-tanzu/carvel-vendir/pkg/vendir/versions/v1alpha1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" diff --git a/pkg/packageinstall/reconciler.go b/pkg/packageinstall/reconciler.go index 99637679b..aacf59d7d 100644 --- a/pkg/packageinstall/reconciler.go +++ b/pkg/packageinstall/reconciler.go @@ -6,6 +6,7 @@ package packageinstall import ( "context" "fmt" + "github.com/go-logr/logr" kappctrlv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" pkgingv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/packaging/v1alpha1" diff --git a/pkg/pkgrepository/app.go b/pkg/pkgrepository/app.go index 3f0bc1c8c..e4304821a 100644 --- a/pkg/pkgrepository/app.go +++ b/pkg/pkgrepository/app.go @@ -34,7 +34,6 @@ type App struct { log logr.Logger - isFirstReconcile bool pendingStatusUpdate bool flushAllStatusUpdates bool } diff --git a/pkg/pkgrepository/app_deploy.go b/pkg/pkgrepository/app_deploy.go index 7b8b236e4..1f5bc6131 100644 --- a/pkg/pkgrepository/app_deploy.go +++ b/pkg/pkgrepository/app_deploy.go @@ -5,6 +5,7 @@ package pkgrepository import ( "fmt" + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" ctldep "github.com/vmware-tanzu/carvel-kapp-controller/pkg/deploy" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/exec" diff --git a/pkg/pkgrepository/app_factory.go b/pkg/pkgrepository/app_factory.go index 198ce4fed..930d29cf3 100644 --- a/pkg/pkgrepository/app_factory.go +++ b/pkg/pkgrepository/app_factory.go @@ -4,7 +4,6 @@ package pkgrepository import ( - "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "path/filepath" "github.com/go-logr/logr" @@ -17,6 +16,7 @@ import ( "github.com/vmware-tanzu/carvel-kapp-controller/pkg/fetch" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/kubeconfig" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/memdir" + "github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/template" "k8s.io/client-go/kubernetes" ) diff --git a/pkg/pkgrepository/app_reconcile.go b/pkg/pkgrepository/app_reconcile.go index 5386fd428..3013436f5 100644 --- a/pkg/pkgrepository/app_reconcile.go +++ b/pkg/pkgrepository/app_reconcile.go @@ -23,7 +23,7 @@ func (a *App) Reconcile(force bool) (reconcile.Result, error) { var err error - a.appMetrics.ReconcileCountMetrics.InitMetrics("pkgr", a.Name(), a.Namespace()) + a.appMetrics.ReconcileCountMetrics.InitMetrics(packageRepoResourceType, a.Name(), a.Namespace()) switch { case a.app.Spec.Paused: @@ -95,9 +95,9 @@ func (a *App) reconcileDeploy() error { func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { reconcileStartTime := time.Now() - a.isFirstReconcile = a.appMetrics.ReconcileCountMetrics.GetReconcileAttemptCounterValue(packageRepoResourceType, a.app.Name, a.app.Namespace) == 1 + a.appMetrics.IsFirstReconcile = a.appMetrics.ReconcileCountMetrics.GetReconcileAttemptCounterValue(packageRepoResourceType, a.Name(), a.Namespace()) == 1 defer func() { - a.appMetrics.ReconcileTimeMetrics.RegisterOverallTime(packageRepoResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterOverallTime(packageRepoResourceType, a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, time.Since(reconcileStartTime)) }() @@ -127,7 +127,7 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.appMetrics.ReconcileTimeMetrics.RegisterFetchTime(packageRepoResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterFetchTime(packageRepoResourceType, a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, a.app.Status.Fetch.UpdatedAt.Sub(a.app.Status.Fetch.StartedAt.Time)) err := a.updateStatus("marking fetch completed") @@ -151,7 +151,7 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.appMetrics.ReconcileTimeMetrics.RegisterTemplateTime(packageRepoResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterTemplateTime(packageRepoResourceType, a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, a.app.Status.Template.UpdatedAt.Sub(templateStartTime)) err = a.updateStatus("marking template completed") @@ -181,7 +181,7 @@ func (a *App) updateLastDeploy(result exec.CmdRunResult) exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.appMetrics.ReconcileTimeMetrics.RegisterDeployTime(packageRepoResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterDeployTime(packageRepoResourceType, a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, a.Status().Deploy.UpdatedAt.Sub(a.Status().Deploy.StartedAt.Time)) a.updateStatus("marking last deploy") @@ -215,7 +215,7 @@ func (a *App) setReconciling() { Status: corev1.ConditionTrue, }) - a.appMetrics.ReconcileCountMetrics.RegisterReconcileAttempt("pkgr", a.app.Name, a.app.Namespace) + a.appMetrics.ReconcileCountMetrics.RegisterReconcileAttempt(packageRepoResourceType, a.Name(), a.Namespace()) a.app.Status.FriendlyDescription = "Reconciling" } From 68d3e5db1ef7bd466470b5f7567d1e297942341d Mon Sep 17 00:00:00 2001 From: sethiyash Date: Fri, 5 Jan 2024 00:42:50 +0530 Subject: [PATCH 7/9] making metrics port configurable and constructor for metrics Signed-off-by: sethiyash --- cmd/controller/run.go | 13 +++++-------- config/config/deployment.yml | 2 +- config/values-schema.yml | 2 ++ pkg/app/app_reconcile_test.go | 18 +++--------------- pkg/app/app_template_test.go | 2 +- pkg/app/app_test.go | 8 ++++---- pkg/metrics/metrics.go | 9 +++++++++ pkg/metrics/reconcile_time_metrics.go | 5 +++-- .../packageinstall_deletion_test.go | 2 +- .../packageinstall_downgrade_test.go | 10 ++++------ pkg/packageinstall/packageinstall_test.go | 16 ++++------------ pkg/pkgrepository/app_reconcile_test.go | 12 +++--------- 12 files changed, 40 insertions(+), 59 deletions(-) diff --git a/cmd/controller/run.go b/cmd/controller/run.go index 4ed897a27..223c36f8b 100644 --- a/cmd/controller/run.go +++ b/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() diff --git a/config/config/deployment.yml b/config/config/deployment.yml index 6c56e3c7f..2aabfe770 100644 --- a/config/config/deployment.yml +++ b/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: diff --git a/config/values-schema.yml b/config/values-schema.yml index 1504d9be7..fecbd8469 100644 --- a/config/values-schema.yml +++ b/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." diff --git a/pkg/app/app_reconcile_test.go b/pkg/app/app_reconcile_test.go index e7d4808de..e5cbab246 100644 --- a/pkg/app/app_reconcile_test.go +++ b/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) diff --git a/pkg/app/app_template_test.go b/pkg/app/app_template_test.go index 77a081958..4e3941d31 100644 --- a/pkg/app/app_template_test.go +++ b/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) diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index 91fa25a4c..522147f44 100644 --- a/pkg/app/app_test.go +++ b/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) diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index 4434534e9..df69bd1f3 100644 --- a/pkg/metrics/metrics.go +++ b/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, + } +} diff --git a/pkg/metrics/reconcile_time_metrics.go b/pkg/metrics/reconcile_time_metrics.go index eaae34230..06ac30352 100644 --- a/pkg/metrics/reconcile_time_metrics.go +++ b/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 diff --git a/pkg/packageinstall/packageinstall_deletion_test.go b/pkg/packageinstall/packageinstall_deletion_test.go index 3545808fd..ba8ab4d7d 100644 --- a/pkg/packageinstall/packageinstall_deletion_test.go +++ b/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) diff --git a/pkg/packageinstall/packageinstall_downgrade_test.go b/pkg/packageinstall/packageinstall_downgrade_test.go index fa6a089ef..be0265a43 100644 --- a/pkg/packageinstall/packageinstall_downgrade_test.go +++ b/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) diff --git a/pkg/packageinstall/packageinstall_test.go b/pkg/packageinstall/packageinstall_test.go index 69677d6e7..7a5c576e9 100644 --- a/pkg/packageinstall/packageinstall_test.go +++ b/pkg/packageinstall/packageinstall_test.go @@ -354,8 +354,6 @@ func Test_PackageRefUsesName(t *testing.T) { } func Test_PlaceHolderSecretCreated_WhenPackageHasNoSecretRef(t *testing.T) { - var timeMetrics = metrics.NewReconcileTimeMetrics() - var countMetrics = metrics.NewCountMetrics() pkg := datapkgingv1alpha1.Package{ ObjectMeta: metav1.ObjectMeta{ Name: "expected-pkg", @@ -407,7 +405,7 @@ func Test_PlaceHolderSecretCreated_WhenPackageHasNoSecretRef(t *testing.T) { ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, - &metrics.Metrics{ReconcileCountMetrics: countMetrics, ReconcileTimeMetrics: timeMetrics}) + metrics.NewMetrics()) _, err := ip.Reconcile() assert.Nil(t, err) @@ -433,8 +431,6 @@ func Test_PlaceHolderSecretCreated_WhenPackageHasNoSecretRef(t *testing.T) { } func Test_PlaceHolderSecretsCreated_WhenPackageHasMultipleFetchStages(t *testing.T) { - var timeMetrics = metrics.NewReconcileTimeMetrics() - var countMetrics = metrics.NewCountMetrics() pkg := datapkgingv1alpha1.Package{ ObjectMeta: metav1.ObjectMeta{ Name: "expected-pkg", @@ -490,7 +486,7 @@ func Test_PlaceHolderSecretsCreated_WhenPackageHasMultipleFetchStages(t *testing ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, - &metrics.Metrics{ReconcileCountMetrics: countMetrics, ReconcileTimeMetrics: timeMetrics}) + metrics.NewMetrics()) _, err := ip.Reconcile() assert.Nil(t, err) @@ -526,8 +522,6 @@ func Test_PlaceHolderSecretsCreated_WhenPackageHasMultipleFetchStages(t *testing } func Test_PlaceHolderSecretsNotCreated_WhenFetchStagesHaveSecrets(t *testing.T) { - var timeMetrics = metrics.NewReconcileTimeMetrics() - var countMetrics = metrics.NewCountMetrics() pkg := datapkgingv1alpha1.Package{ ObjectMeta: metav1.ObjectMeta{ Name: "expected-pkg", @@ -584,7 +578,7 @@ func Test_PlaceHolderSecretsNotCreated_WhenFetchStagesHaveSecrets(t *testing.T) ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, - &metrics.Metrics{ReconcileCountMetrics: countMetrics, ReconcileTimeMetrics: timeMetrics}) + metrics.NewMetrics()) _, err := ip.Reconcile() assert.Nil(t, err) @@ -604,8 +598,6 @@ func Test_PlaceHolderSecretsNotCreated_WhenFetchStagesHaveSecrets(t *testing.T) } func Test_PlaceHolderSecretCreated_WhenPackageInstallUpdated(t *testing.T) { - var timeMetrics = metrics.NewReconcileTimeMetrics() - var countMetrics = metrics.NewCountMetrics() appSpec := v1alpha1.AppSpec{ Fetch: []v1alpha1.AppFetch{ { @@ -665,7 +657,7 @@ func Test_PlaceHolderSecretCreated_WhenPackageInstallUpdated(t *testing.T) { fakek8s := fake.NewSimpleClientset() ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, - &metrics.Metrics{ReconcileCountMetrics: countMetrics, ReconcileTimeMetrics: timeMetrics}) + metrics.NewMetrics()) // mock the kubernetes server version fakeDiscovery, _ := fakek8s.Discovery().(*fakediscovery.FakeDiscovery) diff --git a/pkg/pkgrepository/app_reconcile_test.go b/pkg/pkgrepository/app_reconcile_test.go index d93938e93..6306b437c 100644 --- a/pkg/pkgrepository/app_reconcile_test.go +++ b/pkg/pkgrepository/app_reconcile_test.go @@ -25,8 +25,6 @@ import ( func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) { log := logf.Log.WithName("kc") - var timeMetrics = metrics.NewReconcileTimeMetrics() - var appMetrics = metrics.NewCountMetrics() // The url under fetch is invalid, which will cause this // app to fail before deploy. @@ -49,7 +47,7 @@ func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) { deployFac := deploy.NewFactory(k8scs, kubeconfig.NewKubeconfig(k8scs, log), nil, exec.NewPlainCmdRunner(), log) pkgr := v1alpha12.PackageRepository{} - crdApp := NewCRDApp(&app, &pkgr, log, kappcs, &metrics.Metrics{ReconcileCountMetrics: appMetrics, ReconcileTimeMetrics: timeMetrics}, + crdApp := NewCRDApp(&app, &pkgr, log, kappcs, metrics.NewMetrics(), fetchFac, tmpFac, deployFac) _, err := crdApp.Reconcile(false) if err != nil { @@ -82,8 +80,6 @@ func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) { func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing.T) { log := logf.Log.WithName("kc") - var timeMetrics = metrics.NewReconcileTimeMetrics() - var appMetrics = metrics.NewCountMetrics() fetchInline := map[string]string{ "packages/file.yml": `foo: #@ data.values.nothere`, @@ -108,7 +104,7 @@ func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing. deployFac := deploy.NewFactory(k8scs, kubeconfig.NewKubeconfig(k8scs, log), nil, exec.NewPlainCmdRunner(), log) pkgr := v1alpha12.PackageRepository{} - crdApp := NewCRDApp(&app, &pkgr, log, kappcs, &metrics.Metrics{ReconcileCountMetrics: appMetrics, ReconcileTimeMetrics: timeMetrics}, + crdApp := NewCRDApp(&app, &pkgr, log, kappcs, metrics.NewMetrics(), fetchFac, tmpFac, deployFac) _, err := crdApp.Reconcile(false) if err != nil { @@ -142,8 +138,6 @@ func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing. func TestInvalidPackageRepositoryFormat(t *testing.T) { log := logf.Log.WithName("kc") - var timeMetrics = metrics.NewReconcileTimeMetrics() - var appMetrics = metrics.NewCountMetrics() fetchInline := map[string]string{ "file.yml": `foo: hi`, @@ -167,7 +161,7 @@ func TestInvalidPackageRepositoryFormat(t *testing.T) { deployFac := deploy.NewFactory(k8scs, kubeconfig.NewKubeconfig(k8scs, log), nil, exec.NewPlainCmdRunner(), log) pkgr := v1alpha12.PackageRepository{} - crdApp := NewCRDApp(&app, &pkgr, log, kappcs, &metrics.Metrics{ReconcileCountMetrics: appMetrics, ReconcileTimeMetrics: timeMetrics}, + crdApp := NewCRDApp(&app, &pkgr, log, kappcs, metrics.NewMetrics(), fetchFac, tmpFac, deployFac) _, err := crdApp.Reconcile(false) if err != nil { From f9b21d34ad869bee82088ed5de14f5d6c9165604 Mon Sep 17 00:00:00 2001 From: sethiyash Date: Wed, 10 Jan 2024 22:47:10 +0530 Subject: [PATCH 8/9] Added E2E testcases for exposing prometheus metrics Signed-off-by: sethiyash Signed-off-by: Yash Sethiya --- pkg/app/app.go | 3 + pkg/app/app_reconcile.go | 26 ++- pkg/metrics/reconcile_count_metrics.go | 4 +- pkg/packageinstall/packageinstall.go | 14 +- pkg/pkgrepository/app.go | 3 + pkg/pkgrepository/app_reconcile.go | 16 +- test/e2e/kappcontroller/metrics_test.go | 205 ++++++++++++++++++++++++ 7 files changed, 241 insertions(+), 30 deletions(-) create mode 100644 test/e2e/kappcontroller/metrics_test.go diff --git a/pkg/app/app.go b/pkg/app/app.go index 23d626fd5..eeed9d676 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -76,6 +76,9 @@ func NewApp(app v1alpha1.App, hooks Hooks, func (a *App) Name() string { return a.app.Name } func (a *App) Namespace() string { return a.app.Namespace } +// Kind return kind of App +func (a *App) Kind() string { return "app" } + func (a *App) Status() v1alpha1.AppStatus { return a.app.Status } func (a *App) StatusAsYAMLBytes() ([]byte, error) { diff --git a/pkg/app/app_reconcile.go b/pkg/app/app_reconcile.go index d80809f48..d4c615a65 100644 --- a/pkg/app/app_reconcile.go +++ b/pkg/app/app_reconcile.go @@ -15,15 +15,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" ) -const appResourceType = "app" - // Reconcile is not expected to be called concurrently func (a *App) Reconcile(force bool) (reconcile.Result, error) { defer a.flushUpdateStatus("app reconciled") var err error - a.appMetrics.ReconcileCountMetrics.InitMetrics(appResourceType, a.Name(), a.Namespace()) + a.appMetrics.ReconcileCountMetrics.InitMetrics(a.Kind(), a.Name(), a.Namespace()) timerOpts := ReconcileTimerOpts{ DefaultSyncPeriod: a.opts.DefaultSyncPeriod, @@ -106,9 +104,9 @@ func (a *App) reconcileDeploy() error { func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { reconcileStartTime := time.Now() - a.appMetrics.IsFirstReconcile = a.appMetrics.ReconcileCountMetrics.GetReconcileAttemptCounterValue(appResourceType, a.Name(), a.Namespace()) == 1 + a.appMetrics.IsFirstReconcile = a.appMetrics.ReconcileCountMetrics.GetReconcileAttemptCounterValue(a.Kind(), a.Name(), a.Namespace()) == 1 defer func() { - a.appMetrics.ReconcileTimeMetrics.RegisterOverallTime(appResourceType, a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterOverallTime(a.Kind(), a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, time.Since(reconcileStartTime)) }() @@ -138,7 +136,7 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.appMetrics.ReconcileTimeMetrics.RegisterFetchTime(appResourceType, a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterFetchTime(a.Kind(), a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, a.app.Status.Fetch.UpdatedAt.Sub(a.app.Status.Fetch.StartedAt.Time)) err := a.updateStatus("marking fetch completed") @@ -162,7 +160,7 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.appMetrics.ReconcileTimeMetrics.RegisterTemplateTime(appResourceType, a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterTemplateTime(a.Kind(), a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, a.app.Status.Template.UpdatedAt.Sub(templateStartTime)) err = a.updateStatus("marking template completed") @@ -213,7 +211,7 @@ func (a *App) updateLastDeploy(result exec.CmdRunResult) exec.CmdRunResult { }, } - a.appMetrics.ReconcileTimeMetrics.RegisterDeployTime(appResourceType, a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterDeployTime(a.Kind(), a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, a.Status().Deploy.UpdatedAt.Sub(a.Status().Deploy.StartedAt.Time)) return result @@ -267,7 +265,7 @@ func (a *App) setReconciling() { Status: corev1.ConditionTrue, }) - a.appMetrics.ReconcileCountMetrics.RegisterReconcileAttempt(appResourceType, a.Name(), a.Namespace()) + a.appMetrics.ReconcileCountMetrics.RegisterReconcileAttempt(a.Kind(), a.Name(), a.Namespace()) a.app.Status.FriendlyDescription = "Reconciling" } @@ -283,7 +281,7 @@ func (a *App) setReconcileCompleted(result exec.CmdRunResult) { a.app.Status.ConsecutiveReconcileFailures++ a.app.Status.ConsecutiveReconcileSuccesses = 0 a.app.Status.FriendlyDescription = fmt.Sprintf("Reconcile failed: %s", result.ErrorStr()) - a.appMetrics.ReconcileCountMetrics.RegisterReconcileFailure(appResourceType, a.Name(), a.Namespace()) + a.appMetrics.ReconcileCountMetrics.RegisterReconcileFailure(a.Kind(), a.Name(), a.Namespace()) a.setUsefulErrorMessage(result) } else { a.app.Status.Conditions = append(a.app.Status.Conditions, v1alpha1.Condition{ @@ -294,7 +292,7 @@ func (a *App) setReconcileCompleted(result exec.CmdRunResult) { a.app.Status.ConsecutiveReconcileSuccesses++ a.app.Status.ConsecutiveReconcileFailures = 0 a.app.Status.FriendlyDescription = "Reconcile succeeded" - a.appMetrics.ReconcileCountMetrics.RegisterReconcileSuccess(appResourceType, a.Name(), a.Namespace()) + a.appMetrics.ReconcileCountMetrics.RegisterReconcileSuccess(a.Kind(), a.Name(), a.Namespace()) a.app.Status.UsefulErrorMessage = "" } } @@ -307,7 +305,7 @@ func (a *App) setDeleting() { Status: corev1.ConditionTrue, }) - a.appMetrics.ReconcileCountMetrics.RegisterReconcileDeleteAttempt(appResourceType, a.Name(), a.Namespace()) + a.appMetrics.ReconcileCountMetrics.RegisterReconcileDeleteAttempt(a.Kind(), a.Name(), a.Namespace()) a.app.Status.FriendlyDescription = "Deleting" } @@ -323,10 +321,10 @@ func (a *App) setDeleteCompleted(result exec.CmdRunResult) { a.app.Status.ConsecutiveReconcileFailures++ a.app.Status.ConsecutiveReconcileSuccesses = 0 a.app.Status.FriendlyDescription = fmt.Sprintf("Delete failed: %s", result.ErrorStr()) - a.appMetrics.ReconcileCountMetrics.RegisterReconcileDeleteFailed(appResourceType, a.Name(), a.Namespace()) + a.appMetrics.ReconcileCountMetrics.RegisterReconcileDeleteFailed(a.Kind(), a.Name(), a.Namespace()) a.setUsefulErrorMessage(result) } else { - a.appMetrics.ReconcileCountMetrics.DeleteMetrics(appResourceType, a.Name(), a.Namespace()) + a.appMetrics.ReconcileCountMetrics.DeleteMetrics(a.Kind(), a.Name(), a.Namespace()) } } diff --git a/pkg/metrics/reconcile_count_metrics.go b/pkg/metrics/reconcile_count_metrics.go index c0e6c4635..27b14fc70 100644 --- a/pkg/metrics/reconcile_count_metrics.go +++ b/pkg/metrics/reconcile_count_metrics.go @@ -61,7 +61,7 @@ func NewCountMetrics() *ReconcileCountMetrics { prometheus.CounterOpts{ Namespace: metricNamespace, Name: "app_reconcile_delete_attempt_total", - Help: "Total number of attempted reconcile deletion", + Help: "Total number of attempted reconcile deletions", }, []string{resourceTypeLabel, kappNameLabel, kappNamespaceLabel}, ), @@ -69,7 +69,7 @@ func NewCountMetrics() *ReconcileCountMetrics { prometheus.CounterOpts{ Namespace: metricNamespace, Name: "app_reconcile_delete_failed_total", - Help: "Total number of failed reconcile deletion", + Help: "Total number of failed reconcile deletions", }, []string{resourceTypeLabel, kappNameLabel, kappNamespaceLabel}, ), diff --git a/pkg/packageinstall/packageinstall.go b/pkg/packageinstall/packageinstall.go index 2ae9614fb..6b7fddba5 100644 --- a/pkg/packageinstall/packageinstall.go +++ b/pkg/packageinstall/packageinstall.go @@ -42,7 +42,6 @@ const ( // PackageInstall to indicate that lower version of the package // can be selected vs whats currently installed. DowngradableAnnKey = "packaging.carvel.dev/downgradable" - packageInstallType = "pkgi" ) // nolint: revive @@ -60,6 +59,11 @@ type PackageInstallCR struct { pkgMetrics *metrics.Metrics } +// Kind return kind of pkg install +func (pi *PackageInstallCR) Kind() string { + return "pkgi" +} + // nolint: revive type Opts struct { DefaultSyncPeriod time.Duration @@ -79,7 +83,7 @@ func (pi *PackageInstallCR) Reconcile() (reconcile.Result, error) { func(st kcv1alpha1.GenericStatus) { pi.model.Status.GenericStatus = st }, } - pi.pkgMetrics.ReconcileCountMetrics.InitMetrics(packageInstallType, pi.model.Name, pi.model.Namespace) + pi.pkgMetrics.ReconcileCountMetrics.InitMetrics(pi.Kind(), pi.model.Name, pi.model.Namespace) var result reconcile.Result var err error @@ -107,12 +111,12 @@ func (pi *PackageInstallCR) Reconcile() (reconcile.Result, error) { func (pi *PackageInstallCR) reconcile(modelStatus *reconciler.Status) (reconcile.Result, error) { pi.log.Info("Reconciling") - pi.pkgMetrics.ReconcileCountMetrics.RegisterReconcileAttempt(packageInstallType, pi.model.Name, pi.model.Namespace) + pi.pkgMetrics.ReconcileCountMetrics.RegisterReconcileAttempt(pi.Kind(), pi.model.Name, pi.model.Namespace) reconcileStartTime := time.Now() - pi.pkgMetrics.IsFirstReconcile = pi.pkgMetrics.ReconcileCountMetrics.GetReconcileAttemptCounterValue(packageInstallType, pi.model.Name, pi.model.Namespace) == 1 + pi.pkgMetrics.IsFirstReconcile = pi.pkgMetrics.ReconcileCountMetrics.GetReconcileAttemptCounterValue(pi.Kind(), pi.model.Name, pi.model.Namespace) == 1 defer func() { - pi.pkgMetrics.ReconcileTimeMetrics.RegisterOverallTime(packageInstallType, pi.model.Name, pi.model.Namespace, + pi.pkgMetrics.ReconcileTimeMetrics.RegisterOverallTime(pi.Kind(), pi.model.Name, pi.model.Namespace, pi.pkgMetrics.IsFirstReconcile, time.Since(reconcileStartTime)) }() diff --git a/pkg/pkgrepository/app.go b/pkg/pkgrepository/app.go index e4304821a..32abcdfc5 100644 --- a/pkg/pkgrepository/app.go +++ b/pkg/pkgrepository/app.go @@ -49,6 +49,9 @@ func NewApp(app v1alpha1.App, hooks Hooks, fetchFactory fetch.Factory, templateF func (a *App) Name() string { return a.app.Name } func (a *App) Namespace() string { return a.app.Namespace } +// Kind return kind of pkg repo +func (a *App) Kind() string { return "pkgr" } + func (a *App) Status() v1alpha1.AppStatus { return a.app.Status } func (a *App) blockDeletion() error { return a.hooks.BlockDeletion() } diff --git a/pkg/pkgrepository/app_reconcile.go b/pkg/pkgrepository/app_reconcile.go index 3013436f5..f5ce30e2e 100644 --- a/pkg/pkgrepository/app_reconcile.go +++ b/pkg/pkgrepository/app_reconcile.go @@ -15,15 +15,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" ) -const packageRepoResourceType = "pkgr" - // Reconcile is not expected to be called concurrently func (a *App) Reconcile(force bool) (reconcile.Result, error) { defer a.flushUpdateStatus("packagerepository reconciled") var err error - a.appMetrics.ReconcileCountMetrics.InitMetrics(packageRepoResourceType, a.Name(), a.Namespace()) + a.appMetrics.ReconcileCountMetrics.InitMetrics(a.Kind(), a.Name(), a.Namespace()) switch { case a.app.Spec.Paused: @@ -95,9 +93,9 @@ func (a *App) reconcileDeploy() error { func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { reconcileStartTime := time.Now() - a.appMetrics.IsFirstReconcile = a.appMetrics.ReconcileCountMetrics.GetReconcileAttemptCounterValue(packageRepoResourceType, a.Name(), a.Namespace()) == 1 + a.appMetrics.IsFirstReconcile = a.appMetrics.ReconcileCountMetrics.GetReconcileAttemptCounterValue(a.Kind(), a.Name(), a.Namespace()) == 1 defer func() { - a.appMetrics.ReconcileTimeMetrics.RegisterOverallTime(packageRepoResourceType, a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterOverallTime(a.Kind(), a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, time.Since(reconcileStartTime)) }() @@ -127,7 +125,7 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.appMetrics.ReconcileTimeMetrics.RegisterFetchTime(packageRepoResourceType, a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterFetchTime(a.Kind(), a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, a.app.Status.Fetch.UpdatedAt.Sub(a.app.Status.Fetch.StartedAt.Time)) err := a.updateStatus("marking fetch completed") @@ -151,7 +149,7 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.appMetrics.ReconcileTimeMetrics.RegisterTemplateTime(packageRepoResourceType, a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterTemplateTime(a.Kind(), a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, a.app.Status.Template.UpdatedAt.Sub(templateStartTime)) err = a.updateStatus("marking template completed") @@ -181,7 +179,7 @@ func (a *App) updateLastDeploy(result exec.CmdRunResult) exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.appMetrics.ReconcileTimeMetrics.RegisterDeployTime(packageRepoResourceType, a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, + a.appMetrics.ReconcileTimeMetrics.RegisterDeployTime(a.Kind(), a.Name(), a.Namespace(), a.appMetrics.IsFirstReconcile, a.Status().Deploy.UpdatedAt.Sub(a.Status().Deploy.StartedAt.Time)) a.updateStatus("marking last deploy") @@ -215,7 +213,7 @@ func (a *App) setReconciling() { Status: corev1.ConditionTrue, }) - a.appMetrics.ReconcileCountMetrics.RegisterReconcileAttempt(packageRepoResourceType, a.Name(), a.Namespace()) + a.appMetrics.ReconcileCountMetrics.RegisterReconcileAttempt(a.Kind(), a.Name(), a.Namespace()) a.app.Status.FriendlyDescription = "Reconciling" } diff --git a/test/e2e/kappcontroller/metrics_test.go b/test/e2e/kappcontroller/metrics_test.go new file mode 100644 index 000000000..09c7f7694 --- /dev/null +++ b/test/e2e/kappcontroller/metrics_test.go @@ -0,0 +1,205 @@ +// Copyright 2021 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package kappcontroller + +import ( + "context" + "fmt" + "io" + "net/http" + "os" + "os/exec" + "strings" + "sync" + //"fmt" + //"os/exec" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/vmware-tanzu/carvel-kapp-controller/test/e2e" +) + + +func TestPrometheusMetrics(t *testing.T) { + env := e2e.BuildEnv(t) + logger := e2e.Logger{} + kapp := e2e.Kapp{t, env.Namespace, logger} + sas := e2e.ServiceAccounts{env.Namespace} + + pkgRepoYAML := fmt.Sprintf(` +apiVersion: packaging.carvel.dev/v1alpha1 +kind: PackageRepository +metadata: + name: minimal-repo.tanzu.carvel.dev + namespace: %s + annotations: + kapp.k14s.io/disable-original: "" +spec: + fetch: + inline: + paths: + + packages/pkg.test.carvel.dev/pkg.test.carvel.dev.0.0.0.yml: | + --- + apiVersion: data.packaging.carvel.dev/v1alpha1 + kind: Package + metadata: + name: pkg.test.carvel.dev.0.0.0 + spec: + refName: pkg.test.carvel.dev + version: 0.0.0 + template: + spec: {} +`, env.Namespace) + + installPkgYAML := fmt.Sprintf(`--- +apiVersion: data.packaging.carvel.dev/v1alpha1 +kind: PackageMetadata +metadata: + name: pkg.test.carvel.dev + namespace: %[1]s +spec: + # This is the name we want to reference in resources such as PackageInstall. + displayName: "Test PackageMetadata in repo" + shortDescription: "PackageMetadata used for testing" + longDescription: "A longer, more detailed description of what the package contains and what it is for" + providerName: Carvel + maintainers: + - name: carvel + categories: + - testing + supportDescription: "Description of support provided for the package" +--- +apiVersion: data.packaging.carvel.dev/v1alpha1 +kind: Package +metadata: + name: pkg.test.carvel.dev.1.0.0 + namespace: %[1]s +spec: + refName: pkg.test.carvel.dev + version: 1.0.0 + licenses: + - Apache 2.0 + capactiyRequirementsDescription: "cpu: 1,RAM: 2, Disk: 3" + releaseNotes: | + - Introduce simple-app package + releasedAt: 2021-05-05T18:57:06Z + template: + spec: + fetch: + - imgpkgBundle: + image: k8slt/kctrl-example-pkg:v1.0.0 + template: + - ytt: {} + - kbld: + paths: + - "-" + - ".imgpkg/images.yml" + deploy: + - kapp: + inspect: {} +--- +apiVersion: packaging.carvel.dev/v1alpha1 +kind: PackageInstall +metadata: + name: instl-pkg-test + namespace: %[1]s + annotations: + kapp.k14s.io/change-group: kappctrl-e2e.k14s.io/packageinstalls +spec: + serviceAccountName: kappctrl-e2e-ns-sa + packageRef: + refName: pkg.test.carvel.dev + versionSelection: + constraints: 1.0.0 + values: + - secretRef: + name: pkg-demo-values +--- +apiVersion: v1 +kind: Secret +metadata: + name: pkg-demo-values +stringData: + values.yml: | + hello_msg: "hi" +`, env.Namespace) + sas.ForNamespaceYAML() + + cleanUp := func() { + kapp.Run([]string{"delete", "-a", "simple-app.app"}) + kapp.Run([]string{"delete", "-a", "simple-app"}) + kapp.Run([]string{"delete", "-a", "default-ns-rbac"}) + kapp.Run([]string{"delete", "-a", "instl-pkg-test"}) + kapp.Run([]string{"delete", "-a", "minimal-repo.tanzu.carvel.dev"}) + } + cleanUp() + defer cleanUp() + + ctx, cancel := context.WithCancel(context.Background()) + + var wg sync.WaitGroup + wg.Add(1) + + // port-forwarding goroutine + go func() { + defer wg.Done() + portForward(ctx) + }() + + // Allow some time for port-forwarding + time.Sleep(2 * time.Second) + + kapp.Run([]string{"deploy", "-a", "default-ns-rbac", "-f", + "https://raw.githubusercontent.com/carvel-dev/kapp-controller/develop/examples/rbac/default-ns.yml"}) + + kapp.Run([]string{"deploy", "-a", "simple-app", "-f", + "https://raw.githubusercontent.com/k14s/kapp-controller/develop/examples/simple-app-git/1.yml"}) + + kapp.RunWithOpts([]string{"deploy", "-a", "minimal-repo.tanzu.carvel.dev", "-f", "-"}, e2e.RunOpts{StdinReader: strings.NewReader(pkgRepoYAML)}) + + kapp.RunWithOpts([]string{"deploy", "-a", "instl-pkg-test", "-f", "-"}, e2e.RunOpts{StdinReader: strings.NewReader(installPkgYAML)}) + + t.Logf("Hitting URL") + + resp, err := http.Get("http://localhost:8080/metrics") + assert.NoError(t, err) + defer resp.Body.Close() + + // Check if the response is successful + assert.Equal(t, http.StatusOK, resp.StatusCode) + + bodyBytes, err := io.ReadAll(resp.Body) + + response := string(bodyBytes) + + bodyContains := assert.Contains(t, response, "kappctrl_reconcile_deploy_time_seconds") && + assert.Contains(t, response, "kappctrl_reconcile_fetch_time_seconds") && + assert.Contains(t, response, "kappctrl_reconcile_template_time_seconds") && + assert.Contains(t, response, "kappctrl_reconcile_time_seconds") + + assert.True(t, bodyContains) + + // Stop port-forwarding by canceling the context + cancel() + + // Wait for the port-forwarding goroutine to complete + wg.Wait() +} + +func portForward(ctx context.Context) { + cmd := exec.CommandContext(ctx, "kubectl", "port-forward", "svc/packaging-api", "8080", "-n", "kapp-controller") + + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + err := cmd.Run() + if err != nil { + if ctx.Err() == context.Canceled { + fmt.Println("Port-forwarding stopped by context cancellation.") + } else { + fmt.Printf("Error running kubectl port-forward: %v\n", err) + } + } +} \ No newline at end of file From b292117cebee39ba6c59fe7dc152b213c2baa920 Mon Sep 17 00:00:00 2001 From: Yash Sethiya Date: Wed, 24 Jan 2024 13:35:02 +0530 Subject: [PATCH 9/9] Updated CR names in kind method Signed-off-by: Yash Sethiya --- pkg/app/app.go | 2 +- pkg/packageinstall/packageinstall.go | 2 +- pkg/pkgrepository/app.go | 2 +- test/e2e/kappcontroller/metrics_test.go | 2 -- 4 files changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/app/app.go b/pkg/app/app.go index eeed9d676..e1e937074 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -77,7 +77,7 @@ func (a *App) Name() string { return a.app.Name } func (a *App) Namespace() string { return a.app.Namespace } // Kind return kind of App -func (a *App) Kind() string { return "app" } +func (a *App) Kind() string { return "App" } func (a *App) Status() v1alpha1.AppStatus { return a.app.Status } diff --git a/pkg/packageinstall/packageinstall.go b/pkg/packageinstall/packageinstall.go index 6b7fddba5..b5aba623d 100644 --- a/pkg/packageinstall/packageinstall.go +++ b/pkg/packageinstall/packageinstall.go @@ -61,7 +61,7 @@ type PackageInstallCR struct { // Kind return kind of pkg install func (pi *PackageInstallCR) Kind() string { - return "pkgi" + return "PackageInstall" } // nolint: revive diff --git a/pkg/pkgrepository/app.go b/pkg/pkgrepository/app.go index 32abcdfc5..fd94fd3c3 100644 --- a/pkg/pkgrepository/app.go +++ b/pkg/pkgrepository/app.go @@ -50,7 +50,7 @@ func (a *App) Name() string { return a.app.Name } func (a *App) Namespace() string { return a.app.Namespace } // Kind return kind of pkg repo -func (a *App) Kind() string { return "pkgr" } +func (a *App) Kind() string { return "PackageRepository" } func (a *App) Status() v1alpha1.AppStatus { return a.app.Status } diff --git a/test/e2e/kappcontroller/metrics_test.go b/test/e2e/kappcontroller/metrics_test.go index 09c7f7694..ab6526a38 100644 --- a/test/e2e/kappcontroller/metrics_test.go +++ b/test/e2e/kappcontroller/metrics_test.go @@ -12,8 +12,6 @@ import ( "os/exec" "strings" "sync" - //"fmt" - //"os/exec" "testing" "time"