From 34cf103c9e32f94b88ea62b76e4e4c5410e7f9b4 Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Wed, 11 Oct 2023 16:38:20 -0500 Subject: [PATCH] fix: deletionProposed branch throws error for `main` revisions (#4056) --- porch/pkg/git/git.go | 14 ++---- porch/test/e2e/e2e_test.go | 100 +++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 9 deletions(-) diff --git a/porch/pkg/git/git.go b/porch/pkg/git/git.go index e89582ac5..a4b08caf0 100644 --- a/porch/pkg/git/git.go +++ b/porch/pkg/git/git.go @@ -426,7 +426,7 @@ func (r *gitRepository) DeletePackageRevision(ctx context.Context, old repositor refSpecs.AddRefToDelete(ref) // If this revision was proposed for deletion, we need to delete the associated branch. - if err := r.removeDeletionProposedBranchIfExists(ctx, oldGit.path, oldGit.revision, oldGit.commit); err != nil { + if err := r.removeDeletionProposedBranchIfExists(ctx, oldGit.path, oldGit.revision); err != nil { return err } @@ -443,7 +443,7 @@ func (r *gitRepository) DeletePackageRevision(ctx context.Context, old repositor // Remove the proposed for deletion branch. We end up here when users // try to delete the main branch version of a packagerevision. - if err := r.removeDeletionProposedBranchIfExists(ctx, oldGit.path, oldGit.revision, oldGit.commit); err != nil { + if err := r.removeDeletionProposedBranchIfExists(ctx, oldGit.path, oldGit.revision); err != nil { return err } @@ -461,18 +461,14 @@ func (r *gitRepository) DeletePackageRevision(ctx context.Context, old repositor return nil } -func (r *gitRepository) removeDeletionProposedBranchIfExists(ctx context.Context, path, revision string, commit plumbing.Hash) error { +func (r *gitRepository) removeDeletionProposedBranchIfExists(ctx context.Context, path, revision string) error { refSpecsForDeletionProposed := newPushRefSpecBuilder() deletionProposedBranch := createDeletionProposedName(path, revision) - refSpecsForDeletionProposed.AddRefToDelete(plumbing.NewHashReference(deletionProposedBranch.RefInLocal(), commit)) + refSpecsForDeletionProposed.AddRefToDelete(plumbing.NewHashReference(deletionProposedBranch.RefInLocal(), plumbing.ZeroHash)) if err := r.pushAndCleanup(ctx, refSpecsForDeletionProposed); err != nil { - if strings.HasPrefix(err.Error(), - fmt.Sprintf("remote ref %s%s required to be", branchPrefixInRemoteRepo, deletionProposedBranch)) && - strings.HasSuffix(err.Error(), "but is absent") { - + if errors.Is(err, git.NoErrAlreadyUpToDate) { // the deletionProposed branch might not have existed, so we ignore this error klog.Warningf("branch %s does not exist", deletionProposedBranch) - } else { klog.Errorf("unexpected error while removing deletionProposed branch: %v", err) return err diff --git a/porch/test/e2e/e2e_test.go b/porch/test/e2e/e2e_test.go index e751d21fe..aa939014d 100644 --- a/porch/test/e2e/e2e_test.go +++ b/porch/test/e2e/e2e_test.go @@ -1115,6 +1115,106 @@ func (t *PorchSuite) TestDeleteAndRecreate(ctx context.Context) { t.Logf("successfully recreated package revision %q", packageName) } +func (t *PorchSuite) TestDeleteFromMain(ctx context.Context) { + const ( + repository = "delete-main" + packageNameFirst = "test-delete-main-first" + packageNameSecond = "test-delete-main-second" + workspace = "workspace" + ) + + // Register the repository + t.registerMainGitRepositoryF(ctx, repository) + + // Create the first draft package + createdFirst := t.createPackageDraftF(ctx, repository, packageNameFirst, workspace) + + // Check the package exists + var pkgFirst porchapi.PackageRevision + t.mustExist(ctx, client.ObjectKey{Namespace: t.namespace, Name: createdFirst.Name}, &pkgFirst) + + // Propose the package revision to be finalized + pkgFirst.Spec.Lifecycle = porchapi.PackageRevisionLifecycleProposed + t.UpdateF(ctx, &pkgFirst) + + pkgFirst.Spec.Lifecycle = porchapi.PackageRevisionLifecyclePublished + t.UpdateApprovalF(ctx, &pkgFirst, metav1.UpdateOptions{}) + + t.mustExist(ctx, client.ObjectKey{Namespace: t.namespace, Name: createdFirst.Name}, &pkgFirst) + + // Create the second draft package + createdSecond := t.createPackageDraftF(ctx, repository, packageNameSecond, workspace) + + // Check the package exists + var pkgSecond porchapi.PackageRevision + t.mustExist(ctx, client.ObjectKey{Namespace: t.namespace, Name: createdSecond.Name}, &pkgSecond) + + // Propose the package revision to be finalized + pkgSecond.Spec.Lifecycle = porchapi.PackageRevisionLifecycleProposed + t.UpdateF(ctx, &pkgSecond) + + pkgSecond.Spec.Lifecycle = porchapi.PackageRevisionLifecyclePublished + t.UpdateApprovalF(ctx, &pkgSecond, metav1.UpdateOptions{}) + + t.mustExist(ctx, client.ObjectKey{Namespace: t.namespace, Name: createdSecond.Name}, &pkgSecond) + + // We need to wait for the sync for the "main" revisions to get created + time.Sleep(75 * time.Second) + + var list porchapi.PackageRevisionList + t.ListE(ctx, &list, client.InNamespace(t.namespace)) + + var firstPkgRevFromMain porchapi.PackageRevision + var secondPkgRevFromMain porchapi.PackageRevision + + for _, pkgrev := range list.Items { + if pkgrev.Spec.PackageName == packageNameFirst && pkgrev.Spec.Revision == "main" { + firstPkgRevFromMain = pkgrev + } + if pkgrev.Spec.PackageName == packageNameSecond && pkgrev.Spec.Revision == "main" { + secondPkgRevFromMain = pkgrev + } + } + + // Propose deletion of both main packages + firstPkgRevFromMain.Spec.Lifecycle = porchapi.PackageRevisionLifecycleDeletionProposed + t.UpdateApprovalF(ctx, &firstPkgRevFromMain, metav1.UpdateOptions{}) + secondPkgRevFromMain.Spec.Lifecycle = porchapi.PackageRevisionLifecycleDeletionProposed + t.UpdateApprovalF(ctx, &secondPkgRevFromMain, metav1.UpdateOptions{}) + + // Delete the first package revision from main + t.DeleteE(ctx, &porchapi.PackageRevision{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: t.namespace, + Name: firstPkgRevFromMain.Name, + }, + }) + + // We need to wait for the sync + time.Sleep(75 * time.Second) + + // Delete the second package revision from main + t.DeleteE(ctx, &porchapi.PackageRevision{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: t.namespace, + Name: secondPkgRevFromMain.Name, + }, + }) + + // Propose and delete the original package revisions (cleanup) + t.ListE(ctx, &list, client.InNamespace(t.namespace)) + for _, pkgrev := range list.Items { + pkgrev.Spec.Lifecycle = porchapi.PackageRevisionLifecycleDeletionProposed + t.UpdateApprovalF(ctx, &pkgrev, metav1.UpdateOptions{}) + t.DeleteE(ctx, &porchapi.PackageRevision{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: t.namespace, + Name: pkgrev.Name, + }, + }) + } +} + func (t *PorchSuite) TestCloneLeadingSlash(ctx context.Context) { const ( repository = "clone-ls"