diff --git a/pkg/cache/memory/cache_test.go b/pkg/cache/memory/cache_test.go index e419075e..f7baf304 100644 --- a/pkg/cache/memory/cache_test.go +++ b/pkg/cache/memory/cache_test.go @@ -113,7 +113,7 @@ func TestPublishedLatest(t *testing.T) { if err := update.UpdateLifecycle(ctx, api.PackageRevisionLifecyclePublished); err != nil { t.Fatalf("UpdateLifecycle failed; %v", err) } - closed, err := update.Close(ctx) + closed, err := update.Close(ctx, "") if err != nil { t.Fatalf("Close failed: %v", err) } @@ -128,6 +128,94 @@ func TestPublishedLatest(t *testing.T) { } } +func TestDeletePublishedMain(t *testing.T) { + ctx := context.Background() + testPath := filepath.Join("../..", "git", "testdata") + cachedRepo := openRepositoryFromArchive(t, ctx, testPath, "nested") + + revisions, err := cachedRepo.ListPackageRevisions(ctx, repository.ListPackageRevisionFilter{ + Package: "catalog/gcp/bucket", + WorkspaceName: "v2", + }) + if err != nil { + t.Fatalf("ListPackageRevisions failed: %v", err) + } + + // Expect a single result + if got, want := len(revisions), 1; got != want { + t.Fatalf("ListPackageRevisions returned %d packages; want %d", got, want) + } + + bucket := revisions[0] + // Expect draft package + if got, want := bucket.Lifecycle(), api.PackageRevisionLifecycleDraft; got != want { + t.Fatalf("Bucket package lifecycle: got %s, want %s", got, want) + } + + update, err := cachedRepo.UpdatePackageRevision(ctx, bucket) + if err != nil { + t.Fatalf("UpdatePackage(%s) failed: %v", bucket.Key(), err) + } + if err := update.UpdateLifecycle(ctx, api.PackageRevisionLifecyclePublished); err != nil { + t.Fatalf("UpdateLifecycle failed; %v", err) + } + closed, err := update.Close(ctx, "") + if err != nil { + t.Fatalf("Close failed: %v", err) + } + _, err = closed.GetPackageRevision(ctx) + if err != nil { + t.Errorf("didn't expect error, but got %v", err) + } + + publishedRevisions, err := cachedRepo.ListPackageRevisions(ctx, repository.ListPackageRevisionFilter{ + Package: "catalog/gcp/bucket", + WorkspaceName: "v2", + Lifecycle: api.PackageRevisionLifecyclePublished, + Revision: "main", + }) + if err != nil { + t.Fatalf("ListPackageRevisions failed: %v", err) + } + + // Expect a single result + if got, want := len(publishedRevisions), 1; got != want { + t.Fatalf("ListPackageRevisions returned %d packages; want %d", got, want) + } + + approvedBucket := publishedRevisions[0] + + if got, want := approvedBucket.Lifecycle(), api.PackageRevisionLifecyclePublished; got != want { + t.Fatalf("Approved Bucket package lifecycle: got %s, want %s", got, want) + } + + err = approvedBucket.UpdateLifecycle(ctx, api.PackageRevisionLifecycleDeletionProposed) + if err != nil { + t.Fatalf("Deletion proposal for approved Bucket failed; %v", err) + } + err = cachedRepo.DeletePackageRevision(ctx, approvedBucket) + if err != nil { + t.Fatalf("Deleting Main packageRevision failed; %v", err) + } + + postDeletePublishedRevisions, err := cachedRepo.ListPackageRevisions(ctx, repository.ListPackageRevisionFilter{ + Package: "catalog/gcp/bucket", + WorkspaceName: "v2", + Lifecycle: api.PackageRevisionLifecyclePublished, + Revision: "main", + }) + + if err != nil { + t.Fatalf("ListPackageRevisions failed: %v", err) + } + + //Expect 0 entries + if got, want := len(postDeletePublishedRevisions), 0; got != want { + t.Fatalf("ListPackageRevisions returned %d packages; want %d", got, want) + } + +} + func openRepositoryFromArchive(t *testing.T, ctx context.Context, testPath, name string) cache.CachedRepository { t.Helper() diff --git a/pkg/cache/memory/draft.go b/pkg/cache/memory/draft.go index aa36435d..1a9066a1 100644 --- a/pkg/cache/memory/draft.go +++ b/pkg/cache/memory/draft.go @@ -17,8 +17,10 @@ package memory import ( "context" + "github.com/nephio-project/porch/api/porch/v1alpha1" "github.com/nephio-project/porch/pkg/cache" "github.com/nephio-project/porch/pkg/repository" + "go.opentelemetry.io/otel/trace" ) type cachedDraft struct { @@ -29,8 +31,40 @@ type cachedDraft struct { var _ repository.PackageDraft = &cachedDraft{} var _ cache.CachedPackageDraft = &cachedDraft{} -func (cd *cachedDraft) Close(ctx context.Context) (repository.PackageRevision, error) { - if closed, err := cd.PackageDraft.Close(ctx); err != nil { +func (cd *cachedDraft) Close(ctx context.Context, version string) (repository.PackageRevision, error) { + ctx, span := tracer.Start(ctx, "cachedDraft::Close", trace.WithAttributes()) + defer span.End() + v, err := cd.cache.Version(ctx) + if err != nil { + return nil, err + } + if v != cd.cache.lastVersion { + _, _, err = cd.cache.refreshAllCachedPackages(ctx) + if err != nil { + return nil, err + } + } + + revisions, err := cd.cache.ListPackageRevisions(ctx, repository.ListPackageRevisionFilter{ + Package: cd.GetName(), + }) + if err != nil { + return nil, err + } + + var publishedRevisions []string + for _, rev := range revisions { + if v1alpha1.LifecycleIsPublished(rev.Lifecycle()) { + publishedRevisions = append(publishedRevisions, rev.Key().Revision) + } + } + + nextVersion, err := repository.NextRevisionNumber(publishedRevisions) + if err != nil { + return nil, err + } + + if closed, err := cd.PackageDraft.Close(ctx, nextVersion); err != nil { return nil, err } else { return cd.cache.update(ctx, closed) diff --git a/pkg/cache/memory/repository.go b/pkg/cache/memory/repository.go index 959182a9..432daeb4 100644 --- a/pkg/cache/memory/repository.go +++ b/pkg/cache/memory/repository.go @@ -28,6 +28,7 @@ import ( "github.com/nephio-project/porch/pkg/repository" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/trace" + "k8s.io/apimachinery/pkg/api/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/watch" @@ -222,13 +223,67 @@ func (r *cachedRepository) update(ctx context.Context, updated repository.Packag r.cachedPackageRevisions[k] = cached // Recompute latest package revisions. - // TODO: Just updated package? identifyLatestRevisions(r.cachedPackageRevisions) - // TODO: Update the latest revisions for the r.cachedPackages + // Create the main package revision + if v1alpha1.LifecycleIsPublished(updated.Lifecycle()) { + updatedMain := updated.ToMainPackageRevision() + r.createMainPackageRevision(ctx, updatedMain) + } else { + version, err := r.repo.Version(ctx) + if err != nil { + return nil, err + } + r.lastVersion = version + } + return cached, nil } +func (r *cachedRepository) createMainPackageRevision(ctx context.Context, updatedMain repository.PackageRevision) error { + + //Search and delete any old main pkgRev of an older workspace in the cache + for pkgRevKey := range r.cachedPackageRevisions { + if (pkgRevKey.Repository == updatedMain.Key().Repository) && (pkgRevKey.Package == updatedMain.Key().Package) && (pkgRevKey.Revision == updatedMain.Key().Revision) { + oldMainKey := repository.PackageRevisionKey{ + Repository: updatedMain.Key().Repository, + Package: updatedMain.Key().Package, + Revision: updatedMain.Key().Revision, + WorkspaceName: v1alpha1.WorkspaceName(string(pkgRevKey.WorkspaceName)), + } + delete(r.cachedPackageRevisions, oldMainKey) + } + } + cachedMain := &cachedPackageRevision{PackageRevision: updatedMain} + r.cachedPackageRevisions[updatedMain.Key()] = cachedMain + + pkgRevMetaNN := types.NamespacedName{ + Name: updatedMain.KubeObjectName(), + Namespace: updatedMain.KubeObjectNamespace(), + } + + // Create the package if it doesn't exist + _, err := r.metadataStore.Get(ctx, pkgRevMetaNN) + if errors.IsNotFound(err) { + pkgRevMeta := meta.PackageRevisionMeta{ + Name: updatedMain.KubeObjectName(), + Namespace: updatedMain.KubeObjectNamespace(), + } + _, err := r.metadataStore.Create(ctx, pkgRevMeta, r.repoSpec.Name, updatedMain.UID()) + if err != nil { + klog.Warningf("unable to create PackageRev CR for %s/%s: %v", + updatedMain.KubeObjectNamespace(), updatedMain.KubeObjectName(), err) + } + } + version, err := r.repo.Version(ctx) + if err != nil { + return err + } + r.lastVersion = version + + return nil +} + func (r *cachedRepository) DeletePackageRevision(ctx context.Context, old repository.PackageRevision) error { // Unwrap unwrapped := old.(*cachedPackageRevision).PackageRevision diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 8ff622a5..cead2a78 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -65,7 +65,7 @@ type CaDEngine interface { ListPackageRevisions(ctx context.Context, repositorySpec *configapi.Repository, filter repository.ListPackageRevisionFilter) ([]*PackageRevision, error) CreatePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, obj *api.PackageRevision, parent *PackageRevision) (*PackageRevision, error) - UpdatePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, oldPackage *PackageRevision, old, new *api.PackageRevision, parent *PackageRevision) (*PackageRevision, error) + UpdatePackageRevision(ctx context.Context, version string, repositoryObj *configapi.Repository, oldPackage *PackageRevision, old, new *api.PackageRevision, parent *PackageRevision) (*PackageRevision, error) DeletePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, obj *PackageRevision) error ListPackages(ctx context.Context, repositorySpec *configapi.Repository, filter repository.ListPackageFilter) ([]*Package, error) @@ -313,7 +313,7 @@ func (cad *cadEngine) CreatePackageRevision(ctx context.Context, repositoryObj * } // Updates are done. - repoPkgRev, err := draft.Close(ctx) + repoPkgRev, err := draft.Close(ctx, "") if err != nil { return nil, err } @@ -507,7 +507,7 @@ func (cad *cadEngine) mapTaskToMutation(ctx context.Context, obj *api.PackageRev } } -func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, repositoryObj *configapi.Repository, oldPackage *PackageRevision, oldObj, newObj *api.PackageRevision, parent *PackageRevision) (*PackageRevision, error) { +func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, version string, repositoryObj *configapi.Repository, oldPackage *PackageRevision, oldObj, newObj *api.PackageRevision, parent *PackageRevision) (*PackageRevision, error) { ctx, span := tracer.Start(ctx, "cadEngine::UpdatePackageRevision", trace.WithAttributes()) defer span.End() @@ -580,7 +580,7 @@ func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, repositoryObj * if err != nil { return nil, err } - repoPkgRev, err := cad.recloneAndReplay(ctx, repo, repositoryObj, newObj, packageConfig) + repoPkgRev, err := cad.recloneAndReplay(ctx, version, repo, repositoryObj, newObj, packageConfig) if err != nil { return nil, err } @@ -681,7 +681,7 @@ func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, repositoryObj * } // Updates are done. - repoPkgRev, err = draft.Close(ctx) + repoPkgRev, err = draft.Close(ctx, version) if err != nil { return nil, err } @@ -694,14 +694,6 @@ func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, repositoryObj * sent := cad.watcherManager.NotifyPackageRevisionChange(watch.Modified, repoPkgRev, pkgRevMeta) klog.Infof("engine: sent %d for updated PackageRevision %s/%s", sent, repoPkgRev.KubeObjectNamespace(), repoPkgRev.KubeObjectName()) - // Refresh Cache after package is approved so that 'main' package rev is available instantly after creation - if repoPkgRev.Lifecycle() == api.PackageRevisionLifecyclePublished { - err := repo.RefreshCache(ctx) - if err != nil { - return nil, err - } - } - return ToPackageRevision(repoPkgRev, pkgRevMeta), nil } @@ -1023,7 +1015,7 @@ func (cad *cadEngine) UpdatePackageResources(ctx context.Context, repositoryObj }}) // No lifecycle change when updating package resources; updates are done. - repoPkgRev, err := draft.Close(ctx) + repoPkgRev, err := draft.Close(ctx, "") if err != nil { return nil, renderStatus, err } @@ -1354,7 +1346,7 @@ func isRecloneAndReplay(oldObj, newObj *api.PackageRevision) bool { // recloneAndReplay performs an update by recloning the upstream package and replaying all tasks. // This is more like a git rebase operation than the "classic" kpt update algorithm, which is more like a git merge. -func (cad *cadEngine) recloneAndReplay(ctx context.Context, repo repository.Repository, repositoryObj *configapi.Repository, newObj *api.PackageRevision, packageConfig *builtins.PackageConfig) (repository.PackageRevision, error) { +func (cad *cadEngine) recloneAndReplay(ctx context.Context, version string, repo repository.Repository, repositoryObj *configapi.Repository, newObj *api.PackageRevision, packageConfig *builtins.PackageConfig) (repository.PackageRevision, error) { ctx, span := tracer.Start(ctx, "cadEngine::recloneAndReplay", trace.WithAttributes()) defer span.End() @@ -1373,7 +1365,7 @@ func (cad *cadEngine) recloneAndReplay(ctx context.Context, repo repository.Repo return nil, err } - return draft.Close(ctx) + return draft.Close(ctx, version) } // ExtractContextConfigMap returns the package-context configmap, if found diff --git a/pkg/engine/fake/packagerevision.go b/pkg/engine/fake/packagerevision.go index 697e9614..713c0b99 100644 --- a/pkg/engine/fake/packagerevision.go +++ b/pkg/engine/fake/packagerevision.go @@ -39,6 +39,13 @@ func (pr *PackageRevision) KubeObjectName() string { return pr.Name } +var _ repository.PackageRevision = &PackageRevision{} + +// ToMainPackageRevision implements repository.PackageRevision. +func (f *PackageRevision) ToMainPackageRevision() repository.PackageRevision { + panic("unimplemented") +} + func (pr *PackageRevision) KubeObjectNamespace() string { return pr.Namespace } diff --git a/pkg/git/draft.go b/pkg/git/draft.go index f5fa62ea..00ec1a8a 100644 --- a/pkg/git/draft.go +++ b/pkg/git/draft.go @@ -16,6 +16,7 @@ package git import ( "context" + "strings" "time" "github.com/go-git/go-git/v5/plumbing" @@ -63,9 +64,15 @@ func (d *gitPackageDraft) UpdateLifecycle(ctx context.Context, new v1alpha1.Pack } // Finish round of updates. -func (d *gitPackageDraft) Close(ctx context.Context) (repository.PackageRevision, error) { +func (d *gitPackageDraft) Close(ctx context.Context, version string) (repository.PackageRevision, error) { ctx, span := tracer.Start(ctx, "gitPackageDraft::Close", trace.WithAttributes()) defer span.End() - return d.parent.CloseDraft(ctx, d) + return d.parent.CloseDraft(ctx, version, d) +} + +func (d *gitPackageDraft) GetName() string { + packageDirectory := d.parent.directory + packageName := strings.TrimPrefix(d.path, packageDirectory+"/") + return packageName } diff --git a/pkg/git/git.go b/pkg/git/git.go index 985085b9..4794acba 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -1480,7 +1480,7 @@ func (r *gitRepository) UpdateDraftResources(ctx context.Context, draft *gitPack return nil } -func (r *gitRepository) CloseDraft(ctx context.Context, d *gitPackageDraft) (*gitPackageRevision, error) { +func (r *gitRepository) CloseDraft(ctx context.Context, version string, d *gitPackageDraft) (*gitPackageRevision, error) { r.mutex.Lock() defer r.mutex.Unlock() @@ -1492,27 +1492,11 @@ func (r *gitRepository) CloseDraft(ctx context.Context, d *gitPackageDraft) (*gi switch d.lifecycle { case v1alpha1.PackageRevisionLifecyclePublished, v1alpha1.PackageRevisionLifecycleDeletionProposed: - // Finalize the package revision. Assign it a revision number of latest + 1. - packageDirectory := d.parent.directory - packageName := strings.TrimPrefix(d.path, packageDirectory+"/") - revisions, err := r.listPackageRevisions(ctx, repository.ListPackageRevisionFilter{ - Package: packageName, - }) - if err != nil { - return nil, err - } - var revs []string - for _, rev := range revisions { - if v1alpha1.LifecycleIsPublished(r.getLifecycle(ctx, rev)) { - revs = append(revs, rev.Key().Revision) - } - } - - d.revision, err = repository.NextRevisionNumber(revs) - if err != nil { - return nil, err + if version == "" { + return nil, errors.New("Version cannot be empty for the next package revision") } + d.revision = version // Finalize the package revision. Commit it to main branch. commitHash, newTreeHash, commitBase, err := r.commitPackageToMain(ctx, d) diff --git a/pkg/git/git_test.go b/pkg/git/git_test.go index 46c315cb..1578caeb 100644 --- a/pkg/git/git_test.go +++ b/pkg/git/git_test.go @@ -180,7 +180,7 @@ func (g GitSuite) TestGitPackageRoundTrip(t *testing.T) { t.Fatalf("draft.UpdateResources(%#v, %#v) failed: %v", newResources, task, err) } - revision, err := draft.Close(ctx) + revision, err := draft.Close(ctx, "v1") if err != nil { t.Fatalf("draft.Close() failed: %v", err) } @@ -207,7 +207,7 @@ func (g GitSuite) TestGitPackageRoundTrip(t *testing.T) { if err := update.UpdateLifecycle(ctx, v1alpha1.PackageRevisionLifecyclePublished); err != nil { t.Fatalf("UpdateLifecycle failed: %v", err) } - approved, err := update.Close(ctx) + approved, err := update.Close(ctx, "v1") if err != nil { t.Fatalf("Close() of %q, %q failed: %v", packageName, workspace, err) } @@ -418,7 +418,7 @@ func (g GitSuite) TestListPackagesTrivial(t *testing.T) { }); err != nil { t.Fatalf("UpdateResources() failed: %v", err) } - newRevision, err := draft.Close(ctx) + newRevision, err := draft.Close(ctx, "") if err != nil { t.Fatalf("draft.Close() failed: %v", err) } @@ -506,7 +506,7 @@ func (g GitSuite) TestCreatePackageInTrivialRepository(t *testing.T) { }); err != nil { t.Fatalf("UpdateResources() failed: %v", err) } - newRevision, err := draft.Close(ctx) + newRevision, err := draft.Close(ctx, "") if err != nil { t.Fatalf("draft.Close() failed: %v", err) } @@ -688,7 +688,7 @@ func (g GitSuite) TestApproveDraft(t *testing.T) { update.UpdateLifecycle(ctx, v1alpha1.PackageRevisionLifecyclePublished) - new, err := update.Close(ctx) + new, err := update.Close(ctx, "v1") if err != nil { t.Fatalf("Close failed: %v", err) } @@ -750,7 +750,7 @@ func (g GitSuite) TestApproveDraftWithHistory(t *testing.T) { update.UpdateLifecycle(ctx, v1alpha1.PackageRevisionLifecyclePublished) - new, err := update.Close(ctx) + new, err := update.Close(ctx, "v1") if err != nil { t.Fatalf("Close failed: %v", err) } diff --git a/pkg/git/package.go b/pkg/git/package.go index b23c05f0..c39962d6 100644 --- a/pkg/git/package.go +++ b/pkg/git/package.go @@ -219,6 +219,29 @@ func (p *gitPackageRevision) GetResources(ctx context.Context) (*v1alpha1.Packag }, nil } +// Creates a gitPackageRevision reference that is acting as the main branch package revision. +// It doesn't do any git operations, so this package should only be used if the actual git updates +// were exectued on the main branch. +func (p *gitPackageRevision) ToMainPackageRevision() repository.PackageRevision { + //Need to compute a separate reference, otherwise the ref will be the same as the versioned package, + //while the main gitPackageRevision needs to point at the main branch. + mainBranchRef := plumbing.NewHashReference(p.repo.branch.RefInLocal(), p.commit) + p1 := &gitPackageRevision{ + repo: p.repo, + path: p.path, + revision: string(p.repo.branch), + workspaceName: p.workspaceName, + updated: p.updated, + updatedBy: p.updatedBy, + ref: mainBranchRef, + tree: p.tree, + commit: p.commit, + tasks: p.tasks, + } + return p1 + +} + func (p *gitPackageRevision) GetKptfile(ctx context.Context) (kptfile.KptFile, error) { resources, err := p.repo.GetResources(p.tree) if err != nil { diff --git a/pkg/oci/mutate.go b/pkg/oci/mutate.go index e25b9716..adbf9719 100644 --- a/pkg/oci/mutate.go +++ b/pkg/oci/mutate.go @@ -195,8 +195,12 @@ func (p *ociPackageDraft) UpdateLifecycle(ctx context.Context, new v1alpha1.Pack return nil } +func (p *ociPackageDraft) GetName() string { + return p.packageName +} + // Finish round of updates. -func (p *ociPackageDraft) Close(ctx context.Context) (repository.PackageRevision, error) { +func (p *ociPackageDraft) Close(ctx context.Context, version string) (repository.PackageRevision, error) { ctx, span := tracer.Start(ctx, "ociPackageDraft::Close", trace.WithAttributes()) defer span.End() diff --git a/pkg/oci/oci.go b/pkg/oci/oci.go index 47c0dd68..e43157fe 100644 --- a/pkg/oci/oci.go +++ b/pkg/oci/oci.go @@ -244,6 +244,11 @@ func (r *ociRepository) buildPackageRevision(ctx context.Context, name oci.Image return p, nil } +// ToMainPackageRevision implements repository.PackageRevision. +func (p *ociPackageRevision) ToMainPackageRevision() repository.PackageRevision { + panic("unimplemented") +} + type ociPackageRevision struct { digestName oci.ImageDigestName packageName string diff --git a/pkg/registry/porch/packagecommon.go b/pkg/registry/porch/packagecommon.go index 9a6c6254..cb2f66af 100644 --- a/pkg/registry/porch/packagecommon.go +++ b/pkg/registry/porch/packagecommon.go @@ -309,7 +309,7 @@ func (r *packageCommon) updatePackageRevision(ctx context.Context, name string, } if !isCreate { - rev, err := r.cad.UpdatePackageRevision(ctx, &repositoryObj, oldRepoPkgRev, oldApiPkgRev.(*api.PackageRevision), newApiPkgRev, parentPackage) + rev, err := r.cad.UpdatePackageRevision(ctx, "", &repositoryObj, oldRepoPkgRev, oldApiPkgRev.(*api.PackageRevision), newApiPkgRev, parentPackage) if err != nil { return nil, false, apierrors.NewInternalError(err) } diff --git a/pkg/repository/repository.go b/pkg/repository/repository.go index 68837045..96e21c8e 100644 --- a/pkg/repository/repository.go +++ b/pkg/repository/repository.go @@ -93,6 +93,8 @@ type PackageRevision interface { // ResourceVersion returns the Kube resource version of the package ResourceVersion() string + + ToMainPackageRevision() PackageRevision } // Package is an abstract package. @@ -117,7 +119,8 @@ type PackageDraft interface { // Updates desired lifecycle of the package. The lifecycle is applied on Close. UpdateLifecycle(ctx context.Context, new v1alpha1.PackageRevisionLifecycle) error // Finish round of updates. - Close(ctx context.Context) (PackageRevision, error) + Close(ctx context.Context, version string) (PackageRevision, error) + GetName() string } // ListPackageRevisionFilter is a predicate for filtering PackageRevision objects;