Skip to content

Commit

Permalink
final refactoring
Browse files Browse the repository at this point in the history
Signed-off-by: sethiyash <[email protected]>
  • Loading branch information
sethiyash committed Dec 7, 2023
1 parent 09d6cd0 commit 196bc45
Show file tree
Hide file tree
Showing 22 changed files with 287 additions and 270 deletions.
40 changes: 20 additions & 20 deletions cmd/controller/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand All @@ -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,
Expand All @@ -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,
Expand Down
14 changes: 7 additions & 7 deletions pkg/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,24 +55,24 @@ 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
}

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 }
Expand Down
4 changes: 2 additions & 2 deletions pkg/app/app_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(),
})
Expand Down
6 changes: 0 additions & 6 deletions pkg/app/app_fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
30 changes: 14 additions & 16 deletions pkg/app/app_reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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))
}()

Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"
}

Expand All @@ -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{
Expand All @@ -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 = ""
}
}
Expand All @@ -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"
}

Expand All @@ -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)
}
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/app/app_reconcile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
)

Expand Down Expand Up @@ -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()
)

Expand Down Expand Up @@ -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()
)

Expand Down
2 changes: 1 addition & 1 deletion pkg/app/app_template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 7 additions & 7 deletions pkg/app/crd_app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 196bc45

Please sign in to comment.