Skip to content

Commit

Permalink
Add metrics to monitor reconcile status (#346)
Browse files Browse the repository at this point in the history
  • Loading branch information
mhoshi-vm authored Sep 9, 2021
1 parent c35c85f commit 22299af
Show file tree
Hide file tree
Showing 11 changed files with 182 additions and 23 deletions.
7 changes: 6 additions & 1 deletion cmd/controller/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/app"
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"
pkginstall "github.com/vmware-tanzu/carvel-kapp-controller/pkg/packageinstall"
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/pkgrepository"
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/reftracker"
Expand Down Expand Up @@ -81,6 +82,10 @@ func Run(opts Options, runLog logr.Logger) error {
return fmt.Errorf("Building packaging client: %s", err)
}

runLog.Info("setting up metrics")
appMetrics := metrics.NewAppMetrics()
appMetrics.RegisterAllMetrics()

// assign bindPort to env var KAPPCTRL_API_PORT if available
var bindPort int
if apiPort, ok := os.LookupEnv(kappctrlAPIPORTEnvKey); ok {
Expand All @@ -106,7 +111,7 @@ func Run(opts Options, runLog logr.Logger) error {
updateStatusTracker := reftracker.NewAppUpdateStatus()

{ // add controller for apps
appFactory := app.CRDAppFactory{coreClient, kcClient, kcConfig}
appFactory := app.CRDAppFactory{coreClient, kcClient, kcConfig, appMetrics}
reconciler := app.NewReconciler(kcClient, runLog.WithName("app"),
appFactory, refTracker, updateStatusTracker)

Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ require (
github.com/jonboulle/clockwork v0.2.0 // indirect
github.com/onsi/ginkgo v1.16.1 // indirect
github.com/onsi/gomega v1.11.0 // indirect
github.com/prometheus/client_golang v1.7.1
github.com/sirupsen/logrus v1.8.1 // indirect
github.com/stretchr/testify v1.7.0
github.com/tmc/grpc-websocket-proxy v0.0.0-20200427203606-3cfed13b9966 // indirect
Expand Down
8 changes: 5 additions & 3 deletions pkg/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,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"
)
Expand All @@ -31,19 +32,20 @@ type App struct {
templateFactory template.Factory
deployFactory deploy.Factory

log logr.Logger
log logr.Logger
appMetrics *metrics.AppMetrics

pendingStatusUpdate bool
flushAllStatusUpdates bool
}

func NewApp(app v1alpha1.App, hooks Hooks,
fetchFactory fetch.Factory, templateFactory template.Factory,
deployFactory deploy.Factory, log logr.Logger) *App {
deployFactory deploy.Factory, log logr.Logger, appMetrics *metrics.AppMetrics) *App {

return &App{app: app, appPrev: *(app.DeepCopy()), hooks: hooks,
fetchFactory: fetchFactory, templateFactory: templateFactory,
deployFactory: deployFactory, log: log}
deployFactory: deployFactory, log: log, appMetrics: appMetrics}
}

func (a *App) Name() string { return a.app.Name }
Expand Down
4 changes: 3 additions & 1 deletion pkg/app/app_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/config"
"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/template"
"k8s.io/client-go/kubernetes"
)
Expand All @@ -19,12 +20,13 @@ type CRDAppFactory struct {
CoreClient kubernetes.Interface
AppClient kcclient.Interface
KcConfig *config.Config
AppMetrics *metrics.AppMetrics
}

// NewCRDApp creates a CRDApp injecting necessary dependencies.
func (f *CRDAppFactory) NewCRDApp(app *kcv1alpha1.App, log logr.Logger) *CRDApp {
fetchFactory := fetch.NewFactory(f.CoreClient, f.KcConfig)
templateFactory := template.NewFactory(f.CoreClient, fetchFactory)
deployFactory := deploy.NewFactory(f.CoreClient)
return NewCRDApp(app, log, f.AppClient, fetchFactory, templateFactory, deployFactory)
return NewCRDApp(app, log, f.AppMetrics, f.AppClient, fetchFactory, templateFactory, deployFactory)
}
9 changes: 8 additions & 1 deletion pkg/app/app_reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.Canceled || a.app.Spec.Paused:
a.log.Info("App is canceled or paused, not reconciling")
Expand Down Expand Up @@ -211,6 +213,7 @@ func (a *App) setReconciling() {
Status: corev1.ConditionTrue,
})

a.appMetrics.RegisterReconcileAttempt(a.app.Name, a.app.Namespace)
a.app.Status.FriendlyDescription = "Reconciling"
}

Expand All @@ -226,6 +229,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.setUsefulErrorMessage(result)
} else {
a.app.Status.Conditions = append(a.app.Status.Conditions, v1alpha1.AppCondition{
Expand All @@ -236,6 +240,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.app.Status.UsefulErrorMessage = ""
}
}
Expand All @@ -248,6 +253,7 @@ func (a *App) setDeleting() {
Status: corev1.ConditionTrue,
})

a.appMetrics.RegisterReconcileDeleteAttempt(a.app.Name, a.app.Namespace)
a.app.Status.FriendlyDescription = "Deleting"
}

Expand All @@ -261,9 +267,10 @@ func (a *App) setDeleteCompleted(result exec.CmdRunResult) {
Message: result.ErrorStr(),
})
a.app.Status.FriendlyDescription = fmt.Sprintf("Delete failed: %s", result.ErrorStr())
a.appMetrics.RegisterReconcileDeleteFailed(a.app.Name, a.app.Namespace)
a.setUsefulErrorMessage(result)
} else {
// assume resource will be deleted, hence nothing to update
a.appMetrics.DeleteMetrics(a.app.Name, a.app.Namespace)
}
}

Expand Down
13 changes: 9 additions & 4 deletions pkg/app/app_reconcile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/client/clientset/versioned/fake"
"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/template"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -22,12 +23,14 @@ import (

func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) {
log := logf.Log.WithName("kc")
var appMetrics = metrics.NewAppMetrics()

// The url under fetch is invalid, which will cause this
// app to fail before deploy.
app := v1alpha1.App{
ObjectMeta: metav1.ObjectMeta{
Name: "simple-app",
Name: "simple-app",
Namespace: "pkg-standalone",
},
Spec: v1alpha1.AppSpec{
Fetch: []v1alpha1.AppFetch{
Expand All @@ -42,7 +45,7 @@ func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) {
tmpFac := template.NewFactory(k8scs, fetchFac)
deployFac := deploy.NewFactory(k8scs)

crdApp := NewCRDApp(&app, log, kappcs, fetchFac, tmpFac, deployFac)
crdApp := NewCRDApp(&app, log, appMetrics, kappcs, fetchFac, tmpFac, deployFac)
_, err := crdApp.Reconcile(false)
assert.Nil(t, err, "unexpected error with reconciling", err)

Expand Down Expand Up @@ -78,13 +81,15 @@ func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) {

func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing.T) {
log := logf.Log.WithName("kc")
var appMetrics = metrics.NewAppMetrics()

fetchInline := map[string]string{
"file.yml": `foo: #@ data.values.nothere`,
}
app := v1alpha1.App{
ObjectMeta: metav1.ObjectMeta{
Name: "simple-app",
Name: "simple-app",
Namespace: "pkg-standalone",
},
Spec: v1alpha1.AppSpec{
Fetch: []v1alpha1.AppFetch{
Expand All @@ -102,7 +107,7 @@ func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing.
tmpFac := template.NewFactory(k8scs, fetchFac)
deployFac := deploy.NewFactory(k8scs)

crdApp := NewCRDApp(&app, log, kappcs, fetchFac, tmpFac, deployFac)
crdApp := NewCRDApp(&app, log, appMetrics, kappcs, fetchFac, tmpFac, deployFac)
_, err := crdApp.Reconcile(false)
assert.Nil(t, err, "Unexpected error with reconciling", err)

Expand Down
8 changes: 4 additions & 4 deletions pkg/app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func Test_SecretRefs_RetrievesAllSecretRefs(t *testing.T) {
tmpFac := template.NewFactory(k8scs, fetchFac)
deployFac := deploy.NewFactory(k8scs)

app := apppkg.NewApp(appWithRefs, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log)
app := apppkg.NewApp(appWithRefs, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, nil)

out := app.SecretRefs()
if !reflect.DeepEqual(out, expected) {
Expand All @@ -85,7 +85,7 @@ func Test_SecretRefs_RetrievesNoSecretRefs_WhenNonePresent(t *testing.T) {
tmpFac := template.NewFactory(k8scs, fetchFac)
deployFac := deploy.NewFactory(k8scs)

app := apppkg.NewApp(appEmpty, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log)
app := apppkg.NewApp(appEmpty, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, nil)

out := app.SecretRefs()
if len(out) != 0 {
Expand Down Expand Up @@ -125,7 +125,7 @@ func Test_ConfigMapRefs_RetrievesAllConfigMapRefs(t *testing.T) {
tmpFac := template.NewFactory(k8scs, fetchFac)
deployFac := deploy.NewFactory(k8scs)

app := apppkg.NewApp(appWithRefs, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log)
app := apppkg.NewApp(appWithRefs, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, nil)

out := app.ConfigMapRefs()
if !reflect.DeepEqual(out, expected) {
Expand All @@ -151,7 +151,7 @@ func Test_ConfigMapRefs_RetrievesNoConfigMapRefs_WhenNonePresent(t *testing.T) {
tmpFac := template.NewFactory(k8scs, fetchFac)
deployFac := deploy.NewFactory(k8scs)

app := apppkg.NewApp(appEmpty, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log)
app := apppkg.NewApp(appEmpty, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, nil)

out := app.ConfigMapRefs()
if len(out) != 0 {
Expand Down
17 changes: 10 additions & 7 deletions pkg/app/crd_app.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,31 +13,34 @@ 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"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

type CRDApp struct {
app *App
appModel *kcv1alpha1.App
log logr.Logger
appClient kcclient.Interface
app *App
appModel *kcv1alpha1.App
log logr.Logger
appMetrics *metrics.AppMetrics
appClient kcclient.Interface
}

func NewCRDApp(appModel *kcv1alpha1.App, log logr.Logger,
// NewCRDApp creates new CRD app
func NewCRDApp(appModel *kcv1alpha1.App, log logr.Logger, appMetrics *metrics.AppMetrics,
appClient kcclient.Interface, fetchFactory fetch.Factory,
templateFactory template.Factory, deployFactory deploy.Factory) *CRDApp {

crdApp := &CRDApp{appModel: appModel, log: log, appClient: appClient}
crdApp := &CRDApp{appModel: appModel, log: log, appMetrics: appMetrics, appClient: appClient}

crdApp.app = NewApp(*appModel, Hooks{
BlockDeletion: crdApp.blockDeletion,
UnblockDeletion: crdApp.unblockDeletion,
UpdateStatus: crdApp.updateStatus,
WatchChanges: crdApp.watchChanges,
}, fetchFactory, templateFactory, deployFactory, log)
}, fetchFactory, templateFactory, deployFactory, log, appMetrics)

return crdApp
}
Expand Down
Loading

0 comments on commit 22299af

Please sign in to comment.