From a21d8af2958374bd7201afab90fe742b9aeb51b2 Mon Sep 17 00:00:00 2001 From: Daniel Helfand Date: Fri, 4 Jun 2021 11:22:55 -0500 Subject: [PATCH] use packagerepo status instead of app status --- config/crds.yml | 16 ---- .../packaging/v1alpha1/package_repository.go | 17 +++- .../v1alpha1/zz_generated.deepcopy.go | 32 +++++++ pkg/pkgrepository/app_deploy.go | 33 +------ pkg/pkgrepository/app_reconcile.go | 26 ++---- pkg/pkgrepository/crd_app.go | 28 +++--- pkg/pkgrepository/crd_app_watcher.go | 87 ------------------- pkg/pkgrepository/package_repo_app.go | 19 +++- 8 files changed, 83 insertions(+), 175 deletions(-) delete mode 100644 pkg/pkgrepository/crd_app_watcher.go diff --git a/config/crds.yml b/config/crds.yml index b9110e06c..623b9dca2 100644 --- a/config/crds.yml +++ b/config/crds.yml @@ -1178,22 +1178,6 @@ spec: type: object friendlyDescription: type: string - inspect: - properties: - error: - type: string - exitCode: - type: integer - stderr: - type: string - stdout: - type: string - updatedAt: - format: date-time - type: string - type: object - managedAppName: - type: string observedGeneration: format: int64 type: integer diff --git a/pkg/apis/packaging/v1alpha1/package_repository.go b/pkg/apis/packaging/v1alpha1/package_repository.go index 0b3200a8f..c8ace86b7 100644 --- a/pkg/apis/packaging/v1alpha1/package_repository.go +++ b/pkg/apis/packaging/v1alpha1/package_repository.go @@ -22,7 +22,7 @@ type PackageRepository struct { Spec PackageRepositorySpec `json:"spec"` // +optional - Status v1alpha1.AppStatus `json:"status,omitempty"` + Status PackageRepositoryStatus `json:"status,omitempty"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -57,3 +57,18 @@ type PackageRepositoryFetch struct { // +optional ImgpkgBundle *v1alpha1.AppFetchImgpkgBundle `json:"imgpkgBundle,omitempty"` } + +type PackageRepositoryStatus struct { + // +optional + Fetch *v1alpha1.AppStatusFetch `json:"fetch,omitempty"` + // +optional + Template *v1alpha1.AppStatusTemplate `json:"template,omitempty"` + // +optional + Deploy *v1alpha1.AppStatusDeploy `json:"deploy,omitempty"` + // +optional + ConsecutiveReconcileSuccesses int `json:"consecutiveReconcileSuccesses,omitempty"` + // +optional + ConsecutiveReconcileFailures int `json:"consecutiveReconcileFailures,omitempty"` + // +optional + v1alpha1.GenericStatus `json:",inline"` +} diff --git a/pkg/apis/packaging/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/packaging/v1alpha1/zz_generated.deepcopy.go index 720c47d90..2201e88d1 100644 --- a/pkg/apis/packaging/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/packaging/v1alpha1/zz_generated.deepcopy.go @@ -287,6 +287,38 @@ func (in *PackageRepositorySpec) DeepCopy() *PackageRepositorySpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PackageRepositoryStatus) DeepCopyInto(out *PackageRepositoryStatus) { + *out = *in + if in.Fetch != nil { + in, out := &in.Fetch, &out.Fetch + *out = new(kappctrlv1alpha1.AppStatusFetch) + (*in).DeepCopyInto(*out) + } + if in.Template != nil { + in, out := &in.Template, &out.Template + *out = new(kappctrlv1alpha1.AppStatusTemplate) + (*in).DeepCopyInto(*out) + } + if in.Deploy != nil { + in, out := &in.Deploy, &out.Deploy + *out = new(kappctrlv1alpha1.AppStatusDeploy) + (*in).DeepCopyInto(*out) + } + in.GenericStatus.DeepCopyInto(&out.GenericStatus) + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PackageRepositoryStatus. +func (in *PackageRepositoryStatus) DeepCopy() *PackageRepositoryStatus { + if in == nil { + return nil + } + out := new(PackageRepositoryStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PackageVersionRef) DeepCopyInto(out *PackageVersionRef) { *out = *in diff --git a/pkg/pkgrepository/app_deploy.go b/pkg/pkgrepository/app_deploy.go index 09794da0a..d2f78e501 100644 --- a/pkg/pkgrepository/app_deploy.go +++ b/pkg/pkgrepository/app_deploy.go @@ -11,6 +11,7 @@ import ( "github.com/vmware-tanzu/carvel-kapp-controller/pkg/exec" ) +// TODO: Remove changedFunc logic func (a *App) deploy(tplOutput string, changedFunc func(exec.CmdRunResult)) exec.CmdRunResult { err := a.blockDeletion() if err != nil { @@ -88,38 +89,6 @@ func (a *App) delete(changedFunc func(exec.CmdRunResult)) exec.CmdRunResult { return result } -func (a *App) inspect() exec.CmdRunResult { - if len(a.app.Spec.Deploy) != 1 { - return exec.NewCmdRunResultWithErr(fmt.Errorf("Expected exactly one deploy option")) - } - - var result exec.CmdRunResult - - for _, dep := range a.app.Spec.Deploy { - switch { - case dep.Kapp != nil: - cancelCh, closeCancelCh := a.newCancelCh() - defer closeCancelCh() - - kapp, err := a.newKapp(*dep.Kapp, cancelCh) - if err != nil { - return exec.NewCmdRunResultWithErr(fmt.Errorf("Preparing kapp: %s", err)) - } - - result = kapp.Inspect() - - default: - result.AttachErrorf("%s", fmt.Errorf("Unsupported way to inspect")) - } - - if result.Error != nil { - break - } - } - - return result -} - func (a *App) newKapp(kapp v1alpha1.AppDeployKapp, cancelCh chan struct{}) (*ctldep.Kapp, error) { genericOpts := ctldep.GenericOpts{Name: a.app.Name, Namespace: a.app.Namespace} return a.deployFactory.NewKappPrivileged(kapp, genericOpts, cancelCh) diff --git a/pkg/pkgrepository/app_reconcile.go b/pkg/pkgrepository/app_reconcile.go index c215380d1..f1c3e113d 100644 --- a/pkg/pkgrepository/app_reconcile.go +++ b/pkg/pkgrepository/app_reconcile.go @@ -81,12 +81,6 @@ func (a *App) reconcileDeploy() error { result := a.reconcileFetchTemplateDeploy() a.setReconcileCompleted(result) - // Reconcile inspect regardless of deploy success - // but don't inspect if deploy never attempted - if a.app.Status.Deploy != nil { - _ = a.reconcileInspect() - } - return a.updateStatus("marking reconcile completed") } @@ -147,7 +141,11 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { a.resetLastDeployStartedAt() - return a.updateLastDeploy(a.deploy(tplResult.Stdout, a.updateLastDeployNoReturn)) + // Deployment should not take long time, so no + // need to regularly update status. Passing in + // empty func for a.deploy so only updateLastDeploy + // sets status for Deploy portion of status. + return a.updateLastDeploy(a.deploy(tplResult.Stdout, func(result exec.CmdRunResult) {})) } func (a *App) updateLastDeploy(result exec.CmdRunResult) exec.CmdRunResult { @@ -186,20 +184,6 @@ func (a *App) resetLastDeployStartedAt() { a.app.Status.Deploy.StartedAt = metav1.NewTime(time.Now().UTC()) } -func (a *App) reconcileInspect() error { - inspectResult := a.inspect().WithFriendlyYAMLStrings() - - a.app.Status.Inspect = &v1alpha1.AppStatusInspect{ - Stdout: inspectResult.Stdout, - Stderr: inspectResult.Stderr, - ExitCode: inspectResult.ExitCode, - Error: inspectResult.ErrorStr(), - UpdatedAt: metav1.NewTime(time.Now().UTC()), - } - - return a.updateStatus("marking inspect completed") -} - func (a *App) markObservedLatest() { a.app.Status.ObservedGeneration = a.app.Generation } diff --git a/pkg/pkgrepository/crd_app.go b/pkg/pkgrepository/crd_app.go index 1c4dcb485..4a9f893e0 100644 --- a/pkg/pkgrepository/crd_app.go +++ b/pkg/pkgrepository/crd_app.go @@ -6,11 +6,9 @@ package pkgrepository import ( "context" "fmt" - pkgingv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/packaging/v1alpha1" - "reflect" - "github.com/go-logr/logr" kcv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" + pkgingv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/packaging/v1alpha1" 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" @@ -38,7 +36,6 @@ func NewCRDApp(appModel *kcv1alpha1.App, packageRepo *pkgingv1alpha1.PackageRepo BlockDeletion: crdApp.blockDeletion, UnblockDeletion: crdApp.unblockDeletion, UpdateStatus: crdApp.updateStatus, - WatchChanges: crdApp.watchChanges, }, fetchFactory, templateFactory, deployFactory, log) return crdApp @@ -81,18 +78,23 @@ func (a *CRDApp) updateStatus(desc string) error { } func (a *CRDApp) updateStatusOnce() error { - existingApp, err := a.appClient.PackagingV1alpha1().PackageRepositories(a.pkgrModel.Namespace).Get(context.Background(), a.pkgrModel.Name, metav1.GetOptions{}) + existingRepo, err := a.appClient.PackagingV1alpha1().PackageRepositories(a.pkgrModel.Namespace).Get(context.Background(), a.pkgrModel.Name, metav1.GetOptions{}) if err != nil { return fmt.Errorf("Fetching app: %s", err) } - if !reflect.DeepEqual(existingApp.Status, a.app.Status()) { + existingRepo.Status = pkgingv1alpha1.PackageRepositoryStatus{ + Fetch: a.app.Status().Fetch, + Template: a.app.Status().Template, + Deploy: a.app.Status().Deploy, + GenericStatus: a.app.Status().GenericStatus, + ConsecutiveReconcileSuccesses: a.app.Status().ConsecutiveReconcileSuccesses, + ConsecutiveReconcileFailures: a.app.Status().ConsecutiveReconcileFailures, + } - existingApp.Status = a.app.Status() - _, err = a.appClient.PackagingV1alpha1().PackageRepositories(existingApp.Namespace).UpdateStatus(context.Background(), existingApp, metav1.UpdateOptions{}) - if err != nil { - return err - } + _, err = a.appClient.PackagingV1alpha1().PackageRepositories(existingRepo.Namespace).UpdateStatus(context.Background(), existingRepo, metav1.UpdateOptions{}) + if err != nil { + return err } return nil @@ -120,10 +122,6 @@ func (a *CRDApp) Reconcile(force bool) (reconcile.Result, error) { return a.app.Reconcile(force) } -func (a *CRDApp) watchChanges(callback func(kcv1alpha1.App), cancelCh chan struct{}) error { - return NewCRDAppWatcher(*a.appModel, a.appClient).Watch(callback, cancelCh) -} - func (a *CRDApp) ResourceRefs() map[reftracker.RefKey]struct{} { return a.app.SecretRefs() } diff --git a/pkg/pkgrepository/crd_app_watcher.go b/pkg/pkgrepository/crd_app_watcher.go deleted file mode 100644 index e8d77f9a3..000000000 --- a/pkg/pkgrepository/crd_app_watcher.go +++ /dev/null @@ -1,87 +0,0 @@ -// Copyright 2020 VMware, Inc. -// SPDX-License-Identifier: Apache-2.0 - -package pkgrepository - -import ( - "context" - "fmt" - - "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" - kcclient "github.com/vmware-tanzu/carvel-kapp-controller/pkg/client/clientset/versioned" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/fields" -) - -type CRDAppWatcher struct { - app v1alpha1.App - appClient kcclient.Interface -} - -func NewCRDAppWatcher(app v1alpha1.App, appClient kcclient.Interface) CRDAppWatcher { - return CRDAppWatcher{app, appClient} -} - -func (w CRDAppWatcher) Watch(callback func(v1alpha1.App), cancelCh chan struct{}) error { - // canceled before starting - select { - case <-cancelCh: - return nil - default: - } - - for { - retry, err := w.watch(callback, cancelCh) - if err != nil { - return err - } - if !retry { - return nil - } - } -} - -func (w CRDAppWatcher) watch(callback func(v1alpha1.App), cancelCh chan struct{}) (bool, error) { - listOpts := metav1.ListOptions{ - // Only single resource that has ns+name combination. - // metadata.uid cannot be used as it's not indexed. - FieldSelector: fields.OneTermEqualSelector("metadata.name", string(w.app.Name)).String(), - } - - watcher, err := w.appClient.KappctrlV1alpha1().Apps(w.app.Namespace).Watch(context.Background(), listOpts) - if err != nil { - return false, fmt.Errorf("Creating app watcher: %s", err) - } - - defer func() { - watcher.Stop() - - // Watcher may be trying to send Event before being channel is closed - // https://github.com/kubernetes/apimachinery/blob/d8530e6c952f75365336be8ea29cfd758ce49ee8/pkg/watch/streamwatcher.go#L127 - // (Found this problem by observing open stuck goroutines via pprof) - for range watcher.ResultChan() { - // Drain any pending events, so that channel is - // closed and internal goroutine is discarded - } - }() - - for { - select { - case e, ok := <-watcher.ResultChan(): - if !ok || e.Object == nil { - // Watcher may expire, hence try to retry - return true, nil - } - - app, ok := e.Object.(*v1alpha1.App) - if !ok { - continue - } - - callback(*app) - - case <-cancelCh: - return false, nil - } - } -} diff --git a/pkg/pkgrepository/package_repo_app.go b/pkg/pkgrepository/package_repo_app.go index 2b732a1b1..dc6999639 100644 --- a/pkg/pkgrepository/package_repo_app.go +++ b/pkg/pkgrepository/package_repo_app.go @@ -6,6 +6,8 @@ package pkgrepository import ( kcv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" pkgingv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/packaging/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "time" ) func NewPackageRepoApp(pkgRepository *pkgingv1alpha1.PackageRepository) (*kcv1alpha1.App, error) { @@ -13,7 +15,6 @@ func NewPackageRepoApp(pkgRepository *pkgingv1alpha1.PackageRepository) (*kcv1al desiredApp.Name = pkgRepository.Name desiredApp.Namespace = pkgRepository.Namespace - desiredApp.Status = pkgRepository.Status desiredApp.DeletionTimestamp = pkgRepository.DeletionTimestamp desiredApp.Spec = kcv1alpha1.AppSpec{ @@ -32,8 +33,11 @@ func NewPackageRepoApp(pkgRepository *pkgingv1alpha1.PackageRepository) (*kcv1al Deploy: []kcv1alpha1.AppDeploy{{ Kapp: &kcv1alpha1.AppDeployKapp{}, }}, - SyncPeriod: pkgRepository.Spec.SyncPeriod, - Paused: pkgRepository.Spec.Paused, + Paused: pkgRepository.Spec.Paused, + } + + if pkgRepository.Spec.SyncPeriod == nil { + desiredApp.Spec.SyncPeriod = &metav1.Duration{Duration: time.Minute * 5} } if desiredApp.Spec.Fetch[0].ImgpkgBundle != nil { @@ -41,5 +45,14 @@ func NewPackageRepoApp(pkgRepository *pkgingv1alpha1.PackageRepository) (*kcv1al kcv1alpha1.AppTemplate{Kbld: &kcv1alpha1.AppTemplateKbld{Paths: []string{"-", ".imgpkg/images.yml"}}}) } + desiredApp.Status = kcv1alpha1.AppStatus{ + Fetch: pkgRepository.Status.Fetch, + Template: pkgRepository.Status.Template, + Deploy: pkgRepository.Status.Deploy, + GenericStatus: pkgRepository.Status.GenericStatus, + ConsecutiveReconcileSuccesses: pkgRepository.Status.ConsecutiveReconcileSuccesses, + ConsecutiveReconcileFailures: pkgRepository.Status.ConsecutiveReconcileFailures, + } + return desiredApp, nil }