Repository: GoogleContainerTools/kpt
Pull request #3355 (2022-07-09 02:26:01)

recreating package revision only needs to walk back to the clone/init task
Fixes https://github.com/GoogleContainerTools/kpt/issues/3331





natasha41575: > Code looks good, but can we add a test for this?

Added

Patch:
diff --git a/porch/pkg/git/git.go b/porch/pkg/git/git.go
index 211df89d..9212ed12 100644
--- a/porch/pkg/git/git.go
+++ b/porch/pkg/git/git.go
@@ -827,16 +827,30 @@ func (r *gitRepository) loadTasks(ctx context.Context, startCommit *object.Commi
 
 		for _, gitAnnotation := range gitAnnotations {
 			if gitAnnotation.PackagePath == packagePath && gitAnnotation.Revision == revision {
+				// We are iterating through the commits in reverse order.
+				// Tasks that are read from separate commits will be recorded in
+				// reverse order.
+				// The entire `tasks` slice will get reversed later, which will give us the
+				// tasks in chronological order.
 				if gitAnnotation.Task != nil {
 					tasks = append(tasks, *gitAnnotation.Task)
 				}
-				// Iterate in reverse order here. The reason is that tasks in
-				// separate commits end up in reverse order while we include
-				// multiple tasks in a single commit in the actual order.
+
+				// This case handles multiple tasks that appear in a single commit, which
+				// appear in chronological order within the gitAnnotation.
+				// The entire `tasks` slice will get reversed later, so in order for the tasks
+				// to stay in chronological order, we iterate through them in reverse order.
 				for i := len(gitAnnotation.Tasks) - 1; i >= 0; i-- {
 					t := gitAnnotation.Tasks[i]
 					tasks = append(tasks, *t)
 				}
+
+				lastTask := tasks[len(tasks)-1]
+				if lastTask.Type == v1alpha1.TaskTypeClone || lastTask.Type == v1alpha1.TaskTypeInit {
+					// we have reached the beginning of this package revision and don't need to
+					// continue further
+					break
+				}
 			}
 		}
 


Test patch:
diff --git a/porch/test/e2e/e2e_test.go b/porch/test/e2e/e2e_test.go
index 3645e9c2..71b7272a 100644
--- a/porch/test/e2e/e2e_test.go
+++ b/porch/test/e2e/e2e_test.go
@@ -731,6 +731,61 @@ func (t *PorchSuite) TestDeleteFinal(ctx context.Context) {
 	t.mustNotExist(ctx, &pkg)
 }
 
+func (t *PorchSuite) TestDeleteAndRecreate(ctx context.Context) {
+	const (
+		repository  = "delete-and-recreate"
+		packageName = "test-delete-and-recreate"
+		revision    = "v1"
+	)
+
+	// Register the repository
+	t.registerMainGitRepositoryF(ctx, repository)
+
+	// Create a draft package
+	created := t.createPackageDraftF(ctx, repository, packageName, revision)
+
+	// Check the package exists
+	var pkg porchapi.PackageRevision
+	t.mustExist(ctx, client.ObjectKey{Namespace: t.namespace, Name: created.Name}, &pkg)
+
+	// Propose the package revision to be finalized
+	pkg.Spec.Lifecycle = porchapi.PackageRevisionLifecycleProposed
+	t.UpdateF(ctx, &pkg)
+
+	pkg.Spec.Lifecycle = porchapi.PackageRevisionLifecyclePublished
+	t.UpdateApprovalF(ctx, &pkg, metav1.UpdateOptions{})
+
+	t.mustExist(ctx, client.ObjectKey{Namespace: t.namespace, Name: created.Name}, &pkg)
+
+	// Delete the package
+	t.DeleteE(ctx, &porchapi.PackageRevision{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: t.namespace,
+			Name:      created.Name,
+		},
+	})
+
+	t.mustNotExist(ctx, &pkg)
+
+	// Recreate the package with the same name and revision
+	created = t.createPackageDraftF(ctx, repository, packageName, revision)
+
+	// Check the package exists
+	t.mustExist(ctx, client.ObjectKey{Namespace: t.namespace, Name: created.Name}, &pkg)
+
+	// Ensure that there is only one init task in the package revision history
+	foundInitTask := false
+	for _, task := range pkg.Spec.Tasks {
+		if task.Type == porchapi.TaskTypeInit {
+			if foundInitTask {
+				t.Fatalf("found two init tasks in recreated package revision")
+			}
+			foundInitTask = true
+		}
+	}
+	t.Logf("successfully recreated package revision %q", packageName)
+}
+
 func (t *PorchSuite) TestCloneLeadingSlash(ctx context.Context) {
 	const (
 		repository  = "clone-ls"